-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
void is inconsistent about whether non-void values can be assigned to it #32558
Comments
In summary, I guess I don't understand why |
Ah, I misunderstood the confusion (Null vs void rather than return vs arg). So in this case you're right its pretty inconsistent and I agree with you. I do think its important to clarify that allowing both or neither is not an issue of soundness -- void can not be used, so its "safe" to put anything (or nothing) inside a void value. I say "safe" (with quotes) because it is usually not something people want to do -- statically determine that people want to put data somewhere that it can't be used! That said, drawing the line is kind of funky to determine.
In this example we can't statically determine when this is a Good idea or a bad one. I think there are a few places where we can determine that its a bad one, such as I would say
verify has to accept void results to have this API. The type of verify should then be
And this is a "safe" way to pass around If we made it illegal to assign non-void to void, then
would no longer work without using a different signature for
though, its funny to say, this is what we already do, but just for backwards compatibility so that it works on pre-generalized-void VMs. This I think would be a safe restriction but still can be misused in two ways
As well as the original "funcOk" "funcBug" example before. None of these examples result in type errors, but they can still result in bugs. So I think we have good reason for drawing the lines where we've drawn them, but I personally would like to see "void" not be assignable from non-void, and keep the mockito solution as-written. That said, haven't done experiments on how this works in practice. |
This is not true. Something of void can not be used. This implies that it can be any value, as that will be safe since we guarantee it will not be accessed. Notably, methods with no return statement return null. So
This is an open issue.
The reified type of
This is because a generic of type T may use that type T (for instance, in |
I don't think that's a good intuition to have about The fact that The intuition that does work is that
It is still possible to justify that based on the understanding that
Here, the error message explicitly spells out that a value of type For the next example, you're right that we need a correction: Future<void> foo2() async {
return 0; // no error (!!)
} I added a sentence about this situation and a reference to this issue here, where
Right, you're allowed to discard any value. We discussed this, and you could certainly claim that it's equally bad to discard a value by passing it as an actual argument to a function as it is to discard it with
This is allowed until we add a 'voidness preservation' analysis (which is mentioned in the generalized void feature specification). Voidness preservation deals with all the situations where a "derived type" can be void printVoidList(List<void> list) {
print(list); // Formal parameter type `Object`, no problem.
}
void main() {
printVoidList(const <int>[0]); // Assign `List<int>` to `List<void>`, no problem.
} First, note that it was decided at a very early point that Hence, it is purely a static property to be In particular, we don't want to generate code for This illustrates that being Next, However, if you had done something like With that, the first example is also covered:
For the last example, However, this is a rather draconian rule and in the end we may decide that we won't enforce it. If we don't enforce it then there is a known soundness hole in the treatment of Again, we know how to plug this hole, but it's draconian in the sense that it forces type variables without bounds to be treated like The other flagged statement is So you are pointing out several sharp bends on the road to |
+Everything that Erik just said!
I think you're thinking that void is kind of like Haskell's
This means you can do the following:
|
I think I'm mostly saying two things here:
IMHO what mockito is doing is wildly bogus and i have no problem saying that to do that you need lots of explicit casts or pragmas or //ignores or whatever. |
Note: I edited this a bunch since first posting... How about a lint for assigning to a void value? And a lint for inferring a type parameter of type void? The latter will require you to use //ignore in many cases with mockito and be hard to write, but the former would be very easy. And these makes sense as a lint, because it isn't a soundness problem, but more like a "you seem to be holding this wrong" problem. here's what we could/couldn't catch with these two lints.
...
But I do respectfully disagree here. I'd invite you to consider that what mockito is doing has no soundness issues, produces no bugs, is not surprising to users, is cleaner than the Java version, and is relied upon to work like this in thousands of places. You can say it relies upon something bad, but that does not mean that it itself is bad. I still think that the type you want here is
and I think you should be proud for being one of those rare types of users that is using the type system sufficiently enough to need to use Bottom instead of Top. But those have strong theoretical grounding, so we can't really "fix" them for you if you find it unintuitive (which it is) to know which one to use. As a general rule of thumb, data coming in will use Bottom when data going out uses Top, and vice versa. Void is a form of Top, and Null is Bottom. So if you would be returning a void value, then you want to accept a Null value. |
As another example of
This seems like a good idea until you realize that its typesafe to abort the program anywhere.
^^ this is fine. It won't print, it will throw. To do this, we use the Type indicating "Nothing." Namely, Null, or Bottom. This is the subtype of all types.
Granted, this makes more sense in lazy languages:
but it still can be useful in dart for the same type of reason:
Here, if we use "() -> void" then that's not a subtype of "() -> T". But "() -> Null" is for any T.
You can see how this is a better example of "Nothing" than "void." By using Bottom, we can say "this value is usable anywhere" and the reason we can safely say it is because it will never take on any values -- you type a function as return Null to say it never returns. This is the opposite of saying it returns any value: "void." In that case, it does take/return a value, we just make zero promises about what it is. So if it never takes/returns a value -> we can promise it does everything. Its a subtype of all types. Because it won't happen. Note that if dart gets non-nullable types, then Null will no longer be the Bottom type. We'll probably add a new type named Nothing or something. |
I understand the theory (mostly), but I think you're fighting an uphill battle. "void" simply looks like it means "nothing" (also void. n. a completely empty space.). Writing code like "void foo() {}" feels natural. Writing code like "Null foo() {}" does not. As soon as "void" was a valid keyword, we started using We should go with the flow here and just have |
I think the lint is still a good approach, because values being thrown away isn't a type error but it is a code smell. Its still tricky to know where to draw the line, and I think It will be better in the future if we make non-nullable types, and Bottom is called Nothing instead of Null. Then I think people will feel proud of themselves for using the type system to the point of distinguishing |
I think it might be easier to reconcile common intuitions and language semantics by focusing on the other words spelled So when we encounter a |
If we were able to take each developer aside and show them each dictionary definition and have a discussion with them about why they were wrong to think In practice, people will look at Dart code and be confused, then they'll either write buggy code, or they'll give up and use another language. |
The other difference is that it is an error for a function that returns Object to not return anything. But, I have an actual practical question: I'm designing an API and need to return a Future that has no value associated with completion. Should I use |
A Dart function will always return something, but
Use
|
@MichaelRFairhurst Is there still work to be done here? If so, can you add this to the 2.1 milestone? If not, please close it. |
As far as I can tell the issues mentioned in the very first comment in this bug still exist. |
I suspect that they may be covered by invalid_returns.md, in which case we're just awaiting implementations to catch up (and that's handled in other issues). The following is a verbatim copy of the code in the initial text of this issue, with my comments placed after each snippet of code. void foo1() {
return 0; // "The return type 'int' isn't a 'void', as defined by the method 'foo1'"
} This an error, and that's also the requested behavior as far as I understand: Done. (I believe all implementations behave in that way as well.) import 'dart:async';
void main() {
final Completer<void> completer = new Completer<void>();
completer.complete(0);
completer.future.then((void value) {
print(value); // "The expression here has a type of 'void', and therefore cannot be used"
});
} Again an error (and implemented as such by the analyzer as well as the vm today), which is also what this issue requested: Done. Future<void> foo2() async {
return 0; // no error (!!)
} True, invalid_returns.md does not make this an error. It's a tricky case: In an asynchronous function, So we are not returning o, we are using it to complete a future. This makes the situation similar to So this may be a controversial choice, but it's not going to be realistic to change it now (that Dart 2 has been released). void main() {
final List<void> a = <void>[];
a.add(0); // no error at analysis time or runtime (!!)
print(a); // prints [0] (!!)
} This is again as intended: Any non-void value can be discarded (except, as mentioned, via Note that it was explicitly part of the design discussions that a function like So I'll consider that as done as well. void addNumber(List<int> list) {
list.add(0);
}
void main() {
final List<void> list = <void>[];
addNumber(list); // no analyzer error, though the Dart2 runtime does throw here
print(list);
} During There is a dynamic error (as mentioned: 'does throw'), but that's just because a Another matter is that the downcast is also a voidness preservation violation, but we are not checking that at this point. void printVoidList(List<void> list) {
print(list); // prints [0] (!!)
}
void main() {
printVoidList(const <int>[0]); // no error at analysis time or runtime (!!)
} It is not an error to pass a
import 'dart:async';
List<Completer<dynamic>> pendingWork = <Completer<dynamic>>[];
int addWork<T>(Completer<T> work) {
pendingWork.add(work);
return pendingWork.length - 1;
}
void completeWork<T>(int index, T value) {
pendingWork[index].complete(value);
}
void main() {
Completer<int> completer = new Completer<int>();
int index = addWork<void>(completer); // no error (!!)
completeWork<void>(index, 0); // no error (!!)
completer.future.then(print); // prints "0"
} The issue I can see here (presumably motivating the claim that 'you can be completely messing up the types') is that we do not enforce the constraints of So we can do this: That's just a fact of life: The treatment of Other than that, I don't really see any types going wrong: You would just replace So what you are showing here, I suppose, is that (1) when we treat type variables like non-void types, and (2) when we (still) don't check for voidness preservation, even a short snippet of code will allow us to "break the rules" around We might consider adding more strictness, e.g., by flagging situations where an expression e whose type is a type variable with no bound is subject to an upcast to a top type (like The other topic that remains on the table is that we don't prevent So I don't think we're talking about bugs, and it might well be a useful clarification to submit the "discussion issues" separately, in new issues, and this issue could then be closed. @Hixie, would it work for you to proceed in that manner? |
I don't mind if this is refiled as a new issue, I'll leave that up to you. If having these issues flagged as errors in the analyzer is not possible, would it be possible instead to just have a lint that catches these issues? e.g. a lint that treats an async return the same way a regular return would be treated, and so on? My goal here isn't to litigate what should be an error or not, I just wish to have the tooling help us catch bugs sooner. |
Sounds good! I just created #34455. I think we can certainly cover these things with lints: (1) The special case on The reason why it's no problem to make them all lints (from a technical point of view) is that all these restrictions are outside the core notion of soundness of Dart: We can access all properties of an instance just fine even though we obtained it from an expression of static type |
I think null safety cleaned up some of this. I'll take @Hixie 's original 4 examples of "there should be a static error here":
This now does result in an error. 🎉
The
This is now a compile-time error. 🎉
This continues to pass static analysis, and has no errors at runtime. 😦 |
Note that we have used the phrase 'voidness preservation' to describe a feature which is concerned with the detection of higher-order versions of errors like the error at Some efforts were made in this direction, cf. dart-lang/linter#1124. On the other hand, we never had any proposals (as far as I know) which would make it an error/warning/lint to execute I think it would be difficult to do anything meaningful in this area. For instance, for an |
I may be confused, but my understanding and intuition is that
void
meansnothing
, and something of typevoid
cannot take on any values. This understanding is supported by the way that Dart doesn't let you return anything fromvoid
-returning methods:It also doesn't let you read anything from arguments that are explicitly typed
void
:(In both these cases, these are only analyzer restructions; the compiler allows both. But that's fine, so long as one of them complains, I consider it "invalid Dart".)
Somehow, though, this doesn't always hold. For example, you can return a value from an
async
method that returnsFuture<void>
:You can also pass values to methods expecting
void
with no ill effects:According to the analyzer, you can pass "void"-expecting objects into APIs that expect other types, though that is a runtime error in Dart2 (not Dart1):
You can pass non-void expecting objects into APIs that expect void objects, with unexpected results:
This leads to situations where you can be completely messing up the types, but neither the analyzer nor the compiler will complain. This code is based on actual code in the Flutter framework that tries to use generics to catch programmer errors, but currently fails to catch any errors because of this:
The text was updated successfully, but these errors were encountered: