v1nh1shungry added a comment.
Thank you for reviewing and giving suggestions! @Febbe
Please take a look at my reply. Sorry if I misunderstood anything!
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:215
(Twine("static_cast<") + TyAsString + ">(").str())
- << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
else
----------------
Febbe wrote:
> Actually, I find it pretty weird/suspicious, that `<AnyBinaryExr*> ->
> getEndLoc()` does not return the end of its `Expr`. The code looked correct
> already.
> Can you elaborate, if this is a bug in the `getEndLoc()` of those `Expr`s? It
> might be better to fix it there directly.
>
I'm really not an expert on `SourceLocation`. I just took a rough look at these
`getEndLoc()`s.
Take `1 + 2` as an example, this is a `BinaryOperator` with two
`IntegerLiteral`s. When we call `Expr::getEndLoc()` which is actually
`Stmt::getEndLoc()` on it, it will call `BinaryOperator::getEndLoc()` and this
actually returns the `RHS->getEndLoc()`, that is the result of
`IntegerLiteral(2)->getEndLoc()`. And the interesting stuff is that
`IntegerLiteral::getBeginLoc()` and `IntegerLiteral::getEndLoc()` return the
same `Loc`. Although I'm not sure which location these will return, I guess
that's why `BinaryOperator::getEndLoc()` doesn't return the end of the
expression.
Sorry if I misunderstood anything! After all, I didn't find documents saying
which location these `Expr`s' `getEndLoc()` should return. I think it's better
to have experts take a look at this.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp:18
// CHECK-NOTES-C: (ptrdiff_t)( )
// CHECK-NOTES-CXX: static_cast<ptrdiff_t>( )
// CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider
type
----------------
Actually I have tried adding
```
// CHECK-FIXES-C: return &base[(ptrdiff_t)(a * b)];
// CHECK-FIXES-CXX: return &base[static_cast<ptrdiff_t>(a * b)];
```
under this line, but the test failed, and when I took a look at
`build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/implicit-widening-of-multiplication-result-array-subscript-expression.cpp.tmp.cpp`,
I found that these codes didn't get modified. And I took a look at other files
which have `CHECK-FIXES` lines, I found the codes in the corresponding
temporary files got fixed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141058/new/
https://reviews.llvm.org/D141058
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
