[PATCH] Add support for optimization reports.

Richard Smith richard at metafoo.co.uk
Tue Apr 15 14:45:32 PDT 2014


  LGTM with a fix for the memory leak.


================
Comment at: lib/Frontend/CompilerInvocation.cpp:526
@@ +525,3 @@
+    std::string RegexError;
+    Opts.OptimizationRemarkPattern = new llvm::Regex(Val);
+    if (!Opts.OptimizationRemarkPattern->isValid(RegexError)) {
----------------
Diego Novillo wrote:
> Richard Smith wrote:
> > Diego Novillo wrote:
> > > Richard Smith wrote:
> > > > It looks like this is leaked. Maybe use an `llvm::Optional<llvm::Regex>` instead?
> > > I tried but llvm::Regex has an explicitly deleted constructor. I'm not sure how to deal with that, so I ended up using regular pointer semantics.
> > `llvm::Optional` shouldn't call the `llvm::Regex` default constructor.
> 
> Sure, but it's trying to when instantiating CodeGenOptions:
> 
> llvm/include/llvm/ADT/Optional.h:39:28: error: call to deleted constructor of 'llvm::Regex'
>       new (storage.buffer) T(*O);
>                            ^ ~~
> llvm/tools/clang/include/clang/Frontend/CodeGenOptions.h:39:7: note: in instantiation of member function 'llvm::Optional<llvm::Regex>::Optional' requested here
> class CodeGenOptions : public CodeGenOptionsBase {
>       ^
> llvm/include/llvm/Support/Regex.h:49:5: note: 'Regex' has been explicitly marked deleted here
>     Regex(const Regex &) LLVM_DELETED_FUNCTION;
>     ^
> 1 error generated.
> 
> 
> I'm not quite sure how to deal with this.  All I'm doing is:
> 
> -  llvm::Regex *OptimizationRemarkPattern;
> +  llvm::Optional<llvm::Regex> OptimizationRemarkPattern;
> 
> I feel like I'm missing something.
Oh, I see. What this crummy diagnostic is trying to tell you is that `Regex` isn't copyable, but someone is trying to make a copy of the `CodeGenOptions` object somewhere (we don't actually say where, due to an interaction of recursive special member generation and delayed function template instantiation...). `CodeGeneratorImpl::CodeGenOpts` does this, there might be other places.

Next idea: use `std::shared_ptr<llvm::Regex>`.


http://reviews.llvm.org/D3226

BRANCH
  opt-report

ARCANIST PROJECT
  clang






More information about the cfe-commits mailing list