[cfe-dev] Patch for C++ namespaces

[Chris Lattner]

On Apr 17, 2008, at 8:26 AM, Argiris Kirtzidis wrote:
> Chris Lattner wrote:
>> What is Scope::NamespaceScope needed for?  It controls the setting  
>> of NamespaceParent, but I don't see any uses of it.
>
> I was going to use it later but I could use "getFnParent()- 
> >getParent()" for this. Shall I remove it ?

Yes please.

>>  Doesn't the notion of context subsume this?
>
> Note that there's a subtle difference between Scope and DeclContext,  
> i.e:
>
> namespace A {
> class C {
>   void f();
> }
> }
>
> void A::C::f() {
> int x;
> }
>
> For 'x', the Scope chain would be
> TU scope -> A::C::f() method -> x
>
> while the DeclContext chain would be
> TU -> A namespace -> C class -> f() method -> x
>
> This is how I see it, does this sound reasonable ?

Yep, this makes perfect sense to me.

>> +  // For extended namespace definitions:
>> +  //
>> +  // namespace A { int x; }
>> +  // namespace A { int y; }
>> +  //
>> +  // there will be one NamespaceDecl for each declaration.
>> +  // NextDeclarator points to the next extended declaration.
>>
>> I'm a bit concerned about this.  Particularly when DeclContext  
>> eventually contains a list of all the decls corresponding to its  
>> context, I would expect one context for 'A' and that context should  
>> eventually have both x and y in it.  This example is a bit similar  
>> to "extern int x; int x;" which is two decls but are merged to only  
>> refer to one.  Is there a reason to keep these namespaces as two  
>> separate namespaces?


> Actually it works similar to your example:
>
> extern int x;
> int x;
>
> For the above, there are two decls created and reported at  
> ASTConsumer::HandleTopLevelDecl, but they are merged and x refers to  
> the second.
>
> For:
> namespace A { int x; }
> namespace A { int y; }
>
> There are two NamespaceDecls created and reported at  
> ASTConsumer::HandleTopLevelDecl, but 'A' refers only to the first  
> one (the 'original' namespace).
> And while there's no list of decls associated with NamespaceDecls  
> yet, the intention is that the 'original namespace' will contain the  
> list (y will be associated with the first NamespaceDecl).

Ok.

> By reporting 2 namespace decls, the AST maps to the source code  
> better, and a refactoring tool would be able to do (for example)  
> "rename all 'A' namespaces" by traversing the AST.
>
> If this sound reasonable, how about adding a OriginalNamespace  
> member to NamespaceDecl so that extended namespace definition decls  
> point to the 'original' one?

I think we should tackle this in a more systematic way.  All "merging"  
of decls should be treated the same.  It really would be nice to be  
able to walk all declarations of some decl.

The best way to implement this (in terms of space efficiency) is an on- 
the-side DenseMap<Decl*,Decl*> which would implement a linked list of  
decls.  Then each decl can just have one bit that says whether it has  
an entry in the map (i.e. whether there is a 'next').  This can be  
hidden around behind an accessor, just like the attribute stuff is.   
Does this seem reasonable?

-Chris