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

rust-call call ABI is not feature gated at call-sites #34901

Open
pnkfelix opened this issue Jul 18, 2016 · 7 comments
Open

rust-call call ABI is not feature gated at call-sites #34901

pnkfelix opened this issue Jul 18, 2016 · 7 comments
Assignees
Labels
A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. C-bug Category: This is a bug. F-unboxed_closures `#![feature(unboxed_closures)]` P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

Note: This may be a feature, not a bug. But I wanted to make the point explicit.

If I make a crate named defs with this:

#![feature(unboxed_closures)]
pub extern "rust-call" fn foo(_: (), _: ()) { println!("defs::foo"); }

and then a bin crate uses with a dependency on defs and this for its main.rs:

extern crate defs;
fn main() {
    println!("Hello");
    defs::foo((), ());
    println!("world!");

}

Compiling the former obviously provides the feature gate it needs to define something with the rust-call ABI.

But compiling the latter does not require any feature gate, even though it is calling something via the rust-call ABI which is not stable.

This may be fine, since the point of origin is gated (apart from problems like #34900). But it seems worth discussion.

@pnkfelix pnkfelix added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 18, 2016
@nikomatsakis
Copy link
Contributor

Conclusion: this seems like we ought to feature gate the call sites (but not a big deal ultimately).

@Mark-Simulacrum Mark-Simulacrum added A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. C-bug Category: This is a bug. and removed I-papercut labels Jul 25, 2017
@pnkfelix
Copy link
Member Author

pnkfelix commented Jun 3, 2019

I want to fix this. The longer we leave holes like this open, the worst it will be when we fix it.

triage: P-high. Assigning to self.

@pnkfelix pnkfelix self-assigned this Jun 3, 2019
@pnkfelix pnkfelix added the P-high High priority label Jun 3, 2019
@nikomatsakis
Copy link
Contributor

The real danger would be if you could invoke rust-call things on stable. I think the main danger vector would be Fn::call , but the following is (separately) unstable for me, not sure how one would do it:

fn main() {
    let x = || ();
    Fn::call(&x, ());
}

@cramertj
Copy link
Member

cramertj commented Jun 6, 2019

I'm confused as to why this is a problem. There's no stability hole here, since it's impossible to define functions with this ABI on stable. Just in the same way that it is possible to use a proc macro that uses #![feature(proc_macro_hygiene)] without using the feature gate yourself, or to use a function written using a library feature that you have not enabled in your own crate, I would not expect that callers of a function defined using an unstable ABI feature would need to use #![feature(...)].

If there are particular APIs we'd like to remain unstable to call directly (such as the Fn ones) then we should certainly have feature gates for those (and, to my knowledge, we do).

@nikomatsakis
Copy link
Contributor

(Not to say we shouldn't require a feature gate at call sites, I dont' have a problem with that)

@cramertj
Copy link
Member

cramertj commented Jun 6, 2019

@nikomatsakis yeah, sorry, to clarify: I don't think it's wrong to feature-gate call sites, I just don't think it's a serious bug to not feature-gate call sites.

@Centril Centril added P-medium Medium priority and removed P-high High priority labels Jun 16, 2019
@Centril
Copy link
Contributor

Centril commented Jun 16, 2019

We discussed this on Thursday's language team meeting. The conclusion was that if @pnkfelix wants to make a PR to fix this then @nikomatsakis would be happy to accept it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stability Area: issues related to #[stable] and #[unstable] attributes themselves. C-bug Category: This is a bug. F-unboxed_closures `#![feature(unboxed_closures)]` P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants