<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 24, 2014 at 12:39 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi rnk, rsmith,<br>
<br>
If the compiler is choosing pointer members using a best-case scheme,<br>
diagnose when we select the unspecified representation.  It is unlikely<br>
that this is what the user intended.<br>
<br>
<a href="http://reviews.llvm.org/D4651" target="_blank">http://reviews.llvm.org/D4651</a><br>
<br>
Files:<br>
  include/clang/Basic/DiagnosticGroups.td<br>
  include/clang/Basic/DiagnosticSemaKinds.td<br>
  lib/Sema/SemaDeclAttr.cpp<br>
  test/SemaCXX/member-pointer-ms.cpp<br>
<br>
Index: include/clang/Basic/DiagnosticGroups.td<br>
===================================================================<br>
--- include/clang/Basic/DiagnosticGroups.td<br>
+++ include/clang/Basic/DiagnosticGroups.td<br>
@@ -324,6 +324,7 @@<br>
 def Varargs : DiagGroup<"varargs">;<br>
<br>
 def Unsequenced : DiagGroup<"unsequenced">;<br>
+def UnspecifiedInheritanceModel : DiagGroup<"ms-unspecified-inheritance-model">;<br>
 // GCC name for -Wunsequenced<br>
 def : DiagGroup<"sequence-point", [Unsequenced]>;<br>
<br>
Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
===================================================================<br>
--- include/clang/Basic/DiagnosticSemaKinds.td<br>
+++ include/clang/Basic/DiagnosticSemaKinds.td<br>
@@ -2505,6 +2505,10 @@<br>
   InGroup<IgnoredAttributes>;<br>
 def note_previous_ms_inheritance : Note<<br>
   "previous inheritance model specified here">;<br>
+def warn_unspecified_ms_inheritance : Warning<<br>
+  "the 'full_generality' inheritance model was selected for this class, "<br>
+  "this will result in very large pointers-to-members">,<br></blockquote><div><br></div><div>I would have no idea what to do when the compiler tells me this. ("I suppose this is true, but so what?") Should there be a note that says what to do to prevent this? ("define class above this statement or use __foo_inheritance on declaration" or similar)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+  InGroup<UnspecifiedInheritanceModel>, DefaultIgnore;<br></blockquote><div><br></div><div>I think the usual guidance for warnings is "useful enough to be on by default, or don't have it". Since this is warning is more about optimizable code and not about correctness, I think this is ok in this case.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 def err_machine_mode : Error<"%select{unknown|unsupported}0 machine mode %1">;<br>
 def err_mode_not_primitive : Error<<br>
   "mode attribute only supported for integer and floating-point types">;<br>
Index: lib/Sema/SemaDeclAttr.cpp<br>
===================================================================<br>
--- lib/Sema/SemaDeclAttr.cpp<br>
+++ lib/Sema/SemaDeclAttr.cpp<br>
@@ -3917,6 +3917,14 @@<br>
     }<br>
   }<br>
<br>
+  // The user probably did not want the unspecified inheritance model in the<br>
+  // best case.  Diagnose this case so that the code could be reworked by<br>
+  // providing a definition prior to the use of a pointer-to-member or via the<br>
+  // use of inheritance keywords.<br>
+  if (MSPointerToMemberRepresentationMethod == LangOptions::PPTMK_BestCase &&<br>
+      SemanticSpelling == MSInheritanceAttr::Keyword_unspecified_inheritance)<br>
+    Diag(Range.getBegin(), diag::warn_unspecified_ms_inheritance);<br>
+<br>
   return ::new (Context)<br>
       MSInheritanceAttr(Range, Context, BestCase, AttrSpellingListIndex);<br>
 }<br>
Index: test/SemaCXX/member-pointer-ms.cpp<br>
===================================================================<br>
--- test/SemaCXX/member-pointer-ms.cpp<br>
+++ test/SemaCXX/member-pointer-ms.cpp<br>
@@ -1,5 +1,5 @@<br>
-// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=i386-pc-win32 -verify -DVMB %s<br>
-// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify -DVMB %s<br>
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=i386-pc-win32 -verify -DVMB -Wms-unspecified-inheritance-model %s<br>
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify -DVMB -Wms-unspecified-inheritance-model %s<br>
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify -DVMV -fms-memptr-rep=virtual %s<br>
 //<br>
 // This file should also give no diagnostics when run through cl.exe from MSVS<br>
@@ -167,6 +167,9 @@<br>
<br>
 // Re-declare to force us to iterate decls when adding attributes.<br>
 struct ForwardDecl1;<br>
+#ifdef VMB<br>
+// expected-warning@-2 {{the 'full_generality' inheritance model was selected}}<br>
+#endif<br>
 struct ForwardDecl2;<br>
<br>
 typedef int ForwardDecl1::*MemPtr1;<br>
@@ -212,6 +215,9 @@<br>
 };<br>
<br>
 struct NewUnspecified;<br>
+#ifdef VMB<br>
+// expected-warning@-2 {{the 'full_generality' inheritance model was selected}}<br>
+#endif<br>
 SingleTemplate<void (IncSingle::*)()> tmpl_single;<br>
 UnspecTemplate<void (NewUnspecified::*)()> tmpl_unspec;<br>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>