This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Union(s) cannot have virtual functions
ClosedPublic

Authored by davide on Jun 25 2015, 4:54 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 28516.Jun 25 2015, 4:54 PM
davide retitled this revision from to [Sema] Union(s) cannot have virtual functions.
davide updated this object.
davide edited the test plan for this revision. (Show Details)
davide added reviewers: rsmith, doug.gregor.
davide set the repository for this revision to rL LLVM.
davide added a subscriber: Unknown Object (MLST).

Example after patching:

$ clang++ union.cpp -o union
union.cpp:2:3: error: function 'f' declared 'virtual' within a union

virtual void f(...);
^

1 error generated.

g++ output, for reference

$ g++5 union.cpp -o union
union.cpp:2:21: error: function 'f' declared virtual inside a union

virtual void f(...);
                  ^
rsmith accepted this revision.Jun 25 2015, 6:44 PM
rsmith edited edge metadata.

LGTM with minor tweaks.

include/clang/Basic/DiagnosticSemaKinds.td
1266 ↗(On Diff #28516)

This diagnostic is true, but doesn't actually explain what the problem is to someone unaware of the language rule. Also, listing the function name doesn't seem useful as the diagnostic is pointing at the function. Would just "unions cannot have virtual functions" work for you?

lib/Sema/SemaDecl.cpp
7211 ↗(On Diff #28516)

Do we need to mark the function as invalid? It seems like we should be able to recover from this case fine, without any follow-on problems. (In fact, it's a little disturbing how /well/ we support unions with virtual functions today...)

This revision is now accepted and ready to land.Jun 25 2015, 6:44 PM
This revision was automatically updated to reflect the committed changes.

Tweaked after your suggestions and committed as r240889.