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

Deprecate-in-future the constants superceded by RFC 2700 #80958

Merged
merged 1 commit into from
Jan 21, 2021

Conversation

bstrie
Copy link
Contributor

@bstrie bstrie commented Jan 12, 2021

Successor to #78335, re-opened after addressing the issues tracked in #68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via rustc_deprecated(since="TBD", see #78381). We might call this soft deprecation; unlike with deprecation, users will not receive warnings when compiling code that uses these items unless they opt-in via #[warn(deprecated_in_future)]. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see #68490 (comment)).

@rust-highfive
Copy link
Collaborator

r? @cramertj

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 12, 2021
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9000/11250
.................................................................................................... 9100/11250
.................................................................................................... 9200/11250
.............................................i......i............................................... 9300/11250
....................................................................................iiiiii..iiiiii.i 9400/11250
.................................................................................................... 9600/11250
.................................................................................................... 9700/11250
.................................................................................................... 9800/11250
.................................................................................................... 9900/11250
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 27 tests
iiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.061 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
 finished in 9.747 seconds
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii....i..i...ii..........iiii.........i......i..i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.28s

 finished in 2.356 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
failures:

---- [rustdoc] rustdoc/reexport-check.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/reexport-check" "/checkout/src/test/rustdoc/reexport-check.rs"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
7: @has check failed
 `XPATH PATTERN` did not match
 // @has 'foo/index.html' '//tr[@class="module-item"]' 'i32'
Encountered 1 errors

------------------------------------------


info: generating a diff against nightly rustdoc
thread '[rustdoc] rustdoc/reexport-check.rs' panicked at 'failed to exec `"rustc" "/checkout/src/test/rustdoc/auxiliary/reexport-check.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/reexport-check/auxiliary" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--crate-type" "dylib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/reexport-check/auxiliary"`', src/tools/compiletest/src/runtest.rs:1871:33


failures:
    [rustdoc] rustdoc/reexport-check.rs
    [rustdoc] rustdoc/reexport-check.rs

test result: FAILED. 408 passed; 1 failed; 2 ignored; 0 measured; 0 filtered out; finished in 36.22s



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" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "rustdoc" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-9/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "9.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 codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver engine executionengine 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 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" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:17:57

@jyn514 jyn514 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 13, 2021
@KodrAus
Copy link
Contributor

KodrAus commented Jan 13, 2021

Nice! I wasn’t aware of TBD.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 13, 2021

📌 Commit c0c607c has been approved by KodrAus

@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 Jan 13, 2021
@bstrie
Copy link
Contributor Author

bstrie commented Jan 14, 2021

Nice! I wasn’t aware of TBD.

@KodrAus Well, I only just implemented it. :) I imagine it will be worth keeping in mind for any future deprecation discussions.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 14, 2021
Deprecate-in-future the constants superceded by RFC 2700

Successor to rust-lang#78335, re-opened after addressing the issues tracked in rust-lang#68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via `rustc_deprecated(since="TBD"`, see rust-lang#78381). We might call this *soft deprecation*; unlike with deprecation, users will *not* receive warnings when compiling code that uses these items *unless* they opt-in via `#[warn(deprecated_in_future)]`. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see rust-lang#68490 (comment)).
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 14, 2021
Deprecate-in-future the constants superceded by RFC 2700

Successor to rust-lang#78335, re-opened after addressing the issues tracked in rust-lang#68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via `rustc_deprecated(since="TBD"`, see rust-lang#78381). We might call this *soft deprecation*; unlike with deprecation, users will *not* receive warnings when compiling code that uses these items *unless* they opt-in via `#[warn(deprecated_in_future)]`. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see rust-lang#68490 (comment)).
@JohnTitor
Copy link
Member

Failed in rollup (#80999 (comment)):

error: use of module `core::i64` that will be deprecated in a future Rust version: all constants in this module replaced by associated constants on `i64`
 --> library/std/src/sys/unix/process/zircon.rs:4:5
4 | use crate::i64;
  |     ^^^^^^^^^^
  |
  |
  = note: `-D deprecated-in-future` implied by `-D warnings`

error: use of constant `core::i64::MAX` that will be deprecated in a future Rust version: replaced by the `MAX` associated constant on this type
  --> library/std/src/sys/unix/process/zircon.rs:19:41
   |
19 | pub const ZX_TIME_INFINITE: zx_time_t = i64::MAX;

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2021
@bstrie
Copy link
Contributor Author

bstrie commented Jan 15, 2021

Removed that crate::i64 in the Zircon target, plus while I was at it I also found and fixed few misleading crate::foo in the stdlib docs. Ready for re-approve.

library/core/src/convert/mod.rs Outdated Show resolved Hide resolved
library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved
library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved
library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved
library/core/src/iter/traits/marker.rs Outdated Show resolved Hide resolved
@bstrie
Copy link
Contributor Author

bstrie commented Jan 21, 2021

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 21, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 21, 2021

@bors r=KodrAus

@bors
Copy link
Contributor

bors commented Jan 21, 2021

📌 Commit 6f3df00 has been approved by KodrAus

@bors
Copy link
Contributor

bors commented Jan 21, 2021

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@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 Jan 21, 2021
@bors
Copy link
Contributor

bors commented Jan 21, 2021

⌛ Testing commit 6f3df00 with merge 339e196...

@bors
Copy link
Contributor

bors commented Jan 21, 2021

☀️ Test successful - checks-actions
Approved by: KodrAus
Pushing 339e196 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 21, 2021
@bors bors merged commit 339e196 into rust-lang:master Jan 21, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 21, 2021
@rylev
Copy link
Member

rylev commented Jan 26, 2021

@bstrie this caused a relatively small performance regression in the regex-check perf test. I don't see why that might be the case. Do you have any thoughts? Both expand_crate and hir_lowering seem to be affected.

Edit (Mark): we've also noted this for the compiler team in perf nags.

@bstrie
Copy link
Contributor Author

bstrie commented Jan 26, 2021

@rylev Regarding code in the compiler, this PR does exactly four things:

  1. Change doc comments on stdlib items
  2. Remove unused imports
  3. Add rustc_deprecated attributes to items
  4. Add allow(deprecated) attributes to re-exports

The only possible new codepaths that the compiler would execute as a result of this PR would be in code making use of the newly-deprecated items. regex-check is just running cargo check on the regex crate, yes? regex does appear to be making use of the old items, due to the fact that regex currently supports Rust 1.28 whereas the replacements for these items were stabilized in 1.43. That's the only explanation that springs to mind.

@rylev
Copy link
Member

rylev commented Jan 29, 2021

@bstrie I thought that deprecation warnings were opt-in, so would these code paths actually fire on any of these code bases?

@bstrie
Copy link
Contributor Author

bstrie commented Jan 29, 2021

@rylev deprecated_in_future is an allow-by-default lint, but that still means that whenever you use a deprecated_in_future item the compiler will need to check if your crate has configured the lint to emit a warning or a denial. Furthermore, the way deprecated_in_future was designed means that for every usage it will check whether or not the current version of the compiler is greater than the since version, and upgrade the lint to deprecated if so.

I find it unlikely that the deprecation lint specifically could be responsible for any regressions; these are simple checks with no computational complexity, and regex only has like 20 uses of the deprecated constants in the first place (though I didn't check to see if they were in macro expansions). My first instinct would be that something deeper in the lint/attribute machinery is more expensive/heavyweight than expected, but I'm not at all familiar with this part of the compiler.

Though if it were a problem with the deprecation lint itself I'd start by investigating the rustc version lookup that happens, since for whatever reason the rustc version appears to be stored in TLS rather than just being a constant string.

@jyn514
Copy link
Member

jyn514 commented Jan 29, 2021

bstrie I thought that deprecation warnings were opt-in, so would these code paths actually fire on any of these code bases?

see also #74704

@bstrie
Copy link
Contributor Author

bstrie commented Feb 14, 2021

With regard to the mention of #74704, after some research it appears that there are two kinds of lints in rustc: "built-in" lints that are emitted directly by the compiler, and normal lints that are each implemented as a "pass". #74704 appears to be about skipping certain passes that are known to be disabled. However, I don't know that that approach would suffice to disable "built-in" lints, which is a category that includes deprecated and deprecated_in_future.

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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants