0x8000-0000 marked an inline comment as done. 0x8000-0000 added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp:129 + // expanded class enumeration value. + if (Parent.get<CStyleCastExpr>()) + return true; ---------------- 0x8000-0000 wrote: > aaron.ballman wrote: > > So will this no longer treat *any* C-style cast as being a magic number? > > e.g., `int i; i = (int)12;` > Sadly, it would seem so. > > if (ValueArray[(int)4] > ValueArray[1]) produces the following AST: > > ``` > | | | | `-ArraySubscriptExpr 0x11545f0 <col:7, col:24> 'int' lvalue > | | | | |-ImplicitCastExpr 0x11545d8 <col:7> 'int *' <ArrayToPointerDecay> > | | | | | `-DeclRefExpr 0x1154558 <col:7> 'int [5]' lvalue Var 0x1153cb0 > 'ValueArray' 'int [5]' > > | | | | `-CStyleCastExpr 0x11545b0 <col:18, col:23> 'int' <NoOp> > | | | | `-IntegerLiteral 0x1154578 <col:23> 'int' 4 > ``` > > While the test case we're trying to avoid is: > ``` > `-TemplateSpecializationType 0x1169ee0 'holder<(Letter)9U>' sugar holder > |-TemplateArgument expr > | `-ConstantExpr 0x1169dc0 <col:59> 'Letter' 9 > | `-SubstNonTypeTemplateParmExpr 0x1169da0 <col:59> 'Letter' > | `-CStyleCastExpr 0x1169d78 <col:59> 'Letter' <IntegralCast> > | `-IntegerLiteral 0x1169d48 <col:59> 'unsigned int' 9 > `-RecordType 0x1169ec0 'holder<Letter::J>' > `-ClassTemplateSpecialization 0x1169de0 'holder' > ``` > > Now, the question is - do we care? How many people use a C cast with a bare > integer? > > I suppose I can add a check for the grandparent node to be > SubstNonTypeTemplateParmExpr, in order to filter this out? An interesting fact is that the C-style cast does not produce a warning now anyway (even before applying this change.) and I don't have any idea why. Adding these lines does not create a test failure: ``` + 79 if (((int)4) > IntSquarer[10]) + 80 return 0; ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71686/new/ https://reviews.llvm.org/D71686 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits