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

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea accepted this revision.
asbirlea added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:300
 
-namespace {
+namespace llvm {
 
----------------
ychen wrote:
> 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? 
IMO a comment clarifying these are a special case will work here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83498