[cfe-dev] Embyonic patch for type-alias

[Douglas Gregor]

On Jul 1, 2009, at 6:09 PM, AlisdairM(public) wrote:

> Attached is a preliminary patch for C++0x type alias (but not  
> template alias)
>
> As this is my first feature, I would appreciate feedback on any  
> issues with how I am structuring the code/design so I can clean it  
> up before solving the outstanding issues.

The general structure looks good. I'll probably have more to say when  
you get deeper into semantic analysis and representation.

> I'm not sure how to package a new file, so my test case is attached  
> separately.

If you "svn add" the new file, it will show up in a diff.

Detailed comments:

+/// ParseUsingDeclaration - Parse C++ using-declaration. Assumes that
+/// 'using' was already seen.
+///
+///     using-declaration: [C++ 7.3.p3: namespace.udecl]
+///       'using' 'typename'[opt] ::[opt] nested-name-specifier
+///               unqualified-id
+///       'using' :: unqualified-id
+///

Please either add a comment noting that this function also parses  
alias-declarations, or, better yet, rename the function to something  
like ParseUsingOrAliasDeclaration.

+  if( !ParseOptionalCXXScopeSpecifier(SS) ) {
+    if (Tok.is(tok::identifier) && NextToken().getKind() == tok::equal)
+      return ParseAliasDeclaration(Context, UsingLoc, DeclEnd);
+  }

We prefer not to use NextToken(), if we can avoid it, for performance  
reasons. One could structure the code like this:

   IdentifierInfo *TargetName = 0;
   SourceLocation IdentLoc;
   if (!ParseOptionalCXXScopeSpecifier(SS)) {
     if (Tok.is(tok::identifier)) {
       TargetName = Tok.getIdentifierInfo();
       IdentLoc = ConsumeToken();

       if (Tok.is(tok::equal))
         return ParseAliasDeclaration(Context, UsingLoc, TargetName,  
IdentLoc, DeclEnd);
     }
   }

   // ...

+/// ParseAliasDeclaration - Parse C++ alias-declaration. Assumes that
+/// 'using' was already seen, and next token is alias' simple identifer
+/// without a nested name specifer

Typos "identifer" and "specifer".

+  // Alias declarations are only supported in C++0x
+  if (!getLang().CPlusPlus0x) {
+    Diag(Tok, diag::err_alias_declaration);
+    SkipUntil(tok::semi);
+    return DeclPtrTy();
+  }

I prefer to let Sema issue diagnostics about the use of C++0x features  
in C++03. The parser, for cases like this where it is obvious what the  
user intended, should just parse the syntax and ignore the dialect.  
Actually, within Sema, I would suggest diagnosing the error but not  
returning a failure. We can recover perfectly when we see use of a  
feature that's only invalid because we're in the wrong dialect. (It  
would be a different story if we couldn't figure out what the user  
meant without knowing the dialect, as with >> closing templates)

+Sema::DeclPtrTy Sema::ActOnAliasDeclaration(Scope *S,
+                                          SourceLocation UsingLoc,
+                                          SourceLocation IdentLoc,
+                                          IdentifierInfo *IdentName,
+                                          TypeTy *Target) {
+  assert(IdentLoc.isValid() && "Invalid TargetName location.");
+
+  QualType TargetType = QualType::getFromOpaquePtr(Target);
+  TypedefDecl * TypeAlias = TypedefDecl::Create(Context, CurContext,
+                                                IdentLoc,IdentName,
+                                                TargetType );
+  PushOnScopeChains(TypeAlias, S);
+  return DeclPtrTy::make(TypeAlias);
+}

At the very least, we'll need to encode the fact that this was a C++0x  
alias-declaration in the TypedefDecl we create, because it's important  
that our ASTs provide good fidelity with the source code. Most optimal  
would be new Decl kind (TypeAliasDecl, for example) that only applies  
to C++0x alias declarations, but has generally the same behavior as  
TypedefDecl (perhaps it is just a subclass of TypedefDecl). More  
importantly, once we get a TypeAliasTemplateDecl, the  
TypeAliasTemplateDecl could point at the TypeAliasDecl for its "body".

Your patch looks like a good start!

   - Doug