Skip to content

Fix FIXME in Builder::and and Builder::or impls #101

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

Merged
merged 2 commits into from
Oct 12, 2021

Conversation

fisherdarling
Copy link
Contributor

@fisherdarling fisherdarling commented Oct 12, 2021

Since a bug in gccjit was resolved: https://gcc.gnu.org/bugzilla//show_bug.cgi?id=95498 the Builder::and and Builder::or impl can be fixed and made much cleaner.

I think I did this right? Is there a list somewhere with the proper casting rules? I was a able to run ./test.sh and it made it through testing rust tests:
test result: FAILED. 4352 passed; 86 failed; 50 ignored; 0 measured; 0 filtered out; finished in 93.79s

closes #80

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Yup, you did it perfectly.

I was a bit surprised by your results locally, but the CI has 85 failures, so that's ok.

@fisherdarling
Copy link
Contributor Author

Glad to hear that was right! I was a little confused if I had to do anything with int promotion (?) or something like that, but I guess not.

Also, is there a reason why rustfmt is not being used?

@antoyo
Copy link
Contributor

antoyo commented Oct 12, 2021

I tried using rustfmt but couldn't get the formatting I wanted.

@antoyo antoyo merged commit 863cfb2 into rust-lang:master Oct 12, 2021
@antoyo
Copy link
Contributor

antoyo commented Oct 12, 2021

Thanks for your work!
You had nothing to do with int promotion: there was actually a bug related to that in libgccjit and I fixed it.

@fisherdarling
Copy link
Contributor Author

Thanks! I'll definitely pick up some other low-hanging fruit in the future.

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.

Don't put results in variables when unneeded
2 participants