NoQ added a comment.
This looks great to me but i'd very much rather have someone else have a look,
just because of the nature of the problem (:
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1586
+ // 4. Update disequality relations
+ if (const ClassSet *DisequalToOther = DisequalityInfo.lookup(Other)) {
+ ClassSet DisequalToThis = getDisequalClasses(State);
----------------
Nit: I think it's a bit messy to use `getDisequalClasses()` in some places but
manual lookup in other places. Like, someone may change the function and
believe that they changed the entire lookup mechanism but fail to notice that
the function wasn't used consitently. I think this may be worth encapsulating.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1727
+
+ // Let's check if know anything about these two classes being not equal to
+ // each other.
----------------
Parse error; missing "we"?
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1913-1915
+ // DisequalToDisequalSet is guaranteed to be non-null for consistent
+ // disequality info.
+ ClassSet NewSet = ClassSetFactory.remove(*DisequalToDisequalSet,
Class);
----------------
I actually wouldn't mind duplicating a direct assertion here, given that
consistency checks are pretty complicated on their own :)
================
Comment at: clang/test/Analysis/mutually_exclusive_null_fp.cpp:25
+
+ return compare(*aData, *bData);
+}
----------------
`// no-warning` please? These marks make tests a lot easier to understand when
you break them 5 years later.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83286/new/
https://reviews.llvm.org/D83286
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
