[llvm] 4e2216e - [libc++][test] `ADDITIONAL_COMPILE_FLAGS` should be a space-separated list (#73541)

via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 30 13:55:00 PST 2023


Author: Stephan T. Lavavej
Date: 2023-11-30T13:54:52-08:00
New Revision: 4e2216e184a2ba4b9e7e40b3cfa11e0c516a6ed6

URL: https://github.com/llvm/llvm-project/commit/4e2216e184a2ba4b9e7e40b3cfa11e0c516a6ed6
DIFF: https://github.com/llvm/llvm-project/commit/4e2216e184a2ba4b9e7e40b3cfa11e0c516a6ed6.diff

LOG: [libc++][test] `ADDITIONAL_COMPILE_FLAGS` should be a space-separated list (#73541)

Found while running libc++'s test suite with MSVC's STL.

`ADDITIONAL_COMPILE_FLAGS` is a `ParserKind.LIST`:

https://github.com/llvm/llvm-project/blob/3c23ed156f0151923b168bdff0c34ec73fb37f38/libcxx/utils/libcxx/test/format.py#L104-L108

With a comma-separated example:

https://github.com/llvm/llvm-project/blob/3c23ed156f0151923b168bdff0c34ec73fb37f38/libcxx/utils/libcxx/test/format.py#L223-L228

And comma-separated test coverage:

https://github.com/llvm/llvm-project/blob/dd3184c30ff531b8aecea280e65233337dd02815/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp#L12-L15

Because the machinery splits on commas:

https://github.com/llvm/llvm-project/blob/dd09221a29506031415cad8a1308998358633d48/llvm/utils/lit/lit/TestRunner.py#L1882-L1883

https://github.com/llvm/llvm-project/blob/dd09221a29506031415cad8a1308998358633d48/llvm/utils/lit/lit/TestRunner.py#L1951-L1956

However, most (although not all) usage of `ADDITIONAL_COMPILE_FLAGS` is
treating it as space-separated. That apparently works in the normal
Clang environment, but in my exotic configuration it causes `"-DMEOW
-DWOOF"` to be passed as a single argument to MSVC, which then emits
"warning C5102: ignoring invalid command-line macro definition
`'_LIBCPP_DISABLE_DEPRECATION_WARNINGS
-D_LIBCPP_ENABLE_CXX26_REMOVED_CODECVT'`", causing test failures due to
warnings-as-errors.

This PR changes `ADDITIONAL_COMPILE_FLAGS` to actually be parsed as a
space-separated list, and changes the few uses/examples that had commas.

Added: 
    

Modified: 
    libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
    libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp
    libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp
    libcxx/utils/libcxx/test/format.py
    llvm/utils/lit/lit/TestRunner.py
    llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt
    llvm/utils/lit/tests/unit/TestRunner.py

Removed: 
    


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp b/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
index 499d8da97cea960..561d77c569473c3 100644
--- a/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
+++ b/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
@@ -13,6 +13,6 @@
 // Make sure that substitutions are performed inside additional compiler flags.
 
 // ADDITIONAL_COMPILE_FLAGS: -I %t.1
-// ADDITIONAL_COMPILE_FLAGS: -isystem %t.2 , -isysroot %t.3
+// ADDITIONAL_COMPILE_FLAGS: -isystem %t.2 -isysroot %t.3
 // RUN: echo "-I %t.1 -isystem %t.2 -isysroot %t.3" | sed "s/\\\/\\\\\\\/g" > %t.escaped.grep
 // RUN: echo "%{compile_flags}" | grep -e -f %t.escaped.grep

diff  --git a/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp b/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp
index 387b1a1c3440cfa..c8ddddcef0a7f27 100644
--- a/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp
+++ b/libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-run.sh.cpp
@@ -11,5 +11,5 @@
 
 // ADDITIONAL_COMPILE_FLAGS: -foo
 // ADDITIONAL_COMPILE_FLAGS: -bar
-// ADDITIONAL_COMPILE_FLAGS: -baz, -foom
+// ADDITIONAL_COMPILE_FLAGS: -baz -foom
 // RUN: echo "%{compile_flags}" | grep -e '-foo -bar -baz -foom'

diff  --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp
index d9907779466f154..df8b96447262f0f 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_fsizeddeallocation.pass.cpp
@@ -15,7 +15,7 @@
 // XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx10.{{9|10|11}}
 
 // REQUIRES: -fsized-deallocation
-// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation, -O3
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation -O3
 
 #if !defined(__cpp_sized_deallocation)
 # error __cpp_sized_deallocation should be defined

diff  --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 5eb17d417489c62..e58e404bfcd2a5f 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -99,7 +99,7 @@ def parseScript(test, preamble):
         ),
         lit.TestRunner.IntegratedTestKeywordParser(
             "ADDITIONAL_COMPILE_FLAGS:",
-            lit.TestRunner.ParserKind.LIST,
+            lit.TestRunner.ParserKind.SPACE_LIST,
             initial_value=additionalCompileFlags,
         ),
     ]
