rjmccall accepted this revision.
rjmccall added inline comments.
This revision is now accepted and ready to land.
================
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:
> > 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)>();
> > ```
> With my current patch, that first example mangles to:
> `@_Z3fooIXtl1ALNS0_Ut_EELi5EEEEvv` (that is, it doesn't touch this crash).
> Unfortunately, that doesn't 'demangle' with llvm-cxxfilt.
>
> For the 2nd example, i had to modify it to be:
>
> ```
> struct B {
> struct Nested {
> union {
> unsigned : 10;
> };
> int x;
> } nested;
> constexpr B(int x) : nested{{}, x} {}
> };
>
>
> template <B b> void bar() {}
> template void bar<B(10)>();
>
> ```
>
> As you wrote it, we get the error:
> `temp.cpp:20:31: error: initializer for aggregate with no elements requires
> explicit braces `
>
> I tried putting a '5' in those brackets, and get:
> `temp.cpp:20:32: error: excess elements in union initializer`
>
>
> HOWEVER, I think it makes sense to have a Diag here rather than an
> llvm-unreachable, so I've done that instead.
Hmm, alright, works for me.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122820/new/
https://reviews.llvm.org/D122820
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
