[cfe-dev] Default arguments considered harmful?
David Tweed
david.tweed at arm.com
Fri May 2 01:48:27 PDT 2014
Ah forget that. It's been a while since I tried these games and remember now
why it doesn't work.
-----Original Message-----
From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] On
Behalf Of David Tweed
Sent: 02 May 2014 09:17
To: 'Alp Toker'; cfe-commits at cs.uiuc.edu; 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: cfe-commits-bounces at cs.uiuc.edu
[mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Alp Toker
Sent: 02 May 2014 04:58
To: cfe-commits at cs.uiuc.edu; 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-dev mailing list
cfe-dev at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
More information about the cfe-commits
mailing list