Skip to content

pprust::print_if does not anticipate evil #652

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
jruderman opened this issue Jul 10, 2011 · 11 comments
Closed

pprust::print_if does not anticipate evil #652

jruderman opened this issue Jul 10, 2011 · 11 comments

Comments

@jruderman
Copy link
Contributor

If my fuzzer replaces the 'els' node with something other than an expr_if or expr_block, I get:

upcall fail 'non-exhaustive match failure', ../src/comp/syntax/print/pprust.rs:636

@marijnh
Copy link
Contributor

marijnh commented Jul 10, 2011

You'll get that in several other places. Our ast structure accepts only those two forms in that position. Though other things are representable, they aren't proper rust asts.

@jruderman
Copy link
Contributor Author

Maybe a
case(_) { fail "Invalid AST" }
would help the fuzzer distinguish between this kind of thing and actual bugs in the pretty-printer.

@marijnh
Copy link
Contributor

marijnh commented Jul 11, 2011

I'd recommend rigging the fuzzer to simply not output such ASTs. You probably already have similar code to ensure only lvalues are output as the lhs of assign/move expressions.

@pcwalton
Copy link
Contributor

How about just adding a constraint to the AST? We have refinement types after all (or are supposed to) :)

@brson
Copy link
Contributor

brson commented Jul 11, 2011

Yeah, we should start adding constraints where it makes sense

@jruderman
Copy link
Contributor Author

Will that change the fold signature, or will it make fold do a check/fail?

@brson
Copy link
Contributor

brson commented Jul 12, 2011

It will make it impossible to construct an expr_if that contains the wrong kind of else expression. I think in practice the code that wants to do such a think would be forced to do an

if check (is_else_expr(e)) {
  ... build my expr_if ...
} else {
  ... try something different ...
}

@brson
Copy link
Contributor

brson commented Jul 12, 2011

I realize that that didn't actually answer your question. I don't think it would change the fold signature, but would change the code that uses the fold.

@catamorphism
Copy link
Contributor

We don't have constrained types, so "add a constraint to the AST" won't work yet. It's my next to-do, though!

@ghost ghost assigned catamorphism Jul 12, 2011
@catamorphism
Copy link
Contributor

(Assigning to myself so I can use this as a test case for constrained types once they're implemented)

@nikomatsakis
Copy link
Contributor

Not relevant as of commit 41a21f0, which removed current incarnation of typestate.

keeperofdakeys pushed a commit to keeperofdakeys/rust that referenced this issue Dec 12, 2017
Clean up CI configuration and add s390x

We can't test s390x because qemu segfaults but we can at least verify that it
compiles.

Closes rust-lang#650
pdietl pushed a commit to pdietl/rust that referenced this issue Apr 23, 2020
Correct errors in the reference of extern functions definitions and declarations
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
antoyo added a commit to antoyo/rust that referenced this issue Apr 25, 2025
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

No branches or pull requests

6 participants