aaron.ballman added inline comments. ================ Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:29 @@ +28,3 @@ + implicitCastExpr(unless(hasParent(arraySubscriptExpr())), + unless(hasSourceExpression(declRefExpr(to(varDecl(hasName("__range")))))), + unless(hasSourceExpression(stringLiteral())) ---------------- mgehre wrote: > sbenza wrote: > > __range is an implementation detail and should not be used here. > > Check that this is the range init of a cxxForRangeStmt parent node or > > something like that. > That was my first attempt, but I couldn't quite figure it out. > > I tried > > unless(hasAncestor(cxxForRangeStmt(hasRangeInit(hasDescendant(equalsBoundNode("cast")))))) > but that does not compile. > > Any ideas? I'm not certain that you'll be able to do that. Looking at the AST dump:
``` TranslationUnitDecl 0xa40168 <<invalid sloc>> <invalid sloc> |-TypedefDecl 0xa40458 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'char *' `-FunctionDecl 0xa404c0 <E:\Desktop\test1.cpp:1:1, line:5:1> line:1:6 f 'void (void)' `-CompoundStmt 0xa40b88 <col:10, line:5:1> |-DeclStmt 0xa40600 <line:2:3, col:11> | `-VarDecl 0xa405c8 <col:3, col:10> col:7 used a 'int [5]' `-CXXForRangeStmt 0xa40b50 <line:3:3, line:4:5> |-DeclStmt 0xa40828 <line:3:16> | `-VarDecl 0xa406b8 <col:16> col:16 implicit used __range 'int (&&)[5]' cinit | `-DeclRefExpr 0xa40610 <col:16> 'int [5]' lvalue Var 0xa405c8 'a' 'int [5]' |-DeclStmt 0xa40a00 <col:14> | |-VarDecl 0xa40870 <col:14> col:14 implicit used __begin 'int *':'int *' cinit | | `-ImplicitCastExpr 0xa40960 <col:14> 'int *' <ArrayToPointerDecay> | | `-DeclRefExpr 0xa40838 <col:14> 'int [5]' lvalue Var 0xa406b8 '__range' 'int (&&)[5]' | `-VarDecl 0xa408b0 <col:14, col:16> col:14 implicit used __end 'int *':'int *' cinit | `-BinaryOperator 0xa409a0 <col:14, col:16> 'int *' '+' | |-ImplicitCastExpr 0xa40990 <col:14> 'int *' <ArrayToPointerDecay> | | `-DeclRefExpr 0xa40850 <col:14> 'int [5]' lvalue Var 0xa406b8 '__range' 'int (&&)[5]' | `-IntegerLiteral 0xa40970 <col:16> 'int' 5 |-BinaryOperator 0xa40a60 <col:14> '_Bool' '!=' | |-ImplicitCastExpr 0xa40a40 <col:14> 'int *':'int *' <LValueToRValue> | | `-DeclRefExpr 0xa40a10 <col:14> 'int *':'int *' lvalue Var 0xa40870 '__begin' 'int *':'int *' | `-ImplicitCastExpr 0xa40a50 <col:14> 'int *':'int *' <LValueToRValue> | `-DeclRefExpr 0xa40a28 <col:14> 'int *':'int *' lvalue Var 0xa408b0 '__end' 'int *':'int *' |-UnaryOperator 0xa40a90 <col:14> 'int *':'int *' lvalue prefix '++' | `-DeclRefExpr 0xa40a78 <col:14> 'int *':'int *' lvalue Var 0xa40870 '__begin' 'int *':'int *' |-DeclStmt 0xa40680 <col:7, col:18> | `-VarDecl 0xa40648 <col:7, col:14> col:12 e 'int':'int' cinit | `-ImplicitCastExpr 0xa40b40 <col:14> 'int' <LValueToRValue> | `-UnaryOperator 0xa40ad0 <col:14> 'int' lvalue prefix '*' | `-ImplicitCastExpr 0xa40ac0 <col:14> 'int *':'int *' <LValueToRValue> | `-DeclRefExpr 0xa40aa8 <col:14> 'int *':'int *' lvalue Var 0xa40870 '__begin' 'int *':'int *' `-NullStmt 0xa40b78 <line:4:5> ``` There is no range init in the AST. I'm not certain if that is an issue we want to rectify in the AST or not. ================ Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:40 @@ +39,3 @@ + + diag(MatchedCast->getExprLoc(), "do not (implicitly) convert an array to a pointer"); +} ---------------- klimek wrote: > Can't we provide a fixit? I think the fixit that the C++ Core Guidelines wants is to use array_view, which isn't available to everyone and can't be applied locally (for instance, it may require changing a function parameter instead of the argument passed to the function). ================ Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp:26 @@ +25,2 @@ + return "clang"; // OK, decay string literal to pointer +} ---------------- I would like to see a test case demonstrating that this does not trigger a diagnostic when creating an array_view object. We don't need to pull in all of the array_view machinery from GSL, but it would be good to have a skeleton of the implementation to ensure we aren't diagnosing there. http://reviews.llvm.org/D13640 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits