[cfe-dev] Patch for C++ namespaces

[Chris Lattner]

On Apr 15, 2008, at 11:04 AM, Argiris Kirtzidis wrote:
> The attached patch implements parsing of C++ namespaces:

woo!  This is great.

>
>
> -Added NamespaceDecl for the AST
> -Added NamespaceScope flag in the Scope class
> -Added checks for name clashes between namespaces and tag/normal  
> declarations.


Ok, a few questions below.

> Note that proper name lookup for namespaces is not implemented,  
> (i.e) :
>
> namespace A {
> int x;
> }
>
> namespace A {
> void f() { x=0; }
> }
>
> I'll add it in a separate patch.

Sounds good.  I'm all for incremental patches :)


Some questions:

What is Scope::NamespaceScope needed for?  It controls the setting of  
NamespaceParent, but I don't see any uses of it.  Doesn't the notion  
of context subsume this?


+  // 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?

-Chris