Reword error when data-less enum variant called as function #36520

estebank · 2016-09-16T01:49:07Z

Given a file like:

enum Test {
    Variant,
    Variant2 {a: u32},
}

fn main(){
    let x = Test::Variant("Hello");
    let y = Test::Variant2("World");
}

Both errors now look similar:

error[E0423]: `Test::Variant2` is the name of a struct or struct variant, but this expression uses it like a function name
  --> file3.rs:10:13
   |
10 |     let y = Test::Variant2("Hello");
   |             ^^^^^^^^^^^^^^ struct called like a function
   |
   = help: did you mean to write: `Test::Variant2 { /* fields */ }`?

error: `Test::Variant` is the name of a data-less enum, but this expression uses it like a function name
 --> file3.rs:9:13
  |
9 |     let x = Test::Variant("World");
  |             ^^^^^^^^^^^^^^^^^^^^^^ data-less enum called like a function
  |
  = help: did you mean to write: `Test::Variant`?
note: defined here
 --> file3.rs:2:5
  |
2 |     Variant,
  |     ^^^^^^^

error: aborting due to previous error

Re: #28533

rust-highfive · 2016-09-16T01:49:20Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

pnkfelix · 2016-09-16T10:03:55Z

My initial reaction: I don't think I've seen the phrase "data-less enum" before, and I'd prefer to try to use a phrase consistent with our docs, if possible.

However, I do see "variant with no data" in the book. The book also uses "unit-like" elsewhere (in the context of "unit-like struct"), which I also see at rust-by-example

cc @rust-lang/docs

GuillaumeGomez · 2016-09-16T10:15:54Z

First time I see "data-less enum" expression as well. Sounds really strange.

@jonathandturner: Has it been added in the update to the new error format maybe?

petrochenkov · 2016-09-16T10:31:18Z

It's called "unit variant"/"unit struct" in other resolution-related error messages.

FWIW, I don't think all this extra cruft is an improvement.
Simply "expected function, found E" => "expected function, found unit variant E::Empty4" would be better.

EDIT: the help line "did you mean to write E::Empty4" may be useful though.

sophiajt · 2016-09-16T15:30:48Z

