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

librustc_metadata: Pass a default value when unwrapping a span #57650

Merged
merged 2 commits into from Jan 19, 2019

Conversation

Projects
None yet
5 participants
@AB1908
Copy link

AB1908 commented Jan 16, 2019

Fixes #57323.

When compiling with static-nobundle a-la

rustc -l static-nobundle=nonexistent main.rs

we now get a neat output in the form of:

error[E0658]: kind="static-nobundle" is feature gated (see issue #37403)
  |
  = help: add #![feature(static_nobundle)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.

The build and tests completed successfully on my machine. Should I be adding a new test?

AB1908
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 16, 2019

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

@AB1908

This comment has been minimized.

Copy link
Author

AB1908 commented Jan 16, 2019

Even though I've fixed the issue (at least to my knowledge), I have no clue what static-nobundle is or does. Could someone point me to some info? I also tried compiling a Hello World! program after adding #![feature(static_nobundle)] but I got an error linking with cc failed. If I'm understanding correctly, this allows us to link with other compilers?

@petrochenkov petrochenkov assigned petrochenkov and unassigned eddyb Jan 16, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 16, 2019

@AB1908

Even though I've fixed the issue (at least to my knowledge), I have no clue what static-nobundle is or does.

rustc -l static-nobundle=foo main.rs will link our main program with a static library foo (file libfoo.a on Linux) found somewhere in library search directories.
This is usually used for linking to code written in C or Assembler.

"linking with cc failed" is reported because no library named nonexistent is found, so it's a linking error.

Should I be adding a new test?

Yes, this needs a test.
A proper place for the test is probably src/test/ui/feature-gates/feature-gate-static-nobundle-2.rs and the test will look something like this

// compile-flags: -l static-nobundle=nonexistent

fn main() {}

+ ERROR annotation + .stderr file with test output.

@AB1908

This comment has been minimized.

Copy link
Author

AB1908 commented Jan 17, 2019

  • ERROR annotation + .stderr file with test output.

I think I've added the necessary test but I have another issue to raise. From the docs, I couldn't figure out
where to place the ERROR annotation as it should fail because of a compile flag in this case rather than from the source code. However, the testing framework pointed out that it was looking for an ERROR in line 1 rather than say 7, and I was able to fix that up. Should we add some relevant snippet about this in the testing docs to save the hassle for a new contributor or did I not look hard enough?

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 17, 2019

@bors r+ rollup
Thanks!

Should we add some relevant snippet about this in the testing docs to save the hassle for a new contributor

Sure, if you think it would be useful.
As you said, the testing framework already kind of helps here pointing to the line 1.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 17, 2019

📌 Commit 1e3f475 has been approved by petrochenkov

Centril added a commit to Centril/rust that referenced this pull request Jan 18, 2019

Rollup merge of rust-lang#57650 - AB1908:master, r=petrochenkov
librustc_metadata: Pass a default value when unwrapping a span

Fixes rust-lang#57323.

When compiling with `static-nobundle` a-la

`rustc -l static-nobundle=nonexistent main.rs`

we now get a neat output in the form of:

```
error[E0658]: kind="static-nobundle" is feature gated (see issue rust-lang#37403)
  |
  = help: add #![feature(static_nobundle)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
```
The build and tests completed successfully on my machine. Should I be adding a new test?

bors added a commit that referenced this pull request Jan 18, 2019

Auto merge of #57727 - Centril:rollup, r=Centril
Rollup of 22 pull requests

Successful merges:

 - #56594 (Remove confusing comment about ideally using `!` for `c_void`)
 - #57107 (Add a regression test for mutating a non-mut #[thread_local])
 - #57340 (Use correct tracking issue for c_variadic)
 - #57357 (Cleanup PartialEq docs.)
 - #57370 (Support passing cflags/cxxflags/ldflags to LLVM build)
 - #57475 (Add signed num::NonZeroI* types)
 - #57501 (High priority resolutions for associated variants)
 - #57551 (resolve: Add a test for issue #57539)
 - #57610 (Fix nested `?` matchers)
 - #57634 (Remove an unused function argument)
 - #57635 (use structured macro and path resolve suggestions)
 - #57636 (Fix sources sidebar not showing up)
 - #57646 (Fixes text becoming invisible when element targetted)
 - #57649 (privacy: Account for associated existential types)
 - #57650 (librustc_metadata: Pass a default value when unwrapping a span)
 - #57654 (Add some links in std::fs.)
 - #57658 (Two HIR tweaks)
 - #57659 (Fix release manifest generation)
 - #57683 (Document Unpin in std::prelude documentation)
 - #57685 (Enhance `Pin` impl applicability for `PartialEq` and `PartialOrd`.)
 - #57698 (Fix typo bug in DepGraph::try_mark_green().)
 - #57720 (Fix suggestions given mulitple bad lifetimes)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Jan 18, 2019

Rollup merge of rust-lang#57650 - AB1908:master, r=petrochenkov
librustc_metadata: Pass a default value when unwrapping a span

Fixes rust-lang#57323.

When compiling with `static-nobundle` a-la

`rustc -l static-nobundle=nonexistent main.rs`

we now get a neat output in the form of:

```
error[E0658]: kind="static-nobundle" is feature gated (see issue rust-lang#37403)
  |
  = help: add #![feature(static_nobundle)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
```
The build and tests completed successfully on my machine. Should I be adding a new test?

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 18, 2019

Rollup merge of rust-lang#57650 - AB1908:master, r=petrochenkov
librustc_metadata: Pass a default value when unwrapping a span

Fixes rust-lang#57323.

When compiling with `static-nobundle` a-la

`rustc -l static-nobundle=nonexistent main.rs`

we now get a neat output in the form of:

```
error[E0658]: kind="static-nobundle" is feature gated (see issue rust-lang#37403)
  |
  = help: add #![feature(static_nobundle)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
```
The build and tests completed successfully on my machine. Should I be adding a new test?

bors added a commit that referenced this pull request Jan 18, 2019

Auto merge of #57732 - Centril:rollup, r=Centril
Rollup of 21 pull requests

Successful merges:

 - #56594 (Remove confusing comment about ideally using `!` for `c_void`)
 - #57107 (Add a regression test for mutating a non-mut #[thread_local])
 - #57340 (Use correct tracking issue for c_variadic)
 - #57357 (Cleanup PartialEq docs.)
 - #57370 (Support passing cflags/cxxflags/ldflags to LLVM build)
 - #57501 (High priority resolutions for associated variants)
 - #57551 (resolve: Add a test for issue #57539)
 - #57610 (Fix nested `?` matchers)
 - #57634 (Remove an unused function argument)
 - #57635 (use structured macro and path resolve suggestions)
 - #57636 (Fix sources sidebar not showing up)
 - #57646 (Fixes text becoming invisible when element targetted)
 - #57650 (librustc_metadata: Pass a default value when unwrapping a span)
 - #57654 (Add some links in std::fs.)
 - #57658 (Two HIR tweaks)
 - #57659 (Fix release manifest generation)
 - #57683 (Document Unpin in std::prelude documentation)
 - #57685 (Enhance `Pin` impl applicability for `PartialEq` and `PartialOrd`.)
 - #57698 (Fix typo bug in DepGraph::try_mark_green().)
 - #57720 (Fix suggestions given mulitple bad lifetimes)
 - #57725 (Use structured suggestion to surround struct literal with parenthesis)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Jan 18, 2019

Rollup merge of rust-lang#57650 - AB1908:master, r=petrochenkov
librustc_metadata: Pass a default value when unwrapping a span

Fixes rust-lang#57323.

When compiling with `static-nobundle` a-la

`rustc -l static-nobundle=nonexistent main.rs`

we now get a neat output in the form of:

```
error[E0658]: kind="static-nobundle" is feature gated (see issue rust-lang#37403)
  |
  = help: add #![feature(static_nobundle)] to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
```
The build and tests completed successfully on my machine. Should I be adding a new test?

bors added a commit that referenced this pull request Jan 18, 2019

Auto merge of #57747 - Centril:rollup, r=Centril
Rollup of 11 pull requests

Successful merges:

 - #57107 (Add a regression test for mutating a non-mut #[thread_local])
 - #57132 (Document that `-C opt-level=0` implies `-C debug-assertions`.)
 - #57212 (docs(rustc): Link to the book's source in rustc)
 - #57302 (Fix unused_assignments false positive)
 - #57350 (Better error note on unimplemented Index trait for string)
 - #57635 (use structured macro and path resolve suggestions)
 - #57650 (librustc_metadata: Pass a default value when unwrapping a span)
 - #57657 (Add regression test to close #53787)
 - #57658 (Two HIR tweaks)
 - #57720 (Fix suggestions given mulitple bad lifetimes)
 - #57725 (Use structured suggestion to surround struct literal with parenthesis)

Failed merges:

r? @ghost

@bors bors merged commit 1e3f475 into rust-lang:master Jan 19, 2019

@@ -163,7 +163,7 @@ impl<'a, 'tcx> Collector<'a, 'tcx> {
!self.tcx.features().static_nobundle {
feature_gate::emit_feature_err(&self.tcx.sess.parse_sess,
"static_nobundle",
span.unwrap(),
span.unwrap_or_else(|| syntax_pos::DUMMY_SP),

This comment has been minimized.

@eddyb

eddyb Jan 30, 2019

Member

This should be span.unwrap_or(syntax_pos::DUMMY_SP), as it's just a constant.

This comment has been minimized.

@petrochenkov

petrochenkov Jan 30, 2019

Contributor

Oh. I didn't even notice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment