search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.08k stars 1.56k forks source link

Crash with invalid named mixin application #48919

Open scheglov opened 2 years ago

scheglov commented 2 years ago 
mixin M {}
class A = void Function() with M;

main() {}

I see from dart run:

Crash when compiling file:///Users/scheglov/tmp/333/test.dart,
at character offset null:
type 'Null' is not a subtype of type 'String'
#0      extractName (package:front_end/src/fasta/source/source_library_builder.dart:5243:51)
#1      SourceLibraryBuilder.applyMixins (package:front_end/src/fasta/source/source_library_builder.dart:2147:28)
#2      SourceLibraryBuilder.addNamedMixinApplication (package:front_end/src/fasta/source/source_library_builder.dart:2352:17)
#3      OutlineBuilder.endNamedMixinApplication (package:front_end/src/fasta/source/outline_builder.dart:2105:22)
#4      Parser.parseNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2223:14)
#5      Parser.parseClassOrNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2202:14)
#6      Parser.parseTopLevelKeywordDeclaration (package:_fe_analyzer_shared/src/parser/parser_impl.dart:564:14)
#7      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:497:14)
#8      Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:377:15)
#9      SourceLoader.buildOutline (package:front_end/src/fasta/source/source_loader.dart:1114:37)
<asynchronous suspension>
#10     SourceLoader.buildOutlines (package:front_end/src/fasta/source/source_loader.dart:1004:7)
<asynchronous suspension>
#11     KernelTarget.computeNeededPrecompilations.<anonymous closure> (package:front_end/src/fasta/kernel/kernel_target.dart:364:7)
<asynchronous suspension>
#12     withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#13     generateKernelInternal.<anonymous closure> (package:front_end/src/kernel_generator_impl.dart:99:11)
<asynchronous suspension>
#14     withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#15     generateKernel.<anonymous closure> (package:front_end/src/kernel_generator_impl.dart:47:12)
<asynchronous suspension>
#16     generateKernel (package:front_end/src/kernel_generator_impl.dart:46:10)
<asynchronous suspension>
#17     kernelForModule (package:front_end/src/api_prototype/kernel_generator.dart:100:11)
<asynchronous suspension>
#18     SingleShotCompilerWrapper.compileInternal (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:400:11)
<asynchronous suspension>
#19     Compiler.compile.<anonymous closure> (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:216:45)
<asynchronous suspension>
#20     _processLoadRequest (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:894:37)
<asynchronous suspension>

#0      extractName (package:front_end/src/fasta/source/source_library_builder.dart:5243:51)
#1      SourceLibraryBuilder.applyMixins (package:front_end/src/fasta/source/source_library_builder.dart:2147:28)
#2      SourceLibraryBuilder.addNamedMixinApplication (package:front_end/src/fasta/source/source_library_builder.dart:2352:17)
#3      OutlineBuilder.endNamedMixinApplication (package:front_end/src/fasta/source/outline_builder.dart:2105:22)
#4      Parser.parseNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2223:14)
#5      Parser.parseClassOrNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2202:14)
#6      Parser.parseTopLevelKeywordDeclaration (package:_fe_analyzer_shared/src/parser/parser_impl.dart:564:14)
#7      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:497:14)
#8      Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:377:15)
#9      SourceLoader.buildOutline (package:front_end/src/fasta/source/source_loader.dart:1114:37)
<asynchronous suspension>
#10     SourceLoader.buildOutlines (package:front_end/src/fasta/source/source_loader.dart:1004:7)
<asynchronous suspension>
#11     KernelTarget.computeNeededPrecompilations.<anonymous closure> (package:front_end/src/fasta/kernel/kernel_target.dart:364:7)
<asynchronous suspension>
#12     withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#13     generateKernelInternal.<anonymous closure> (package:front_end/src/kernel_generator_impl.dart:99:11)
<asynchronous suspension>
#14     withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#15     generateKernel.<anonymous closure> (package:front_end/src/kernel_generator_impl.dart:47:12)
<asynchronous suspension>
#16     generateKernel (package:front_end/src/kernel_generator_impl.dart:46:10)
<asynchronous suspension>
#17     kernelForModule (package:front_end/src/api_prototype/kernel_generator.dart:100:11)
<asynchronous suspension>
#18     SingleShotCompilerWrapper.compileInternal (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:400:11)
<asynchronous suspension>
#19     Compiler.compile.<anonymous closure> (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:216:45)
<asynchronous suspension>
#20     _processLoadRequest (org-dartlang-kernel-service:///pkg/vm/bin/kernel_service.dart:894:37)
<asynchronous suspension>
scheglov commented 2 years ago

Similar code causes a crash in the analyzer AstBuilder (lines might be slightly different):

  type 'GenericFunctionTypeImpl' is not a subtype of type 'NamedType' in type cast
  #0      AstBuilder.endNamedMixinApplication (package:analyzer/src/fasta/ast_builder.dart:2146:26)
  #1      Parser.parseNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2223:14)
  #2      Parser.parseClassOrNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2202:14)
  #3      Parser.parseTopLevelKeywordDeclaration (package:_fe_analyzer_shared/src/parser/parser_impl.dart:564:14)
  #4      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:497:14)
  #5      Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:377:15)
  #6      Parser.parseCompilationUnit2 (package:analyzer/src/generated/parser.dart:108:32)
  #7      Parser.parseCompilationUnit (package:analyzer/src/generated/parser.dart:104:12)
  #8      FileState._parse (package:analyzer/src/dart/analysis/file_state.dart:550:23)

Which raises a question - does the parser know whether it is a function type or a named type? If it does, it would be nice to report an error and recover.

scheglov commented 2 years ago

And similar issue with extends void Function(), and least in the analyzer.

  type 'GenericFunctionTypeImpl' is not a subtype of type 'NamedType?' in type cast
  #0      AstBuilder.handleClassExtends (package:analyzer/src/fasta/ast_builder.dart:2725:27)
  #1      Parser.parseClassExtendsSeenExtendsClause (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2391:14)
  #2      Parser.parseClassExtendsOpt (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2362:15)
  #3      Parser.parseClassHeaderOpt (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2255:13)
  #4      Parser.parseClass (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2242:13)
  #5      Parser.parseClassOrNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2206:14)
  #6      Parser.parseTopLevelKeywordDeclaration (package:_fe_analyzer_shared/src/parser/parser_impl.dart:564:14)
  #7      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:497:14)
  #8      Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:377:15)
  #9      Parser.parseCompilationUnit2 (package:analyzer/src/generated/parser.dart:108:32)
  #10     Parser.parseCompilationUnit (package:analyzer/src/generated/parser.dart:104:12)
  #11     FileState._parse (package:analyzer/src/dart/analysis/file_state.dart:550:23)
scheglov commented 2 years ago

...and for with clause.

  type 'GenericFunctionTypeImpl' is not a subtype of type 'NamedType' in type cast
  #0      StackImpl.popList (package:_fe_analyzer_shared/src/parser/stack_listener.dart:597:25)
  #1      AstBuilder.popTypedList (package:analyzer/src/fasta/ast_builder.dart:4140:11)
  #2      AstBuilder.endTypeList (package:analyzer/src/fasta/ast_builder.dart:2505:10)
  #3      Parser.parseTypeList (package:_fe_analyzer_shared/src/parser/parser_impl.dart:975:14)
  #4      Parser.parseClassWithClauseOpt (package:_fe_analyzer_shared/src/parser/parser_impl.dart:1270:15)
  #5      Parser.parseClassHeaderOpt (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2256:13)
  #6      Parser.parseClass (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2242:13)
  #7      Parser.parseClassOrNamedMixinApplication (package:_fe_analyzer_shared/src/parser/parser_impl.dart:2206:14)
  #8      Parser.parseTopLevelKeywordDeclaration (package:_fe_analyzer_shared/src/parser/parser_impl.dart:564:14)
  #9      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:497:14)
  #10     Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:377:15)
  #11     Parser.parseCompilationUnit2 (package:analyzer/src/generated/parser.dart:108:32)
  #12     Parser.parseCompilationUnit (package:analyzer/src/generated/parser.dart:104:12)
  #13     FileState._parse (package:analyzer/src/dart/analysis/file_state.dart:550:23)
jensjoha commented 2 years ago

@johnniwinther why was I assigned to this?

I initially thought it was some sort of parsing error, but the parser seems to conform to the spec to me, e.g.

<classDeclaration> ::=
  \ABSTRACT? \CLASS{} <identifier> <typeParameters>?
  \gnewline{} <superclass>? <interfaces>?
  \gnewline{} `{' (<metadata> <classMemberDeclaration>)* `}'
  \alt \ABSTRACT? \CLASS{} <mixinApplicationClass>

<mixinApplicationClass> ::= \gnewline{}
  <identifier> <typeParameters>? `=' <mixinApplication> `;'

<mixinApplication> ::= <typeNotVoid> <mixins> <interfaces>?

<interfaces> ::= \IMPLEMENTS{} <typeNotVoidList>

<superclass> ::= \EXTENDS{} <typeNotVoid> <mixins>?
    \alt <mixins>

<mixins> ::= \WITH{} <typeNotVoidList>

