[llvm] [llvm-lit] Fix `TypeError` string argument expected in lit's internal shell (PR #105925)

via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 26 22:28:10 PDT 2024


================
@@ -413,7 +413,7 @@ def maybeUnescape(arg):
         for arg in args[:-1]:
             stdout.write(encode(maybeUnescape(arg)))
             stdout.write(encode(" "))
-        stdout.write(encode(maybeUnescape(args[-1])))
+        stdout.write(encode(maybeUnescape(str(args[-1]))))
----------------
Harini0924 wrote:

You're right that expanding globs should ideally happen before any commands are invoked. The fact that the issue only occurs with the last argument does seem odd, and it might just be that the current test suite only triggers the problem there.

We could address this by placing the line `cmd.args = expand_glob_expressions(cmd.args, shenv.cwd)` at the top of the `executeBuiltinEcho` function. This would ensure that all arguments are expanded consistently before the command is executed. However, I believe adding `str()` to the last argument might be a more targeted and effective solution in this specific case. The problem appears to be that the last argument isn't being expanded correctly, leading to the error. By converting just the last argument to a string, we directly address the failing scenario without introducing broader changes that might impact other parts of the code.

While expanding all arguments at the top is a comprehensive solution, adding `str()` to the last argument is a simpler fix that directly resolves the current issue. It keeps the changes minimal and focused on the specific failure encountered, which might be preferable in this context IMO. What are your thoughts on this approach?

https://github.com/llvm/llvm-project/pull/105925


More information about the llvm-commits mailing list