From bf572ac952d7ddf2b7208efc56f104844aea72e2 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 11 Oct 2018 17:22:45 -0400 Subject: cmListCommand: check list(FILTER) operation before the list A future commit will make the not-a-list case a success, but invalid operations should still be diagnosed in that case. --- Source/cmListCommand.cxx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/cmListCommand.cxx b/Source/cmListCommand.cxx index d7de2fa..b9c7ada 100644 --- a/Source/cmListCommand.cxx +++ b/Source/cmListCommand.cxx @@ -1289,14 +1289,6 @@ bool cmListCommand::HandleFilterCommand(std::vector const& args) return false; } - const std::string& listName = args[1]; - // expand the variable - std::vector varArgsExpanded; - if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command FILTER requires list to be present."); - return false; - } - const std::string& op = args[2]; bool includeMatches; if (op == "INCLUDE") { @@ -1308,6 +1300,14 @@ bool cmListCommand::HandleFilterCommand(std::vector const& args) return false; } + const std::string& listName = args[1]; + // expand the variable + std::vector varArgsExpanded; + if (!this->GetList(varArgsExpanded, listName)) { + this->SetError("sub-command FILTER requires list to be present."); + return false; + } + const std::string& mode = args[3]; if (mode == "REGEX") { if (args.size() != 5) { -- cgit v0.12 From acfe53c58817c662b935fbe0f0443de298371731 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 11 Oct 2018 17:23:12 -0400 Subject: cmListCommand: make list(ACTION not_a_list) succeed when idempotent The operations changed here all are no-ops on empty lists anyways, so just have them succeed when given non-extant lists. - `list(REMOVE_ITEM)` - `list(REMOVE_DUPLICATES)` - `list(SORT)` - `list(FILTER)` - `list(REVERSE)` --- Help/release/dev/better-empty-list-behavior.rst | 6 ++++++ Source/cmListCommand.cxx | 16 +++++----------- Tests/RunCMake/list/FILTER-NotList-result.txt | 1 - Tests/RunCMake/list/FILTER-NotList-stderr.txt | 4 ---- Tests/RunCMake/list/FILTER-NotList.cmake | 4 ++++ Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt | 1 - Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt | 4 ---- Tests/RunCMake/list/REMOVE_DUPLICATES-NotList.cmake | 4 ++++ Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt | 1 - Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt | 4 ---- Tests/RunCMake/list/REMOVE_ITEM-NotList.cmake | 4 ++++ Tests/RunCMake/list/REVERSE-NotList-result.txt | 1 - Tests/RunCMake/list/REVERSE-NotList-stderr.txt | 4 ---- Tests/RunCMake/list/REVERSE-NotList.cmake | 4 ++++ Tests/RunCMake/list/SORT-NotList-result.txt | 1 - Tests/RunCMake/list/SORT-NotList-stderr.txt | 4 ---- Tests/RunCMake/list/SORT-NotList.cmake | 4 ++++ 17 files changed, 31 insertions(+), 36 deletions(-) create mode 100644 Help/release/dev/better-empty-list-behavior.rst delete mode 100644 Tests/RunCMake/list/FILTER-NotList-result.txt delete mode 100644 Tests/RunCMake/list/FILTER-NotList-stderr.txt delete mode 100644 Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt delete mode 100644 Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt delete mode 100644 Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt delete mode 100644 Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt delete mode 100644 Tests/RunCMake/list/REVERSE-NotList-result.txt delete mode 100644 Tests/RunCMake/list/REVERSE-NotList-stderr.txt delete mode 100644 Tests/RunCMake/list/SORT-NotList-result.txt delete mode 100644 Tests/RunCMake/list/SORT-NotList-stderr.txt diff --git a/Help/release/dev/better-empty-list-behavior.rst b/Help/release/dev/better-empty-list-behavior.rst new file mode 100644 index 0000000..12a131f --- /dev/null +++ b/Help/release/dev/better-empty-list-behavior.rst @@ -0,0 +1,6 @@ +better-empty-list-behavior +-------------------------- + +* The :command:`list` operations ``REMOVE_ITEM``, ``REMOVE_DUPLICATES``, + ``SORT``, ``REVERSE``, and ``FILTER`` all now accept a non-existent variable + as the list since these operations on empty lists is also the empty list. diff --git a/Source/cmListCommand.cxx b/Source/cmListCommand.cxx index b9c7ada..b46eb6d 100644 --- a/Source/cmListCommand.cxx +++ b/Source/cmListCommand.cxx @@ -346,8 +346,7 @@ bool cmListCommand::HandleRemoveItemCommand( // expand the variable std::vector varArgsExpanded; if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command REMOVE_ITEM requires list to be present."); - return false; + return true; } std::vector remove(args.begin() + 2, args.end()); @@ -376,8 +375,7 @@ bool cmListCommand::HandleReverseCommand(std::vector const& args) // expand the variable std::vector varArgsExpanded; if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command REVERSE requires list to be present."); - return false; + return true; } std::string value = cmJoin(cmReverseRange(varArgsExpanded), ";"); @@ -399,9 +397,7 @@ bool cmListCommand::HandleRemoveDuplicatesCommand( // expand the variable std::vector varArgsExpanded; if (!this->GetList(varArgsExpanded, listName)) { - this->SetError( - "sub-command REMOVE_DUPLICATES requires list to be present."); - return false; + return true; } std::vector::const_iterator argsEnd = @@ -1152,8 +1148,7 @@ bool cmListCommand::HandleSortCommand(std::vector const& args) // expand the variable std::vector varArgsExpanded; if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command SORT requires list to be present."); - return false; + return true; } if ((sortCompare == cmStringSorter::Compare::STRING) && @@ -1304,8 +1299,7 @@ bool cmListCommand::HandleFilterCommand(std::vector const& args) // expand the variable std::vector varArgsExpanded; if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command FILTER requires list to be present."); - return false; + return true; } const std::string& mode = args[3]; diff --git a/Tests/RunCMake/list/FILTER-NotList-result.txt b/Tests/RunCMake/list/FILTER-NotList-result.txt deleted file mode 100644 index d00491f..0000000 --- a/Tests/RunCMake/list/FILTER-NotList-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/list/FILTER-NotList-stderr.txt b/Tests/RunCMake/list/FILTER-NotList-stderr.txt deleted file mode 100644 index 159c28d..0000000 --- a/Tests/RunCMake/list/FILTER-NotList-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -^CMake Error at FILTER-NotList.cmake:2 \(list\): - list sub-command FILTER requires list to be present. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/FILTER-NotList.cmake b/Tests/RunCMake/list/FILTER-NotList.cmake index 1e15635..bf09ec7 100644 --- a/Tests/RunCMake/list/FILTER-NotList.cmake +++ b/Tests/RunCMake/list/FILTER-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(FILTER nosuchlist EXCLUDE REGEX "^FILTER_THIS_.+") +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(FILTER) created our list") +endif () diff --git a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt b/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt deleted file mode 100644 index d00491f..0000000 --- a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt b/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt deleted file mode 100644 index 96f3446..0000000 --- a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -^CMake Error at REMOVE_DUPLICATES-NotList.cmake:2 \(list\): - list sub-command REMOVE_DUPLICATES requires list to be present. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList.cmake b/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList.cmake index 218f227..b9f3999 100644 --- a/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList.cmake +++ b/Tests/RunCMake/list/REMOVE_DUPLICATES-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(REMOVE_DUPLICATES nosuchlist) +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(REMOVE_DUPLICATES) created our list") +endif () diff --git a/Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt b/Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt deleted file mode 100644 index d00491f..0000000 --- a/Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt b/Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt deleted file mode 100644 index c32a4c0..0000000 --- a/Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -^CMake Error at REMOVE_ITEM-NotList.cmake:2 \(list\): - list sub-command REMOVE_ITEM requires list to be present. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REMOVE_ITEM-NotList.cmake b/Tests/RunCMake/list/REMOVE_ITEM-NotList.cmake index 079e7fb..0c66837 100644 --- a/Tests/RunCMake/list/REMOVE_ITEM-NotList.cmake +++ b/Tests/RunCMake/list/REMOVE_ITEM-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(REMOVE_ITEM nosuchlist alpha) +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(REMOVE_ITEM) created our list") +endif () diff --git a/Tests/RunCMake/list/REVERSE-NotList-result.txt b/Tests/RunCMake/list/REVERSE-NotList-result.txt deleted file mode 100644 index d00491f..0000000 --- a/Tests/RunCMake/list/REVERSE-NotList-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/list/REVERSE-NotList-stderr.txt b/Tests/RunCMake/list/REVERSE-NotList-stderr.txt deleted file mode 100644 index e9dcc06..0000000 --- a/Tests/RunCMake/list/REVERSE-NotList-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -^CMake Error at REVERSE-NotList.cmake:2 \(list\): - list sub-command REVERSE requires list to be present. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REVERSE-NotList.cmake b/Tests/RunCMake/list/REVERSE-NotList.cmake index 977e2cc..7138329 100644 --- a/Tests/RunCMake/list/REVERSE-NotList.cmake +++ b/Tests/RunCMake/list/REVERSE-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(REVERSE nosuchlist) +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(REVERSE) created our list") +endif () diff --git a/Tests/RunCMake/list/SORT-NotList-result.txt b/Tests/RunCMake/list/SORT-NotList-result.txt deleted file mode 100644 index d00491f..0000000 --- a/Tests/RunCMake/list/SORT-NotList-result.txt +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/Tests/RunCMake/list/SORT-NotList-stderr.txt b/Tests/RunCMake/list/SORT-NotList-stderr.txt deleted file mode 100644 index 396c5b5..0000000 --- a/Tests/RunCMake/list/SORT-NotList-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -^CMake Error at SORT-NotList.cmake:2 \(list\): - list sub-command SORT requires list to be present. -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/SORT-NotList.cmake b/Tests/RunCMake/list/SORT-NotList.cmake index 8f48e10..6314f14 100644 --- a/Tests/RunCMake/list/SORT-NotList.cmake +++ b/Tests/RunCMake/list/SORT-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(SORT nosuchlist) +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(SORT) created our list") +endif () -- cgit v0.12 From 121a036f73665a18ccadeaf50b00cc623d8ed9df Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 11 Oct 2018 17:26:44 -0400 Subject: cmListCommand: handle empty lists for list(REMOVE_AT) Treat an empty list as a list with no valid bounds and return an error message indicating that any given indices are out-of-bounds. --- Help/release/dev/better-empty-list-behavior.rst | 3 +++ Source/cmListCommand.cxx | 18 +++++++++++------- Tests/RunCMake/list/EmptyRemoveAt0-stderr.txt | 2 +- Tests/RunCMake/list/REMOVE_AT-EmptyList-result.txt | 1 + Tests/RunCMake/list/REMOVE_AT-EmptyList-stderr.txt | 4 ++++ Tests/RunCMake/list/REMOVE_AT-EmptyList.cmake | 6 ++++++ Tests/RunCMake/list/REMOVE_AT-NotList-stderr.txt | 2 +- Tests/RunCMake/list/REMOVE_AT-NotList.cmake | 4 ++++ Tests/RunCMake/list/RunCMakeTest.cmake | 2 ++ 9 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 Tests/RunCMake/list/REMOVE_AT-EmptyList-result.txt create mode 100644 Tests/RunCMake/list/REMOVE_AT-EmptyList-stderr.txt create mode 100644 Tests/RunCMake/list/REMOVE_AT-EmptyList.cmake diff --git a/Help/release/dev/better-empty-list-behavior.rst b/Help/release/dev/better-empty-list-behavior.rst index 12a131f..cd864f4 100644 --- a/Help/release/dev/better-empty-list-behavior.rst +++ b/Help/release/dev/better-empty-list-behavior.rst @@ -4,3 +4,6 @@ better-empty-list-behavior * The :command:`list` operations ``REMOVE_ITEM``, ``REMOVE_DUPLICATES``, ``SORT``, ``REVERSE``, and ``FILTER`` all now accept a non-existent variable as the list since these operations on empty lists is also the empty list. + +* The :command:`list` operation ``REMOVE_AT`` now indicates that the given + indices are invalid for a non-existent variable or empty list. diff --git a/Source/cmListCommand.cxx b/Source/cmListCommand.cxx index b46eb6d..b2acb90 100644 --- a/Source/cmListCommand.cxx +++ b/Source/cmListCommand.cxx @@ -1225,13 +1225,17 @@ bool cmListCommand::HandleRemoveAtCommand(std::vector const& args) const std::string& listName = args[1]; // expand the variable std::vector varArgsExpanded; - if (!this->GetList(varArgsExpanded, listName)) { - this->SetError("sub-command REMOVE_AT requires list to be present."); - return false; - } - // FIXME: Add policy to make non-existing lists an error like empty lists. - if (varArgsExpanded.empty()) { - this->SetError("REMOVE_AT given empty list"); + if (!this->GetList(varArgsExpanded, listName) || varArgsExpanded.empty()) { + std::ostringstream str; + str << "index: "; + for (size_t i = 1; i < args.size(); ++i) { + str << args[i]; + if (i != args.size() - 1) { + str << ", "; + } + } + str << " out of range (0, 0)"; + this->SetError(str.str()); return false; } diff --git a/Tests/RunCMake/list/EmptyRemoveAt0-stderr.txt b/Tests/RunCMake/list/EmptyRemoveAt0-stderr.txt index b24a0ed..9368e88 100644 --- a/Tests/RunCMake/list/EmptyRemoveAt0-stderr.txt +++ b/Tests/RunCMake/list/EmptyRemoveAt0-stderr.txt @@ -1,4 +1,4 @@ CMake Error at EmptyRemoveAt0.cmake:2 \(list\): - list REMOVE_AT given empty list + list index: mylist, 0 out of range \(0, 0\) Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REMOVE_AT-EmptyList-result.txt b/Tests/RunCMake/list/REMOVE_AT-EmptyList-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/list/REMOVE_AT-EmptyList-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/list/REMOVE_AT-EmptyList-stderr.txt b/Tests/RunCMake/list/REMOVE_AT-EmptyList-stderr.txt new file mode 100644 index 0000000..582b74b --- /dev/null +++ b/Tests/RunCMake/list/REMOVE_AT-EmptyList-stderr.txt @@ -0,0 +1,4 @@ +^CMake Error at REMOVE_AT-EmptyList.cmake:2 \(list\): + list index: nosuchlist, 0 out of range \(0, 0\) +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REMOVE_AT-EmptyList.cmake b/Tests/RunCMake/list/REMOVE_AT-EmptyList.cmake new file mode 100644 index 0000000..ff0dde8 --- /dev/null +++ b/Tests/RunCMake/list/REMOVE_AT-EmptyList.cmake @@ -0,0 +1,6 @@ +set(nosuchlist "") +list(REMOVE_AT nosuchlist 0) +if (NOT DEFINED nosuchlist OR NOT nosuchlist STREQUAL "") + message(FATAL_ERROR + "list(REMOVE_AT) modified our list") +endif () diff --git a/Tests/RunCMake/list/REMOVE_AT-NotList-stderr.txt b/Tests/RunCMake/list/REMOVE_AT-NotList-stderr.txt index d6e8d85..563d865 100644 --- a/Tests/RunCMake/list/REMOVE_AT-NotList-stderr.txt +++ b/Tests/RunCMake/list/REMOVE_AT-NotList-stderr.txt @@ -1,4 +1,4 @@ ^CMake Error at REMOVE_AT-NotList.cmake:2 \(list\): - list sub-command REMOVE_AT requires list to be present. + list index: nosuchlist, 0 out of range \(0, 0\) Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\)$ diff --git a/Tests/RunCMake/list/REMOVE_AT-NotList.cmake b/Tests/RunCMake/list/REMOVE_AT-NotList.cmake index 5266c7f..090df49 100644 --- a/Tests/RunCMake/list/REMOVE_AT-NotList.cmake +++ b/Tests/RunCMake/list/REMOVE_AT-NotList.cmake @@ -1,2 +1,6 @@ unset(nosuchlist) list(REMOVE_AT nosuchlist 0) +if (DEFINED nosuchlist) + message(FATAL_ERROR + "list(REMOVE_AT) created our list") +endif () diff --git a/Tests/RunCMake/list/RunCMakeTest.cmake b/Tests/RunCMake/list/RunCMakeTest.cmake index a8a0b57..bf3d22d 100644 --- a/Tests/RunCMake/list/RunCMakeTest.cmake +++ b/Tests/RunCMake/list/RunCMakeTest.cmake @@ -22,6 +22,8 @@ run_cmake(REMOVE_DUPLICATES-TooManyArguments) run_cmake(REVERSE-TooManyArguments) run_cmake(SUBLIST-TooManyArguments) +run_cmake(REMOVE_AT-EmptyList) + run_cmake(FILTER-NotList) run_cmake(REMOVE_AT-NotList) run_cmake(REMOVE_DUPLICATES-NotList) -- cgit v0.12