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

Inferred types _::Enum #3444

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

JoshuaBrest
Copy link

@JoshuaBrest JoshuaBrest commented Jun 7, 2023

This RFC is all about allowing types to be inferred without any compromises. The syntax is as follows. For additional information, please read the bellow.

struct MyStruct {
    value: usize
}

fn my_func(data: MyStruct) { /* ... */ }

my_func(_ {
    value: 0
});

I think this is a much better and more concise syntax.

If you plan on pressing the dislike button, please leave a comment explaining your disproval. Every piece of constructive feedback helps.

Rendered

@JoshuaBrest JoshuaBrest changed the title Infered enums Infered types Jun 7, 2023
@Lokathor
Copy link
Contributor

Lokathor commented Jun 7, 2023

I'm not necessarily against the RFC, but the motivation and the RFC's change seem completely separate.

I don't understand how "people have to import too many things to make serious projects" leads to "and now _::new() can have a type inferred by the enclosing expression".

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jun 7, 2023
@JoshuaBrest
Copy link
Author

I don't understand how "people have to import too many things to make serious projects" leads to "and now _::new() can have a type inferred by the enclosing expression".

In crates like windows-rs even in the examples, they import *. This doesn't seem like good practice and with this feature, I hope to avoid it.

use windows::{
    core::*, Data::Xml::Dom::*, Win32::Foundation::*, Win32::System::Threading::*,
    Win32::UI::WindowsAndMessaging::*,
};

@Lokathor
Copy link
Contributor

Lokathor commented Jun 7, 2023

Even assuming I agreed that's bad practice (which, I don't), it is not clear how that motivation has lead to this proposed change.

@JoshuaBrest
Copy link
Author

Even assuming I agreed that's bad practice (which, I don't), it is not clear how that motivation has lead to this proposed change.

How can I make this RFC more convincing? I am really new to this and seeing as you are a contributor I would like to ask for your help.

@Lokathor
Copy link
Contributor

Lokathor commented Jun 7, 2023

First, I'm not actually on any team officially, so please don't take my comments with too much weight.

That said:

  • the problem is that you don't like glob imports.
  • glob imports are usually done because listing every item individually is too big of a list, or is just annoying to do.
  • I would expect the solution to somehow be related to the import system. Instead you've expanded how inference works.

Here's my question: Is your thinking that an expansion of inference will let people import less types, and then that would cause them to use glob imports less?

Assuming yes, well this inference change wouldn't make me glob import less. I like the glob imports. I want to write it once and just "make the compiler stop bugging me" about something that frankly always feels unimportant. I know it's obviously not actually unimportant but it feels unimportant to stop and tell the compiler silly details over and over.

Even if the user doesn't have to import as many types they still have to import all the functions, so if we're assuming that "too many imports" is the problem and that reducing the number below some unknown threshold will make people not use glob imports, I'm not sure this change reduces the number of imports below that magic threshold. Because for me the threshold can be as low as two items. If I'm adding a second item from the same module and I think I might ever want a third from the same place I'll just make it a glob.

Is the problem with glob imports that they're not explicit enough about where things come from? Because if the type of _::new() is inferred, whatever the type of the _ is it still won't show up in the imports at the top of the file. So you still don't know specifically where it comes from, and now you don't even know the type's name so you can't search it in the generated rustdoc.

I hope this isn't too harsh all at once, and I think more inference might be good, but I'm just not clear what your line of reasoning is about how the problem leads to this specific solution.

@JoshuaBrest
Copy link
Author

Is your thinking that an expansion of inference will let people import less types, and then that would cause them to use glob imports less?

Part of it yes, but, I sometimes get really frustrated that I keep having to specify types and that simple things like match statements require me to sepcigy the type every single time.

whatever the type of the _ is it still won't show up in the imports at the top of the file. So you still don't know specifically where it comes from, and now you don't even know the type's name so you can't search it in the generated rustdoc.

Its imported in the background. Although we don't need the exact path, the compiler knows and it can be listed in the rust doc.

I hope this isn't too harsh all at once, and I think more inference might be good, but I'm just not clear what your line of reasoning is about how the problem leads to this specific solution.

Definitely not, you point out some great points and your constructive feedback is welcome.

@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 7, 2023

Personally _::new() and _::Variant "look wrong" to me although i cant tell why, I would expect <_>::new() and <_>::Variant to be the syntax, no suggestion on _ { ... } for struct exprs which tbh also looks wrong but <_> { ... } isnt better and we dont even support <MyType> { field: expr }

text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
[unresolved-questions]: #unresolved-questions


A few kinks on this are whether it should be required to have the type in scope. Lots of people could point to traits and say that they should but others would disagree. From an individual standpoint, I don’t think it should require any imports but, it really depends on the implementers as personally, I am not an expert in *this* subject.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this question needs to be resolved before the RFC is landed, since it pretty drastically changes the implementation and behavior of the RFC.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to discuss it (:

text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
text/0000-infered-types.md Outdated Show resolved Hide resolved
@SOF3
Copy link

SOF3 commented Jun 12, 2023

I would like to suggest an alternative rigorous definition that satisfies the examples mentioned in the RFC (although not very intuitive imo):


When one of the following expression forms (set A) is encountered as the top-level expression in the following positions (set B), the _ token in the expression form should be treated as the type expected at the position.

Set A:

  • Path of the function call (e.g. _::function())
  • Path expression (e.g. _::EnumVariant)
  • When an expression in set A appears in a dot-call expression (expr.method())
  • When an expression in set A appears in a try expression (expr?)
  • When an expression in set A appears in an await expression (expr.await)

Set B:

  • A pattern, or a pattern option (delimited by |) in one of such patterns
  • A value in a function/method call argument list
  • The value of a field in a struct literal
  • The value of a value in an array/slice literal
  • An operand in a range literal (i.e. if an expression is known to be of type Range<T>, expr..expr can infer that both exprs are of type T)
  • The value used with return/break/yield

Set B only applies when the type of the expression at the position can be inferred without resolving the expression itself.


Note that this definition explicitly states that _ is the type expected at the position in set B, not the expression in set A. This means we don't try to infer from whether the result is actually feasible (e.g. if _::new() returns Result<MyStruct>, we still set _ as MyStruct and don't care whether new() actually returns MyStruct).

Set B does not involve macros. Whether this works for macros like vec![_::Expr] depends on the macro implementation and is not part of the spec (unless it is in the standard library).

Set A is a pretty arbitrary list for things that typically seem to want the expected type. We aren't really inferring anything in set A, just blind expansion based on the inference from set B. These lists will need to be constantly maintained and updated when new expression types/positions appear.

@JoshuaBrest
Copy link
Author

s (set A) is encountered as the top-level expression in the following positions (set B), the _ token in the expression form should be treated as the type expected at th

That is so useful! Let me fix it now.

@SOF3
Copy link

SOF3 commented Jun 12, 2023

One interesting quirk to think about (although unlikely):

fn foo<T: Default>(t: T) {}

foo(_::default())

should this be allowed? we are not dealing with type inference here, but more like "trait inference".

@JoshuaBrest
Copy link
Author

One interesting quirk to think about (although unlikely):

fn foo<T: Default>(t: T) {}

foo(_::default())

should this be allowed? we are not dealing with type inference here, but more like "trait inference".

I think you would have to specify the type arg on this one because Default is a trait and the type is not specific enough.

fn foo<T: Default>(t: T) {}

foo::<StructImplementingDefault>(_::default())

@SOF3
Copy link

SOF3 commented Jun 12, 2023

oh never mind, right, we don't really need to reference the trait directly either way.

text/0000-infered-types.md Outdated Show resolved Hide resolved
@clarfonthey
Copy link
Contributor

I've been putting off reading this RFC, and looking at the latest version, I can definitely feel like once the aesthetic arguments are put aside, the motivation isn't really there.

And honestly, it's a bit weird to me to realise how relatively okay I am with glob imports in Rust, considering how I often despise them in other languages like JavaScript. The main reason for this is that basically all of the tools in the Rust ecosystem directly interface with compiler internals one way or another, even if by reimplementing parts of the compiler in the case of rust-analyzer.

In the JS ecosystem, if you see a glob import, all hope is essentially lost. You can try and strip away all of the unreasonable ways of interfacing with names like eval but ultimately, unless you want to reimplement the module system yourself and do a lot of work, a person seeing a glob import knows as much as a machine reading it does. This isn't the case for Rust, and something like rust-analyzer will easily be able to tell what glob something is coming from.

So really, this is an aesthetic argument. And honestly… I don't think that importing everything by glob, or by name, is really that big a deal, especially with adequate tooling. Even renaming things.

Ultimately, I'm not super against this feature in principle. But I'm also not really sure if it's worth it. Rust's type inference is robust and I don't think it would run into technical issues, just… I don't really know if it's worth the effort.

@SOF3
Copy link

SOF3 commented Jun 12, 2023

@clarfonthey glob imports easily have name collision when using multiple globs in the same module. And it is really common with names like Context. Plus, libraries providing preludes do not necessarily have the awareness that adding to the prelude breaks BC.

@JoshuaBrest
Copy link
Author

And honestly, it's a bit weird to me to realise how relatively okay I am with glob imports in Rust, considering how I often despise them in other languages like JavaScript. The main reason for this is that basically all of the tools in the Rust ecosystem directly interface with compiler internals one way or another, even if by reimplementing parts of the compiler in the case of rust-analyzer.

In the JS ecosystem, if you see a glob import, all hope is essentially lost. You can try and strip away all of the unreasonable ways of interfacing with names like eval but ultimately, unless you want to reimplement the module system yourself and do a lot of work, a person seeing a glob import knows as much as a machine reading it does. This isn't the case for Rust, and something like rust-analyzer will easily be able to tell what glob something is coming from.

I can understand your point, but, when using large libraries in conjunction, like @SOF3 said, it can be easy to run into name collisions. I use actix and seaorm and they often have simular type names.

@JoshuaBrest
Copy link
Author

JoshuaBrest commented Jun 12, 2023

Personally _::new() and _::Variant "look wrong" to me although i cant tell why, I would expect <_>::new() and <_>::Variant to be the syntax, no suggestion on _ { ... } for struct exprs which tbh also looks wrong but <_> { ... } isnt better and we dont even support <MyType> { field: expr }

In my opinion, it's really annoying to type those set of keys. Using the QWERTY layout requires lots of hand movement. Additionally, it's syntax similar to what you mentioned has already been used to infer lifetimes, I am concerned people will confuse these.
Frame 1

@clarfonthey
Copy link
Contributor

Right, I should probably clarify my position--

I think that not liking globs is valid, but I also think that using globs is more viable in Rust than in other languages. Meaning, it's both easier to use globs successfully, and also easier to just import everything you need successfully. Rebinding is a bit harder, but still doable.

Since seeing how useful rust-analyzer is for lots of tasks, I've personally found that the best flows for these kinds of things involve a combination of auto-import and auto-complete. So, like mentioned, _ is probably a lot harder to type than the first letter or two of your type name plus whatever your auto-completion binding is (usually tab, but for me it's Ctrl-A).

Even if you're specifically scoping various types to modules since they conflict, that's still just the first letter of the module, autocomplete, two colons, the first letter of the type, autocomplete. Which may be more to type than _, but accomplishes the goal you need to accomplish.

My main opinion here is that _ as a type inference keyword seems… suited to a very niche set of aesthetics that I'm not sure is worth catering to. You don't want to glob-import, you don't want to have to type as much, but also auto-completing must be either too-much or not available. It's even not about brevity in some cases: for example, you mention cases where you're creating a struct inside a function which already has to be annotated with the type of the struct, which cannot be inferred, and therefore you're only really saving typing it once.

Like, I'm not convinced that this can't be better solved by improving APIs. Like, for example, you mentioned that types commonly in preludes for different crates used together often share names. I think that this is bad API design, personally, but maybe I'm just not getting it.

@programmerjake
Copy link
Member

I do think inferred types are useful when matching for brevity's sake:
e.g. in a RV32I emulator:

#[derive(Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd, Debug, Hash)]
pub struct Reg(pub Option<NonZeroU8>);

#[derive(Debug)]
pub struct Regs {
    pub pc: u32,
    pub regs: [u32; 31],
}

impl Regs {
    pub fn reg(&self, reg: Reg) -> u32 {
        reg.0.map_or(0, |reg| self.regs[reg.get() - 1])
    }
    pub fn set_reg(&mut self, reg: Reg, value: u32) {
        if let Some(reg) = reg {
            self.regs[reg.get() - 1] = value;
        }
    }
}

#[derive(Debug)]
pub struct Memory {
    bytes: Box<[u8]>,
}

impl Memory {
    pub fn read_bytes<const N: usize>(&self, mut addr: u32) -> [u8; N] {
        let mut retval = [0u8; N];
        for v in &mut retval {
            *v = self.bytes[addr.try_into().unwrap()];
            addr = addr.wrapping_add(1);
        }
        retval
    }
    pub fn write_bytes<const N: usize>(&mut self, mut addr: u32, bytes: [u8; N]) {
        for v in bytes {
            self.bytes[addr.try_into().unwrap()] = v;
            addr = addr.wrapping_add(1);
        }
    }
}

pub fn run_one_insn(regs: &mut Regs, mem: &mut Memory) {
    let insn = Insn::decode(u32::from_le_bytes(mem.read_bytes(regs.pc))).unwrap();
    match insn {
        _::RType(_ { rd, rs1, rs2, rest: _::Add }) => {
            regs.set_reg(rd, regs.reg(rs1).wrapping_add(regs.reg(rs2)));
        }
        _::RType(_ { rd, rs1, rs2, rest: _::Sub }) => {
            regs.set_reg(rd, regs.reg(rs1).wrapping_sub(regs.reg(rs2)));
        }
        _::RType(_ { rd, rs1, rs2, rest: _::Sll }) => {
            regs.set_reg(rd, regs.reg(rs1).wrapping_shl(regs.reg(rs2)));
        }
        _::RType(_ { rd, rs1, rs2, rest: _::Slt }) => {
            regs.set_reg(rd, ((regs.reg(rs1) as i32) < regs.reg(rs2) as i32) as u32);
        }
        _::RType(_ { rd, rs1, rs2, rest: _::Sltu }) => {
            regs.set_reg(rd, (regs.reg(rs1) < regs.reg(rs2)) as u32);
        }
        // ...
        _::IType(_ { rd, rs1, imm, rest: _::Jalr }) => {
            let pc = regs.reg(rs1).wrapping_add(imm as u32) & !1;
            regs.set_reg(rd, regs.pc.wrapping_add(4));
            regs.pc = pc;
            return;
        }
        _::IType(_ { rd, rs1, imm, rest: _::Lb }) => {
            let [v] = mem.read_bytes(regs.reg(rs1).wrapping_add(imm as u32));
            regs.set_reg(rd, v as i8 as u32);
        }
        _::IType(_ { rd, rs1, imm, rest: _::Lh }) => {
            let v = mem.read_bytes(regs.reg(rs1).wrapping_add(imm as u32));
            regs.set_reg(rd, i16::from_le_bytes(v) as u32);
        }
        _::IType(_ { rd, rs1, imm, rest: _::Lw }) => {
            let v = mem.read_bytes(regs.reg(rs1).wrapping_add(imm as u32));
            regs.set_reg(rd, u32::from_le_bytes(v));
        }
        // ...
    }
    regs.pc = regs.pc.wrapping_add(4);
}

pub enum Insn {
    RType(RTypeInsn),
    IType(ITypeInsn),
    SType(STypeInsn),
    BType(BTypeInsn),
    UType(UTypeInsn),
    JType(JTypeInsn),
}

