Repository navigation
Remove obsolete no_stack_check attribute and test #34915
Description
Here is the start of the failure
failures:
---- [debuginfo-gdb] debuginfo-gdb/function-prologue-stepping-no-stack-check.rs stdout ----
executing armv7-unknown-linux-gnueabihf/stage2/bin/rustc /home/infinity0/rust/src/test/debuginfo/function-prologue-stepping-no-stack-check.rs -L armv7-unknown-linux-gnueabihf/test/debuginfo-gdb/ --target=armv7-unknown-linux-gnueabihf -L armv7-unknown-linux-gnueabihf/test/debuginfo-gdb/function-prologue-stepping-no-stack-check.stage2-armv7-unknown-linux-gnueabihf.debuginfo-gdb.libaux -C prefer-dynamic -o armv7-unknown-linux-gnueabihf/test/debuginfo-gdb/function-prologue-stepping-no-stack-check.stage2-armv7-unknown-linux-gnueabihf -C link-args=-Wl,-z,relro --cfg rtopt -C rpath -L armv7-unknown-linux-gnueabihf/rt -g
------stdout------------------------------
------stderr------------------------------
/home/infinity0/rust/src/test/debuginfo/function-prologue-stepping-no-stack-check.rs:253:1: 253:18 warning: unused attribute, #[warn(unused_attributes)] on by default
/home/infinity0/rust/src/test/debuginfo/function-prologue-stepping-no-stack-check.rs:253 #[no_stack_check]
^~~~~~~~~~~~~~~~~
/home/infinity0/rust/src/test/debuginfo/function-prologue-stepping-no-stack-check.rs:270:1: 270:18 warning: unused attribute, #[warn(unused_attributes)] on by default
/home/infinity0/rust/src/test/debuginfo/function-prologue-stepping-no-stack-check.rs:270 #[no_stack_check]
^~~~~~~~~~~~~~~~~
[..] many of these, basically one for every instance of no_stack_check in the file
After this basically everything is broken which you can see in more detail here. However, I see that no_stack_check isn't used in the code anywhere any more - there is only one grep result for stack[-_]check that excludes this test itself, and one of them says this:
src/libsyntax/feature_gate.rs- // Not used any more, but we can't feature gate it
src/libsyntax/feature_gate.rs: ("no_stack_check", Normal, Ungated),
So I wonder if this test should just be gotten rid of?
Activity
nagisa commentedon Jul 19, 2016
The test probably should go.
nagisa commentedon Jul 19, 2016
I think its also a time to deprecate the attribute by now. cc @rust-lang/compiler @rust-lang/lang
michaelwoerister commentedon Jul 19, 2016
If the attribute doesn't have any effect anymore, the test case should be removed (while making sure that's it's still possible to break on a function, which might mean that we have to write another test case if that isn't covered anywhere else).
infinity0 commentedon Jul 21, 2016
fwiw, this test does pass on x86, x86-64 and even arm64
pnkfelix commentedon Jul 21, 2016
It seems odd that the test is failing in this context, but not in the other targets that @infinity0 mentioned.
Nonetheless, the compiler team concluded we should:
nikomatsakis commentedon Jul 21, 2016
I'll happily mentor someone to do this.
[-]debuginfo/function-prologue-stepping-no-stack-check fails on armv7 hf[/-][+]Delete obsolete no_stack_check attribute and test[/+][-]Delete obsolete no_stack_check attribute and test[/-][+]Remove obsolete no_stack_check attribute and test[/+]abhijeetbhagat commentedon Jul 23, 2016
I can work on this.
abhijeetbhagat commentedon Jul 25, 2016
So this is as simple as just deleting
/src/test/debuginfo/function-prologue-stepping-regular.rsand removing this:// Not used any more, but we can't feature gate it("no_stack_check", Normal, Ungated),from
/src/libsyntax/feature_gate.rs?6 remaining items
pmatos commentedon Sep 23, 2016
What's still missing here?
lambda-fairy commentedon Oct 22, 2016
@pmatos AFAIK, the only thing left to do is have
-C no-stack-checkprint a deprecation warning when used. There was a pull request for this at #35156, but it was closed due to inactivity.Rollup merge of rust-lang#37636 - karpinski:issue-34915, r=nikomatsakis
mgattozzi commentedon Jan 13, 2017
This issue should be closed now right? It mentions in the release notes of #38966 that this has been fixed and it seems like #34915 was the one that closed the issue.
michaelwoerister commentedon Jan 13, 2017
@mgattozzi Yes, looks like it. Thanks for the reminder!
nagisa commentedon Jan 13, 2017
No. We still need to remove the flag. Please reopen.
michaelwoerister commentedon Jan 13, 2017
Oh, I thought the issue was just about the test, sorry.
danobi commentedon Feb 25, 2017
This issue was fixed in PR #37636. I believe this should be closed.
nikomatsakis commentedon Mar 2, 2017
per @danobi's comment, closing