From 18b6676bff6d50ebc38c75e7998298fca75a01c5 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 13 Apr 2016 09:41:24 -0400 Subject: [PATCH 1/5] cmState: Add Snapshot method to get bottom of call stack The bottom of the call stack is always a long-lived snapshot and can be saved for later use with cmOutputConverter. --- Source/cmState.cxx | 15 +++++++++++++++ Source/cmState.h | 1 + 2 files changed, 16 insertions(+) diff --git a/Source/cmState.cxx b/Source/cmState.cxx index be8e4182ce..c48f9b1fe5 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -1213,6 +1213,21 @@ cmState::Snapshot cmState::Snapshot::GetCallStackParent() const return snapshot; } +cmState::Snapshot cmState::Snapshot::GetCallStackBottom() const +{ + assert(this->State); + assert(this->Position != this->State->SnapshotData.Root()); + + PositionType pos = this->Position; + while (pos->SnapshotType != cmState::BaseType && + pos->SnapshotType != cmState::BuildsystemDirectoryType && + pos != this->State->SnapshotData.Root()) + { + ++pos; + } + return Snapshot(this->State, pos); +} + void cmState::Snapshot::PushPolicy(cmPolicies::PolicyMap entry, bool weak) { PositionType pos = this->Position; diff --git a/Source/cmState.h b/Source/cmState.h index ef61406384..0cce6444a6 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -75,6 +75,7 @@ public: bool IsValid() const; Snapshot GetBuildsystemDirectoryParent() const; Snapshot GetCallStackParent() const; + Snapshot GetCallStackBottom() const; SnapshotType GetType() const; void SetPolicy(cmPolicies::PolicyID id, cmPolicies::PolicyStatus status); From 1f6bd8a93f602f9d4dde1c3ea697211897f25827 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 12 Apr 2016 17:07:08 -0400 Subject: [PATCH 2/5] cmState: Avoid accumulating snapshot storage for backtraces Changes during post-3.3/pre-3.4 development refactored storage of most configure-time information, including variable bindings and function scopes. All scopes (even short-lived) were kept persistently for possible future debugging features, causing huge accumulated memory usage. This was mostly addressed by commit v3.4.1~4^2 (cmState: Avoid accumulating snapshot storage for short-lived scopes, 2015-11-24). Since then we still keep short-lived scopes when they are needed for a backtrace. This is because since commit v3.4.0-rc1~378^2 (cmListFileBacktrace: Implement in terms of cmState::Snapshot, 2015-05-29) backtraces have been lightweight objects that simply point into the snapshot tree. While the intention of this approach was to avoid duplicating the call stack file path strings, the cost turned out to be holding on to the entire call stack worth of scope snapshots, which is much worse. Furthermore, since commit v3.4.0-rc2~1^2 (cmIfCommand: Issue CMP0054 warning with appropriate context, 2015-10-20) all conditions used in `if()` commands hold a backtrace for use in diagnostic messages. Even though the backtrace is short-lived it still causes the scope snapshot to be kept. This means that code like function(foo) if(0) endif() endfunction() foreach(i RANGE 1000000) foo() endforeach() accumulates storage for the function call scope snapshots. Fix this by partially reverting commit v3.4.0-rc1~378^2 and saving the entire call stack during cmListFileBacktrace construction. This way we can avoid keeping short-lived scope snapshot storage in all cases. --- Source/cmListFileCache.cxx | 67 +++++++++++++++++++++----------------- Source/cmListFileCache.h | 3 +- Source/cmState.cxx | 5 --- Source/cmState.h | 1 - 4 files changed, 39 insertions(+), 37 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index d5d0184957..f198ac3021 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -400,13 +400,40 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot snapshot, cmCommandContext const& cc) - : Context(cc) - , Snapshot(snapshot) + : Snapshot(snapshot) { - if (this->Snapshot.IsValid()) + if (!this->Snapshot.IsValid()) { - this->Snapshot.Keep(); + return; } + + // Record the entire call stack now so that the `Snapshot` we + // save for later refers to a long-lived scope. This avoids + // having to keep short-lived scopes around just to extract + // their backtrace information later. + + cmListFileContext lfc = + cmListFileContext::FromCommandContext( + cc, this->Snapshot.GetExecutionListFile()); + this->push_back(lfc); + + cmState::Snapshot parent = this->Snapshot.GetCallStackParent(); + while (parent.IsValid()) + { + lfc.Name = this->Snapshot.GetEntryPointCommand(); + lfc.Line = this->Snapshot.GetEntryPointLine(); + lfc.FilePath = parent.GetExecutionListFile(); + if (lfc.FilePath.empty()) + { + break; + } + this->push_back(lfc); + + this->Snapshot = parent; + parent = parent.GetCallStackParent(); + } + + this->Snapshot = this->Snapshot.GetCallStackBottom(); } cmListFileBacktrace::~cmListFileBacktrace() @@ -415,48 +442,30 @@ cmListFileBacktrace::~cmListFileBacktrace() void cmListFileBacktrace::PrintTitle(std::ostream& out) const { - if (!this->Snapshot.IsValid()) + if (this->empty()) { return; } cmOutputConverter converter(this->Snapshot); - cmListFileContext lfc = - cmListFileContext::FromCommandContext( - this->Context, this->Snapshot.GetExecutionListFile()); + cmListFileContext lfc = this->front(); lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); out << (lfc.Line ? " at " : " in ") << lfc; } void cmListFileBacktrace::PrintCallStack(std::ostream& out) const { - if (!this->Snapshot.IsValid()) - { - return; - } - cmState::Snapshot parent = this->Snapshot.GetCallStackParent(); - if (!parent.IsValid() || parent.GetExecutionListFile().empty()) + if (this->size() <= 1) { return; } + out << "Call Stack (most recent call first):\n"; cmOutputConverter converter(this->Snapshot); - std::string commandName = this->Snapshot.GetEntryPointCommand(); - long commandLine = this->Snapshot.GetEntryPointLine(); - - out << "Call Stack (most recent call first):\n"; - while(parent.IsValid()) + for (const_iterator i = this->begin() + 1; i != this->end(); ++i) { - cmListFileContext lfc; - lfc.Name = commandName; - lfc.Line = commandLine; - - lfc.FilePath = converter.Convert(parent.GetExecutionListFile(), - cmOutputConverter::HOME); + cmListFileContext lfc = *i; + lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); out << " " << lfc << "\n"; - - commandName = parent.GetEntryPointCommand(); - commandLine = parent.GetEntryPointLine(); - parent = parent.GetCallStackParent(); } } diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 4d3055f2ca..d3cab22bab 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -87,7 +87,7 @@ struct cmListFileFunction: public cmCommandContext std::vector Arguments; }; -class cmListFileBacktrace +class cmListFileBacktrace: private std::vector { public: cmListFileBacktrace(cmState::Snapshot snapshot = cmState::Snapshot(), @@ -97,7 +97,6 @@ class cmListFileBacktrace void PrintTitle(std::ostream& out) const; void PrintCallStack(std::ostream& out) const; private: - cmCommandContext Context; cmState::Snapshot Snapshot; }; diff --git a/Source/cmState.cxx b/Source/cmState.cxx index c48f9b1fe5..bec56822df 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -1098,11 +1098,6 @@ void cmState::Directory::SetCurrentBinary(std::string const& dir) this->Snapshot_.SetDefinition("CMAKE_CURRENT_BINARY_DIR", loc.c_str()); } -void cmState::Snapshot::Keep() -{ - this->Position->Keep = true; -} - void cmState::Snapshot::SetListFile(const std::string& listfile) { *this->Position->ExecutionListFile = listfile; diff --git a/Source/cmState.h b/Source/cmState.h index 0cce6444a6..507d500eaf 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -63,7 +63,6 @@ public: std::vector ClosureKeys() const; bool RaiseScope(std::string const& var, const char* varDef); - void Keep(); void SetListFile(std::string const& listfile); std::string GetExecutionListFile() const; From 7c36d2067b00996fcead56310f4fc4fa58434247 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 14 Apr 2016 11:24:08 -0400 Subject: [PATCH 3/5] cmListFileBacktrace: Refactor storage to provide efficient value semantics Since commit v3.4.0-rc1~321^2~2 (Genex: Store a backtrace, not a pointer to one, 2015-07-08) we treat cmListFileBacktrace instances as lightweight values. This was true at the time only because the backtrace information was kept in the cmState snapshot hierarchy. However, that forced us to accumulate a lot of otherwise short-lived snapshots just to have the backtrace fields available for reference by cmListFileBacktrace instances. Recent refactoring made backtrace instances independent of the snapshot hierarchy to avoid accumulating short-lived snapshots. This came at the cost of making backtrace values heavy again, leading to lots of string coying and slower execution. Fix this by refactoring cmListFileBacktrace to provide value semantics with efficient shared storage underneath. Teach cmMakefile to maintain its call stack using an instance of cmListFileBacktrace. This approach allows the current backtrace to be efficiently saved whenever it is needed. Also teach cmListFileBacktrace the notion of a file-level scope. This is useful for messages about the whole file (e.g. during parsing) that are not specific to any line within it. Push the CMakeLists.txt scope for each directory and never pop it. This ensures that we always have some context information and simplifies cmMakefile::IssueMessage. Push/pop a file-level scope as each included file is processed. This supersedes cmParseFileScope and improves diagnostic message context information in a few places. Fix the corresponding test cases to expect the improved output. --- Source/cmListFileCache.cxx | 177 ++++++++++++++---- Source/cmListFileCache.h | 54 +++++- Source/cmMakefile.cxx | 140 +++++--------- Source/cmMakefile.h | 2 +- Source/cmState.cxx | 28 +-- Source/cmState.h | 16 +- .../RunCMake/PolicyScope/NotClosed-stderr.txt | 2 +- .../Syntax/FunctionUnmatched-stderr.txt | 6 +- .../RunCMake/Syntax/MacroUnmatched-stderr.txt | 6 +- .../TargetSources/OriginDebugIDE-stderr.txt | 2 +- .../find_package/PolicyPush-stderr.txt | 2 +- Tests/RunCMake/while/EndMismatch-stderr.txt | 2 +- Tests/RunCMake/while/EndMissing-stderr.txt | 6 +- 13 files changed, 249 insertions(+), 194 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index f198ac3021..1cc66f47a9 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -398,73 +398,172 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, } } -cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot snapshot, - cmCommandContext const& cc) - : Snapshot(snapshot) +struct cmListFileBacktrace::Entry: public cmListFileContext { - if (!this->Snapshot.IsValid()) + Entry(cmListFileContext const& lfc, Entry* up): + cmListFileContext(lfc), Up(up), RefCount(0) { - return; - } - - // Record the entire call stack now so that the `Snapshot` we - // save for later refers to a long-lived scope. This avoids - // having to keep short-lived scopes around just to extract - // their backtrace information later. - - cmListFileContext lfc = - cmListFileContext::FromCommandContext( - cc, this->Snapshot.GetExecutionListFile()); - this->push_back(lfc); - - cmState::Snapshot parent = this->Snapshot.GetCallStackParent(); - while (parent.IsValid()) - { - lfc.Name = this->Snapshot.GetEntryPointCommand(); - lfc.Line = this->Snapshot.GetEntryPointLine(); - lfc.FilePath = parent.GetExecutionListFile(); - if (lfc.FilePath.empty()) + if (this->Up) { - break; + this->Up->Ref(); } - this->push_back(lfc); - - this->Snapshot = parent; - parent = parent.GetCallStackParent(); } + ~Entry() + { + if (this->Up) + { + this->Up->Unref(); + } + } + void Ref() + { + ++this->RefCount; + } + void Unref() + { + if (--this->RefCount == 0) + { + delete this; + } + } + Entry* Up; + unsigned int RefCount; +}; - this->Snapshot = this->Snapshot.GetCallStackBottom(); +cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot bottom, + Entry* up, + cmListFileContext const& lfc): + Bottom(bottom), Cur(new Entry(lfc, up)) +{ + assert(this->Bottom.IsValid()); + this->Cur->Ref(); +} + +cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot bottom, Entry* cur): + Bottom(bottom), Cur(cur) +{ + if (this->Cur) + { + assert(this->Bottom.IsValid()); + this->Cur->Ref(); + } +} + +cmListFileBacktrace::cmListFileBacktrace(): Bottom(), Cur(0) +{ +} + +cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot snapshot): + Bottom(snapshot.GetCallStackBottom()), Cur(0) +{ +} + +cmListFileBacktrace::cmListFileBacktrace(cmListFileBacktrace const& r): + Bottom(r.Bottom), Cur(r.Cur) +{ + if (this->Cur) + { + assert(this->Bottom.IsValid()); + this->Cur->Ref(); + } +} + +cmListFileBacktrace& +cmListFileBacktrace::operator=(cmListFileBacktrace const& r) +{ + cmListFileBacktrace tmp(r); + std::swap(this->Cur, tmp.Cur); + std::swap(this->Bottom, tmp.Bottom); + return *this; } cmListFileBacktrace::~cmListFileBacktrace() { + if (this->Cur) + { + this->Cur->Unref(); + } +} + +cmListFileBacktrace +cmListFileBacktrace::Push(std::string const& file) const +{ + // We are entering a file-level scope but have not yet reached + // any specific line or command invocation within it. This context + // is useful to print when it is at the top but otherwise can be + // skipped during call stack printing. + cmListFileContext lfc; + lfc.FilePath = file; + return cmListFileBacktrace(this->Bottom, this->Cur, lfc); +} + +cmListFileBacktrace +cmListFileBacktrace::Push(cmListFileContext const& lfc) const +{ + return cmListFileBacktrace(this->Bottom, this->Cur, lfc); +} + +cmListFileBacktrace cmListFileBacktrace::Pop() const +{ + assert(this->Cur); + return cmListFileBacktrace(this->Bottom, this->Cur->Up); +} + +cmListFileContext const& cmListFileBacktrace::Top() const +{ + if (this->Cur) + { + return *this->Cur; + } + else + { + static cmListFileContext const empty; + return empty; + } } void cmListFileBacktrace::PrintTitle(std::ostream& out) const { - if (this->empty()) + if (!this->Cur) { return; } - cmOutputConverter converter(this->Snapshot); - cmListFileContext lfc = this->front(); - lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); + cmOutputConverter converter(this->Bottom); + cmListFileContext lfc = *this->Cur; + if (!this->Bottom.GetState()->GetIsInTryCompile()) + { + lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); + } out << (lfc.Line ? " at " : " in ") << lfc; } void cmListFileBacktrace::PrintCallStack(std::ostream& out) const { - if (this->size() <= 1) + if (!this->Cur || !this->Cur->Up) { return; } - out << "Call Stack (most recent call first):\n"; - cmOutputConverter converter(this->Snapshot); - for (const_iterator i = this->begin() + 1; i != this->end(); ++i) + bool first = true; + cmOutputConverter converter(this->Bottom); + for (Entry* i = this->Cur->Up; i; i = i->Up) { + if (i->Name.empty()) + { + // Skip this whole-file scope. When we get here we already will + // have printed a more-specific context within the file. + continue; + } + if (first) + { + first = false; + out << "Call Stack (most recent call first):\n"; + } cmListFileContext lfc = *i; - lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); + if (!this->Bottom.GetState()->GetIsInTryCompile()) + { + lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); + } out << " " << lfc << "\n"; } } diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index d3cab22bab..9fa858510b 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -87,17 +87,53 @@ struct cmListFileFunction: public cmCommandContext std::vector Arguments; }; -class cmListFileBacktrace: private std::vector +// Represent a backtrace (call stack). Provide value semantics +// but use efficient reference-counting underneath to avoid copies. +class cmListFileBacktrace { - public: - cmListFileBacktrace(cmState::Snapshot snapshot = cmState::Snapshot(), - cmCommandContext const& cc = cmCommandContext()); - ~cmListFileBacktrace(); +public: + // Default-constructed backtrace may not be used until after + // set via assignment from a backtrace constructed with a + // valid snapshot. + cmListFileBacktrace(); - void PrintTitle(std::ostream& out) const; - void PrintCallStack(std::ostream& out) const; - private: - cmState::Snapshot Snapshot; + // Construct an empty backtrace whose bottom sits in the directory + // indicated by the given valid snapshot. + cmListFileBacktrace(cmState::Snapshot snapshot); + + // Backtraces may be copied and assigned as values. + cmListFileBacktrace(cmListFileBacktrace const& r); + cmListFileBacktrace& operator=(cmListFileBacktrace const& r); + ~cmListFileBacktrace(); + + // Get a backtrace with the given file scope added to the top. + // May not be called until after construction with a valid snapshot. + cmListFileBacktrace Push(std::string const& file) const; + + // Get a backtrace with the given call context added to the top. + // May not be called until after construction with a valid snapshot. + cmListFileBacktrace Push(cmListFileContext const& lfc) const; + + // Get a backtrace with the top level removed. + // May not be called until after a matching Push. + cmListFileBacktrace Pop() const; + + // Get the context at the top of the backtrace. + // Returns an empty context if the backtrace is empty. + cmListFileContext const& Top() const; + + // Print the top of the backtrace. + void PrintTitle(std::ostream& out) const; + + // Print the call stack below the top of the backtrace. + void PrintCallStack(std::ostream& out) const; +private: + struct Entry; + cmState::Snapshot Bottom; + Entry* Cur; + cmListFileBacktrace(cmState::Snapshot bottom, Entry* up, + cmListFileContext const& lfc); + cmListFileBacktrace(cmState::Snapshot bottom, Entry* cur); }; struct cmListFile diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 13bcdac4aa..3a56c2aac5 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -46,7 +46,8 @@ cmMakefile::cmMakefile(cmGlobalGenerator* globalGenerator, cmState::Snapshot const& snapshot) : GlobalGenerator(globalGenerator), - StateSnapshot(snapshot) + StateSnapshot(snapshot), + Backtrace(snapshot) { this->IsSourceFileTryCompile = false; @@ -115,24 +116,8 @@ void cmMakefile::IssueMessage(cmake::MessageType t, { this->ExecutionStatusStack.back()->SetNestedError(true); } - this->GetCMakeInstance()->IssueMessage(t, text, this->GetBacktrace(), - force); - } - else - { - cmListFileContext lfc; - // We are not currently executing a command. Add whatever context - // information we have. - lfc.FilePath = this->GetExecutionFilePath(); - - if(!this->GetCMakeInstance()->GetIsInTryCompile()) - { - cmOutputConverter converter(this->StateSnapshot); - lfc.FilePath = converter.Convert(lfc.FilePath, cmOutputConverter::HOME); - } - lfc.Line = 0; - this->GetCMakeInstance()->IssueMessage(t, text, lfc, force); } + this->GetCMakeInstance()->IssueMessage(t, text, this->GetBacktrace(), force); } cmStringRange cmMakefile::GetIncludeDirectoriesEntries() const @@ -170,29 +155,28 @@ cmBacktraceRange cmMakefile::GetCompileDefinitionsBacktraces() const //---------------------------------------------------------------------------- cmListFileBacktrace cmMakefile::GetBacktrace() const { - cmListFileBacktrace backtrace; - if (!this->ContextStack.empty()) - { - backtrace = cmListFileBacktrace(this->StateSnapshot, - *this->ContextStack.back()); - } - return backtrace; + return this->Backtrace; } //---------------------------------------------------------------------------- -cmListFileBacktrace -cmMakefile::GetBacktrace(cmCommandContext const& cc) const +cmListFileBacktrace cmMakefile::GetBacktrace(cmCommandContext const& cc) const { - cmState::Snapshot snp = this->StateSnapshot; - return cmListFileBacktrace(snp, cc); + cmListFileContext lfc; + lfc.Name = cc.Name; + lfc.Line = cc.Line; + lfc.FilePath = this->StateSnapshot.GetExecutionListFile(); + return this->Backtrace.Push(lfc); } //---------------------------------------------------------------------------- cmListFileContext cmMakefile::GetExecutionContext() const { - return cmListFileContext::FromCommandContext( - *this->ContextStack.back(), - this->StateSnapshot.GetExecutionListFile()); + cmListFileContext const& cur = this->Backtrace.Top(); + cmListFileContext lfc; + lfc.Name = cur.Name; + lfc.Line = cur.Line; + lfc.FilePath = this->StateSnapshot.GetExecutionListFile(); + return lfc; } //---------------------------------------------------------------------------- @@ -226,17 +210,20 @@ void cmMakefile::PrintCommandTrace(const cmListFileFunction& lff) const class cmMakefileCall { public: - cmMakefileCall(cmMakefile* mf, const cmCommandContext& lfc, + cmMakefileCall(cmMakefile* mf, cmCommandContext const& cc, cmExecutionStatus& status): Makefile(mf) { - this->Makefile->ContextStack.push_back(&lfc); + cmListFileContext const& lfc = + cmListFileContext::FromCommandContext( + cc, this->Makefile->StateSnapshot.GetExecutionListFile()); + this->Makefile->Backtrace = this->Makefile->Backtrace.Push(lfc); this->Makefile->ExecutionStatusStack.push_back(&status); } ~cmMakefileCall() { this->Makefile->ExecutionStatusStack.pop_back(); - this->Makefile->ContextStack.pop_back(); + this->Makefile->Backtrace = this->Makefile->Backtrace.Pop(); } private: cmMakefile* Makefile; @@ -350,13 +337,14 @@ cmMakefile::IncludeScope::IncludeScope(cmMakefile* mf, Makefile(mf), NoPolicyScope(noPolicyScope), CheckCMP0011(false), ReportError(true) { + this->Makefile->Backtrace = + this->Makefile->Backtrace.Push(filenametoread); + this->Makefile->PushFunctionBlockerBarrier(); this->Makefile->StateSnapshot = this->Makefile->GetState()->CreateIncludeFileSnapshot( this->Makefile->StateSnapshot, - this->Makefile->ContextStack.back()->Name, - this->Makefile->ContextStack.back()->Line, filenametoread); if(!this->NoPolicyScope) { @@ -416,6 +404,8 @@ cmMakefile::IncludeScope::~IncludeScope() this->Makefile->PopSnapshot(this->ReportError); this->Makefile->PopFunctionBlockerBarrier(this->ReportError); + + this->Makefile->Backtrace = this->Makefile->Backtrace.Pop(); } //---------------------------------------------------------------------------- @@ -458,25 +448,6 @@ void cmMakefile::IncludeScope::EnforceCMP0011() } } -class cmParseFileScope -{ -public: - cmParseFileScope(cmMakefile* mf) - : Makefile(mf) - { - this->Makefile->ContextStack.push_back(&this->Context); - } - - ~cmParseFileScope() - { - this->Makefile->ContextStack.pop_back(); - } - -private: - cmMakefile* Makefile; - cmCommandContext Context; -}; - bool cmMakefile::ReadDependentFile(const char* filename, bool noPolicyScope) { this->AddDefinition("CMAKE_PARENT_LIST_FILE", @@ -488,13 +459,10 @@ bool cmMakefile::ReadDependentFile(const char* filename, bool noPolicyScope) IncludeScope incScope(this, filenametoread, noPolicyScope); cmListFile listFile; - { - cmParseFileScope pfs(this); if (!listFile.ParseFile(filenametoread.c_str(), false, this)) { return false; } - } this->ReadListFile(listFile, filenametoread); if(cmSystemTools::GetFatalErrorOccured()) @@ -510,16 +478,12 @@ public: ListFileScope(cmMakefile* mf, std::string const& filenametoread) : Makefile(mf), ReportError(true) { - long line = 0; - std::string name; - if (!this->Makefile->ContextStack.empty()) - { - line = this->Makefile->ContextStack.back()->Line; - name = this->Makefile->ContextStack.back()->Name; - } + this->Makefile->Backtrace = + this->Makefile->Backtrace.Push(filenametoread); + this->Makefile->StateSnapshot = this->Makefile->GetState()->CreateInlineListFileSnapshot( - this->Makefile->StateSnapshot, name, line, filenametoread); + this->Makefile->StateSnapshot, filenametoread); assert(this->Makefile->StateSnapshot.IsValid()); this->Makefile->PushFunctionBlockerBarrier(); @@ -529,6 +493,7 @@ public: { this->Makefile->PopSnapshot(this->ReportError); this->Makefile->PopFunctionBlockerBarrier(this->ReportError); + this->Makefile->Backtrace = this->Makefile->Backtrace.Pop(); } void Quiet() { this->ReportError = false; } @@ -546,13 +511,10 @@ bool cmMakefile::ReadListFile(const char* filename) ListFileScope scope(this, filenametoread); cmListFile listFile; - { - cmParseFileScope pfs(this); if (!listFile.ParseFile(filenametoread.c_str(), false, this)) { return false; } - } this->ReadListFile(listFile, filenametoread); if(cmSystemTools::GetFatalErrorOccured()) @@ -1525,9 +1487,7 @@ void cmMakefile::PushFunctionScope(std::string const& fileName, { this->StateSnapshot = this->GetState()->CreateFunctionCallSnapshot( - this->StateSnapshot, - this->ContextStack.back()->Name, this->ContextStack.back()->Line, - fileName); + this->StateSnapshot, fileName); assert(this->StateSnapshot.IsValid()); this->PushLoopBlockBarrier(); @@ -1563,9 +1523,7 @@ void cmMakefile::PushMacroScope(std::string const& fileName, { this->StateSnapshot = this->GetState()->CreateMacroCallSnapshot( - this->StateSnapshot, - this->ContextStack.back()->Name, this->ContextStack.back()->Line, - fileName); + this->StateSnapshot, fileName); assert(this->StateSnapshot.IsValid()); this->PushFunctionBlockerBarrier(); @@ -1633,6 +1591,15 @@ private: //---------------------------------------------------------------------------- void cmMakefile::Configure() { + std::string currentStart = + this->StateSnapshot.GetDirectory().GetCurrentSource(); + currentStart += "/CMakeLists.txt"; + + // Add the bottom of all backtraces within this directory. + // We will never pop this scope because it should be available + // for messages during the generate step too. + this->Backtrace = this->Backtrace.Push(currentStart); + BuildsystemFileScope scope(this); // make sure the CMakeFiles dir is there @@ -1640,20 +1607,14 @@ void cmMakefile::Configure() filesDir += cmake::GetCMakeFilesDirectory(); cmSystemTools::MakeDirectory(filesDir.c_str()); - std::string currentStart = - this->StateSnapshot.GetDirectory().GetCurrentSource(); - currentStart += "/CMakeLists.txt"; assert(cmSystemTools::FileExists(currentStart.c_str(), true)); this->AddDefinition("CMAKE_PARENT_LIST_FILE", currentStart.c_str()); cmListFile listFile; - { - cmParseFileScope pfs(this); if (!listFile.ParseFile(currentStart.c_str(), this->IsRootMakefile(), this)) { return; } - } this->ReadListFile(listFile, currentStart); if(cmSystemTools::GetFatalErrorOccured()) { @@ -1740,9 +1701,7 @@ void cmMakefile::AddSubDirectory(const std::string& srcPath, } cmState::Snapshot newSnapshot = this->GetState() - ->CreateBuildsystemDirectorySnapshot(this->StateSnapshot, - this->ContextStack.back()->Name, - this->ContextStack.back()->Line); + ->CreateBuildsystemDirectorySnapshot(this->StateSnapshot); newSnapshot.GetDirectory().SetCurrentSource(srcPath); newSnapshot.GetDirectory().SetCurrentBinary(binPath); @@ -4128,17 +4087,8 @@ std::string cmMakefile::FormatListFileStack() const void cmMakefile::PushScope() { - std::string commandName; - long line = 0; - if (!this->ContextStack.empty()) - { - commandName = this->ContextStack.back()->Name; - line = this->ContextStack.back()->Line; - } this->StateSnapshot = this->GetState()->CreateVariableScopeSnapshot( - this->StateSnapshot, - commandName, - line); + this->StateSnapshot); this->PushLoopBlockBarrier(); #if defined(CMAKE_BUILD_WITH_CMAKE) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 72179442f6..089a83d128 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -835,6 +835,7 @@ private: cmMakefile& operator=(const cmMakefile& mf); cmState::Snapshot StateSnapshot; + cmListFileBacktrace Backtrace; void ReadListFile(cmListFile const& listFile, const std::string& filenametoread); @@ -862,7 +863,6 @@ private: std::vector EvaluationFiles; - std::vector ContextStack; std::vector ExecutionStatusStack; friend class cmMakefileCall; friend class cmParseFileScope; diff --git a/Source/cmState.cxx b/Source/cmState.cxx index bec56822df..f4ca113417 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -845,14 +845,10 @@ cmState::Snapshot cmState::CreateBaseSnapshot() } cmState::Snapshot -cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine) +cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot) { assert(originSnapshot.IsValid()); PositionType pos = this->SnapshotData.Push(originSnapshot.Position); - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->DirectoryParent = originSnapshot.Position; pos->ScopeParent = originSnapshot.Position; pos->SnapshotType = BuildsystemDirectoryType; @@ -886,15 +882,11 @@ cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, cmState::Snapshot cmState::CreateFunctionCallSnapshot(cmState::Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine, std::string const& fileName) { PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); pos->ScopeParent = originSnapshot.Position; - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = FunctionCallType; pos->Keep = false; pos->ExecutionListFile = this->ExecutionListFiles.Push( @@ -912,14 +904,10 @@ cmState::CreateFunctionCallSnapshot(cmState::Snapshot originSnapshot, cmState::Snapshot cmState::CreateMacroCallSnapshot(cmState::Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine, std::string const& fileName) { PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = MacroCallType; pos->Keep = false; pos->ExecutionListFile = this->ExecutionListFiles.Push( @@ -932,14 +920,10 @@ cmState::CreateMacroCallSnapshot(cmState::Snapshot originSnapshot, cmState::Snapshot cmState::CreateIncludeFileSnapshot(cmState::Snapshot originSnapshot, - const std::string& entryPointCommand, - long entryPointLine, const std::string& fileName) { PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = IncludeFileType; pos->Keep = true; pos->ExecutionListFile = this->ExecutionListFiles.Push( @@ -951,15 +935,11 @@ cmState::CreateIncludeFileSnapshot(cmState::Snapshot originSnapshot, } cmState::Snapshot -cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine) +cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot) { PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); pos->ScopeParent = originSnapshot.Position; - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = VariableScopeType; pos->Keep = false; pos->PolicyScope = originSnapshot.Position->Policies; @@ -975,14 +955,10 @@ cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, cmState::Snapshot cmState::CreateInlineListFileSnapshot(cmState::Snapshot originSnapshot, - const std::string& entryPointCommand, - long entryPointLine, const std::string& fileName) { PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); - pos->EntryPointLine = entryPointLine; - pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = InlineListFileType; pos->Keep = true; pos->ExecutionListFile = this->ExecutionListFiles.Push( diff --git a/Source/cmState.h b/Source/cmState.h index 507d500eaf..5ceaf24d01 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -192,27 +192,15 @@ public: Snapshot CreateBaseSnapshot(); Snapshot - CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine); + CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot); Snapshot CreateFunctionCallSnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine, std::string const& fileName); Snapshot CreateMacroCallSnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine, std::string const& fileName); Snapshot CreateIncludeFileSnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine, std::string const& fileName); - Snapshot CreateVariableScopeSnapshot(Snapshot originSnapshot, - std::string const& entryPointCommand, - long entryPointLine); + Snapshot CreateVariableScopeSnapshot(Snapshot originSnapshot); Snapshot CreateInlineListFileSnapshot(Snapshot originSnapshot, - const std::string& entryPointCommand, - long entryPointLine, std::string const& fileName); Snapshot CreatePolicyScopeSnapshot(Snapshot originSnapshot); Snapshot Pop(Snapshot originSnapshot); diff --git a/Tests/RunCMake/PolicyScope/NotClosed-stderr.txt b/Tests/RunCMake/PolicyScope/NotClosed-stderr.txt index 293d16117a..08c42a01dd 100644 --- a/Tests/RunCMake/PolicyScope/NotClosed-stderr.txt +++ b/Tests/RunCMake/PolicyScope/NotClosed-stderr.txt @@ -1,4 +1,4 @@ -^CMake Error at NotClosed.cmake:[0-9]+ \(include\): +^CMake Error in NotClosed.cmake: cmake_policy PUSH without matching POP Call Stack \(most recent call first\): CMakeLists.txt:[0-9]+ \(include\)$ diff --git a/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt b/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt index 776a8f260a..306c2555a7 100644 --- a/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt +++ b/Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt @@ -1,6 +1,8 @@ -^CMake Error at CMakeLists.txt:[0-9]+ \(include\): +^CMake Error in FunctionUnmatched.cmake: A logical block opening on the line .*/Tests/RunCMake/Syntax/FunctionUnmatched.cmake:[0-9]+ \(function\) - is not closed.$ + is not closed. +Call Stack \(most recent call first\): + CMakeLists.txt:[0-9]+ \(include\)$ diff --git a/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt b/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt index 1699c436d7..440d863940 100644 --- a/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt +++ b/Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt @@ -1,6 +1,8 @@ -^CMake Error at CMakeLists.txt:[0-9]+ \(include\): +^CMake Error in MacroUnmatched.cmake: A logical block opening on the line .*/Tests/RunCMake/Syntax/MacroUnmatched.cmake:[0-9]+ \(macro\) - is not closed.$ + is not closed. +Call Stack \(most recent call first\): + CMakeLists.txt:[0-9]+ \(include\)$ diff --git a/Tests/RunCMake/TargetSources/OriginDebugIDE-stderr.txt b/Tests/RunCMake/TargetSources/OriginDebugIDE-stderr.txt index fad7073c65..6fdcce73ea 100644 --- a/Tests/RunCMake/TargetSources/OriginDebugIDE-stderr.txt +++ b/Tests/RunCMake/TargetSources/OriginDebugIDE-stderr.txt @@ -25,7 +25,7 @@ Call Stack \(most recent call first\): OriginDebugIDE.cmake:4 \(include\) CMakeLists.txt:3 \(include\) + -CMake Debug Log: +CMake Debug Log in CMakeLists.txt: Used sources for target OriginDebug: * .*CMakeLists.txt diff --git a/Tests/RunCMake/find_package/PolicyPush-stderr.txt b/Tests/RunCMake/find_package/PolicyPush-stderr.txt index 1afcb16271..73cf2e6c82 100644 --- a/Tests/RunCMake/find_package/PolicyPush-stderr.txt +++ b/Tests/RunCMake/find_package/PolicyPush-stderr.txt @@ -1,4 +1,4 @@ -^CMake Error at PolicyPush/PolicyPushConfigVersion.cmake:1 \(find_package\): +^CMake Error in PolicyPush/PolicyPushConfigVersion.cmake: cmake_policy PUSH without matching POP Call Stack \(most recent call first\): PolicyPush.cmake:1 \(find_package\) diff --git a/Tests/RunCMake/while/EndMismatch-stderr.txt b/Tests/RunCMake/while/EndMismatch-stderr.txt index d7439e8fce..54d299f8fb 100644 --- a/Tests/RunCMake/while/EndMismatch-stderr.txt +++ b/Tests/RunCMake/while/EndMismatch-stderr.txt @@ -1,4 +1,4 @@ -^CMake Warning \(dev\) at EndMismatch.cmake:3 \(include\): +^CMake Warning \(dev\) in EndMismatch.cmake: A logical block opening on the line .*/Tests/RunCMake/while/EndMismatch.cmake:1 \(while\) diff --git a/Tests/RunCMake/while/EndMissing-stderr.txt b/Tests/RunCMake/while/EndMissing-stderr.txt index 099a8b2730..964792ff23 100644 --- a/Tests/RunCMake/while/EndMissing-stderr.txt +++ b/Tests/RunCMake/while/EndMissing-stderr.txt @@ -1,6 +1,8 @@ -^CMake Error at CMakeLists.txt:3 \(include\): +^CMake Error in EndMissing.cmake: A logical block opening on the line .*/Tests/RunCMake/while/EndMissing.cmake:1 \(while\) - is not closed.$ + is not closed. +Call Stack \(most recent call first\): + CMakeLists.txt:[0-9]+ \(include\)$ From 563bf9dd8a207e04697a92d8bcd239f52400b355 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 15 Apr 2016 11:05:33 -0400 Subject: [PATCH 4/5] cmState: Remove unused entry point fields from snapshot data This information is now kept in cmMakefile::Backtrace. --- Source/cmState.cxx | 12 ------------ Source/cmState.h | 2 -- 2 files changed, 14 deletions(-) diff --git a/Source/cmState.cxx b/Source/cmState.cxx index f4ca113417..bafb09e142 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -35,8 +35,6 @@ struct cmState::SnapshotDataType cmLinkedTree::iterator Vars; cmLinkedTree::iterator Root; cmLinkedTree::iterator Parent; - std::string EntryPointCommand; - long EntryPointLine; std::vector::size_type IncludeDirectoryPosition; std::vector::size_type CompileDefinitionsPosition; std::vector::size_type CompileOptionsPosition; @@ -1116,16 +1114,6 @@ std::string cmState::Snapshot::GetExecutionListFile() const return *this->Position->ExecutionListFile; } -std::string cmState::Snapshot::GetEntryPointCommand() const -{ - return this->Position->EntryPointCommand; -} - -long cmState::Snapshot::GetEntryPointLine() const -{ - return this->Position->EntryPointLine; -} - bool cmState::Snapshot::IsValid() const { return this->State && this->Position.IsValid() diff --git a/Source/cmState.h b/Source/cmState.h index 5ceaf24d01..7ead7e02e3 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -68,8 +68,6 @@ public: std::string GetExecutionListFile() const; std::vector GetChildren(); - std::string GetEntryPointCommand() const; - long GetEntryPointLine() const; bool IsValid() const; Snapshot GetBuildsystemDirectoryParent() const; From 0f96ef00cbe8dd82cee32f61f9da43e66e42f446 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 15 Apr 2016 10:32:54 -0400 Subject: [PATCH 5/5] Remove unused cmake::IssueMessage overload All callers now pass a full backtrace so we do not need the alternative that takes a cmListFileContext directly. Drop this overload to remove the code duplication. --- Source/cmake.cxx | 35 ----------------------------------- Source/cmake.h | 3 --- 2 files changed, 38 deletions(-) diff --git a/Source/cmake.cxx b/Source/cmake.cxx index dcc95aff24..8e3380f56b 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2784,41 +2784,6 @@ void cmake::IssueMessage(cmake::MessageType t, std::string const& text, displayMessage(t, msg); } -//---------------------------------------------------------------------------- -void cmake::IssueMessage(cmake::MessageType t, std::string const& text, - cmListFileContext const& lfc, - bool force) -{ - if (!force) - { - // override the message type, if needed, for warnings and errors - cmake::MessageType override = this->ConvertMessageType(t); - if (override != t) - { - t = override; - force = true; - } - } - - if (!force && !this->IsMessageTypeVisible(t)) - { - return; - } - - std::ostringstream msg; - if (!this->PrintMessagePreamble(t, msg)) - { - return; - } - - // Add the immediate context. - msg << (lfc.Line ? " at " : " in ") << lfc; - - printMessageText(msg, text); - - displayMessage(t, msg); -} - //---------------------------------------------------------------------------- std::vector cmake::GetDebugConfigs() { diff --git a/Source/cmake.h b/Source/cmake.h index 84967056a7..8644dda1c3 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -358,9 +358,6 @@ class cmake void IssueMessage(cmake::MessageType t, std::string const& text, cmListFileBacktrace const& backtrace = cmListFileBacktrace(), bool force = false); - void IssueMessage(cmake::MessageType t, std::string const& text, - cmListFileContext const& lfc, - bool force = false); ///! run the --build option int Build(const std::string& dir,