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 an after_expansion callback in the driver #62679

Merged
merged 2 commits into from
Jul 19, 2019

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented Jul 14, 2019

To format a given file RLS needs to know the Rust edition associated with it. It's not enough to look at the edition key in Cargo.toml - each crate target can have a different edition associated with it so the sure way to fetch a correct edition is to scan the input files used to compile a given crate target.

Right now this was done in the after_analysis callback of our shim, however this leads to other problems - if a crate cannot be successfully compiled (e.g. it has a type error) then a callback would not be invoked meaning we can't populate the files -> edition mapping.

However, doing this only after parsing is not enough, since expansion can pull in additional source files (e.g. by invoking macro_rules! include_my_mod { () => { mod some_mod; }; }).

Without copy-pasting the entire driver setup it's also not possible to expand the crate ourselves in the after_parsing callback - to expand crate we'd need to register plugins and initialize session ourselves. However, this is done normally after executing the callback itself, thus triggering the Once::set assertions in Session::init_features.

r? @Zoxc

cc @RalfJung @oli-obk this affects public driver interface used by Miri and Clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2019
src/librustc_driver/lib.rs Outdated Show resolved Hide resolved
src/librustc_driver/lib.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

this affects public driver interface used by Miri and Clippy

How hard is it to adjust existing drivers to the changed interface? Do we just have replace the bool by this Compilation? That's a welcome change actually. :) Thanks for the heads-up.

@Xanewok
Copy link
Member Author

Xanewok commented Jul 15, 2019

Addressed the feedback. Is this good now?

@Xanewok
Copy link
Member Author

Xanewok commented Jul 15, 2019

How hard is it to adjust existing drivers to the changed interface? Do we just have replace the bool by this Compilation?

Yup, that's it! Figured it'd be more readable than bool, I found myself commenting

// Continue execution
true

so a Compilation::Continue should hopefully be more readable here.

@Zoxc
Copy link
Contributor

Zoxc commented Jul 15, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Jul 15, 2019

📌 Commit ff63336 has been approved by Zoxc

@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 Jul 15, 2019
@bors
Copy link
Contributor

bors commented Jul 18, 2019

⌛ Testing commit ff63336 with merge e8e0c666bad1a6e0af6a11fcff0db39a881e04a0...

@bors
Copy link
Contributor

bors commented Jul 18, 2019

💔 Test failed - checks-azure

@rust-highfive
Copy link
Collaborator

The job x86_64-msvc-cargo of your PR failed (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.
2019-07-18T12:18:01.0202751Z do so (now or later) by using -b with the checkout command again. Example:
2019-07-18T12:18:01.0203247Z 
2019-07-18T12:18:01.0203541Z   git checkout -b <new-branch-name>
2019-07-18T12:18:01.0203697Z 
2019-07-18T12:18:01.0203882Z HEAD is now at e8e0c666b Auto merge of #62679 - Xanewok:after-expansion, r=Zoxc
2019-07-18T12:18:01.0569364Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-07-18T12:18:01.0691851Z ==============================================================================
2019-07-18T12:18:01.0691925Z Task         : Bash
2019-07-18T12:18:01.0691998Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-07-18T13:07:24.6996374Z [551/2435] Linking CXX executable bin\llvm-tblgen.exe
2019-07-18T13:07:24.7449352Z [552/2435] Building Options.inc...
2019-07-18T13:07:24.7618049Z [553/2435] Building Options.inc...
2019-07-18T13:07:25.0875071Z [554/2435] Building CXX object lib\ToolDrivers\llvm-lib\CMakeFiles\LLVMLibDriver.dir\LibDriver.cpp.obj
2019-07-18T13:07:25.0875847Z FAILED: lib/ToolDrivers/llvm-lib/CMakeFiles/LLVMLibDriver.dir/LibDriver.cpp.obj 
2019-07-18T13:07:25.0876541Z D:\a\1\s\build\bootstrap\debug\sccache-plus-cl.exe  /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_DEBUG_POINTER_IMPL="" -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\ToolDrivers\llvm-lib -ID:\a\1\s\src\llvm-project\llvm\lib\ToolDrivers\llvm-lib -Iinclude -ID:\a\1\s\src\llvm-project\llvm\include /nologo /MT /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Brepro /W4  -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wno-noexcept-type -Wno-comment /MT /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes /Folib\ToolDrivers\llvm-lib\CMakeFiles\LLVMLibDriver.dir\LibDriver.cpp.obj /Fdlib\ToolDrivers\llvm-lib\CMakeFiles\LLVMLibDriver.dir\LLVMLibDriver.pdb -c D:\a\1\s\src\llvm-project\llvm\lib\ToolDrivers\llvm-lib\LibDriver.cpp
2019-07-18T13:07:25.0877227Z clang-cl.exe: warning: argument unused during compilation: '-mno-incremental-linker-compatible' [-Wunused-command-line-argument]
2019-07-18T13:07:25.0877455Z In file included from D:\a\1\s\src\llvm-project\llvm\lib\ToolDrivers\llvm-lib\LibDriver.cpp:18:
2019-07-18T13:07:25.0877686Z In file included from D:\a\1\s\src\llvm-project\llvm\include\llvm/Bitcode/BitcodeReader.h:19:
2019-07-18T13:07:25.0878009Z In file included from D:\a\1\s\src\llvm-project\llvm\include\llvm/IR/ModuleSummaryIndex.h:27:
2019-07-18T13:07:25.0878155Z In file included from D:\a\1\s\src\llvm-project\llvm\include\llvm/IR/Module.h:23:
2019-07-18T13:07:25.0878301Z D:\a\1\s\src\llvm-project\llvm\include\llvm/IR/Attributes.h(73,14):  fatal error: 'llvm/IR/Attributes.inc' file not found
2019-07-18T13:07:25.0878387Z     #include "llvm/IR/Attributes.inc"
2019-07-18T13:07:25.0878720Z 1 error generated.
2019-07-18T13:07:25.1652973Z [555/2435] Building CXX object lib\ToolDrivers\llvm-dlltool\CMakeFiles\LLVMDlltoolDriver.dir\DlltoolDriver.cpp.obj
2019-07-18T13:07:25.1652973Z [555/2435] Building CXX object lib\ToolDrivers\llvm-dlltool\CMakeFiles\LLVMDlltoolDriver.dir\DlltoolDriver.cpp.obj
2019-07-18T13:07:25.1655529Z ninja: build stopped: subcommand failed.
2019-07-18T13:07:25.1760604Z command did not execute successfully, got: exit code: 1
2019-07-18T13:07:25.1760645Z 
2019-07-18T13:07:25.1760645Z 
2019-07-18T13:07:25.1760718Z build script failed, must exit now', C:\Program Files\Rust\.cargo\registry\src\github.com-1ecc6299db9ec823\cmake-0.1.38\src\lib.rs:813:5
2019-07-18T13:07:25.1765696Z  finished in 175.807
2019-07-18T13:07:25.1804949Z failed to run: D:\a\1\s\build\bootstrap\debug\bootstrap test src/tools/cargotest src/tools/cargo
2019-07-18T13:07:25.1805066Z Build completed unsuccessfully in 0:37:38
2019-07-18T13:07:25.1805066Z Build completed unsuccessfully in 0:37:38
2019-07-18T13:07:25.2576867Z ##[error]Bash exited with code '1'.
2019-07-18T13:07:25.3017895Z ##[section]Starting: Upload CPU usage statistics
2019-07-18T13:07:25.3119827Z ==============================================================================
2019-07-18T13:07:25.3119902Z Task         : Bash
2019-07-18T13:07:25.3119975Z Description  : Run a Bash script on macOS, Linux, or Windows

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)

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 18, 2019
@Xanewok
Copy link
Member Author

Xanewok commented Jul 19, 2019

2019-07-18T13:07:25.0878301Z D:\a\1\s\src\llvm-project\llvm\include\llvm/IR/Attributes.h(73,14):  fatal error: 'llvm/IR/Attributes.inc' file not found

Seems spurious?

@bors retry

