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

Fasta inserts initializer for not initialized fields #31073

Open
scheglov opened this Issue Oct 11, 2017 · 5 comments

Comments

3 participants
@scheglov
Contributor

scheglov commented Oct 11, 2017

class A {
  final int x;
  A() {}
}
library;
import self as self;
import "dart:core" as core;

class A extends core::Object {
  final field core::int x = null;
  constructor •() → void
    : super core::Object::•() {}
}

So, Analyzer cannot distinguish uninitialized fields and report the appropriate error.

image

@scheglov

This comment has been minimized.

Show comment
Hide comment
@scheglov

scheglov Oct 11, 2017

Contributor

Can we get a flag like "artificial initializer" for such fields?
Maybe in metadata?

Contributor

scheglov commented Oct 11, 2017

Can we get a flag like "artificial initializer" for such fields?
Maybe in metadata?

@scheglov

This comment has been minimized.

Show comment
Hide comment
@scheglov

scheglov Oct 11, 2017

Contributor

The same problem is for default values of optional parameters.
Fasta inserts null, and this does not let Analyzer to report a warning.

class A {
  foo([x]) {}
}
class B extends A {
  foo([x = 1]) {}
}
library;
import self as self;
import "dart:core" as core;

class A extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  method foo([dynamic x = null]) → dynamic {}
}
class B extends self::A {
  default constructor •() → void
    : super self::A::•()
    ;
  method foo([dynamic x = 1]) → dynamic {}
}

There is a semantic difference between not having a default value, and having null.
See 10.1 Instance Methods in the specification (the selection is mine).

It is a static warning if an instance method m1 overrides an instance member
m2 and the type of m1 is not a subtype of the type of m2. It is a static warning
if an instance method m1 overrides an instance member m2, the signature of m2
explicitly specifies a default value for a formal parameter p and the signature
of m1 implies a different default value for p.

Contributor

scheglov commented Oct 11, 2017

The same problem is for default values of optional parameters.
Fasta inserts null, and this does not let Analyzer to report a warning.

class A {
  foo([x]) {}
}
class B extends A {
  foo([x = 1]) {}
}
library;
import self as self;
import "dart:core" as core;

class A extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  method foo([dynamic x = null]) → dynamic {}
}
class B extends self::A {
  default constructor •() → void
    : super self::A::•()
    ;
  method foo([dynamic x = 1]) → dynamic {}
}

There is a semantic difference between not having a default value, and having null.
See 10.1 Instance Methods in the specification (the selection is mine).

It is a static warning if an instance method m1 overrides an instance member
m2 and the type of m1 is not a subtype of the type of m2. It is a static warning
if an instance method m1 overrides an instance member m2, the signature of m2
explicitly specifies a default value for a formal parameter p and the signature
of m1 implies a different default value for p.

@peter-ahe-google

This comment has been minimized.

Show comment
Hide comment
@peter-ahe-google

peter-ahe-google Oct 12, 2017

Contributor

Fasta should report these errors.

Contributor

peter-ahe-google commented Oct 12, 2017

Fasta should report these errors.

@MichaelRFairhurst

This comment has been minimized.

Show comment
Hide comment
@MichaelRFairhurst

MichaelRFairhurst Dec 8, 2017

Contributor

Did this get fixed recently? I'm seeing tests passing that referenced this issue but its status is unchanged.

Contributor

MichaelRFairhurst commented Dec 8, 2017

Did this get fixed recently? I'm seeing tests passing that referenced this issue but its status is unchanged.

@MichaelRFairhurst

This comment has been minimized.

Show comment
Hide comment
@MichaelRFairhurst

MichaelRFairhurst Dec 8, 2017

Contributor

Just realized only some of the test failures matching this ticket have started passing. I'm assuming its being worked on, is all.

Contributor

MichaelRFairhurst commented Dec 8, 2017

Just realized only some of the test failures matching this ticket have started passing. I'm assuming its being worked on, is all.

whesse pushed a commit that referenced this issue Dec 8, 2017

Enable previewDart2 in static warning code kernel tests. Others passing.
Enable previewDart2 and annotate all @potentialAnalyzerProblems.

Also fix two of the @failingTests related to
#31073 which appear to be
passing now.
Bug:
Change-Id: Ic2fd534c8a9cf45eccf0f5d09dc47cd4e78aaf42
Reviewed-on: https://dart-review.googlesource.com/27468
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>

@kmillikin kmillikin added this to Incoming in Dart Front End Jan 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment