rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.98k stars 12.79k forks source link

Structure instantiation doesn't handle structure typedefs #4508

Closed jdm closed 10 years ago

jdm commented 11 years ago

This is from a build of Rust from around Christmas, so it would bear retesting on master.

rusti> struct foo_struct { a: int };
()
rusti> type foo = foo_struct;
c()
rusti> let f: foo = foo { a: 0 };
<anon>:41:15: 41:18 error: `foo` does not name a structure
<anon>:41 let f: foo = foo { a: 0 };
                         ^~~
error: aborting due to previous error
rust: task failed at 'explicit failure', /run/media/jdm/ssd/rust/src/libsyntax/diagnostic.rs:88
rusti> let f: foo = foo_struct { a: 0 };
()
catamorphism commented 11 years ago

I think this is behaving as intended. foo is the name of a type, so you can't use foo as a term. You have to write let f: foo = foo_struct { a: 0 };

graydon commented 11 years ago

non-critical for 0.6, de-milestoning

bstrie commented 11 years ago

It's somewhat misleading if you can only use the alias in certain places. Need a verdict on this. Nominating for Maturity 3.

catamorphism commented 11 years ago

I don't think it's misleading. You can use an alias declared by type = in the type namespace; not in the value namespace.

glaebhoerl commented 11 years ago

I don't think that's obvious. Haskell works that way, because in Haskell type and data constructors are declared separately. But for Rust's structs a single declared name is used for both. I don't think it's unreasonable to expect that if you introduce an alias for that one name, you can use it in the same places as the original name, IOW that the constructor has the same name as the type. That's what C++ does. E.g. this is valid:

struct foo_struct { int a; foo_struct(int _a): a(_a) { } };
typedef foo_struct foo;
foo f = foo(0);

I think the logical thing would be for it to work the Haskell way for enums (where there are separate names) and the C++ way for structs (where there's a single name), but at the very least I don't think the opposite is self-evident.

jdm commented 11 years ago

@pcwalton is under the impression that this should be fixed. Reopening for clarification.

ckarmann commented 11 years ago

Without the possibility to instantiate struct with them, the aliases would be somewhat useless. Combined with parameters, like for example in:

struct Vector3<T>{
    x:T,
    y:T,
    z:T,
}
type Vec3f = Vector3<f32>;

The use of the alias would allow to not having to carry around the type of the parameter everywhere. The client code does not even have to have knowledge of the aliased struct. But if one has to use the original name to instantiate it, the purpose of aliasing is defeated.

jdm commented 10 years ago

The relevant code sits under the ExprStruct arm of resolve_pattern in resolve.rs. Assuming that DefTy is a type Foo = Bar, it looks like there's some sort of expected condition that isn't being met here.

Manishearth commented 10 years ago

You probably meant to link to this code

Manishearth commented 10 years ago

The code being matched is a Some(DefTy(syntax::ast::DefId{krate: 0u32, node: 8u32}), LastMod(AllPublic)). This seems to match the first arm, so it's the guard that's the issue. Basically, the class_id is not in self.structs even though, as an alias, it should be there.

Removing the guard leads to error: internal compiler error: struct ID bound to non-struct, which makes sense. We'll have to add the type to the structs array when its parsed -- the code seems to expect this to happen. Somewhere here, I guess, but I'll have to figure out how I can check if the ItemTy is a struct.

zwarich commented 10 years ago

This presents a backwards compatibility hazard. The meaning of the following program will change when this bug is fixed:

#[deriving(Show)]
struct Foo(int);

fn Bar(a: int) -> int { a }

type Bar = Foo;

fn main() {
    println!("{}", Bar(1));
}
huonw commented 10 years ago

(Nominating)

zwarich commented 10 years ago

Actually, it would probably be rejected.

huonw commented 10 years ago

Another example that doesn't involve defining the same name twice in a single namespace:

use foo::Bar;

struct Foo(int);
type Bar = Foo;

fn main() { Bar(1); }
pnkfelix commented 10 years ago

Assigning P-backcompat-lang, 1.0 milestone.

pcwalton commented 10 years ago

I don't think this will ever work for tuple structs. If typedefs could inject names into the value namespace, then resolve would become quite difficult.

I don't think this was P-backcompat-lang, but I just finished the patch so let's get it in anyway.

glaebhoerl commented 10 years ago

@pcwalton Why is the tuple struct case different from the structural struct one?

pcwalton commented 10 years ago

Tuple struct constructors are first-class functions, like variant constructors. So the declaration of a tuple struct injects a value into the value namespace. The variable in an expression like x() looks for a name in the value namespace. However, the structure name in a structure literal expression like S { ... } (also structure patterns like S { ... }) looks for a name in the type namespace. The typedef declaration type only adds a name to the type namespace, not the value namespace. So it works for named structure literals and patterns, but not for tuple structs.

pcwalton commented 10 years ago

Nominating for not P-backcompat-lang per above, although I think this will be closed before the next triage meeting.

ben0x539 commented 10 years ago

This, and the thing where you can't call static methods of a type through a type alias, is going to keep biting people coming from C++, and probably places like Ruby as well. :(

huonw commented 10 years ago

@ben0x539 I believe @pcwalton means "closed as fixed".

ben0x539 commented 10 years ago

@huonw Oh, well, if that includes the tuple/unit-like struct case I'll happily take my complaining to the issue for calling static methods. ;)

glaebhoerl commented 10 years ago

@pcwalton Thanks for explaining.

Am I right to suspect that would no longer hold if structural enum variants enter the picture?