-
Notifications
You must be signed in to change notification settings - Fork 17.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
proposal: spec: Compile-time recognition of functions that cannot return normally, using !return
#71553
Comments
I honestly don't think those changes anything from #69591. You save writing a return, at the cost of a language change and decreased readability as you have to check every function signature to understand control flow, instead of just learning the few standard ones. |
Hi @seankhliao, Over in #71528 I'm hearing that it's desirable for readers to just "know" that functions like I know that you in particular didn't argue that, and so I'm not meaning to say that this is contradictory feedback -- everyone is allowed to prioritize different things, after all -- but I do find this tension quite interesting. Perhaps the trick is that people have different preferences and expectations for test code vs main code when it comes to what is "explicit enough" and what is "readable"? 🤔 |
see also #37193 testing's heavy use of runtime.Goexit compared to regular code already enables many helper functions that are more compact than most proposed error handling syntaxes. Over indexing on testing code is probably not a great idea. |
I don't see how this is backwards compatible. Consider: package example
func Exit() !return {
panic("exit")
} package example2
var _ func() = example.Exit If |
Hi @tmthrgd, Per this proposal, The opposite is not true, but there cannot be any existing program containing a variable of type |
package example
func Exit() !return {
panic("exit")
} package example2
var exit = example.Exit
func init() {
exit = func() {}
} That's not backward compatible. |
I assume the point you are making is that without some sort of exception to type inference, the compiler would infer It seems that to make this case work (without a weird type inference exception) it would be necessary for It would not be possible to create a function pointer that can only have I'm not going to update the proposal to reflect that since I expect this proposal to be declined for other reasons anyway, but I'm just noting the above in case someone finds this discussion in future and wishes to embark on the dubious errand of making yet another "no return" proposal. 😀 |
r.e. @seankhliao's further remarks about testing: The other proposal #37193 is compatible with "keep going", but would require explicitly including a ThingUnderTest() ? err {
t.Errorf("unexpected error: %s")
fallthrough
} In my experience "keep going" tends to apply more to non-error-related outcomes like Subtests using I do broadly agree with you though that the error-handling proposal seems less relevant to test code than to main code. Others in the discussion seem to disagree however, and that is what prompted me to start this proposal: I have the sense that for some there is no value in an error-handling proposal that is not useful and ergonomic in test code. |
I don't understand how this decreases readability compared to the status quo, where you need to look at the implementation or (hopefully) the documentation to know that a function never actually returns, and where you are sometimes required to have a return that is dead code and might deceive the reader into thinking that the return is reachable |
There is a patch set which implements the compiler optimizations without a language change https://go-review.googlesource.com/c/go/+/635344/9, instead it interprocedurally scans the function to see if it returns or not. |
Thanks for linking to that, @Jorropo! That's interesting. I would note though that this particular proposal intentionally calls for changing the surface language because it's more about representing a guarantee that a function does not return normally today and it won't begin returning in the future without that being considered a breaking change. I think that is required in order for other language features to be able to rely on it, but I fully agree that it's not necessary if approaching this purely as an optimization problem in the compiler. I would expect the compiler to make use of interprocedural analysis for this optimization whenever possible even if this language change were accepted, because the promise that no future version of a function will return (a human design decision) is quite separate to the question of whether this particular version of a function can possibly return (a program analysis problem). |
It would probably be finicky for the compiler to take existing functions to prove that they don't return and probably complicated to write in the spec, including taking into account build tags and conditionals and syscalls and whatnot. (See for example some related comments from Dominik Honnef in dominikh/go-tools#787, dominikh/go-tools#793, or the example in staticcheck SA5011). There are also considerations for other implementations like TinyGo or Go interpreters like yaegi (though they could always "cheat" for well-known functions). Maybe (?) an alternative to this proposal could be to target simple cases that are "easy" to prove, such as if the implementation of log.Fatal was changed in the stdlib to: func Fatal(v ...any) {
realFatal(v...)
panic("unreachable")
}
func realFatal(v ...any) {
// ... current log.Fatal implementation ...
} In other words, don't require full blown analysis in the spec, but something like if a function call X is to be treated as non-returning, X must panic at the end of all control paths without a defer/recover (or even foo must only have a single control path and it must end in a panic, like the example above). That would then need to be documented by such functions, and the author would need to consider that behavior as a published part of their API. (I understand a goal of this proposal is to make it a machine understandable attribute of the function signature, but not all parts of an API today are statically enforced today, including panic behavior for an API is sometimes today documented "just" in writing). This suggestion would not handle interface method calls and function pointers and whatnot (though someone could wrap in a concrete function that panics at the end if they really wanted). But even that is complicated (and maybe flawed, too). Personally, I don't think anything here (including what I wrote above) is worthwhile as a general change on its own. As Ian wrote ~10 years ago:
If something in this area was part of a specific error handling proposal, it could be defined as part of that proposal, where it would have to be judged on the overall merits of that proposal. |
Hi @thepudds, Indeed, I don't think it would be appropriate for the compiler to try to prove by analysis that a function does not return unless its author added This proposal defines how the compiler might verify that a function that has been explicitly marked as I did notice Ian's comments in that email thread, and the similar remark in the earlier proposal I linked. I also noticed Ian's somewhat-different remarks in the more recent error-handling proposal:
...and that is, in part, what prompted me to open this proposal: to see if the situation has changed enough to constitute new information. That error-handling proposal would effectively change it from "relatively rare cases"/"certain kinds of code" to "the majority of the I suppose it is reasonable to argue that this should just be proposed as a part of #71528. I opened it separately because I see it as an orthogonal language change that the other proposal would happen to significantly benefit from, but it is not exclusively for that proposal since it would also address the complaint in that old mailing list thread, and give more explicit information to linting tools about which statements are reachable, and so it's potentially of interest even to those who would not be interested in the error-handling proposal. The documentation you linked for staticcheck's SA5011 includes an interesting example: func Log(msg string, level int) {
fmt.Println(msg)
if level == levelFatal {
os.Exit(1)
}
}
func Fatal(msg string) {
Log(msg, levelFatal)
}
func fn(x *int) {
if x == nil {
Fatal("unexpected nil pointer")
}
fmt.Println(*x)
} This is a nice example of a situation where it would require some relatively-complicated interprocedural analysis to automatically infer that Under this proposal, the author of func Fatal(msg string) !return {
Log(msg, levelFatal)
panic("unreachable") // Log does not return when level == levelFatal
} Of course, there is nothing here to stop With all of that said, I do concede that this situation is likely an edge case. I just highlighted it here because it seemed like a nice example of how a language feature like this can potentially make life considerably easier for a static-analysis-based linting tool. (I do acknowledge that staticcheck in particular can already notice that there's a |
@tmthrgd makes a good point above: it would be hard to annotate existing functions such as Setting that aside, this is much the same as #69591, except that it does not require a new keyword. The arguments there seem to apply here. Also, the emoji voting is not in support. Therefore, this is a likely decline. Leaving open for four weeks for final comments. -- for @golang/proposal-review |
My understanding of this proposal is that the I wonder if another approach could be to have a general way to add annotations to functions similar to struct tags, and the compiler itself doesn't do anything with them, but a static analysis tool can access to check that some property (like never returning) is preserved. That wouldn't have all of the benefits of built-in compiler support, but it wouldn't have to change the language as much. |
The current proposal text, which I wrote before @tmthrgd's comment, does still say that it If that were the only objection to this proposal then I would revise it to say that However, as noted in #71553 (comment) I did not revise the proposal in that way because the emoji voting was already trending negative and there were already several other negative comments, so I expected that this proposal would be declined regardless of that change. If anyone strongly believes otherwise then I'd be willing to try, but I was already pretty pessimistic about this proposal before I even opened it. @tmccombs's idea about a general-purpose annotation feature seems interesting, but unless there were a specific annotation recognized by the compiler as "does not return" I don't think that would actually meet the main goal I had for this proposal, which was for the compiler to treat I agree that it could still potentially be useful for linting tools, but they already seem to be doing okay with machine-readable "directive comments" today, like |
Programming Experience
Go experience: Experienced
Other languages experience: Rust, C, Python, JavaScript
Related Idea
A similar idea was previously proposed as #69591.
Compared to that proposal, this one proposes "non-returning" as a separate characteristic of a function signature independent of its return type, to limit the impact of the change on existing tools, and on
package reflect
.This also proposes a different syntax that uses a combination of a punctuation character followed by an existing keyword, rather than introducing a new predeclared identifier.
This proposal does not directly affect error handling, but I am proposing it today primarily for its interaction with #71528, which is an error-handling proposal. In particular, I am interested to find out whether the idea in #71528 might potentially constitute sufficient new information (relative to the discussion in #69591, which was already declined) to reconsider the value in explicitly tracking functions that do not return.
Although I've opened these as separate issues because they are technically independent of one another, in practice I think that this proposal only has potential value if something like #71528 were also accepted. Without that, this proposal is effectively the same as #69591, which was already declined.
Proposal
I propose allowing a new syntax form in the "Result" production of a function or function type declaration, which declares that a function is guaranteed not to return normally. For example:
!return
is pronounced "not return".When a function is marked in this way, it is prohibited from including any
return
statements and it must end with a non-return
terminating statement.An unconditional call to a function marked in this way is a new kind of terminating statement.
Because such a function is a terminating statement, the compiler no longer requires an in-practice-unreachable terminating statement after a call to any of these functions to convince the compiler that the end of the function is unreachable. For example:
If #71528 were also accepted, and
t.FailNow
and all of its unconditional callers were modified to be!return
, the two proposals would combine to permit test code like the following:For most type-checking purposes, and for the purposes of all existing APIs for either static analysis or reflection over function types, a
!return
function is treated as a function whose signature includes no results.!return
is an entirely new, separate property of a function type, exposed via new, separate reflection API features. In particular, a function type with!return
is assignable to another function type that is identical except for not having!return
, although the opposite is not true.Most existing reflection or static analysis code can ignore this new property and treat such a function as if it returns normally with no results, and thus this change is backward-compatible with existing tools which can treat
!return
functions just like normal functions that have no results.The
!return
property of a function signature affects only what is allowed inside the function's body (as described above) and the function's recognition as a terminating statement when called from elsewhere. Tools that use the "terminating statement" concept for control flow analysis should be updated to recognize!return
functions as terminating statements. For example, a tool which detects and warns about unreachable code would begin reporting that any code dominated by a call to a!return
function as being unreachable.Many existing library functions whose documented behavior is consistent with the new
!return
property have that behavior due to delegating to a lower-level function that has that behavior. For example,testing.T.FailNow
delegates toruntime.Goexit
. Therefore existing library functions can be gradually improved with!return
markings by working outwards from the lowest-level functions. The lowest-level functions will, if implemented in Go themselves, need to include a dynamically-unreachable call topanic
as the final non-library-based terminating statement that therefore makes the entire call chain valid as!return
. Those which are implemented in assembly language and only declared in Go can have their declaration updated to be!return
as long as the assembly implementation guarantees that indeed the function will not return.Language Spec Changes
Under Function Types, insert the new production
NotReturn
and redefineResult
as follows, leaving all of the other productions unchanged:Also in that section, add:
Under Terminating Statements, point 2 changes from "A call to the built-in function
panic
", to "A call to any!return
function".Under Handling panics, the signature of
panic
changes to the following, thereby makingpanic
itself a!return
function so that it qualifies under the change from the previous paragraph:Under Function declarations, change the paragraph beginning "If the function's signature declares result parameters", to start instead with "If the function's signature declares result parameters, or
!return
", and add a new paragraph after its example:Informal Change
Most functions return control back to their caller after completing execution, optionally returning one or more values. Some functions may behave in more unusual ways, such as by terminating the entire program. A function marked as
!return
is special in that it is guaranteed not to return control, and so any statements after it in the program will not be executed.panic
is a!return
function that is built into the language. The standard library also includes some!return
functions. You can write!return
functions in your own program if you wish, although most programs use only the ones from the standard library.If you write your own
!return
function then the compiler will return an error unless it can prove that the function does not return. A typical way to ensure that your function does not return is to unconditionally call another, lower-level function that is also marked as!return
.Is this change backward compatible?
Since
!return
is defined as a tighter constraint on a function's behavior, it is always backward-compatible to add!return
to any existing function that returns no results, as long as that function already has no normal return path due to characteristics of its existing implementation.A function that does not return due to calling another function that does not return cannot be marked as
!return
until all of its non-returning callees have already been marked as!return
, unless the author introduces dynamically-unreachablepanic
calls orreturn
statements to satisfy the compiler that the function definitely cannot return.!return
is defined as a new, separate property of a function type, as opposed to a new return type, to avoid making breaking changes topackage reflect
, the static analysis packages, and the tools that use them. Any existing code that is not aware of!return
will find what appears to be just a normal function that returns no results. Only tools that actually need to rely on the!return
property need be updated to make use of new API features for reporting that.EDIT: Refer to #71553 (comment) and #71553 (comment) for some further discussion on a backward-compatibility gap that the original proposal text did not identify, and some informal ideas on how it could be addressed by some modifications to the proposal.
Orthogonality: How does this change interact or overlap with existing features?
This change extends the definition of "terminating statement" to include calls to certain user-defined functions. It therefore increases the number of statements that are valid as the last statement in a normal function.
It gives the compiler awareness of a property of a function that is currently not represented in the type system and captured only in human-readable documentation, allowing the compiler to perform more precise analysis and in particular to avoid the need for a redundant, dynamically-unreachable terminating statement after a call to a function that is guaranteed by its documentation to never return.
Although this is only a proposed rather than existing feature, it also addresses one of the ergonomic gaps in #71528, again by allowing the compiler to perform more precise analysis of the control flow within an exceptional block and thus avoid a redundant, dynamically-unreachable terminating statement.
Since I've defined
!return
as a property of a function type, it is technically possible to declare a variable that can only have!return
functions assigned to it. However, it's unclear to me whether that's actually useful in practice.Would this change make Go easier or harder to learn, and why?
Since there are various functions in the standard library that are already effectively
!return
per their documentation, most Go programmers can continue to rely on the documentation to learn about this characteristic and so would not need to directly learn about!return
in order to successfully write most Go code, unless they want to provide a function that has that characteristic itself.Several other programming languages with similar characteristics to Go have a feature similar to this. C23 and C++11 both represent this using a function attribute, although those languages do not actually prevent such a function from returning: it's simply undefined behavior to return. Rust includes the "never" type, spelled
!
, which represents this characteristic as a type that has no values and thus cannot be constructed to return it.As with those other languages, I expect this feature would be used primarily by standard library features that interact with the runtime or the operating system, and so most Go application or library developers would not learn about it at all.
Some new Go developers might find it helpful for their tools to indicate when a
!return
function makes a later statement effectively unreachable, thereby avoiding them having to refer to the function's documentation to learn that. However, I expect that benefit is marginal.Cost Description
This change introduces yet another property to track about a function type, and a new assignability rule for function types that differ only in
!return
. Function types are already quite complicated.The runtime type representation used by
reflect
would need to use another bit ofabi.Type.TFlag
to represent the presence of!return
. This flag applies only to function types, which is not true for any other bit of that field that's currently allocated, and that's potentially confusing. (Other implementations are possible, but this one avoids growing the overall size of the runtime type representation.)A function marked as
!return
will often require at least one of its callees to also be marked as!return
, and so although there is no requirement to immediately add!return
to all existing non-returning functions in the ecosystem there would still likely be pressure from higher-level library authors on lower-level library authors to add!return
annotations, so that the higher-level authors can avoid introducing redundant dynamically-unreachable terminating statements to satisfy the compiler that a function is indeed!return
. In turn, users of those higher-level libraries may pressure the library authors to add!return
to existing functions so that they can be used ergonomically as the terminating statement of a function or in an exception block.Changes to Go ToolChain
Any tool that attempts to detect and report unreachable code should be changed to reflect the new definition of "terminating statements", which in turn means becoming able to check whether all callees are
!return
. This includes theunreachable
check ingo vet
.All tools that include a Go source code parser must learn to treat
!return
as valid in the function signature result position, rather than reporting it as a syntax error.Performance Costs
The main additional cost would be felt at compile-time, in the code which verifies whether a function ends with a terminating statement, and in the new code which verifies that a
!return
function definitely does not return.!return
has no significant runtime performance impact, since all of the checks related to it should complete at compile time. At runtime, a!return
function is exactly equivalent to a function that returns no results, except that the control flow instructions in the generated code will definitely not include something equivalent toret
.The text was updated successfully, but these errors were encountered: