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

analysis issues when compiled with ddc #1612

Closed
devoncarew opened this issue Jul 29, 2015 · 9 comments
Closed

analysis issues when compiled with ddc #1612

devoncarew opened this issue Jul 29, 2015 · 9 comments

Comments

@devoncarew
Copy link
Member

I see two analysis issues when this package is compiled with ddc:

severe: [AnalyzerMessage] The getter '_end' is not defined for the class 'SourceSpan' (package:source_span/src/file.dart, line 237, col 19)
severe: [AnalyzerMessage] The getter '_start' is not defined for the class 'SourceSpan' (package:source_span/src/file.dart, line 237, col 34)
@devoncarew
Copy link
Member Author

This is for 1.1.2.

@nex3
Copy link
Member

nex3 commented Jul 29, 2015

This looks to be caused by dart-archive/dev_compiler#31.

@nex3 nex3 closed this as completed Jul 29, 2015
@jmesserly
Copy link

yeah still investigating. Type promotion does seem to be working in DDC. I think this our inference exposing the fact that type promotion doesn't work for the pattern if (x is! SomeType) return. But I need to double check this in relevant spec section.

You can see in the compareTo method above https://github.com/dart-lang/source_span/blob/master/lib/src/file.dart#L230-L242 how another local variable was used to workaround the issue.

@jmesserly
Copy link

maybe someone like @bwilkerson could confirm, but from reading the Dart spec, it seems that type promotion does not work in this case:

class T {}
class S extends T {
  someMethodOnS() { print('S'); }
}
someFunction(T t) {
  if (t is! S) return;

  t.someMethodOnS(); // warning in analyzer 
}
main() => someFunction(new S());

on VM it prints "S" but analyzer gives:

[warning] The method 'someMethodOnS' is not defined for the class 'T' (/Users/jmesserly/scratch/test.dart, line 8, col 5)

also CC @vsmenon, @leafpetersen ... we should certainly consider improving type promotion in DDC mode, though.

I wonder if propagatedType currently does anything smarter in analyzer.

@nex3
Copy link
Member

nex3 commented Jul 29, 2015

How is it that this code doesn't produce a warning in the analyzer without --strong?

@bwilkerson
Copy link
Member

... it seems that type promotion does not work in this case ...

Correct. The specification does not allow for promoting anything based on an is! test, nor does it promote anything after the then block. Specifying those cases was deemed too complex.

I wonder if propagatedType currently does anything smarter in analyzer.

Yes. The propagated type of t is S after the if statement. But analyzer isn't able to use that knowledge to remove the warning.

How is it that this code doesn't produce a warning in the analyzer without --strong?

What warning are you expecting (or what does strong mode report)?

My guess is that strong mode produces a warning because it infers a type in a place where the specification defines the type to be "dynamic". Because it replaces the specification's notion of the static type of an expression with a stronger version, it produces a warning as if you had explicitly declared the inferred type.

@jmesserly
Copy link

My guess is that strong mode produces a warning because it infers a type in a place where the specification defines the type to be "dynamic". Because it replaces the specification's notion of the static type of an expression with a stronger version, it produces a warning as if you had explicitly declared the inferred type.

yeah, @vsmenon and @leafpetersen and I just looked over this. We conclude that inference is the culprit. The code in that class is very interesting, contrast the compareTo method above, and the operator== method after: each tells a different story.

  • for compareTo, introducing "otherFile" was necessary to avoid a warning in analyzer (and --strong).
  • for operator==, because "other" is dynamic, no warning is produced by any of them (--strong-hints will warn about a dynamic dispatch there).
  • In union, a warning is not produced by analyzer because the type of beginSpan and endSpan are dynamic. But with --strong, because of inference, we conclude they are SourceSpan and issue a warning.

@jmesserly
Copy link

fyi, I opened dart-archive/dev_compiler#274 to improve type promotion in DDC mode, but I suspect it will be a while before we get there. Unless there's a clever way of hooking up this in strong mode:

Yes. The propagated type of t is S after the if statement. But analyzer isn't able to use that knowledge to remove the warning.

@bwilkerson off hand do you know what class computes that in analyzer? StaticTypeAnalyzer?

@bwilkerson
Copy link
Member

The logic is in ResolverVisitor.visitIfStatement.

nex3 referenced this issue in nex3/source_span Jul 2, 2024
@mosuem mosuem transferred this issue from dart-lang/source_span Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants