Skip to content

Don't render const keyword on stable #55327

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

Closed
wants to merge 1 commit into from

Conversation

GuillaumeGomez
Copy link
Member

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

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.
travis_time:end:14859b34:start=1540417010655314435,finish=1540417067651549201,duration=56996234766
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
---

[00:03:53] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:53] tidy error: /checkout/src/librustdoc/html/render.rs:2675: TODO is deprecated; use FIXME
[00:03:53] tidy error: /checkout/src/librustdoc/html/render.rs:3135: TODO is deprecated; use FIXME
[00:03:54] some tidy checks failed
[00:03:54] 
[00:03:54] 
[00:03:54] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:54] 
[00:03:54] 
[00:03:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:54] Build completed unsuccessfully in 0:00:49
[00:03:54] Build completed unsuccessfully in 0:00:49
[00:03:54] Makefile:79: recipe for target 'tidy' failed
[00:03:54] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:09853af0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:1dd96e10:start=1540417313464505721,finish=1540417313471801019,duration=7295298
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04cf9250
$ 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 --batch -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:1d0f5380
travis_time:start:1d0f5380
$ 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:3045745c
$ 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)

@@ -2672,9 +2672,14 @@ fn item_static(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,

fn item_function(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
f: &clean::Function) -> fmt::Result {
// FIXME: to be removed when "const" keyword is stabilised.
let vis_constness = match UnstableFeatures::from_environment().is_nightly_build() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we could move this check earlier in the process and store the result in the Cache or something? That way we wouldn't have to keep poking the environment variables over and over.

The Context already has the codes field, which can be used as a stand-in for "can i do nightly things", but that won't help the other spot where you put this check, since it doesn't receive a &Context.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 25, 2018

Uhm... const fns are stable on nightly already... at least some of them. This should be in the current beta, too. to figure out whether a const fn from the libstd is available on stable you can call tcx.is_min_const_fn(def_id)

@GuillaumeGomez
Copy link
Member Author

Oh nice! That'll make things simpler!

@GuillaumeGomez
Copy link
Member Author

@oli-obk: So we might have a "little" problem with using tcx alias TyCtxt: it's not available anymore at this stage and trying to pass it to this stage is problematic to say the least. It requires huge changes for a check that'll disappear in one version (or two? Not sure...).

@oli-obk
Copy link
Contributor

oli-obk commented Oct 27, 2018

not quite. This check will stay around for libstd for quite a while, because it's very hard to know which const fns are stable and which are not. How are you figuring out any details that are not in the syntax tree without a TyCtxt available? You still have ty::Ty types available, right?

Side note: You can "cheat" depending on what you mean by "stage". This is the trick that Debug impls for internal types use when they need a TyCtxt for printing

ty::tls::with(|tcx| tcx.is_min_const_fn(def_id))

@@ -2672,9 +2672,14 @@ fn item_static(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,

fn item_function(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you strip the constness during the generation of clean::Function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of tricking that way yes.

@GuillaumeGomez
Copy link
Member Author

@oli-obk Updated.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work for methods, free const functions aren't touched at all by this PR

@@ -2002,7 +2002,8 @@ impl<'tcx> Clean<Item> for ty::AssociatedItem {
ty::TraitContainer(_) => self.defaultness.has_value()
};
if provided {
let constness = if cx.tcx.is_const_fn(self.def_id) {
let constness = if cx.tcx.is_const_fn(self.def_id) &&
cx.tcx.is_min_const_fn(self.def_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the is_const_fn call, just the is_min_const_fn suffices.

@GuillaumeGomez
Copy link
Member Author

Updated.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2018

What about

This will only work for methods, free const functions aren't touched at all by this PR

Also, is it possible to test this somehow?

@GuillaumeGomez
Copy link
Member Author

Yes but that also means that when it'll be stabilized, a rustdoc test will break. If that sounds fine to you then I'll add one.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2018

const fn is already stabilized ;)

I think the important test is whether
only bar2 and bar2_gated show up in docs as const fn

#![unstable(feature = "humans",
            reason = "who ever let humans program computers,
            we're apparently really bad at it",
            issue = "0")]

#![feature(rustc_const_unstable, const_fn, foo, foo2)]
#![feature(min_const_unsafe_fn)]
#![feature(staged_api)]

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature="foo")]
const unsafe fn foo() -> u32 { 42 }

#[unstable(feature = "rust1", issue="0")]
const fn foo2() -> u32 { 42 }

#[stable(feature = "rust1", since = "1.0.0")]
const fn bar2() -> u32 { 42 }

#[unstable(feature = "foo2", issue="0")]
const unsafe fn foo2_gated() -> u32 { 42 }

#[stable(feature = "rust1", since = "1.0.0")]
const unsafe fn bar2_gated() -> u32 { 42 }

@rust-highfive
Copy link
Contributor

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.
travis_time:end:095df54e:start=1541331713572879165,finish=1541331771325309554,duration=57752430389
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
[00:52:39] .................................................................................................... 100/4988
[00:52:42] .................................................................................................... 200/4988
[00:52:45] ...........................................................................................ii....... 300/4988
[00:52:48] .........................................................................................iii........ 400/4988
[00:52:50] iiiiiiii.iii...........................iii...........................................i...........i.. 500/4988
[00:52:58] .................................................................................................... 700/4988
[00:53:04] ..................................................................i...........i..................... 800/4988
[00:53:07] .....................................................................................iiiii.......... 900/4988
[00:53:11] .................................................................................................... 1000/4988
---
[00:53:49] .................................................................................................... 2200/4988
[00:53:53] .................................................................................................... 2300/4988
[00:53:57] .................................................................................................... 2400/4988
[00:54:01] .................................................................................................... 2500/4988
[00:54:04] ......................................................................iiiiiiiii..................... 2600/4988
[00:54:11] .....................ii............................................................................. 2800/4988
[00:54:14] .................................................................................................... 2900/4988
[00:54:18] .................................................................................................... 3000/4988
[00:54:21] ................i................................................................................... 3100/4988
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:08:31] 
[01:08:31] running 115 tests
[01:08:34] i..ii...iii..iii.....i...i.........i..iii...........i.....i.....ii...i..i.ii..............i...ii..ii 100/115
[01:08:35] .i....iiii.....
[01:08:35] 
[01:08:35]  finished in 3.603
[01:08:35] travis_fold:end:test_codegen

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:08:50] 
[01:08:50] running 118 tests
[01:09:15] .iiiii...i.....i..i...i..i.i..i.i..i.....i..i....i..........iiii.........i.i....i...i.......ii.i.i.i 100/118
[01:09:20] ......iii.i.....ii
[01:09:20] 
[01:09:20]  finished in 29.661
[01:09:20] travis_fold:end:test_debuginfo

---
travis_time:start:test_rustdoc
Check compiletest suite=rustdoc mode=rustdoc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:16:08] 
[01:16:08] running 272 tests
[01:17:14] .......................i..........................................F................................. 100/272
Sun, 04 Nov 2018 13:02:01 GMT
travis_time:end:0246f774:start=1541336521048690089,finish=1541336521088151329,duration=39461240

The command "date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

@Mark-Simulacrum
Copy link
Member

@GuillaumeGomez I think this'll need some updates as const fn has now stabilized; I've changed the status to waiting-on-author

@Mark-Simulacrum Mark-Simulacrum 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2018
@GuillaumeGomez
Copy link
Member Author

A check @oli-obk proposed and suggested to add isn't working as expected. I need to speak with him about it.

@Dylan-DPC-zz
Copy link

Pinging from triage. @GuillaumeGomez any updates on this?

@GuillaumeGomez
Copy link
Member Author

I didn't speak with him yet.

@TimNN
Copy link
Contributor

TimNN commented Dec 4, 2018

Ping from triage! This PR hasn't made progress in a while, so I'm closing it per our guidelines. Thanks for your contributions and please feel free to re-open in the future.

@TimNN TimNN closed this Dec 4, 2018
@TimNN TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 20, 2018
bors added a commit that referenced this pull request Dec 20, 2018
Don't render const keyword on stable

Fixes #55246.

Continuation of #55327.

r? @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants