[cfe-dev] Default arguments considered harmful?
Alp Toker
alp at nuanti.com
Thu May 1 20:57:31 PDT 2014
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=207825&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=207825&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/SourceManagerTest.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?rev=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/PPConditionalDirectiveRecordTest.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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
More information about the cfe-dev
mailing list