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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
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. '");
+}
----------------
erichkeane wrote:
> 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?  
I think the test case would be something like:

```
struct A {
  union { unsigned: 10; };
  int x;
  constexpr A(int x) : x(x) {}
};

template <A a> void foo() {}
template void foo<A(5)>();
```

But also consider:

```
struct B {
  struct Nested {
    union { unsigned: 10; };
    int x;
  } nested;
  constexpr B(int x) : nested{5, x} {}
};

template <B b> void bar() {}
template void bar<B(10)>();
```


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

https://reviews.llvm.org/D122820