[PATCH] D19987: Allow the LTO code generator to drop malformed debug info from the input

[Adrian Prantl via llvm-commits]

aprantl added inline comments.

================
Comment at: lib/IR/Verifier.cpp:4434
@@ +4433,3 @@
+bool llvm::verifyModule(bool &BrokenDebugInfo, const Module &M,
+                        raw_ostream *OS) {
+  // Don't use a raw_null_ostream.  Printing IR is expensive.
----------------
joker.eph wrote:
> The only difference between this function and the one above is the output parameter `BrokenDebugInfo` right?
> 
> What about having a single function with an optional argument, something like:
> 
> ```
> // Returns true if the module is broken. If BrokenDebugInfo is supplied, DebugInfo 
> // verification failures won't be considered as error and instead *BrokenDebugInfo
> // will be set to true.
> bool llvm::verifyModule(const Module &M, raw_ostream *OS, bool *BrokenDebugInfo) {
>   // Don't use a raw_null_ostream.  Printing IR is expensive.
>   Verifier V(OS, /*TreatBrokenDebugInfoAsError=*/!BrokenDebugInfo);
> 
>   bool Broken = false;
>   for (const Function &F : M)
>     if (!F.isDeclaration() && !F.isMaterializable())
>       Broken |= !V.verify(F);
>   Broken |= !V.verify(M)
>   if (BrokenDebugInfo) *BrokenDebugInfo = V.hasBrokenDebugInfo();
> 
>   // Note that this function's return value is inverted from what you would
>   // expect of a function called "verify".
>   return Broken;
> }
> ```
For some reason I thought that that would break the C API, however, after actually checking I now realize that it uses a wrapper function (LLVMVerifyModule) anyway, so, yes, we can definitely do that!

================
Comment at: lib/IR/Verifier.cpp:4442
@@ +4441,3 @@
+
+  Broken = !V.verify(M);
+  BrokenDebugInfo = V.hasBrokenDebugInfo();
----------------
joker.eph wrote:
> This statement unconditionally overwrite `Broken`, I doubt this is what you intended?
Yes, that should be a |=.


http://reviews.llvm.org/D19987