Merge topic 'better-empty-list-behavior'

121a036f73 cmListCommand: handle empty lists for list(REMOVE_AT)
acfe53c588 cmListCommand: make list(ACTION not_a_list) succeed when idempotent
bf572ac952 cmListCommand: check list(FILTER) operation before the list

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2478

Help/release/dev/better-empty-list-behavior.rst

@@ -0,0 +1,9 @@
+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.

Source/cmListCommand.cxx

@@ -346,8 +346,7 @@ bool cmListCommand::HandleRemoveItemCommand(
   // expand the variable
   std::vector<std::string> varArgsExpanded;
   if (!this->GetList(varArgsExpanded, listName)) {
-    this->SetError("sub-command REMOVE_ITEM requires list to be present.");
-    return false;
+    return true;
   }
 
   std::vector<std::string> remove(args.begin() + 2, args.end());
@@ -376,8 +375,7 @@ bool cmListCommand::HandleReverseCommand(std::vector<std::string> const& args)
   // expand the variable
   std::vector<std::string> 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<std::string> varArgsExpanded;
   if (!this->GetList(varArgsExpanded, listName)) {
-    this->SetError(
-      "sub-command REMOVE_DUPLICATES requires list to be present.");
-    return false;
+    return true;
   }
 
   std::vector<std::string>::const_iterator argsEnd =
@@ -1152,8 +1148,7 @@ bool cmListCommand::HandleSortCommand(std::vector<std::string> const& args)
   // expand the variable
   std::vector<std::string> 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) &&
@@ -1230,13 +1225,17 @@ bool cmListCommand::HandleRemoveAtCommand(std::vector<std::string> const& args)
   const std::string& listName = args[1];
   // expand the variable
   std::vector<std::string> 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;
   }
 
@@ -1289,14 +1288,6 @@ bool cmListCommand::HandleFilterCommand(std::vector<std::string> const& args)
     return false;
   }
 
-  const std::string& listName = args[1];
-  // expand the variable
-  std::vector<std::string> 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 +1299,13 @@ bool cmListCommand::HandleFilterCommand(std::vector<std::string> const& args)
     return false;
   }
 
+  const std::string& listName = args[1];
+  // expand the variable
+  std::vector<std::string> varArgsExpanded;
+  if (!this->GetList(varArgsExpanded, listName)) {
+    return true;
+  }
+
   const std::string& mode = args[3];
   if (mode == "REGEX") {
     if (args.size() != 5) {

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\)$

Tests/RunCMake/list/FILTER-NotList-stderr.txt

@@ -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\)$

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 ()

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\)$

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 ()

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\)$

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 ()

Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-result.txt

@@ -1 +0,0 @@
-1

Tests/RunCMake/list/REMOVE_DUPLICATES-NotList-stderr.txt

@@ -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\)$

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 ()

Tests/RunCMake/list/REMOVE_ITEM-NotList-result.txt

@@ -1 +0,0 @@
-1

Tests/RunCMake/list/REMOVE_ITEM-NotList-stderr.txt

@@ -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\)$

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 ()

Tests/RunCMake/list/REVERSE-NotList-result.txt

@@ -1 +0,0 @@
-1

Tests/RunCMake/list/REVERSE-NotList-stderr.txt

@@ -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\)$

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 ()

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)

Tests/RunCMake/list/SORT-NotList-result.txt

@@ -1 +0,0 @@
-1

Tests/RunCMake/list/SORT-NotList-stderr.txt

@@ -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\)$

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 ()