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

Change indentation rules to allow copy-paste #7114

merged 11 commits into from Aug 30, 2019


Copy link

commented Aug 27, 2019

Change significant indentation rules so that indented code can appear inside braces.

@odersky odersky force-pushed the dotty-staging:change-indent branch 3 times, most recently from 0605327 to dab226c Aug 27, 2019

@odersky odersky marked this pull request as ready for review Aug 28, 2019

@odersky odersky force-pushed the dotty-staging:change-indent branch 6 times, most recently from 3dc3ccb to 956e99e Aug 28, 2019

@odersky odersky force-pushed the dotty-staging:change-indent branch 2 times, most recently from 40b550c to c802c4e Aug 29, 2019


This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

To bring this over the finish line, we still need to do two things

  • fix the error message diagnostics (the messages look OK, the tests need to be updated, but
    their architecture does not make it easy).
  • fix the community build so that dotc is invoked with option noindent.

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

The following fixes had to be done to the community build to make this pass:

  • compile ScalaTest and SemanticDB with -noindent. ScalaTest had multiple errors with significant indentation on, SemanticDB only one.
  • comment out a line in FasterParserParser.scala, which is a fastparse test. The change in #7130 broke the type inference at that line. We should follow up to see how to fix this. I opened an issue in #7133.
odersky added 5 commits Aug 29, 2019
Make compiler sources invariant under rewriting
Also: fix indentation in Implicits
Enable significant indentation inside braces.

 - Make indentation opt in, controlled by -indent.
   indent is set as a default option when testing.
 - Allow rewritings under constructs that could not be rewritten
 - More systematic checking of conflicting rewrite options
 - Don't insert an <outdent> where a statement is logically continued

The last item fixes a case observed in SJSCodeGen, which was basically like this:

    if (x) {
    } else /*
    } else */ {
The problem is that the commented out `}` in front of the uncommented `{` caused
an <outdent> to be inserte between `else` and `{`.
Don't interpolate downwards to a bottom type
If the result of a covariant type variable interpolation would be a bottom type,
wait instead. This could make the variable be inferred to its upper bound after all,
if we do not need a fully instantiated type right away.

This makes a difference for indentation whereever we have a situation like this:
  def foo: T =
With indentation on, this is now equivalent to
  def foo: T = {
This means there is now an added opportunity for an interpolation step, of
the asInstanceOf, which would go downwards to Nothing. So indentation changes
the inferred type, which is very bad. With the commit the problem is

Two tests are reclassified:

i536.scala previously compiled since the type argument was inferred to be Nothing.
But that inference is useless; it just hides a runtime failure. The issue #536
only complained that the compiler crashed, so having a negative outcome
is permissible.

enum-interop is similar except that the example makes even less sense and compiles only
because Nothing was inferred by accedient.
Copy link

left a comment

This still breaks on -rewrite -noindent and no newline on the end. What I am doing is building a vanilla sbt project with version 0.18.1-bin-SNAPSHOT after running community-build/prepareCommunityBuild

object ifexample:
  if true then

@odersky odersky force-pushed the dotty-staging:change-indent branch from 70401ce to fe3562d Aug 29, 2019

Don't always enclose partial ofs in braces
This needs to be done only if the partial if is followed by `else`.

Change all dotc code to be stable under the new rewrite scheme.

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

@bishabosha I found it. I'll add a fix.

Fix crash when trying to insert braces at EOF
This fixes a crash when trying to compile pos/indent2.scala with -rewrite -noindent.
Ideally we should have a test for this. However, I am not sure whether our current
test infrastructure supports rewrite tests.

@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 29, 2019

@odersky odersky merged commit 96b0854 into lampepfl:master Aug 30, 2019

2 checks passed

CLA User signed CLA
continuous-integration/drone/pr Build is passing

@odersky odersky deleted the dotty-staging:change-indent branch Aug 30, 2019

@anatoliykmetyuk anatoliykmetyuk referenced this pull request Aug 30, 2019
37 of 42 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
3 participants
You can’t perform that action at this time.