From 6b9e647239b67aff3119efbe2691fefe9c2dc28b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 14 May 2015 23:47:40 +0200 Subject: cmMakefile: Port CurrentListFile clients to GetDefinition. There is no need to store this as a member variable. --- Source/cmExtraQbsGenerator.cxx | 4 +++- Source/cmLocalNinjaGenerator.cxx | 4 ++-- Source/cmMakefile.cxx | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/cmExtraQbsGenerator.cxx b/Source/cmExtraQbsGenerator.cxx index 4cc4650..c15f8da 100644 --- a/Source/cmExtraQbsGenerator.cxx +++ b/Source/cmExtraQbsGenerator.cxx @@ -182,7 +182,9 @@ void cmExtraQbsGenerator::AppendSources(cmGeneratedFileStream &fout, std::vector genSources; std::vector::const_iterator itr = sources.begin(); fout << "\t\t\tfiles: [\n" - << "\t\t\t\t\"" << t.GetMakefile()->GetCurrentListFile() << "\",\n"; + << "\t\t\t\t\"" + << t.GetMakefile()->GetDefinition("CMAKE_CURRENT_LIST_FILE") + << "\",\n"; for (; itr != sources.end(); ++itr) { if (!(*itr)->GetPropertyAsBool("GENERATED")) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index f1f1202..bcae486 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -305,8 +305,8 @@ void cmLocalNinjaGenerator::WriteProcessedMakefile(std::ostream& os) cmGlobalNinjaGenerator::WriteDivider(os); os << "# Write statements declared in CMakeLists.txt:" << std::endl - << "# " << this->Makefile->GetCurrentListFile() << std::endl - ; + << "# " + << this->Makefile->GetDefinition("CMAKE_CURRENT_LIST_FILE") << std::endl; if(this->IsRootMakefile()) os << "# Which is the root file." << std::endl; cmGlobalNinjaGenerator::WriteDivider(os); diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 3e19cbb..0a112cb 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -609,7 +609,8 @@ bool cmMakefile::ProcessBuildsystemFile(const char* listfile) bool cmMakefile::ReadDependentFile(const char* listfile, bool noPolicyScope) { - this->AddDefinition("CMAKE_PARENT_LIST_FILE", this->GetCurrentListFile()); + this->AddDefinition("CMAKE_PARENT_LIST_FILE", + this->GetDefinition("CMAKE_CURRENT_LIST_FILE")); this->cmCurrentListFile = cmSystemTools::CollapseFullPath(listfile, this->GetCurrentSourceDirectory()); -- cgit v0.12 From f3e6a336f29b0675e4eb3852ce7544f3c9342071 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 14 May 2015 23:49:10 +0200 Subject: cmMakefile: Remove CurrentListFile member. It is never read externally. The CollapseFullPath removed in this commit is a repeat of a similar call inside ReadListFile. --- Source/cmMakefile.cxx | 7 +------ Source/cmMakefile.h | 10 ---------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 0a112cb..e69c2ba 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -601,7 +601,6 @@ void cmMakefile::IncludeScope::EnforceCMP0011() bool cmMakefile::ProcessBuildsystemFile(const char* listfile) { this->AddDefinition("CMAKE_PARENT_LIST_FILE", listfile); - this->cmCurrentListFile = listfile; std::string curSrc = this->GetCurrentSourceDirectory(); return this->ReadListFile(listfile, true, curSrc == this->GetHomeDirectory()); @@ -611,11 +610,7 @@ bool cmMakefile::ReadDependentFile(const char* listfile, bool noPolicyScope) { this->AddDefinition("CMAKE_PARENT_LIST_FILE", this->GetDefinition("CMAKE_CURRENT_LIST_FILE")); - this->cmCurrentListFile = - cmSystemTools::CollapseFullPath(listfile, - this->GetCurrentSourceDirectory()); - return this->ReadListFile(this->cmCurrentListFile.c_str(), - noPolicyScope); + return this->ReadListFile(listfile, noPolicyScope); } //---------------------------------------------------------------------------- diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 82f2715..2a72cca 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -418,14 +418,6 @@ public: void SetCurrentBinaryDirectory(const std::string& dir); const char* GetCurrentBinaryDirectory() const; - /* Get the current CMakeLists.txt file that is being processed. This - * is just used in order to be able to 'branch' from one file to a second - * transparently */ - const char* GetCurrentListFile() const - { - return this->cmCurrentListFile.c_str(); - } - //@} /** @@ -845,8 +837,6 @@ protected: // Check for a an unused variable void CheckForUnused(const char* reason, const std::string& name) const; - std::string cmCurrentListFile; - std::string ProjectName; // project name // libraries, classes, and executables -- cgit v0.12 From 2dd5d42f5254abda64009434f0b11a47ca16640e Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 15 May 2015 00:09:53 +0200 Subject: cmMakefile: Make the public ReadListFile method take one param. Make the existing method a private overload. All external callers invoke the method with only one argument. --- Source/cmMakefile.cxx | 10 ++++++---- Source/cmMakefile.h | 11 +++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index e69c2ba..016425e 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -610,12 +610,14 @@ bool cmMakefile::ReadDependentFile(const char* listfile, bool noPolicyScope) { this->AddDefinition("CMAKE_PARENT_LIST_FILE", this->GetDefinition("CMAKE_CURRENT_LIST_FILE")); - return this->ReadListFile(listfile, noPolicyScope); + return this->ReadListFile(listfile, noPolicyScope, false); +} + +bool cmMakefile::ReadListFile(const char* listfile) +{ + return this->ReadListFile(listfile, true, false); } -//---------------------------------------------------------------------------- -// Parse the given CMakeLists.txt file executing all commands -// bool cmMakefile::ReadListFile(const char* listfile, bool noPolicyScope, bool requireProjectCommand) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 2a72cca..8c50e8e 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -84,12 +84,7 @@ public: */ ~cmMakefile(); - /** - * Read and parse a CMakeLists.txt file. - */ - bool ReadListFile(const char* listfile, - bool noPolicyScope = true, - bool requireProjectCommand = false); + bool ReadListFile(const char* listfile); bool ReadDependentFile(const char* listfile, bool noPolicyScope = true); @@ -902,6 +897,10 @@ private: cmState::Snapshot StateSnapshot; + bool ReadListFile(const char* listfile, + bool noPolicyScope, + bool requireProjectCommand); + bool ReadListFileInternal(const char* filenametoread, bool noPolicyScope, bool requireProjectCommand); -- cgit v0.12 From 8ab1cce7047b637f504b4b83cecc104e5cd1adc5 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 07:30:33 +0200 Subject: cmMakefile: Rename method to something more appropriate. Allow the name to be used for something more-suitable. --- Source/cmMakefile.cxx | 2 +- Source/cmMakefile.h | 2 +- Source/cmTryRunCommand.cxx | 2 +- Source/cmakemain.cxx | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 016425e..c1bbd12 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4362,7 +4362,7 @@ void cmMakefile::AddCMakeDependFilesFromUser() } } -std::string cmMakefile::GetListFileStack() const +std::string cmMakefile::FormatListFileStack() const { std::ostringstream tmp; size_t depth = this->ListFileStack.size(); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 8c50e8e..72d8ec6 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -575,7 +575,7 @@ public: { this->ListFiles.push_back(file);} void AddCMakeDependFilesFromUser(); - std::string GetListFileStack() const; + std::string FormatListFileStack() const; /** * Get the current context backtrace. diff --git a/Source/cmTryRunCommand.cxx b/Source/cmTryRunCommand.cxx index 3cd92cb..b9ffe5e 100644 --- a/Source/cmTryRunCommand.cxx +++ b/Source/cmTryRunCommand.cxx @@ -375,7 +375,7 @@ void cmTryRunCommand::DoNotRunExecutable(const std::string& runArgs, comment += "Run arguments : "; comment += runArgs; comment += "\n"; - comment += " Called from: " + this->Makefile->GetListFileStack(); + comment += " Called from: " + this->Makefile->FormatListFileStack(); cmsys::SystemTools::ReplaceString(comment, "\n", "\n# "); file << comment << "\n\n"; diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 577dcd9..e5f4700 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -130,7 +130,7 @@ static std::string cmakemainGetStack(void *clientdata) cmMakefile* mf=cmakemainGetMakefile(clientdata); if (mf) { - msg = mf->GetListFileStack(); + msg = mf->FormatListFileStack(); if (!msg.empty()) { msg = "\n Called from: " + msg; -- cgit v0.12 From 390bc3244fd94d687bd48860eb2a70ad9674d755 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 2 May 2015 10:25:13 +0200 Subject: cmMakefile: Remove redundant condition. As this is called in the constructor, the definition will never be already set. --- Source/cmMakefile.cxx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index c1bbd12..e84ea4e 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -230,18 +230,12 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) { const char* dir = this->GetCMakeInstance()->GetHomeDirectory(); this->AddDefinition("CMAKE_SOURCE_DIR", dir); - if ( !this->GetDefinition("CMAKE_CURRENT_SOURCE_DIR") ) - { - this->AddDefinition("CMAKE_CURRENT_SOURCE_DIR", dir); - } + this->AddDefinition("CMAKE_CURRENT_SOURCE_DIR", dir); } { const char* dir = this->GetCMakeInstance()->GetHomeOutputDirectory(); this->AddDefinition("CMAKE_BINARY_DIR", dir); - if ( !this->GetDefinition("CMAKE_CURRENT_BINARY_DIR") ) - { - this->AddDefinition("CMAKE_CURRENT_BINARY_DIR", dir); - } + this->AddDefinition("CMAKE_CURRENT_BINARY_DIR", dir); } } -- cgit v0.12 From 5b7ff35c4d8b41e3d08305b91b0e5973f2e3906b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 07:29:36 +0200 Subject: cmMakefile: Don't expect the VarStack iterator to support size(). --- Source/cmMakefile.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index e84ea4e..1a192f5 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -105,9 +105,8 @@ public: bool RaiseScope(std::string const& var, const char* varDef, cmMakefile* mf) { - assert(this->VarStack.size() > 0); - std::list::reverse_iterator it = this->VarStack.rbegin(); + assert(it != this->VarStack.rend()); ++it; if(it == this->VarStack.rend()) { -- cgit v0.12 From 1363bff83a8e525814bbcfd1e90173d392ecc52b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 16:39:36 +0200 Subject: cmMakefile: Remove duplicate variable initialization. --- Source/cmMakefile.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 1a192f5..a95f4b8 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -152,8 +152,8 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) this->Internal->IsSourceFileTryCompile = false; // Initialize these first since AddDefaultDefinitions calls AddDefinition - this->WarnUnused = false; - this->CheckSystemVars = false; + this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); + this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); this->GeneratingBuildSystem = false; this->SuppressWatches = false; @@ -223,8 +223,6 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) #endif this->Properties.SetCMakeInstance(this->GetCMakeInstance()); - this->WarnUnused = this->GetCMakeInstance()->GetWarnUnused(); - this->CheckSystemVars = this->GetCMakeInstance()->GetCheckSystemVars(); { const char* dir = this->GetCMakeInstance()->GetHomeDirectory(); -- cgit v0.12 From bb1e8c3adfc4b188b0f340d757591039ffda4ef9 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 16:43:17 +0200 Subject: cmMakefile: Remove Print() debugging facilities. They don't print things that are important in the modern implementation. --- Source/cmMakefile.cxx | 53 --------------------------------------------------- Source/cmMakefile.h | 9 --------- 2 files changed, 62 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a95f4b8..1eadecf 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -253,59 +253,6 @@ cmMakefile::~cmMakefile() } } -void cmMakefile::PrintStringVector(const char* s, - const std::vector& v) const -{ - std::cout << s << ": ( \n" << cmWrap('"', v, '"', " ") << ")\n"; -} - -void cmMakefile -::PrintStringVector(const char* s, - const std::vector >& v) const -{ - std::cout << s << ": ( \n"; - for(std::vector >::const_iterator i - = v.begin(); i != v.end(); ++i) - { - std::cout << i->first << " " << i->second; - } - std::cout << " )\n"; -} - - -// call print on all the classes in the makefile -void cmMakefile::Print() const -{ - // print the class lists - std::cout << "classes:\n"; - - std::cout << " this->Targets: "; - for (cmTargets::iterator l = this->Targets.begin(); - l != this->Targets.end(); l++) - { - std::cout << l->first << std::endl; - } - - std::cout << " this->StartOutputDirectory; " << - this->GetCurrentBinaryDirectory() << std::endl; - std::cout << " this->HomeOutputDirectory; " << - this->GetHomeOutputDirectory() << std::endl; - std::cout << " this->cmStartDirectory; " << - this->GetCurrentSourceDirectory() << std::endl; - std::cout << " this->cmHomeDirectory; " << - this->GetHomeDirectory() << std::endl; - std::cout << " this->ProjectName; " - << this->ProjectName << std::endl; - this->PrintStringVector("this->LinkDirectories", this->LinkDirectories); -#if defined(CMAKE_BUILD_WITH_CMAKE) - for( std::vector::const_iterator i = - this->SourceGroups.begin(); i != this->SourceGroups.end(); ++i) - { - std::cout << "Source Group: " << i->GetName() << std::endl; - } -#endif -} - //---------------------------------------------------------------------------- void cmMakefile::IssueMessage(cmake::MessageType t, std::string const& text) const diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 72d8ec6..f96ca92 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -157,11 +157,6 @@ public: */ void FinalPass(); - /** - * Print the object state to std::cout. - */ - void Print() const; - /** Add a custom command to the build. */ void AddCustomCommandToTarget(const std::string& target, const std::vector& byproducts, @@ -912,10 +907,6 @@ private: friend class cmMakeDepend; // make depend needs direct access // to the Sources array - void PrintStringVector(const char* s, const - std::vector >& v) const; - void PrintStringVector(const char* s, - const std::vector& v) const; void AddDefaultDefinitions(); typedef std::vector FunctionBlockersType; -- cgit v0.12 From caff8e5a3e03c7841a35e563c014322f26611aef Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 16:46:12 +0200 Subject: cmCTest: Remove unimplemented method. --- Source/cmTest.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Source/cmTest.h b/Source/cmTest.h index c6e7e42..ca88afe 100644 --- a/Source/cmTest.h +++ b/Source/cmTest.h @@ -40,11 +40,6 @@ public: return this->Command; } - /** - * Print the structure to std::cout. - */ - void Print() const; - ///! Set/Get a property of this source file void SetProperty(const std::string& prop, const char *value); void AppendProperty(const std::string& prop, -- cgit v0.12 From c42f0e2b3e7d3b8eed0ef0ab5a973fe9375339b1 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 12:07:10 +0200 Subject: cmMakefile: Remove redundant conditions. This container is never empty. --- Source/cmMakefile.cxx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 1eadecf..56766dc 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1730,8 +1730,7 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) } this->Internal->SetDefinition(name, value); - if (!this->Internal->VarUsageStack.empty() && - this->VariableInitialized(name)) + if (this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); @@ -1806,8 +1805,7 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, void cmMakefile::AddDefinition(const std::string& name, bool value) { this->Internal->SetDefinition(name, value ? "ON" : "OFF"); - if (!this->Internal->VarUsageStack.empty() && - this->VariableInitialized(name)) + if (this->VariableInitialized(name)) { this->CheckForUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); @@ -1904,8 +1902,7 @@ void cmMakefile::CheckForUnused(const char* reason, void cmMakefile::RemoveDefinition(const std::string& name) { this->Internal->RemoveDefinition(name); - if (!this->Internal->VarUsageStack.empty() && - this->VariableInitialized(name)) + if (this->VariableInitialized(name)) { this->CheckForUnused("unsetting", name); this->Internal->VarUsageStack.top().erase(name); -- cgit v0.12 From bdd1aa91ae28f5df7193e724b1351c616e56dc0a Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 13:20:34 +0200 Subject: cmMakefile: Don't use else after return. --- Source/cmMakefile.cxx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 56766dc..5be6b1b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -126,10 +126,7 @@ public: } return true; } - else - { - return false; - } + return false; } // First localize the definition in the current scope. this->GetDefinition(var); -- cgit v0.12 From c8cb66880c233414b6656ea3d23776f9ba07a4e4 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 13:24:18 +0200 Subject: cmMakefile: Use early return to reduce nested code. --- Source/cmMakefile.cxx | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 5be6b1b..21c9f47 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -110,23 +110,24 @@ public: ++it; if(it == this->VarStack.rend()) { - if(cmLocalGenerator* plg = mf->GetLocalGenerator()->GetParent()) + cmLocalGenerator* plg = mf->GetLocalGenerator()->GetParent(); + if(!plg) { - // Update the definition in the parent directory top scope. This - // directory's scope was initialized by the closure of the parent - // scope, so we do not need to localize the definition first. - cmMakefile* parent = plg->GetMakefile(); - if (varDef) - { - parent->AddDefinition(var, varDef); - } - else - { - parent->RemoveDefinition(var); - } - return true; + return false; } - return false; + // Update the definition in the parent directory top scope. This + // directory's scope was initialized by the closure of the parent + // scope, so we do not need to localize the definition first. + cmMakefile* parent = plg->GetMakefile(); + if (varDef) + { + parent->AddDefinition(var, varDef); + } + else + { + parent->RemoveDefinition(var); + } + return true; } // First localize the definition in the current scope. this->GetDefinition(var); -- cgit v0.12 From ea7b962be2f157f60f143725948e56b2f9f07042 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 13:21:02 +0200 Subject: cmMakefile: Raise variable in scope explicitly when needed. The Get method implicitly pulls a copy of all variables into a local scope. This is not necessary. --- Source/cmDefinitions.cxx | 16 +++++++++++++--- Source/cmDefinitions.h | 6 +++--- Source/cmMakefile.cxx | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index e2c6876..d7b6279 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -18,7 +18,7 @@ cmDefinitions::Def cmDefinitions::NoDef; //---------------------------------------------------------------------------- cmDefinitions::Def const& cmDefinitions::GetInternal( - const std::string& key, StackIter begin, StackIter end) + const std::string& key, StackIter begin, StackIter end, bool raise) { assert(begin != end); MapType::const_iterator i = begin->Map.find(key); @@ -32,7 +32,11 @@ cmDefinitions::Def const& cmDefinitions::GetInternal( { return cmDefinitions::NoDef; } - Def const& def = cmDefinitions::GetInternal(key, it, end); + Def const& def = cmDefinitions::GetInternal(key, it, end, raise); + if (!raise) + { + return def; + } return begin->Map.insert(MapType::value_type(key, def)).first->second; } @@ -40,10 +44,16 @@ cmDefinitions::Def const& cmDefinitions::GetInternal( const char* cmDefinitions::Get(const std::string& key, StackIter begin, StackIter end) { - Def const& def = cmDefinitions::GetInternal(key, begin, end); + Def const& def = cmDefinitions::GetInternal(key, begin, end, false); return def.Exists? def.c_str() : 0; } +void cmDefinitions::Raise(const std::string& key, + StackIter begin, StackIter end) +{ + cmDefinitions::GetInternal(key, begin, end, true); +} + //---------------------------------------------------------------------------- void cmDefinitions::Set(const std::string& key, const char* value) { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index bf791ed..17b9c7c 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -35,11 +35,11 @@ class cmDefinitions typedef std::list::reverse_iterator StackIter; typedef std::list::const_reverse_iterator StackConstIter; public: - /** Get the value associated with a key; null if none. - Store the result locally if it came from a parent. */ static const char* Get(const std::string& key, StackIter begin, StackIter end); + static void Raise(const std::string& key, StackIter begin, StackIter end); + /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); @@ -80,7 +80,7 @@ private: MapType Map; static Def const& GetInternal(const std::string& key, - StackIter begin, StackIter end); + StackIter begin, StackIter end, bool raise); }; #endif diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 21c9f47..adba110 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -130,7 +130,7 @@ public: return true; } // First localize the definition in the current scope. - this->GetDefinition(var); + cmDefinitions::Raise(var, this->VarStack.rbegin(), this->VarStack.rend()); // Now update the definition in the parent scope. it->Set(var, varDef); -- cgit v0.12 From f58c3774d15f16ea287ca52fcbd04c17f0a5612d Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 15:20:38 +0200 Subject: cmMakefile: Mark definitions explicitly erased, even at top level. Presumably the intention here is to attempt to optimize memory by not storing what is not needed. However, all keys need to be tracked anyway to implement initialization tracking, and this special case gets in the way of simplifying the implementation of that. This doesn't change any observable effects because values set to 0 are considered not to exist by the cmDefinitions API. --- Source/cmDefinitions.cxx | 5 ----- Source/cmDefinitions.h | 2 -- Source/cmMakefile.cxx | 10 +--------- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index d7b6279..97a16ea 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -61,11 +61,6 @@ void cmDefinitions::Set(const std::string& key, const char* value) this->Map[key] = def; } -void cmDefinitions::Erase(const std::string& key) -{ - this->Map.erase(key); -} - //---------------------------------------------------------------------------- std::vector cmDefinitions::LocalKeys() const { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 17b9c7c..894ff7a 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -43,8 +43,6 @@ public: /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); - void Erase(const std::string& key); - /** Get the set of all local keys. */ std::vector LocalKeys() const; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index adba110..ad48bb7 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -76,15 +76,7 @@ public: void RemoveDefinition(std::string const& name) { - if (this->VarStack.size() > 1) - { - // In lower scopes we store keys, defined or not. - this->VarStack.back().Set(name, 0); - } - else - { - this->VarStack.back().Erase(name); - } + this->VarStack.back().Set(name, 0); } std::vector LocalKeys() const -- cgit v0.12 From 9118b53b79afc1791d920b8ea6f9cedf26010148 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 16:27:47 +0200 Subject: cmMakefile: Move internal method to private scope. --- Source/cmMakefile.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index f96ca92..9054472 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -65,8 +65,6 @@ class cmMakefile class Internals; cmsys::auto_ptr Internal; public: - /* Check for unused variables in this scope */ - void CheckForUnusedVariables() const; /* Mark a variable as used */ void MarkVariableAsUsed(const std::string& var); /* return true if a variable has been initialized */ @@ -1042,6 +1040,8 @@ private: bool HaveCxxStandardAvailable(cmTarget const* target, const std::string& feature) const; + void CheckForUnusedVariables() const; + mutable bool SuppressWatches; }; -- cgit v0.12 From 528d68021c6769b2aa86ea9751a7308a84101ca2 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 16:28:39 +0200 Subject: cmMakefile: Use more suitable method name to log var usage. --- Source/cmMakefile.cxx | 12 ++++++------ Source/cmMakefile.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index ad48bb7..645a433 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1722,7 +1722,7 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) this->Internal->SetDefinition(name, value); if (this->VariableInitialized(name)) { - this->CheckForUnused("changing definition", name); + this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } this->Internal->VarInitStack.top().insert(name); @@ -1797,7 +1797,7 @@ void cmMakefile::AddDefinition(const std::string& name, bool value) this->Internal->SetDefinition(name, value ? "ON" : "OFF"); if (this->VariableInitialized(name)) { - this->CheckForUnused("changing definition", name); + this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } this->Internal->VarInitStack.top().insert(name); @@ -1821,7 +1821,7 @@ void cmMakefile::CheckForUnusedVariables() const std::vector::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { - this->CheckForUnused("out of scope", *it); + this->LogUnused("out of scope", *it); } } @@ -1850,7 +1850,7 @@ bool cmMakefile::VariableUsed(const std::string& var) const return false; } -void cmMakefile::CheckForUnused(const char* reason, +void cmMakefile::LogUnused(const char* reason, const std::string& name) const { if (this->WarnUnused && !this->VariableUsed(name)) @@ -1894,7 +1894,7 @@ void cmMakefile::RemoveDefinition(const std::string& name) this->Internal->RemoveDefinition(name); if (this->VariableInitialized(name)) { - this->CheckForUnused("unsetting", name); + this->LogUnused("unsetting", name); this->Internal->VarUsageStack.top().erase(name); } this->Internal->VarInitStack.top().insert(name); @@ -4347,7 +4347,7 @@ void cmMakefile::PopScope() init.erase(*it); if (!this->VariableUsed(*it)) { - this->CheckForUnused("out of scope", *it); + this->LogUnused("out of scope", *it); } else { diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 9054472..a8873ff 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -823,7 +823,7 @@ protected: void AddGlobalLinkInformation(const std::string& name, cmTarget& target); // Check for a an unused variable - void CheckForUnused(const char* reason, const std::string& name) const; + void LogUnused(const char* reason, const std::string& name) const; std::string ProjectName; // project name -- cgit v0.12 From 2b09d9f346bd3220b059771a6da1bafb06ce0f5b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 13:37:41 +0200 Subject: cmMakefile: Remove VarInitStack. In cmMakefile::PushScope, a copy of the closure of keys initialized in the parent scope is made. In PopScope, essentially the same copy is inserted back into the parent. That means a lot of duplication of strings and a lot of string comparisons. None of it is needed, because the cmDefinitions keys already provide a canonical representation of what is initialized. The removal of the separate container also makes the variable handling code more easy to reason about in general. Before this patch, configuring llvm uses 200 KiB for the VarInitStack. Overall peak memory consumption goes from 35.5 MiB to 35.1 MiB. --- Source/cmDefinitions.cxx | 14 ++++++++++++++ Source/cmDefinitions.h | 3 +++ Source/cmMakefile.cxx | 32 +++++++++++--------------------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 97a16ea..5b03bd4 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -54,6 +54,20 @@ void cmDefinitions::Raise(const std::string& key, cmDefinitions::GetInternal(key, begin, end, true); } +bool cmDefinitions::HasKey(const std::string& key, + StackConstIter begin, StackConstIter end) +{ + for (StackConstIter it = begin; it != end; ++it) + { + MapType::const_iterator i = it->Map.find(key); + if (i != it->Map.end()) + { + return true; + } + } + return false; +} + //---------------------------------------------------------------------------- void cmDefinitions::Set(const std::string& key, const char* value) { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 894ff7a..bd3d392 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -40,6 +40,9 @@ public: static void Raise(const std::string& key, StackIter begin, StackIter end); + static bool HasKey(const std::string& key, + StackConstIter begin, StackConstIter end); + /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 645a433..bad4e6a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -47,7 +47,6 @@ class cmMakefile::Internals { public: std::list VarStack; - std::stack > VarInitStack; std::stack > VarUsageStack; bool IsSourceFileTryCompile; @@ -69,6 +68,12 @@ public: this->VarStack.rend()); } + bool IsInitialized(std::string const& name) + { + return cmDefinitions::HasKey(name, this->VarStack.rbegin(), + this->VarStack.rend()); + } + void SetDefinition(std::string const& name, std::string const& value) { this->VarStack.back().Set(name, value.c_str()); @@ -137,7 +142,6 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) StateSnapshot(localGenerator->GetStateSnapshot()) { this->Internal->PushDefinitions(); - this->Internal->VarInitStack.push(std::set()); this->Internal->VarUsageStack.push(std::set()); this->Internal->IsSourceFileTryCompile = false; @@ -1719,13 +1723,12 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) return; } - this->Internal->SetDefinition(name, value); if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->SetDefinition(name, value); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); @@ -1794,13 +1797,12 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, void cmMakefile::AddDefinition(const std::string& name, bool value) { - this->Internal->SetDefinition(name, value ? "ON" : "OFF"); if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->SetDefinition(name, value ? "ON" : "OFF"); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -1832,12 +1834,7 @@ void cmMakefile::MarkVariableAsUsed(const std::string& var) bool cmMakefile::VariableInitialized(const std::string& var) const { - if(this->Internal->VarInitStack.top().find(var) != - this->Internal->VarInitStack.top().end()) - { - return true; - } - return false; + return this->Internal->IsInitialized(var); } bool cmMakefile::VariableUsed(const std::string& var) const @@ -1891,13 +1888,12 @@ void cmMakefile::LogUnused(const char* reason, void cmMakefile::RemoveDefinition(const std::string& name) { - this->Internal->RemoveDefinition(name); if (this->VariableInitialized(name)) { this->LogUnused("unsetting", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->RemoveDefinition(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -4316,9 +4312,7 @@ std::string cmMakefile::FormatListFileStack() const void cmMakefile::PushScope() { this->Internal->PushDefinitions(); - const std::set& init = this->Internal->VarInitStack.top(); const std::set& usage = this->Internal->VarUsageStack.top(); - this->Internal->VarInitStack.push(init); this->Internal->VarUsageStack.push(usage); this->PushLoopBlockBarrier(); @@ -4336,7 +4330,6 @@ void cmMakefile::PopScope() this->PopLoopBlockBarrier(); - std::set init = this->Internal->VarInitStack.top(); std::set usage = this->Internal->VarUsageStack.top(); const std::vector& locals = this->Internal->LocalKeys(); // Remove initialization and usage information for variables in the local @@ -4344,7 +4337,6 @@ void cmMakefile::PopScope() std::vector::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { - init.erase(*it); if (!this->VariableUsed(*it)) { this->LogUnused("out of scope", *it); @@ -4356,10 +4348,8 @@ void cmMakefile::PopScope() } this->Internal->PopDefinitions(); - this->Internal->VarInitStack.pop(); this->Internal->VarUsageStack.pop(); - // Push initialization and usage up to the parent scope. - this->Internal->VarInitStack.top().insert(init.begin(), init.end()); + // Push usage up to the parent scope. this->Internal->VarUsageStack.top().insert(usage.begin(), usage.end()); } -- cgit v0.12 From b9f9915516c9b426f4f528bb1ec5a79d115e21ab Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 16:19:55 +0200 Subject: cmMakefile: Remove VarUsageStack. Store usage information in the cmDefintions value instead. We already pay for the memory as padding in the Def struct, so we might as well use it. --- Source/cmDefinitions.cxx | 7 +++--- Source/cmDefinitions.h | 16 ++++++++----- Source/cmMakefile.cxx | 58 +++++++----------------------------------------- Source/cmMakefile.h | 2 -- 4 files changed, 22 insertions(+), 61 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 5b03bd4..2dab169 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -21,9 +21,10 @@ cmDefinitions::Def const& cmDefinitions::GetInternal( const std::string& key, StackIter begin, StackIter end, bool raise) { assert(begin != end); - MapType::const_iterator i = begin->Map.find(key); + MapType::iterator i = begin->Map.find(key); if (i != begin->Map.end()) { + i->second.Used = true; return i->second; } StackIter it = begin; @@ -76,7 +77,7 @@ void cmDefinitions::Set(const std::string& key, const char* value) } //---------------------------------------------------------------------------- -std::vector cmDefinitions::LocalKeys() const +std::vector cmDefinitions::UnusedKeys() const { std::vector keys; keys.reserve(this->Map.size()); @@ -84,7 +85,7 @@ std::vector cmDefinitions::LocalKeys() const for(MapType::const_iterator mi = this->Map.begin(); mi != this->Map.end(); ++mi) { - if (mi->second.Exists) + if (!mi->second.Used) { keys.push_back(mi->first); } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index bd3d392..5fdcaab 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -46,8 +46,7 @@ public: /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); - /** Get the set of all local keys. */ - std::vector LocalKeys() const; + std::vector UnusedKeys() const; static std::vector ClosureKeys(StackConstIter begin, StackConstIter end); @@ -61,11 +60,16 @@ private: private: typedef std::string std_string; public: - Def(): std_string(), Exists(false) {} - Def(const char* v): std_string(v?v:""), Exists(v?true:false) {} - Def(const std_string& v): std_string(v), Exists(true) {} - Def(Def const& d): std_string(d), Exists(d.Exists) {} + Def(): std_string(), Exists(false), Used(false) {} + Def(const char* v) + : std_string(v ? v : ""), + Exists(v ? true : false), + Used(false) + {} + Def(const std_string& v): std_string(v), Exists(true), Used(false) {} + Def(Def const& d): std_string(d), Exists(d.Exists), Used(d.Used) {} bool Exists; + bool Used; }; static Def NoDef; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index bad4e6a..31ab66d 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -38,7 +38,6 @@ #include #include -#include #include #include // for isspace #include @@ -47,7 +46,6 @@ class cmMakefile::Internals { public: std::list VarStack; - std::stack > VarUsageStack; bool IsSourceFileTryCompile; void PushDefinitions() @@ -84,9 +82,9 @@ public: this->VarStack.back().Set(name, 0); } - std::vector LocalKeys() const + std::vector UnusedKeys() const { - return this->VarStack.back().LocalKeys(); + return this->VarStack.back().UnusedKeys(); } std::vector ClosureKeys() const @@ -142,7 +140,6 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) StateSnapshot(localGenerator->GetStateSnapshot()) { this->Internal->PushDefinitions(); - this->Internal->VarUsageStack.push(std::set()); this->Internal->IsSourceFileTryCompile = false; // Initialize these first since AddDefaultDefinitions calls AddDefinition @@ -588,7 +585,6 @@ bool cmMakefile::ReadListFile(const char* listfile, if (res) { - // Check for unused variables this->CheckForUnusedVariables(); } @@ -1726,7 +1722,6 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); - this->Internal->VarUsageStack.top().erase(name); } this->Internal->SetDefinition(name, value); @@ -1800,7 +1795,6 @@ void cmMakefile::AddDefinition(const std::string& name, bool value) if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); - this->Internal->VarUsageStack.top().erase(name); } this->Internal->SetDefinition(name, value ? "ON" : "OFF"); #ifdef CMAKE_BUILD_WITH_CMAKE @@ -1819,9 +1813,9 @@ void cmMakefile::CheckForUnusedVariables() const { return; } - const std::vector& locals = this->Internal->LocalKeys(); - std::vector::const_iterator it = locals.begin(); - for (; it != locals.end(); ++it) + const std::vector& unused = this->Internal->UnusedKeys(); + std::vector::const_iterator it = unused.begin(); + for (; it != unused.end(); ++it) { this->LogUnused("out of scope", *it); } @@ -1829,7 +1823,7 @@ void cmMakefile::CheckForUnusedVariables() const void cmMakefile::MarkVariableAsUsed(const std::string& var) { - this->Internal->VarUsageStack.top().insert(var); + this->Internal->GetDefinition(var); } bool cmMakefile::VariableInitialized(const std::string& var) const @@ -1837,20 +1831,10 @@ bool cmMakefile::VariableInitialized(const std::string& var) const return this->Internal->IsInitialized(var); } -bool cmMakefile::VariableUsed(const std::string& var) const -{ - if(this->Internal->VarUsageStack.top().find(var) != - this->Internal->VarUsageStack.top().end()) - { - return true; - } - return false; -} - void cmMakefile::LogUnused(const char* reason, const std::string& name) const { - if (this->WarnUnused && !this->VariableUsed(name)) + if (this->WarnUnused) { std::string path; cmListFileBacktrace bt(this->GetLocalGenerator()); @@ -1891,7 +1875,6 @@ void cmMakefile::RemoveDefinition(const std::string& name) if (this->VariableInitialized(name)) { this->LogUnused("unsetting", name); - this->Internal->VarUsageStack.top().erase(name); } this->Internal->RemoveDefinition(name); #ifdef CMAKE_BUILD_WITH_CMAKE @@ -2360,7 +2343,6 @@ const char* cmMakefile::GetRequiredDefinition(const std::string& name) const bool cmMakefile::IsDefinitionSet(const std::string& name) const { const char* def = this->Internal->GetDefinition(name); - this->Internal->VarUsageStack.top().insert(name); if(!def) { def = this->GetState()->GetInitializedCacheValue(name); @@ -2381,10 +2363,6 @@ bool cmMakefile::IsDefinitionSet(const std::string& name) const const char* cmMakefile::GetDefinition(const std::string& name) const { - if (this->WarnUnused) - { - this->Internal->VarUsageStack.top().insert(name); - } const char* def = this->Internal->GetDefinition(name); if(!def) { @@ -4312,8 +4290,6 @@ std::string cmMakefile::FormatListFileStack() const void cmMakefile::PushScope() { this->Internal->PushDefinitions(); - const std::set& usage = this->Internal->VarUsageStack.top(); - this->Internal->VarUsageStack.push(usage); this->PushLoopBlockBarrier(); @@ -4330,27 +4306,9 @@ void cmMakefile::PopScope() this->PopLoopBlockBarrier(); - std::set usage = this->Internal->VarUsageStack.top(); - const std::vector& locals = this->Internal->LocalKeys(); - // Remove initialization and usage information for variables in the local - // scope. - std::vector::const_iterator it = locals.begin(); - for (; it != locals.end(); ++it) - { - if (!this->VariableUsed(*it)) - { - this->LogUnused("out of scope", *it); - } - else - { - usage.erase(*it); - } - } + this->CheckForUnusedVariables(); this->Internal->PopDefinitions(); - this->Internal->VarUsageStack.pop(); - // Push usage up to the parent scope. - this->Internal->VarUsageStack.top().insert(usage.begin(), usage.end()); } void cmMakefile::RaiseScope(const std::string& var, const char *varDef) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index a8873ff..efd73a1 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -69,8 +69,6 @@ public: void MarkVariableAsUsed(const std::string& var); /* return true if a variable has been initialized */ bool VariableInitialized(const std::string& ) const; - /* return true if a variable has been used */ - bool VariableUsed(const std::string& ) const; /** * Construct an empty makefile. -- cgit v0.12