[PATCH] D142710: [clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 10:41:52 PST 2023


ymandel added a comment.

In D142710#4097032 <https://reviews.llvm.org/D142710#4097032>, @xazax.hun wrote:

> In D142710#4096325 <https://reviews.llvm.org/D142710#4096325>, @ymandel wrote:
>
>> In D142710#4094934 <https://reviews.llvm.org/D142710#4094934>, @xazax.hun wrote:
>>
>>> This change looks good to me. I wonder, however, whether the behavior should be parameterized in the future. E.g., whether the user of the analysis should be able to make a decision whether the analysis should be pessimistic or optimistic about unmodeled values.
>>
>> Interesting idea. I think this goes along with other places where we are unsound. Here, we err on the side of soundness. but, in general, we should have a configuration mechanism for this.  FWIW, the only reason we have uninitialized values at this point is recursive types. We also limit the depth of structs, but that should be removed given my recent patch to only model relevant fields. I have an idea for lazy initialization of values that I think could solve the recursion issue. Together, we could remove this concept of unmodeled values altogether from the framework.
>
> Oh, sounds great! I do think lazy initialization will be really valuable to reduce the number of unmodeled values, but not entirely sure if we can completely eliminate them. In case we end up creating new locations (different from the earlier ones) in every iteration of the loop it might be harder to reach a fixed point.

True, and in some sense `TopBoolValue` is already that. If we extended Top to other value domains (like struct) the code would end up looking very similar, just spelled "top" instead of "nullptr". I'd prefer Top over nullptr, but it admittedly wouldn't change things in a fundamental way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142710/new/

https://reviews.llvm.org/D142710



More information about the cfe-commits mailing list