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

[Louis Dionne via Phabricator via llvm-commits]

ldionne marked 3 inline comments as done.
ldionne added a comment.

In D89493#2334002 <https://reviews.llvm.org/D89493#2334002>, @delcypher wrote:

>> 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?

The implementation is already tested -- the shtest-not tests are running in the internal shell, and are testing that very implementation. As it stands, it mostly means that the original `not.cpp` in LLVM's utilities is the thing that's not tested anymore, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89493