cmFunctionBlocker: Move check for matching args
diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx
index 53ae479..10ce459 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -28,7 +28,9 @@
cm::string_view StartCommandName() const override { return "foreach"_s; }
cm::string_view EndCommandName() const override { return "endforeach"_s; }
- bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
+ bool ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const override;
+
bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& inStatus) override;
@@ -49,20 +51,12 @@
this->Makefile->PopLoopBlock();
}
-bool cmForEachFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
- cmMakefile& mf)
+bool cmForEachFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const
{
- if (lff.Name.Lower == "endforeach") {
- std::vector<std::string> expandedArguments;
- mf.ExpandArguments(lff.Arguments, expandedArguments);
- // if the endforeach has arguments then make sure
- // they match the begin foreach arguments
- if ((expandedArguments.empty() ||
- (expandedArguments[0] == this->Args[0]))) {
- return true;
- }
- }
- return false;
+ std::vector<std::string> expandedArguments;
+ mf.ExpandArguments(lff.Arguments, expandedArguments);
+ return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
}
bool cmForEachFunctionBlocker::Replay(
diff --git a/Source/cmFunctionBlocker.cxx b/Source/cmFunctionBlocker.cxx
index 2999691..437d4b5 100644
--- a/Source/cmFunctionBlocker.cxx
+++ b/Source/cmFunctionBlocker.cxx
@@ -2,8 +2,12 @@
file Copyright.txt or https://cmake.org/licensing for details. */
#include "cmFunctionBlocker.h"
+#include <cassert>
+#include <sstream>
+
#include "cmExecutionStatus.h"
#include "cmMakefile.h"
+#include "cmMessageType.h"
bool cmFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmExecutionStatus& status)
@@ -14,7 +18,24 @@
this->ScopeDepth--;
if (this->ScopeDepth == 0U) {
cmMakefile& mf = status.GetMakefile();
- auto self = mf.RemoveFunctionBlocker(this, lff);
+ auto self = mf.RemoveFunctionBlocker();
+ assert(self.get() == this);
+
+ if (!this->ArgumentsMatch(lff, mf)) {
+ cmListFileContext const& lfc = this->GetStartingContext();
+ cmListFileContext closingContext =
+ cmListFileContext::FromCommandContext(lff, lfc.FilePath);
+ std::ostringstream e;
+ /* clang-format off */
+ e << "A logical block opening on the line\n"
+ << " " << lfc << "\n"
+ << "closes on the line\n"
+ << " " << closingContext << "\n"
+ << "with mis-matching arguments.";
+ /* clang-format on */
+ mf.IssueMessage(MessageType::AUTHOR_WARNING, e.str());
+ }
+
return this->Replay(this->Functions, status);
}
}
diff --git a/Source/cmFunctionBlocker.h b/Source/cmFunctionBlocker.h
index 8fc2c1c..8b05993 100644
--- a/Source/cmFunctionBlocker.h
+++ b/Source/cmFunctionBlocker.h
@@ -23,15 +23,6 @@
bool IsFunctionBlocked(cmListFileFunction const& lff,
cmExecutionStatus& status);
- /**
- * should this function blocker be removed, useful when one function adds a
- * blocker and another must remove it
- */
- virtual bool ShouldRemove(const cmListFileFunction&, cmMakefile&)
- {
- return false;
- }
-
virtual ~cmFunctionBlocker() = default;
/** Set/Get the context in which this blocker is created. */
@@ -48,6 +39,9 @@
virtual cm::string_view StartCommandName() const = 0;
virtual cm::string_view EndCommandName() const = 0;
+ virtual bool ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const = 0;
+
virtual bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& status) = 0;
diff --git a/Source/cmFunctionCommand.cxx b/Source/cmFunctionCommand.cxx
index 6a04196..dd0a5d0 100644
--- a/Source/cmFunctionCommand.cxx
+++ b/Source/cmFunctionCommand.cxx
@@ -113,29 +113,22 @@
cm::string_view StartCommandName() const override { return "function"_s; }
cm::string_view EndCommandName() const override { return "endfunction"_s; }
- bool ShouldRemove(const cmListFileFunction&, cmMakefile& mf) override;
+ bool ArgumentsMatch(cmListFileFunction const&,
+ cmMakefile& mf) const override;
+
bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& status) override;
std::vector<std::string> Args;
};
-bool cmFunctionFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
- cmMakefile& mf)
+bool cmFunctionFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const
{
- if (lff.Name.Lower == "endfunction") {
- std::vector<std::string> expandedArguments;
- mf.ExpandArguments(lff.Arguments, expandedArguments,
- this->GetStartingContext().FilePath.c_str());
- // if the endfunction has arguments then make sure
- // they match the ones in the opening function command
- if ((expandedArguments.empty() ||
- (expandedArguments[0] == this->Args[0]))) {
- return true;
- }
- }
-
- return false;
+ std::vector<std::string> expandedArguments;
+ mf.ExpandArguments(lff.Arguments, expandedArguments,
+ this->GetStartingContext().FilePath.c_str());
+ return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
}
bool cmFunctionFunctionBlocker::Replay(
diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx
index 63cdd20..418a74b 100644
--- a/Source/cmIfCommand.cxx
+++ b/Source/cmIfCommand.cxx
@@ -38,7 +38,9 @@
cm::string_view StartCommandName() const override { return "if"_s; }
cm::string_view EndCommandName() const override { return "endif"_s; }
- bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
+ bool ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile&) const override;
+
bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& inStatus) override;
@@ -48,18 +50,10 @@
bool ElseSeen = false;
};
-bool cmIfFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
- cmMakefile&)
+bool cmIfFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile&) const
{
- if (lff.Name.Lower == "endif") {
- // if the endif has arguments, then make sure
- // they match the arguments of the matching if
- if (lff.Arguments.empty() || lff.Arguments == this->Args) {
- return true;
- }
- }
-
- return false;
+ return lff.Arguments.empty() || lff.Arguments == this->Args;
}
bool cmIfFunctionBlocker::Replay(
diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx
index 030bb66..eb328cc 100644
--- a/Source/cmMacroCommand.cxx
+++ b/Source/cmMacroCommand.cxx
@@ -146,29 +146,22 @@
cm::string_view StartCommandName() const override { return "macro"_s; }
cm::string_view EndCommandName() const override { return "endmacro"_s; }
- bool ShouldRemove(const cmListFileFunction&, cmMakefile& mf) override;
+ bool ArgumentsMatch(cmListFileFunction const&,
+ cmMakefile& mf) const override;
+
bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& status) override;
std::vector<std::string> Args;
};
-bool cmMacroFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
- cmMakefile& mf)
+bool cmMacroFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const
{
- if (lff.Name.Lower == "endmacro") {
- std::vector<std::string> expandedArguments;
- mf.ExpandArguments(lff.Arguments, expandedArguments,
- this->GetStartingContext().FilePath.c_str());
- // if the endmacro has arguments make sure they
- // match the arguments of the macro
- if ((expandedArguments.empty() ||
- (expandedArguments[0] == this->Args[0]))) {
- return true;
- }
- }
-
- return false;
+ std::vector<std::string> expandedArguments;
+ mf.ExpandArguments(lff.Arguments, expandedArguments,
+ this->GetStartingContext().FilePath.c_str());
+ return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
}
bool cmMacroFunctionBlocker::Replay(
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 8c11786..015453a 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -3220,30 +3220,12 @@
this->FunctionBlockers.push(std::move(fb));
}
-std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker(
- cmFunctionBlocker* fb, const cmListFileFunction& lff)
+std::unique_ptr<cmFunctionBlocker> cmMakefile::RemoveFunctionBlocker()
{
assert(!this->FunctionBlockers.empty());
- assert(this->FunctionBlockers.top().get() == fb);
assert(this->FunctionBlockerBarriers.empty() ||
this->FunctionBlockers.size() > this->FunctionBlockerBarriers.back());
- // Warn if the arguments do not match, but always remove.
- if (!fb->ShouldRemove(lff, *this)) {
- cmListFileContext const& lfc = fb->GetStartingContext();
- cmListFileContext closingContext =
- cmListFileContext::FromCommandContext(lff, lfc.FilePath);
- std::ostringstream e;
- /* clang-format off */
- e << "A logical block opening on the line\n"
- << " " << lfc << "\n"
- << "closes on the line\n"
- << " " << closingContext << "\n"
- << "with mis-matching arguments.";
- /* clang-format on */
- this->IssueMessage(MessageType::AUTHOR_WARNING, e.str());
- }
-
auto b = std::move(this->FunctionBlockers.top());
this->FunctionBlockers.pop();
return b;
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 941e335..4d61c05 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -107,8 +107,7 @@
* Remove the function blocker whose scope ends with the given command.
* This returns ownership of the function blocker object.
*/
- std::unique_ptr<cmFunctionBlocker> RemoveFunctionBlocker(
- cmFunctionBlocker* fb, const cmListFileFunction& lff);
+ std::unique_ptr<cmFunctionBlocker> RemoveFunctionBlocker();
/**
* Try running cmake and building a file. This is used for dynalically
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index bacee58..1e442e5 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -27,7 +27,9 @@
cm::string_view StartCommandName() const override { return "while"_s; }
cm::string_view EndCommandName() const override { return "endwhile"_s; }
- bool ShouldRemove(const cmListFileFunction& lff, cmMakefile& mf) override;
+ bool ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile& mf) const override;
+
bool Replay(std::vector<cmListFileFunction> const& functions,
cmExecutionStatus& inStatus) override;
@@ -48,17 +50,10 @@
this->Makefile->PopLoopBlock();
}
-bool cmWhileFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
- cmMakefile&)
+bool cmWhileFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
+ cmMakefile&) const
{
- if (lff.Name.Lower == "endwhile") {
- // if the endwhile has arguments, then make sure
- // they match the arguments of the matching while
- if (lff.Arguments.empty() || lff.Arguments == this->Args) {
- return true;
- }
- }
- return false;
+ return lff.Arguments.empty() || lff.Arguments == this->Args;
}
bool cmWhileFunctionBlocker::Replay(
