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

Be more strict on inferred types with no constraints? #4197

Open
eernstg opened this issue Dec 6, 2024 · 5 comments
Open

Be more strict on inferred types with no constraints? #4197

eernstg opened this issue Dec 6, 2024 · 5 comments
Labels
inference question Further information is requested type-inference Type inference, issues or improvements

Comments

@eernstg
Copy link
Member

eernstg commented Dec 6, 2024

The specification of type inference in inference.md says about the greatest closure of a type in the section Type variable elimination that

- If `S` is `X` where `X` is in `L`
  - The least closure of `S` with respect to L is `Never`
  - The greatest closure of `S` with respect to L is `Object?`

which in the context means that the greatest closure of a type parameter which is in the target set L is Object?.

The section Type schema elimination says that _ is treated as a type parameter in the target set.

Next, the section Grounded constraint solution for a type variable says that

- Otherwise the solution is the greatest closure of `Mt` with respect to `_`.

which in the context means that when a type variable X has the combined constraint _ <: X <: _ then the solution is the greatest closure of _, that is, Object?.

Nevertheless, the implemented behavior chooses dynamic, at least in some cases:

X getContextType<X>(Object? o) {
  print(X);
  return o as X;
}

void main() {
  getContextType(1)..arglebargle; // 'dynamic'.
}

The fact that the analyzer does not report a compile-time error at the invocation of arglebargle shows that the type inferred for getContextType(1) is dynamic rather than Object?. The common front end confirms this by also not reporting a compile-time error, and further by printing dynamic at run time.

It is a breaking change, but I'd recommend that type inference is adjusted to infer Object? rather than dynamic in such cases. We could at least experiment with this outcome and see how severe the breakage is. The change would be language versioned such that developers can opt in to the more strict analysis when they are ready.

@dart-lang/language-team, WDYT?

@eernstg eernstg added question Further information is requested inference type-inference Type inference, issues or improvements labels Dec 6, 2024
@lrhn
Copy link
Member

lrhn commented Dec 6, 2024

(Why is the greatest closure of a type variable with a bound not its bound? Is it because that doesn't work for F-bounded types, or because we want to allow super-bounded types?)

This would be one way to avoid an implicit and un-asked-for dynamics, so I'm all for it.

Might be better to combine it with other "no implicit dynamic changes, though, so we can wipe them all out in one swoop, and users only need to migrate once.

(I'd also make the context type of an expressionStatement's expression be void, but that's another thing.)

@eernstg
Copy link
Member Author

eernstg commented Dec 17, 2024

Is it because that doesn't work for F-bounded types

Exactly.

I'm all for it.

Sounds good!

We should avoid breaking existing code whenever possible, and there might be some kinds of code where the current approach (where a number of expressions have type dynamic rather than Object?) allows for smooth exploration of JSON data or similar entities. Apart from that, I'd expect the breakage to be "good" in the sense that it reveals locations in code where something has the type dynamic even though that wasn't intended (and perhaps nobody noticed it).

So it would be great if we could learn about specific projects or types of coding where this kind of breakage is unwanted and inconvenient. In those situations, it might be possible to introduce the type dynamic by using dynamic as the bound of a type parameter (rather than nothing, or rather than Object?).

@Wdestroier, do you have some situations in mind where you'd prefer to get the type dynamic from type inference, rather than Object??

@Wdestroier
Copy link

I always interpret the code as ommiting the type means dynamic. I can try to compile this project https://github.com/wdestroier/asimox (for example) with this feature enabled. The application is more prone to erros, but maybe they're simple to fix, or the library can be updated. Inferring Object? anywhere is not useful imo. It means omitting the type returns an useless type (Object?) and, effectively, the type necessarily needs to be specified. If Object? is inferred, who wrote X getContextType<X>(Object? o) would still need to use the return value to figure out they forgot to specify the type (2 steps are required to try to notice the problem). They would be better by enabling the lint to always specify generic types. There are 2 useful extremes I guess, allowing to omit the type and inferring dynamic or always requiring to specify the type.

Generics are another source of complexity, is the cast at the end necessary? Maybe it makes sense in the real code, but not in the example and I'm overcomplicating. Or is the cast syntax ugly and people are trying to write method<X>(o) instead of (method(o) as X)?

// Strange cast at the end.
X method<X>(Object? o) {
  // ...
  return o as X;
}

// No generics and no dynamic.
Object? method(Object? o) {
  // ...
  return o;
}

method(o)..arglebargle // Error.
(method(o) as X)..arglebargle; // Ok.

@eernstg
Copy link
Member Author

eernstg commented Dec 18, 2024

This is interesting! I don't hear this perspective on dynamic very often these days, it's more common to wish for all kinds of strictness in typing (which is also typically my preference).

@Wdestroier wrote:

It means omitting the type returns an useless type (Object?)

When it comes to declarations, type inference is actually only used with local function declarations and function literals (lambdas). This means that top-level functions and static and instance methods and getters would still have the return type dynamic when no return type is specified, because that's a separate rule (not part of type inference). For example, the following snippet has no compile-time errors today, and will remain so:

class A {
  // The return type is `dynamic` in all cases.
  static staticMethod() {}
  static get staticGetter => "Hello, I'm a String!";
  method() { return 1; }
  get getter => "Hello!";
}

// The return type of `main` is `dynamic`, too.
main() {
  A.staticMethod().arglebargle;
  A.staticGetter.arglebargle;
  A().method().arglebargle;
  A().getter.arglebargle;
  if (1 > 2) main().arglebargle;
}

Similarly for formal parameter declarations.

I'm not sure how well this covers the use cases that you care about, but it seems likely that it would cover much of it.

About the generic functions:

X method<X>(Object? o) {
  // ...
  return o as X;
}

The purpose of this declaration could be that every call site is allowed to request a result of an arbitrary type ("I want an int!", "I want a List<Iterable<Symbol>>!", whatever it wants) and then method is required to satisfy this request (or throw). I can see how this kind of typing could be useful in cases where it is always possible to live up to the request:

List<X> method2<X>() => <X>[];

void main() {
  List<Iterable<Symbol>> xs = method2(); // OK!
  Iterable<int> ys = method2(); // Sure, no problem!
}

When the return type is X (rather than some type that uses X as a type argument), it may or may not be possible for method to get hold of an actual object with that type. But I'll assume that you're surrounding method by some application specific logic that justifies the expectation, even though the type system doesn't understand why. In any case, the compiler/analyzer shows that the construct isn't known to be safe because we must have the cast o as X.

So why wouldn't you just use this approach, which seems to give you the same type safety properties in a simpler manner?:

method(Object? o) {
  // ...
  return o;
}

This means that the cast to the context type will occur at the call site rather than in the return statement, but the only difference between those two behaviors is that the former has a stack trace which is one line shorter. I don't think there is any other way to see the difference.

The next variant uses the type Object? everywhere. This is safe, but it does require some kind of re-typing (perhaps using a downcast, or a cast to dynamic) before we can do the same things as with the previous approaches:

Object? method(Object? o) {
  // ...
  return o;
}

My assumption is that you don't want this because your application context makes dynamic expressions more convenient, and sufficiently safe.

This proposal would make type inference slightly more strict, but I haven't seen anything in your examples that would stop working. Perhaps it doesn't actually create any difficulties for you?

@Wdestroier
Copy link

Thanks, @eernstg!

It's only my perspective, because I was looking for a language like Java in the past and with dynamic features and Dart (possibly by accident) has a great combination of both.

Perhaps it doesn't actually create any difficulties for you?

These test cases aren't very complete, sorry, I took a real example and tried to wipe as much as possible.

"button" function with generic types and the "onClick" parameter:

// button is one of all the generated HTML elements.
DomElement<Element, Event> button<Element, Event>({
  DomEventConsumer<Element, Event>? onClick,
  ...
}) {...}

"DomEventConsumer" definition:

import 'package:domino/domino.dart' as domino show DomEvent, DomLifecycleEvent;

// domino has effectively the same typedef, except it doesn't tell the return type.
// This typedef exists to ensure domino is only part of the internal implementation.
typedef DomEventConsumer<Element, Event> = FutureOr<void> Function(
  domino.DomEvent<Element, Event> event,
);

Usage:

class MyButton extends DomNode {
  @override
  DomNode render() {
    return button(
      type: 'button',
      text: 'Click me',
      onClick: (event) {
        print('Clicked!');
        event.event // <-- Currently dynamic, would be an error.
          ..stopPropagation()
          ..preventDefault();
      },
    ),
  }
}

Others packages use domino too if I recall correctly, such as jaspr and deact.
Can you tell me if the problem would happen in this example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inference question Further information is requested type-inference Type inference, issues or improvements
Projects
None yet
Development

No branches or pull requests

3 participants