[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman closed this revision. aaron.ballman added a comment. Committed (with improved comment) in r345073. https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
rsmith added inline comments. Comment at: lib/AST/DeclBase.cpp:1704-1705 + + // 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 && 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.) https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. (Looks fine with a suitably-adjusted comment.) https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman added a comment. Ping https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman added inline comments. Comment at: lib/AST/DeclBase.cpp:1711-1712 + // contexts are always skipped. + while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord() + : Ctx->isTransparentContext()) Ctx = Ctx->getParent(); majnemer wrote: > Seems simpler as: > while ((SkipRecords && Ctx->isRecord()) || Ctx->isTransparentContext()) Agreed; I've changed it. https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman updated this revision to Diff 167496.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
Herald added a subscriber: jsji.
Updated based on review feedback.
https://reviews.llvm.org/D52384
Files:
lib/AST/DeclBase.cpp
test/Sema/enum.c
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
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;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,15 @@
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;
}
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
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;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,15 @@
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;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
majnemer added inline comments. Comment at: lib/AST/DeclBase.cpp:1711-1712 + // contexts are always skipped. + while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord() + : Ctx->isTransparentContext()) Ctx = Ctx->getParent(); Seems simpler as: while ((SkipRecords && Ctx->isRecord()) || Ctx->isTransparentContext()) https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman added a reviewer: echristo. aaron.ballman added a comment. Ping? https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.
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.
https://reviews.llvm.org/D52384
Files:
lib/AST/DeclBase.cpp
test/Sema/enum.c
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
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;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,16 @@
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->isTransparentContext() || Ctx->isRecord()
+ : Ctx->isTransparentContext())
Ctx = Ctx->getParent();
return Ctx;
}
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
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;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,16 @@
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->isTransparentContext() || Ctx->isRecord()
+ : Ctx->isTransparentContext())
Ctx = Ctx->getParent();
return Ctx;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
