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

Type annotations affects Dart VM program execution (cannot resolve class name) #396

Closed
DartBot opened this issue Nov 9, 2011 · 10 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@DartBot
Copy link

DartBot commented Nov 9, 2011

This issue was originally filed by [email protected]


This program should run without errors in production mode:

f(foo x) => x;
main() {
  print(f("yoyoma"));
}

~/projects/dart % dart test-types.dart
'/Users/olov/projects/dart/test-types.dart': line 1 pos 3: cannot resolve class name 'foo' from ''

f(foo x) => x;
  ^

I think we'd like a warning instead of an error. dartc gets this right, it seems.

@ghost
Copy link

ghost commented Nov 9, 2011

Set owner to @crelier.
Added Area-VM, Triaged labels.

@iposva-google
Copy link
Contributor

Added Accepted label.

@crelier
Copy link
Contributor

crelier commented Jan 19, 2012

It is not clear to me that this program should run without errors in production mode.

Let's consider this other program:

typedef F(bool x);
f(foo x) => x;
main() {
  if (f is F) {
    print(f("yoyoma"));
  }
}

Should this program also print "yoyoma" without error? In my opinion, it should report a compilation error that foo is not defined. Now, why should the first one behave differently?

I think that type annotations for formal parameters and function results should not be ignored, even in production mode, because these types are relevant when a function is closurized and can therefore change the behavior of a program. The same could apply to the types of fields if implicit getters/setters can be closurized (not supported yet, I think). Only the types of local variables can be ignored.

Reassigning the bug to Gilad for clarification.


Set owner to @gbracha.

@crelier
Copy link
Contributor

crelier commented Jan 19, 2012

One more comment:

We could state that erroneous type annotations are ignored until the type they represent is used in a type test. In that case, the first program would run without error, but not the second one. On the implementation side, this would require the introduction of a "bad type" in order to delay error reporting until a type test is applied to a "bad type".

The claim that type annotations cannot change the behavior of a program in production mode remains invalid, though.

@gbracha
Copy link
Contributor

gbracha commented Jan 19, 2012

The first program should run without error. The fact that foo is undefined is NOT an error. It is a type warning.
The 2nd program has a different issue - we need to compare the type of f against the type F, and we need to decide how that comparison should work.

f is F means (foo) -> Dynamic <: (bool) -> Dynamic, which comes down to foo <-> bool (where <: is subtype-of, and <-> is assignable-to). Since foo is undefined, we need to decide how to interpret it as a type. In order to avoid spurious type errors propagating, in an optional type system one interprets undeclared types as type Dynamic (while still giving a warning where they occur). And so, we have Dynamic <-> bool, which is true.

Conclusion - the 2nd program should run and print "yoyoma".

The next question is what should happen in checked mode. The same, since checked mode simply adds more is-tests.

@gbracha
Copy link
Contributor

gbracha commented Jan 19, 2012

On the other hand ...

1 similar comment
@gbracha
Copy link
Contributor

gbracha commented Jan 19, 2012

On the other hand ...

@gbracha
Copy link
Contributor

gbracha commented Jan 19, 2012

On the other hand, the spec actually addresses this, and says that in checked mode, it is an error if a malformed type is used in a subtype test. In other words, in checked mode the 2nd program will fail at the point where the is test occurs.

@crelier
Copy link
Contributor

crelier commented Feb 28, 2012

Set owner to @crelier.
Added Started label.

@crelier
Copy link
Contributor

crelier commented Feb 29, 2012

Fixed in r4732.


Added Fixed label.

@DartBot DartBot added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Feb 29, 2012
nex3 pushed a commit that referenced this issue Aug 31, 2016
…rams (issue #396)

Right now one also needs to regenerate the runtime (see #397):
./tools/build_sdk --no-destructure-named-params
nex3 pushed a commit that referenced this issue Aug 31, 2016
Allow disabling named param destructuring with --destructure-named-params (issue #396)
nex3 pushed a commit that referenced this issue Aug 31, 2016
This is so the Atom plugin doesn't need to regenerate the SDK (it still
needs to be compiled with --no-destructure-named-params, though, until
Atom is updated to more ES6-compliant node/Chrome).
nex3 pushed a commit that referenced this issue Aug 31, 2016
Compile the sdk with --no-destructure-named-params (#396)
copybara-service bot pushed a commit that referenced this issue Jun 2, 2022
Changes:
```
> git log --format="%C(auto) %h %s" 5699caf..e3f4bd2
 https://dart.googlesource.com/markdown.git/+/e3f4bd2 example: update CDN asset links (#435)
 https://dart.googlesource.com/markdown.git/+/a678bfc example: add GitHub markdown CSS (#434)
 https://dart.googlesource.com/markdown.git/+/bc79c43 Merge pull request #425 from dart-lang/pq-patch-1
 https://dart.googlesource.com/markdown.git/+/4e8aa03 add pub badge
 https://dart.googlesource.com/markdown.git/+/7987e1e Remove dependency on third party package:charcode.
 https://dart.googlesource.com/markdown.git/+/90995fd Split block_parser.dart and inline_parser.dart (#422)
 https://dart.googlesource.com/markdown.git/+/8bb9062 Add trailing commas to some parameter lists to get better formatting (#420)
 https://dart.googlesource.com/markdown.git/+/1c5f2e7 Enable raw strings lint rules (#418)
 https://dart.googlesource.com/markdown.git/+/4784153 Enable use_if_null_to_convert_nulls_to_bools lint rule (#417)
 https://dart.googlesource.com/markdown.git/+/0d67e99 Enable prefer_interpolation_to_compose_strings (#416)
 https://dart.googlesource.com/markdown.git/+/5561351 Enable prefer_final_locals lint rule (#415)
 https://dart.googlesource.com/markdown.git/+/6d39147 Create DelimiterSyntax to replace TagSyntax (#407)
 https://dart.googlesource.com/markdown.git/+/4f4e899 Add caseSensitive parameter on the InlineSyntax constructor (#400)
 https://dart.googlesource.com/markdown.git/+/e16aff0 Check parser.isDone when title is null in _parseInlineBracketedLink (#394)
 https://dart.googlesource.com/markdown.git/+/3471578 Use `Uri.toFilePath()` instead of `Uri.path` for locating tests (#396)

```

Diff: https://dart.googlesource.com/markdown.git/+/5699cafa9ef004875fd7de8ae9ea00e5295e87a4~..e3f4bd28c9e61b522f75f291d4d6cfcfeccd83ee/
Change-Id: Ie04b17dfcce57fcd9e814bd8b9a09677a91136d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246984
Auto-Submit: Devon Carew <[email protected]>
Commit-Queue: Devon Carew <[email protected]>
Reviewed-by: Phil Quitslund <[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-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants