-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
proposal: spec: compile-time boolean assertions #34868
Comments
I'm not sure I see the need. What's wrong with init-time assertions? We can write them in regular Go code, no language changes needed. They don't fail at compile time, but they would fail unconditionally at binary startup time, so as long as you at least run a test, you'll find out. |
In general, I don't think there's anything wrong with them. But evidently people do like having compile-time assertions, as I've seen the compile-time assert pattern occur in a few places, despite its awkwardness. As a minor nit, cmd/compile fails to dead-code eliminate
so adding init-level tests to your package currently bloats the resulting executables. (You can add it to the test package, but at that point it seems like you might as well just write it as a unit test.) We should just fix that though (filed as #34869). -- One alternative solution worth mentioning: go/types implements an This is probably an even simpler solution, since it only requires the language spec change to introduce a builtin. It doesn't require a new dummy package, or worrying about weird ways the assert.True type might be used. |
We can use the |
This approach is also a bit awkward, though. We could perhaps do this via a vet check. Add a package func Assert(b bool) bool { return true } This can be used as const _ = assert.Assert(x == y) From the language perspective, this is an inlined function that does nothing. But vet could look for this, and produce an error if the argument to (As a completely different approach, we could add a builtin function |
This seems like a Go2 change to me. |
The notion of adding a special @griesemer suggests a different idea: if you write func init() {
if condition {
panic("oh no!!") // or some other value
}
} then if the compiler can prove that the condition is true (if the condition is a constant expression), it reports an error |
@mdempsky Any thoughts on the idea above? Thanks. |
Is the |
Ping @mdempsky again. |
@ianlancetaylor That seems like a reasonable counter proposal to me. Do you or @griesemer have any specific spec wording in mind? I'd probably suggest something like:
That is, in addition to your example, I think this should be allowed to produce a compile-time error:
As would:
(In practice, cmd/compile probably wouldn't complain about this though, since we conservatively assume But this should not:
|
Personally, I find the idea of using init() functions and panics to do static compile time checks very confusing. Panics have always been at run time, having to teach this idea to (new) Go language users that panics can also be compile time, sometimes, but not always, I expect to see a lot of puzzled faces. My counter proposal is to provide a built in function, say, Like this, we avoid the traps of |
@beoran We might just as well call that function |
Are the counter-proposals above also restricted to Go2? Would adding a |
Do we have data on how common the usage actually is? I just encountered a situation where I considered compile-time assertion, but in the end, writing a unit test is much clearer, so I ended up doing that. I don't see what is the advantage of failing during build time, since we run all the tests in CI anyway. A vet check does not make sense in my opinion, since it does not bring any advantage over running a unit test, both run as a separate step after build. Making panics compile-time seems confusing to me as well. I suggest we just decline this proposal and recommend writing tests instead. |
I propose adding compile-time boolean assertions to Go.
[I don't feel strongly about this proposal, but it seems pretty minimal; easy to implement; and to make some real world code somewhat easier to read/write. I've also not found any past discussion of this idea, so it seemed worth at least writing down even if rejected.]
Proposal
Concretely, I propose making these changes:
Introduce a new "assert" package like:
Add a language rule that it's an error to have a constant of type assert.True but value false.
(Optional) Add a language rule that it's an error to use assert.True except as the type of a constant.
Uses
There are somewhat common idioms of writing:
But I at least find these awkward to reason about, even being very familiar with the details of how they work.
With this proposal, they could instead be written more clearly as:
(Showing off two different ways to write const declarations using assert.True.)
Further, generalizing to boolean expressions allows us to easily use boolean operators to combine multiple tests. It also potentially allows static assertions involving non-integer constants (i.e., floats, complex, bools, and strings).
For example, package gc's sizeof_test.go could be rewritten as compile time asserts like:
Backwards compatibility
assert.True doesn't exist today, so there's no code using it that we have to worry about.
Old tools unaware of the special semantics for assert.True (e.g., old compilers or tools using go/types) will continue working for old code. They'll also continue working correctly for new code that correctly use assert.True. The tools will, however, fail to detect failing assertions.
Related proposals
#9367 proposed allowing bool->int conversions, which be an alternative way of extending the current integer static assertions idiom to arbitrary boolean static assertions. However, it would still be somewhat awkward to read/write.
#30582 proposes an assertion to indicate unreachable code paths. Technically orthogonal to this one, but it might be worth ensuring they expose a consistent API to users.
C++11 added static_assert: https://en.cppreference.com/w/cpp/language/static_assert (Counter argument: C++11 has templates and constexpr, which make static_assert more broadly useful than assert.True would be.)
The text was updated successfully, but these errors were encountered: