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

"is" operator should support an expression on the right hand side #27680

Open
Hixie opened this issue Oct 27, 2016 · 11 comments
Open

"is" operator should support an expression on the right hand side #27680

Hixie opened this issue Oct 27, 2016 · 11 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Oct 27, 2016

This should be possible:

void main() {
   Type t = num;
   var foo = 1.0;
   if (foo is t)
     print('foo is $t');
}

Instead you get a runtime exception ("malformed type: line 4 pos 15: using 't' in this context is invalid") and an analyzer error ("The name 't' is not a type and cannot be used in an 'is' expression").

@Hixie
Copy link
Contributor Author

Hixie commented Oct 27, 2016

@rmacnak-google rmacnak-google added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Nov 1, 2016
@leafpetersen
Copy link
Member

This was discussed by the language team earlier this year, and the sentiment seemed broadly favorable. I don't think we will be able to get to it this quarter though.

@matanlurey
Copy link
Contributor

I'm curious - does this have potentially expensive cost for say, dart2js?

One reason I bring this up is we are constantly check compiled output and trying to avoid reified type information in production. As far as I know ( @rakudrama @hterkelsen ), dart2js does a decent job today of removing reified type information when it's not used (i.e. no is call, no .runtimeType).

Would this language feature mean dart2js defensively has to hold all reified type information?

@leafpetersen
Copy link
Member

I don't think it should make any difference at all, but @rakudrama can weigh in.

You already have (x is T) where T is a statically unknown type variable, so I don't see much difference from an implementation perspective between (Type t, var x) => x is t and <T>(var x) => x is T, at least with respect to what types you have to retain. In principle you can be more aggressive with monomorphisation of generics than you can with inlining, but I don't dart2js does much - it has a code size penalty.

@matanlurey
Copy link
Contributor

matanlurey commented Aug 3, 2017

But x is T is easily catchable in static/global analysis/optimization, i.e. if we never see x is FooBar, we can omit keeping reified type information for FooBar. If x is t means unless we are definitive of all values assigned to t we can't?

@mraleph
Copy link
Member

mraleph commented Aug 3, 2017

@matanlurey you just look at which type literals are created in the program - that gives you a conservative approximation of all possible values of t.

@matanlurey
Copy link
Contributor

But I want a liberal optimization not a conservative approximation :(

@leafpetersen
Copy link
Member

But x is T is easily catchable in static/global analysis/optimization, i.e. if we never see x is FooBar, we can omit keeping reified type information for FooBar.

No. Counter-example:

bool check<T>(var x) => x is T;
void test() {
  check<FooBar>(3);
}

There is no static occurrence of x is FooBar, but dynamically you need to be prepared to answer that query. Now, you can easily inline and/or mono-morphise here, but you can't do that everywhere (code size), and you can also do that for type literals.

@matanlurey
Copy link
Contributor

Ah, that's fair. I guess I'd just be concerned about language features that make it too hard to optimize, but if you're not concerned I guess I'm not concerned either 🤣

@rakudrama
Copy link
Member

dart2js perspective:

TL;DR: I'd need to read a spec before trying to determine if this would be bad for dart2js.

Does this feature carry it's weight? The check<T>() trick is strictly more powerful - consider check<List<int>>().
If all you can do with a Type is 'is-check' and print it, it feels like a half-feature where a predicate typedef would do.

I would be concerned about many other ideas like this that would feel natural is x is t was supported.
x is t feels like the edge of a slippery slope. (Using a Type value as a constructor is a bad idea for tree shaking. That would make our type analysis for constructors as bad as our type analysis for closure calls, which is weak - most closures escape to unknown call sites, the same would be true of type literals).

We would have to do a lot of work with the dart2js runtime representations to support x is t but it should not lead to extravagant bloat. We would not consider working on this until after completing the kernel transition.

What is true of the current analysis is that we find a relatively small number of types from type expressions in constructor type parameters flow to a context like x is T. This is usually much smaller than the number of type literals, thousands of which are used as keys in Angular injection. Since the current type bindings are restricted in how they work and sparse within the program text, the flow can be computed precisely and quickly. This is not true of general values.

How would this actually work?

dynamic t = ...
print(1 is t);

Would it be permissible to implements Type and provide an operator is method?
There might be some devil in the details that might complicate tree-shaking.
I would want these details (i.e. a spec) before making a final call on whether the feature is problematic for dart2js.

@Hixie
Copy link
Contributor Author

Hixie commented Aug 3, 2017

The most recent case I'm aware of where Flutter would have liked this is that we have a Map of Type to instance, where the instances must implement the type given by the key. We want to assert that this invariant is met:

  assert(map[key] is key);

...but today the best we can do is:

  assert(map[key].runtimeType == key);

...which isn't enough because the instance might be a subclass of key, or just implement key.

@lrhn lrhn added the type-enhancement A request for a change that isn't a bug label Jun 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants