[llvm-lit] Support curly brace syntax in lit internal shell

[connieyzhu]

This patch implements parsing and execution of a series of commands grouped in curly braces in lit's internal shell. This addresses goals mentioned in this RFC: https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179.

The curly brace functionality is defined by section 2.9.4 Grouping Commands of this page.

Fixes #102382

[connieyzhu]

CC: @ilovepi @petrhosek

Hi Paul and Petr, this patch is still a WIP but I put it up just for your reference.

[github-actions]

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:

darker --check --diff -r 7c4c72b52038810a8997938a2b3485363cd6be3a...2bc71171d13fdda0bf16f14c4c75c1fd597fd80f llvm/utils/lit/lit/ShCommands.py llvm/utils/lit/lit/ShUtil.py llvm/utils/lit/lit/TestRunner.py

View the diff from darker here.

--- ShUtil.py	2024-08-12 16:05:58.000000 +0000
+++ ShUtil.py	2024-08-12 16:09:36.489856 +0000
@@ -159,11 +159,11 @@
         c = self.eat()
         if c == "{" or c == "}":
             return (c,)
         if c == ";":
             if self.maybe_eat("}") or (self.maybe_eat(" ") and self.maybe_eat("}")):
-                return("}",)
+                return ("}",)
             return (c,)
         if c == "|":
             if self.maybe_eat("|"):
                 return ("||",)
             return (c,)
@@ -203,11 +203,11 @@
     def __init__(self, data, win32Escapes=False, pipefail=False):
         self.data = data
         self.pipefail = pipefail
         self.tokens = ShLexer(data, win32Escapes=win32Escapes).lex()
         self.brace_stack = []
-        self.brace_dict = {'{': '}'}
+        self.brace_dict = {"{": "}"}
 
     def lex(self):
         for item in self.tokens:
             return item
         return None
@@ -259,22 +259,21 @@
         commands = [self.parse_command()]
         while self.look() == ("|",):
             self.lex()
             commands.append(self.parse_command())
         return Pipeline(commands, negate, self.pipefail)
-            
-    
+
     # {echo foo; echo bar;} && echo hello
     # echo hello && {echo foo; echo bar}
-    
+
     def parse(self, seq_type):
         lhs = None
         if isinstance(self.look(), tuple):
             brace = self.lex()
             self.brace_stack.append(brace)
-            if brace[0] == '{':
-                lhs = self.parse(('{', '}'))
+            if brace[0] == "{":
+                lhs = self.parse(("{", "}"))
             else:
                 raise ValueError("syntax error near unexpected token %r" % brace[0])
 
         else:
             lhs = self.parse_pipeline()
@@ -289,14 +288,14 @@
             if not self.look():
                 raise ValueError("missing argument to operator %r" % operator[0])
 
             # FIXME: Operator precedence!!
             if isinstance(self.look(), tuple):
-                lhs = self.parse(('{', '}'))
-            else: 
+                lhs = self.parse(("{", "}"))
+            else:
                 lhs = Seq(lhs, operator[0], self.parse_pipeline(), seq_type)
             seq_type = None
-        
+
         if not stack.empty():
             raise ValueError("missing token to %r" % stack.peek())
 
         return lhs
--- TestRunner.py	2024-08-12 16:05:58.000000 +0000
+++ TestRunner.py	2024-08-12 16:09:37.152689 +0000
@@ -1054,11 +1054,13 @@
                 ln += "has no command after substitutions"
         else:
             ln = command
         try:
             cmds.append(
-                ShUtil.ShParser(ln, litConfig.isWindows, test.config.pipefail).parse(None)
+                ShUtil.ShParser(ln, litConfig.isWindows, test.config.pipefail).parse(
+                    None
+                )
             )
         except:
             raise ScriptFatal(
                 f"shell parser error on {dbg}: {command.lstrip()}\n"
             ) from None

[ilovepi]

what is the type of seq_type? What's it's purpose?

[ilovepi]

Can we get a comment here about what's going on?

[ilovepi]

do you need a brace_dict if there's only one thing in it? If its intended to hold more matches, perhaps a different name is more appropriate?

[ilovepi]

Why is the seq_type thrown away?