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

Add long diagnostics for E0018 #24525

Merged
merged 2 commits into from
Apr 20, 2015
Merged

Conversation

GuillaumeGomez
Copy link
Member

Part of #24407.

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -112,6 +112,11 @@ reference when using guards or refactor the entire expression, perhaps by
putting the condition inside the body of the arm.
"##,

E0018: r##"
The value of static and const variables must be known at the compile-time which is
Copy link
Contributor

Choose a reason for hiding this comment

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

s/the compile-time which/compile time, which/

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm don't get your point.

Copy link
Contributor

Choose a reason for hiding this comment

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

He means "... value of static and const variables must be known at compile time, which is ...".
And I think he's right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right ! Thanks !

@GuillaumeGomez GuillaumeGomez force-pushed the check-const branch 2 times, most recently from 55eabfc to 3434062 Compare April 17, 2015 17:21
@pnkfelix
Copy link
Member

Don't you need to delete the other entry too?

@GuillaumeGomez
Copy link
Member Author

@pnkfelix: Totally, thanks for noticing me !

@GuillaumeGomez
Copy link
Member Author

And it's done !

@bors
Copy link
Contributor

bors commented Apr 18, 2015

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

@GuillaumeGomez
Copy link
Member Author

@apasel422: Did you find another way ? For my french mind, it seems totally fine haha.

@michaelsproul
Copy link
Contributor

I think the explanation needs a bit more detail to fully convey the subtlety of this error. Pointers are allowed in const items, just not pointers cast to integers. See RFC 246, particularly the section on const => const and the bug that introduced this error, #18294.

I think an ideal explanation for this error should cover these points:

  • Pointers to other constants are allowed in constants, although the address will be non-constant.
// This is fine.
const X: u32 = 50;
const Y: *const u32 = &X;
println!("{:?}", Y);
  • Therefore, casting one of these non-constant pointers to an integer results in a non-constant integer (as observed by running the above program), thus triggering E0018.

You could also link to the RFC and the issue.

@GuillaumeGomez
Copy link
Member Author

@michaelsproul: I don't think giving links to RFC/issue was a good idea. I added your explanations. Do you find something else to correct or add ?

E0018: r##"
The value of static and const variables must be known at compile time. You
can't cast an integer as a pointer because we can't know what value will be
at the address.
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks great except for the second sentence here. I think integer as a pointer should be pointer as an integer and what value will be at the address should be what value the address will take.

@GuillaumeGomez
Copy link
Member Author

@michaelsproul: It's corrected ! Thanks again !

@michaelsproul
Copy link
Contributor

@GuillaumeGomez: Looks good to me 👍

```

Therefore, casting one of these non-constant pointers to an integer results
in a non-constant integer whichs lead to this error. Example:
Copy link
Member

Choose a reason for hiding this comment

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

typo: "whichs lead to this error" is not correct. Should be "which"

@GuillaumeGomez
Copy link
Member Author

@pnkfelix: It's corrected, thanks !

@pnkfelix
Copy link
Member

@bors r+ 737005a rollup

@bors
Copy link
Contributor

bors commented Apr 20, 2015

⌛ Testing commit 737005a with merge fe11393...

bors added a commit that referenced this pull request Apr 20, 2015
@bors bors merged commit 737005a into rust-lang:master Apr 20, 2015
@GuillaumeGomez GuillaumeGomez deleted the check-const branch April 20, 2015 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants