[PATCH] D109290: [OpaquePtr] Forbid mixing typed and opaque pointers

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 7 16:46:19 PDT 2021


dblaikie added a comment.

In D109290#2988234 <https://reviews.llvm.org/D109290#2988234>, @aeubanks wrote:

> I think this makes sense, but this may affect how we'll mass-update tests so we should figure out how we're going to transition tests to opaque pointers.
>
> We could make copies of most test but with opaque pointers. I don't like this because if people have to update tests, they'll be updating twice as many. And keeping the copies in sync with the originals will be all but impossible.
> We could slowly start adding `--opaque-pointers` to RUN lines and update tests. Then only when we have different code paths for opaque vs typed pointers do we have duplicate tests. My idea is that if things work with opaque pointers, they should probably also work with typed pointers. @dblaikie wasn't a fan of this due to us potentially losing test coverage for typed pointers.
> We could have a bot running all tests with --opaque-pointers and continuously gather crashes. We won't be able to use the number of test failures since CHECK lines will still be wrong. Then mass-migrate all tests in the same change where we flip `--opaque-pointers`. Personally I don't like a humongous change, even if it's automatable. We'd also have to update a bunch of non-llvm tests in the same change, like clang tests.
>
> Thoughts?

My preference was, hopefully, to do one major test transition before the opaque pointer flag-flip to make the tests opaque-agnostic (using regex matches for pointer types so they could match typed or untyped pointers - wouldn't even be averse to makiyng some kind of pre-canned regex in FileCheck for this purpose given the size of the effort involved) - under the theory that we are going to have to do a big test cleanup one way or another, and this way we could avoid losing production test coverage pre-flip, and avoid the flip requiring a huge amount of change. Depends how much more expensive it is to transition tests to be typed-agnostic, compared to just transitioning them from typed to opaque explicitly. If it's not a /lot/ more work, I think it's worth it to keep the production test coverage in the interim.

But if folks (llvm-dev thread may be needed) are generally OK with losing production test coverage in the interim, just migrating to opaque pointer-based testing might be the way to go despite my aversion.


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

https://reviews.llvm.org/D109290



More information about the llvm-commits mailing list