Add rust_analyzer as a predefined tool by Veykril · Pull Request #125241 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation19 Commits1 Checks6 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
r? @BoxyUwU
rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
I am not the correct person to review this.
r? compiler
Also, I would expect this to go through the MCP process. This is adding a new attribute which can be used on stable.
I think there is still an open design question around how tools are registered, and I think caution should be used when adding more built-in attributes. I think there should be some mechanism for being able to support tool attributes that doesn't bake them into the language, or require different compiler implementations to also hard-code this list. My concern is that if I handed this code to some other compiler:
#[rustfmt::skip] fn f() {}
should all implementations know what rustfmt is? What about the other tools listed here? If so, then that bakes that into the language, or else makes code non-portable (assuming an error is generated for an unknown attribute).
#66079 has implemented #![register_tool(my_tool)] for this purpose, but then do we expect anyone that uses clippy or rustfmt (or any of these other built-in-tools) to have to specify something like #![register_tool(clippy)]?
I'm not quite seeing how tools could be registered, while keeping code portable across implementations. One option would be to keep all tools in a tool namespace (like #[tool::rust_analyzer::foo]), and then ignore everything under tool (similar to the diagnostic namespace).
cc @petrochenkov FYI, not sure if you have an opinion here.
From what I remember the existing tools were hardcoded because #![register_tool] wasn't yet stable, after it's stable all tools were supposed to be registered explicitly.
(I also missed the moment miri was added to the list.)
Is there any specific motivation for adding rust_analyzer to the list now?
(I also missed the moment
miriwas added to the list.)
I saw the miri one being added without second thoughts and figured rust project tools would be just fine.
after it's stable all tools were supposed to be registered explicitly.
How would that even work? Removing them would break things so that doesn't seem like an option?
Is there any specific motivation for adding
rust_analyzerto the list now?
rust-lang/rust-analyzer#11556 has a bunch of ideas listed, but if you ask right now there isn't a concrete plan yet. This would make experimenting for these feature for r-a nicer though. I'm fine with this being closed though for now, if we don't want to add every rust-project tool here (though as I read the register_tool issue it does feel like its ages away from ever being stabilized, and as eric pointed out, the feature is questionable designed in the first place)
I think this makes sense to do. Tools have been added to this list in the past without any broader approvals other than the review (most recently, miri in #124293), but I think we probably should do an MCP for this (and tools going forward). After that, r=me.
How would that even work? Removing them would break things so that doesn't seem like an option?
They can be removed on an edition bump.
@rustbot labels +I-lang-nominated +T-lang
Generally the attribute namespace is a lang concern, so let's nominate for lang to decide on the handling here.
Nominated for discussion during a lang team meeting.
Relevant to the language team, which will review and decide on the PR/issue.
labels
@Veykril 👍 for having an attribute namespace for rust-analyzer. Do you want this to be rust_analyzer, or just analyzer?
Well the tool is called rust-analyzer so it should be rust_analyzer :)
@rustbot labels -I-lang-nominated
We discussed this in the lang triage call today.
We affirmed that lang owns the attribute namespace, and so these additions require lang approval.
(We're discussing various policies that would perhaps delegate these decisions in certain cases or reframe the problem so that these additions for tools weren't needed, but those all are all future work.)
On this question specifically, for rust-analyzer, we're OK with adding rust_analyzer (or analyzer, if that were to be preferred) as a predefined tool as this PR does. Consequently, this PR is OK to move forward as far as lang is concerned.
In terms of the general principles on which we're accepting this and would consider others in the future, we discussed that the ones we add must have rules for what is valid that we are willing to encode into rustc, and so, therefore, in practice the tool will need to be part of the Rust project or be some other well-known crate.
In terms of a broader scheme that could obviate needing to add these, we discussed both the "worse is better" approach of, e.g. allowing any attribute with :: (or some other similarly permissive rule), and the "better is better" approach of having something type checked, e.g. along the lines of Java's similar attributes.
📌 Commit 4c5e954 has been approved by davidtwco
It is now in the queue for this repository.
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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…dtwco
Add rust_analyzer as a predefined tool
Given all the other rust-lang tools have it, I'd expect r-a to have it too. (we have a few ideas for using this rust-lang/rust-analyzer#11556).
@bors r-
needs x.py fmt 😿
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
📌 Commit 3390159 has been approved by davidtwco
It is now in the queue for this repository.
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
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 4 pull requests
Successful merges:
- rust-lang#125241 (Add
rust_analyzeras a predefined tool) - rust-lang#126213 (Update docs for AtomicBool/U8/I8 with regard to alignment)
- rust-lang#126414 (Tier 2 std support must always be known)
- rust-lang#126882 (Special case when a code line only has multiline span starts)
r? @ghost
@rustbot modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…dtwco
Add rust_analyzer as a predefined tool
Given all the other rust-lang tools have it, I'd expect r-a to have it too. (we have a few ideas for using this rust-lang/rust-analyzer#11556).
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#125241 - Veykril:tool-rust-analyzer, r=davidtwco
Add rust_analyzer as a predefined tool
Given all the other rust-lang tools have it, I'd expect r-a to have it too. (we have a few ideas for using this rust-lang/rust-analyzer#11556).
jaisnan pushed a commit to jaisnan/rust-dev that referenced this pull request
Update Rust toolchain from nightly-2024-06-24 to nightly-2024-06-25
without any other source changes.
This is an automatically generated pull request. If any of the CI checks
fail, manual intervention is required. In such a case, review the
changes at https://github.com/rust-lang/rust from
rust-lang@bcf94de
up to
rust-lang@6b0f4b5.
The log for this commit range is:
rust-lang@6b0f4b5ec3 Auto merge of
rust-lang#126914 - compiler-errors:rollup-zx0hchm, r=compiler-errors
rust-lang@16bd6e25e1 Rollup merge of
rust-lang#126911 - oli-obk:do_not_count_errors, r=compiler-errors
rust-lang@59c258f51f Rollup merge of
rust-lang#126909 - onur-ozkan:add-kobzol, r=matthiaskrgr
rust-lang@85eb835a14 Rollup merge of
rust-lang#126904 - GrigorenkoPV:nonzero-fixme, r=joboet
rust-lang@a7721a0373 Rollup merge of
rust-lang#126899 - GrigorenkoPV:suggest-const-block, r=davidtwco
rust-lang@9ce2a070b3 Rollup merge of
rust-lang#126682 - Zalathar:coverage-attr, r=lcnr
rust-lang@49bdf460a2 Rollup merge of
rust-lang#126673 - oli-obk:dont_rely_on_err_reporting, r=compiler-errors
rust-lang@46e43984d1 Rollup merge of
rust-lang#126413 - matthiaskrgr:crshmsg, r=oli-obk
rust-lang@ed460d2eaa Rollup merge of
rust-lang#125575 - dingxiangfei2009:derive-smart-ptr, r=davidtwco
rust-lang@c77dc28f87 Rollup merge of
rust-lang#125082 - kpreid:const-uninit, r=dtolnay
rust-lang@faa28be2f1 Rollup merge of
rust-lang#124712 - Enselic:deprecate-inline-threshold, r=pnkfelix
rust-lang@00e5f5886a Rollup merge of
rust-lang#124460 - long-long-float:show-notice-about-enum-with-debug, r=pnkfelix
rust-lang@d8d5732456 Auto merge of
rust-lang#126784 - scottmcm:smaller-terminator, r=compiler-errors
rust-lang@13fca73f49 Replace
MaybeUninit::uninit_array() with array repeat expression.
rust-lang@5a3e2a4e92 Auto merge of
rust-lang#126523 - joboet:the_great_big_tls_refactor, r=Mark-Simulacrum
rust-lang@45261ff2ec add @Kobzol to
bootstrap team for triagebot
rust-lang@84474a25a4 Small fixme in core
now that NonZero is generic
rust-lang@50a02ed789 std: fix wasm builds
rust-lang@8fc6b3de19 Separate the mir
body lifetime from the other lifetimes
rust-lang@1c4d0ced58 Separate the
lifetimes of the BorrowckInferCtxt from the other borrowed items
rust-lang@d371d17496 Auto merge of
rust-lang#126900 - matthiaskrgr:rollup-24ah97b, r=matthiaskrgr
rust-lang@8ffb5f936a compiletest: make
the crash test error message abit more informative
rust-lang@a80ee9159b Rollup merge of
rust-lang#126882 - estebank:multiline-order, r=WaffleLapkin
rust-lang@8bfde609e2 Rollup merge of
rust-lang#126414 - ChrisDenton:target-known, r=Nilstrieb
rust-lang@94b9ea417d Rollup merge of
rust-lang#126213 - zachs18:atomicbool-u8-i8-from-ptr-alignment, r=Nilstrieb
rust-lang@9d24ecc37b Rollup merge of
rust-lang#125241 - Veykril:tool-rust-analyzer, r=davidtwco
rust-lang@ba5ec1fc5c Suggest inline const
blocks for array initialization
rust-lang@06c072f158 Auto merge of
rust-lang#126788 - GuillaumeGomez:migrate-rustdoc-tests-syntax, r=fmease,oli-obk
rust-lang@1852141219 coverage: Bless
coverage attribute tests
rust-lang@b7c057c9b2 coverage: Always
error on #[coverage(..)] in unexpected places
rust-lang@a000fa8b54 coverage: Tighten
validation of #[coverage(off)] and #[coverage(on)]
rust-lang@b5dfeba0e1 coverage: Forbid
multiple #[coverage(..)] attributes
rust-lang@6909feab8e Allow numbers in
rustdoc tests commands
rust-lang@4e258bb4c3 Fix tidy issue for
rustdoc tests commands
rust-lang@51fedf65ff Remove commands
duplication between compiletest and tests/rustdoc
rust-lang@1b67035579 Update
tests/rustdoc to new test syntax
rust-lang@d3ec92e16e Move tests/rustdoc
testsuite to //@ syntax
rust-lang@2c243d9570 Auto merge of
rust-lang#126891 - matthiaskrgr:rollup-p6dl1gk, r=matthiaskrgr
rust-lang@b94d2754b5 Rollup merge of
rust-lang#126888 - compiler-errors:oops-debug-printing, r=dtolnay
rust-lang@9892b3e9fe Rollup merge of
rust-lang#126854 - devnexen:std_unix_os_fallback_upd, r=Mark-Simulacrum
rust-lang@3108dfaced Rollup merge of
rust-lang#126849 - workingjubilee:correctly-classify-arm-low-dregs, r=Amanieu
rust-lang@dcace866f0 Rollup merge of
rust-lang#126845 - rust-lang:cargo_update, r=Mark-Simulacrum
rust-lang@21850f5bd8 Rollup merge of
rust-lang#126807 - devnexen:copy_file_macos_simpl, r=Mark-Simulacrum
rust-lang@b24e3df0df Rollup merge of
rust-lang#126754 - compiler-errors:use-rustfmt, r=calebcartwright
rust-lang@ad0531ae0d Rollup merge of
rust-lang#126455 - surechen:fix_126222, r=estebank
rust-lang@7babf99ea9 Rollup merge of
rust-lang#126298 - heiher:loongarch64-musl-ci, r=Mark-Simulacrum
rust-lang@9a591ea1ce Rollup merge of
rust-lang#126177 - carbotaniuman:unsafe_attr_errors, r=jieyouxu
rust-lang@25446c25fc Remove stray println
from rustfmt
rust-lang@d49994b060 Auto merge of
rust-lang#126023 - amandasystems:you-dropped-this-again, r=nikomatsakis
rust-lang@a23917cfd0 Add hard error and
migration lint for unsafe attrs
rust-lang@284437d434 Special case when a
code line only has multiline span starts
rust-lang@f1be59fa72 SmartPointer
derive-macro
rust-lang@a426d6fdf0 Implement use<>
formatting in rustfmt
rust-lang@16fef40896 Promote
loongarch64-unknown-linux-musl to Tier 2 with host tools
rust-lang@03d73fa6ba ci: Add support for
dist-loongarch64-musl
rust-lang@fc50acae90 fix build
rust-lang@bd9ce3e074
std::unix::os::home_dir: fallback's optimisation.
rust-lang@0d8f734172 compiler: Fix arm32
asm issues by hierarchically sorting reg classes
rust-lang@e8b5ba1111 For [E0308]:
mismatched types, when expr is in an arm's body, not add semicolon ';'
at the end of it.
rust-lang@990535723d cargo update
rust-lang@b28efb11af Save 2 pointers in
TerminatorKind (96 → 80 bytes)
rust-lang@65530ba100 std::unix::fs: copy
simplification for apple.
rust-lang@339015920d Add rust_analyzer
as a predefined tool
rust-lang@3f2f8438b4 Ensure we don't
accidentally succeed when we want to report an error
rust-lang@32f9b8bf76 std: rename module
for clarity
rust-lang@35f050b8da std: update TLS
module documentation
rust-lang@b2f29edc81 std: use the c_int
from core::ffi instead of libc
rust-lang@d70f071392 std: simplify
#[cfg]s for TLS
rust-lang@d630f5da7a Show notice about
"never used" for enum
rust-lang@f3facf1175 std: refactor the
TLS implementation
rust-lang@f5f067bf9d Deprecate no-op
codegen option -Cinline-threshold=...
rust-lang@651ff643ae Fix typo in
-Cno-stack-check deprecation warning
rust-lang@3af624272a rustc_codegen_ssa:
Remove unused ModuleConfig::inline_threshold
rust-lang@34e6ea1446 Tier 2 std support
must always be known
rust-lang@2d4cb7aa5a Update docs for
AtomicU8/I8.
rust-lang@7885c7b7b2 Update safety docs
for AtomicBool::from_ptr.
rust-lang@7b5b7a7010 Remove confusing
use_polonius flag and do less cloning
Co-authored-by: tautschnig 1144736+tautschnig@users.noreply.github.com
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the language team, which will review and decide on the PR/issue.