george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed.
Thanks! Looks good with minor changes. Would it be possible to add tests? I know we have very few unit tests, but I assume you could actually use an integration test to exercise this path? ================ Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1044 Z3Expr Exp = getZ3Expr(Sym, &RetTy); - - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType LTy; + llvm::APSInt NewFromInt; ---------------- What does `L` stand for here? It's confusing because `L/R` usually stand for left/right-hand-side in this context. ================ Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1154 + BasicValueFactory &BVF = getBasicVals(); + ASTContext &Ctx = BVF.getContext(); ---------------- that's a separate change, but OK ================ Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1426 + *Ty = getAPSIntType(Int); + return Int; +} ---------------- It's redundant to mutate the argument passed by reference and also return it. Could we take a single `APSInt` parameter by value and return `std::pair<APSInt, QualType>` ? ================ Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1640 #else - llvm::report_fatal_error("Clang was not compiled with Z3 support!", false); + llvm::report_fatal_error("Clang was not compiled with Z3 support, rebuild " + "with -DCLANG_ANALYZER_BUILD_Z3=ON", ---------------- that's a separate change, but OK Repository: rC Clang https://reviews.llvm.org/D47603 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits