Rust: [Edition vNext] Consider deprecating weird nesting of items

Created on 17 Oct 2019  Â·  11Comments  Â·  Source: rust-lang/rust

For example, do we really want mods inside random function? Similarly, IIRC https://youtu.be/LIYkT3p5gTs mentioned that it's unfortunate for incremental that we allow impls of random things inside functions.

Though at the same time, it's nice to allow all sorts of things for how ?eval bots work on Discord and IRC -- just put all the code inside a block that's passed to println! and it just works.

T-lang lang-team-202x-edition

Most helpful comment

(Note it can be nice, however, to be able to define types locally within functions and then attach impls to those types...)

All 11 comments

(Note it can be nice, however, to be able to define types locally within functions and then attach impls to those types...)

If we decide to spend time investigating this, it'd be great to also consider IDE support case.

IIRC (and somewhat oversimplifying) we can introduce impl blocks (and trait impls) in nested scoped that affect the outer scope, which means that the local information is not enough to reason about associated consts/methods of a given type (to simplify this somehow) and we potentially need to scan the entire crate to do that.

@matklad I think this is related to "quick parse" that we talked about which could also skip parsing the entire function bodies, right?

This approach, if feasible at all and if it doesn't reasonably reduce expressiveness, could potentially speed up some name/type resolution.

Though at the same time, it's nice to allow all sorts of things for how ?eval bots work on Discord and IRC

It’s also nice in cases like evcxr, which provides a repl and jupyter notebook support for rust. plotters uses evcxr in some of its examples https://plotters-rs.github.io/plotters-doc-data/evcxr-jupyter-integration.html

unfortunate for incremental

It’s bad for laziness, not for incremental.

The property we want here for IDE is:

Function body can be type-checked without looking at other functions’ bodies

That is, it’s not the nesting itself that creates troubles, but the fact that nested items are sometimes observable from the outside. Here’s the list of things that cause trouble that I know of:

  • local inherent impls
  • local impls of global traits for global types
  • local macro_export ed macros

The firsts two can be fixed by treating item bodies as downstream crates from coherence pov.

Additionally, another trouble some feature for ide is

  • local mod declarations (mod foo;, with semicolon)

IDE gets requests like “what’s the definition of thing at offset 92 in file foo.rs”. The first thing IDE needs to do is to figure out, from the set of known crate roots, how foo.rs fits into the module system. At the moment, to do this precisely one has to look into the body of every function, precisely because a mod declaration might be hidden there. Note that we currently require an explicit path attribute for local module, but that doesn’t really help, as you need still need to parse everything to find out there isn’t a module with an attr.

Note also that we would still need to parse some function bodies due to autotrait leakage via impl trait, but only if you directly use that function. This is different from having to parse all function bodies because somewhere and odd impl might be hidden, which affects type inference.

Beyond function bodies, it's currently permitted inside const bodies, including const generics.
For a hopefully unrealistic example:

#![feature(const_generics)]

struct Foo<const T: usize>;
struct Bar;

const C: usize = {
    impl Bar { const fn a() -> usize { 42 } };
    Bar::a() + Bar::b()
};

impl Foo<{
    mod m { impl super::Bar { pub const fn b() -> usize { 42 } } }
    C
}> {}

imo being able to impl a trait on a struct defined in the same scope is a think i would want to keep. my main example for it would be a serde use case

#[derive(Serialize, Debug, Eq, PartialEq, Clone)]
#[serde(untagged)]
pub enum StringOrNumber {
    String(String),
    Number(u64),
}

impl<'de> de::Deserialize<'de> for StringOrNumber {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: de::Deserializer<'de>,
    {
        struct Visitor;

        impl<'de> de::Visitor<'de> for Visitor {
            type Value = StringOrNumber;

            /** .... further Visitor impl here **/
        }

        deserializer.deserialize_any(Visitor)
    }
}

it allows one to implement the visitor for a struct, without exposing it to the scope around it.

I think we should keep the ability to do this.

Given that there are already four comments saying "we should allow impls for types that are themselves nested", I feel it would be useful to repeat:

  1. Obviously yes
  2. The precise rule we want is "function body acts as a downstream crate from coherence point of view for trait impls; for inherent impls, only private visibility (scoped to block/function) is allowed".

EDIT: I think I first heard the coherence formulation from @vlad20012.

To give an example on the case of trait impls:

// very useful, not problematic for lazy compilation. keep this
pub fn f() {
    struct S;
    impl Trait for S {}
}
// bad, also not usually useful. do not keep
pub struct S;
pub fn f() {
    impl Trait for S {}
}

Another slightly problematic thing (again by vlad20012 ) are inner function attributes:

fn foo() {
    #![cfg(unix)]
}

fn main() {
  foo()
}

Strictly speaking, we can live with that, as it only requires bounded parsing of the prefix of the function. But not having that seems better.

Was this page helpful?
0 / 5 - 0 ratings