From f1ad71d7f8c066a7e0d0c11bb1ce9d5a5719ec5e Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 6 Sep 2016 15:25:26 -0400 Subject: [PATCH] cmMakefile: Restore nested error logic use of cmExecutionStatus Since commit 14a8d61f (cmMakefile: Port nested error logic away from cmExecutionStatus) we fail to continue processing function and macro bodies after non-fatal errors. A non-fatal error should not stop foreach loops, macro bodies, nested bodies, or the outer script. Add a test covering these cases, and revert the change to fix them. Also revert commit 2af853de (cmMakefile: Simplify IssueMessage implementation) because the assertion it added (which was removed by the above commit and is restored by reverting it) is incorrect. We do have code paths that call cmMakefile::IssueMessage with an empty execution stack, such as in CheckForUnusedVariables's LogUnused call. --- Source/cmExecutionStatus.h | 4 ++++ Source/cmFunctionCommand.cxx | 6 +++--- Source/cmMacroCommand.cxx | 4 ++-- Source/cmMakefile.cxx | 19 ++++++++----------- .../Configure/ContinueAfterError-result.txt | 1 + .../Configure/ContinueAfterError-stderr.txt | 13 +++++++++++++ .../Configure/ContinueAfterError-stdout.txt | 11 +++++++++++ .../Configure/ContinueAfterError.cmake | 19 +++++++++++++++++++ Tests/RunCMake/Configure/RunCMakeTest.cmake | 1 + 9 files changed, 62 insertions(+), 16 deletions(-) create mode 100644 Tests/RunCMake/Configure/ContinueAfterError-result.txt create mode 100644 Tests/RunCMake/Configure/ContinueAfterError-stderr.txt create mode 100644 Tests/RunCMake/Configure/ContinueAfterError-stdout.txt create mode 100644 Tests/RunCMake/Configure/ContinueAfterError.cmake diff --git a/Source/cmExecutionStatus.h b/Source/cmExecutionStatus.h index 14e14543b..730283769 100644 --- a/Source/cmExecutionStatus.h +++ b/Source/cmExecutionStatus.h @@ -40,12 +40,16 @@ public: this->ReturnInvoked = false; this->BreakInvoked = false; this->ContinueInvoked = false; + this->NestedError = false; } + void SetNestedError(bool val) { this->NestedError = val; } + bool GetNestedError() { return this->NestedError; } private: bool ReturnInvoked; bool BreakInvoked; bool ContinueInvoked; + bool NestedError; }; #endif diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx index f0e4854bf..40c54dbdc 100644 --- a/Source/cmFunctionCommand.cxx +++ b/Source/cmFunctionCommand.cxx @@ -76,7 +76,7 @@ public: }; bool cmFunctionHelperCommand::InvokeInitialPass( - const std::vector& args, cmExecutionStatus&) + const std::vector& args, cmExecutionStatus& inStatus) { // Expand the argument list to the function. std::vector expandedArgs; @@ -129,11 +129,11 @@ bool cmFunctionHelperCommand::InvokeInitialPass( for (unsigned int c = 0; c < this->Functions.size(); ++c) { cmExecutionStatus status; if (!this->Makefile->ExecuteCommand(this->Functions[c], status) || - (cmSystemTools::GetErrorOccuredFlag() && - !cmSystemTools::GetFatalErrorOccured())) { + status.GetNestedError()) { // The error message should have already included the call stack // so we do not need to report an error here. functionScope.Quiet(); + inStatus.SetNestedError(true); return false; } if (status.GetReturnInvoked()) { diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 9d312eebc..ee9dc8af7 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -159,11 +159,11 @@ bool cmMacroHelperCommand::InvokeInitialPass( } cmExecutionStatus status; if (!this->Makefile->ExecuteCommand(newLFF, status) || - (cmSystemTools::GetErrorOccuredFlag() && - !cmSystemTools::GetFatalErrorOccured())) { + status.GetNestedError()) { // The error message should have already included the call stack // so we do not need to report an error here. macroScope.Quiet(); + inStatus.SetNestedError(true); return false; } if (status.GetReturnInvoked()) { diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 11ccca122..e5a5e6e27 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -117,6 +117,11 @@ cmMakefile::~cmMakefile() void cmMakefile::IssueMessage(cmake::MessageType t, std::string const& text) const { + if (!this->ExecutionStatusStack.empty()) { + if ((t == cmake::FATAL_ERROR) || (t == cmake::INTERNAL_ERROR)) { + this->ExecutionStatusStack.back()->SetNestedError(true); + } + } this->GetCMakeInstance()->IssueMessage(t, text, this->GetBacktrace()); } @@ -277,19 +282,11 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff, if (this->GetCMakeInstance()->GetTrace()) { this->PrintCommandTrace(lff); } - - bool hadPreviousNonFatalError = cmSystemTools::GetErrorOccuredFlag() && - !cmSystemTools::GetFatalErrorOccured(); - cmSystemTools::ResetErrorOccuredFlag(); - + // Try invoking the command. bool invokeSucceeded = pcmd->InvokeInitialPass(lff.Arguments, status); - bool hadNestedError = cmSystemTools::GetErrorOccuredFlag() && - !cmSystemTools::GetFatalErrorOccured(); - if (hadPreviousNonFatalError) { - cmSystemTools::SetErrorOccured(); - } + bool hadNestedError = status.GetNestedError(); if (!invokeSucceeded || hadNestedError) { - if (!hadNestedError && !cmSystemTools::GetFatalErrorOccured()) { + if (!hadNestedError) { // The command invocation requested that we report an error. this->IssueMessage(cmake::FATAL_ERROR, pcmd->GetError()); } diff --git a/Tests/RunCMake/Configure/ContinueAfterError-result.txt b/Tests/RunCMake/Configure/ContinueAfterError-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/Configure/ContinueAfterError-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/Configure/ContinueAfterError-stderr.txt b/Tests/RunCMake/Configure/ContinueAfterError-stderr.txt new file mode 100644 index 000000000..f40a3ef65 --- /dev/null +++ b/Tests/RunCMake/Configure/ContinueAfterError-stderr.txt @@ -0,0 +1,13 @@ +^CMake Error at ContinueAfterError.cmake:[0-9]+ \(message\): + error in loop body +Call Stack \(most recent call first\): + ContinueAfterError.cmake:[0-9]+ \(m\) + ContinueAfterError.cmake:[0-9]+ \(f\) + CMakeLists.txt:[0-9]+ \(include\) ++ +CMake Error at ContinueAfterError.cmake:[0-9]+ \(message\): + error in loop body +Call Stack \(most recent call first\): + ContinueAfterError.cmake:[0-9]+ \(m\) + ContinueAfterError.cmake:[0-9]+ \(f\) + CMakeLists.txt:[0-9]+ \(include\)$ diff --git a/Tests/RunCMake/Configure/ContinueAfterError-stdout.txt b/Tests/RunCMake/Configure/ContinueAfterError-stdout.txt new file mode 100644 index 000000000..f03aa0741 --- /dev/null +++ b/Tests/RunCMake/Configure/ContinueAfterError-stdout.txt @@ -0,0 +1,11 @@ +-- before f +-- start f +-- start m +-- start loop body +-- end loop body +-- start loop body +-- end loop body +-- end m +-- end f +-- after f +-- Configuring incomplete, errors occurred! diff --git a/Tests/RunCMake/Configure/ContinueAfterError.cmake b/Tests/RunCMake/Configure/ContinueAfterError.cmake new file mode 100644 index 000000000..d094390b2 --- /dev/null +++ b/Tests/RunCMake/Configure/ContinueAfterError.cmake @@ -0,0 +1,19 @@ +macro(m) + message(STATUS " start m") + foreach(i 1 2) + message(STATUS " start loop body") + message(SEND_ERROR "error in loop body") + message(STATUS " end loop body") + endforeach() + message(STATUS " end m") +endmacro() + +function(f) + message(STATUS " start f") + m() + message(STATUS " end f") +endfunction() + +message(STATUS "before f") +f() +message(STATUS "after f") diff --git a/Tests/RunCMake/Configure/RunCMakeTest.cmake b/Tests/RunCMake/Configure/RunCMakeTest.cmake index 58e1a2a98..91adb4e78 100644 --- a/Tests/RunCMake/Configure/RunCMakeTest.cmake +++ b/Tests/RunCMake/Configure/RunCMakeTest.cmake @@ -1,5 +1,6 @@ include(RunCMake) +run_cmake(ContinueAfterError) run_cmake(CustomTargetAfterError) run_cmake(ErrorLogs) run_cmake(FailCopyFileABI)