<typeNotVoidList> ::= <typeNotVoid> (`,' <typeNotVoid>)*

<typeNotVoid> ::= <functionType> `?'?
  \alt <typeNotVoidNotFunction>

<functionType> ::= <functionTypeTails>
  \alt <typeNotFunction> <functionTypeTails>

<functionTypeTails> ::= <functionTypeTail> `?'? <functionTypeTails>
  \alt <functionTypeTail>

<functionTypeTail> ::= \FUNCTION{} <typeParameters>? <parameterTypeList>

<typeNotFunction> ::= \VOID{}
  \alt <typeNotVoidNotFunction>

As I read it all of these should be OK:

mixin M {}
class X {}

class A = void Function() with M;
class B = void Function() with void Function();
class C = void Function() with void Function() implements void Function();
class D extends X with void Function() {}
class E extends void Function() {}

main() {}
johnniwinther commented 2 years ago

This sounds like this is an issue in both the CFE and analyzer. Reassigning.

scheglov commented 2 years ago

@eernstg Is this correct? Do we allow function types as superclasses? Just syntactically or also semantically?

It looks that 10.9 forbids using type aliases that are not classes, but nothing about function types written directly.

image
eernstg commented 2 years ago

It is definitely intended that a function type as a superclass should be a compile-time error. The change which was made in order to ensure that it has no effect if a class implements Function (and otherwise uses Function as a superinterface) was intended to ensure that no expression whose static type is Function or a function type would ever yield any other kind of object than a function object (so an object can't pretend to be a function object). This means that it is possible to use a faster approach to invoke function objects than the approach where it is done as a virtual method invocation of call.

The PR where null safety is integrated into the language specification is ongoing work (a snippet of it is available at https://github.com/dart-lang/language/pull/2023), but it contains a short section defining 'class building types'. That section corrects a couple of mistakes and updates the definitions to take null safety into account, because it specifies that void is not a class building type, T? is not a class building type for any T, and no function type is a class building type.

The concept of being a class building type is then used in the sections about extends, with and implements, including the location you mention.

This means that function types aren't mentioned today, but that's clearly a mistake, and it is being corrected when the null safety updates are landed (reviews are ongoing, we will get there ;-).

chloestefantsova commented 2 years ago

The fix for the CFE has landed: https://dart-review.googlesource.com/c/sdk/+/244240.

scheglov commented 2 years ago

If this is a compile-time error to use a function type as a superclass, then should the parser report this error instead? When we see that this is an explicit function type, not a type alias, of course.

bwilkerson commented 2 years ago

The grammar currently includes the following:

<superclass> ::= extends <typeNotVoid> <mixins>?
    | <mixins>
<typeNotVoid> ::= <functionType> ‘?’?
    | <typeNotVoidNotFunction>

While it would make sense to me for the grammar to exclude function types in the extends clause (@eernstg), it currently doesn't. Because the grammar allows it, IMO it shouldn't be a parse error.

scheglov commented 2 years ago

My understanding was that it is an oversight that the grammar does not exclude function types. And because we know this, it does not seem right to hold to the letter of the current grammar and insist that CFE and analyzer should do their own error reporting for invalid syntax here.

I did this initially in the analyzer, but thought that it might be beneficial to check syntax in one place - in the parser.

eernstg commented 2 years ago

I think it would be complicating matters (slightly) for no good reason if we were to make it a syntax error: As @scheglov mentioned, it would still be possible to use a function type as a superinterface via a type alias, so we would still have to check syntactic forms that aren't a <functionType>. A syntax error would just catch that same error a bit earlier.

So unless there are grammatical reasons for doing it (parser speed? disambiguation?), I think we shouldn't build double-walls against dangers when one wall will do just fine. ;-)

renatoathaydes commented 1 year ago

I saw this issue on emacs so I had reported it on the lsp project: https://github.com/emacs-lsp/lsp-dart/issues/191

Just declaring a mixin caused the LSP to keep dying all the time.

renatoathaydes commented 1 year ago

Maybe this is related: The Dart compiler doesn't seem to auto-cast this:

mixin Foo {
  void bar();
}

class FooFunction with Foo {
  void call() {}
  void bar() {}
}

main() {
  Function() fun = FooFunction();

  if (fun is Foo) {
    fun.bar();
  }
}

Fails with:

Error compiling to JavaScript:
Info: Compiling with sound null safety
lib/main.dart:14:9:
Error: The method 'bar' isn't defined for the class 'dynamic Function()'.
    fun.bar();
        ^^^
Error: Compilation failed.

An explicit cast is needed:

  if (fun is Foo) {
    (fun as Foo).bar();
  }

So there's something funky about functions and mixins?!