Skip to content

Improve the error explanations for check_const#30932

Merged
bors merged 1 commit into
rust-lang:masterfrom
arielb1:raw-const-errors
Jan 24, 2016
Merged

Improve the error explanations for check_const#30932
bors merged 1 commit into
rust-lang:masterfrom
arielb1:raw-const-errors

Conversation

@arielb1

@arielb1 arielb1 commented Jan 15, 2016

Copy link
Copy Markdown
Contributor

Fixes #30705

r? @nagisa

Comment thread src/librustc/diagnostics.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"// ..and also"? Isn't "// ... and also" better?

@nagisa

nagisa commented Jan 15, 2016

Copy link
Copy Markdown
Member

Looks like an overall improvement.

@arielb1

arielb1 commented Jan 15, 2016

Copy link
Copy Markdown
Contributor Author

improved

Comment thread src/librustc/diagnostics.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is wrong. This document already contains various cases of wrong advice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wrong?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the problem is that you need the value behind a pointer in a static variable, then not putting the value in a static variable does not fix the problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not like you can solve the problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how important is the "how to fix this" advice, because each issue will have its own fix.

Comment thread src/librustc/diagnostics.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: This paragraph should use inline code for the constant names.

@nagisa

nagisa commented Jan 15, 2016

Copy link
Copy Markdown
Member

r=me once the two nits are fixed.

@arielb1

arielb1 commented Jan 19, 2016

Copy link
Copy Markdown
Contributor Author

fixed ^

@nagisa

nagisa commented Jan 19, 2016

Copy link
Copy Markdown
Member

@bors r+ cad3882

@nagisa

nagisa commented Jan 19, 2016

Copy link
Copy Markdown
Member

@bors rollup

@bors

bors commented Jan 21, 2016

Copy link
Copy Markdown
Collaborator

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

@bors

bors commented Jan 23, 2016

Copy link
Copy Markdown
Collaborator

🔒 Merge conflict

@arielb1

arielb1 commented Jan 24, 2016

Copy link
Copy Markdown
Contributor Author

@bors r=nagisa rollup

@bors

bors commented Jan 24, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 47593da has been approved by nagisa

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.

6 participants