NoQ added a comment.
I think the overall idea is correct.
Yup, the patch will need the new positive tests that started passing. If you
can't demonstrate it with integration tests you should do unit tests because
the contract you're trying to achieve is fairly clear and obviously correct and
it makes sense to test it separately.
================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1056
}
+ } else if (LeftER && isa<SymbolicRegion>(RightMR)) {
+ if (LeftER->getSuperRegion() != RightMR)
----------------
Symbolic regions aren't special. Any other parent region should work similarly.
================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1059-1060
+ return UnknownVal();
+ if (Optional<NonLoc> LeftIndex = MaybeNonLocIndexOfElementRegion(LeftER))
+ return LeftIndex.getValue();
+ return UnknownVal();
----------------
You'll have to multiply by the size of a single element - it's an index, not an
offset.
================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1067-1069
+ llvm_unreachable(
+ "TODO: return the negated value of RightIndex - figure out how
:D\n"
+ "Maybe a unary minus operator would do the job.");
----------------
Yeah, sounds like we'll have to stick to `UnknownVal` for now when `RightIndex`
is symbolic. You can easily compute unary minus for a concrete index though,
and that'll probably be your most important case anyway.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84736/new/
https://reviews.llvm.org/D84736
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
