[llvm] [llvm-lit] Adding -e option to lit's built-in cat command (PR #102061)

via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 7 10:19:55 PDT 2024


https://github.com/connieyzhu updated https://github.com/llvm/llvm-project/pull/102061

>From bfad1edd445a5bde5ea229eb0f5165daca2eb33a Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Tue, 30 Jul 2024 20:18:15 +0000
Subject: [PATCH 1/7] [llvm-lit] Adding -e option to lit's built-in cat command

This patch extends support for using cat -e in lit tests that use the internal
shell. The logic for enabling options is changed to accommodate for the
possibility of having several options.
---
 llvm/utils/lit/lit/builtin_commands/cat.py | 51 +++++++++++++---------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index d6dbe889cc2b1..4d80eaeb59ae0 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -7,34 +7,43 @@
     from io import StringIO
 
 
-def convertToCaretAndMNotation(data):
+def convertTextNotation(data, options):
     newdata = StringIO()
     if isinstance(data, str):
         data = bytearray(data.encode())
 
     for intval in data:
-        if intval == 9 or intval == 10:
-            newdata.write(chr(intval))
-            continue
-        if intval > 127:
-            intval = intval - 128
-            newdata.write("M-")
-        if intval < 32:
-            newdata.write("^")
-            newdata.write(chr(intval + 64))
-        elif intval == 127:
-            newdata.write("^?")
-        else:
-            newdata.write(chr(intval))
+        if "show_ends" in options:
+            if intval == 10:
+                newdata.write("$")
+                newdata.write(chr(intval))
+                continue
+        if "show_nonprinting" in options:
+            if intval == 9 or intval == 10:
+                newdata.write(chr(intval))
+                continue
+            if intval > 127:
+                intval = intval - 128
+                newdata.write("M-")
+            if intval < 32:
+                newdata.write("^")
+                newdata.write(chr(intval + 64))
+            elif intval == 127:
+                newdata.write("^?")
+            else:
+                newdata.write(chr(intval))
+
+    if "show_ends" in options:
+        newdata.write("$")
 
     return newdata.getvalue().encode()
 
 
 def main(argv):
     arguments = argv[1:]
-    short_options = "v"
+    short_options = "ve"
     long_options = ["show-nonprinting"]
-    show_nonprinting = False
+    enabled_options = []
 
     try:
         options, filenames = getopt.gnu_getopt(arguments, short_options, long_options)
@@ -43,8 +52,10 @@ def main(argv):
         sys.exit(1)
 
     for option, value in options:
-        if option == "-v" or option == "--show-nonprinting":
-            show_nonprinting = True
+        if option == "-v" or option == "--show-nonprinting" or option == "-e":
+            enabled_options.append("show_nonprinting")
+        if option == "-e":
+            enabled_options.append("show_ends")
 
     writer = getattr(sys.stdout, "buffer", None)
     if writer is None:
@@ -69,8 +80,8 @@ def main(argv):
                 fileToCat = open(filename, "rb")
                 contents = fileToCat.read()
 
-            if show_nonprinting:
-                contents = convertToCaretAndMNotation(contents)
+            if enabled_options:
+                contents = convertTextNotation(contents, enabled_options)
             elif is_text:
                 contents = contents.encode()
             writer.write(contents)

>From 1d41bfab0449a6caf026eee6ca4d4121db7307b3 Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Mon, 5 Aug 2024 21:02:21 +0000
Subject: [PATCH 2/7] [llvm-lit] Restructure the tracking of enabled cat
 options

This patch changes the storing of enabled options from a list of bools to a
dataclass for better efficiency when accessing and checking the options.
---
 llvm/utils/lit/lit/builtin_commands/cat.py | 24 +++++++++++++---------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index 4d80eaeb59ae0..14056e3ccd444 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -1,24 +1,29 @@
 import getopt
 import sys
+from dataclasses import dataclass, fields
 
 try:
     from StringIO import StringIO
 except ImportError:
     from io import StringIO
 
+ at dataclass
+class Options:
+    show_ends: bool
+    show_nonprinting: bool
 
 def convertTextNotation(data, options):
     newdata = StringIO()
     if isinstance(data, str):
         data = bytearray(data.encode())
-
+    
     for intval in data:
