[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:5561
+  // field represents an anonymous record type.
+  const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
+
----------------
rjmccall wrote:
> Alas, bit-fields are also allowed to be anonymous.
Ah, good point, i'll update the comment and add a test.


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:5576
+      "program to refer to the anonymous union, and there is therefore no need "
+      "to mangle its name. '");
+}
----------------
rjmccall wrote:
> Unfortunately, I think class value mangling has a counter-example to this: you can have an anonymous struct or union which doesn't declare any named members but will of course nonetheless show up in the list of members in the enclosing class.  Code can even give it a non-zero initializer using list-initialization; that value can never be read, but it's there, and presumably it's part of uniquely determining a different template argument value and therefore needs to be mangled.
> 
> I don't know what we should do about this in the ABI, but we shouldn't crash in the compiler.
I'm not really understanding what you mean?  Can you provide an example?  All of the cases I could come up with like this ended up getting caught by the 'zero -init' case.

That said, I considered this 'unreachable' to be a distinct improvement over our current behavior which is to crash at effectively line 5558. 

Would you prefer some sort of 'fatal-error' emit here?  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122820/new/

https://reviews.llvm.org/D122820