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

Patterns in extern fns either ICE or aren't type-checked #10877

Closed
huonw opened this issue Dec 9, 2013 · 10 comments · Fixed by #12715
Closed

Patterns in extern fns either ICE or aren't type-checked #10877

huonw opened this issue Dec 9, 2013 · 10 comments · Fixed by #12715
Labels
A-FFI Area: Foreign function interface (FFI) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Milestone

Comments

@huonw
Copy link
Member

huonw commented Dec 9, 2013

The following compiles fine, but uncommenting any of the currently-commented extern functions makes the compiler ICE.

struct Foo { x: int }
extern {
    // fn foo(1: ());
    // fn bar((): int);
    // fn baz(Foo { x }: int);
    fn qux((x,y): ());
}
fn main() {}

For foo the ICE message is

error: internal compiler error: node_id_to_type: no type for node `expr 1 (id=12)`

and the others are similar.

@alexcrichton
Copy link
Member

I'm inclined to say that the correct behavior is to deny all patterns other than ident (give a parser error perhaps).

@alexcrichton
Copy link
Member

Nominating.

@nikomatsakis
Copy link
Contributor

@alexcrichton I agree

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Accepted for 1.0, P-backcompat-lang.

@brson
Copy link
Contributor

brson commented Jan 18, 2014

Good newbie bug.

@mankyKitty
Copy link
Contributor

Being a good newbie, I'll have a crack at this one. :)

@mankyKitty
Copy link
Contributor

Might have aimed a little too high by trying this issue.. but from what i understand, the type checking is working as people would like, but the message is incorrect and should be a actual syntax error and not an ICE... so the below is some thing like how it should work?

struct Foo { x: int }

extern {
    // This one should fail because you've supplied a literal as a fn arg with
    // no type
    fn foo(1: ());
    // This one should fail because you've specified '()' as a variable name for
    // the fn args
    fn bar((): int);
    // This one should pass because you've declared a deconstructed variable name
    // for the input parm 'x' and you've declared a type. This still should trigger
    // the warning for not using a 'libc::c_int'.
    fn baz(Foo {x}: int);
    // This should pass because you've specified input parameters and rust will 
    // infer the type?
    fn qux((x,y): ());
}

Sorry if I've completely misunderstood, still pretty nubbins with Rust. :\

@pnkfelix
Copy link
Member

@mankyKitty my reading of acrichto's comment is that all of your examples should fail; ideally only an identfier should pass.

@huonw
Copy link
Member Author

huonw commented Jan 25, 2014

They should all fail, the only thing that should pass is fn foo(some_ident: int);, nothing involving destructuring or pattern matching.

One way to implement this would be to make it an actual syntax error. You can find a starting place for this change by grepping for ForeignItemFn in src/libsyntax/parse/parser.rs. (I personally would recommend implementing this by threading a require_ident_patterns_only argument through parse_fn_decl, parse_fn_args to parse_arg_general where you chose between the existing parse_pat call and calling parse_pat_ident directly based on that boolean.)

// This should pass because you've specified input parameters and rust will 
// infer the type?
fn qux((x,y): ());

No inference; this is always incorrect because you're trying to match a 2-tuple pattern (i.e. type (T, U)) in a slot of type () (i.e. nil, void, 0-tuple).

@mankyKitty
Copy link
Contributor

@pnkfelix @huonw

Ahhh okay, that makes more sense. Thanks for that. I'll see what I can make happen.

mankyKitty added a commit to mankyKitty/rust that referenced this issue Jan 25, 2014
Functions declared in an 'extern' were not properly typed checked and
will report syntax errors on failure instead of an ICE.

Fixes rust-lang#10877
bors added a commit that referenced this issue Mar 5, 2014
Fixes #10877

There was another PR which attempted to fix this in the parser (#11804) and which was closed due to inactivity.
This PR modifies typeck instead (as suggested in #11804), which indeed seems to be simpler than modifying the parser and allows for a better error message.
@bors bors closed this as completed in 53f3442 Mar 5, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 26, 2024
[`new_without_default`]: Now emits on const fns

While `Default::default` is not const, it can still call `const new`; there's no reason this shouldn't be linted as well.

fixes rust-lang#10877

changelog: [`new_without_default`]: Now emits on const fns
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
6 participants