Skip to content

rustc crashes when attempting to match a &str #3222

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

Closed
Dretch opened this issue Aug 18, 2012 · 6 comments
Closed

rustc crashes when attempting to match a &str #3222

Dretch opened this issue Aug 18, 2012 · 6 comments
Labels
A-codegen Area: Code generation I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Milestone

Comments

@Dretch
Copy link
Contributor

Dretch commented Aug 18, 2012

Given this test case:

fn f(s: &str) {
    match s {
        "a" => (),
         _  => (),
    }
}

fn main() {}

rustc fails with:

rustc: /home/gareth/projects/rust/src/llvm/lib/VMCore/Instructions.cpp:2279: static llvm::CastInst* llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
Aborted (core dumped)

I have no idea if this code should work or not. If it shouldn't work, I think rustc should emit a nicer error message.

@ghost ghost assigned catamorphism Aug 31, 2012
@catamorphism
Copy link
Contributor

#3316 (since it's not obvious what pull request the commit is from).

@nikomatsakis , @brson said you had reservations about the pull request? I'm not even sure if the code should compile or not.

@Vincent-Belliard
Copy link
Contributor

About the code: I think it must compile. The following code compile and execute well:


fn f(s: &str) {
    if s == "a" {}
}

You must be able to match a borrowed string with constant. Of course, the syntax may be different but, first, this one is consistent with a if test, second, the syntax is understood by the compiler, only the generation has a problem.

@Vincent-Belliard
Copy link
Contributor

About the reservations: I'm absolutely sure that my analysis of the problem is good and that what I did will work. However, there are two problems.
A first minor one is that I had a test in compile_submatch to see if the match expression is or is not a borrowed string. It's not orthogonal but it works.
A second more serious problem is that I substitute:
rslt(bcx, consts::const_expr(ccx, l))
by
trans_temp_expr(bcx, l)
I did that to be consistent with a if test. However, I think that the generated LLVM bytecode is slightly different from the previous one for other cases (for example when you match unique strings). I wonder if the solution could be to modify const_expr instead of calling trans_temp_expr.
For someone which didn't write the LLVM generation it's quite difficult to be sure to write something consistent.
The major problem is that when you infer the abstract syntax tree, you didn't generate an inferred tree. Doing this, some of the job did during the inference must be done again during the LLVM generation: you must test a lot of special cases when you generate. That's why, for someone who discover your LLVM generation, it's quite difficult to be sure that a patch is consistent with the former code.

@Vincent-Belliard
Copy link
Contributor

About the strings: during one week, I really wonder why you used such a syntax. I finally understood that strings can be modified. Even if the characters are not mutable, a string can grow. I think that most the problems come from that. Most of the time, a string is never modified. You can concatenate strings, take sub strings, ... Each time you generate a new string and you don't modify existing strings. In Java, you have the StringBuffer class to build dynamically a string. For all other cases, you use a String which is constant.

Now, the string syntax is not orthogonal nor consistent. If you write:
@int
you define a shared box which held an integer but if you write:
@str
you define a shared string. The integer can be deferenced but not the string. It's not easy to use and, most of all, it's not orthogonal.
I think that a good solution should be to define a new type: str which would hold a constant string and would replace most of the ~str, @str and &str. Then you could add an other new type str_buf (or something like that). This type would nearly behave like a [mut u8]. The only difference would be that when you add one UTF8 character, several bytes would be added. The other advantage of this is that the generated LLVM code would be more efficient.
I insist a lot because Rust is very well designed. It may be the only new language I would like to use but only if the string model become consistent.

With a consistent string syntax, this issue may has never appeared.

Vincent-Belliard added a commit to Vincent-Belliard/rust that referenced this issue Sep 5, 2012
@pcwalton
Copy link
Contributor

pcwalton commented Sep 5, 2012

The reason why @str cannot be dereferenced is that str is dynamically sized. If we allowed strings to be copied to the stack like ints can, then we'd have to add dynamic allocas and that would break the segmented stacks model. Other than that, @int and @str are intended to be conceptually very similar.

Your proposal would basically rename str as it exists today to str_buf and make it mutable. Remember, [mut u8] is also a dynamically sized type, so it's really ~[mut u8], @[mut u8], and &[mut u8]; thus there would be three str_bufs, ~str_buf, @str_buf, and &str_buf. Your new type str would be equivalent to &static/str today. All of this would add complexity to the language for little gain, as I see it. I also don't see how the generated code would be more efficient; adding to a ~[mut u8] is exactly the same operation as adding to a ~str.

That said, you're right that building up a unique string by appending to it is confusing. We should add a StringBuffer type to the standard library.

@catamorphism
Copy link
Contributor

#3316 closes this. If you want to suggest additional enhancements to strings, please open a separate issue for it :-) (And thanks for the patch!)

bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
fix the visitor's starting position when visiting a labelled block
RalfJung pushed a commit to RalfJung/rust that referenced this issue Dec 17, 2023
tests: use Waker::noop instead of defining our own Waker
jaisnan pushed a commit to jaisnan/rust-dev that referenced this issue Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

4 participants