https://github.com/jdenny-ornl updated 
https://github.com/llvm/llvm-project/pull/65242:

>From 26d5891879583b9addd2a6d4d7caf4241ba55b85 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.o...@gmail.com>
Date: Thu, 31 Aug 2023 20:24:51 -0400
Subject: [PATCH 1/2] [lit] Are all RUN lines skipped in windows cmd?

Key issue
---------

When lit is configured to use windows `cmd` as an external shell, it
appears that all RUN lines are effectively commented out.  The
problematic change landed in July 26, 2023, so it's surprising it
hasn't come up yet.  I'm afraid I don't have a windows setup, so I
could use some help to verify what's happening.

Details
-------

In the case that lit is configured to use an external shell, D154280
(landed in 64d19542e78a on July 26, 2023) causes
`%dbg(RUN: at line N)` to be expanded in RUN lines early and in a
manner that is specific to sh-like shells.  As a result, later code in
lit that expands it in a shell-specific manner is useless.

That sh-like expansion uses the `:` command as follows:

```
: 'RUN: at line N'; original-commands
```

In sh-like shells, the `:` command simply discards its arguments.
However, in windows `cmd`, `:` indicates a goto label.  That appears
to effectively comment out the rest of the line, including the
original commands from the RUN line.

I am not aware of any complaints about this change.  Did I miss them?
Are all tests still passing and so no one noticed?  Lit's own test
suite has some tests that normally fail if RUN lines don't execute.
Is no one running lit's test suite with windows `cmd` as a lit
external shell?  Or is no one using windows `cmd` as a lit external
shell at all anymore?

Another issue
-------------

D154280 doesn't implement `--per-test-coverage` for lit's internal
shell.

Fix
---

This patch fixes the above problems by implementing
`--per-test-coverage` before selecting the shell (internal or
external) and by leaving `%dbg(RUN: at line N)` unexpanded.  Thus, it
is expanded later in a shell-specific manner, as before D154280.

I would like to understand whether windows `cmd` as a lit external
shell is worthwhile to support anymore.
---
 llvm/utils/lit/lit/TestRunner.py              | 44 ++++++++++++-------
 .../per-test-coverage-by-lit-cfg/lit.cfg      |  4 +-
 .../per-test-coverage-by-lit-cfg.py           |  5 ++-
 .../tests/Inputs/per-test-coverage/lit.cfg    |  5 ++-
 .../per-test-coverage/per-test-coverage.py    |  5 ++-
 .../lit/tests/per-test-coverage-by-lit-cfg.py |  9 +++-
 llvm/utils/lit/tests/per-test-coverage.py     |  9 +++-
 7 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 461cf63d6b9685..d564d6150a04a8 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1144,25 +1144,10 @@ def executeScriptInternal(test, litConfig, tmpBase, 
commands, cwd,
 def executeScript(test, litConfig, tmpBase, commands, cwd):
     bashPath = litConfig.getBashPath()
     isWin32CMDEXE = litConfig.isWindows and not bashPath
-    coverage_index = 0  # Counter for coverage file index
     script = tmpBase + ".script"
     if isWin32CMDEXE:
         script += ".bat"
 
-    # Set unique LLVM_PROFILE_FILE for each run command
-    for j, ln in enumerate(commands):
-        match = re.match(kPdbgRegex, ln)
-        if match:
-            command = match.group(2)
-            commands[j] = match.expand(": '\\1'; \\2" if command else ": 
'\\1'")
-            if litConfig.per_test_coverage:
-                # Extract the test case name from the test object
-                test_case_name = test.path_in_suite[-1]
-                test_case_name = test_case_name.rsplit(".", 1)[0]  # Remove 
the file extension
-                llvm_profile_file = f"{test_case_name}{coverage_index}.profraw"
-                commands[j] = f"export LLVM_PROFILE_FILE={llvm_profile_file} 
&& {commands[j]}"
-                coverage_index += 1
-
     # Write script file
     mode = "w"
     open_kwargs = {}
@@ -2115,10 +2100,35 @@ def parseIntegratedTestScript(test, 
additional_parsers=[], require_script=True):
 
 def _runShTest(test, litConfig, useExternalSh, script, tmpBase):
     def runOnce(execdir):
+        # Set unique LLVM_PROFILE_FILE for each run command
+        if litConfig.per_test_coverage:
+            # Extract the test case name from the test object, and remove the
+            # file extension.
+            test_case_name = test.path_in_suite[-1]
+            test_case_name = test_case_name.rsplit(".", 1)[0]
+            coverage_index = 0  # Counter for coverage file index
+            scriptProf = []
+            for ln in script:
+                match = re.match(kPdbgRegex, ln)
+                if match:
+                    dbg = match.group(1)
+                    command = match.group(2)
+                else:
+                    command = ln
+                profile = f"{test_case_name}{coverage_index}.profraw"
+                coverage_index += 1
+                command = f"export LLVM_PROFILE_FILE={profile} && {command}"
+                if match:
+                    command = buildPdbgCommand(dbg, command)
+                scriptProf.append(command)
+        else:
+            scriptProf = script
+
         if useExternalSh:
-            res = executeScript(test, litConfig, tmpBase, script, execdir)
+            res = executeScript(test, litConfig, tmpBase, scriptProf, execdir)
         else:
-            res = executeScriptInternal(test, litConfig, tmpBase, script, 
execdir)
+            res = executeScriptInternal(test, litConfig, tmpBase, scriptProf,
+                                        execdir)
         if isinstance(res, lit.Test.Result):
             return res
 
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg 
b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
index 186c81ebd17283..710a9220464ac0 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
@@ -3,6 +3,8 @@ import os
 
 config.name = "per-test-coverage-by-lit-cfg"
 config.suffixes = [".py"]
-config.test_format = lit.formats.ShTest(execute_external=True)
+config.test_format = lit.formats.ShTest(
+    execute_external=eval(lit_config.params.get("execute_external"))
+)
 lit_config.per_test_coverage = True
 config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
diff --git 
a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
 
b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
index 0abe00bdedb1f2..349bbafab2e1b6 100644
--- 
a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
+++ 
b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
@@ -1,5 +1,6 @@
 # Check that the environment variable is set correctly
-# RUN: %{python} %s | FileCheck %s
+# RUN: %{python} %s | FileCheck -DINDEX=0 %s
+# RUN: %{python} %s | FileCheck -DINDEX=1 %s
 
 # Python script to read the environment variable
 # and print its value
@@ -8,4 +9,4 @@
 llvm_profile_file = os.environ.get('LLVM_PROFILE_FILE')
 print(llvm_profile_file)
 
-# CHECK: per-test-coverage-by-lit-cfg0.profraw
+# CHECK: per-test-coverage-by-lit-cfg[[INDEX]].profraw
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg 
b/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
index 6e7f4c0e3bc97d..dae6db561a65db 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
@@ -3,6 +3,7 @@ import os
 
 config.name = "per-test-coverage"
 config.suffixes = [".py"]
-config.test_format = lit.formats.ShTest(execute_external=True)
+config.test_format = lit.formats.ShTest(
+    execute_external=eval(lit_config.params.get("execute_external"))
+)
 config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
-
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py 
b/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
index 65e4aa69bff079..025ecb43fce0ec 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
@@ -1,5 +1,6 @@
 # Check that the environment variable is set correctly
-# RUN: %{python} %s | FileCheck %s
+# RUN: %{python} %s | FileCheck -DINDEX=0 %s
+# RUN: %{python} %s | FileCheck -DINDEX=1 %s
 
 # Python script to read the environment variable
 # and print its value
@@ -8,4 +9,4 @@
 llvm_profile_file = os.environ.get('LLVM_PROFILE_FILE')
 print(llvm_profile_file)
 
-# CHECK: per-test-coverage0.profraw
+# CHECK: per-test-coverage[[INDEX]].profraw
diff --git a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py 
b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
index 1cc3927dc89041..0b9ecf5346a70d 100644
--- a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
+++ b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
@@ -1,6 +1,11 @@
 # Test if lit_config.per_test_coverage in lit.cfg sets individual test case 
coverage.
 
-# RUN: %{lit} -a -v 
%{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py \
-# RUN: | FileCheck -match-full-lines %s
+# RUN: %{lit} -a -v -Dexecute_external=False \
+# RUN:     
%{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
+# RUN:   FileCheck -match-full-lines %s
+#
+# RUN: %{lit} -a -v -Dexecute_external=True \
+# RUN:     
%{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
+# RUN:   FileCheck -match-full-lines %s
 #
 # CHECK: PASS: per-test-coverage-by-lit-cfg :: per-test-coverage-by-lit-cfg.py 
({{[^)]*}})
diff --git a/llvm/utils/lit/tests/per-test-coverage.py 
b/llvm/utils/lit/tests/per-test-coverage.py
index 428712e4792576..c521189e4aeb83 100644
--- a/llvm/utils/lit/tests/per-test-coverage.py
+++ b/llvm/utils/lit/tests/per-test-coverage.py
@@ -1,6 +1,11 @@
 # Test LLVM_PROFILE_FILE is set when --per-test-coverage is passed to command 
line.
 
-# RUN: %{lit} -a -v --per-test-coverage 
%{inputs}/per-test-coverage/per-test-coverage.py \
-# RUN: | FileCheck -match-full-lines %s
+# RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=False \
+# RUN:     %{inputs}/per-test-coverage/per-test-coverage.py | \
+# RUN:   FileCheck -match-full-lines %s
+#
+# RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=True \
+# RUN:        %{inputs}/per-test-coverage/per-test-coverage.py | \
+# RUN:   FileCheck -match-full-lines %s
 #
 # CHECK: PASS: per-test-coverage :: per-test-coverage.py ({{[^)]*}})

>From 013329da6442eb5c96b01a2403972e07b4e6cb35 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.o...@gmail.com>
Date: Mon, 4 Sep 2023 14:51:25 -0400
Subject: [PATCH 2/2] Test 'RUN: at line N' if --per-test-coverage

---
 .../lit/tests/per-test-coverage-by-lit-cfg.py | 18 +++++++++++++-----
 llvm/utils/lit/tests/per-test-coverage.py     | 19 ++++++++++++++-----
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py 
b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
index 0b9ecf5346a70d..39e2595f999689 100644
--- a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
+++ b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
@@ -2,10 +2,18 @@
 
 # RUN: %{lit} -a -v -Dexecute_external=False \
 # RUN:     
%{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
-# RUN:   FileCheck -match-full-lines %s
-#
+# RUN:   FileCheck -DOUT=stdout %s
+
 # RUN: %{lit} -a -v -Dexecute_external=True \
 # RUN:     
%{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
-# RUN:   FileCheck -match-full-lines %s
-#
-# CHECK: PASS: per-test-coverage-by-lit-cfg :: per-test-coverage-by-lit-cfg.py 
({{[^)]*}})
+# RUN:   FileCheck -DOUT=stderr %s
+
+#      CHECK: {{^}}PASS: per-test-coverage-by-lit-cfg :: 
per-test-coverage-by-lit-cfg.py ({{[^)]*}})
+#      CHECK: Command Output ([[OUT]]):
+# CHECK-NEXT: --
+#      CHECK: {{RUN}}: at line 2
+#      CHECK: export LLVM_PROFILE_FILE=per-test-coverage-by-lit-cfg0.profraw
+#      CHECK: per-test-coverage-by-lit-cfg.py
+#      CHECK: {{RUN}}: at line 3
+#      CHECK: export LLVM_PROFILE_FILE=per-test-coverage-by-lit-cfg1.profraw
+#      CHECK: per-test-coverage-by-lit-cfg.py
diff --git a/llvm/utils/lit/tests/per-test-coverage.py 
b/llvm/utils/lit/tests/per-test-coverage.py
index c521189e4aeb83..dfa6f4aabf76c2 100644
--- a/llvm/utils/lit/tests/per-test-coverage.py
+++ b/llvm/utils/lit/tests/per-test-coverage.py
@@ -2,10 +2,19 @@
 
 # RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=False \
 # RUN:     %{inputs}/per-test-coverage/per-test-coverage.py | \
-# RUN:   FileCheck -match-full-lines %s
-#
+# RUN:   FileCheck -DOUT=stdout %s
+
 # RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=True \
 # RUN:        %{inputs}/per-test-coverage/per-test-coverage.py | \
-# RUN:   FileCheck -match-full-lines %s
-#
-# CHECK: PASS: per-test-coverage :: per-test-coverage.py ({{[^)]*}})
+# RUN:   FileCheck -DOUT=stderr %s
+
+# CHECK: {{^}}PASS: per-test-coverage :: per-test-coverage.py ({{[^)]*}})
+t-cfg.py ({{[^)]*}})
+#      CHECK: Command Output ([[OUT]]):
+# CHECK-NEXT: --
+#      CHECK: {{RUN}}: at line 2
+#      CHECK: export LLVM_PROFILE_FILE=per-test-coverage0.profraw
+#      CHECK: per-test-coverage.py
+#      CHECK: {{RUN}}: at line 3
+#      CHECK: export LLVM_PROFILE_FILE=per-test-coverage1.profraw
+#      CHECK: per-test-coverage.py

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to