steakhal wrote:

> This is the cleanup of `ExplodedGraph::trim()` that I promised at 
> [31e981c](https://github.com/llvm/llvm-project/commit/31e981ca1dc323c8a32012cb60a0a8fe3985db1a).
>  This significantly simplifies the algorithm and should be equivalent in a 
> theoretical sense, but unfortunately it changes the (annoyingly fragile) 
> ordering of reports within a bug report equivalence class 😞

It not only simplifies it but also makes if faster. There are parts in the 
analyzer that have more legacy than others.
One of the worst parts are the bug reporting and processing - just like what 
you touch in the PR.

I think to make this sustainable, we need to think about paying back the 
technical debt even at the expense of convenience.
If we could harden the bug EQclass part for deterministically selecting the 
sequence of the bug reports we would try, that would make the outcomes stable 
no matter how the egraph looks, right?

Anyway, we should probably focus first hardening stability of the reports and 
then on disruptive changes to mitigate the pain upfront. Because if we would 
land this, and then more similar changes, then we would face this instability 
again and again with a similar magnitude.

On linux, it says "only" 11 tests failed:
```
Failed Tests (11):
  Clang :: Analysis/ArrayBound/verbose-tests.c
  Clang :: Analysis/cast-value-notes.cpp
  Clang :: Analysis/cert/env34-c.c
  Clang :: Analysis/division-by-zero-track-zero.cpp
  Clang :: Analysis/fuchsia_handle.cpp
  Clang :: Analysis/malloc-plist.c
  Clang :: Analysis/operator-calls.cpp
  Clang :: Analysis/osobject-retain-release.cpp
  Clang :: Analysis/retain-release.m
  Clang :: Analysis/smart-ptr-text-output.cpp
  Clang :: Analysis/valist-uninitialized-no-undef.c
```

This isn't too bad actually, but I also get that the user-facing change would 
be much more signifficant. They would probably see differences of bug reports 
that had equal long candidate counterparts in the bug eqclass. This usually 
means that some "true branch taken" notes would turn into "false branch taken" 
for the conditions that didn't really matter anyway for the bug.

https://github.com/llvm/llvm-project/pull/139939
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to