[PATCH] D89493: [lit] Implement `not` as a builtin in the Lit internal shell

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:650
+                not_args.append(sys.executable)
+                not_args.append(os.path.join(builtin_commands_dir, args.pop(0) + ".py"))
                 not_count += 1
----------------
Nit: I don't like having `args.pop(0) + ".py"` here. I think it makes the code harder to read due to `args.pop(0)` both returning a value but also having a side-effect.



================
Comment at: llvm/utils/lit/lit/builtin_commands/not.py:33
+    if found is None:
+        sys.stderr.write('Failed to find program {}'.format(prog))
+    return found
----------------
Are you missing a `\n` here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89493/new/

https://reviews.llvm.org/D89493