ASDenysPetrov added a comment. @martong
> Nice, assiduous work! Many thanks for your time! Your work is not less assiduous! > (LHS, RHS) swaps it doesn't really matter, as the operation is comutative, but, yes, it does matter to depict the particular test case. I'll revise twise LHS-RHS's. > I am going to continue with the next patch in the stack. I recognize that the > handling of casts is very important and problems related to not handling them > occurs even more frequently as other parts of the engine evolve (e.g. > https://reviews.llvm.org/D113753#3167134) Aha. I saw you patch. I'll join to review it. ================ Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:441 + // ___/__/_____\__\___ ___/___________\___ + this->checkUnite({{MID, MID}}, {A, D}, {{A, D}}); + this->checkUnite({{B, C}}, {A, D}, {{A, D}}); ---------------- martong wrote: > There are three overloads of `checkUnite` which correspond to three overloads of `RangeSet::unite`. I made it consistent to other check-functions (`add` e.g.). ================ Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:449 + // ___/___________\___ ___/___________\___ + this->checkUnite({{MIN, MIN}}, MIN, {{MIN, MIN}}); + this->checkUnite({{A, B}}, {A, B}, {{A, B}}); ---------------- martong wrote: > I think, either we should use `{{X, X}}` or `X` everywhere, but not mixed. This tests different oveloaded versions of `RangeSet::unite`. ================ Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:528-529 + // _/__\_/__\_/\_/\_/\_ _/__\_/__\_/\_/\_/\_ + this->checkUnite({{MIN, A}, {A + 2, B}}, {{MID, C}, {C + 2, D - 2}, {D, MAX}}, + {{MIN, A}, {A + 2, B}, {MID, C}, {C + 2, D - 2}, {D, MAX}}); + this->checkUnite({{MIN, MIN}, {A, A}}, {{B, B}, {C, C}, {MAX, MAX}}, ---------------- martong wrote: > I think we could better format these more complex cases. clang-fromat acts on its own. But I agree, it looks the way better. I'll consider wrapping it into `// clang-format on/off` directives. ================ Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:590-592 + this->checkUnite({{A, B - 1}, {B + 1, C - 1}, {C + 2, D}, {MAX - 1, MAX}}, + {{MIN, MIN}, {B, MID}, {MID + 1, C}, {C + 4, D - 1}}, + {{MIN, MIN}, {A, C}, {C + 2, D}, {MAX - 1, MAX}}); ---------------- martong wrote: > martong wrote: > > > What do you think about this format? The result can be easily verified this > way I think, but a bit ugly ... I think ASCII art does this job. Let code look as code :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
