Skip to content

Change const error message to use 'literal' #46341

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

Closed
wants to merge 1 commit into from

Conversation

archer884
Copy link

The verbiage for E0015 ("calls in constants are limited to...") gives "struct and enum constructors" as a suggestion. The word "constructor," while a good descriptor for a particular concept in Rust, also describes a special class of user-defined functions in other languages. This commit substitutes the word "literal" in place of the word "constructor" in an effort to allay potential confusion.

Related issue here.

The verbiage for E0015 (calls in constants are limited to...) gives
"struct and enum constructors" as a suggestion. The word "constructor,"
while a good descriptor for a particular concept in Rust, also describes
a special class of user-defined functions in other languages.

This commit uses the word "literal" in place of the word "constructor."
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (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.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2017
@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 28, 2017

This error is reported for calls specifically, e.g. syntax EXPR(ARGS), not braced form PATH { FIELDS }, so "struct and enum constructors" refer to function-like constructors of tuple structs and variants:TupleVariant(Fields)/TupleStruct(Fields), so I wouldn't use the word "literals" here.

If we want to avoid the word "constructor" then we can say

"calls in {}s are limited to tuple structs and tuple variants"
"calls in {}s are limited to constant functions, tuple structs and tuple variants"

or

"function calls in {}s are limited to tuple structs and tuple variants"
"function calls in {}s are limited to constant functions, tuple structs and tuple variants"

@archer884
Copy link
Author

@petrochenkov, I'm not sure I understand. Are you saying that PATH { FIELDS } doesn't work, or shouldn't work, or that we just should not mention it here, or...? (In my particular case, the solution was to write exactly that.)

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 28, 2017

we just should not mention it here

This.
What this specific error wants to convey is "you wrote function call f(args). normally you can call function f, but in constant context you can't call it because it's not constant enough".

I mean, maybe there's a chance that you didn't actually want to call anything, but that's guessing. Maybe an extra suggestion attached to the primary error mentioning Struct { ... } would be better in this case.

@petrochenkov
Copy link
Contributor

r? @estebank

@archer884
Copy link
Author

Maybe an extra suggestion attached to the primary error mentioning Struct { ... } would be better in this case.

That seems to me to be the clearest option, although I'm probably not the best to make that judgment. :)

@estebank
Copy link
Contributor

estebank commented Dec 1, 2017

@archer884 I would go with @petrochenkov's suggestion.

Also, I'm surprised that there isn't an existing test for this output that caught your change. Could you add one?

@archer884
Copy link
Author

Honestly, all this other stuff is just beyond what I have knowledge/time for.

@archer884 archer884 closed this Dec 7, 2017
@estebank
Copy link
Contributor

estebank commented Dec 7, 2017

@archer884 sad to hear that. If you ever get the time and wish to contribute again and get familiar with the codebase/rust, we have issues with mentoring instructions for the impl period. Also, feel free to reach out in gitter and IRC for help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants