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

Analyzer hasReferenceToSuper is not always computed before referenced #34113

Closed
leafpetersen opened this issue Aug 9, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@leafpetersen
Copy link
Member

commented Aug 9, 2018

Consider this code:

// test1.dart

abstract class Super {
  void foo();
}

abstract class Sub extends Super {
  void foo () {
    super.foo();
  }
}

and the following in another file:

import 'test1.dart';


abstract class C extends Super with Sub {}

void main() {

}

Without super mixins enabled, _checkForMixinInheritsNotFromObject in error_verifier.dart will report an error, but _checkForMixinReferencesSuper does not (even though the check is done).

With debug printing in place, I see that hasReferenceToSuper is checked on Sub in _checkForMixinReferencesSuper before Sub is visited, and is still false. Later, Sub is visited by the TypeResolver, and hasReferenceToSuper is set.

@leafpetersen leafpetersen changed the title Analyzer hadReferenceToSuper is not always computed before referenced Analyzer hasReferenceToSuper is not always computed before referenced Aug 9, 2018

@bwilkerson bwilkerson added this to the Dart2.1 milestone Aug 28, 2018

@scheglov scheglov added the p1-high label Sep 19, 2018

@scheglov scheglov self-assigned this Sep 19, 2018

@scheglov

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2018

Could you please clarify, which errors you expect when super mixins feature is: (1) enabled; (2) disabled? My current understanding of your comment is that you expect two errors when the feature is disabled; and zero when it is enabled. I tried to search the language specification PDF, but it does not talk anything about "super mixin".

@leafpetersen

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

If by the super mixins feature you mean the old class based super mixins, I have no expectations: it is being deleted. :)

For the new feature, the relevant code is to replace the definition of Sub above with the following:

mixin Sub on Super {
  void foo () {
    super.foo();
  }
}

If I do that and test it with a recent analyzer build, I get a crash. If I do that and test it with the CFE I get the following error (which is what I expect):

file:///Users/leafp/tmp/test1.dart:7:11: Error: Superclass has no method named 'foo'.
    super.foo();
@scheglov

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2018

OK, then I will stop looking at the original error, as I understand, it does not matter anymore.

What crash do you get? I see a problem that we don't report an error (and will fix it), but not a crash.

@leafpetersen

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

Haven't tried bleeding edge. This is the crash I see from a recent (yesterday?) build:

Unhandled exception:
NoSuchMethodError: The getter 'element' was called on null.
Receiver: null
Tried calling: element
#0      Object.noSuchMethod (dart:core/runtime/libobject_patch.dart:50:5)
#1      TypeSystem.gatherMixinSupertypeConstraints.addIfGeneric (package:analyzer/src/generated/type_system.dart:1764:16)
#2      TypeSystem.gatherMixinSupertypeConstraints (package:analyzer/src/generated/type_system.dart:1769:17)
#3      ErrorVerifier._checkMixinInference (package:analyzer/src/generated/error_verifier.dart:5818:27)
#4      ErrorVerifier._checkClassInheritance (package:analyzer/src/generated/error_verifier.dart:1357:7)
#5      ErrorVerifier.visitClassDeclaration (package:analyzer/src/generated/error_verifier.dart:504:9)
#6      ClassDeclarationImpl.accept (package:analyzer/src/dart/ast/ast.dart:1765:49)
#7      NodeListImpl.accept (package:analyzer/src/dart/ast/ast.dart:8304:20)
#8      CompilationUnitImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:2437:21)
#9      RecursiveAstVisitor.visitCompilationUnit (package:analyzer/dart/ast/visitor.dart:714:10)
#10     ErrorVerifier.visitCompilationUnit (package:analyzer/src/generated/error_verifier.dart:547:18)
#11     CompilationUnitImpl.accept (package:analyzer/src/dart/ast/ast.dart:2430:49)
#12     LibraryAnalyzer._computeVerifyErrors (package:analyzer/src/dart/analysis/library_analyzer.dart:328:10)
#13     LibraryAnalyzer._analyze.<anonymous closure>.<anonymous closure> (package:analyzer/src/dart/analysis/library_analyzer.dart:111:11)
#14     __InternalLinkedHashMap&_HashVMBase&MapMixin&_LinkedHashMapMixin.forEach (dart:collection/runtime/libcompact_hash.dart:370:8)
#15     LibraryAnalyzer._analyze.<anonymous closure> (package:analyzer/src/dart/analysis/library_analyzer.dart:110:15)
#16     _PerformanceTagImpl.makeCurrentWhile (package:analyzer/src/generated/utilities_general.dart:210:15)
#17     LibraryAnalyzer._analyze (package:analyzer/src/dart/analysis/library_analyzer.dart:109:36)
#18     LibraryAnalyzer.analyze.<anonymous closure> (package:analyzer/src/dart/analysis/library_analyzer.dart:78:14)
<asynchronous suspension>
#19     _PerformanceTagImpl.makeCurrentWhileAsync (package:analyzer/src/generated/utilities_general.dart:222:21)
<asynchronous suspension>
#20     LibraryAnalyzer.analyze (package:analyzer/src/dart/analysis/library_analyzer.dart:77:43)
<asynchronous suspension>
#21     AnalysisDriver._computeAnalysisResult.<anonymous closure> (package:analyzer/src/dart/analysis/driver.dart:1244:71)
<asynchronous suspension>
#22     PerformanceLog.runAsync (package:front_end/src/base/performance_logger.dart:52:21)
<asynchronous suspension>
#23     AnalysisDriver._computeAnalysisResult (package:analyzer/src/dart/analysis/driver.dart:1220:20)
<asynchronous suspension>
#24     AnalysisDriver.getErrors (package:analyzer/src/dart/analysis/driver.dart:634:43)
<asynchronous suspension>
#25     AnalyzerImpl.prepareErrors (package:analyzer_cli/src/analyzer_impl.dart:130:58)
<asynchronous suspension>
#26     AnalyzerImpl._analyze (package:analyzer_cli/src/analyzer_impl.dart:170:11)
<asynchronous suspension>
#27     AnalyzerImpl.analyze (package:analyzer_cli/src/analyzer_impl.dart:106:18)
<asynchronous suspension>
#28     Driver._runAnalyzer (package:analyzer_cli/src/driver.dart:748:21)
#29     Driver._analyzeAllImpl (package:analyzer_cli/src/driver.dart:405:40)
#30     _RootZone.runUnary (dart:async/zone.dart:1379:54)
#31     _FutureListener.handleValue (dart:async/future_impl.dart:129:18)
#32     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:642:45)
#33     Future._propagateToListeners (dart:async/future_impl.dart:671:32)
#34     Future._addListener.<anonymous closure> (dart:async/future_impl.dart:351:9)
#35     _microtaskLoop (dart:async/schedule_microtask.dart:41:21)
#36     _startMicrotaskLoop (dart:async/schedule_microtask.dart:50:5)
#37     _runPendingImmediateCallback (dart:isolate/runtime/libisolate_patch.dart:115:13)
#38     _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:172:5)

#0      AnalysisDriver._computeAnalysisResult.<anonymous closure> (package:analyzer/src/dart/analysis/driver.dart:1283:9)
<asynchronous suspension>
#1      PerformanceLog.runAsync (package:front_end/src/base/performance_logger.dart:52:21)
<asynchronous suspension>
#2      AnalysisDriver._computeAnalysisResult (package:analyzer/src/dart/analysis/driver.dart:1220:20)
<asynchronous suspension>
#3      AnalysisDriver.getErrors (package:analyzer/src/dart/analysis/driver.dart:634:43)
<asynchronous suspension>
#4      AnalyzerImpl.prepareErrors (package:analyzer_cli/src/analyzer_impl.dart:130:58)
<asynchronous suspension>
#5      AnalyzerImpl._analyze (package:analyzer_cli/src/analyzer_impl.dart:170:11)
<asynchronous suspension>
#6      AnalyzerImpl.analyze (package:analyzer_cli/src/analyzer_impl.dart:106:18)
<asynchronous suspension>
#7      Driver._runAnalyzer (package:analyzer_cli/src/driver.dart:748:21)
#8      Driver._analyzeAllImpl (package:analyzer_cli/src/driver.dart:405:40)
#9      _RootZone.runUnary (dart:async/zone.dart:1379:54)
#10     _FutureListener.handleValue (dart:async/future_impl.dart:129:18)
#11     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:642:45)
#12     Future._propagateToListeners (dart:async/future_impl.dart:671:32)
#13     Future._addListener.<anonymous closure> (dart:async/future_impl.dart:351:9)
#14     _microtaskLoop (dart:async/schedule_microtask.dart:41:21)
#15     _startMicrotaskLoop (dart:async/schedule_microtask.dart:50:5)
#16     _runPendingImmediateCallback (dart:isolate/runtime/libisolate_patch.dart:115:13)
#17     _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:172:5)
@scheglov

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2018

Ah, yes, Paul fixed this crash yesterday.

@scheglov

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2018

With https://dart-review.googlesource.com/c/sdk/+/75783 we will produce the requested error.

@scheglov scheglov closed this Sep 20, 2018

dart-bot pushed a commit that referenced this issue Sep 20, 2018

Fix checks for absence of concrete implementations of super-invoked m…
…embers.

R=brianwilkerson@google.com

Bug: #34113
Change-Id: Iad8ce7f2b7787a5c9640df617d99129032f8eee4
Reviewed-on: https://dart-review.googlesource.com/75783
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.