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

AB1908 · 2019-01-16T00:38:16Z

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?

Fixes #57323

rust-highfive · 2019-01-16T00:38:19Z

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 · 2019-01-16T00:59:11Z

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 · 2019-01-16T14:34:30Z

@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 · 2019-01-17T03:43:42Z

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 · 2019-01-17T10:09:41Z

@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 · 2019-01-17T10:09:42Z

📌 Commit 1e3f475 has been approved by 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?

@ghost

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

eddyb · 2019-01-30T11:40:52Z

src/librustc_metadata/native_libs.rs

@@ -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 should be span.unwrap_or(syntax_pos::DUMMY_SP), as it's just a constant.

Oh. I didn't even notice.

Pass a default value when unwrapping a span

e5e9867

Fixes #57323

rust-highfive assigned eddyb Jan 16, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2019

petrochenkov assigned petrochenkov and unassigned eddyb Jan 16, 2019

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2019

Add test for linking non-existent static library

1e3f475

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 17, 2019

Centril mentioned this pull request Jan 18, 2019

Rollup of 22 pull requests #57727

Closed

Centril mentioned this pull request Jan 18, 2019

Rollup of 21 pull requests #57732

Closed

GuillaumeGomez mentioned this pull request Jan 18, 2019

Rollup of 17 pull requests #57733

Closed

Centril mentioned this pull request Jan 18, 2019

Rollup of 11 pull requests #57747

Merged

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

eddyb reviewed Jan 30, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

AB1908 commented Jan 16, 2019 •

edited

Loading

rust-highfive commented Jan 16, 2019

AB1908 commented Jan 16, 2019 •

edited

Loading

petrochenkov commented Jan 16, 2019

AB1908 commented Jan 17, 2019 •

edited

Loading

petrochenkov commented Jan 17, 2019

bors commented Jan 17, 2019

eddyb Jan 30, 2019

petrochenkov Jan 30, 2019

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

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

Conversation

AB1908 commented Jan 16, 2019 • edited Loading

rust-highfive commented Jan 16, 2019

AB1908 commented Jan 16, 2019 • edited Loading

petrochenkov commented Jan 16, 2019

AB1908 commented Jan 17, 2019 • edited Loading

petrochenkov commented Jan 17, 2019

bors commented Jan 17, 2019

eddyb Jan 30, 2019

Choose a reason for hiding this comment

petrochenkov Jan 30, 2019

Choose a reason for hiding this comment

AB1908 commented Jan 16, 2019 •

edited

Loading

AB1908 commented Jan 16, 2019 •

edited

Loading

AB1908 commented Jan 17, 2019 •

edited

Loading