Fix more problems detected by fuzzing in #4389 #4411

odersky · 2018-04-29T12:19:44Z

Fix more problems detected by fuzzing in #4389

smarter · 2018-04-29T12:29:59Z

compiler/src/dotty/tools/dotc/ast/Trees.scala

            }
+            recur(buf, remaining)


at this point remaining = Thicket(elems) :: remaining1 so isn't this call equivalent to the one in the else branch below?

Indeed it is. Scope for simplification, then.

smarter · 2018-04-29T12:40:49Z

compiler/src/dotty/tools/dotc/typer/Typer.scala

+  def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context): Tree = track("typedClassDef") {
+    if (!cls.info.isInstanceOf[ClassInfo]) {
+      assert(ctx.reporter.errorsReported)
+      return cdef.withType(UnspecifiedErrorType)


I think ideally we should have a way to recursively set the type of the tree children to maintain the invariant that a typed tree only contains typed trees.

Good point, yes.

Actually, that's not so easy. Sometimes, there's simply no way to proceed. And we cannot just assign an ErrorType, because a lot of trees are not typed trees, so we'd have to replace them with something else.

I guess they could be replaced by ErrorTrees.

In fact, maybe cdef should just be replaced by an ErrorTree since it's unusable anyway.

OK, yes. Let's do that.

Hmm wait. What do you mean by ErrorTree? We only have errorTree which gives an error type to an existing tree.

OK, we can replace it with an EmptyTree instead.

`checkedParentType` tests that the underlying class symbol of a type is a class. So `computeBaseClasses` should assume exactly the same property.

odersky · 2018-04-29T13:15:23Z

Rebased to master

Also: fix a cyclic reference caused by 5252b15

- Handle the case where Thickets can contain themselves Thickets - Make it tail recursive instead of using an outer while loop

In the face of previous errors: - Constant#classBound cannot assume all class parents have good types - dominator cannot assume that all classes in an OrType inherit from the same parent

These are sometimes detected after they are computed. Need to avoid a crash in AndType due to an operand that is not a value type.

smarter · 2018-04-29T14:34:02Z

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -3478,7 +3478,7 @@ object Types {
      if (selfTypeCache == null)
        selfTypeCache = {
          val given = cls.givenSelfType
-          if (!given.exists) appliedRef
+          if (!given.isValueType) appliedRef


Might be worth explicitly checking that an error happened like other commits in this PR do, e.g.:

if (!given.isValueType) { assert(!given.exists || ctx.reporter.errorsReported) appliedRef }

The given type might be higher-kinded (in fact that's what it was in the test) but this would be detected only later.

OK, in that case maybe just add a comment explaining that non-value types self can only happen with erroneous code? Otherwise readers of this code might be puzzled.

smarter · 2018-04-29T14:35:46Z

compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala

  /** The result type of `mt`, where all references to parameters of `mt` are
   *  replaced by either wildcards (if typevarsMissContext) or TypeParamRefs.
   */
  def resultTypeApprox(mt: MethodType)(implicit ctx: Context): Type =
    if (mt.isResultDependent) {
      def replacement(tp: Type) =
-        if (ctx.mode.is(Mode.TypevarsMissContext)) WildcardType else newDepTypeVar(tp)
+        if (ctx.mode.is(Mode.TypevarsMissContext) ||
+            !tp.widenExpr.isValueTypeOrWildcard) WildcardType


Here too, the error recovery case could be more explicit:

if (!tp.widenExpr.isValueTypeOrWildcard) { assert(ctx.reporter.errorsReported) WildcardType } else if (ctx.mode.is(Mode.TypevarsMissContext)) WildcardType else ...

Same response: I am not sure the error would always be detected beforehand.

In the tested case, an unApply ended up with an inserted toplevel apply method, which confused the follow-on logic. This case is now rejected.

Parent types can be aliases, so .typeSymbol can be wrong.

smarter · 2018-04-29T16:10:13Z

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

-      		// A test case is CompilationTests.compileMixed
-      	pcls match {
+      for (p <- classParents)
+       	p.classSymbol match {


incorrect indentation.

odersky · 2018-04-29T16:17:52Z

So that's all bugs in #4389, except those that contain a _ in a wrong place.

smarter · 2018-04-29T16:46:32Z

compiler/src/dotty/tools/dotc/ast/Desugar.scala

-              if (ctx.reporter.errorsReported) ErrorType(msg)
-              else throw new java.lang.Error(msg)
-            }
+			  ErrorType(msg).assertingErrorsReported(msg)


Wrong indentation.

smarter reviewed Apr 29, 2018

View reviewed changes

odersky added 2 commits April 29, 2018 15:14

Give up typechecking class definitions if basic type info is missing

09eb5f7

Align computeBaseClasses with checkedParentType

77f75b7

`checkedParentType` tests that the underlying class symbol of a type is a class. So `computeBaseClasses` should assume exactly the same property.

odersky force-pushed the fix-fuzzing-2 branch from c6e1820 to c66c50e Compare April 29, 2018 13:15

smarter approved these changes Apr 29, 2018

View reviewed changes

odersky added 6 commits April 29, 2018 15:47

Survive bad symbols of module vals

297c216

Also: fix a cyclic reference caused by 5252b15

Rework Trees.flatten

5597dd0

- Handle the case where Thickets can contain themselves Thickets - Make it tail recursive instead of using an outer while loop

Survive ill-formed parent types

de58653

In the face of previous errors: - Constant#classBound cannot assume all class parents have good types - dominator cannot assume that all classes in an OrType inherit from the same parent

Handle refinements where the refined type is a class

a6ba33b

Surivive bad selftypes

64e36f1

These are sometimes detected after they are computed. Need to avoid a crash in AndType due to an operand that is not a value type.

Harden resultTypeApprox

f7c46af

odersky force-pushed the fix-fuzzing-2 branch from c66c50e to f7c46af Compare April 29, 2018 14:19

smarter reviewed Apr 29, 2018

View reviewed changes

odersky added 3 commits April 29, 2018 17:17

Harden typedUnaply

f8f5176

In the tested case, an unApply ended up with an inserted toplevel apply method, which confused the follow-on logic. This case is now rejected.

Be careful to always use .classSymbol to retrieve a parent symbol

5ad42d1

Parent types can be aliases, so .typeSymbol can be wrong.

Harden typedPackageDef

c3bd95a

smarter reviewed Apr 29, 2018

View reviewed changes

Fix indentation

5f9f3a7

Streamline "assert errors reported" logic

690321d

smarter reviewed Apr 29, 2018

View reviewed changes

odersky merged commit 75b66c8 into scala:master Apr 30, 2018

allanrenucci deleted the fix-fuzzing-2 branch April 30, 2018 18:49

Fix more problems detected by fuzzing in #4389 #4411

Fix more problems detected by fuzzing in #4389 #4411

Uh oh!

Conversation

odersky commented Apr 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

odersky Apr 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

odersky commented Apr 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter Apr 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

odersky commented Apr 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

odersky Apr 29, 2018 •

edited

Loading

smarter Apr 29, 2018 •

edited

Loading