[clang] [clang] Fix ASTWriter crash after merging named enums (PR #114240)

Michael Jabbour via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 26 12:54:49 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:

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](https://godbolt.org/#z:OYLghAFBqd5TKALEAXATgU0wGgJYgQDGAtgIYDWmAgusAM4gDkAtACIDCAstQNICiAfQBCAVQCSAGTaCAKgE0ACvwCkAZjZtMAIwCuwJjiIB7EgAc8AG0zpJZAHbBdZYJnEATEERUAmYb%2BEjXXpUUwB5XVQzSIAxK0x7MhJMNEwQnAAzeMZYTHc8UPQPEB9M%2BOK1MutE5JB6JDIsdwA6JBxQJkM8enF7IktddzzmAAYu%2Bi4yPHsAZWNddCIUpgBGcbCzBNHDSwdgYs6AShwIPILjIs9KrOtigBYqhKSUzAAPJLNrZqIzM3bmca9fqDYZMMb4CZTWbzRbLcHdDZbME7PYHJjHU75QrFFaPYoANkeNRSJGM7l0X1J7nIfw6gL6AyGnmREMm0zmCyWzDWEMR9m2OF2jmK3j8AQxZ2xngeNzcngA7ETniAuGTqHFrCtWv9OhCgYzQfDIeyYVzVutNvyWUL9p5Rf4/BKsRdigBWPGeAAcStqqvc6viPm1dL1DJBzKNbOhnOWPIRloFNpFAXFJ0lLs8hNlxQAnD6Un7hBrMMGAaHgUyBd0oxzYdyLUjwUm7SnHcdutxKJhFOhjAArTBEVD1nB9kC4%2ByYADuxZxI2OlmYrsMVvBxmYsiwmAA1L41Dzh8iMSgTEN%2BM70PhCLLiSOMsZ7KgZkQyNZxzKH6hRPRu68BWP3RtLxW0CMwHEwAA5ZU/QDawgzaH9rCHPAH0IBJ3A4YwBhIflcXQyRpkg3QSG0GwShwMxjHoAoUPsTDsNwiiqJoh8CMnCDiNI9ByMQwdUFomZUEaVB6OIxjeOQh9BOEtiiJIsjShCYTRJw8ccCU9BUFkjj5O4nxjnoU1lgAemM7d%2BHMVAAE9t1lFQRmodFBSXFcBXXGDi3gnBDzGY8vDJTBz3OS8CAgG9lUMmNWl%2BTJP2fV8UhWD9H2/X9/3IoD7QCCjwKg2p6kaPJtQk/jUNOewMKwsS1PwwidK48jKOo0q6Kq1TcSalj7G0ziFPUzAkJa6TNJU8SBr4gShM0nrdJ4qaRLa8T5pmhr9PUozmASYjty4Kz%2BHsYifF3eVhB2qyADVXxEY62HUfwHKcxcmGXHBV0MdcCqabUfKciATwCoLCivML4lvchpm%2BGL70feK3yS2KUp/RQ/xZMdKkykCcvY5U3g%2BL4fj%2BEraLQirRpKUpavY3ruMqTqWrJnxSjp2iVrIyoiak%2BaGcU8bJNmZa6upkB2a5xbyfUgWqdmtQDI2pg92mCsd18HxC2LVoVfs6gtemVBt3B%2BwIEOY7TqwVAFnsbcODum6tcelzXrckBcfMfGYp%2BvzT0Ci9gfC2pzQRp8Xzh5KvyRlHwTHXEMbFPwscg5UqQpEsqRpfrBuJ8rKoYmqKtZ7iVkJZmHzJjrmJa/Px0JDn%2BeUsXcRr4atMF2bC4luuc4byW5IawvZZjZgk%2BsHa1RNrXt31slk5H/1ixWMeHIniekEwMghnQXdGY8%2BItSQTXF6Xt5KM07cACpx9tg%2Bh53be4IX6gl%2B3Fe15sTfVbVTyNcZi%2BJ6Pi49fPgfFQ8pboOWAaA7WDlr4z1OsA%2B6D8n6r3Xm/NW8Qv4%2BAvn/E%2BgDtYgLtguB2b0cDrmvs0VOZA/gexOP9M8PtQp%2B1jIYaGQcErvkDqlZG6UHgxwdKBXK0EP472KrzFqJNs7VTwnnFuDVabl1oqXJizUWbSLZunCanMO4SLUXzJulcRaaPau3aaKiab9zrPLBy1lNhDAyNuLaJATZWxuuZO6%2BDnLPVciydygjNTfW2IcY4z916EGOBQEAroRjuLuIYEg4TIlEPXLqRg7oqFwFgDQ72wVfag2VAHZhsNEqhw4RHQwY5CQ8OymBbGvoyRFjQQhERmd0IKMpj3MiRc5El3roorqldq6NI0SNbpjdu71XaUYhancJl9LMWaUyMDizbiGJadw9BtwPhnrBTA8F1n2EsFZZoWsFZhiGCgnx2z0FHLAZYqy1jMC2PsY4jgOBtxsGcaoNQ8D7YeMdl4lUtT1ZtA9oEpBZEjZRJiXE35a4yyMEicCnAYSInuLUJ4mFupUmIDScgfy5h4ghUICYPF1huL9D2IIMg9AfyaVovQQQGBdD2AoFUVANhGAQG0AKbQ0xGhWRHNyxI6ArJhG0AOIcI4iXJEfGEPZfKWRDD0MADgr5LCMADkMcgjg3zyrwFgZCAA3NIAo3iDkiIw/Aj4BqJjwNodAvLMJDAFBgPAsSA6GvQNoKimAtCauAJYaYHRfKZF2Awc6eBpx8jvHFYOhT2Hh3StHPYwFY6BH9doHI4LjBmBamqlgrxtxEG3CwDI181nFuviwGkxb4jqDYKQ8hZgi0ZHoFZR8ZBXgsAfPsotAANMljgi2UtQCwck5gWAqveu69AeAhgZoMgM/kWcWlSKlr3Hp9NunF26iYtSIyDFjQzoM5uq6yJd33bu0ZQsVjHAwNgccBCfkJOYNuV4np8QsHxHcAtNptwQEpdSnN24GVMuNhATc2BN77kOIYBFQSwUPuiTgWJrotTynxJ6NQOYVg%2BHxFhnMOZ8TyndE%2BpJIB4X%2BJOFilA8woiRGBkSiwJK8rLFfe%2Bz9369jbhWDmZoWpwT5JjWw5hxSE3eS3DOtSU47VmHrA%2Bl6JGIi0b1sYWxrGP1fv7cAX9/6bCAeAxQUDmFiWvz3NemD/iEOQruJ6ZoPh9z4hGJ6eUahHMjBGGoHwxGnakfI0eUJUKnqouhe9MsCKno%2BDRSFjFFmcDuuoqhO4QA), 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 for what I mentioned above, 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 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.

https://github.com/llvm/llvm-project/pull/114240


More information about the cfe-commits mailing list