Yeah, we have one for E0423 (#35796), though I'm happy to update if we have better wording.

We've been trying to not use any technical terminology unless it's a part of the language (eg mutable is fine because of mut), so I'm reluctant to go with "unit variant". I think "empty struct" or "empty enum" gets the idea across, with the exception that we may want to say "empty enum variant".

estebank · 2016-09-16T16:50:55Z

I used the terminology from the original ticket, but I agree the error message as is isn't great.

I see some value in the errors for the different alternatives to have similar wording and format. The current error is a bit ambiguous, naming the enum's name, and a pointer to the variant's definition:

error: expected function, found `Test`
 --> file.rs:7:13
  |
7 |     let x = Test::Variant(1);
  |             ^^^^^^^^^^^^^^^^
  |
note: defined here
 --> file.rs:2:5
  |
2 |     Variant,
  |     ^^^^^^^

This gives you enough information to fix the issue, but I feel it is confusing for newcomers (as evidenced by #28533).

A compromise might be doing:

error: expected function, found `Test`
 --> file.rs:7:13
  |
7 |     let x = Test::Variant(1);
  |             ^^^^^^^^^^^^^^^^ empty enum variant called like a function
  |
  = help: did you mean to write: `Test::Variant`?
note: defined here
 --> file.rs:2:5
  |
2 |     Variant,
  |     ^^^^^^^

Also, I just tried using a struct variant as a unit variant, and there's no specialization in that error either, claiming it was called like a function:

error[E0423]: `Test::Variant2` is the name of a struct or struct variant, but this expression uses it like a function name
 --> file.rs:8:13
  |
8 |     let y = Test::Variant2;
  |             ^^^^^^^^^^^^^^ struct called like a function
  |
  = help: did you mean to write: `Test::Variant2 { /* fields */ }`?

petrochenkov · 2016-09-16T17:13:59Z

@jonathandturner

We've been trying to not use any technical terminology unless it's a part of the language (eg mutable is fine because of mut), so I'm reluctant to go with "unit variant".

It's in the book, in the reference, in other docs.
Also, the error is about unit variants specifically, not empty variants in general. I'd prefer to establish some terminology and use it consistently.

sophiajt · 2016-09-16T17:17:31Z

@petrochenkov - we're updating the book to follow a similar scheme, where we only use terminology that will be necessary to understand the language as we go. Since we don't know the level of experience of the user when giving them an error message, it's better to err on the side of using language that more people can understand.

It's totally okay for the reference and RFCs to be more technical, I think.

petrochenkov · 2016-09-16T18:18:57Z

@jonathandturner
At least some part of diagnostics should be on the "technical side" and tell precisely what's wrong and give keywords to lookup in the glossary or reference. The primary error message is good candidate.
Error explanations, labels, notes, help - they can go into more novice friendly territory, give examples, etc.

GuillaumeGomez · 2016-09-16T22:05:35Z

I agree with @petrochenkov here: people are using rust after all. If they don't know what a terminology is, it just means that they still need to learn it. Having a difference between "technical side" and any other side will bring more problems than solutions.

sophiajt · 2016-09-16T22:10:24Z

@GuillaumeGomez - that's a fair point for terminology that's already established terms. I did a quick google through Google Scholar and didn't see either "unit struct" and "unit variant" mentioned, so I suspect these are terms we made up and there are easier ways to say those things.

GuillaumeGomez · 2016-09-17T11:10:54Z

I'm pretty sure to already have saw them when I did ocaml, but maybe I'm wrong... Anyway, if there is another "official" terminology, we should fully switch to it (but I'm still pretty sure to have already saw them before...).

estebank · 2016-09-17T19:36:18Z

New output

error[E0423]: `Test::Variant2` is the name of a struct or struct variant, but this expression uses it like a function name
 --> unit.rs:8:13
  |
8 |     let y = Test::Variant2("World");
  |             ^^^^^^^^^^^^^^ struct called like a function
  |
  = help: did you mean to write: `Test::Variant2 { /* fields */ }`?

error: expected function, found unit variant `Test::Variant`
 --> unit.rs:7:13
  |
7 |     let x = Test::Variant("Hello");
  |             ^^^^^^^^^^^^^^^^^^^^^^ unit enum variant called like a function
  |
  = help: did you mean to write: `Test::Variant`?
note: defined here
 --> unit.rs:2:5
  |
2 |     Variant,
  |     ^^^^^^^

error: aborting due to previous error

I defer the final wording to the team, but would like to know if the underlying code is sound.

nikomatsakis · 2016-09-17T22:48:58Z

FWIW I have never liked the term "unit" (unit type, unit struct, etc). I know its origins in type theory and all but it doesn't really convey (to me) a strong sense of what it is and I don't think it's a term in widespread use. That said, I also don't like "data-less". It just doesn't...have a natural ring for me. "Empty enum variant" might be my favorite of the bunch, but maybe we can reword to avoid an adjective or in some other way?

Some thoughts:

Test::Variant2 is being called, but it is not a function
Test::Variant2 is being called, but it is an enum variant defined without arguments
- Hint: Just write Test::Variant2, not Test::Variant2()

I see the value in sharing terminology between the book and the error messages -- but I think we should try to adopt terminology that is readily understood without having to go look it up. Basically I think avoiding "jargon" should be a high priority for us, so long as we are not introducing imprecision and confusion.

bors · 2016-09-22T05:47:14Z

☔ The latest upstream changes (presumably #36551) made this pull request unmergeable. Please resolve the merge conflicts.

pnkfelix · 2016-09-22T12:03:12Z

@nikomatsakis Any chance we could revise teminology in book and messaging to rename "unit enum variant" to "tag" ? Or is that already used to talk about the discriminant in general?

Update: I proposed this b/c I was under impression that this was common terminology in PL (e.g. "type tag"), but a quick review of Google results lead me to doubt that theory.

nikomatsakis · 2016-09-27T21:10:56Z

@pnkfelix I'm not aware of us saying "tag" much, I guess people sometimes say "tagged union"?

estebank · 2016-09-30T03:15:14Z

Rebased to fix the merge conflict and reworded the message slightly:

GuillaumeGomez · 2016-09-30T09:42:21Z

src/librustc_typeck/check/callee.rs

+                            Some(print::expr_to_string(expr))
+                        } else { None }
+                    } else { None }
+                } else { None };


Uuuuuuuurgh.

GuillaumeGomez · 2016-09-30T09:42:25Z

src/librustc_typeck/check/callee.rs

+                    if adt_def.is_enum() {
+                        if let hir::ExprCall(ref expr, _) = call_expr.node {
+                            Some(print::expr_to_string(expr))
+                        } else { None }


Uuuuuuuurgh.

GuillaumeGomez · 2016-09-30T09:42:29Z

src/librustc_typeck/check/callee.rs

+                        if let hir::ExprCall(ref expr, _) = call_expr.node {
+                            Some(print::expr_to_string(expr))
+                        } else { None }
+                    } else { None }


Uuuuuuuurgh.

GuillaumeGomez · 2016-09-30T09:43:20Z

src/librustc_typeck/check/callee.rs

+            ref t => {
+                let unit_variant = if let &ty::TyAdt(adt_def, ..) = t {
+                    if adt_def.is_enum() {
+                        if let hir::ExprCall(ref expr, _) = call_expr.node {


Create a None variable and remove all else cases.

GuillaumeGomez

All good for me.

bors · 2016-10-12T16:08:42Z

☔ The latest upstream changes (presumably #36737) made this pull request unmergeable. Please resolve the merge conflicts.

Given a file like: ```rust enum Test { Variant, Variant2 {a: u32}, } fn main(){ let x = Test::Variant("Hello"); let y = Test::Variant2("World"); } ``` The errors now look this way: ```bash error[E0423]: `Test::Variant2` is the name of a struct or struct variant, but this expression uses it like a function name --> file3.rs:10:13 | 10 | let y = Test::Variant2("Hello"); | ^^^^^^^^^^^^^^ struct called like a function | = help: did you mean to write: `Test::Variant2 { /* fields */ }`? error: `Test::Variant` is being called, but it is not a function --> file3.rs:9:13 | 9 | let x = Test::Variant("World"); | ^^^^^^^^^^^^^^^^^^^^^^ | = help: did you mean to write: `Test::Variant`? note: defined here --> file3.rs:2:5 | 2 | Variant, | ^^^^^^^ error: aborting due to previous error ```

estebank · 2016-10-22T18:38:41Z

Rebased to fix merge conflict and squash history.

estebank · 2016-10-25T16:04:27Z

r? @pnkfelix

brson · 2016-11-09T02:00:23Z

@bors r+

Thanks @estebank and sorry for the delay!

bors · 2016-11-09T02:00:25Z

📌 Commit a449bdb has been approved by brson

bors · 2016-11-09T16:51:23Z

⌛ Testing commit a449bdb with merge bca365e...

Reword error when data-less enum variant called as function Given a file like: ``` rust enum Test { Variant, Variant2 {a: u32}, } fn main(){ let x = Test::Variant("Hello"); let y = Test::Variant2("World"); } ``` Both errors now look similar: ``` bash error[E0423]: `Test::Variant2` is the name of a struct or struct variant, but this expression uses it like a function name --> file3.rs:10:13 | 10 | let y = Test::Variant2("Hello"); | ^^^^^^^^^^^^^^ struct called like a function | = help: did you mean to write: `Test::Variant2 { /* fields */ }`? error: `Test::Variant` is the name of a data-less enum, but this expression uses it like a function name --> file3.rs:9:13 | 9 | let x = Test::Variant("World"); | ^^^^^^^^^^^^^^^^^^^^^^ data-less enum called like a function | = help: did you mean to write: `Test::Variant`? note: defined here --> file3.rs:2:5 | 2 | Variant, | ^^^^^^^ error: aborting due to previous error ``` Re: #28533

bors · 2016-11-09T19:58:23Z

rust-highfive assigned pnkfelix Sep 16, 2016

estebank force-pushed the dataless-enum branch 3 times, most recently from cd13bb8 to 25d08cf Compare September 16, 2016 03:16

estebank force-pushed the dataless-enum branch from 2d23371 to 6afe86a Compare September 30, 2016 03:11

GuillaumeGomez requested changes Sep 30, 2016

View reviewed changes

GuillaumeGomez approved these changes Sep 30, 2016

View reviewed changes

estebank force-pushed the dataless-enum branch from 6a13b65 to a449bdb Compare October 22, 2016 18:37

bors mentioned this pull request Nov 9, 2016

rustc_llvm: Require 3.9 for --link-static #37667

Merged

bors merged commit a449bdb into rust-lang:master Nov 9, 2016

estebank deleted the dataless-enum branch November 9, 2023 05:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reword error when data-less enum variant called as function #36520

Reword error when data-less enum variant called as function #36520

estebank commented Sep 16, 2016

rust-highfive commented Sep 16, 2016

pnkfelix commented Sep 16, 2016

GuillaumeGomez commented Sep 16, 2016

petrochenkov commented Sep 16, 2016 •

edited

Loading

sophiajt commented Sep 16, 2016

estebank commented Sep 16, 2016

petrochenkov commented Sep 16, 2016 •

edited

Loading

sophiajt commented Sep 16, 2016

petrochenkov commented Sep 16, 2016

GuillaumeGomez commented Sep 16, 2016

sophiajt commented Sep 16, 2016

GuillaumeGomez commented Sep 17, 2016

estebank commented Sep 17, 2016

nikomatsakis commented Sep 17, 2016

bors commented Sep 22, 2016

pnkfelix commented Sep 22, 2016 •

edited

Loading

nikomatsakis commented Sep 27, 2016

estebank commented Sep 30, 2016

GuillaumeGomez Sep 30, 2016

GuillaumeGomez Sep 30, 2016

GuillaumeGomez Sep 30, 2016

GuillaumeGomez Sep 30, 2016 •

edited

Loading

estebank Sep 30, 2016

GuillaumeGomez left a comment

bors commented Oct 12, 2016

estebank commented Oct 22, 2016

estebank commented Oct 25, 2016

brson commented Nov 9, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

Reword error when data-less enum variant called as function #36520

Reword error when data-less enum variant called as function #36520

Conversation

estebank commented Sep 16, 2016

rust-highfive commented Sep 16, 2016

pnkfelix commented Sep 16, 2016

GuillaumeGomez commented Sep 16, 2016

petrochenkov commented Sep 16, 2016 • edited Loading

sophiajt commented Sep 16, 2016

estebank commented Sep 16, 2016

petrochenkov commented Sep 16, 2016 • edited Loading

sophiajt commented Sep 16, 2016

petrochenkov commented Sep 16, 2016

GuillaumeGomez commented Sep 16, 2016

sophiajt commented Sep 16, 2016

GuillaumeGomez commented Sep 17, 2016

estebank commented Sep 17, 2016

nikomatsakis commented Sep 17, 2016

bors commented Sep 22, 2016

pnkfelix commented Sep 22, 2016 • edited Loading

nikomatsakis commented Sep 27, 2016

estebank commented Sep 30, 2016

GuillaumeGomez Sep 30, 2016

Choose a reason for hiding this comment

GuillaumeGomez Sep 30, 2016

Choose a reason for hiding this comment

GuillaumeGomez Sep 30, 2016

Choose a reason for hiding this comment

GuillaumeGomez Sep 30, 2016 • edited Loading

Choose a reason for hiding this comment

estebank Sep 30, 2016

Choose a reason for hiding this comment

GuillaumeGomez left a comment

Choose a reason for hiding this comment

bors commented Oct 12, 2016

estebank commented Oct 22, 2016

estebank commented Oct 25, 2016

brson commented Nov 9, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

bors commented Nov 9, 2016

petrochenkov commented Sep 16, 2016 •

edited

Loading

petrochenkov commented Sep 16, 2016 •

edited

Loading

pnkfelix commented Sep 22, 2016 •

edited

Loading

GuillaumeGomez Sep 30, 2016 •

edited

Loading