[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

[Adam Nemet via Phabricator via cfe-commits]

anemet added a comment.

In https://reviews.llvm.org/D37196#868320, @vivekvpandya wrote:

> > Why? That was inside BackendConsumer.
>
> I was getting incomplete type error.


You may have to move the class declaration of ClangDiagnosticHandler before the BackendConsumer and move the definition of ClangDiagnosticHandler::handleDiagnositic out of line since that is where you use BackendConsumer.  Something like:

  class BackendConsumer;
  
  class ClangDiagnosticHandler {
  public:
    ...  // as before
    bool handleDiagnostics(const DiagnosticInfo &DI) override;
    ...  
  private:
    ... 
  }
  
  class BackendConsumer {
    ...
  }
  
  bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo &DI) {
    ...
  }



>> You can just save the old DiagHandler object instead.
> 
> I don't think now we need to do that as per my understanding CodeGen i.e emitting LLVM IR is last phase in clang which will pass control to LLVMContext. LLVMContext will have Base class of DiagnosticHandler which will not handle diagnostic and return false so LLVMContext::diagnose() will print diagnostic with
> 
> DiagnosticPrinterRawOStream DP(errs());
> 
>   errs() << getDiagnosticMessagePrefix(DI.getSeverity()) << ": ";
>   DI.print(DP);
>   errs() << "\n";
>    
> 
> and clang's diagnostic handler also does similar thing so if we keep ClangDiagnosticHandler pointer in LLVMContext it should not break.

That would have been true even before and we still restored it, no?  I guess I am not sure why we used to restore this so I would just do it regardless and not change functionality.

If you want we can separately take a look whether we need to restore these original handlers.


https://reviews.llvm.org/D37196