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

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

> I'm not entirely satisfied with this patch -- if folks think we should implement not as an in-process builtin, let me know and I can look into that.

I guess the advantage here would be avoiding spawning another process? A downside might mean it's harder to test the builtin implementation unless we write the implementation to be testable. I hadn't looked at the builtin code in lit before. It's interesting that it already tries to parse the `--crash` arg to not.

Shouldn't we have some tests to test your implementation of the `not` builtin?



================
Comment at: llvm/utils/lit/lit/builtin_commands/not.py:64
+if __name__ == '__main__':
+    exit(main(sys.argv[1:]))
----------------
You might want to use `sys.exit()` here instead of `exit()`. `exit()` is meant for use in interactive shells.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89493