[clang][Sema] Don't try to initialize implicit variable of invalid anonymous union/struct

[TaWeiTu]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=49534, where the call to the constructor
of the anonymous union is checked and triggers assertion failure when trying to retrieve
the alignment of the this argument (which is a union with virtual function).

The extra check for alignment was introduced in D97187.

[TaWeiTu]

I'm not familiar with this code and clang/sema in general, so not sure if the fix makes sense.

[tmatheson]

This seems quite an early point to bail out, so it will prevent some errors/warnings associated with initialization from being emitted. For example, this warning is currently emitted but would be suppressed by this patch:

union {
  virtual int a();
  int b = 'c'; // expected-warning {{in-class initialization of non-static data member is a C++11 extension [-Wc++11-extensions]}}
}

I don't know how much that matters seeing as the union is invalid by this point.

[TaWeiTu]

In D98548#2625593, @tmatheson wrote:

This seems quite an early point to bail out, so it will prevent some errors/warnings associated with initialization from being emitted. For example, this warning is currently emitted but would be suppressed by this patch:

union {
  virtual int a();
  int b = 'c'; // expected-warning {{in-class initialization of non-static data member is a C++11 extension [-Wc++11-extensions]}}
}

I don't know how much that matters seeing as the union is invalid by this point.

Hi,

Thanks for the comment!
I understand the problem, but the warning you mentioned is not suppressed in this patch from what I've seen.
After poking around, I saw that diagnostics related to anonymous unions are all emitted in another call to ActOnUninitializedDecl in the parser, which takes place before this check.
If I completely remove this call to ActOnUninitializedDecl (on line 5215 to be specific), all the failed testcases seem to be related to CodeGen, so I suspect that removing this call for invalid unions will not suppress further warnings/errors.

What do you think?

[TaWeiTu]

ping.

[tmatheson]

That makes sense, you are correct that that warning is not prevented. I'm not sure what I did differently when I checked. LGTM.

clang/lib/Sema/SemaDecl.cpp

if (RecordDecl *OwningClass = dyn_cast<RecordDecl>(Owner)) {
     Anon = VarDecl::Create(Context, Owner, DS.getBeginLoc(),
                            Record->getLocation(), /*IdentifierInfo=*/nullptr,
                            Context.getTypeDeclType(Record), TInfo, SC);
 
     // Default-initialize the implicit variable. This initialization will be
     // trivial in almost all cases, except if a union member has an in-class
     // initializer:
     //   union { int n = 0; };
+    if (!Invalid)
       ActOnUninitializedDecl(Anon);
   }
   Anon->setImplicit();
 
   // Mark this as an anonymous struct/union type.
   Record->setAnonymousStructOrUnion(true);
 
   // Add the anonymous struct/union object to the current
   // context. We'll be referencing this object when we refer to one of

clang/test/SemaCXX/PR49534.cpp

+// RUN: %clang_cc1 -x c++ -fsyntax-only %s -verify
+
+static union {     // expected-warning {{declaration does not declare anything}}
+  virtual int a(); // expected-error {{unions cannot have virtual functions}} \
+                   // expected-error {{functions cannot be declared in an anonymous union}}
+};