Giter VIP home page Giter VIP logo

Comments (10)

Noratrieb avatar Noratrieb commented on August 30, 2024

@petrochenkov do you know whether this is possible? (I suspect the people in the compiler meeting will not know)

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

@Nilstrieb
What exactly, removing the "Cell is ambiguous" error?

It is a part of the "time travel prevention" machinery in macro/import resolution that ensures that resolution results are independent of things like order of items in the crate, or some specific expansion order used by the compiler.
The issue is not related to derive specifically, it would also appear if enum Cell were emitted by any other macro.

In the specific example above, I don't think the error can be avoided.
The use of Cell (use Cell::A) does materialize earlier than its definition (enum Cell) emitted by the derive macro, so we must restrict shadowing.

from rust.

joshtriplett avatar joshtriplett commented on August 30, 2024

@petrochenkov I really appreciate the pointer to that writeup, thank you. I think I understand why we restrict shadowing in that case.

So, if I'm understanding correctly, the ordering problem is roughly:

  1. We encounter Cell in the prelude.
  2. We encounter use Cell::A;.
  3. We encounter a macro resolution (for the derive).
  4. In this first pass, we tentatively assume the use Cell::A; will refer to the Cell we've seen, from the prelude.
  5. Also in the first pass, we expand the macro, which produces a definition of Cell, which would shadow the one in the prelude.
  6. In a second pass, we notice that use Cell::A; could refer to the definition just produced by the macro expansion in the first pass, so we error.

Is that an accurate description?

And, in the case where we don't have a derive, both definitions come from the first pass, so we allow one to shadow the other?

I understand that we can't fix the fully general case of that. Would it be possible to mitigate this issue in the common case of a derive? We know that a derive (even a proc-macro derive) applied to a top-level item X can't change that top-level item, it can only add additional things. So, given that, could we arrange to know in the first pass that we have a Cell, even if it has a derive attached to it? That would then let us fall into the same case as if we didn't have the proc macro, where all the names showed up in the first pass alongside the use.

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

Is that an accurate description?

Yes.
When we see use Cell::A we have two choices

  • either to use the definition of Cell that we see right now (the prelude one), even if it can be potentially shadowed later
    • that's what we do right now
    • if the name does gets shadowed some time later, after expanding an arbitrary number of macros, then we produce a restricted shadowing error, like one in the example above
  • or block and wait until all macros are expanded and we are sure that it certainly won't be shadowed
    • this is not a viable choice for resolution in "current scope", because it will make resolution stuck pretty much always
    • however, it's a viable choice when resolving in a specific module (like specific_module::Cell), then we can block and wait instead of possibly producing restricted shadowing errors later

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

Would it be possible to mitigate this issue in the common case of a derive?

Only if derive is turned from a macro into a built-in syntax.
Basically, the main requirement for this is that derive should not go through name resolution.

  • If on seeing #[derive] we immediately know that it is indeed the well known https://doc.rust-lang.org/stable/std/prelude/rust_2024/attr.derive.html, then we can immediately expand it and produce enum Cell in the "first pass".
  • If we need to go and find derive in std::prelude::rust_2024 where it normally resides, or in some other place if it's shadowed, then it can potentially send the derive's expansion to "second/third/etc pass" even if it ends up being the standard well known derive eventually.

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

It may be possible to amend macro expansion algorithm with "immediate expansion" attempts.

If we see a macro invocation (that includes #[derive]) and an immediate attempt at resolving it succeeds, then we immediately expand it as well, instead of putting it into the usual queue.
This should lift some restrictions from code emitted by such macro, including shadowing restrictions like in the example above.

In practice most derive invocations will be able to be promoted to "immediate expansions".

Not sure how viable this idea is, someone needs to try prototyping.

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

If ... an immediate attempt at resolving it succeeds

On a second thought, results of a probing like this are going depend on the specific expansion order, and that's what we are trying to avoid now.

from rust.

joshtriplett avatar joshtriplett commented on August 30, 2024

@petrochenkov

Would it be possible to mitigate this issue in the common case of a derive?

Only if derive is turned from a macro into a built-in syntax. Basically, the main requirement for this is that derive should not go through name resolution.

That sounds potentially feasible, and seems unlikely to create conflicts. I'd be genuinely surprised if anything in the ecosystem is overriding the name derive.

from rust.

petrochenkov avatar petrochenkov commented on August 30, 2024

I've re-read the code in compiler\rustc_builtin_macros\src\derive.rs and fn resolve_derives.

#[derive(Foo, Bar)] enum Cell { ... } doesn't emit enum Cell { ... } immediately, it resolves Foo and Bar first, which may delay it to a "second pass".
It's necessary to resolve derive helper attributes in enum Cell { ... } correctly, see comments in fn resolve_derives.

In general I'd personally not want to go this way, the "macrofication" of derive fixed quite a number of issues, I wouldn't want to reintroduce them back or workaround them with hacks.

What is the root motivation for this in the first place?
There are few names that are used universally enough to be added to prelude, and extending the prelude with new names is a major decision.
It's quite reasonable for it to happen once in 3 years on an edition bump.

from rust.

joshtriplett avatar joshtriplett commented on August 30, 2024

In general I'd personally not want to go this way, the "macrofication" of derive fixed quite a number of issues, I wouldn't want to reintroduce them back or workaround them with hacks.

I wasn't aware of that; thanks for that context.

Is there no reasonable low-friction way to keep the macro-based derive while teaching the compiler that that derive will definitely end up emitting the named item (e.g. a derive on Cell will definitely produce a Cell)?

If that would still be too much of a hack, we can work around it. In any case I appreciate you considering the possibilities, thank you.

What is the root motivation for this in the first place?
There are few names that are used universally enough to be added to prelude, and extending the prelude with new names is a major decision.
It's quite reasonable for it to happen once in 3 years on an edition bump.

The point is to make it less of a major decision. There are many names that we may want to add to the prelude, and if it's possible to do so incrementally without waiting for an edition that would make it a smaller impact. It'd also reduce the amount of burden at edition time; anything we can make not require an edition means the edition becomes less work, and we've already had long conversations about how to make the edition less work.

Even with this limitation, we can still potentially add prelude types without an edition, if we do a crater run. And the fact that the conflict only occurs with the combination of a derive and an import of a macro variant means it's unlikely to arise often.

I'm trying to find out if there's any reasonable step we can take to further reduce the likelihood of such problems. If not, then we can document this limitation and work around it with additional care when extending the prelude.

from rust.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.