<div dir="ltr">Thank you!</div><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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Mar 21, 2017 at 9:15 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">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">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-<wbr>project?rev=298421&view=rev</a><br>
>> Log:<br>
>> Prevent cppcoreguidelines-pro-bounds-<wbr>array-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/<wbr>ProBoundsArrayToPointerDecayCh<wbr>eck.cpp<br>
>><br>
>> clang-tools-extra/trunk/test/<wbr>clang-tidy/cppcoreguidelines-<wbr>pro-bounds-array-to-pointer-<wbr>decay.cpp<br>
>><br>
>> Modified:<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/<wbr>ProBoundsArrayToPointerDecayCh<wbr>eck.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-<wbr>project/clang-tools-extra/<wbr>trunk/clang-tidy/<wbr>cppcoreguidelines/<wbr>ProBoundsArrayToPointerDecayCh<wbr>eck.cpp?rev=298421&r1=298420&<wbr>r2=298421&view=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> ---<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/<wbr>ProBoundsArrayToPointerDecayCh<wbr>eck.cpp<br>
>> (original)<br>
>> +++<br>
>> clang-tools-extra/trunk/clang-<wbr>tidy/cppcoreguidelines/<wbr>ProBoundsArrayToPointerDecayCh<wbr>eck.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().<wbr>getSourceManager();<br>
>> + if (SM.isInSystemMacro(Node.<wbr>getLocStart())) {<br>
>> + if (isa<PredefinedExpr>(Node.<wbr>getSubExpr()))<br>
>> + return true;<br>
>> +<br>
>> + if (const auto *SymbolDeclRef =<br>
>> dyn_cast<DeclRefExpr>(Node.<wbr>getSubExpr())) {<br>
>> + const ValueDecl *SymbolDecl = SymbolDeclRef->getDecl();<br>
>> + if (SymbolDecl && SM.isInSystemHeader(<wbr>SymbolDecl->getLocation()))<br>
>> + return true;<br>
>> + }<br>
>> + }<br>
>> + return false;<br>
>> +}<br>
>> +<br>
>> void ProBoundsArrayToPointerDecayCh<wbr>eck::registerMatchers(<wbr>MatchFinder<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(<wbr>hasParent(arraySubscriptExpr()<wbr>)),<br>
>> + implicitCastExpr(<wbr>isArrayToPointerDecay(),<br>
>> + unless(hasParent(<wbr>arraySubscriptExpr())),<br>
>><br>
>> unless(<wbr>hasParentIgnoringImpCasts(<wbr>explicitCastExpr())),<br>
>> unless(<wbr>isInsideOfRangeBeginEndStmt())<wbr>,<br>
>> - unless(hasSourceExpression(<wbr>stringLiteral())))<br>
>> + unless(hasSourceExpression(<wbr>stringLiteral())),<br>
>> + unless(<wbr>sysSymbolDecayInSysHeader()))<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<<wbr>ImplicitCastExpr>("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/<wbr>clang-tidy/cppcoreguidelines-<wbr>pro-bounds-array-to-pointer-<wbr>decay.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-<wbr>project/clang-tools-extra/<wbr>trunk/test/clang-tidy/<wbr>cppcoreguidelines-pro-bounds-<wbr>array-to-pointer-decay.cpp?<wbr>rev=298421&r1=298420&r2=<wbr>298421&view=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> ---<br>
>> clang-tools-extra/trunk/test/<wbr>clang-tidy/cppcoreguidelines-<wbr>pro-bounds-array-to-pointer-<wbr>decay.cpp<br>
>> (original)<br>
>> +++<br>
>> clang-tools-extra/trunk/test/<wbr>clang-tidy/cppcoreguidelines-<wbr>pro-bounds-array-to-pointer-<wbr>decay.cpp<br>
>> Tue Mar 21 14:01:17 2017<br>
>> @@ -1,4 +1,5 @@<br>
>> // RUN: %check_clang_tidy %s<br>
>> cppcoreguidelines-pro-bounds-<wbr>array-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="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><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">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>