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

fix issue #3222 #3316

Merged
merged 1 commit into from
Sep 5, 2012
Merged

Conversation

Vincent-Belliard
Copy link
Contributor

Borrowed strings have one indirection less than other strings. With the matched expression, an extra Load was done. With patterns, constant strings weren't Store/Load as needed.

I add a test: src/test/run-pass/alt-borrowed_str.rs

I start watching rust 2 weeks ago. Even if I did my best to make something consistent, the LLVM generator is quite hard to understand: I may have miss something. Please, before merging, check I didn't break the model.

@brson
Copy link
Contributor

brson commented Aug 31, 2012

Awesome! @nikomatsakis can you review this?

@nikomatsakis
Copy link
Contributor

I think the logic is basically right but the code should make use of some of the helper functions that are already there. Rather than creating a new function type_is_borrowed_str() you could you !type_is_immediate(), or better yet the helper load_if_immediate().

@Vincent-Belliard
Copy link
Contributor Author

I use now load_if_immediate. It was typically the kind of function I was looking for and why I asked you to validate the commit.
I rebased everything on incoming.

catamorphism added a commit that referenced this pull request Sep 5, 2012
@catamorphism catamorphism merged commit 9db4445 into rust-lang:incoming Sep 5, 2012
@catamorphism
Copy link
Contributor

Looks good, merged; thanks!

RalfJung pushed a commit to RalfJung/rust that referenced this pull request Feb 25, 2024
Windows miri-script execution egronomics

This allows for Windows users to use miri-script without pain. As working on miri earlier I was doing
`.\miri-script\target\debug\miri-script.exe { install | build | ... }` which wasn't fun.
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.

4 participants