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

RFC: Keeping a stack of expressions in trans #13175

Closed
emberian opened this issue Mar 28, 2014 · 10 comments
Closed

RFC: Keeping a stack of expressions in trans #13175

emberian opened this issue Mar 28, 2014 · 10 comments

Comments

@emberian
Copy link
Member

For #13146, adding the lint is easy, but getting a span to a useful place is very difficult. I propose we keep a stack of expressions being translated by pushing the ast::Expr to a task-local stack in expr::trans and expr::trans_into. Then trans_intrinsic could look up that stack for an expression with a useful span, and could even print out everything (including type params) that led to the substitution failure. I think this will be helpful if rust-lang/rfcs#8 gets merged, too.

@emberian emberian added the B-RFC label Mar 28, 2014
@emberian
Copy link
Member Author

I'm not just implementing this because I'm not sure this is the best course of action, or very clean. cc @pcwalton @nikomatsakis @alexcrichton @brson

@eddyb
Copy link
Member

eddyb commented Mar 28, 2014

Don't we already get an expression ID in trans_intrinsic? ref_id IIRC.

@emberian
Copy link
Member Author

It's garbage, it points to the call to the intrinsic, which is in std::cast

@emberian
Copy link
Member Author

(This is what causes bad spans on invalid transmutes)

@eddyb
Copy link
Member

eddyb commented Mar 28, 2014

As we've discussed in IRC, a monomorphization stack would be better, producing errors not unlike C++ ones, e.g. invalid transmute (from std::cast::transmute<&T, &mut T> from my_transmute<&mut T> from foobar)

But I'd much prefer transmute<T, U: TransmutableFrom<T>>, with type-level enforcement, instead of trans errors.

@huonw
Copy link
Member

huonw commented Mar 28, 2014

@eddyb #12898

@nikomatsakis
Copy link
Contributor

I'm confused as to the purpose of this. If the purpose is just to identify spans on monomorphized or inlined content, isn't that better addressed by implementing the serialization and deserialization of spans? (#1972) (Well, maybe not, that could be an expensive proposition)

That said, I think our general goal is to move the transmute checking out of trans and into typeck, precisely because we want to be sure monomorphization will succeed as a general principle. In which case, I would expect this lint to also operate on the AST pre-monomorphization, and not during trans. This does reduce its usefulness somewhat, but that's why it's a lint after all.

@huonw
Copy link
Member

huonw commented Mar 28, 2014

@nikomatsakis seeing spans on the inlined/monomorphised items themselves isn't so useful, it's the actual generic-instantiation site that is relevant, i.e. the user doesn't want know that the intrinsics::transmute call inside std::cast::transmute failed to instantiate, but rather some parameterisation they specified at a higher level broke things. (Keeping the current approach, I like @eddyb's idea of a monorphisation stack, showing every place that specifies type parameters.)

But... moving the checks into typeck is by far the best strategy.

@nikomatsakis
Copy link
Contributor

@huonw I think both are important, but I agree that knowing the local site isn't as useful without the stack of instantiations.

@steveklabnik
Copy link
Member

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#681

JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Sep 6, 2022
Clarify the state of (extern) preludes for block def maps
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

5 participants