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

AssertionError in TypeAnalyzer #753

Closed
bwilkerson opened this issue Dec 8, 2011 · 8 comments
Closed

AssertionError in TypeAnalyzer #753

bwilkerson opened this issue Dec 8, 2011 · 8 comments
Labels
closed-obsolete Closed as the reported issue is no longer relevant

Comments

@bwilkerson
Copy link
Member

The following exception occurred while attempting to invoke code completion, so the source code probably had syntactic errors. Unfortunately we do not have the original source with which to reproduce the bug, but the type analyzer should not be throwing an exception when it cannot look up a property because the type of the object defining the property is not known.

The full context for this exception, if it's useful, is in http://code.google.com/p/dart/issues/detail?id=745.


!SUBENTRY 1 com.google.dart.tools.core 4 0 2011-12-08 00:28:46.693
!MESSAGE Failed to parse file:${user.home}/dart/Promote/Promote.dart
!STACK 0
java.lang.AssertionError: Internal error: unexpected kind DYNAMIC
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.internalError(TypeAnalyzer.java:223)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitPropertyAccess(TypeAnalyzer.java:1189)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitPropertyAccess(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartPropertyAccess.accept(DartPropertyAccess.java:82)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.typeOf(TypeAnalyzer.java:610)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitExprStmt(TypeAnalyzer.java:847)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitExprStmt(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartExprStmt.accept(DartExprStmt.java:36)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visit(TypeAnalyzer.java:1444)
       at com.google.dart.compiler.ast.DartBlock.visitChildren(DartBlock.java:44)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.typeAsVoid(TypeAnalyzer.java:654)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitBlock(TypeAnalyzer.java:650)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitBlock(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartBlock.accept(DartBlock.java:49)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.typeOf(TypeAnalyzer.java:610)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitFunction(TypeAnalyzer.java:905)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitFunction(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartFunction.accept(DartFunction.java:66)
       at com.google.dart.compiler.ast.DartMethodDefinition.visitChildren(DartMethodDefinition.java:85)
       at com.google.dart.compiler.ast.DartMethodDefinition$DartMethodWithInitializersDefinition.visitChildren(DartMethodDefinition.java:121)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.typeAsVoid(TypeAnalyzer.java:654)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitMethodDefinition(TypeAnalyzer.java:1008)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitMethodDefinition(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartMethodDefinition.accept(DartMethodDefinition.java:90)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visit(TypeAnalyzer.java:1444)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitClass(TypeAnalyzer.java:729)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitClass(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartClass.accept(DartClass.java:214)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visit(TypeAnalyzer.java:1444)
       at com.google.dart.compiler.ast.DartUnit.visitChildren(DartUnit.java:99)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.typeAsVoid(TypeAnalyzer.java:654)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitUnit(TypeAnalyzer.java:1325)
       at com.google.dart.compiler.type.TypeAnalyzer$Analyzer.visitUnit(TypeAnalyzer.java:1)
       at com.google.dart.compiler.ast.DartUnit.accept(DartUnit.java:107)
       at com.google.dart.compiler.type.TypeAnalyzer.exec(TypeAnalyzer.java:153)
       at com.google.dart.compiler.DeltaAnalyzer.analyze(DeltaAnalyzer.java:73)
       at com.google.dart.compiler.DartCompiler.analyzeDelta(DartCompiler.java:1240)
       at com.google.dart.tools.core.utilities.compiler.DartCompilerUtilities$DeltaAnalysisRunnable.run(DartCompilerUtilities.java:267)
       at com.google.dart.tools.core.utilities.compiler.DartCompilerUtilities$CompilerRunner.runSafe(DartCompilerUtilities.java:93)
       at com.google.dart.tools.core.utilities.compiler.DartCompilerUtilities.analyzeDelta(DartCompilerUtilities.java:482)

@scheglov
Copy link
Contributor

Unfortunately, I can not reproduce this without the original source.
I've tried several variants and none of them causes problem like this.
Reopen when you will have a test case.


Set owner to @scheglov.
Added WontFix label.

@bwilkerson
Copy link
Member Author

Please take another look at this issue. Unfortunately we will probably never have a test case unless we come up with one ourselves because the feedback mechanism is so careful to remove personal information that we cannot contact the original reporter. But this is a serious problem and we need to handle it. I would be happy to work with you to try to create a test case, but I don't know the internals of the compiler well enough to be able to do it on my own in a timely manner, so we need to work together.

After looking at the method in question it seems obvious to me (though I admit I'm not very familiar with the code and could easily be wrong) that we have a member in a type that is neither a constructor, method, or field. I'm not sure what the alternatives are, but whatever it is it has a type that prints as "DYNAMIC". Perhaps if you could tell me what kind of element(s) prints out as "DYNAMIC" we could find a test case that would reproduce this exception. (Keep in mind that the code could easily have been syntactically invalid. The might, for example, have been a missing '}' somewhere before this spot in the code that caused the compiler to add a member of an incorrect kind to the list of members.)

At the very least I would like to suggest two possible changes based on this bug report. First, if only constructors, methods and fields are valid members of types, then we could add a check at the point at which members get added to a type and catch the invalid condition earlier. That would, if nothing else, help us track down the ultimate cause of the bug the next time someone reports it.

Second, I would like to propose that this should not be considered an internal error. Clearly the code is not valid, but we should do our best to report errors (and this one was probably reported at the point at which the parser encountered the invalid syntax) and go on as if we hadn't found an error. There must be some kind of type that we can return here that will minimize the number of follow-on errors and allow us to finish building an AST structure. There's a good chance that the AST structure is complete enough at the point where the cursor is that we could provide code completion suggestions without needing this latter part of the code to also be correct.


Added Accepted label.

@scheglov
Copy link
Contributor

AFAIK, only DynamicElementImplementation return DYNAMIC from getKind().
I can of course convert this fatal exception into user-level error, but I can not test it.

Can we ask user if he allows to include his unit into error report?
Without this we are doomed to be in state like this - guess without and be not able to reproduce.

@bwilkerson
Copy link
Member Author

AFAIK, only DynamicElementImplementation return DYNAMIC from getKind().

So when does an element of that type get created? Looking at the call hierarchy, it looks like the most likely place is DynamicTypeImplementation.lookupMember(String), which implies to me that we have an object whose type we do not know. Perhaps something like:

  class Tester {
    var object;

    void method() {
      object.field = null;
    }
  }

(I tried this example and it didn't reproduce the bug, but it might be close.)

I can of course convert this fatal exception into user-level error, but I can not test it.

We should try to discover, first, whether it has already been reported, and if it hasn't already been reported we need to understand what to tell the user that would help them track down the bug in their code.

Can we ask user if he allows to include his unit into error report?

We have decided to fill in the entry field in the dialog with a template (similar or identical to the one used by the public bug database) that specifically prompts for steps to reproduce the problem. I'm hoping that will help with future reports.

@scheglov
Copy link
Contributor

To understand what to do we need to reproduce it first.
I've tried everything what I could, but still unsuccessfully.
I'm not going to continue to work on this until we will have a test case.

Just asking for steps is not enough.
Steps are always same: "I did don't remember what and it crashed".
Add unit by default and allow to opt-out by unchecking this option or something like this.
Just like we did in WindowBuilder - including every possible information allows to solve 80% problems with single report, without discussions with users and begging for a test case.


Removed the owner.
Added Triaged label.

@bwilkerson
Copy link
Member Author

Add unit by default and allow to opt-out by unchecking this option or something like this.
Just like we did in WindowBuilder - including every possible information allows to solve 80%
problems with single report, without discussions with users and begging for a test case.

I wish we could, but Google's policies on collecting user data prevent this approach.

That's why I'm asking you to add an assertion to the code at the point at which the invalid kind of member is being added to the class. If you do that then the exception we get will occur earlier and give us more information. We might have to repeat the process, but it's the only way I know of to track down the problem without a test case.

@scheglov
Copy link
Contributor

Can we don't add user unit automatically, but insistently recommend user to allow Editor include unit content into report? For example open question dialog (with default on button No) and say something like "Including source code will allow us to fix this problem significantly faster. Include source code? Yes/No"

Basically I can not reproduce most of the reports like this and I think that we could just stop receiving user error reports from Editor and will not loose much, but will get more time for handling better error reports.

@DartBot
Copy link

DartBot commented Apr 3, 2012

This comment was originally written by [email protected]


There is now a case stmt in this mehtod for DYNAMIC. Assuming fixed.


Added AssumedStale label.

@bwilkerson bwilkerson added Type-Defect closed-obsolete Closed as the reported issue is no longer relevant labels Apr 3, 2012
copybara-service bot pushed a commit that referenced this issue Sep 22, 2022
Re-submitting 259546 with updated golden tests.

https://dart.googlesource.com/protobuf/+log/cd0ff30759d8..1d175bef6043

2022-09-16 [email protected] Fix $_getMap return value when mutability when message is read-only (#754)
2022-09-16 [email protected] Eliminate more type casts when targeting JS (#732)
2022-09-15 [email protected] Make subBuilder args of BuilderInfo message methods required (#753)

Change-Id: I0447b0ba277251e49dc373941599039e0e041019
Tested: CL updates test golden expectations
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259740
Reviewed-by: Alexander Markov <[email protected]>
Reviewed-by: Devon Carew <[email protected]>
Commit-Queue: Ömer Ağacan <[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
closed-obsolete Closed as the reported issue is no longer relevant
Projects
None yet
Development

No branches or pull requests

3 participants