<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 21 March 2017 at 18:57, Breno Guimarães via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hey guys,<div><br></div><div>I'm sorry the test did not make the decay explicit. It's indeed sort of tricky: <a href="https://bugs.llvm.org/show_bug.cgi?id=32239" target="_blank">https://bugs.llvm.org/<wbr>show_bug.cgi?id=32239</a></div><div><br></div><div>Basically, assert is expanded into:</div><div><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) </pre></div></div></blockquote><div>Maybe in your libc, and even in most libc's, but not in general, and we can't and shouldn't rely on that in this test. (And there may not even *be* a usable <assert.h> in the include path...)</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">Well, __func__ and __FILE__ are const char[], while __assert_rtn expects const char*.</pre></div></div></blockquote><div>I would think __func__ (and its friends __FUNCTION__, __PRETTY_FUNCTION__, etc) should be exempted from this warning just like decay on a string literal is. __FILE__ is string literal, so should be exempted already. Perhaps checking for a system header is not the right change at all?</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">I've also seen this in my own environment (Linux) with slightly different function names, but basically the same problem.</pre><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">Any ideas of how to rewrite the test (or the code?) to have the change pushed?</pre></div></div></blockquote><div>For the current approach: add a header file (to Inputs/...) that contains the macro you want to use and add a -isystem path to the test pointing at your Inputs/... directory. To whitelist conversions of __func__ etc you should also check for PredefinedExpr wherever we're currently checking for StringLiteral.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">Thanks!</pre><pre class="gmail-m_-5044091989478664758gmail-bz_comment_text" style="white-space:pre-wrap;width:50em;color:rgb(0,0,0)">Breno G.</pre></div><div><br></div><div><br></div></div><div class="gmail_extra"><div><div class="gmail-h5"><br><div class="gmail_quote">On Tue, Mar 21, 2017 at 10:36 PM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thank you!</div><div class="gmail-m_-5044091989478664758HOEnZb"><div class="gmail-m_-5044091989478664758h5"><div class="gmail_extra"><br><div class="gmail_quote">On 21 March 2017 at 18:22, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_-5044091989478664758m_4153817042893638496HOEnZb"><div class="gmail-m_-5044091989478664758m_4153817042893638496h5">On Tue, Mar 21, 2017 at 9:15 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
> On 21 March 2017 at 12:01, Aaron Ballman via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: aaronballman<br>
>> Date: Tue Mar 21 14:01:17 2017<br>
>> New Revision: 298421<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=298421&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=298421&view=rev</a><br>
>> Log:<br>
>> Prevent cppcoreguidelines-pro-bounds-a<wbr>rray-to-pointer-decay from<br>
>> diagnosing array to pointer decay stemming from system macros.<br>
>><br>
>> Patch by Breno Rodrigues Guimaraes.<br>
>><br>
>> Modified:<br>
>><br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/ProBoun<wbr>dsArrayToPointerDecayCheck.cpp<br>
>><br>
>> clang-tools-extra/trunk/test/c<wbr>lang-tidy/cppcoreguidelines-pr<wbr>o-bounds-array-to-pointer-deca<wbr>y.cpp<br>
>><br>
>> Modified:<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/ProBoun<wbr>dsArrayToPointerDecayCheck.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp?rev=298421&r1=298420&r2=298421&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/clang-tools-extra/trunk/<wbr>clang-tidy/cppcoreguidelines/P<wbr>roBoundsArrayToPointerDecayChe<wbr>ck.cpp?rev=298421&r1=298420&r2<wbr>=298421&view=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> ---<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/ProBoun<wbr>dsArrayToPointerDecayCheck.cpp<br>
>> (original)<br>
>> +++<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/ProBoun<wbr>dsArrayToPointerDecayCheck.cpp<br>
>> Tue Mar 21 14:01:17 2017<br>
>> @@ -47,6 +47,25 @@ AST_MATCHER_P(Expr, hasParentIgnoringImp<br>
>> return InnerMatcher.matches(*E, Finder, Builder);<br>
>> }<br>
>><br>
>> +AST_MATCHER(ImplicitCastExpr, isArrayToPointerDecay) {<br>
>> + return Node.getCastKind() == CK_ArrayToPointerDecay;<br>
>> +}<br>
>> +<br>
>> +AST_MATCHER(ImplicitCastExpr, sysSymbolDecayInSysHeader) {<br>
>> + auto &SM = Finder->getASTContext().getSou<wbr>rceManager();<br>
>> + if (SM.isInSystemMacro(Node.getLo<wbr>cStart())) {<br>
>> + if (isa<PredefinedExpr>(Node.getS<wbr>ubExpr()))<br>
>> + return true;<br>
>> +<br>
>> + if (const auto *SymbolDeclRef =<br>
>> dyn_cast<DeclRefExpr>(Node.get<wbr>SubExpr())) {<br>
>> + const ValueDecl *SymbolDecl = SymbolDeclRef->getDecl();<br>
>> + if (SymbolDecl && SM.isInSystemHeader(SymbolDecl<wbr>->getLocation()))<br>
>> + return true;<br>
>> + }<br>
>> + }<br>
>> + return false;<br>
>> +}<br>
>> +<br>
>> void ProBoundsArrayToPointerDecayCh<wbr>eck::registerMatchers(MatchFin<wbr>der<br>
>> *Finder) {<br>
>> if (!getLangOpts().CPlusPlus)<br>
>> return;<br>
>> @@ -56,10 +75,12 @@ void ProBoundsArrayToPointerDecayCh<wbr>eck::<br>
>> // 2) inside a range-for over an array<br>
>> // 3) if it converts a string literal to a pointer<br>
>> Finder->addMatcher(<br>
>> - implicitCastExpr(unless(hasPar<wbr>ent(arraySubscriptExpr())),<br>
>> + implicitCastExpr(isArrayToPoin<wbr>terDecay(),<br>
>> + unless(hasParent(arraySubscri<wbr>ptExpr())),<br>
>><br>
>> unless(hasParentIgnoringImpCas<wbr>ts(explicitCastExpr())),<br>
>> unless(isInsideOfRangeBeginEn<wbr>dStmt()),<br>
>> - unless(hasSourceExpression(st<wbr>ringLiteral())))<br>
>> + unless(hasSourceExpression(st<wbr>ringLiteral())),<br>
>> + unless(sysSymbolDecayInSysHea<wbr>der()))<br>
>> .bind("cast"),<br>
>> this);<br>
>> }<br>
>> @@ -67,8 +88,6 @@ void ProBoundsArrayToPointerDecayCh<wbr>eck::<br>
>> void ProBoundsArrayToPointerDecayCh<wbr>eck::check(<br>
>> const MatchFinder::MatchResult &Result) {<br>
>> const auto *MatchedCast =<br>
>> Result.Nodes.getNodeAs<Implici<wbr>tCastExpr>("cast");<br>
>> - if (MatchedCast->getCastKind() != CK_ArrayToPointerDecay)<br>
>> - return;<br>
>><br>
>> diag(MatchedCast->getExprLoc()<wbr>, "do not implicitly decay an array into<br>
>> a "<br>
>> "pointer; consider using<br>
>> gsl::array_view or "<br>
>><br>
>> Modified:<br>
>> clang-tools-extra/trunk/test/c<wbr>lang-tidy/cppcoreguidelines-pr<wbr>o-bounds-array-to-pointer-deca<wbr>y.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp?rev=298421&r1=298420&r2=298421&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/clang-tools-extra/trunk/<wbr>test/clang-tidy/cppcoreguideli<wbr>nes-pro-bounds-array-to-pointe<wbr>r-decay.cpp?rev=298421&r1=<wbr>298420&r2=298421&view=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> ---<br>
>> clang-tools-extra/trunk/test/c<wbr>lang-tidy/cppcoreguidelines-pr<wbr>o-bounds-array-to-pointer-deca<wbr>y.cpp<br>
>> (original)<br>
>> +++<br>
>> clang-tools-extra/trunk/test/c<wbr>lang-tidy/cppcoreguidelines-pr<wbr>o-bounds-array-to-pointer-deca<wbr>y.cpp<br>
>> Tue Mar 21 14:01:17 2017<br>
>> @@ -1,4 +1,5 @@<br>
>> // RUN: %check_clang_tidy %s<br>
>> cppcoreguidelines-pro-bounds-a<wbr>rray-to-pointer-decay %t<br>
>> +#include <assert.h><br>
><br>
><br>
> This test is using a header that we do not supply (unlike stddef.h which<br>
> Clang provides itself). That does not seem especially reasonable to me; this<br>
> test is failing for us as a result. Can you supply a fake <assert.h> system<br>
> header as an input to this test?<br>
><br>
>><br>
>> #include <stddef.h><br>
>><br>
>> namespace gsl {<br>
>> @@ -34,6 +35,11 @@ void f() {<br>
>><br>
>> for (auto &e : a) // OK, iteration internally decays array to pointer<br>
>> e = 1;<br>
>> +<br>
>> + assert(false); // OK, array decay inside system header macro<br>
><br>
><br>
> Huh? What decay is this referring to?<br>
<br>
</div></div>I am now wondering the same question...<br>
<br>
I've rolled this back r298470 and will follow up with the author.<br>
<br>
Thanks!<br>
<span class="gmail-m_-5044091989478664758m_4153817042893638496HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="gmail-m_-5044091989478664758m_4153817042893638496HOEnZb"><div class="gmail-m_-5044091989478664758m_4153817042893638496h5"><br>
><br>
>> +<br>
>> + assert(a);<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not implicitly decay an<br>
>> array into a pointer; consider using gsl::array_view or an explicit cast<br>
>> instead [cppcoreguidelines-pro-bounds-<wbr>array-to-pointer-decay]<br>
>> }<br>
>><br>
>> const char *g() {<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div></div></div><span class="gmail-HOEnZb"><font color="#888888">-- <br><div class="gmail-m_-5044091989478664758gmail_signature">Breno Rodrigues Guimarães<div>Universidade Federal de Minas Gerais - UFMG, Brasil</div><div>(Federal University of Minas Gerais, Brazil)</div><div><br></div><div><br></div></div>
</font></span></div>
<br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>