Fwd: r249328 - Undo the unique_ptr'fication of CodeGenABITypes::CGM introduced in r248762.

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 5 11:46:05 PDT 2015


Oops, forgot to reply-all. Sorry Adrian.

---------- Forwarded message ----------
From: David Blaikie <dblaikie at gmail.com>
Date: Mon, Oct 5, 2015 at 11:45 AM
Subject: Re: r249328 - Undo the unique_ptr'fication of CodeGenABITypes::CGM
introduced in r248762.
To: Adrian Prantl <aprantl at apple.com>




On Mon, Oct 5, 2015 at 11:43 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Oct 5, 2015 at 10:41 AM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: adrian
>> Date: Mon Oct  5 12:41:16 2015
>> New Revision: 249328
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=249328&view=rev
>> Log:
>> Undo the unique_ptr'fication of CodeGenABITypes::CGM introduced in
>> r248762.
>> include/clang/CodeGenABITypes.h is in meant to be included by external
>> users,
>
>
Oh, and it might be handy to have some kind of test coverage for this -
however that might happen. (I wonder if a "unit test" that didn't even have
any executable code/tests and just included the header, would've caught
this at compile-time rather than, presumably, when it was integrated into
another codebase with the actual external use of the header - though a more
fully fledged test would be nice, of course)


> but using a unique_ptr on the private CodeGenModule introduces a
>
> dependency on the type definition that prevents such a use.
>>
>
> If you just make the CodeGenABITypes dtor out of line (but still
> defaulted) this will address the issue without needing to use raw
> pointers/manual delete:
>
> foo.h:
>
> struct bar;
> struct foo {
>   unique_ptr<bar> b;
>   foo();
>   ~foo();
> };
>
> foo.cpp:
> struct bar {
> };
> foo::~foo() = default;
>
>
> We have similar code in a variety of parts of LLVM/Clang to cope with
> calling dtors of types that are implementation details, etc.
>
>
>>
>> NFC
>>
>> Modified:
>>     cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
>>     cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp
>>
>> Modified: cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h?rev=249328&r1=249327&r2=249328&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h (original)
>> +++ cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h Mon Oct  5 12:41:16
>> 2015
>> @@ -52,6 +52,7 @@ class CodeGenABITypes
>>  public:
>>    CodeGenABITypes(ASTContext &C, llvm::Module &M,
>>                    CoverageSourceInfo *CoverageInfo = nullptr);
>> +  ~CodeGenABITypes();
>>
>>    /// These methods all forward to methods in the private implementation
>> class
>>    /// CodeGenTypes.
>> @@ -79,7 +80,7 @@ private:
>>    std::unique_ptr<PreprocessorOptions> PPO;
>>
>>    /// The CodeGenModule we use get to the CodeGenTypes object.
>> -  std::unique_ptr<CodeGen::CodeGenModule> CGM;
>> +  CodeGen::CodeGenModule *CGM;
>>  };
>>
>>  }  // end namespace CodeGen
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp?rev=249328&r1=249327&r2=249328&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenABITypes.cpp Mon Oct  5 12:41:16 2015
>> @@ -33,6 +33,11 @@ CodeGenABITypes::CodeGenABITypes(ASTCont
>>        CGM(new CodeGen::CodeGenModule(C, *HSO, *PPO, *CGO, M,
>> C.getDiagnostics(),
>>                                       CoverageInfo)) {}
>>
>> +CodeGenABITypes::~CodeGenABITypes()
>> +{
>> +  delete CGM;
>> +}
>> +
>>  const CGFunctionInfo &
>>  CodeGenABITypes::arrangeObjCMessageSendSignature(const ObjCMethodDecl
>> *MD,
>>                                                   QualType receiverType) {
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151005/a4ff232f/attachment-0001.html>


More information about the cfe-commits mailing list