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 inspection and setter methods to proc_macro::Diagnostic. #52896

Merged
merged 2 commits into from
Sep 15, 2018

Conversation

SergioBenitez
Copy link
Contributor

A few useful methods for proc_macro::Diagnostic.

r? @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@SergioBenitez
Copy link
Contributor Author

This is the build failure:

[00:23:33] error: This node does not have a stability attribute
[00:23:33]    --> libproc_macro/diagnostic.rs:130:31
[00:23:33]     |
[00:23:33] 130 |     pub fn children(&self) -> impl Iterator<Item = &Diagnostic> {
[00:23:33]     |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

...which doesn't make sense to me. The method is labeled unstable already:

#[unstable(feature = "proc_macro_diagnostic", issue = "38356")]
pub fn children(&self) -> impl Iterator<Item = &Diagnostic> { .. }

The error is pointing at the return type. Is there some extra labeling that needs to happen, or is this a bug elsewhere?

@alexcrichton
Copy link
Member

Looks good to me! That may be a bug in the stability lint perhaps? It hasn't been touched in a long long time :(

@eddyb
Copy link
Member

eddyb commented Jul 31, 2018

cc @oli-obk looks like the stability marking pass is not recursing into existential type nodes?

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@oli-obk
Copy link
Contributor

oli-obk commented Aug 1, 2018

Should be enough to add an arm to

hir::ItemKind::Impl(.., None, _, _) | hir::ItemKind::ForeignMod(..) => {}

for ItemExistential(ExistTy { parent_fn_id: Some(_) }) which also skips the check_missing_stability call

@eddyb
Copy link
Member

eddyb commented Aug 1, 2018

@oli-obk Isn't the problem here that sometimes it doesn't get marked?
This appears to be a problem specifically with -> impl Trait in inherent impl methods.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 1, 2018

that's weird. Maybe lowering places them inside the impl block instead of the surrounding module?

@XAMPPRocky
Copy link
Member

Triage: Marking this as blocked on a fix to the stability marking pass. @oli-obk Is there a PR/issue about this problem? If there's not can you open one?

@XAMPPRocky XAMPPRocky added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2018
@SergioBenitez
Copy link
Contributor Author

@oli-obk Pinging to see if we can move this forward. Worst case, I'll just stop using impl Trait here.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 12, 2018

Probably the same issue as #54045 where the same fix applies

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:08:03]    Compiling proc_macro v0.0.0 (file:///checkout/src/libproc_macro)
[00:08:04] error: This node does not have a stability attribute
[00:08:04]    --> libproc_macro/diagnostic.rs:130:31
[00:08:04]     |
[00:08:04] 130 |     pub fn children(&self) -> impl Iterator<Item = &Diagnostic> {
[00:08:04] 
[00:08:04] error: aborting due to previous error
[00:08:04] 
[00:08:04] error: Could not compile `proc_macro`.
[00:08:04] error: Could not compile `proc_macro`.
[00:08:04] 
[00:08:04] Caused by:
[00:08:04]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name proc_macro libproc_macro/lib.rs --color always --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=87919c9c23cf3652 -C extra-filename=-87919c9c23cf3652 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-05301c67193a930e.so --extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-f4ac364f854372fe.so --extern syntax=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax-0651ffc5a9129db1.so --extern syntax_pos=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-47b99ffec2efbd05.so` (exit code: 1)
[00:08:04] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:08:04] expected success, got: exit code: 101
[00:08:04] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1155:9
[00:08:04] travis_fold:end:stage0-rustc

[00:08:04] travis_time:end:stage0-rustc:start=1536818436003034068,finish=1536818574380296229,duration=138377262161


[00:08:04] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:08:04] Build completed unsuccessfully in 0:03:17
[00:08:04] make: *** [all] Error 1
[00:08:04] Makefile:28: recipe for target 'all' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00073bbd
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:128be956:start=1536818575070752570,finish=1536818575075771607,duration=5019037
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:30eb2508
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0970f98c
travis_time:start:0970f98c
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0f8ea352
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Also updates the issue number for 'proc_macro_diagnostic'.
@SergioBenitez
Copy link
Contributor Author

Worked around the bug by using a special-cased Iterator type. Also added multispan support to boot.

@SergioBenitez
Copy link
Contributor Author

@alexcrichton How does this look?

@alexcrichton
Copy link
Member

@bors: r+

Looks good to me!

@bors
Copy link
Contributor

bors commented Sep 14, 2018

📌 Commit 10bb5ed has been approved by alexcrichton

@bors 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-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Sep 14, 2018
@bors
Copy link
Contributor

bors commented Sep 15, 2018

⌛ Testing commit 10bb5ed with merge 5d083073c915a70b2049d9eed696bf31603e3557...

@bors
Copy link
Contributor

bors commented Sep 15, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 15, 2018
@rust-highfive
Copy link
Collaborator

The job arm-android of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:42:54] test time::tests::system_time_elapsed ... ok
[01:42:54] test time::tests::system_time_math ... ok
[01:42:59] test sync::mpsc::tests::stress_recv_timeout_two_threads ... ok
[01:43:01] test collections::hash::map::test_map::test_lots_of_insertions ... ok
[01:43:35] test process::tests::test_process_output_fail_to_start ... test process::tests::test_process_output_fail_to_start has been running for over 60 seconds
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@SergioBenitez
Copy link
Contributor Author

Failure looks unrelated. Rerun?

@kennytm
Copy link
Member

kennytm commented Sep 15, 2018

@bors retry #43283

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2018
@bors
Copy link
Contributor

bors commented Sep 15, 2018

⌛ Testing commit 10bb5ed with merge cba0fdf...

bors added a commit that referenced this pull request Sep 15, 2018
Add inspection and setter methods to proc_macro::Diagnostic.

A few useful methods for `proc_macro::Diagnostic`.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Sep 15, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing cba0fdf to master...

@bors bors merged commit 10bb5ed into rust-lang:master Sep 15, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 21, 2018
…crichton

Make 'proc_macro::MultiSpan' public.

Oversight from rust-lang#52896.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Sep 22, 2018
…crichton

Make 'proc_macro::MultiSpan' public.

Oversight from rust-lang#52896.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants