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

remove static_assert #1096

Merged
merged 1 commit into from
Jun 2, 2015

Conversation

steveklabnik
Copy link
Member

[issue]: https://github.com/rust-lang/rust/issues/6676

So why not improve `static_assert`? With compile time function evaluation, the
idea of a ‘static assertion’ doesn’t need to have language semantics. Either
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how CTFE obviates the desire for static_assert. If anything, I think CTFE makes static_assert much more powerful. CTFE allows us to calculate things at compile-time, and static_assert then lets you abort compilation if a given CTFE expression doesn't have the expected value.

That said, with CTFE, I'd expect static_assert to take the form of a macro static_assert!(condition[, message]).

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 under the assumption that in a macro, you could just panic, or use something like https://github.com/huonw/compile_msg 's compile_error, which would outright replace the current static_assert.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what the current thinking is on CTFE, but I'm a little skeptical that panicking is something that would be compatible with CTFE. Similarly, I don't know whether CTFE would be able to use syntax extensions like compile_error (as opposed to a syntax extension evaluating a CTFE, which is what my proposed static_assert!() would be).

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose with CTFE, this static_assert!() macro could be provided by a crate, although it seems like a reasonable candidate to bake into the language (e.g. because the standard libraries might want to use it).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I don't see how CTFE obviates it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, assuming a CTFE where begin_unwind[_fmt] (and whatever other functions are needed for panic!) are const fns (throwing compile errors at runtime) then we could just use the regular assert! macro:

static _assert: () = assert!(FOO);
// Or, in a future Rust where statements are allowed at module-level:
assert!(FOO);

@lilyball
Copy link
Contributor

The current form of static_assert is pretty weird. If we really aren't actually using it for anything, then 👍 on removing it. But with full-blown CTFE I'd want to have it reintroduced as a macro static_assert(cond[, message]) as I mentioned in my comment.

@Ericson2314
Copy link
Contributor

+1

@pythonesque
Copy link
Contributor

As someone who uses static_assert, I'd rather not see it removed without a more compelling reason (unless full CTFE can actually fully reintroduce it, but it's not clear to me that it would be able to; panic! diverges, after all).


Throughout its life, `static_assert` has been... weird. Graydon suggested it
[in May of 2013][suggest], and it was
[implemented][https://github.com/rust-lang/rust/pull/6670] shortly after.

Choose a reason for hiding this comment

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

Typo: Should be [text](url), not [text][url]

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll fix this if it's about to get merged.

@ben0x539
Copy link

alternative: provide a macro static_assert!(condition); that expands to #[static_assert] static new_identifier: bool = condition; and stabilize the macro but not the implementation :P

@nagisa
Copy link
Member

nagisa commented Apr 29, 2015

The “want” to remove static_assert is intense here. It infects compiler all the way to trans. If we’re keeping it, I’d rather see this implemented within bounds of CTFE somehow.

@steveklabnik
Copy link
Member Author

@nrc given that I'm the author, I probably shouldn't shepard?

@emberian
Copy link
Member

emberian commented May 6, 2015

@pythonesque what's the use case? When I implemented this I planned on usnig it to assert that sizes of things were the same but constant expressions aren't powerful enough.

As the original designer and implementor, 👍. This is a weird feature that is extremely limited.

@pythonesque
Copy link
Contributor

@cmr I use it to assert that u8::MAX >= usize::MAX_BITS, which is required for correctness for my union find algorithm. With more powerful constant expressions (which I think have been accepted now) I would be able to assert more interesting things (particularly around things like correct alignments / gaps in data structures, where I rely on two representations matching up for memory safety).

@huonw
Copy link
Member

huonw commented May 7, 2015

I'm in favour of removing this and just using #[test]s until we get a nicer solution.

@ssokolow
Copy link

ssokolow commented May 7, 2015

#[test]s are less reliable. In fact, at the moment, I've got an experimental project where, even if I create tests exactly as shown in the book, zero tests are found by cargo test. I haven't had time to figure out why yet.

@huonw
Copy link
Member

huonw commented May 7, 2015

No tests running at all seems like an orthogonal problem that should be solved anyway. :)

In any case, replacing the #[static_assert] const NAME: bool = expression; with #[test] fn name() { assert!(expression); } in a library should "just work" (unless one has explicitly opted for test = false in the [lib] section for that library).

@ssokolow
Copy link

ssokolow commented May 7, 2015

Wait... the [lib] section? The project in question is currently a single-file binary project. Did the book neglect to mention #[test] only working in library projects?

@steveklabnik
Copy link
Member Author

steveklabnik commented May 7, 2015 via email

@steveklabnik
Copy link
Member Author

steveklabnik commented May 7, 2015 via email

@steveklabnik
Copy link
Member Author

steveklabnik commented May 7, 2015 via email

@steveklabnik
Copy link
Member Author

steveklabnik commented May 7, 2015 via email

@huonw
Copy link
Member

huonw commented May 7, 2015

@ssokolow hm, a src/main.rs like the following gets compiled and tested when I run cargo test.

fn main() {
    println!("Hello, world!");
}

#[test]
fn foo() {}

(If you're still having problems I'm happy to help, but we should take it to stackoverflow or users rather than here.)

@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2015

Note: I have created a replacement lint-plugin: https://crates.io/crates/static_assert

So now there's zero reason for this to stay in the compiler.

Usage example:

#![feature(plugin, custom_attribute)]
#![plugin(static_assert_)]

fn main() {
    #[static_assert_]
    const TEST: bool = false;
}

I haven't figured out yet how to silence the "unused" warnings, but I'm working on it. Also the next step is to create a macro that automatically creates a temporary constant + adds the attribute (maybe even the #[allow(dead_code)] attribute) so it can be used just like the C++ static_assert on any constant expression

Note2: once static_assert isn't a builtin-attribute anymore, i can remove the trailing underscore

Edit: I just noticed the original was on static fields not on constants... so it's not a drop-in replacement. sorry about that.

SimonSapin added a commit to servo/string-cache that referenced this pull request May 11, 2015
SimonSapin added a commit to servo/string-cache that referenced this pull request May 11, 2015
@SimonSapin
Copy link
Contributor

FWIW, I’ve replaced one usage of #[static_assert] by #[cfg(target_endian = "little")] so that the function would be missing on big-endian (and compilation would fail). The other usage in that library I’ve made a debug_assert!:

servo/string-cache#85

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 15, 2015
@nikomatsakis
Copy link
Contributor

Hear ye, hear ye. This RFC is now in final comment period until June 2nd.

@nikomatsakis nikomatsakis added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label May 26, 2015
@steveklabnik
Copy link
Member Author

🎊

@ssokolow
Copy link

@SimonSapin Unless I missed an announcement that compiler plugins will work in stable channel, I'd hardly call it zero reason.

@huonw
Copy link
Member

huonw commented May 27, 2015

@ssokolow NB. #[static_assert] also doesn't work in the stable channel, so there's not much loss.

@ssokolow
Copy link

Ahh. That makes sense. It can get a bit difficult to keep track of which features I'm not currently using are stable.

@SimonSapin
Copy link
Contributor

@ssokolow I don’t see how my comment is related to plugins. Or did you mean @oli-obk ?

@nikomatsakis
Copy link
Contributor

I think I agree with @huonw that, in practice, adding a #[test] (or a plugin, if you're so inclined) is better than keeping this feature around. I'm nervous about keeping random features around in unstable fashion for an indefinite period of time without strong motivation -- I don't want to allow de facto dependencies to form that prove troublesome to change. cough macro rules cough

@pnkfelix
Copy link
Member

pnkfelix commented Jun 2, 2015

I'm fine with removing #[static_assert]; when we do, we should probably open a corresponding RFC issue at the same time stating that we'd like to see this functionality provided in some manner in the future (in the abstract; its just an issue, not an RFC, after all). At least, that's my take-away from the comment thread.

@nikomatsakis nikomatsakis mentioned this pull request Jun 2, 2015
@nikomatsakis
Copy link
Contributor

Based on the comment thread, we've decided to accept this RFC to remove static_assert. The major arguments in favor of removing it are that it complicates many parts of the compiler tool-chain and we know we would never stabilize the current design. Existing uses can be converted into tests or via a plugin. Per @pnkfelix's comment, we've created issue #1146.

--- Language Subteam

@nikomatsakis nikomatsakis merged commit c385c9b into rust-lang:master Jun 2, 2015
@steveklabnik
Copy link
Member Author

steveklabnik commented Jun 3, 2015 via email

@Centril Centril added A-attributes Proposals relating to attributes A-const-eval Proposals relating to compile time evaluation (CTFE). A-static Proposals relating to static items. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Proposals relating to attributes A-const-eval Proposals relating to compile time evaluation (CTFE). A-static Proposals relating to static items. final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.