[Sema] Fix redeclaration contexts for enumerators in C

[aaron.ballman]

Summary

In C, enumerators are not hoisted into, say, a struct decl context when the enumeration is declared inside of a struct. Instead, the enumerators are hoisted into the translation unit decl context. This patch fixes getRedeclContext() to skip records as well as transparent contexts when the original context is an enumeration. This allows us to catch enumerator redeclarations as well as silent name hiding + miscompiles.

This patch address PR15071.

[aaron.ballman]

Ping?

[majnemer]

lib/AST/DeclBase.cpp

1711–1712

Seems simpler as:

while ((SkipRecords && Ctx->isRecord()) || Ctx->isTransparentContext())

[aaron.ballman]

Updated based on review feedback.

[aaron.ballman]

lib/AST/DeclBase.cpp

1711–1712

Agreed; I've changed it.

[aaron.ballman]

Ping

[rsmith]

lib/AST/DeclBase.cpp

1704–1705

Nit: "the redeclaration context for enumerators is the translation unit" is not entirely accurate. The point instead is that a record type is only the redeclaration context for the fields of that record, so if we arrive at that context after skipping anything else, we should skip the record as well. (The check for "Enum" here is a red herring in that regard, but it happens to be correct because enumerations are the only transparent context that can exist within a struct or union currently.)

[rsmith]

(Looks fine with a suitably-adjusted comment.)

[aaron.ballman]

Committed (with improved comment) in r345073.

lib/AST/DeclBase.cpp

     if (auto *ND = dyn_cast<NamedDecl>(D))
       if (ND->getDeclName() == Name)
         Results.push_back(ND);
   }
 }
 
 DeclContext *DeclContext::getRedeclContext() {
   DeclContext *Ctx = this;
-  // Skip through transparent contexts.
-  while (Ctx->isTransparentContext())
+
+  // In C, the redeclaration context for enumerators is the translation unit,
+  // so we skip through transparent contexts as well as struct/union contexts.
+  bool SkipRecords = getDeclKind() == Decl::Kind::Enum &&
+                     !getParentASTContext().getLangOpts().CPlusPlus;
+
+  // Skip through contexts to get to the redeclaration context. Transparent
+  // contexts are always skipped.
+  while ((SkipRecords && Ctx->isRecord()) || Ctx->isTransparentContext())
     Ctx = Ctx->getParent();
   return Ctx;
 }
 
 DeclContext *DeclContext::getEnclosingNamespaceContext() {
   DeclContext *Ctx = this;
   // Skip through non-namespace, non-translation-unit contexts.
   while (!Ctx->isFileContext())
     Ctx = Ctx->getParent();

test/Sema/enum.c

struct PR28903 {
   enum {
     PR28903_A = (enum {
       PR28903_B,
       PR28903_C = PR28903_B
     })0
   };
   int makeStructNonEmpty;
 };
+
+static int EnumRedecl; // expected-note 2 {{previous definition is here}}
+struct S {
+  enum {
+    EnumRedecl = 4 // expected-error {{redefinition of 'EnumRedecl'}}
+  } e;
+};
+
+union U {
+  enum {
+    EnumRedecl = 5 // expected-error {{redefinition of 'EnumRedecl'}}
+  } e;
+};
+
+enum PR15071 {
+  PR15071_One // expected-note {{previous definition is here}}
+};
+
+struct EnumRedeclStruct {
+  enum {
+    PR15071_One // expected-error {{redefinition of enumerator 'PR15071_One'}}
+  } e;
+};