[cfe-dev] ParamDecls missing from function DeclContext

[Abramo Bagnara]

Il 16/05/2012 17:57, Douglas Gregor ha scritto:
> 
> On May 16, 2012, at 2:23 AM, Abramo Bagnara wrote:
> 
>> Il 16/05/2012 07:11, Douglas Gregor ha scritto:
>>>
>>> On May 15, 2012, at 4:59 PM, Richard Smith wrote:
>>>
>>>> On Tue, May 15, 2012 at 10:55 AM, Douglas Gregor <dgregor at apple.com
>>>> <mailto:dgregor at apple.com>> wrote:
>>>>
>>>>
>>>>    On May 15, 2012, at 8:04 AM, Abramo Bagnara
>>>>    <abramo.bagnara at gmail.com <mailto:abramo.bagnara at gmail.com>> wrote:
>>>>
>>>>> Il 15/05/2012 16:36, Douglas Gregor ha scritto:
>>>>>>
>>>>>> On May 15, 2012, at 12:40 AM, Abramo Bagnara wrote:
>>>>>>
>>>>>>> Another much related issue is shown by the following:
>>>>>>>
>>>>>>> void sort(int (*compare)(int x, int y), int* x);
>>>>>>>
>>>>>>> Which should be the correct context of ParmVarDecl's inside
>>>>>>> FunctionType (i.e. int x and int y)?
>>>>>>
>>>>>>
>>>>>> The DeclContext notion breaks down a little bit here, because we
>>>>>> don't create a context for FunctionTypes (and shouldn't). I'd
>>>>    rather
>>>>>> that the inner ParmVarDecls be in the 'sort's FunctionDecl.
>>>>>
>>>>> Also if I don't have any problem with this I'd like to
>>>>    understand why
>>>>> you think that FunctionTypeLoc should not have a pointer to a
>>>>> DeclContext where the args are registered (that for FunctionDecl
>>>>> function type is the very same FunctionDecl).
>>>>
>>>>    I'd rather not spend any storage on a DeclContext* in the
>>>>    FunctionTypeLoc.
>>>>
>>>>> Apart this issue we have added to our AST visitors an experimental
>>>>> assertion to verify that if the DeclContext for declaration D is DC,
>>>>> then the DeclContext DC indeed contains declaration D.
>>>>>
>>>>> We get tons of assertion failures (e.g. for FunctionDecl or
>>>>> CXXRecordDecl inside a template declaration).
>>>>>
>>>>> We'd like to understand if this is unexpected (and in general we
>>>>    should
>>>>> always have that if D points to DC then DC should contains D) or
>>>>    if we
>>>>> are equivocating the overall design of DeclContext's.
>>>>
>>>>
>>>>    For non-function, non-block DeclContext, this is an important
>>>>    invariant.
>>>>
>>>>    For function and block contexts, it doesn't matter: all of the
>>>>    declarations will be reachable via other means, because they are
>>>>    part of the declaration or one of the statements in the
>>>>    definition. If we're to do any work in the area of DeclContext for
>>>>    functions and blocks, I would much rather make them simpler---by
>>>>    eliminating both the lookup tables and the list of lexical
>>>>    declarations---and save the storage, rather than making them meet
>>>>    an invariant that doesn't seem to have any benefits.
>>>>
>>>>
>>>> The lookup tables for functions were removed by the work which Nick
>>>> and I did on DeclContext name lookup in March, except for local
>>>> function declarations, which don't seem to use the Scope mechanism for
>>>> name lookup. We didn't dig deeply into that issue at the time, but it
>>>> would need to be fixed before we could abandon the lexical decl chains
>>>> for functions entirely.
>>>
>>> Local function declarations definitely need fixing. IIRC, they still
>>> have the wrong semantic declaration context (as do local extern
>>> variables). And if we fix them, we can refactor just a little to kill
>>> off that LookupPtr in FunctionDecl's DeclContext (e.g., by splitting
>>> DeclContext into DeclContext and SearchableDeclContext).
>>>
>>>> That said, the invariant that the decls list on a DeclContext is
>>>> exactly the list of Decls which have that DC as their lexical context
>>>> would be useful. In particular, it could be useful for some clients to
>>>> guarantee that traversing Decls by recursively iterating all lexical
>>>> decls in all DCs would, in fact, reach all Decls.
>>>
>>> Okay, I agree that this is a useful general invariant.
>>
>> Everything is very nice in this way and we'd love to do in this way, but
>> this contrasts with
>>
>>>> Abramo wrote:
>>>> Also if I don't have any problem with this I'd like to understand why
>>>> you think that FunctionTypeLoc should not have a pointer to a
>>>> DeclContext where the args are registered (that for FunctionDecl
>>>> function type is the very same FunctionDecl).
>>
>>> Doug answer:
>>> I'd rather not spend any storage on a DeclContext* in the FunctionTypeLoc.
>>
>> Suppose we have:
>> typedef int f(int x);
>>
>> We should assign to int x a lexical DeclContext, suppose we choose to
>> assign it to the TranslationUnit... then to satisfy the invariant above
>> we should insert the ParmVarDecl in the TranslationUnit.
>>
>> I really doubt this is sane choice... what happens to all visitors of
>> DeclContext when they encounter these ParmVarDecl (that I'd say are
>> definitely "misplaced"...)
> 
> 
> The C(++) languages don't actually place such declarations in any
> particular context, so it's not clear where they belong in the AST.
> Introducing some kind of abstract function declaration that is the
> DeclContext might conceivably work, but that's a rather heavyweight
> solution just to maintain an invariant. Using FunctionTypeLoc doesn't
> really work, because DeclContexts are supposed to be Decls.

Just to be clear, I was not thinking to transform FunctionTypeLoc in a
DeclContext, but to put a DeclContext* in FunctionTypeLoc data.

This pointer points to FunctionDecl itself when the FunctionTypeLoc is
the type of a FunctionDecl, otherwise it points to a specific class
derived from DeclContext (FunctionTypeDeclContext?) that contains only
the ParmVarDecl's.

> Personally, I'm fine with having parameters in the lexical
> DeclContext of the translation unit for this case, but we can investigate other designs.

How you would skip them?

What about template instantiation of these? Everything would be added to
TranslationUnit?

Perhaps I'm the only to feel this way, but to add Decls to contexts
where they don't belong sincerely appears like a little mess to me...