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

no type check when using 'this' style constructor parameters #464

Closed
DartBot opened this issue Nov 15, 2011 · 7 comments
Closed

no type check when using 'this' style constructor parameters #464

DartBot opened this issue Nov 15, 2011 · 7 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 15, 2011

This issue was originally filed by [email protected]


This program gives no type error/warning in frog:

main() {
  print("hello");
  new Foo(5);
  print("done");
}

class Foo {
  String s;
  Foo(this.s) { }
}

But, if I change the constructor syntax to not use the 'this' style shorthand, so it reads like so:

main() {
  print("hello");
  new Foo(5);
  print("done");
}

class Foo {
  String s;
  Foo(String s) : s = s { }
}

then I do get the correct warning:
4:3: warning: type "int" is not assignable to "String"
  new Foo(5);
  ^^^^^^^^^^

Apparently using 'this' in constructor formals is somehow disabling type checking.

@jmesserly
Copy link

This is sort of "by design". Those parameters are technically typed to "var". It's like if you wrote "Foo(s): this.s = s {}". The grammar does support adding the type to the constructor too, if you want it: "Foo(String this.s) {}". Then you'll get the correct checking.

What you want here is for a bit of simple type propagation from the field's type to the constructor parameter's type. The Dart language spec doesn't currently use type inference anywhere as part of static type warnings, but I agree it would be really useful. Especially for cases like this where the inference is so simple.

Changing to Enhancement but leaving open.


Removed Type-Defect label.
Added Type-Enhancement, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Nov 17, 2011

This comment was originally written by [email protected]


I see. Thanks for the explanation. Makes sense.

@DartBot
Copy link
Author

DartBot commented Nov 17, 2011

This comment was originally written by [email protected]


I've moved this to a language issue and listed it as a defect until Gilad has a chance to look at it and decide it is something else.

I always thought that Matt's example would create the type error that he is expecting. I also expected that doc tools would show the signature of his initial constructor as Foo(String s) and that the this.s part would just disappear as an implementation detail. If we really want people to specify the type twice - once on the this.s parameter and once on the field, I think that's a significant flaw in the language. Whatever the final choice is, I hope that we can keep the prinicple of say it only once.

To be clear: I believe that the language/spec bug is exactly what Matt was pointing out in his original report against frog. I believe that John is correct that frog is following the current language spec and that this issue is with the spec and not the implementation.


Removed Type-Enhancement, Area-Frog labels.
Added Type-Defect, Area-Language labels.
Changed the title to: "no type check when using 'this' style constructor parameters".

@gbracha
Copy link
Contributor

gbracha commented Dec 15, 2011

  1. This is feature request, not a bug. If you don't like the language semantics, that's fine but that means you want them changed.

  2. I think this can be addressed as follows:

Redefine the meaning of initializing formals so that

class Foo {
  T x;
  Foo(this.x);
}

means

class Foo {
  T x;
  Foo(T x'): this.x = x'; // where x' is a fresh variable
}

Technically, no type inference is taking place (which is good; type inference is not something the language wants to rely on).

No promises yet; we'll review and decide soon though.


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

@gbracha
Copy link
Contributor

gbracha commented Dec 16, 2011

Issue #288 has been merged into this issue.

@gbracha
Copy link
Contributor

gbracha commented Dec 16, 2011

Added Accepted label.

@gbracha
Copy link
Contributor

gbracha commented Feb 28, 2012

Fixed in the 0.08 draft.


Added Done 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 Feb 28, 2012
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
copybara-service bot pushed a commit that referenced this issue Oct 5, 2022
http_parser (https://github.com/dart-lang/http_parser/compare/b968f7d..c739675):
  c739675  2022-10-05  Devon Carew  Merge pull request #60 from dart-lang/prep_publish
  a9247c2  2022-10-04  Devon Carew  update CI; prep for publishing

logging (https://github.com/dart-lang/logging/compare/f5d6442..f322480):
  f322480  2022-10-05  Devon Carew  update CI config (#121)

markdown (https://github.com/dart-lang/markdown/compare/5713b50..e7915ed):
  e7915ed  2022-10-04  Zhiguang Chen  Improve the match pattern of inline html (#464)

pub_semver (https://github.com/dart-lang/pub_semver/compare/9fd2875..eadf516):
  eadf516  2022-10-04  dependabot[bot]  Bump actions/checkout from 3.0.2 to 3.1.0 (#70)
  edd9d82  2022-10-04  Devon Carew  update the CI; update readme (#69)

watcher (https://github.com/dart-lang/watcher/compare/e00c0ea..3259107):
  3259107  2022-10-04  Devon Carew  Merge pull request #127 from devoncarew/update_ci
  e70781d  2022-10-04  Devon Carew  add markdown badges
  ff9d1f2  2022-10-04  Devon Carew  update CI; add markdown badges

Change-Id: I81da3d3ba774b16dc0441b7ed371765c7b86d28c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/262860
Reviewed-by: Nate Bosch <[email protected]>
Auto-Submit: Devon Carew <[email protected]>
Commit-Queue: Nate Bosch <[email protected]>
This issue was closed.
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

5 participants