[clang] fix obtaining underlying type for demoted enum definitions #155900
Conversation
Clang skips parsing a TagDecl definition in case a definition was already parsed in another module. In those cases, an EnumDecl might be left without an IntegerType. Take this into account when getting the underlying type of an enum, look for the integer type in the definition instead in those cases. This patch also changes the implementation so it properly marks those skipped tag definitions as demoted. This fixes a regression reported here: #155028 (comment) Since this regression was never released, there are no release notes.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Matheus Izvekov (mizvekov) ChangesClang skips parsing a TagDecl definition in case a definition was already parsed in another module. In those cases, an EnumDecl might be left without an IntegerType. Take this into account when getting the underlying type of an enum, look for the integer type in the definition instead in those cases. This patch also changes the implementation so it properly marks those skipped tag definitions as demoted. This fixes a regression reported here: #155028 (comment) Since this regression was never released, there are no release notes. Full diff: https://github.com/llvm/llvm-project/pull/155900.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a47c5ab3aaff7..73f92f7ec9a1e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18544,8 +18544,14 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
if (PrevDecl)
CheckRedeclarationInModule(New, PrevDecl);
- if (TUK == TagUseKind::Definition && (!SkipBody || !SkipBody->ShouldSkip))
- New->startDefinition();
+ if (TUK == TagUseKind::Definition) {
+ if (!SkipBody || !SkipBody->ShouldSkip) {
+ New->startDefinition();
+ } else {
+ New->setCompleteDefinition();
+ New->demoteThisDefinitionToDeclaration();
+ }
+ }
ProcessDeclAttributeList(S, New, Attrs);
AddPragmaAttributes(S, New);
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 3f31a05d382a6..0f655d7f684a5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9878,7 +9878,14 @@ static QualType GetEnumUnderlyingType(Sema &S, QualType BaseType,
S.DiagnoseUseOfDecl(ED, Loc);
QualType Underlying = ED->getIntegerType();
- assert(!Underlying.isNull());
+ if (Underlying.isNull()) {
+ // This is an enum without a fixed underlying type which we skipped parsing
+ // the body because we saw its definition previously in another module.
+ // Use the definition's integer type in that case.
+ assert(ED->isThisDeclarationADemotedDefinition());
+ Underlying = ED->getDefinition()->getIntegerType();
+ assert(!Underlying.isNull());
+ }
return Underlying;
}
diff --git a/clang/test/Modules/GH155028-1.cpp b/clang/test/Modules/GH155028-1.cpp
new file mode 100644
index 0000000000000..d60112b48c218
--- /dev/null
+++ b/clang/test/Modules/GH155028-1.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+#pragma clang module build M
+module "M" {
+ module "A" {}
+ module "B" {}
+}
+#pragma clang module contents
+#pragma clang module begin M.A
+enum E1 {};
+#pragma clang module end
+#pragma clang module begin M.B
+enum E1 {};
+using T = __underlying_type(E1);
+#pragma clang module end
+#pragma clang module endbuild
|
| New->startDefinition(); | ||
| } else { | ||
| New->setCompleteDefinition(); | ||
| New->demoteThisDefinitionToDeclaration(); |
There was a problem hiding this comment.
Do we ever come here for non-enum tag types? I looked up the ShouldSkipInfo class, but I wasn't able to figure out exactly when or why we skip tag bodies.
There was a problem hiding this comment.
|
Hi -- sidenote, but I noticed that this commit -- and many other of your commits -- are missing the entire commit message other than the PR title. I don't know if that's a github preference or something you're doing manually, but it's really unfortunate because then I cannot see the explanations in git log -- instead needing to indirect through the PR page. Do you know why that's happening for your changes? |
Oh yeah, I have seen that before, it seems like when I click merge through the GitHub iOS App, that happens. I assumed that was spurious and completely forgot about it. |
That is a really awful bug, please don't merge using this method unless we know it is fixed. It is really important to have summaries in the logs. I would file a bug report since this has happened to you a lot. Github should not have bugs this bad. |
Clang skips parsing a TagDecl definition in case a definition was already parsed in another module.
In those cases, an EnumDecl might be left without an IntegerType. Take this into account when getting the underlying type of an enum, look for the integer type in the definition instead in those cases.
This patch also changes the implementation so it properly marks those skipped tag definitions as demoted.
This fixes a regression reported here: #155028 (comment)
Since this regression was never released, there are no release notes.