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-commits mailing list