balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:61
+
+ return NeedsExtraBitToPreserveSigness ? Signed : ExtendedSigned;
+}
----------------
Should not the `ExtendedSigned` be in the true branch?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:87
+/// stored value.
+static APSInt absWithoutWrapping(const llvm::APSInt &Value) {
+ // If unsigned, we might need a sign bit.
----------------
I do not like these function names. `getCommonSignedTypeToFit`,
`isRepresentableBySigned`, `getAbsWithoutWrapping` is better to have the common
"start verbs" in the names.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:241
+ const APSInt &ExclusiveUpperBound)
+ : LastValidState(std::move(State)), SVB(SVB), RootSymbol(RootSymbol) {
+ assert(LastValidState);
----------------
std::move is not needed here
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:269
+ /// Starts the simplification process.
+ SimplificationResult Simplify() {
+ LLVM_DEBUG(llvm::dbgs() << __func__ << ": initially: '";
----------------
Function name should start with lower case.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:285
+ LastValidState->printJson(llvm::dbgs()); llvm::dbgs() << '\n';);
+ return {std::move(LastValidState), std::move(LowerBound),
+ std::move(UpperBound), SymbolVal(RootSymbol),
SimplificationFailed};
----------------
The moves here are not needed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88359/new/
https://reviews.llvm.org/D88359
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
