-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Ensure that the argument to static_assert
is a bool
#55945
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@@ -16,6 +16,6 @@ macro_rules! static_assert { | |||
// Use the bool to access an array such that if the bool is false, the access | |||
// is out-of-bounds. | |||
#[allow(dead_code)] | |||
static $name: () = [()][!$test as usize]; | |||
static $name: () = [()][!($test: bool) as usize]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use #[allow_internal_unstable]
instead, maybe?
Also, has the const _ = ...;
proposal been implemented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, has the const _ = ...; proposal been implemented?
It has been implemented, but it does not cope well with allow_internal_unstable
, because the crate using static_assert
won't have the feature for _
items enabled, thus causing an error about item with name _
being declared multiple times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very confused, how could that even happen?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, I presume it's because we now do have multiple items with the name _
, but the multiple items with same name check doesn't know about allow_internal_unstable
and thus assumes the relevant feature gate (for _
names) isn't active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the implementation should be using gensym
so that should be impossible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oli-obk No, because that won't preserve the association. Anyway, gensyms are just an implementation hack, you don't actually need a name for these items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can refactor AST and HIR to have Option<Symbol>
for their items' names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems overkill for now, and also, we can move this into an issue and land a simpler version of this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by simpler you mean keeping the actual names for the assert items?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah
a9018ea
to
d47b66b
Compare
The job Click to expand the log.
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 |
@oli-obk Looks pretty cool, if you can get it to work |
d47b66b
to
65e6fdb
Compare
@bors r+ |
📌 Commit 65e6fdb has been approved by |
…ichaelwoerister Ensure that the argument to `static_assert` is a `bool` cc @eddyb
Rollup of 14 pull requests Successful merges: - #55767 (Disable some pretty-printers when gdb is rust-enabled) - #55838 (Fix #[cfg] for step impl on ranges) - #55869 (Add std::iter::unfold) - #55945 (Ensure that the argument to `static_assert` is a `bool`) - #56022 (When popping in CTFE, perform validation before jumping to next statement to have a better span for the error) - #56048 (Add rustc_codegen_ssa to sysroot) - #56091 (Fix json output in the self-profiler) - #56097 (Fix invalid bitcast taking bool out of a union represented as a scalar) - #56116 (ci: Download clang/lldb from tarballs) - #56120 (Add unstable Literal::subspan().) - #56154 (Pass additional linker flags when targeting Fuchsia) - #56162 (std::str Adapt documentation to reality) - #56163 ([master] Backport 1.30.1 release notes) - #56168 (Fix the tracking issue for hash_raw_entry) Failed merges: r? @ghost
cc @eddyb