[clang] [clang] Fix ASTWriter crash after merging named enums (PR #114240)
Michael Jabbour via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 21 00:58:29 PST 2025
================
@@ -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);
----------------
michael-jabbour-sonarsource wrote:
Thanks for raising the concern. I am also not entirely sure, but I chose to remove it for two reasons:
1. It seems to me that the presence of this `removeDecl()` call causes an incorrect ODR violation diagnostic when `-ast-dump-all` is 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](https://godbolt.org/#z:OYLghAFBqd5QCxAFwE4FN0BoCWIIDGAtgIYDW6AgqsAM4gDkAtACIDCAspQNICiA%2BgCEAqgEkAMi34AVAJoAFXgFIAzCxboARgFdgDLAQD2RAA44ANulTiSAO2DaSwdKIAmIAkoBMg74IPatMjGAPLayCbhAGIW6LYkROgo6EFYAGax9LDorjjBqG4gXumxhSollvGJILQIJBiuAHQIWKAM%2Bji0orYE5tquOYwADB20HCQ4tgDKhtqoBEkMAIyjISZxw/rmdsCF7QCUWBA5eYYF7uUZloUALBVxCUnoAB4JJpaNBCYmrYyj3b1%2BoMGCNcGMJtNZvNFqDOmsNiCtjs9gxDsdcvlCkt7oUAGz3KpJIiGVzaD7E1ykH5tf49PoDdyIsHjSYzOYLRgrMHw2ybLDbeyFTw%2BPxok6Y9x3K4udwAdgJjxAHBJlBiliWzV%2B7TBAPpwNh4NZUI5y1W615TIFu3cwt8PjFGLOhQArDj3AAOBXVZWuVWxLyamk6ulAxkGlmQ9mLLlw818q1Cvyio7ip3ufHSwoATi9SR9gjV6EDf2DgIZfM6EbZ0M5ZoRoITNqT9sOnU45HQ8lQhgAVugCMha1geyBsbZ0AB3QtYoaHcyMZ36C2gwyMaQYdAAam8Ki5g8RaKQRgGvEdqFw%2BGlhJqxqLCG%2B6UMtmQUwIJEso6lT%2BQwloneefIjsUVoeM2/gmHY6AAHKKrU9Q5Jqf6WAOOBPvgcSuGwhh9EQvIqHcGHiJM0HaEQmhWCAromIYtB5KhthYTheF3NRtHIPRRHjlBpHkaglFYEh/bsU%2BUzIPUyCMaRzECegyHCdMYmoMgnEkWRFGukE4mSbhID4QJinKcR3FqXxzqHLQt6MAA9FZm7CD0xiJM%2BkzAJuyAIFu47PMgbkToYm4vG8li0G5qA4MAzioCF7lbgQqAkLUm4ZM8OSbpMbkIJ0m7yAASkoQyUDZbkAJ7rAMaQBbYpGbhwxW8FVRBLNusqCDVxUAGrvvwlDNSwbX1aRSyqL4BVFXE1W1QNRBeM1rW1Z15hCL1w35ZQq3IKVOToBV41ELNbULfwbC9f1DUqCtBWrbt%2B3zV1fVKLKLAXZQqL8guS58qucENJq%2B4jIeHgkugp6nOeeAQFeiqkJMnwPmk36vu%2BSRLF%2Bz6/v%2BgG6fyOygSKPhYBBXGKoFpgfF8PyCShaHHLYmHYVJo5YIRRk8RRLE0XRT7aby2KsZztgqcZvEgHclPyaJWn0zp2Ji/REtKYLrN8aLBnc4zmkKyzJki%2BZlkMDukxllu3hePmhbNCbq2rZMPnQ7YED7PtGDIHMtgHV1x1%2BO7i1PSorUPU9BWvfODCLlgy76KuJPvEW5NYH9r0QEeQMg/kF4Q7E16mo%2Bz6Ix%2BKM5z%2Bf7yABTIjtiIG2n4BOQTB1QUmSRYUlSMlyfR6G02r2LM1xSujvifPyV3BMc/Jiva0s%2BKyyJqtSzzrdCXLBnj8Lk/6ZLTHq8vWur7iutRowDeWDVKqzatm6bkfW4%2Bn66pnwVF8Xx5JADKg25eKbKqFhqCCWw/j8vGokpTcAAqc%2By1/5XxPr6QsM0HojR6o/Z%2Br936fxgf6C2H9wEX0AWcHyYD/4Byto9K2BUoH5nvog5BVhUFm1iJgrw4DcHAIIWtEhQc5zvXDp9EAV9GjNxID8BOANjzAzPOnSG1Rs7w1zm%2BfOqMi4YzLiLbG9hcZ2nArXRUN9v6IVkovamGFh491UsLYog96LDwsU%2BFeFFijTwUhvBmMt9FU0cZrXu2t7Gz03jLbenizH7xrPrLwKhDZ6lQd9BCv8sEcLeqHD6TJVw6NiD/eOmwREp3EeDSR0Z9AyJfHI5GCj0Yl0xq6SuYEa5E29F/DBLQHEdzpr4pmtNbF8XMaPSxc9GbWIFjvOxC83Hywkj0lxbcZ7iXaUUdeSlh4a0MgEuxQSTRFV4KYDaSVYirWDlwiOWBkl1MsAGFowijjJxPNky8mdFTSIRkUz8hdSml1BCOfElS8aaJqXmEkBZ6ENNcfJJpxi2kDNMiPNi3SWl9OmRpQFS8nHSyGeLfxpj1KzNGS0hZsKVmLCKnQ4%2BAxzSuBCk%2BaBt90AnM3E%2BcwxVGhW1CeEoEtCjmUoYatIqup%2BguU3FEpoCBNweQwJuN8gQUibimOgUgbl/KJBoFuGKm5KBTGkJuWwQMQppG7HtPlzRNwTjyAKxVmVXADDdk%2BcVWrjDkvpZdRlIYBiRLqD9GJjC4khzDvsw5rg/kfFORkw41C%2BIOywGQSiQx4k3H0EQUcQwI1epLPQbEZy4CwBgIgQGphYhg3wEYLNlg%2BK9B2PwBKf4lL0VoPwNA2hbBkAqMgKw9AICaD5JoSY9RipDjbfEVAxUQiaD7AOIceanLIBCLYWlfIBg6GAGwd85h6DZwGKQewH4mS5AwChAAbikPkLx%2BzhDybgZ8sl4w4E0PFXtWEBh8jQDgGN2cd2oE0DRdAGgV3AHMC5DJ6Rth0HajgScPIhwFLzsUp5xcXn6HLqo0AVd8Zfs0FkENhgTDyUXUwZ41KB2Lx3UwAgm4mBpCviFIjV8mBUiI9stQfCBEmEI2kWgxVnwkGeEwGlxVCMAA0i32EIwlZATBSSmCYPOyOT6woDGQ%2BZeFhjO5jNaa4aZvMulcwUzCsF6tZPuMxc45FCKPFor4n4xF89sWaaWIcNAmBRycISdwpJjBNzPHdLiJguIbgiqtJuCAparDodCjWsgjsIDrkwO/Xc%2Bx9DCMDegF%2BFEQ0hyjVgGNKh3SNFCbKXEsoliylc86d0QwbgqFdAm7U9AI0pvTUnEAswIjhHTnmswBa66LBc25jzXmdibiWFmRoGpQSgYeQXApzzMbJo3Dgdw2IJzxRMLWOznqeEMDCPVnyhgKrtfc553jrlfO0DLQF6ttaQtYXzTQnclnosBsjdGkAuINRZdCV4d0Xglj4V686fEZX9AVfSQeQ4YbnQRpDioRJK4SwxfiV4cHkdIc3afbRNCNwgA).
2. I am also not entirely sure if performing the call in the named enum case 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.
https://github.com/llvm/llvm-project/pull/114240
More information about the cfe-commits
mailing list