impl Insn {
    pub fn decode(v: u32) -> Option<Self> {
        // ...
    }
}

pub struct RTypeInsn {
    pub rd: Reg,
    pub rs1: Reg,
    pub rs2: Reg,
    pub rest: RTypeInsnRest,
}

pub enum RTypeInsnRest {
    Add,
    Sub,
    Sll,
    Slt,
    Sltu,
    Xor,
    Srl,
    Sra,
    Or,
    And,
}


pub struct ITypeInsn {
    pub rd: Reg,
    pub rs1: Reg,
    pub imm: i16,
    pub rest: ITypeInsnRest,
}

pub enum ITypeInsnRest {
    Jalr,
    Lb,
    Lh,
    Lw,
    Lbu,
    Lhu,
    Addi,
    Slti,
    Sltiu,
    Xori,
    Ori,
    Andi,
    Slli,
    Srli,
    Srai,
    Fence,
    FenceTso,
    Pause,
    Ecall,
    Ebreak,
}
// rest of enums ...

@Aloso
Copy link

Aloso commented Jun 12, 2023

I do like type inference for struct literals and enum variants.

However, type inference for associated functions doesn't make sense to me. Given this example:

fn expect_foo(_: Foo) {}
foo(_::bar());
  • According to this RFC, the _ should be resolved to Foo (the function argument's type), but this isn't always correct. I suspect that this behavior is often useful in practice, but there are cases where it will fail, and people may find this confusing. For example, Box::pin returns a Pin<Box<T>>, so _::pin(x) couldn't possibly be inferred correctly.

  • Even when Foo has a bar function that returns Foo, there could be another type that also has a matching bar function. Then _ would be inferred as Foo, even though it is actually ambiguous.

  • Another commenter suggested that we could allow method calls after the inferred type (e.g. _::new().expect("..."), or _::builder().arg(42).build()?). But this still wouldn't help in a lot of cases, because methods often return a different type than Self (in contrast to associated functions, where Self is indeed the most common return type).

    For example, the _ in _::new(s).canonicalize()? can't be inferred as Path, because Path::canonicalize returns Option<PathBuf>.

  • Another issue is that it doesn't support auto-deref (e.g. when a function expects a &str and we pass &_::new() 1, which should be resolved as &String::new(), but that may be ambiguous).

All in all, it feels like this would add a lot of complexity and make the language less consistent and harder to learn.

Footnotes

  1. I realize this is a contrived example

@lygstate
Copy link

It is not a pattern in the sense of interchangeability with regular (value) patterns, because all other patterns are used to match values not types. The _ proposed here means an "inferred type" like how let s: Vec<_> works, not a "pattern" like how let _: Vec<()> works. The former is a type while the latter is a pattern.

Yeah, I think you’re right but, would it be better to call it a keyword then?

_ is already know as a placehodler, so better not use it, does ::: are occupied, I think here is about namespace/scope.
so ::: sounds make sense. As we already know :: is about namespace.

@JoshuaBrest
Copy link
Author

It is not a pattern in the sense of interchangeability with regular (value) patterns, because all other patterns are used to match values not types. The _ proposed here means an "inferred type" like how let s: Vec<_> works, not a "pattern" like how let _: Vec<()> works. The former is a type while the latter is a pattern.

Yeah, I think you’re right but, would it be better to call it a keyword then?

_ is already know as a placehodler, so better not use it, does ::: are occupied, I think here is about namespace/scope. so ::: sounds make sense. As we already know :: is about namespace.

I think it's not consistent and hard to read

@JoshuaBrest JoshuaBrest changed the title Inferred types Inferred types _::Enum Jul 12, 2023
@compiler-errors

This comment was marked as resolved.

@compiler-errors compiler-errors removed their request for review July 12, 2023 17:24
@chenyukang
Copy link
Member

chenyukang commented Jul 12, 2023

How is this

match x {
    _::Variant1 => ...,
    _::Variant2 => ...,
    _::Variant3 => ...,
    _::Variant4 => ...,
    _::Variant5 => ...,
    _::Variant6 => ...,
}

any better than this?

use Enum as E;
match x {
    E::Variant1 => ...,
    E::Variant2 => ...,
    E::Variant3 => ...,
    E::Variant4 => ...,
    E::Variant5 => ...,
    E::Variant6 => ...,
}

I can understand how it gets more readable when used in e.g. a function call that accepts many different structs, e.g.

fn_call(LongStructName1 {a: 1}, LongStructName2 {b: 2}, LongStructName3 {c: 3}, LongStructName4 {d: 4})
// vs
fn_call(_ {a: 1}, _ {b: 2}, _ {c: 3}, _ {d: 4})

but for the case of match arms, all patterns have the same enum type and you just need to add one line of alias-import.

@JoshuaBrest
What would happen if there are multiple crates containing the Enum::Variant1, we still need to import path, right?

This proposal may save some typing for the Enum pattern-matching scenario, but this kind of syntax _ { a: 1 } _::method() seems a bad idea for me, any other programming language use type inference for this? Why Swift limits it only to Enum and not extend it to Struct?

I think one obvious reason is this looks too implicit, when we read a snippet code like this, I'd wondering what's the concrete type here.

I prefer to use Target::*; to import more to solve the problem of importing.

@SOF3
Copy link

SOF3 commented Jul 13, 2023

I agree that the _::method() style is too implicit indeed, but _()/_ {} struct literals sound reasonable, especially when you compare it to TypeScript etc. This is particularly common in function calls that accept many struct parameters, e.g. those GUI libraries or APIs that require a lot of newtypes.

@JoshuaBrest
Copy link
Author

JoshuaBrest commented Jul 13, 2023 via email

@JoshuaBrest
Copy link
Author

I agree that the _::method() style is too implicit indeed, but _()/_ {} struct literals sound reasonable, especially when you compare it to TypeScript etc. This is particularly common in function calls that accept many struct parameters, e.g. those GUI libraries or APIs that require a lot of newtypes.

I think that even though it might be a tad too bit implicit, it is important because most structs are built with _::method().

@SOF3
Copy link

SOF3 commented Jul 17, 2023

A struct with all public fields often just means as much as a tuple with the same values, but a new() call usually does more than just containing the fields, e.g. initializing default parameters. I mentioned that _(x, y) is reasonable because it mostly means the same as (x, y) without any significant loss of clarity, but _::new(x, y) is too implicit because the returned value could contain more than just the (x, y) tuple.

@JoshuaBrest
Copy link
Author

A struct with all public fields often just means as much as a tuple with the same values, but a new() call usually does more than just containing the fields, e.g. initializing default parameters. I mentioned that _(x, y) is reasonable because it mostly means the same as (x, y) without any significant loss of clarity, but _::new(x, y) is too implicit because the returned value could contain more than just the (x, y) tuple.

Some structs have private fields so i think that it is important to allow this. However, if the general consensus is that this should be disallowed, I am sure that we can add this to the document.

@SOF3
Copy link

SOF3 commented Jul 18, 2023

That's exactly my point. Private fields imply that the value contains more than just the tuple, so it is important to indicate the type explicitly so that we know that private fields are involved. In fact, I would like to extend this rule to anything that implements Drop as well, but private fields are typically a necessary condition for Drop types anyway.

@JoshuaBrest
Copy link
Author

JoshuaBrest commented Jul 18, 2023

That's exactly my point. Private fields imply that the value contains more than just the tuple, so it is important to indicate the type explicitly so that we know that private fields are involved. In fact, I would like to extend this rule to anything that implements Drop as well, but private fields are typically a necessary condition for Drop types anyway.

That makes sense. I will add that to the RFC. On another note, I think that that might prevent bit flags from working using this syntax. I will need to look into this.

@JoshuaBrest
Copy link
Author

It's been updated.

@Fishrock123
Copy link
Contributor

I wrote a pre-RFC for this some time ago which does not seem to have been mentioned here.

https://internals.rust-lang.org/t/pre-rfc-inferred-enum-type/16100

You may want to check some of the discussion there and mention the that post in some regard.

@JoshuaBrest
Copy link
Author

I wrote a pre-RFC for this some time ago which does not seem to have been mentioned here.

https://internals.rust-lang.org/t/pre-rfc-inferred-enum-type/16100

You may want to check some of the discussion there and mention the that post in some regard.

Is there anything notable? I have read it and it looks like to me the same concerns there have been brought up here.

@kanashimia
Copy link

It seems there already was a RFC that proposed similar feature only for enums: #1949

@Fishrock123
Copy link
Contributor

Is there anything notable? I have read it and it looks like to me the same concerns there have been brought up here.

@JoshuaBrest this along with others would ideally be mentioned somewhere in the RFC "prior work" or such.

Co-authored-by: Josh Triplett <josh@joshtriplett.org>
@JoshuaBrest
Copy link
Author

It's been almost a year since I first opened this RFC. It still seems to need some work. As the academic year is coming to a close I finally have the time and motivation to make some progress here.

Thanks for all the feedback y'all.🤞

@JoshuaBrest
Copy link
Author

JoshuaBrest commented May 9, 2024

I want to (again) gauge what people feel about .Enum and _::Enum.

React with emojis below this comment.
👍 Swift-like syntax (.Enum)
👎 Verbose syntax (_::Enum)

@jjpe
Copy link

jjpe commented May 10, 2024

I have mixed feelings about this.
On the one hand, syntactically speaking, when writing code this can definitely be convenient.

On the other hand, I can see how this has the potential to really hurt code comprehensibility. Visible type names matter a lot for that I've noticed - it's one of many reasons that I prefer statically typed languages like Rust over untyped ones like Python and Javascript. Aside from the type checking features, the type names act as a form of documentation, and when elided that falls away, even if the type checking itself remains.

At the end of the day, comprehensibility needs to be prioritized over conveniences for authoring code, I think.

@JoshuaBrest
Copy link
Author

JoshuaBrest commented May 10, 2024

I have mixed feelings about this. On the one hand, syntactically speaking, when writing code this can definitely be convenient.

On the other hand, I can see how this has the potential to really hurt code comprehensibility. Visible type names matter a lot for that I've noticed - it's one of many reasons that I prefer statically typed languages like Rust over untyped ones like Python and Javascript. Aside from the type checking features, the type names act as a form of documentation, and when elided that falls away, even if the type checking itself remains.

At the end of the day, comprehensibility needs to be prioritized over conveniences for authoring code, I think.

I totally agree. there are situations where this type of syntax should not be used and it has been discussed in earlier parts of the thread. On the other hand, however, it really should be up to the author when and where to use this. this is really intended for places where there are hints to what the type could be. An example could be:

Right now, I'm trying to gauge feedback on what I should propose.

request.set_cookies_mode(_::Disabled)
// Or
request.set_cookies_mode(CookiesMode::Disabled)

@idanarye
Copy link

On the other hand, I can see how this has the potential to really hurt code comprehensibility. Visible type names matter a lot for that I've noticed

I don't think it'll hurt comprehensibility much, because the types it can infer are usually implementation details rather than important bits of the code's logic.

Consider:

  • foo(FooOptions {
        bar: 1,
        baz: 2,
    });
    The important bits of data here are that we are calling the foo() function and passing 1 as bar and 2 as baz. The fact that the type used to bundle together bar and baz is called FooOptions is of little importance, so we lose not comprehensibility by inferring it:
    foo(_ {
        bar: 1,
        baz: 2,
    });
  • Similarly:
    let FooResult { bar, baz, .. } = foo();
    Just like before - the important bits are that we are calling foo() and that we are taking the bar and baz from its result (discarding the rest). The fact that the author of the function has chosen to name the type that organizes all the information it returns as FooResult should not matter for comprehensibility, and that code is just as understandable when it is inferred:
    let _ { bar, baz, .. } = foo();
  • foo(FooChoice::Bar);
    Again - what's important here is that we are invoking foo() and picking Bar. The fact that the choices available when running foo are bundled under a FooChoice enum is not interesting, and should be inferred:
    foo(_::Bar);
  • match foo() {
        FooResult::Bar => { /* ... */ },
        FooResult::Baz => { /* ... */ },
    }
    Hopefully you can already see the pattern - the important bits are that we call foo() what we are going to do in the Bar case and in the Baz case, and the fact that these cases are enumerated under a FooResult enum is not important and should be inferred:
    match foo() {
        _::Bar => { /* ... */ },
        _::Baz => { /* ... */ },
    }
  • Foo {
        bar: Bar::Baz {
            qux: Qux {
                quux: 3,
                corge: 4,
            },
        },
    The thing that identifies the role of each field is the field's name, not its type. We lose no comprehensibility when infering the types:
    Foo {
        bar: _::Baz {
            qux: _ {
                quux: 3,
                corge: 4,
            },
        },
    Note that Foo is not inferred here because that's the type that holds it all together.
  • One case that is semi-interesting is when the inference is done in an earlier statement than the function call (or whatever is used for deciding the type):
    let options = _ {
        bar: 5,
        baz: 6,
    };
    // ... lots of code in the same function ...
    foo(options);
    One can argue that options is so far removed from foo that it is hard to understand what its type is. But I argue that the type itself is still not important, since we could have just as well written:
    let bar = 5;
    let baz = 6;
    // ... lots of code in the same function ...
    foo(_ { bar, baz });

There is one case though where I think this can be a problem - tuple structs. When a tuple struct with public fields is used instead of a regular tuple, this usually means the tuple's type itself is meaningful. The new type pattern is a good example - inferring the type kind of defeats the purpose there. So I suggest that type inference will not work for tuple structs. If it makes sense for a tuple struct to not acknowledge the type, that struct will want to impl From anyway, which can be used instead.

Actually, there is a second case - but its reasoning is the same. Unit structs. If someone decides to have a unit struct as an argument to a function, then they probably really want callers to type that out - and thus it should not be inferred. Of course, there are unit structs that are used for other reasons - like PhantomData or the token types in syn - but these can just impl Default.

@crlf0710
Copy link
Member

This rfc proposes changing how resolution for path works, which is fundamentally hard... I've wrote something that appears the same but much much easier to implement on internals site: https://internals.rust-lang.org/t/rewritten-proposal-about-variant/20801

@JoshuaBrest
Copy link
Author

This rfc proposes changing how resolution for path works, which is fundamentally hard... I've wrote something that appears the same but much much easier to implement on internals site: https://internals.rust-lang.org/t/rewritten-proposal-about-variant/20801

Yours seems to support impls. I don't want to support them because it will make stuff more confusing. impl methods can return incompatible types.

@scottmcm
Copy link
Member

An interesting variant of this came up on IRLO: https://internals.rust-lang.org/t/rewritten-proposal-about-variant/20801/13?u=scottmcm

Specifically, it seems like it might be interesting to support takes_file(_::create_new("hello.txt")?). Or foo(_::bar(2).await).

Is there a rule that might work for that? I don't think the one in the reference section currently does, since create_new is on File, not on Result, and create_new returns a Result.

@programmerjake
Copy link
Member

programmerjake commented May 20, 2024

Is there a rule that might work for that? I don't think the one in the reference section currently does, since create_new is on File, not on Result, and create_new returns a Result.

a rule that might make it work is where types can provide a deduction guide (loosely inspired by C++17):

// in std::result
#[deduce_methods_with(T)]
pub enum Result<T, E> {
    Ok(T),
    Err(E),
}

pub struct MyType {
    pub fn new() -> Result<MyType, Error> {
        todo!()
    }
}

pub fn f(v: MyType) {}

pub fn g() -> Result<(), Error> {
    // decides that it needs Result<MyType, ...>,
    // Result has `deduce_methods_with`, so that
    // makes it check MyType too, where it finds MyType::new
    f(_::new()?);
    Ok(())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet