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

Implementation of #34168 #34186

Merged
merged 2 commits into from
Jun 21, 2016
Merged

Implementation of #34168 #34186

merged 2 commits into from
Jun 21, 2016

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 9, 2016

r? @brson

cc @alexcrichton
cc @steveklabnik
cc @jonathandturner

I only updated librustc_privacy/diagnostics.rs, and I already found a case where the code doesn't throw the expected error code (E0448).

Fixes #34168.

@@ -465,6 +466,7 @@ impl LangString {
rust: true, // NB This used to be `notrust = false`
test_harness: false,
compile_fail: false,
error_codes: vec!(),
Copy link
Member

Choose a reason for hiding this comment

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

s/vec!()/Vec::new()/

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Jun 10, 2016

Strange... We can't see my changes on here but we can on my branch: https://github.com/GuillaumeGomez/rust/blob/err-code-check/src/librustdoc/test.rs#L291 .

So like we said when I saw @alexcrichton (I can finally say it haha), it seems it can be merged and then it'll be updated once the new error lib is out (keep me up to date on this @jonathandturner! :p).

@@ -493,7 +495,15 @@ impl LangString {
data.compile_fail = true;
seen_rust_tags = true;
data.no_run = true;
},
}
x if allow_compile_fail && x.starts_with("E") && x.len() == 5 => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a separate 'feature gate'. Let's not just tie it to compile-fail but add another feature like check_error_codes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok!

@GuillaumeGomez GuillaumeGomez force-pushed the err-code-check branch 3 times, most recently from a3fa261 to bdaf812 Compare June 13, 2016 11:50
@GuillaumeGomez
Copy link
Member Author

Updated.

@GuillaumeGomez
Copy link
Member Author

@alexcrichton: Up?

t("{.example .rust}", false, false, false, true, false, false);
t("{.test_harness .rust}", false, false, false, true, true, false);
// | error_codes
t("", false, false, false, true, false, false, Vec::new());
Copy link
Member

Choose a reason for hiding this comment

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

Can't you leave these unchanged and always use vec![] in t? Or are there other calls I'm not seeing?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're absolutely right.

@GuillaumeGomez
Copy link
Member Author

Updated.

@alexcrichton
Copy link
Member

@bors: r+ ddfaf10

@bors
Copy link
Contributor

bors commented Jun 20, 2016

⌛ Testing commit ddfaf10 with merge 4ba60ab...

bors added a commit that referenced this pull request Jun 20, 2016
Implementation of #34168

r? @brson

cc @alexcrichton
cc @steveklabnik
cc @jonathandturner

I only updated `librustc_privacy/diagnostics.rs`, and I already found a case where the code doesn't throw the expected error code (E0448).

Fixes #34168.
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