[cfe-dev] ParamDecls missing from function DeclContext

[Abramo Bagnara]

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"...)