[llvm] efee579 - Reland "[lit] Handle plain negations directly in the internal shell"

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 15 01:02:42 PDT 2021


Author: Martin Storsjö
Date: 2021-04-15T11:02:14+03:00
New Revision: efee57925c3f46c74c6697234ce4f869e772834a

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

LOG: Reland "[lit] Handle plain negations directly in the internal shell"

Keep running "not --crash" via the external "not" executable, but
for plain negations, and for cases that use the shell "!" operator,
just skip that argument and invert the return code.

The libcxx tests only use the shell operator "!" for negations,
never the "not" executable, because libcxx tests can be run without
having a fully built llvm tree available providing the "not"
executable.

This allows using the internal shell for libcxx tests.

It should be possible to reland this now that D99938 fixed the
one test failure in clang-tidy that broke when "not" was handled
internally, letting lit/python execute grep.exe directly instead
of via not.exe. (See D99330 and D99406 for more commentery on the
exact issue that broke and other potential ways of fixing it.)

Differential Revision: https://reviews.llvm.org/D98859

Added: 
    llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-nested-none.txt
    llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-none.txt
    llvm/utils/lit/tests/Inputs/shtest-not/exclamation-calls-external.txt

Modified: 
    llvm/utils/lit/lit/TestRunner.py
    llvm/utils/lit/tests/lit.cfg
    llvm/utils/lit/tests/shtest-not.py

Removed: 
    


################################################################################
diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 70d641e58df9f..cb4102ec0b66f 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -614,6 +614,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
     assert isinstance(cmd, ShUtil.Pipeline)
 
     procs = []
+    negate_procs = []
     default_stdin = subprocess.PIPE
     stderrTempFiles = []
     opened_files = []
@@ -659,6 +660,12 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
                 if not args:
                     raise InternalShellError(j, "Error: 'not' requires a"
                                                 " subcommand")
+            elif args[0] == '!':
+                not_args.append(args.pop(0))
+                not_count += 1
+                if not args:
+                    raise InternalShellError(j, "Error: '!' requires a"
+                                                " subcommand")
             else:
                 break
 
@@ -705,7 +712,15 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
         # the assumptions that (1) environment variables are not intended to be
         # relevant to 'not' commands and (2) the 'env' command should always
         # blindly pass along the status it receives from any command it calls.
-        args = not_args + args
+
+        # For plain negations, either 'not' without '--crash', or the shell
+        # operator '!', leave them out from the command to execute and
+        # invert the result code afterwards.
+        if not_crash:
+            args = not_args + args
+            not_count = 0
+        else:
+            not_args = []
 
         stdin, stdout, stderr = processRedirects(j, default_stdin, cmd_shenv,
                                                  opened_files)
@@ -769,6 +784,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
                                           stderr = stderr,
                                           env = cmd_shenv.env,
                                           close_fds = kUseCloseFDs))
+            negate_procs.append((not_count % 2) != 0)
             # Let the helper know about this process
             timeoutHelper.addProcess(procs[-1])
         except OSError as e:
@@ -821,6 +837,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
         # Detect Ctrl-C in subprocess.
         if res == -signal.SIGINT:
             raise KeyboardInterrupt
+        if negate_procs[i]:
+            res = not res
 
         # Ensure the resulting output is always of string type.
         try:

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-nested-none.txt b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-nested-none.txt
new file mode 100644
index 0000000000000..86b350d9b2e33
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-nested-none.txt
@@ -0,0 +1 @@
+# RUN: ! ! !

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-none.txt b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-none.txt
new file mode 100644
index 0000000000000..8a5e1f9148197
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-args-none.txt
@@ -0,0 +1 @@
+# RUN: !

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-calls-external.txt b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-calls-external.txt
new file mode 100644
index 0000000000000..035da88075f38
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not/exclamation-calls-external.txt
@@ -0,0 +1,9 @@
+# Simple uses of '!'
+
+# RUN: ! %{python} fail.py
+# RUN: ! ! %{python} pass.py
+# RUN: ! ! ! %{python} fail.py
+# RUN: ! ! ! ! %{python} pass.py
+
+# pass.py succeeds but we expect failure
+# RUN: ! %{python} pass.py

diff  --git a/llvm/utils/lit/tests/lit.cfg b/llvm/utils/lit/tests/lit.cfg
index 3c49f076a66ee..98e02518fe9f4 100644
--- a/llvm/utils/lit/tests/lit.cfg
+++ b/llvm/utils/lit/tests/lit.cfg
@@ -95,7 +95,8 @@ if not llvm_config:
 # that might not be present or behave correctly on all platforms.  Don't do
 # this for 'echo' because an external version is used when it appears in a
 # pipeline.  Don't do this for ':' because it doesn't appear to be a valid file
-# name under Windows.
+# name under Windows. Don't do this for 'not' because lit uses the external
+# 'not' throughout a RUN line that calls 'not --crash'.
 test_bin = os.path.join(os.path.dirname(__file__), 'Inputs', 'fake-externals')
 config.environment['PATH'] = os.path.pathsep.join((test_bin,
                                                    config.environment['PATH']))

diff  --git a/llvm/utils/lit/tests/shtest-not.py b/llvm/utils/lit/tests/shtest-not.py
index a391e7e60a323..2c9a2d4d41be9 100644
--- a/llvm/utils/lit/tests/shtest-not.py
+++ b/llvm/utils/lit/tests/shtest-not.py
@@ -10,7 +10,27 @@
 
 # Make sure not and env commands are included in printed commands.
 
-# CHECK: -- Testing: 13 tests{{.*}}
+# CHECK: -- Testing: 16 tests{{.*}}
+
+# CHECK: FAIL: shtest-not :: exclamation-args-nested-none.txt {{.*}}
+# CHECK: $ "!" "!" "!"
+# CHECK: Error: '!' requires a subcommand
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-not :: exclamation-args-none.txt {{.*}}
+# CHECK: $ "!"
+# CHECK: Error: '!' requires a subcommand
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-not :: exclamation-calls-external.txt {{.*}}
+
+# CHECK: $ "!" "{{[^"]*}}" "fail.py"
+# CHECK: $ "!" "!" "{{[^"]*}}" "pass.py"
+# CHECK: $ "!" "!" "!" "{{[^"]*}}" "fail.py"
+# CHECK: $ "!" "!" "!" "!" "{{[^"]*}}" "pass.py"
+
+# CHECK: $ "!" "{{[^"]*}}" "pass.py"
+# CHECK: error: command failed with exit status: {{.*}}
 
 # CHECK: FAIL: shtest-not :: not-args-last-is-crash.txt {{.*}}
 # CHECK: $ "not" "--crash"
@@ -114,5 +134,5 @@
 # CHECK: error: command failed with exit status: {{.*}}
 
 # CHECK: Passed:  1
-# CHECK: Failed: 12
+# CHECK: Failed: 15
 # CHECK-NOT: {{.}}


        


More information about the llvm-commits mailing list