Re: [PATCH 1/3] fort_dyn_array: Enable dynamic member types inside a structure.

["Heckel, Bernhard" <bernhard.heckel@intel.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8"; format="flowed", Size: 4446 bytes --]

On 05/04/2016 12:46, Yao Qi wrote:
> "Heckel, Bernhard" <bernhard.heckel@intel.com> writes:
>
>> Actually, when resolving dynamic types I don't use the valaddr. From
>> what I understood
>> I could even return if valaddr is not Null as the TYPE should already
>> be resolved at that time.
>> But I was unsure about it and kept the code.
> OK, that is a separate issue.
>
>>>>          pinfo.next = addr_stack;
>>>>            TYPE_FIELD_TYPE (resolved_type, i)
>>>> @@ -2090,8 +2091,13 @@ resolve_dynamic_struct (struct type *type,
>>>>    	resolved_type_bit_length = new_bit_length;
>>>>        }
>>>>    -  TYPE_LENGTH (resolved_type)
>>>> -    = (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR_BIT;
>>>> +  /* Type length won't change for fortran. Keep what we got from DWARF.
>>> Two spaces after ".".  Multiple instances of this problem.
>>>
>>>> +     Dynamic fields might change over time but not the struct definition.
>>>> +     If we would adapt it we run into problems when
>>>> +     calculating the element offset for arrays of structs.  */
>>> What is the problem we run into?
>> Imagine we have a dynamic allocatable array as a member of a
>> structure. The size of the dynamic array can vary over time.
>> When we have resolved this allocatable array we don't want to add it's
>> length to the structure length it belongs to.
>> Dynamic members are not embedded in the structure itself. Only the
>> description of the dynamic type is embedded
>> and the size of the description (address, bounds,...) won't change.
> That is a FORTRAN specific feature.  The structure's size changes when the
> dynamic array's size changes in C.  I am not 100% sure about Ada, but I
> suspect it is the same as C in this aspect after I read
> https://sourceware.org/ml/gdb-patches/2014-05/msg00522.html
>
> Could you rewrite the comments to say the length of type won't change
> for FORTRAN, but the length changes for C (and Ada?).
Sure, I will add this to the next patch series.
>
>>>>    -\f
>>>>    /* Access to the value history.  */
>>>>      /* Record a new value in the value history.
>>>> @@ -2416,6 +2428,12 @@ set_internalvar (struct internalvar *var, struct value *val)
>>>>    	 call error () until new_data is installed into the var->u to avoid
>>>>    	 leaking memory.  */
>>>>          release_value (new_data.value);
>>>> +
>>>> +      /* Internal variables which are created from values with a dynamic location
>>>> +         don't need the location property of the origin anymore.
>>>> +         Remove the location property in case it exist.  */
>>>> +      remove_dyn_prop(DYN_PROP_DATA_LOCATION, value_type(new_data.value));
>>> Space is needed before "(".  What is wrong if we don't do so?  Do you
>>> have a test case for this?
>> An internal variable has it's own valaddr to where the copy is located.
>> If we keep the dynamic location from the origin then this
>> dyn. location will be used to set
>> the value component location.
>> As the internal variable is a copy of an value at a certain point in
>> time I prefer to get rid off the
>> dynamic location attribute then to do some "if else if else"
>> constructs when setting the component location.
> We remove the dynamic properties from type A for internal variable, but
> there is another variable is still using type A.  These dynamic
> properties can't be found any more.  Is it a problem?
This should not be a  problem as for all dynamic types we do first a 
copy of the type before we start to resolve it.
Therefore, next time when we print variables from the target we have to 
resolve it's type again (as we resolved only the copy before).
>> There are tests:
>> gdb.fortran/vla-value.exp: print $myvar set to vla1
>> gdb.fortran/vla-value.exp: print $myvar(3,6,9)
> but they are PASS in mainline.
>
I handle with this patch the dynamic location property in the 
set_value_component_location and set_value_address doesn't allow 
internal variables.
In order to let the test continue to pass I have to remove the location 
property or add an if - else construct to set_value_component_location.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
\x16º&Öéj×!zÊÞ¶êç×};Óyb²Ö«r\x18\x1dn–­r\x17¬