[AST] Fix loss of enum forward decl from decl context

[jdenny]

Summary

For example, given:

enum __attribute__((deprecated)) T *p;

-ast-print produced:

enum T *p;

The attribute was lost because the enum forward decl was lost.

Another example is the loss of enum forward decls from C++ namespaces
(in MS compatibility mode).

The trouble was that the EnumDecl node was suppressed, as revealed by
-ast-dump. The suppression of the EnumDecl was intentional in
r116122, but I don't understand why. The suppression isn't needed for
the test suite to behave.

[rsmith]

Clang's current behavior is observably wrong in MS compatibility mode. For example:

namespace N {
  enum E *p;
  exterm E e; // ok, finds E injected into scope by previous declaration
}
N::E e; // error, no E in N!
namespace N {
  extern E e; // error, no E in N!
}

Please add something like the above to the test suite, to make sure we don't regress this (in some existing -fms-compatibility test for enum forward declarations).

[jdenny]

Made the suggested change. Thanks!

[jdenny]

Rebased. Ping.

lib/Sema/SemaDecl.cpp

   // keyword.
   SourceLocation Loc = NameLoc.isValid() ? NameLoc : KWLoc;
 
   // Otherwise, create a new declaration. If there is a previous
   // declaration of the same entity, the two will be linked via
   // PrevDecl.
   TagDecl *New;
 
-  bool IsForwardReference = false;
   if (Kind == TTK_Enum) {
     // FIXME: Tag decls should be chained to any simultaneous vardecls, e.g.:
     // enum X { A, B, C } D;    D should chain to X.
     New = EnumDecl::Create(Context, SearchDC, KWLoc, Loc, Name,
                            cast_or_null<EnumDecl>(PrevDecl), ScopedEnum,
                            ScopedEnumUsesClassTag, IsFixed);
 
     if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit()))

         Diag(Def->getLocation(), diag::note_previous_definition);
       } else {
         unsigned DiagID = diag::ext_forward_ref_enum;
         if (getLangOpts().MSVCCompat)
           DiagID = diag::ext_ms_forward_ref_enum;
         else if (getLangOpts().CPlusPlus)
           DiagID = diag::err_forward_ref_enum;
         Diag(Loc, DiagID);
-
-        // If this is a forward-declared reference to an enumeration, make a
-        // note of it; we won't actually be introducing the declaration into
-        // the declaration context.
-        if (TUK == TUK_Reference)
-          IsForwardReference = true;
       }
     }
 
     if (EnumUnderlying) {
       EnumDecl *ED = cast<EnumDecl>(New);
       if (TypeSourceInfo *TI = EnumUnderlying.dyn_cast<TypeSourceInfo*>())
         ED->setIntegerTypeSourceInfo(TI);
       else

 
     DeclContext *DC = New->getDeclContext()->getRedeclContext();
     DC->makeDeclVisibleInContext(New);
     if (Name) // can be null along some error paths
       if (Scope *EnclosingScope = getScopeForDeclContext(S, DC))
         PushOnScopeChains(New, EnclosingScope, /* AddToContext = */ false);
   } else if (Name) {
     S = getNonFieldDeclScope(S);
-    PushOnScopeChains(New, S, !IsForwardReference);
-    if (IsForwardReference)
-      SearchDC->makeDeclVisibleInContext(New);
+    PushOnScopeChains(New, S, true);
   } else {
     CurContext->addDecl(New);
   }
 
   // If this is the C FILE type, notify the AST context.
   if (IdentifierInfo *II = New->getIdentifier())
     if (!New->isInvalidDecl() &&
         New->getDeclContext()->getRedeclContext()->isTranslationUnit() &&

test/Sema/ast-print.c

 // RUN: %clang_cc1 %s -ast-print -verify > %t.c
 // RUN: FileCheck %s --input-file %t.c
 //
 // RUN: echo >> %t.c "// expected""-warning@* {{use of GNU old-style field designator extension}}"
 // RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes' is deprecated}}"
 // RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes' has been explicitly marked deprecated here}}"
+// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes2' is deprecated}}"
+// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes2' has been explicitly marked deprecated here}}"
 // RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes3' is deprecated}}"
 // RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes3' has been explicitly marked deprecated here}}"
 // RUN: %clang_cc1 -fsyntax-only %t.c -verify
 
 typedef void func_typedef();
 func_typedef xxx;
 
 typedef void func_t(int x);

 enum EnumWithAttributes { // expected-warning {{'EnumWithAttributes' is deprecated}}
   // CHECK-NEXT: EnumWithAttributesFoo __attribute__((deprecated(""))),
   EnumWithAttributesFoo __attribute__((deprecated)),
   // CHECK-NEXT: EnumWithAttributesBar __attribute__((unavailable(""))) = 50
   EnumWithAttributesBar __attribute__((unavailable)) = 50
   // CHECK-NEXT: } *EnumWithAttributesPtr;
 } __attribute__((deprecated)) *EnumWithAttributesPtr; // expected-note {{'EnumWithAttributes' has been explicitly marked deprecated here}}
 
-// FIXME: If enum is forward-declared at file scope, attributes are lost.
-// CHECK-LABEL: enum EnumWithAttributes2 *EnumWithAttributes2Ptr;
+// CHECK-LABEL: enum __attribute__((deprecated(""))) EnumWithAttributes2 *EnumWithAttributes2Ptr;
 // expected-warning@+2 {{'EnumWithAttributes2' is deprecated}}
 // expected-note@+1 {{'EnumWithAttributes2' has been explicitly marked deprecated here}}
 enum __attribute__((deprecated)) EnumWithAttributes2 *EnumWithAttributes2Ptr;
 
 // CHECK-LABEL: EnumWithAttributes3Fn
 void EnumWithAttributes3Fn() {
   // CHECK-NEXT: enum __attribute__((deprecated(""))) EnumWithAttributes3 *EnumWithAttributes3Ptr;
   // expected-warning@+2 {{'EnumWithAttributes3' is deprecated}}
   // expected-note@+1 {{'EnumWithAttributes3' has been explicitly marked deprecated here}}
   enum __attribute__((deprecated)) EnumWithAttributes3 *EnumWithAttributes3Ptr;
   // Printing must not put the attribute after the tag where it would apply to
   // the variable instead of the type, and then our deprecation warning would
   // move to this use of the variable.
   void *p = EnumWithAttributes3Ptr;
 }