[cfe-dev] PATCH: Cleanup function redeclaration representations

[Argiris Kirtzidis]

Hi Doug,

Doug Gregor wrote:
>>> So, if name lookup is going to continue to find the first declaration
>>> of a function (as it does in your suggestion and in the patch I
>>> committed), that first declaration either needs to accumulate
>>> information from redeclarations or access to its internals needs to
>>> walk through the chain of redeclarations, e.g., to find default
>>> arguments.
>>>
>>>
>>>       
>>  Yes, that's what I'm proposing. The first decl accumulates the information
>> and/or walks the redeclarations for info.
>>  Like the getBody() in your patch that walks the redeclarations to get the
>> body.
>>     
>
> There's a lot of walking to do, here. We'll need to walk the
> redeclaration chain to get parameter names, default arguments, the
> return type, the source location, etc., because we want to use the
> most recent redeclaration for diagnostic purposes. For example:
>
>   void f(int, int);
>   void f(int, int y = 5);
>
>   void g() {
>     f(); // error: not enough arguments in call; should refer to the
> most recent "f" declaration
>   }
>   

Are you saying that the most recent declaration is always best to use 
for diagnostic purposes or are you talking about the specific example ?
What if it was:

  void f(int, int y = 5);
  void f(int, int);


Which decl should the diagnostic use ?

>>> There's some philosophy behind my approach to function redeclarations:
>>> I want "normal" users of the AST to only see a single FunctionDecl for
>>> each function, and that FunctionDecl should contain all of the
>>> information that the AST contains about that function.
>>>
>>>       
>>  I agree. The only difference is in case of
>>
>>
>>   void f(int x); // #1
>>   void f(int x = 2); // #2
>>
>>
>>  instead of merging to #2 and swaping contents with #1, thus pointer of #1
>> now points to #2,
>>  I suggest merging to #1.
>>  In both cases, every use of 'f' will point to the same FunctionDecl node.
>>     
>
> Ah, I understand what you mean. I think the important question is: how
> much do you merge into #1, and do you preserve the previous
> information in #1? Does #1 get the new parameter names from #2? New
> typedefs in the parameter types? What about the source location?
>
>   void f(int x); // #1
>   typedef int MyFlags;
>   void f(MyFlags flags = 0); // #2
>
> What parts of the declaration at #2 will go into the FunctionDecl for
> #1? Additionally: when information gets merged into #1, is that same
> information that was in the FunctionDecl for #1 lost? (e.g., the
> parameter name "x" it used?)
>   

These are questions that apply to either case of merging:

  typedef int MyFlags;
  void f(MyFlags flags = 0); // #1
  void f(int x); // #2


What parts of the declaration at #1 will go into the FunctionDecl for
#2? Additionally: when information gets merged into #2, is that same
information that was in the FunctionDecl for #2 lost? (e.g., the
parameter name "x" it used?)


>>  That way ASTConsumers like the syntax-colorizer I mentioned and
>> pretty-printer (SerializationTest uses this one), can use redeclarations the
>> same way
>>  as single declarations. If a ASTConsumer cares only about single decls, it
>> can check a decl.isRedeclaration() method and act accordingly.
>>     
>
> We can design the interface either way... a syntax-colorizer could
> just override HandleRedeclaration to call the same routine that deals
> with single declarations, because it treats them the same. Other tools
> can handle redeclarations differently.
>   

The difference is that with 'swaping contents' the consumer cannot 
assume that it will get different pointers for different decl objects.
SerializationTest pushes the decl pointers into a vector to feed them 
later to another consumer. With 'swaping contents' it gets
the same pointer for each redeclaration. This translates to feeding the 
same decl multiple times.
This is what I mean about complicating things with the 'swaping content' 
bit.


-Argiris