vsavchenko added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1529 + // b = a; + // c = foo(b); + // ---------------- martong wrote: > I'd rather use `identity` here (and at line 1509) instead of `foo`, I think > that could make this explanation easier to follow. Good idea! ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1532 + // Telling the user that the value of 'a' is assigned to 'c', while + // correct, can be confusing. + StoreManager::FindUniqueBinding FB(V.getAsLocSymbol()); ---------------- martong wrote: > So here, we have two or three bindings attached to `c`? `foo(b)` and `a` (and > `b` as well) ? > What is the guarantee that `FindUniqueBinding` will return with the correct > one `foo(b)`? Seems like we iterate through an ImmutableMap (AVL tree) with > `MemRegion*` keys. And the order of the iteration depends on pointer values. > What I want to express, is that I don't see how do we find the correct > binding, seems like we just find one, which might be the one we look for if > we are lucky. > > Perhaps we could have a lit test for this example? Good idea, I should a test for that! `FindUniqueBinding` does not only have a region, it also carries a flag checking if there are more than one binding. `operator bool` then checks for both, thus guaranteeing that we won't choose random binding out of more than 1 - we won't choose at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101041/new/ https://reviews.llvm.org/D101041 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
