================
@@ -861,7 +861,11 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
// If one of the operands is a symbol and the other is a constant,
// build an expression for use by the constraint manager.
- if (SymbolRef rSym = rhs.getAsLocSymbol()) {
+ // Pass IncludeBaseRegions so that &r->s (a field of a symbolic region)
+ // still counts as a symbol here, like the MemRegionVal case does below.
+ // Without it, `nullptr == &r->s` skips this and hits the "non-null region"
+ // case, which contradicts `&r->s == nullptr` and gives a false NPD
----------------
steakhal wrote:
I had to think about this patch. Sorry for taking this long.
My problem with this comment was that:
- It argues that it should do something because something else already does it
this way. - very weak argument.
- It didn't explain why we need the base beyond the behavior that happen if we
would. This suggested gaps in the reasoning if this was the strongest reason.
Nevertheless, it actually does make sense, but myself I also didn't dig into
the details.
I'd propose just dropping this comment as I don't this it really adds much.
```suggestion
```
https://github.com/llvm/llvm-project/pull/209875
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
