[clang] Fix ASTWriter crash after merging named enums #114240
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Michael Jabbour (michael-jabbour-sonarsource) ChangesClang already has a mechanism to cleanup enumerators for typedefs to anonymous enums. So the following example code used to be handled correctly while merging, and ASTWriter behaves as expected: typedef enum { Val } AnonEnum;However, the mentioned mechanism didn't handle named enums. This lead to stale declarations in typedef enum Enum1 { Val_A } Enum1;
enum Enum2 { Val_B };The PR applies the same mechanism in the named enums case. Additionally, I dropped the call to Might be easier to to review commit by commit. Any feedback is appreciated. ContextThis fixes frontend crashes that were encountered when certain Objective-C modules are included on Xcode 16. For example, by running #include <os/atomic.h>
int main() {
return memory_order_relaxed;
}This crashes the parser in release, when ASTReader tried to load the enumerator declaration. Full diff: https://github.com/llvm/llvm-project/pull/114240.diff 5 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 93d98e1cbb9c81..d53bcf63dd1bd9 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3910,7 +3910,7 @@ class Sema final : public SemaBase {
/// Perform ODR-like check for C/ObjC when merging tag types from modules.
/// Differently from C++, actually parse the body and reject / error out
/// in case of a structural mismatch.
- bool ActOnDuplicateDefinition(Decl *Prev, SkipBodyInfo &SkipBody);
+ bool ActOnDuplicateDefinition(Scope *S, Decl *Prev, SkipBodyInfo &SkipBody);
typedef void *SkippedDefinitionContext;
@@ -4034,6 +4034,12 @@ class Sema final : public SemaBase {
void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
LookupResult &OldDecls);
+ /// RetireNodesFromMergedDecl - We have just merged the decl 'New' by making
+ /// another definition visible.
+ /// This method performs any necessary cleanup on the parser state to discard
+ /// child nodes from newly parsed decl we are retiring.
+ void RetireNodesFromMergedDecl(Scope *S, Decl *New);
+
/// MergeFunctionDecl - We just parsed a function 'New' from
/// declarator D which has the same name and scope as a previous
/// declaration 'Old'. Figure out how to resolve this situation,
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 31984453487aef..750301a9155d79 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -5645,7 +5645,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
Decl *D = SkipBody.CheckSameAsPrevious ? SkipBody.New : TagDecl;
ParseEnumBody(StartLoc, D);
if (SkipBody.CheckSameAsPrevious &&
- !Actions.ActOnDuplicateDefinition(TagDecl, SkipBody)) {
+ !Actions.ActOnDuplicateDefinition(getCurScope(), TagDecl, SkipBody)) {
DS.SetTypeSpecError();
return;
}
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 60aab1411a96c5..505dff17bd80f2 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2350,7 +2350,8 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
// Parse the definition body.
ParseStructUnionBody(StartLoc, TagType, cast<RecordDecl>(D));
if (SkipBody.CheckSameAsPrevious &&
- !Actions.ActOnDuplicateDefinition(TagOrTempResult.get(), SkipBody)) {
+ !Actions.ActOnDuplicateDefinition(getCurScope(),
+ TagOrTempResult.get(), SkipBody)) {
DS.SetTypeSpecError();
return;
}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f8e5f3c6d309d6..a38d2d00a5410d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scope.
- if (isa<EnumDecl>(NewTag)) {
- Scope *EnumScope = getNonFieldDeclScope(S);
- for (auto *D : NewTag->decls()) {
- auto *ED = cast<EnumConstantDecl>(D);
- assert(EnumScope->isDeclScope(ED));
- EnumScope->RemoveDecl(ED);
- IdResolver.RemoveDecl(ED);
- ED->getLexicalDeclContext()->removeDecl(ED);
- }
- }
+ RetireNodesFromMergedDecl(S, NewTag);
}
}
@@ -2639,6 +2628,19 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
notePreviousDefinition(Old, New->getLocation());
}
+void Sema::RetireNodesFromMergedDecl(Scope *S, Decl *New) {
+ // If this was an unscoped enumeration, yank all of its enumerators
+ // out of the scope.
+ if (auto *ED = dyn_cast<EnumDecl>(New); ED && !ED->isScoped()) {
+ Scope *EnumScope = getNonFieldDeclScope(S);
+ for (auto *ECD : ED->enumerators()) {
+ assert(EnumScope->isDeclScope(ECD));
+ EnumScope->RemoveDecl(ECD);
+ IdResolver.RemoveDecl(ECD);
+ }
+ }
+}
+
/// DeclhasAttr - returns true if decl Declaration already has the target
/// attribute.
static bool DeclHasAttr(const Decl *D, const Attr *A) {
@@ -18059,12 +18061,14 @@ void Sema::ActOnTagStartDefinition(Scope *S, Decl *TagD) {
AddPushedVisibilityAttribute(Tag);
}
-bool Sema::ActOnDuplicateDefinition(Decl *Prev, SkipBodyInfo &SkipBody) {
+bool Sema::ActOnDuplicateDefinition(Scope *S, Decl *Prev,
+ SkipBodyInfo &SkipBody) {
if (!hasStructuralCompatLayout(Prev, SkipBody.New))
return false;
// Make the previous decl visible.
makeMergedDefinitionVisible(SkipBody.Previous);
+ RetireNodesFromMergedDecl(S, SkipBody.New);
return true;
}
diff --git a/clang/test/Modules/modules-merge-enum.m b/clang/test/Modules/modules-merge-enum.m
new file mode 100644
index 00000000000000..4873c7e6b4708e
--- /dev/null
+++ b/clang/test/Modules/modules-merge-enum.m
@@ -0,0 +1,94 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang -fmodules -fmodules-cache-path=%t/modcache -fsyntax-only %t/source.m -Xclang -ast-dump-all 2>&1 | FileCheck %s
+
+
+//--- shared.h
+// This header is shared between two modules, but it's not a module itself.
+// The enums defined here are parsed in both modules, and merged while building ModB.
+
+typedef enum MyEnum1 { MyVal_A } MyEnum1;
+// CHECK: |-EnumDecl 0x{{.*}} imported in ModA.ModAFile1 <undeserialized declarations> MyEnum1
+// CHECK-NEXT: | |-also in ModB
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} imported in ModA.ModAFile1 referenced MyVal_A 'int'
+// CHECK-NEXT: |-TypedefDecl 0x{{.*}} imported in ModA.ModAFile1 hidden MyEnum1 'enum MyEnum1'
+// CHECK-NEXT: | `-ElaboratedType 0x{{.*}} 'enum MyEnum1' sugar imported
+// CHECK-NEXT: | `-EnumType 0x{{.*}} 'enum MyEnum1' imported
+// CHECK-NEXT: | `-Enum 0x{{.*}} 'MyEnum1'
+
+
+enum MyEnum2 { MyVal_B };
+// CHECK: |-EnumDecl 0x{{.*}} imported in ModA.ModAFile1 <undeserialized declarations> MyEnum2
+// CHECK-NEXT: | |-also in ModB
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} imported in ModA.ModAFile1 referenced MyVal_B 'int'
+
+
+typedef enum { MyVal_C } MyEnum3;
+// CHECK: |-EnumDecl 0x{{.*}} imported in ModA.ModAFile1 <undeserialized declarations>
+// CHECK-NEXT: | |-also in ModB
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} imported in ModA.ModAFile1 referenced MyVal_C 'int'
+// CHECK-NEXT: |-TypedefDecl 0x{{.*}} imported in ModA.ModAFile1 hidden MyEnum3 'enum MyEnum3':'MyEnum3'
+// CHECK-NEXT: | `-ElaboratedType 0x{{.*}} 'enum MyEnum3' sugar imported
+// CHECK-NEXT: | `-EnumType 0x{{.*}} 'MyEnum3' imported
+// CHECK-NEXT: | `-Enum 0x{{.*}} ''
+
+
+// In this case, no merging happens on the EnumDecl in Objective-C, and ASTWriter writes both EnumConstantDecls when building ModB.
+enum { MyVal_D };
+// CHECK: |-EnumDecl 0x{{.*}} imported in ModA.ModAFile1 hidden <undeserialized declarations>
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} imported in ModA.ModAFile1 hidden MyVal_D 'int'
+
+
+// Redeclarations coming from ModB.
+// CHECK: |-TypedefDecl 0x{{.*}} prev 0x{{.*}} imported in ModB MyEnum1 'enum MyEnum1'
+// CHECK-NEXT: | `-ElaboratedType 0x{{.*}} 'enum MyEnum1' sugar imported
+// CHECK-NEXT: | `-EnumType 0x{{.*}} 'enum MyEnum1' imported
+// CHECK-NEXT: | `-Enum 0x{{.*}} 'MyEnum1'
+
+// CHECK: |-EnumDecl 0x{{.*}} prev 0x{{.*}} imported in ModB <undeserialized declarations>
+// CHECK-NEXT: | |-also in ModB
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} imported in ModB MyVal_C 'int'
+// CHECK-NEXT: |-TypedefDecl 0x{{.*}} prev 0x{{.*}} imported in ModB MyEnum3 'enum MyEnum3':'MyEnum3'
+// CHECK-NEXT: | `-ElaboratedType 0x{{.*}} 'enum MyEnum3' sugar imported
+// CHECK-NEXT: | `-EnumType 0x{{.*}} 'MyEnum3' imported
+// CHECK-NEXT: | `-Enum 0x{{.*}} ''
+
+// CHECK: |-EnumDecl 0x{{.*}} imported in ModB <undeserialized declarations>
+// CHECK-NEXT: | `-EnumConstantDecl 0x{{.*}} first 0x{{.*}} imported in ModB referenced MyVal_D 'int'
+
+//--- module.modulemap
+module ModA {
+ module ModAFile1 {
+ header "ModAFile1.h"
+ export *
+ }
+ module ModAFile2 {
+ header "ModAFile2.h"
+ export *
+ }
+}
+// The goal of writing ModB is to test that ASTWriter can handle the merged AST nodes correctly.
+// For example, a stale declaration in IdResolver can cause an assertion failure while writing the identifier table.
+module ModB {
+ header "ModBFile.h"
+ export *
+}
+
+//--- ModAFile1.h
+#include "shared.h"
+
+//--- ModAFile2.h
+// Including this file, triggers loading of the module ModA with nodes coming ModAFile1.h being hidden.
+
+//--- ModBFile.h
+// ModBFile depends on ModAFile2.h only.
+#include "ModAFile2.h"
+// Including shared.h here causes Sema to merge the AST nodes from shared.h with the hidden ones from ModA.
+#include "shared.h"
+
+
+//--- source.m
+#include "ModBFile.h"
+
+int main() { return MyVal_A + MyVal_B + MyVal_C + MyVal_D; }
|
michael-jabbour-sonarsource
changed the title
michael-jabbour-sonarsource
changed the title
|
Ping? Note that this PR fixes a crash in ASTWriter on valid Objective-C modules code; The initial PR title didn't reflect that accurately. |
|
@jansvoboda11 for OC related things |
Thanks! CC @vsapsai and @ian-twilightcoder. |
|
Gentle ping 😄 The aim of the PR is to fix an ASTWriter crash that currently happens when serializing merged enums. Such a case was encountered while parsing Objective-C code that uses the XCode 16 SDK. If any of the changes look risky, I am happy to rework them and/or add more tests to increase confidence. Thanks in advance. |
|
Gentle ping 😄 I would love to hear your feedback, and if any changes are needed, I am happy to address them promptly. |
|
Sorry for the delay. Will need to look at the change in a debugger. |
|
Thanks for the update @vsapsai. Sounds good. Let me know if there are any changes that can make reviewing the PR easier! |
|
Gentle ping, and I wish you all a happy new year. 😄 |
clang/include/clang/Sema/Sema.h
Outdated
| /// another definition visible. | ||
| /// This method performs any necessary cleanup on the parser state to discard | ||
| /// child nodes from newly parsed decl we are retiring. | ||
| void RetireNodesFromMergedDecl(Scope *S, Decl *New); |
There was a problem hiding this comment.
| void RetireNodesFromMergedDecl(Scope *S, Decl *New); | |
| void CleanupMergedEnum(Scope *S, Decl *New); |
Maybe something like that is clearer
There was a problem hiding this comment.
Thanks for the suggestion, done in f55ca2bdf0e0fabdc672f0f85007d091272a8891.
| assert(EnumScope->isDeclScope(ED)); | ||
| EnumScope->RemoveDecl(ED); | ||
| IdResolver.RemoveDecl(ED); | ||
| ED->getLexicalDeclContext()->removeDecl(ED); |
There was a problem hiding this comment.
I am not super confident about this change, is it related or could it be separate? @AaronBallman
There was a problem hiding this comment.
Thanks for raising the concern. I am also not entirely sure, but I chose to remove it for two reasons:
-
This
removeDecl()call causes an incorrect ODR violation diagnostic when-ast-dump-allis provided (which I am using in my regression test to expect the AST nodes after merging):In module 'ModB' imported from <source>:1: In module 'ModA' imported from ./ModBFile.h:2: ./shared.h:6:9: error: 'MyEnum3' has different definitions in different modules; definition in module 'ModA.ModAFile1' first difference is enum with 1 element 6 | typedef enum { MyVal_C } MyEnum3; | ^~~~~~~~~~~~~~~~ ./shared.h:6:9: note: but in 'ModB' found enum with 0 elements 6 | typedef enum { MyVal_C } MyEnum3; | ^~~~~~~~~~~~~~~~ 1 error generated.This incorrect ODR violation can be observed before this patch. CE link: here.
-
I am also not entirely sure if performing the call in the named enum case (the new case I am introducing) is safe. I can understand why it is important to remove the constant from IdResolver and the Enum context, but I don't know if removing the constant from the enum decl doesn't cause problems in some cases.
There was a problem hiding this comment.
Additionally, as far as I can see, for EnumDeclConstants created by the parser, the DeclContext is always the containing Enum itself:
llvm-project/clang/lib/Sema/SemaDecl.cpp
Lines 19739 to 19740 in 79231a8
Therefore, after the refactoring I did, I think that the call can be simplified to:
ED->removeDecl(ECD);
There was a problem hiding this comment.
I can try to dig deeper to answer the points I mentioned (especially point 2) if desired, but I would need to allocate some time (maybe at the end of the week).
Any pointers/hints/thoughts would be appreciated.
There was a problem hiding this comment.
I had a quick look into this today, and I could see that some other ODR checks aren't emitted unless -ast-dump-all is provided. For example, in this CE example, I have the enum E defined with different enumerators, and I don't get a diagnostic unless I have -Xclang -ast-dump-all on the command line (removing it, somehow removes the diagnostic).
While I still don't have a concrete answer about the points I mentioned in my previous response, it seems to me that this is a separate bug in the ODR check implementation, and that ideally, we should get the diagnostic without the -ast-dump-all.
If my conclusion above is correct, I would argue that dropping this call to removeDecl is necessary for these ODR checks to work (once the bug above is fixed).
I may try to dig deeper later into why the diagnostics are not emitted without -ast-dump-all. I am curious about your thoughts here.
There was a problem hiding this comment.
@ilya-biryukov, this PR is deletes a statement ED->getLexicalDeclContext()->removeDecl(ED); which I think is okay. Is there an easy way to check on your infrastructure if we did not miss some obscure code example which proves us wrong?
There was a problem hiding this comment.
Sure, happy to run this over our codebase. Will report back after a day or two.
There was a problem hiding this comment.
I just wanted to circle back and report that the tests are running right now, but may not finish before the end of the working day.
Sorry for a slight delay. We should definitely know if this breaks anything tomorrow, but maybe even later today.
There was a problem hiding this comment.
No issues on our side with this change. So LG!
There was a problem hiding this comment.
Thank you all very much for checking this. I will be updating the failing test in a few hours or tomorrow.
|
That crash looks pretty annoying, thanks for looking into this issue and debugging it. Can you trigger the crash without Can you use in the test |
|
In general, personally I quite like the idea of removing decls from the scope. Though I think when I've tried to do so [in a different context], there were some problems. I'll try to find what I was doing and why it wasn't working. It's not a blocker for your change but it can be helpful to understand potential pitfalls better. |
|
@vsapsai Thank you very much for the input. 🙏
I addressed both of these in f6f0887d22172c9db4f602b7f272630a97a708bd. I added a separate invocation without I kept the invocation with For convenience, you can also find the crashing example here on CE: link.
I added some of my thoughts about that in this discussion. I would appreciate any feedback. |
There was a problem hiding this comment.
I've realized that the test doesn't have to be Objective-C, the failure is reproducible in C. Curiously, we aren't hitting the assertion in C++/Objective-C++ but I don't know why.
I wanted to try defining the enums in different scopes (e.g. in a struct) and see how it works. Consequences for scopes aren't clear, so that's worth taking another look.
A big surprise for me was that the assertion happens in the stack trace
#7 0x0000000105168664 clang::ASTWriter::WriteCUDAPragmas(clang::Sema&)
#8 0x00000001033ea29c clang::ASTWriter::WriteFPPragmaOptions(clang::FPOptionsOverride const&)
#9 0x00000001033e4e70 clang::ASTWriter::WriteIdentifierTable(clang::Preprocessor&, clang::IdentifierResolver*, bool)
The fact that WriteCUDAPragmas is somehow involved while we have no CUDA pragmas is very suspicious.
|
For the record, the previous work I've abandoned is https://reviews.llvm.org/D114833 Doesn't seem particularly relevant to this change to be honest. |
|
@vsapsai Thanks again for the feedback 🙏
I dug a bit into this today, and I could find a few differences:
I added one more test case for this. I got the same stack trace, and the crash seems to be fixed after the PR as well. However, I couldn’t reproduce the stack trace with |
|
@vsapsai Thank you for sharing the previous patch. This is really interesting.
As far as I can see, it actually seems to introduce the exact same I tried to understand why that work was abandoned, in order to know if the reasoning applies to the changes in my PR.
|
| @@ -2639,6 +2628,19 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, | |||
| notePreviousDefinition(Old, New->getLocation()); | |||
| } | |||
|
|
|||
| void Sema::CleanupMergedEnum(Scope *S, Decl *New) { | |||
There was a problem hiding this comment.
I feel like this is a property of merging. Can we find a common path when merging definitions to put that logic there? Is MergeTypedefNameDecl not called for the motivating example?
There was a problem hiding this comment.
Is MergeTypedefNameDecl not called for the motivating example?
Unfortunately, it seems to me that MergeTypedefNameDecl is only called in the typedef to anonymous enum case (MyEnum3 in the test case I am adding). For the rest of the enum cases, the closest I found was Sema::ActOnDuplicateDefinition, which is called when merging all tags (and this is where I am adding the new call to CleanupMergedEnum).
Can we find a common path when merging definitions to put that logic there?
I could only see that merging enums for C and Obj-C crashes in this case (C++ works differently, see here), and I found Sema::ActOnDuplicateDefinition during my investigation to be the function that handles merging tags.
I am not aware of a central place for merging definitions in general. Could you provide some hints on what should I look for?
There was a problem hiding this comment.
I realize that Sema::ActOnDuplicateDefinition maybe our best bet at the moment.
|
Gentle ping 😄 @vsapsai, @vgvassilev, I think I have responded to your inquiries and added a few related questions. Could you have a look? |
michael-jabbour-sonarsource
force-pushed
the
fix_astwriter_enum_crash
branch
from
f3e900b to
e15e18d
Compare
|
Please rebase this PR. |
michael-jabbour-sonarsource
force-pushed
the
fix_astwriter_enum_crash
branch
from
c9a7b0a to
9eede45
Compare
michael-jabbour-sonarsource
force-pushed
the
fix_astwriter_enum_crash
branch
from
9eede45 to
d5c710e
Compare
|
Thank you all for looking into this. Note that I don't have commit access, and I would appreciate it if someone merges this if/when it is ready to merge! |
|
@michael-jabbour-sonarsource Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Clang already removes parsed enumerators when merging typedefs to
anonymous enums. This is why the following example decl used to be
handled correctly while merging, and ASTWriter behaves as expected:
```c
typedef enum { Val } AnonEnum;
```
However, the mentioned mechanism didn't handle named enums. This leads
to stale declarations in `IdResolver`, causing an assertion violation in
ASTWriter ``Assertion `DeclIDs.contains(D) && "Declaration not
emitted!"' failed`` when a module is being serialized with the following
example merged enums:
```c
typedef enum Enum1 { Val_A } Enum1;
enum Enum2 { Val_B };
```
The PR applies the same mechanism in the named enums case.
Additionally, I dropped the call to
`getLexicalDeclContext()->removeDecl` as it was causing a wrong
odr-violation diagnostic with anonymous enums.
Might be easier to to review commit by commit. Any feedback is
appreciated.
### Context
This fixes frontend crashes that were encountered when certain
Objective-C modules are included on Xcode 16. For example, by running
`CC=/path/to/clang-19 xcodebuild clean build` on a project that contains
the following Objective-C file:
```c
#include <os/atomic.h>
int main() {
return memory_order_relaxed;
}
```
This crashes the parser in release, when ASTReader tries to load the
enumerator declaration.
Clang already removes parsed enumerators when merging typedefs to anonymous enums. This is why the following example decl used to be handled correctly while merging, and ASTWriter behaves as expected:
However, the mentioned mechanism didn't handle named enums. This leads to stale declarations in
IdResolver, causing an assertion violation in ASTWriterAssertion `DeclIDs.contains(D) && "Declaration not emitted!"' failedwhen a module is being serialized with the following example merged enums:The PR applies the same mechanism in the named enums case.
Additionally, I dropped the call to
getLexicalDeclContext()->removeDeclas it was causing a wrong odr-violation diagnostic with anonymous enums.Might be easier to to review commit by commit. Any feedback is appreciated.
Context
This fixes frontend crashes that were encountered when certain Objective-C modules are included on Xcode 16. For example, by running
CC=/path/to/clang-19 xcodebuild clean buildon a project that contains the following Objective-C file:This crashes the parser in release, when ASTReader tries to load the enumerator declaration.