Ah forget that. It's been a while since I tried these games and remember now why it doesn't work.
-----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of David Tweed Sent: 02 May 2014 09:17 To: 'Alp Toker'; [email protected]; clang-dev Developers Subject: Re: [cfe-dev] Default arguments considered harmful? I wonder if, with C++11 named initialization syntax for PODs, something could be done so that in cases like this a (function specific, I guess) "optional options object" could be used. (This would avoid one of the big problems with C++ optional arguments, which is that if one towards the end needs to be set to a non-default value all the preceding options need setting.) That would certainly make things a lot easier to read in cases like these. -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Alp Toker Sent: 02 May 2014 04:58 To: [email protected]; clang-dev Developers Subject: Default arguments considered harmful? This got me thinking, perhaps it's time to introduce coding style policy to discourage excessive use of default arguments? Check out this gem from ASTUnit.h: static ASTUnit *LoadFromCommandLine( const char **ArgBegin, const char **ArgEnd, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath, bool OnlyLocalDecls = false, bool CaptureDiagnostics = false, ArrayRef<RemappedFile> RemappedFiles = None, bool RemappedFilesKeepOriginalName = true, bool PrecompilePreamble = false, TranslationUnitKind TUKind = TU_Complete, bool CacheCodeCompletionResults = false, bool IncludeBriefCommentsInCodeCompletion = false, bool AllowPCHWithCompilerErrors = false, bool SkipFunctionBodies = false, bool UserFilesAreVolatile = false, bool ForSerialization = false, std::unique_ptr<ASTUnit> *ErrAST = 0); Alp. On 02/05/2014 04:43, Alp Toker wrote: > Author: alp > Date: Thu May 1 22:43:30 2014 > New Revision: 207825 > > URL: http://llvm.org/viewvc/llvm-project?rev=207825&view=rev > Log: > Factor TargetInfo pointer/DelayInitialization bool pair out of Preprocessor ctor > > The Preprocessor::Initialize() function already offers a clear interface to > achieve this, further reducing the confusing number of states a newly > constructed preprocessor can have. > > Modified: > cfe/trunk/include/clang/Lex/Preprocessor.h > cfe/trunk/lib/Frontend/ASTUnit.cpp > cfe/trunk/lib/Frontend/CompilerInstance.cpp > cfe/trunk/lib/Lex/Preprocessor.cpp > cfe/trunk/unittests/Basic/SourceManagerTest.cpp > cfe/trunk/unittests/Lex/LexerTest.cpp > cfe/trunk/unittests/Lex/PPCallbacksTest.cpp > cfe/trunk/unittests/Lex/PPConditionalDirectiveRecordTest.cpp > > Modified: cfe/trunk/include/clang/Lex/Preprocessor.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor .h?rev=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/include/clang/Lex/Preprocessor.h (original) > +++ cfe/trunk/include/clang/Lex/Preprocessor.h Thu May 1 22:43:30 2014 > @@ -455,12 +455,10 @@ private: // Cached tokens state. > public: > Preprocessor(IntrusiveRefCntPtr<PreprocessorOptions> PPOpts, > DiagnosticsEngine &diags, LangOptions &opts, > - const TargetInfo *target, > SourceManager &SM, HeaderSearch &Headers, > ModuleLoader &TheModuleLoader, > IdentifierInfoLookup *IILookup = 0, > bool OwnsHeaderSearch = false, > - bool DelayInitialization = false, > TranslationUnitKind TUKind = TU_Complete); > > ~Preprocessor(); > > Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=2 07825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original) > +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Thu May 1 22:43:30 2014 > @@ -718,11 +718,10 @@ ASTUnit *ASTUnit::LoadFromASTFile(const > > AST->PP = new Preprocessor(PPOpts, > AST->getDiagnostics(), AST->ASTFileLangOpts, > - /*Target=*/0, AST->getSourceManager(), HeaderInfo, > + AST->getSourceManager(), HeaderInfo, > *AST, > /*IILookup=*/0, > - /*OwnsHeaderSearch=*/false, > - /*DelayInitialization=*/true); > + /*OwnsHeaderSearch=*/false); > Preprocessor &PP = *AST->PP; > > AST->Ctx = new ASTContext(AST->ASTFileLangOpts, > > Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance. cpp?rev=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) > +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu May 1 22:43:30 2014 > @@ -240,11 +240,11 @@ void CompilerInstance::createPreprocesso > getLangOpts(), > &getTarget()); > PP = new Preprocessor(&getPreprocessorOpts(), > - getDiagnostics(), getLangOpts(), &getTarget(), > + getDiagnostics(), getLangOpts(), > getSourceManager(), *HeaderInfo, *this, PTHMgr, > /*OwnsHeaderSearch=*/true, > - /*DelayInitialization=*/false, > TUKind); > + PP->Initialize(getTarget()); > > // Note that this is different then passing PTHMgr to Preprocessor's ctor. > // That argument is used as the IdentifierInfoLookup argument to > > Modified: cfe/trunk/lib/Lex/Preprocessor.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=2 07825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/lib/Lex/Preprocessor.cpp (original) > +++ cfe/trunk/lib/Lex/Preprocessor.cpp Thu May 1 22:43:30 2014 > @@ -56,11 +56,11 @@ ExternalPreprocessorSource::~ExternalPre > > Preprocessor::Preprocessor(IntrusiveRefCntPtr<PreprocessorOptions> PPOpts, > DiagnosticsEngine &diags, LangOptions &opts, > - const TargetInfo *target, SourceManager &SM, > + SourceManager &SM, > HeaderSearch &Headers, ModuleLoader &TheModuleLoader, > IdentifierInfoLookup *IILookup, bool OwnsHeaders, > - bool DelayInitialization, TranslationUnitKind TUKind) > - : PPOpts(PPOpts), Diags(&diags), LangOpts(opts), Target(target), > + TranslationUnitKind TUKind) > + : PPOpts(PPOpts), Diags(&diags), LangOpts(opts), Target(0), > FileMgr(Headers.getFileMgr()), SourceMgr(SM), HeaderInfo(Headers), > TheModuleLoader(TheModuleLoader), ExternalSource(0), > Identifiers(opts, IILookup), IncrementalProcessing(false), > @@ -132,11 +132,6 @@ Preprocessor::Preprocessor(IntrusiveRefC > Ident___exception_info = Ident___exception_code = Ident___abnormal_termination = 0; > Ident_GetExceptionInfo = Ident_GetExceptionCode = Ident_AbnormalTermination = 0; > } > - > - if (!DelayInitialization) { > - assert(Target && "Must provide target information for PP initialization"); > - Initialize(*Target); > - } > } > > Preprocessor::~Preprocessor() { > > Modified: cfe/trunk/unittests/Basic/SourceManagerTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/SourceManagerT est.cpp?rev=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/unittests/Basic/SourceManagerTest.cpp (original) > +++ cfe/trunk/unittests/Basic/SourceManagerTest.cpp Thu May 1 22:43:30 2014 > @@ -80,11 +80,11 @@ TEST_F(SourceManagerTest, isBeforeInTran > VoidModuleLoader ModLoader; > HeaderSearch HeaderInfo(new HeaderSearchOptions, SourceMgr, Diags, LangOpts, > &*Target); > - Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, Target.getPtr(), > + Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > PP.EnterMainSourceFile(); > > std::vector<Token> toks; > @@ -195,11 +195,11 @@ TEST_F(SourceManagerTest, getMacroArgExp > VoidModuleLoader ModLoader; > HeaderSearch HeaderInfo(new HeaderSearchOptions, SourceMgr, Diags, LangOpts, > &*Target); > - Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, Target.getPtr(), > + Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > PP.EnterMainSourceFile(); > > std::vector<Token> toks; > @@ -293,11 +293,11 @@ TEST_F(SourceManagerTest, isBeforeInTran > VoidModuleLoader ModLoader; > HeaderSearch HeaderInfo(new HeaderSearchOptions, SourceMgr, Diags, LangOpts, > &*Target); > - Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, Target.getPtr(), > + Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > > std::vector<MacroAction> Macros; > PP.addPPCallbacks(new MacroTracker(Macros)); > > Modified: cfe/trunk/unittests/Lex/LexerTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?re v=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/unittests/Lex/LexerTest.cpp (original) > +++ cfe/trunk/unittests/Lex/LexerTest.cpp Thu May 1 22:43:30 2014 > @@ -69,10 +69,10 @@ protected: > VoidModuleLoader ModLoader; > HeaderSearch HeaderInfo(new HeaderSearchOptions, SourceMgr, Diags, LangOpts, > Target.getPtr()); > - Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, Target.getPtr(), > + Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, > SourceMgr, HeaderInfo, ModLoader, /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/ false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/ false); > + PP.Initialize(*Target); > PP.EnterMainSourceFile(); > > std::vector<Token> toks; > > Modified: cfe/trunk/unittests/Lex/PPCallbacksTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest. cpp?rev=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/unittests/Lex/PPCallbacksTest.cpp (original) > +++ cfe/trunk/unittests/Lex/PPCallbacksTest.cpp Thu May 1 22:43:30 2014 > @@ -173,11 +173,10 @@ protected: > > IntrusiveRefCntPtr<PreprocessorOptions> PPOpts = new PreprocessorOptions(); > Preprocessor PP(PPOpts, Diags, LangOpts, > - Target.getPtr(), > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > InclusionDirectiveCallbacks* Callbacks = new InclusionDirectiveCallbacks; > PP.addPPCallbacks(Callbacks); // Takes ownership. > > @@ -208,11 +207,10 @@ protected: > OpenCLLangOpts, Target.getPtr()); > > Preprocessor PP(new PreprocessorOptions(), Diags, OpenCLLangOpts, > - Target.getPtr(), > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > > // parser actually sets correct pragma handlers for preprocessor > // according to LangOptions, so we init Parser to register opencl > > Modified: cfe/trunk/unittests/Lex/PPConditionalDirectiveRecordTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPConditionalDir ectiveRecordTest.cpp?rev=207825&r1=207824&r2=207825&view=diff > ============================================================================ == > --- cfe/trunk/unittests/Lex/PPConditionalDirectiveRecordTest.cpp (original) > +++ cfe/trunk/unittests/Lex/PPConditionalDirectiveRecordTest.cpp Thu May 1 22:43:30 2014 > @@ -97,11 +97,11 @@ TEST_F(PPConditionalDirectiveRecordTest, > VoidModuleLoader ModLoader; > HeaderSearch HeaderInfo(new HeaderSearchOptions, SourceMgr, Diags, LangOpts, > Target.getPtr()); > - Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts,Target.getPtr(), > + Preprocessor PP(new PreprocessorOptions(), Diags, LangOpts, > SourceMgr, HeaderInfo, ModLoader, > /*IILookup =*/ 0, > - /*OwnsHeaderSearch =*/false, > - /*DelayInitialization =*/ false); > + /*OwnsHeaderSearch =*/false); > + PP.Initialize(*Target); > PPConditionalDirectiveRecord * > PPRec = new PPConditionalDirectiveRecord(SourceMgr); > PP.addPPCallbacks(PPRec); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- http://www.nuanti.com the browser experts _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
