vsavchenko added a comment. In D103096#2867021 <https://reviews.llvm.org/D103096#2867021>, @ASDenysPetrov wrote:
> In D103096#2866730 <https://reviews.llvm.org/D103096#2866730>, @vsavchenko > wrote: > >> In D103096#2866704 <https://reviews.llvm.org/D103096#2866704>, >> @ASDenysPetrov wrote: >> >>> @vsavchenko >> >> That's not the question I'm asking. Why do you need to set constraints for >> other symbolic expressions, when `SymbolicInferrer` can look them up on its >> own? Which cases will fail if we remove that part altogether? > > I see. Here is what fails in case if we don't update other constraints: > > void test(int x) { > if ((char)x > -10 && (char)x < 10) { > if ((short)x == 8) { > // If you remove updateExistingConstraints, > // then `c` won't be 8. It would be [-10, 10] instead. > char c = x; > if (c != 8) > clang_analyzer_warnIfReached(); // should no-warning, but fail > } > } > } OK, it's something! Good! I still want to hear a good explanation why is it done this way. Here `c` is mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, but we also have `(short)x` associated with `[8, 8]`. Why can't `VisitSymbolCast` look up constraints for `(short)x` it already looks up for constraints for different casts already. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 + if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + ---------------- Why do you use `VisitSymExpr` here? You want to interrupt all `Visits or... I'm not sure I fully understand. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1215 + QualType T = RootSym->getType(); + if (!T->isIntegralOrEnumerationType()) + return VisitSymExpr(Sym); ---------------- Can we get a test for that? ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1216 + if (!T->isIntegralOrEnumerationType()) + return VisitSymExpr(Sym); + ---------------- Same goes here. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241 + // Find the first constraint and exit the loop. + RSPtr = getConstraint(State, S); + } ---------------- Why do you get associated constraint directly without consulting with what `SymbolRangeInferrer` can tell you about it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
