-
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
Type inference is confusing on pattern context for List<String>.map(jsonDecode) #54676
Comments
Thanks for the bug report, @whesse! I dug into it, and your test case highlights a subtle difference between type inference in the analyzer and the CFE. Here's what's happening:
The question is, what do we want to happen? I can't find anything in https://github.com/dart-lang/language/blob/main/resources/type-system/inference.md specifying whether or not it should be an error if the set of type constraints is contradictory. I can think of good arguments in both directions. On the one hand, if type inference comes up with contradictory type constraints, emitting an error seems like good thing, because mathematical systems like type theories are well known to behave in really weird and counterintuitive ways when trying to reason from a contradiction; it's probably better to just bail out and let the user decide what to do. On the other hand, the type produced by type inference in this case actually works just fine, and so producing an error message is really confusing for the user (as @whesse observed, the analyzer seems to be claiming that a type parameter of I think it's also worth considering whether we can glean some insights from this example into how type inference could be improved in the future. For example, we have been considering adding additional kinds of type schema "operators" to represent sets of types that aren't expressible using the current framework; if we had an operator In any case, I think these are questions for the language team. TL/DR: if, during type inference, a set of contradictory type constraints is created, should that be a de facto compile-time error? Or should it only be a compile-time error if it leads to a set of types being chosen that cause problems in later analysis? The analyzer takes the former approach; the CFE takes the latter. What do you think, @dart-lang/language-team? |
I would say that contradictory type constraints should be detected, and the situation should be considered to be a type inference failure ("could not solve the constraints"). The response to a type inference failure could vary: A compile-time error could be reported. Alternatively, However, I don't think we can ignore the fact that the constraints have been violated (say, if we have |
A couple of preliminary comments. First, here's a simpler reproduction: // The argument to this function will be checked in a context
// that looks like Map<_, String>
void mkMapContext<T>(Map<T, String> argument) {}
// This method returns a T where T is its argument type.
T id<T>(T x) => x;
void main() {
dynamic d = <dynamic, String>{};
mkMapContext(id(d));
} Second: I'm not directly concerned about soundness here. Because of the dynamic nature of Dart, there are lots of places where inference (or other algorithms) may produce unsuitable types. There are type systems in which if inference succeeds, the program is well-typed: Dart does not have such a system. So in general, it is incumbent upon the front-ends to do the necessary assignability, bounds, and well-boundedness checks after inference. If they are not doing so correctly, that is of course potentially a soundness bug, but I don't think that's really the core of this issue. Third: This is, I think, a consequence of the fact that in a few cases (mostly around implicit dynamic casts) context types are not requirements, but the subtype matching algorithm in inference essentially treats them as if they were. That is, when we have a context I don't immediately have in mind what the right answer here is. A couple of thoughts:
I think my weak preliminary thought is that this check should only be done when strict-inference is enabled, but I'm not sure about that. Some further analysis/discussion may be in order. |
I can see how we end up with void mkMapContext<X>(Map<X, String> argument) {} // Renamed type parameter to `X` to not have two `T`s.
T id<T>(T x) => x;
void main() {
dynamic d = <dynamic, String>{};
mkMapContext(id(d));
} The My feeling is that it's a problem because the type system ignores implicit downcasts, and the code author does not. I don't want to not introduce any constraint, because Here we do have a constraint, When we then don't find any other constraints for mkMapContext<dynamic>(id<Map<dynamic, String>>(d /* as Map<dynamic, String>*/)); I'm probably missing a lot of details, but it feels like something like this would match the actual behavior of the program, and how the auther is thinking about dynamic. |
@lrhn, I'm encouraged by your idea. Phrasing it another way, it seems like what you're doing is trying to encode the possibility of an implicit dynamic downcast into the type constraints, so that when it comes time to solve for the type variables, we can see that there is a solution rather than failing. But I'm having trouble trying to generalize it so that it can apply to @whesse's initial example. Removing some non-essential parts for simplicity, we've got this: import "dart:convert";
List<String> lines = [];
List foo(int iterations) => [
for (final {'event': String event} in lines.map(jsonDecode))
...switch (event) {
_ => [],
}
];
void main() {
print(foo(1));
} In this case, we only have a single generic method to infer (
I can't figure out how to apply your I have some ideas (along the lines of the |
Ack. It doesn't work because we don't actually have any So more cleverness might be needed. (I did say I was probably missing something.) The implicit downcast that I, as a reader, would expect here is at the destructuring assignment. If we had just rejected a void main() {
for (var {"a": String s} in <Map<String,dynamic>>[<String, dynamic>{"a": "b"}] as dynamic ) {
print(s);
}
for (var {"a": String s} in <Map<String,dynamic>>[<String, dynamic>{"a": "b"}] as Iterable<dynamic>) {
print(s);
}
for (var {"a": String s} in <Map<String,dynamic>>[<String, dynamic>{"a": "b"}] as Iterable<Map<dynamic, dynamic>>) {
print(s);
}
} These all work. A TypeOf(expr) $iterable = expr; // Compile-time error if `TypeOf(expr)` is not assignable to `Iterable<dynamic>`.
var $iterator = $iterable.iterator; // Compile-time error if type of `$iterator` is not assignable to `Iterator<dynamic>`
while ($iterator.moveNext()) { // Compile-time error if type of `$iterator.moveNext()` is not assignable to `bool`
decl = $iterator.current; // Compile-time error if type of `$iterator.current` is not assignable to `decl`.
{ body }
} This desugaring happens post type-inference (has to, otherwise it cannot know the type of So to match runtime behavior, inference would have to understand this rewrite, and match it in the inference rules. Which is probably non-trivial. So, let's try again. We introduce
Then we try to solve. Somehow (not my speciality, I'll admit, so I'll handwave). For the
As I said, "hand-waving". And I'm also not sure this isn't just "context type hints" with a more cumbersome syntax. |
That's an interesting proposal. If we include coercions into the notion of the assignability constraint, we may finally solve the problem of giving a formal treatment to where assignments and the related checks and transformations actually happen in expressions. |
I feel like we've had a lot of good discussion here about potential future improvements to type inference to allow @whesse's example to work without having to choose a solution to contradictory type constraints. I'm definitely interested in those discussions for the long term, but they all sound like big enough changes to the type system that we'd probably want to tackle one of them as a language-versioned type inference improvement. So perhaps we should move that discussion to a separate language issue. In the shorter term, we have a situation right now, for the current language version (and all previous language versions that are still supported) where there is a difference in behavior between the analyzer and the front end: if type inference leads to contradictory constraints, the analyzer rejects the program outright, whereas the front end "solves" the contradictory constraints by picking the "lower" constraint, and only rejects the program if that choice leads to a static type error. I would love for us to declare one of these two behaviors to be the "correct one", and make a fix so that the two implementations match. Let me try to make a stab at the pros and cons of each approach:
Personally, 3 is my first choice, and 2 is my second. But I could live with any of the three options. |
I'd prefer that one as well. It could then be strengthened to 1 later on. |
Do we know that this only affects If so, would a specification of the 2. behavior be to ignore all constraints of the type I'm OK with 3, but for any choice, we'll also need to specify the actual behavior we end up at. |
I don't think so. It could also apply any time the context type is not a required type, and contains a T id<T>(T x) => x;
f(Object o, Map<String, int> m) {
if (o is String) {
{'x': o} = id(m);
}
} Here, the pattern type schema for
The front end favors the "lower" constraint, so it picks The analyzer gives this error message:
|
I'm sorry if I'm mistaken, but is this similar to what happened at #52117? The downcasting, I mean. |
I'm not seeing a relationship between the two issues. But if you have some insight to share here, I'd love to hear more details. |
As I said, I'm not sure, since some of the inference logic is new to me, but I figured it was similar because of #52117 (comment):
My question for that would be why is More context hereextension Apply<T extends Object> on T {
R apply<R>(R Function(T self) transform) => transform(this);
}
void func<T extends Object?>(T object) {
if (object is DateTime?) {
final formatter = DateFormat('yyyy/MM/dd');
object?.day; // No lint
object?.apply(formatter.format); // The argument type 'String Function(DateTime)' can't be assigned to the parameter type 'dynamic Function(T)'.
}
} Since your comment above (mentioned below) I figured the problem there was the demoting of
P.S.: I'm mostly sure you already know this info, but just to put it here as well since it relates to promoted local variables, and maybe it's part of the problem in this case. I found #56028:
I'm sorry if this is not helpful or is unrelated, for me it sounded like a similar problem. |
@FMorschel In the case of #52117 (comment), we had two issues:
details about #52117When @scheglov says "Apparently we don't infer `R` correctly", I believe he's referring to this part of his code snippet:
(This is part of the text passed to The line extension E<T2 extends Object> on T2 {
void apply<R>(R Function(T2 self) transform) {}
} Note that @scheglov helpfully renamed void f<T extends Object?>(T object) {
if (object is int?) {
object?.apply(g);
}
} Since But this line of the dump:
indicates that the type the analyzer has chosen to substitute in place of the type parameter You ask why My answer to that question is: I think you're right. The ill-formed type being substituted for But anyway, back to the crux of your question, which is, is what's going on here in this issue similar to what happened in #52117? What's happening here in this issue is that type inference is trying to simultaneously satisfy two constraints for the type parameter So no, I don't think the issues are related. But thanks for asking! I hope my answer makes things clearer. |
Applying type inference on
List<String> a;
[ for (final {'event': String event} in a.map(jsonDecode)) switch(event) .... ];
gives an error where the type dynamic can't be inferred for the .map call.
Changing the call to .map<dynamic> makes the error go away.
The error is rather confusing, saying that it tries the candidate dynamic but it doesn't work.
The error (from tip-of-tree 'dart analyze', also is the same on stable 3.2.3) is
Sample code showing the error is below:
The code runs without error, does not fail due to type inference failure.
Simpler cases do not give an error:
The simple case without the pattern (or the control-flow expression) does give an error, but it is more comprehensible.
With the pattern instead, it gives the error shown above:
The comprehensible error tries to propagate the Map type in to the jsonDecode argument:
@johnniwinther @athomas
The text was updated successfully, but these errors were encountered: