Oops, forgot to reply-all. Sorry Adrian. ---------- Forwarded message ---------- From: David Blaikie <dblai...@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 <apra...@apple.com>
On Mon, Oct 5, 2015 at 11:43 AM, David Blaikie <dblai...@gmail.com> wrote: > > > On Mon, Oct 5, 2015 at 10:41 AM, Adrian Prantl via cfe-commits < > cfe-commits@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits