remove useless ident() functions in const tests #61658
Conversation
…l ones by black_box (with a comment)
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ rollup |
|
📌 Commit 4567f22 has been approved by |
bors
added
S-waiting-on-bors
| pub fn main() { | ||
| assert_eq!(DANGLING, ident(NonNull::dangling())); | ||
| assert_eq!(CASTED, ident(NonNull::dangling())); | ||
| assert_eq!(DANGLING, b(NonNull::dangling())); |
There was a problem hiding this comment.
hmm... does this actually have the desired effect? I think we need to do b(NonNull::dangling as fn() -> NonNull<u32>)() in order to guarantee that rustc won't const prop the value by itself.
There was a problem hiding this comment.
I don't think it is reasonable to fight against compiler optimizations like that here. I mean, it'll still get const-prop'ed by LLVM...
There was a problem hiding this comment.
IOW, this is not the const-prop test suite. Then anyway we'd have to do three-way comparison to check both the result with and without const-prop.
There was a problem hiding this comment.
const prop by LLVM is fine. The problem is that this is testing our const evaluator, and if the rhs is also computed by the const evaluator, then the comparison is moot.
I know it's not the const-prop test suite, it should not be testing const prop, it should be testing const eval against runtime. But with sufficiently smart const prop we'll get the rhs const propagated (at least the part inside the b(..), const prop won't touch black_box).
There was a problem hiding this comment.
The comparison is still relevant as it is the const evaluator accessed through very different code paths. We have (at least) three ways the computation might go here and this only compares two of them, I see no good reason why comparing these two would be better than comparing the other two. The const-prop test suite will make sure they are the same.
But as you wish, I changed it. Lucky enough this does not need a type annotation.
|
@bors r- |
bors
added
S-waiting-on-author
|
@bors r+ |
|
📌 Commit a733b87 has been approved by |
bors
added
S-waiting-on-bors
|
actually @bors r- you black-boxed the zst, which is a NOP. The cast to function pointers was on purpose |
bors
added
S-waiting-on-author
|
Now in extra-paranoid mode. |
|
Thanks! @bors r+ |
|
📌 Commit 3f99ad1 has been approved by |
bors
added
S-waiting-on-bors
remove useless ident() functions in const tests and replace the useful ones by black_box (with a comment) r? @oli-obk
Rollup of 6 pull requests Successful merges: - #61646 (Remove useless allocations in macro_rules follow logic.) - #61658 (remove useless ident() functions in const tests) - #61660 (Minimize use of `#![feature(custom_attribute)]`) - #61666 (Add test for trait ICE) - #61669 ( syntax: Remove `Deref` impl from `Token`) - #61670 (Update RLS) Failed merges: r? @ghost
and replace the useful ones by black_box (with a comment)
r? @oli-obk