<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 24, 2014 at 7:11 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">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></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 class="">
<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><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></div></blockquote><div><br></div><div>The idea is similar to -Wpadded, that diagnostic does not presume to answer how to minimize the padding in your struct. Likewise, there are many ways to silence this diagnostic depending on the TU:</div>
<div>1. Move the definition above the point where in the inheritance model is selected.</div><div>2. Use an inheritance keyword like __single_inheritance.</div><div>3. Use #pragma pointers_to_members to select an inheritance model.</div>
<div>4. Use the /vmg and /vm{s,m,v} flags</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
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></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">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>
</blockquote></div><br></div></div>