LGTM with minor tweaks.

REPOSITORY
  rL LLVM

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1266
@@ -1265,1 +1265,3 @@
+def err_virtual_in_union : Error<
+  "function %0 declared 'virtual' within a union">;
 def err_virtual_non_function : Error<
----------------
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?

================
Comment at: lib/Sema/SemaDecl.cpp:7211
@@ +7210,3 @@
+            diag::err_virtual_in_union) << NewFD->getDeclName();
+        NewFD->setInvalidDecl();
+      }
----------------
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...)

http://reviews.llvm.org/D10752

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to