[PATCH] D83498: [NFC] Derive from PassInfoMixin for no-op/printing passes

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:300
 
-namespace {
+namespace llvm {
 
----------------
aeubanks wrote:
> ychen wrote:
> > aeubanks wrote:
> > > ychen wrote:
> > > > How about keeping this local? These are only for testing.
> > > Do you mean keeping this in an anonymous namespace?
> > > As mentioned in the commit, that makes the printed name messed up.
> > Add some regex in lit tests?
> > Running pass: {{.*}}NoOpModulePass
> > 
> I don't see any reason to distinguish it from other passes, even if it's only used for testing. It's a useful tool for sanity checks. Having a `(anonymous namespace)` printed anywhere doesn't look good.
> And it'd require updating more tests than I really want to.
If we really treat them as normal passes, they should be moved to a header file. If we treat them as testing tools only, we put them in .cpp file in an anonymous namespace. It looks confusing to be not in header file and in `llvm` namespace.  

Or perhaps, we don't touch these no-op passes, and add a  comment saying we're overriding the `name()` computing here to make tests cleaner? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83498