Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of ! for older lit versions #720

Closed
wants to merge 1 commit into from

Conversation

keyboardDrummer
Copy link
Collaborator

No description provided.

Copy link

@davidcok davidcok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this is needed. The change is not equivalent: the ! checks that there is a non-zero exit code, whereas the change ignores the exit code.

@shazqadeer
Copy link
Contributor

I'm curious why this is needed. The change is not equivalent: the ! checks that there is a non-zero exit code, whereas the change ignores the exit code.

@davidcok : Could you point me to the documentation for the ! operator?

I see the following on my end:

shaz@shaz-mbp commandline % lit --version
lit 0.11.1

What do you have on your end?

@shazqadeer
Copy link
Contributor

shazqadeer commented May 1, 2023

I updated lit on my machine:

shaz@shaz-mbp commandline % lit --version
lit 16.0.2

Now the test works on my end. It seems fine to continue to use the ! operator. A note about the lit version number on the Test README where we mention the lit dependency might be helpful.

@davidcok
Copy link

davidcok commented May 1, 2023

I also have 0.11.1 - and ! works fine.
I don't see mention of ! in the online llvm-lit documentation, but that documentation is very brief.
But I should not speak for lit in the boogie environment.
In dafny testing we have implemented a number of extensions to lit, such as additional % macros. Perhaps ! is an extension as well. In any case, it and %exits-with are very handy for testing expected-to-fail RUN commands.

auto-merge was automatically disabled May 1, 2023 18:16

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants