[clang] [clang][nullability] Don't discard expression state before end of full-expression. (PR #82611)

via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 5 03:55:26 PST 2024


================
@@ -248,12 +251,12 @@ class JoinedStateBuilder {
       // initialize the state of each basic block differently.
       return {AC.Analysis.typeErasedInitialElement(), AC.InitEnv.fork()};
     if (All.size() == 1)
----------------
martinboehme wrote:

> guard on ExprBehavior as well, like `if (All.size() == 1 && ExprBehavior == DiscardExprState)`?

We would also need to return the existing state in the case `All.size() == 1 && ExprBehavior == KeepExprState`, right? IOW, the code would become something like this:

```
    if (All.size() == 1) {
      if (ExprBehavior == KeepExprState)
        return All[0];
      // Join the environment with itself so that we discard expression state.
      // FIXME: We could consider writing special-case code for this that only
      // does the discarding, but it's not clear if this is worth it.
      return {All[0]->Lattice, Environment::join(All[0]->Env, All[0]->Env,
                                                 AC.Analysis, DiscardExprState)};
    }
```

And it doesn't deal with the FIXME, as we still need some way to discard the expression state.

I'm not sure that, all in all, changing the code in this way would be preferable.

The upside is that we're now a bit more efficient in the case where `All.size() == 1` and we want to keep expression state.

The downside is that we're treating `KeepExprState` specially when `join()` could have handled it for us. Right now, `join()` is the only place that needs to care about what to do for `KeepExprState` versus `DiscardExprState`, and it would be nice to keep the behavior localized in this way.

https://github.com/llvm/llvm-project/pull/82611


More information about the cfe-commits mailing list