@@ -110,7 +110,7 @@ def parseScript(test, preamble):
     for feature in test.config.available_features:
         parser = lit.TestRunner.IntegratedTestKeywordParser(
             "ADDITIONAL_COMPILE_FLAGS({}):".format(feature),
-            lit.TestRunner.ParserKind.LIST,
+            lit.TestRunner.ParserKind.SPACE_LIST,
             initial_value=additionalCompileFlags,
         )
         parsers.append(parser)
@@ -216,7 +216,7 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
             all the inputs necessary to run the test, such that e.g. execution
             on a remote host can be done by simply copying %T to the host.
 
-        // ADDITIONAL_COMPILE_FLAGS: flag1, flag2, flag3
+        // ADDITIONAL_COMPILE_FLAGS: flag1 flag2 flag3
 
             This directive will cause the provided flags to be added to the
             %{compile_flags} substitution for the test that contains it. This

diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 788152846efe2fa..da7fa86fd391733 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1352,7 +1352,7 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
 
     substitutions.append(("%{pathsep}", os.pathsep))
     substitutions.append(("%basename_t", baseName))
-    
+
     substitutions.extend(
         [
             ("%{fs-src-root}", pathlib.Path(sourcedir).anchor),
@@ -1795,6 +1795,7 @@ class ParserKind(object):
     TAG: A keyword taking no value. Ex 'END.'
     COMMAND: A keyword taking a list of shell commands. Ex 'RUN:'
     LIST: A keyword taking a comma-separated list of values.
+    SPACE_LIST: A keyword taking a space-separated list of values.
     BOOLEAN_EXPR: A keyword taking a comma-separated list of
         boolean expressions. Ex 'XFAIL:'
     INTEGER: A keyword taking a single integer. Ex 'ALLOW_RETRIES:'
@@ -1808,11 +1809,12 @@ class ParserKind(object):
     TAG = 0
     COMMAND = 1
     LIST = 2
-    BOOLEAN_EXPR = 3
-    INTEGER = 4
-    CUSTOM = 5
-    DEFINE = 6
-    REDEFINE = 7
+    SPACE_LIST = 3
+    BOOLEAN_EXPR = 4
+    INTEGER = 5
+    CUSTOM = 6
+    DEFINE = 7
+    REDEFINE = 8
 
     @staticmethod
     def allowedKeywordSuffixes(value):
@@ -1820,6 +1822,7 @@ def allowedKeywordSuffixes(value):
             ParserKind.TAG: ["."],
             ParserKind.COMMAND: [":"],
             ParserKind.LIST: [":"],
+            ParserKind.SPACE_LIST: [":"],
             ParserKind.BOOLEAN_EXPR: [":"],
             ParserKind.INTEGER: [":"],
             ParserKind.CUSTOM: [":", "."],
@@ -1833,6 +1836,7 @@ def str(value):
             ParserKind.TAG: "TAG",
             ParserKind.COMMAND: "COMMAND",
             ParserKind.LIST: "LIST",
+            ParserKind.SPACE_LIST: "SPACE_LIST",
             ParserKind.BOOLEAN_EXPR: "BOOLEAN_EXPR",
             ParserKind.INTEGER: "INTEGER",
             ParserKind.CUSTOM: "CUSTOM",
@@ -1881,6 +1885,8 @@ def __init__(self, keyword, kind, parser=None, initial_value=None):
             )
         elif kind == ParserKind.LIST:
             self.parser = self._handleList
+        elif kind == ParserKind.SPACE_LIST:
+            self.parser = self._handleSpaceList
         elif kind == ParserKind.BOOLEAN_EXPR:
             self.parser = self._handleBooleanExpr
         elif kind == ParserKind.INTEGER:
@@ -1955,6 +1961,14 @@ def _handleList(line_number, line, output):
         output.extend([s.strip() for s in line.split(",")])
         return output
 
+    @staticmethod
+    def _handleSpaceList(line_number, line, output):
+        """A parser for SPACE_LIST type keywords"""
+        if output is None:
+            output = []
+        output.extend([s.strip() for s in line.split(" ") if s.strip() != ""])
+        return output
+
     @staticmethod
     def _handleSingleInteger(line_number, line, output):
         """A parser for INTEGER type keywords"""

diff  --git a/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt b/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt
index 31e835d9bd4dff7..a0410adb5552f47 100644
--- a/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt
+++ b/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt
@@ -4,6 +4,11 @@
 // MY_RUN: baz
 // MY_LIST: one, two
 // MY_LIST: three, four
+// MY_SPACE_LIST: orange
+// MY_SPACE_LIST: tabby tortie tuxedo
+// MY_SPACE_LIST: void
+// MY_SPACE_LIST: multiple   spaces
+// MY_SPACE_LIST: cute, fluffy, kittens
 // MY_RUN: foo \
 // MY_RUN: bar
 //
@@ -25,3 +30,4 @@
 //
 // END.
 // MY_LIST: five
+// MY_SPACE_LIST: zebra

diff  --git a/llvm/utils/lit/tests/unit/TestRunner.py b/llvm/utils/lit/tests/unit/TestRunner.py
index e9888d685d3888e..f1baf51c02f3a11 100644
--- a/llvm/utils/lit/tests/unit/TestRunner.py
+++ b/llvm/utils/lit/tests/unit/TestRunner.py
@@ -61,6 +61,7 @@ def custom_parse(line_number, line, output):
             IntegratedTestKeywordParser("MY_TAG.", ParserKind.TAG),
             IntegratedTestKeywordParser("MY_DNE_TAG.", ParserKind.TAG),
             IntegratedTestKeywordParser("MY_LIST:", ParserKind.LIST),
+            IntegratedTestKeywordParser("MY_SPACE_LIST:", ParserKind.SPACE_LIST),
             IntegratedTestKeywordParser("MY_BOOL:", ParserKind.BOOLEAN_EXPR),
             IntegratedTestKeywordParser("MY_INT:", ParserKind.INTEGER),
             IntegratedTestKeywordParser("MY_RUN:", ParserKind.COMMAND),
@@ -104,6 +105,26 @@ def test_lists(self):
         list_parser = self.get_parser(parsers, "MY_LIST:")
         self.assertEqual(list_parser.getValue(), ["one", "two", "three", "four"])
 
+    def test_space_lists(self):
+        parsers = self.make_parsers()
+        self.parse_test(parsers)
+        space_list_parser = self.get_parser(parsers, "MY_SPACE_LIST:")
+        self.assertEqual(
+            space_list_parser.getValue(),
+            [
+                "orange",
+                "tabby",
+                "tortie",
+                "tuxedo",
+                "void",
+                "multiple",
+                "spaces",
+                "cute,",
+                "fluffy,",
+                "kittens",
+            ],
+        )
+
     def test_commands(self):
         parsers = self.make_parsers()
         self.parse_test(parsers)
@@ -222,6 +243,14 @@ def custom_parse(line_number, line, output):
         except BaseException as e:
             self.fail("LIST_WITH_DOT. raised the wrong exception: %r" % e)
 
+        try:
+            IntegratedTestKeywordParser("SPACE_LIST_WITH_DOT.", ParserKind.SPACE_LIST),
+            self.fail("SPACE_LIST_WITH_DOT. failed to raise an exception")
+        except ValueError as e:
+            pass
+        except BaseException as e:
+            self.fail("SPACE_LIST_WITH_DOT. raised the wrong exception: %r" % e)
+
         try:
             IntegratedTestKeywordParser(
                 "CUSTOM_NO_SUFFIX", ParserKind.CUSTOM, custom_parse


        


More information about the llvm-commits mailing list