[Sema] Mark virtual method declaration in union as invalid

[ychen]

Summary

Currently, this is only diagnosed but the decl is not marked invalid. This may hit assertions down the path.

This also reverts the fix for PR49534 since it is not needed anymore.

[hubert.reinterpretcast]

@ychen, an alternative fix would be to avoid getting asking for the layout for a broken union like this. Would it be appropriate to mark the RecordDecl in Sema such that isInvalidDecl returns true? Is that already happening?

[ychen]

Yeah, that would be better. I just realized it earlier today. I'll attempt that instead. Thanks for the suggestion.

[ychen]

• Mark virtual function in union invalid.

[hubert.reinterpretcast]

LGTM; thanks! fyi, I'll be on vacation for the next three weeks. I've added some additional reviewers to the patch based on prior reviews.

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
if (getLangOpts().CPlusPlus) {
     // return true).
     if (const CXXRecordDecl *Parent =
           dyn_cast<CXXRecordDecl>(NewFD->getDeclContext())) {
       if (Parent->isInterface() && cast<CXXMethodDecl>(NewFD)->isUserProvided())
         NewFD->setPure(true);
 
       // C++ [class.union]p2
       //   A union can have member functions, but not virtual functions.
-      if (isVirtual && Parent->isUnion())
+      if (isVirtual && Parent->isUnion()) {
         Diag(D.getDeclSpec().getVirtualSpecLoc(), diag::err_virtual_in_union);
+        NewFD->setInvalidDecl();
+      }
     }
 
     SetNestedNameSpecifier(*this, NewFD, D);
     isMemberSpecialization = false;
     isFunctionTemplateSpecialization = false;
     if (D.isInvalidType())
       NewFD->setInvalidDecl();
 

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}}
+  virtual int a(); // expected-error {{unions cannot have virtual functions}}
 };

clang/test/SemaCXX/virtual-function-in-union.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-union x {
-  virtual void f(); // expected-error {{unions cannot have virtual functions}}
+union U {
+  int d;
+  virtual int f() { return d; }; // expected-error {{unions cannot have virtual functions}}
 };
+
+int foo() { U u; return u.d; }