@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 Jul 19, 2019
@bors
Copy link
Contributor

bors commented Jul 19, 2019

⌛ Testing commit ff63336 with merge e1693a9700e87126a97497ccf3ccc582ed2231f4...

@bors
Copy link
Contributor

bors commented Jul 19, 2019

⌛ Testing commit ff63336 with merge 527dce7...

bors added a commit that referenced this pull request Jul 19, 2019
Add an `after_expansion` callback in the driver

To format a given file RLS needs to know the Rust edition associated with it. It's not enough to look at the `edition` key in Cargo.toml - each crate target can have a different edition associated with it so the sure way to fetch a correct edition is to scan the input files used to compile a given crate target.

Right now this was done in the `after_analysis` callback of our shim, however this leads to other problems - if a crate cannot be successfully compiled (e.g. it has a type error) then a callback would not be invoked meaning we can't populate the files -> edition mapping.

However, doing this only after parsing is not enough, since expansion can pull in additional source files (e.g. by invoking `macro_rules! include_my_mod { () => { mod some_mod; }; }`).

Without copy-pasting the entire driver setup it's also not possible to expand the crate ourselves in the `after_parsing` callback - to expand crate we'd need to register plugins and initialize session ourselves. However, this is done normally after executing the callback itself, thus triggering the `Once::set` assertions in `Session::init_features`.

r? @Zoxc

cc @RalfJung @oli-obk this affects public driver interface used by Miri and Clippy
@bors
Copy link
Contributor

bors commented Jul 19, 2019

☀️ Test successful - checks-azure
Approved by: Zoxc
Pushing 527dce7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 19, 2019
@bors bors merged commit ff63336 into rust-lang:master Jul 19, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #62679!

Tested on commit 527dce7.
Direct link to PR: #62679

💔 clippy-driver on windows: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 miri on windows: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 miri on linux: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 rls on windows: test-pass → build-fail (cc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jul 19, 2019
Tested on commit rust-lang/rust@527dce7.
Direct link to PR: <rust-lang/rust#62679>

💔 clippy-driver on windows: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 miri on windows: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 miri on linux: test-fail → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 rls on windows: test-pass → build-fail (cc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @Xanewok, @rust-lang/infra).
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Jul 19, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request Jul 19, 2019
@Xanewok Xanewok mentioned this pull request Jul 19, 2019
matthiaskrgr added a commit to matthiaskrgr/rls that referenced this pull request Jul 19, 2019
Xanewok pushed a commit to Xanewok/rls that referenced this pull request Jul 21, 2019
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 22, 2019
Changes:
````
Fix breakage due to rust-lang#60913
Fix breakage due to rust-lang#62705
rustup rust-lang#62679
Update pulldown-cmark to 0.5.3
rustup rust-lang#62764
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
````
bors added a commit that referenced this pull request Jul 23, 2019
Update RLS

Supersedes #62537.
Closes #62803.

Fixes fallout after #62679.

r? @ghost
bors added a commit that referenced this pull request Jul 23, 2019
Update RLS

Supersedes #62537.
Closes #62803.

Fixes fallout after #62679.

r? @ghost
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 24, 2019
Changes:
````
update test stderr, not sure which rustc pull request caused this.
rustup rust-lang#62859
Fix tests for edition 2018 compatibility
Revert "Revert global fmt config and use `rustfmt::skip`"
Fix breakage due to rust-lang#60913
Fix breakage due to rust-lang#62705
Revert global fmt config and use `rustfmt::skip`
Fix fmt
rustup rust-lang#62679
Update pulldown-cmark to 0.5.3
rustup rust-lang#62764
Add test
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
Ignore generated fresh lifetimes in elision check.
````
bors added a commit that referenced this pull request Jul 24, 2019
submodules: update clippy from 164310d to f8e04ff

Changes:
````
update test stderr, not sure which rustc pull request caused this.
rustup #62859
Fix tests for edition 2018 compatibility
Revert "Revert global fmt config and use `rustfmt::skip`"
Fix breakage due to #60913
Fix breakage due to #62705
Revert global fmt config and use `rustfmt::skip`
Fix fmt
rustup #62679
Update pulldown-cmark to 0.5.3
rustup #62764
Add test
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
Ignore generated fresh lifetimes in elision check.
````

fixes clippy toolstate
r? @Manishearth
@Xanewok Xanewok deleted the after-expansion branch July 25, 2019 13:57
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 27, 2019
Changes:
````
ci: temporarily disable rustfmt checks/tetss since it's broken for nightly
rustup rust-lang#62964
Bump version of clippy_dummy
update test stderr, not sure which rustc pull request caused this.
rustup rust-lang#62859
Fix tests for edition 2018 compatibility
Revert "Revert global fmt config and use `rustfmt::skip`"
Fix breakage due to rust-lang#60913
Fix breakage due to rust-lang#62705
Revert global fmt config and use `rustfmt::skip`
Fix fmt
rustup rust-lang#62679
Update pulldown-cmark to 0.5.3
rustup rust-lang#62764
Add test
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
Ignore generated fresh lifetimes in elision check.
````
bors added a commit that referenced this pull request Jul 28, 2019
submodules: update clippy from 164310d to dc69a5c

Changes:
````
ci: temporarily disable rustfmt checks/tetss since it's broken for nightly
rustup #62964
Bump version of clippy_dummy
update test stderr, not sure which rustc pull request caused this.
rustup #62859
Fix tests for edition 2018 compatibility
Revert "Revert global fmt config and use `rustfmt::skip`"
Fix breakage due to #60913
Fix breakage due to #62705
Revert global fmt config and use `rustfmt::skip`
Fix fmt
rustup #62679
Update pulldown-cmark to 0.5.3
rustup #62764
Add test
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
Ignore generated fresh lifetimes in elision check.
````

fixes clippy toolstate
r? @Manishearth
Centril added a commit to Centril/rust that referenced this pull request Jul 30, 2019
bors added a commit that referenced this pull request Jul 30, 2019
Update RLS and Rustfmt

Supersedes #62537.
Closes #62803.

Fixes fallout after #62679.

r? @ghost
bors added a commit that referenced this pull request Jul 30, 2019
Update RLS and Rustfmt

Supersedes #62537.
Closes #62803.

Fixes fallout after #62679.

r? @ghost
@Xanewok Xanewok mentioned this pull request Aug 5, 2019
bors added a commit that referenced this pull request Aug 8, 2019
Update RLS

Most importantly this includes:
* Collect file -> edition mapping after AST expansion ([#1513](rust-lang/rls#1513)) (enabled by #62679)
* Fix rustfmt during builds by reading edition from manifest ([#1533](rust-lang/rls#1533))

Which fixes the annoying problem where users couldn't format via RLS while `cargo fmt` worked.

The beta cut-off is drawing near and I'd like to make sure this lands before then.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 5, 2020
Changes:
````
ci: temporarily disable rustfmt checks/tetss since it's broken for nightly
rustup rust-lang/rust#62964
Bump version of clippy_dummy
update test stderr, not sure which rustc pull request caused this.
rustup rust-lang/rust#62859
Fix tests for edition 2018 compatibility
Revert "Revert global fmt config and use `rustfmt::skip`"
Fix breakage due to rust-lang/rust#60913
Fix breakage due to rust-lang/rust#62705
Revert global fmt config and use `rustfmt::skip`
Fix fmt
rustup rust-lang/rust#62679
Update pulldown-cmark to 0.5.3
rustup rust-lang/rust#62764
Add test
Format code
Decrease maximum length for stderr files
Improved imports
Fix "unkown clippy lint" error in UI test.
Corrections for PR review.
Implement lint for inherent to_string() method.
UI Test Cleanup: Extract match_ref_pats tests
Update UI tests
Allow no_effect lint
Remove comment
cargo fmt
UI Test Cleanup: Split up checked_unwrap tests
Removed lintining on never type.
UI Test Cleanup: Split out out_of_bounds_indexing
false positives fixes of `implicit_return`
Ignore generated fresh lifetimes in elision check.
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

5 participants