[PATCH] D54740: [NewPM] fixing asserts on deleted loop in -print-after-all

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev marked an inline comment as done.
fedor.sergeev added inline comments.


================
Comment at: lib/Passes/StandardInstrumentations.cpp:86
     const Loop *L = any_cast<const Loop *>(IR);
+    assert(L && "Loop should be valid for printing");
     const Function *F = L->getHeader()->getParent();
----------------
philip.pfaffe wrote:
> Is there a policy on assertions?
> 
> Those you added imho don't add value. You're asserting for nullptr, but are using the pointer unconditionally after. That's failing just as hard. The message on the other hand isn't very helpful I feel, since you're not actually testing loop //validity//, but whether a nullptr was stored in the any, which I don't think is the same thing (anymore?).
> 
> 
Well... I have no idea about policies on assertions.

To me a very message of assert is usually considerably less helpful than the location of it.
Usually I dont read the message and just jump to the source when it hits.

You are right that now its not checking for validity, and it remains there only for the check against accidental nullptr (read - but in PassManager).
So yes, I might just scrap these...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54740