Skip to content

Analyzer: Still crashing on GenericFunctionType #30858

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

Closed
matanlurey opened this issue Sep 21, 2017 · 7 comments
Closed

Analyzer: Still crashing on GenericFunctionType #30858

matanlurey opened this issue Sep 21, 2017 · 7 comments
Assignees
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead.

Comments

@matanlurey
Copy link
Contributor

matanlurey commented Sep 21, 2017

final _disposeListeners = <void Function()>[];

Analyzer Feedback from IntelliJ

Version information

  • IDEA IU-171.4694.70
  • 1.25.0-dev.9.0
  • IU-171.4694.70, JRE 1.8.0_112-release-736-b21x64 JetBrains s.r.o, OS Linux(amd64) v4.4.0-87-generic unknown, screens 1920x1080, 1920x1080

Exception

Dart analysis server, SDK version 1.25.0-dev.9.0, server version 1.18.3, error: Analysis failed: .../lib/security.dart
Invalid argument(s): Cannot serialize an instance of GenericFunctionTypeImpl
#0      AbstractConstExprSerializer.serializeTypeName (package:analyzer/src/summary/summarize_const_expr.dart:211)
#1      AbstractConstExprSerializer._serializeListLiteral (package:analyzer/src/summary/summarize_const_expr.dart:551)
#2      AbstractConstExprSerializer._serialize (package:analyzer/src/summary/summarize_const_expr.dart:357)
#3      AbstractConstExprSerializer.serialize (package:analyzer/src/summary/summarize_const_expr.dart:139)
#4      _SummarizeAstVisitor.serializeConstExpr (package:analyzer/src/summary/summarize_ast.dart:551)
#5      _SummarizeAstVisitor.serializeFunctionBody (package:analyzer/src/summary/summarize_ast.dart:715)
#6      _SummarizeAstVisitor.serializeInitializerFunction (package:analyzer/src/summary/summarize_ast.dart:784)
#7      _SummarizeAstVisitor.serializeVariables (package:analyzer/src/summary/summarize_ast.dart:984)
#8      _SummarizeAstVisitor.visitFieldDeclaration (package:analyzer/src/summary/summarize_ast.dart:1148)
#9      FieldDeclarationImpl.accept (package:analyzer/src/dart/ast/ast.dart:4364)
#10     _SummarizeAstVisitor.serializeClass (package:analyzer/src/summary/summarize_ast.dart:466)
#11     _SummarizeAstVisitor.visitClassDeclaration (package:analyzer/src/summary/summarize_ast.dart:1014)
@vsmenon vsmenon added legacy-area-analyzer Use area-devexp instead. S1 high devexp-server Issues related to some aspect of the analysis server labels Sep 25, 2017
@MichaelRFairhurst MichaelRFairhurst self-assigned this Nov 9, 2017
@MichaelRFairhurst
Copy link
Contributor

I had a fix for this quite a long while ago, but my fix caused a secondary issue. It's a pretty deep issue related to synthetic generic element types which I've not had much luck with. Latest solution involved turning off an assertion and is up for review: https://dart-review.googlesource.com/c/sdk/+/20481 but it may not be a viable solution. We'll have to wait for an analyzer member with more familiarity to decide.

@jmesserly
Copy link

RE: synthetic generic element types, here are some possibly related bugs: #30379 and #29778 (comment)

If I recall, synthetic function types are not hooked up to their enclosingElement scope, so FunctionTypeImpl cannot determine which type parameters belong to the function type and which ones belong to enclosing scopes. This means some operations do not work correctly on them (such as equality, instantiation, substitution).

@whesse whesse closed this as completed in 60ee2c0 Jan 9, 2018
@MichaelRFairhurst
Copy link
Contributor

Fix landed in https://dart-review.googlesource.com/c/sdk/+/20481. However, it has some limitations. I'll make new tickets regarding those limitations.

@MichaelRFairhurst
Copy link
Contributor

#31804 is that new ticket.

whesse pushed a commit that referenced this issue Jan 9, 2018
This reverts commit 60ee2c0.

Reason for revert: Requires https://dart-review.googlesource.com/c/sdk/+/33180 which can land first.

Original change's description:
> Fix #30858, generic function types not serializable when const.
> 
> Previously declaring for instance const lists (or even final lists) of
> generic function types ie `final x = <void Function()>[]` would throw
> errors during summarization.
> 
> The type parameters don't seem correctly stored yet, but that is an
> edge case. Left a TODO, for now this should go in to prevent the
> analyzer crash.
> 
> One thing I changed as well is that `serializeType` assumed it was
> getting a type name, and `serializeTypeName` accepted types but threw
> when it didn't get a type name. I flipped the names so that
> `serializeTypeName` accepts a type name and `serializeType` accepts a
> type in general and decides whether its a type name or a generic
> function type to proceed with specialization.
> 
> Bug: 30858
> Change-Id: Id128f8625cbf03bb94d05ff0efdbac3b158e637e
> Reviewed-on: https://dart-review.googlesource.com/20481
> Reviewed-by: Paul Berry <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I6fae28335f7815b1e8c43597fb9451519a13335e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 30858
Reviewed-on: https://dart-review.googlesource.com/33200
Reviewed-by: Mike Fairhurst <[email protected]>
Commit-Queue: Mike Fairhurst <[email protected]>
@MichaelRFairhurst
Copy link
Contributor

This was reverted, I currently think https://dart-review.googlesource.com/c/sdk/+/33660 is its best chance at being solved, though it won't handle

final _disposeListeners = <void Function<T>()>[];

@MichaelRFairhurst
Copy link
Contributor

Another quick update, looks like this temp solution is better than I'd realized.

final _disposeListeners = <void Function<T>()>[];

works. You have to go so far as

final _disposeListeners = <void Function<T>(Function<G>(G, T))>[];

to get a crash.

Assuming that fix actually lands...

@whesse whesse closed this as completed in 4c1cbe1 Jan 10, 2018
@MichaelRFairhurst
Copy link
Contributor

Landed, no commit bot issues, looks like this is now ready to be put to bed at least for a while longer :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead.
Projects
None yet
Development

No branches or pull requests

4 participants