<div dir="ltr"><div class="gmail_extra"><div class="gmail_extra">On Fri, Dec 12, 2014 at 6:53 AM, Nathan Sidwell <span dir="ltr"><<a href="mailto:nathan@acm.org" target="_blank">nathan@acm.org</a>></span> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">ping?<div><div class="h5"><br>
<br>
On 12/05/14 16:57, Nathan Sidwell wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
This (proto) patch addresses bug 6037 <a href="http://llvm.org/bugs/show_bug.cgi?id=6037" target="_blank">http://llvm.org/bugs/show_bug.<u></u>cgi?id=6037</a><br>
A request for a class definition to warn about inaccessible direct base classes.<br>
  I've  added a check in Sema::AttachBaseSpecifiers to iterate over the direct<br>
base array looking for ambiguous conversions to the individual bases.  The<br>
output is something like:<br>
<br>
6037.cc:8:15: warning: direct base 'A' is inaccessible due to ambiguity:<br>
     struct C -> struct B -> struct A<br>
     struct C -> struct A<br>
struct C : B, A {<br>
               ^<br>
which matches the form of error for an ambiguous base conversion itself.<br></blockquote></div></div></blockquote><div><br></div><div>This seems like a good idea for a warning to me.<div><br></div><div><div class="gmail_extra"></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">This patch triggers on a pile of testcases, which I've not yet fixed, because<br>
I'd like some feedback on this approach first -- is there perhaps a better way<br>
to do this checking?<br></blockquote></div></div></blockquote><div><br></div><div><div class="gmail_extra">+    CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,</div><div class="gmail_extra">+                     /*DetectVirtual=*/true);</div><div class="gmail_extra">+    (void)IsDerivedFrom(Context.getTagDeclType(Class), BaseType, Paths);</div><div class="gmail_extra">+    if (Paths.isAmbiguous(Context.getCanonicalType(BaseType)</div><div class="gmail_extra">+<span class="" style="white-space:pre">                      </span>  .getUnqualifiedType ()))</div><div><br></div><div>This will do a lot of redundant work if there are multiple base classes. Instead, construct the CXXBasePaths object outside of the loop and initialize it by calling Class->lookupInBases with a predicate that always matches, then just call Paths.isAmbiguous within the loop.</div><div><br></div><div>That said, you should be able to do this even more efficiently by collecting a list of all indirect base classes then checking if any direct base class is on the list. (If any is, you'll still need the code you already have, in order to describe the ambiguity.)</div><div><br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Also, as this is a warning, I presume there should be some way to disable it.<br>
How is that achieved in the clang framework?  (I'm not sure if the now failing<br>
tests should be fixed by disabling the warning, for instance).</blockquote></div></div></blockquote><div><br></div><div>Like so:</div><div><br></div><div>def warn_inaccessible_base_class : Warning<</div><div>  "direct base %0 is inaccessible due to ambiguity:%1">,</div><div>  InGroup<DiagGroup<"warning-flag">>;</div><div><br></div><div>If there are multiple warnings controlled by the flag (or you want to put this group inside another group), the DiagGroup should be factored out (see DiagnosticGroups.td).</div></div></div></div>