-        if "show_ends" in options:
+        if options.show_ends:
             if intval == 10:
                 newdata.write("$")
                 newdata.write(chr(intval))
                 continue
-        if "show_nonprinting" in options:
+        if options.show_nonprinting:
             if intval == 9 or intval == 10:
                 newdata.write(chr(intval))
                 continue
@@ -32,9 +37,8 @@ def convertTextNotation(data, options):
                 newdata.write("^?")
             else:
                 newdata.write(chr(intval))
-
-    if "show_ends" in options:
-        newdata.write("$")
+        else:
+            newdata.write(chr(intval))
 
     return newdata.getvalue().encode()
 
@@ -43,7 +47,7 @@ def main(argv):
     arguments = argv[1:]
     short_options = "ve"
     long_options = ["show-nonprinting"]
-    enabled_options = []
+    enabled_options = Options(show_ends=False, show_nonprinting=False)
 
     try:
         options, filenames = getopt.gnu_getopt(arguments, short_options, long_options)
@@ -53,9 +57,9 @@ def main(argv):
 
     for option, value in options:
         if option == "-v" or option == "--show-nonprinting" or option == "-e":
-            enabled_options.append("show_nonprinting")
+            enabled_options.show_nonprinting = True
         if option == "-e":
-            enabled_options.append("show_ends")
+            enabled_options.show_ends = True
 
     writer = getattr(sys.stdout, "buffer", None)
     if writer is None:
@@ -80,7 +84,7 @@ def main(argv):
                 fileToCat = open(filename, "rb")
                 contents = fileToCat.read()
 
-            if enabled_options:
+            if True in fields(enabled_options):
                 contents = convertTextNotation(contents, enabled_options)
             elif is_text:
                 contents = contents.encode()

>From 9eef7a5fea6e916c8443fd285bc81be780d8cdad Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Mon, 5 Aug 2024 21:24:19 +0000
Subject: [PATCH 3/7] [llvm-lit] Fixed darker formatting issue

---
 llvm/utils/lit/lit/builtin_commands/cat.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index 14056e3ccd444..6afba4543459f 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -7,16 +7,19 @@
 except ImportError:
     from io import StringIO
 
+
 @dataclass
 class Options:
     show_ends: bool
     show_nonprinting: bool
 
+
 def convertTextNotation(data, options):
     newdata = StringIO()
     if isinstance(data, str):
         data = bytearray(data.encode())
-    
+
+
     for intval in data:
         if options.show_ends:
             if intval == 10:

>From fcc87bcb06016ba51c89fcd8c49f1dffaf662c66 Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Mon, 5 Aug 2024 23:31:19 +0000
Subject: [PATCH 4/7] [llvm-lit][NFC] Added comments for dataclass + simplified
 show_ends logic

---
 llvm/utils/lit/lit/builtin_commands/cat.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index 6afba4543459f..a012e338de47b 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -8,9 +8,12 @@
     from io import StringIO
 
 
+# This dataclass defines all currently supported options for cat
 @dataclass
 class Options:
+    # Options: -e. True if newlines should be displayed with a '$'
     show_ends: bool
+    # Options: -v, -e. True if text should be converted to ^ and M- notation
     show_nonprinting: bool
 
 
@@ -21,11 +24,10 @@ def convertTextNotation(data, options):
 
 
     for intval in data:
-        if options.show_ends:
-            if intval == 10:
-                newdata.write("$")
-                newdata.write(chr(intval))
-                continue
+        if options.show_ends and intval == 10:
+            newdata.write("$")
+            newdata.write(chr(intval))
+            continue
         if options.show_nonprinting:
             if intval == 9 or intval == 10:
                 newdata.write(chr(intval))

>From fd3375dcbd2a648722181f8811ba9f436de5d4db Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Tue, 6 Aug 2024 21:54:10 +0000
Subject: [PATCH 5/7] [llvm-lit] Added variable to track when cat should change
 text notation

This patch adds a new bool variable convert_text that is turned on if
show_nonprinting, show_ends, or any option added in the future that is
meant to modify text is True. This fixes the faulty logic that assumes
that any option that is True is one that will modify text. Now,
convertTextNotation() will only be called for options that need it.
---
 llvm/utils/lit/lit/builtin_commands/cat.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index a012e338de47b..9ab5b574e1467 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -1,6 +1,6 @@
 import getopt
 import sys
-from dataclasses import dataclass, fields
+from dataclasses import dataclass
 
 try:
     from StringIO import StringIO
@@ -22,9 +22,8 @@ def convertTextNotation(data, options):
     if isinstance(data, str):
         data = bytearray(data.encode())
 
-
     for intval in data:
-        if options.show_ends and intval == 10:
+        if intval == 10 and options.show_ends:
             newdata.write("$")
             newdata.write(chr(intval))
             continue
@@ -53,6 +52,7 @@ def main(argv):
     short_options = "ve"
     long_options = ["show-nonprinting"]
     enabled_options = Options(show_ends=False, show_nonprinting=False)
+    convert_text = False
 
     try:
         options, filenames = getopt.gnu_getopt(arguments, short_options, long_options)
@@ -63,8 +63,10 @@ def main(argv):
     for option, value in options:
         if option == "-v" or option == "--show-nonprinting" or option == "-e":
             enabled_options.show_nonprinting = True
+            convert_text = True
         if option == "-e":
             enabled_options.show_ends = True
+            convert_text = True
 
     writer = getattr(sys.stdout, "buffer", None)
     if writer is None:
@@ -89,7 +91,7 @@ def main(argv):
                 fileToCat = open(filename, "rb")
                 contents = fileToCat.read()
 
-            if True in fields(enabled_options):
+            if convert_text:
                 contents = convertTextNotation(contents, enabled_options)
             elif is_text:
                 contents = contents.encode()

>From fcf072ad53b7e126b485ed020c4055126e2e44eb Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Wed, 7 Aug 2024 17:15:32 +0000
Subject: [PATCH 6/7] [llvm-lit] Added extra check when calling
 convertTextNotation

This patch adds an assert to guarantee that when calling
convertTextNotation(), the show_ends and show_nonprinting options are
turned on. Some logic is also simplified when converting characters.
---
 llvm/utils/lit/lit/builtin_commands/cat.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index 9ab5b574e1467..8cf438fbf4fbd 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -18,6 +18,8 @@ class Options:
 
 
 def convertTextNotation(data, options):
+    assert options.show_ends or options.show_nonprinting
+
     newdata = StringIO()
     if isinstance(data, str):
         data = bytearray(data.encode())
@@ -37,12 +39,11 @@ def convertTextNotation(data, options):
             if intval < 32:
                 newdata.write("^")
                 newdata.write(chr(intval + 64))
+                continue
             elif intval == 127:
                 newdata.write("^?")
-            else:
-                newdata.write(chr(intval))
-        else:
-            newdata.write(chr(intval))
+                continue
+        newdata.write(chr(intval))
 
     return newdata.getvalue().encode()
 

>From a44eecbf9e83f28b378cfb6e648b02b8bce02f52 Mon Sep 17 00:00:00 2001
From: Connie Zhu <connieyzhu at google.com>
Date: Wed, 7 Aug 2024 17:17:45 +0000
Subject: [PATCH 7/7] [llvm-lit] Added -E option to built-in cat

This patch allows for lit's built-in cat to use the -E option. The
functionality was already implemented in previous patches, but this
patch allows for the parsing and usage of -E.
---
 llvm/utils/lit/lit/builtin_commands/cat.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/utils/lit/lit/builtin_commands/cat.py b/llvm/utils/lit/lit/builtin_commands/cat.py
index 8cf438fbf4fbd..fec31e4a82f18 100644
--- a/llvm/utils/lit/lit/builtin_commands/cat.py
+++ b/llvm/utils/lit/lit/builtin_commands/cat.py
@@ -50,8 +50,8 @@ def convertTextNotation(data, options):
 
 def main(argv):
     arguments = argv[1:]
-    short_options = "ve"
-    long_options = ["show-nonprinting"]
+    short_options = "eEv"
+    long_options = ["show-ends", "show-nonprinting"]
     enabled_options = Options(show_ends=False, show_nonprinting=False)
     convert_text = False
 
@@ -65,7 +65,7 @@ def main(argv):
         if option == "-v" or option == "--show-nonprinting" or option == "-e":
             enabled_options.show_nonprinting = True
             convert_text = True
-        if option == "-e":
+        if option == "-E" or option == "--show-ends" or option == "-e":
             enabled_options.show_ends = True
             convert_text = True
 



More information about the llvm-commits mailing list