[clang-tools-extra] r298421 - Prevent cppcoreguidelines-pro-bounds-array-to-pointer-decay from diagnosing array to pointer decay stemming from system macros.

[Aaron Ballman via cfe-commits]

On Tue, Mar 21, 2017 at 9:15 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On 21 March 2017 at 12:01, Aaron Ballman via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: aaronballman
>> Date: Tue Mar 21 14:01:17 2017
>> New Revision: 298421
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=298421&view=rev
>> Log:
>> Prevent cppcoreguidelines-pro-bounds-array-to-pointer-decay from
>> diagnosing array to pointer decay stemming from system macros.
>>
>> Patch by Breno Rodrigues Guimaraes.
>>
>> Modified:
>>
>> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
>>
>> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp?rev=298421&r1=298420&r2=298421&view=diff
>>
>> ==============================================================================
>> ---
>> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
>> Tue Mar 21 14:01:17 2017
>> @@ -47,6 +47,25 @@ AST_MATCHER_P(Expr, hasParentIgnoringImp
>>    return InnerMatcher.matches(*E, Finder, Builder);
>>  }
>>
>> +AST_MATCHER(ImplicitCastExpr, isArrayToPointerDecay) {
>> +  return Node.getCastKind() == CK_ArrayToPointerDecay;
>> +}
>> +
>> +AST_MATCHER(ImplicitCastExpr, sysSymbolDecayInSysHeader) {
>> +  auto &SM = Finder->getASTContext().getSourceManager();
>> +  if (SM.isInSystemMacro(Node.getLocStart())) {
>> +    if (isa<PredefinedExpr>(Node.getSubExpr()))
>> +      return true;
>> +
>> +    if (const auto *SymbolDeclRef =
>> dyn_cast<DeclRefExpr>(Node.getSubExpr())) {
>> +      const ValueDecl *SymbolDecl = SymbolDeclRef->getDecl();
>> +      if (SymbolDecl && SM.isInSystemHeader(SymbolDecl->getLocation()))
>> +        return true;
>> +    }
>> +  }
>> +  return false;
>> +}
>> +
>>  void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder
>> *Finder) {
>>    if (!getLangOpts().CPlusPlus)
>>      return;
>> @@ -56,10 +75,12 @@ void ProBoundsArrayToPointerDecayCheck::
>>    // 2) inside a range-for over an array
>>    // 3) if it converts a string literal to a pointer
>>    Finder->addMatcher(
>> -      implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
>> +      implicitCastExpr(isArrayToPointerDecay(),
>> +                       unless(hasParent(arraySubscriptExpr())),
>>
>> unless(hasParentIgnoringImpCasts(explicitCastExpr())),
>>                         unless(isInsideOfRangeBeginEndStmt()),
>> -                       unless(hasSourceExpression(stringLiteral())))
>> +                       unless(hasSourceExpression(stringLiteral())),
>> +                       unless(sysSymbolDecayInSysHeader()))
>>            .bind("cast"),
>>        this);
>>  }
>> @@ -67,8 +88,6 @@ void ProBoundsArrayToPointerDecayCheck::
>>  void ProBoundsArrayToPointerDecayCheck::check(
>>      const MatchFinder::MatchResult &Result) {
>>    const auto *MatchedCast =
>> Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
>> -  if (MatchedCast->getCastKind() != CK_ArrayToPointerDecay)
>> -    return;
>>
>>    diag(MatchedCast->getExprLoc(), "do not implicitly decay an array into
>> a "
>>                                    "pointer; consider using
>> gsl::array_view or "
>>
>> Modified:
>> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
>> URL:
>> 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
>>
>> ==============================================================================
>> ---
>> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
>> Tue Mar 21 14:01:17 2017
>> @@ -1,4 +1,5 @@
>>  // RUN: %check_clang_tidy %s
>> cppcoreguidelines-pro-bounds-array-to-pointer-decay %t
>> +#include <assert.h>
>
>
> This test is using a header that we do not supply (unlike stddef.h which
> Clang provides itself). That does not seem especially reasonable to me; this
> test is failing for us as a result. Can you supply a fake <assert.h> system
> header as an input to this test?
>
>>
>>  #include <stddef.h>
>>
>>  namespace gsl {
>> @@ -34,6 +35,11 @@ void f() {
>>
>>    for (auto &e : a) // OK, iteration internally decays array to pointer
>>      e = 1;
>> +
>> +  assert(false); // OK, array decay inside system header macro
>
>
> Huh? What decay is this referring to?

I am now wondering the same question...

I've rolled this back r298470 and will follow up with the author.

Thanks!

~Aaron

>
>> +
>> +  assert(a);
>> +  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not implicitly decay an
>> array into a pointer; consider using gsl::array_view or an explicit cast
>> instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
>>  }
>>
>>  const char *g() {
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>