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 inconsistent with type coercion for member variables #21530

Open
DartBot opened this issue Nov 6, 2014 · 6 comments
Open

"is" operator inconsistent with type coercion for member variables #21530

DartBot opened this issue Nov 6, 2014 · 6 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

@DartBot
Copy link

DartBot commented Nov 6, 2014

This issue was originally filed by [email protected]


What steps will reproduce the problem?

  1. Create an if statement with an is operator to test a member variable type against a sub-type.
  2. In the if block try to use the member variable as the coerced type.

What is the expected output? What do you see instead?
Expected: Dart should coerce the member as it does with local variables and function parameters.
Actual result: Dart editor produces an error and requires an explicit cast to use the type.

What version of the product are you using?
Dart 1.6.0

On what operating system?
Mac OSX

What browser (if applicable)?
N/A

Please provide any additional information below.
I would actually prefer Issue #5031 be addressed then an implicit coercion be used, but since the implicit coercion appears to be the preferred method in Dart it should be consistent.

@kasperl
Copy link

kasperl commented Nov 7, 2014

Added Area-Language, Triaged labels.

@lrhn
Copy link
Member

lrhn commented Nov 7, 2014

Removed Type-Defect label.
Added Type-Enhancement label.

@lrhn
Copy link
Member

lrhn commented Nov 7, 2014

The problem with allowing
  if (x is Foo) {
    bar();
    x.fooMethod();
  }
to type-check even if x is an instance getter, is that it might hide a bug.
The "x" field may be typed as Object, and it might be set to something else during the "bar()" call, without breaking the contract of the object.
If you call "x.fooMethod()", the warning you get correctly tell you that you can't be sure that "x" has a fooMethod.
If you absolutely know that x won't change, you can easily change it to:
  if (x is Foo) {
    Foo foo = x;
    bar();
    foo.fooMethod();
  }
and avoid the warning (and also avoid any problems if x does in fact change).

@DartBot
Copy link
Author

DartBot commented Nov 7, 2014

This comment was originally written by [email protected]


That is a good point and another fair argument in favor of Issue #5031 as the pattern would remain consistent across all usages.

ex:

Foo foo = x as Foo; // <-- not possible with throw :(
if(foo != null)
{
    bar();
    foo.fooMethod();
}

Currently 'as' is basically useless at this point and actually could introduce a bug in the implicit / non-implicit scenario you have described.

consider:

if(x is Foo)
{
    bar();
    (x as Foo).fooMethod();
}

With the current generic warning generated from the 'is' block with member usage; a programmer could very well think the above approach would fix the problem, but would yield a similar result as the bug you described.

In the end I would prefer casting to be more explicit regardless. There are valid situations when they are handy and necessary, but the programmer should pretty much always be cognizant of the usage and possible side effects. What if, for example, the user actually intends to call fooMethod on the current member and not a local variable as they know that bar may change said member. Maybe removing the reference was an intended side effect of 'destroying' the previous object.

The programmer would need to do another cast regardless in this case (any casting pattern will do):

if(x is Foo)
{
    bar();
    if(x is Foo) // <-- Make sure we are still substitutable for a Foo
    {
        Foo foo = x;
        foo.fooMethod();
    }
}

In the end casting is an explicit act and would preferably be used in that way.

I would think at least 1 of two resolutions could improve the situation a bit.

  1. Issue make 'as' not throw if the expression does not have the type. #5031
  2. Add more explicit error output on the member 'is' block to inform the developer 'Implicit cast of %MEM to %TYPE may have unintended side effects'

@gbracha
Copy link
Contributor

gbracha commented Dec 23, 2014

We are looking to make type promotion more accepting, at the cost of further unsoundness.


Set owner to @gbracha.
Added Accepted label.

@DartBot DartBot added Type-Enhancement area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Dec 23, 2014
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed accepted labels Feb 29, 2016
@munificent munificent changed the title is operator is inconsistent with type coercion for member variables "is" operator inconsistent with type coercion for member variables Dec 14, 2016
@srawlins
Copy link
Member

srawlins commented Jun 6, 2019

Duplicate of #34480 which, while opened later, has more discussion and examples.

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

6 participants