aaron.ballman added a comment.
There are a couple of questions from the previous iteration of the review that
aren't answered yet:
- the diagnostic wording doesn't actually say what's wrong with the code or how
to fix it; how should users silence the diagnostic if they've found a case
where it's a false positive for some reason?
- do you expect this check to catch other hidden provenance gotchas like
grabbing values through printf/scanf, fread, etc)? (Not for this patch
iteration, just wondering if you expect to extend the check in the future.)
================
Comment at: clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.cpp:21
+ Finder->addMatcher(castExpr(hasCastKind(CK_IntegralToPointer),
+ unless(hasSourceExpression(integerLiteral())))
+ .bind("x"),
----------------
Do we also want to exclude the case where the destination is a `volatile`
pointer on the assumption that something out of the ordinary is going on. e.g.,
```
intptr_t addr;
if (something) {
addr = SOME_CONSTANT;
} else {
addr = SOME_OTHER_CONSTANT;
}
volatile int *register_bank = (volatile int *)addr;
```
================
Comment at:
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp:53
"performance-no-automatic-move");
+ CheckFactories.registerCheck<NoIntToPtrCheck>("performance-no-inttoptr");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
----------------
Does anyone else have opinions on `inttoptr` vs `int-to-ptr`? I feel like the
latter is easier to read than the former, but I'm wondering if I'm just strange.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:15
- `abseil-duration-addition <abseil-duration-addition.html>`_, "Yes"
- `abseil-duration-comparison <abseil-duration-comparison.html>`_, "Yes"
- `abseil-duration-conversion-cast <abseil-duration-conversion-cast.html>`_,
"Yes"
- `abseil-duration-division <abseil-duration-division.html>`_, "Yes"
- `abseil-duration-factory-float <abseil-duration-factory-float.html>`_, "Yes"
- `abseil-duration-factory-scale <abseil-duration-factory-scale.html>`_, "Yes"
- `abseil-duration-subtraction <abseil-duration-subtraction.html>`_, "Yes"
- `abseil-duration-unnecessary-conversion
<abseil-duration-unnecessary-conversion.html>`_, "Yes"
- `abseil-faster-strsplit-delimiter
<abseil-faster-strsplit-delimiter.html>`_, "Yes"
+ `abseil-duration-addition <abseil-duration-addition.html>`_,
+ `abseil-duration-comparison <abseil-duration-comparison.html>`_,
----------------
It looks like there's a whole bunch of unrelated changes happening here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91055/new/
https://reviews.llvm.org/D91055
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
