Make feature key optional for rustc_stable, rustc_const_stable attributes #88588

jplatte · 2021-09-02T10:46:09Z

… so partial stabilizations no longer have to make up a feature name that never existed as unstable for the stabilized subset of the API.

This seems to break at runtime, presumably because the on-disk representation of the affected rustc_attr types changed. Not sure how to proceed here. If sbd. could give me a hint about how this could be made to work that would be appreciated.

Brief Zulip thread about this: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/.60feature.60.20key.20in.20rustc_stable.20and.20rustc_const_stable

rust-highfive · 2021-09-02T10:46:12Z

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

jplatte · 2021-09-02T11:23:57Z

Updated rustdoc & clippy, maybe now the failure I was seeing will pop up here. Although I'm not seeing it anymore myself after commenting out profile = "compiler" in config.toml 🤔

…utes … so partial stabilizations no longer have to make up a feature name that never existed as unstable for the stabilized subset of the API.

jplatte · 2021-09-02T11:34:08Z

src/librustdoc/lib.rs

@@ -12,6 +12,7 @@
 #![feature(crate_visibility_modifier)]
 #![feature(never_type)]
 #![feature(once_cell)]
+#![feature(option_result_contains)]


Just noticed I left this in but removed the use of Option::contains. Will remove with the next push.

m-ou-se · 2021-09-02T11:39:47Z

I'm not sure what this will achieve. Unstable feature names are unstable. Feature names changing or changing meaning happens regularly, not only during partial stabilizations. We reguarly change the api of something during stabilization. Then it does use the same name as an unstable feature before, but the api is different, so it gets blurry whether that's the same feature as before or not. Sometimes features get split and stay unstable for a while, so the old feature name disappears completely. Sometimes features get renamed or split, unstably, but then later stabilized in the same version, which means that there was never a 'release' in which it was named like that. Sometimes we stabilize most of something, and keep the feature name for the stabilized feature, and rename the feature of the remaining unstable ones.

If you want to work towards more stable 'unsteable feature names', I don't think this will help much. And even for that purpose, it might even be good to give every stable feature a name, even if that name has not appeared as unstable before. Then you can at least talk about 'the first rust version that had abc_xyz stable', even if that feature didn't appear as explicitly unstable in any version.

rust-log-analyzer · 2021-09-02T11:43:24Z

The job x86_64-gnu-llvm-10 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)


---- [ui] ui/stability-attribute/stability-attribute-sanity.rs stdout ----
diff of stderr:

40 LL |     #[unstable(feature = "b")]
42 
- error[E0546]: missing 'feature'
-   --> $DIR/stability-attribute-sanity.rs:31:5
-    |
-    |
- LL |     #[stable(since = "a")]
- 
49 error[E0542]: missing 'since'
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
50   --> $DIR/stability-attribute-sanity.rs:36:5
50   --> $DIR/stability-attribute-sanity.rs:36:5
51    |

120 LL | #[rustc_deprecated(since = "a", reason = "text")]
122 
- error: aborting due to 19 previous errors
+ error: aborting due to 18 previous errors
124 
---
To only update this specific test, also pass `--test-args stability-attribute/stability-attribute-sanity.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/stability-attribute/stability-attribute-sanity" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/stability-attribute/stability-attribute-sanity/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error[E0541]: unknown meta item 'reason'
   |
   |
LL |     #[stable(feature = "a", since = "b", reason)] //~ ERROR unknown meta item 'reason' [E0541]
   |                                          ^^^^^^ expected one of `since`, `note`
error[E0539]: incorrect meta item
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:11:29
   |
   |
LL |     #[stable(feature = "a", since)] //~ ERROR incorrect meta item [E0539]

error[E0539]: incorrect meta item
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:14:14
   |
   |
LL |     #[stable(feature, since = "a")] //~ ERROR incorrect meta item [E0539]

error[E0539]: incorrect meta item
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:17:29
   |
   |
LL |     #[stable(feature = "a", since(b))] //~ ERROR incorrect meta item [E0539]

error[E0539]: incorrect meta item
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:20:14
   |
   |
LL |     #[stable(feature(b), since = "a")] //~ ERROR incorrect meta item [E0539]

error[E0546]: missing 'feature'
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:25:5
   |
   |
LL |     #[unstable(issue = "none")] //~ ERROR missing 'feature' [E0546]

error[E0547]: missing 'issue'
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:28:5
   |
   |
LL |     #[unstable(feature = "b")] //~ ERROR missing 'issue' [E0547]

error[E0542]: missing 'since'
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:36:5
   |
   |
LL |     #[stable(feature = "a")] //~ ERROR missing 'since' [E0542]

error[E0542]: missing 'since'
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:40:5
   |
   |
LL |     #[rustc_deprecated(reason = "a")] //~ ERROR missing 'since' [E0542]

error[E0543]: missing 'reason'
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:44:5
   |
   |
LL |     #[rustc_deprecated(since = "a")] //~ ERROR missing 'reason' [E0543]

error[E0544]: multiple stability levels
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:49:1
   |
   |
LL | #[stable(feature = "a", since = "b")] //~ ERROR multiple stability levels [E0544]

error[E0544]: multiple stability levels
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:53:1
   |
   |
LL | #[unstable(feature = "b", issue = "none")] //~ ERROR multiple stability levels [E0544]

error[E0544]: multiple stability levels
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:57:1
   |
   |
LL | #[stable(feature = "a", since = "b")] //~ ERROR multiple stability levels [E0544]

error[E0550]: multiple deprecated attributes
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:62:1
   |
   |
LL | #[rustc_deprecated(since = "b", reason = "text")]
   | ------------------------------------------------- first deprecation attribute
LL | #[rustc_deprecated(since = "b", reason = "text")] //~ ERROR multiple deprecated attributes
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ repeated deprecation attribute
error[E0544]: multiple stability levels
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:64:1
   |
   |
LL | #[rustc_const_unstable(feature = "d", issue = "none")] //~ ERROR multiple stability levels

error: invalid stability version found
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:60:1
   |
   |
LL | #[stable(feature = "a", since = "b")] //~ ERROR invalid stability version found
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid stability version
LL | pub const fn multiple4() { }
   | ---------------------------- the stability attribute annotates this item

error: invalid deprecation version found
error: invalid deprecation version found
  --> /checkout/src/test/ui/stability-attribute/stability-attribute-sanity.rs:67:1
   |
LL | #[stable(feature = "a", since = "1.0.0")] //~ ERROR invalid deprecation version found
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid deprecation version
LL | #[rustc_deprecated(since = "invalid", reason = "text")]
LL | fn invalid_deprecation_version() {}


error[E0549]: rustc_deprecated attribute must be paired with either stable or unstable attribute
   |
   |
LL | #[rustc_deprecated(since = "a", reason = "text")]

error: aborting due to 18 previous errors

Some errors have detailed explanations: E0539, E0541, E0542, E0543, E0544, E0546, E0547, E0549, E0550.
---
test result: FAILED. 11988 passed; 1 failed; 102 ignored; 0 measured; 0 filtered out; finished in 130.14s



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-10/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "10.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine frontendopenmp fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcerror orcjit passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86utils xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"


Build completed unsuccessfully in 0:13:07

jplatte · 2021-09-02T11:49:26Z

I'm not sure what this will achieve.

I think this achieves:

Less confusion when looking at stabilization PRs as sbd. who isn't that familiar with these procedures. I maintain https://caniuse.rs and have added a bunch of entirely useless data to it because of this. (I'm still not sure what I will do with it – remove entirely or note the feature flag before stabilization)
Simpler / less weird procedure for stabilization PRs. I wanted to post a stabilization PR for BTree{Map,Set}::new as const fn earlier which are under the feature flag const_btree_new, but so are the is_empty and len methods for those types for some reason. It just didn't feel right at all to put a different, entirely meaningless feature name attribute to them.

Then you can at least talk about 'the first rust version that had abc_xyz stable', even if that feature didn't appear as explicitly unstable in any version.

Since we're talking about library features, not language features here, where would this identifier be any clearer than the item name? I'd say "the first rust version that had Abc::xyz stable" is clearer. I also don't think I ever had much of a problem giving library features titles for caniuse.rs.

bors · 2021-09-09T00:23:04Z

☔ The latest upstream changes (presumably #80522) made this pull request unmergeable. Please resolve the merge conflicts.

jplatte · 2021-09-17T14:31:00Z

One more thing that would be simplified: New trait impls (which are always insta-stable) for which authors would no longer have to make up a meaningless feature name.

ping @wesleywiser @m-ou-se what do I do to move this forwards (towards a decision on whether this changes makes sense or not)? Open an MCP?

wesleywiser · 2021-09-20T17:21:44Z

what do I do to move this forwards (towards a decision on whether this changes makes sense or not)?

This feels to me like a question for the libs team. I'm not sure what processes they use but bumping that Zulip thread would probably be a good idea. I'm happy to review the implementation once they've signed off on the feature design.

jackh726 · 2021-10-21T18:35:18Z

(Waiting on T-libs)

jplatte · 2021-12-17T10:57:11Z

I posted to the Zulip thread again on Oct 6, but got no responses. Is this going to be discussed?

m-ou-se · 2022-03-09T18:20:45Z

This was discussed in the library team meeting last week. Our conclusion was that we don't think this change really solves much, while it does make it easier to make the mistake of not having a feature name in cases where we do want it. We also do not want to give the impression that unstable feature names are something 'stable' that can be tracked and doesn't change meaning over time.

jplatte · 2022-03-09T18:25:43Z

What does it mean that this is now S-waiting-on-author? Shouldn't it just be closed?

m-ou-se · 2022-03-09T21:18:54Z

I didn't close it yet because I wanted to make there's space for you to respond. But unless you have new arguments to change opinions here, this should probably be closed, yes.

rust-highfive assigned wesleywiser Sep 2, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2021

This comment has been minimized.

Sign in to view

jplatte force-pushed the opt-rustc_stable-feature branch from 4e11681 to 4336cca Compare September 2, 2021 11:22

Make feature key optional for rustc_stable, rustc_const_stable attrib…

bf8a53e

…utes … so partial stabilizations no longer have to make up a feature name that never existed as unstable for the stabilized subset of the API.

jplatte force-pushed the opt-rustc_stable-feature branch from 4336cca to bf8a53e Compare September 2, 2021 11:27

jplatte commented Sep 2, 2021

View reviewed changes

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 8, 2021

m-ou-se 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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Mar 9, 2022

m-ou-se closed this Mar 9, 2022

jplatte deleted the opt-rustc_stable-feature branch March 9, 2022 21:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make feature key optional for rustc_stable, rustc_const_stable attributes #88588

Make feature key optional for rustc_stable, rustc_const_stable attributes #88588

jplatte commented Sep 2, 2021 •

edited

Loading

rust-highfive commented Sep 2, 2021

This comment has been minimized.

jplatte commented Sep 2, 2021

jplatte Sep 2, 2021

m-ou-se commented Sep 2, 2021

rust-log-analyzer commented Sep 2, 2021

jplatte commented Sep 2, 2021 •

edited

Loading

bors commented Sep 9, 2021

jplatte commented Sep 17, 2021

wesleywiser commented Sep 20, 2021

jackh726 commented Oct 21, 2021

jplatte commented Dec 17, 2021

m-ou-se commented Mar 9, 2022

jplatte commented Mar 9, 2022

m-ou-se commented Mar 9, 2022

Make feature key optional for rustc_stable, rustc_const_stable attributes #88588

Make feature key optional for rustc_stable, rustc_const_stable attributes #88588

Conversation

jplatte commented Sep 2, 2021 • edited Loading

rust-highfive commented Sep 2, 2021

This comment has been minimized.

jplatte commented Sep 2, 2021

jplatte Sep 2, 2021

Choose a reason for hiding this comment

m-ou-se commented Sep 2, 2021

rust-log-analyzer commented Sep 2, 2021

jplatte commented Sep 2, 2021 • edited Loading

bors commented Sep 9, 2021

jplatte commented Sep 17, 2021

wesleywiser commented Sep 20, 2021

jackh726 commented Oct 21, 2021

jplatte commented Dec 17, 2021

m-ou-se commented Mar 9, 2022

jplatte commented Mar 9, 2022

m-ou-se commented Mar 9, 2022

jplatte commented Sep 2, 2021 •

edited

Loading

jplatte commented Sep 2, 2021 •

edited

Loading