Change indentation rules to allow copy-paste #7114
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
odersky
force-pushed
the
change-indent
branch
3 times, most recently
from
0605327 to
dab226c
Compare
odersky
force-pushed
the
change-indent
branch
6 times, most recently
from
3dc3ccb to
956e99e
Compare
odersky
force-pushed
the
change-indent
branch
2 times, most recently
from
40b550c to
c802c4e
Compare
|
To bring this over the finish line, we still need to do two things
|
|
The following fixes had to be done to the community build to make this pass:
|
Also: fix indentation in Implicits
Also:
- 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:
```scala
if (x) {
...
} else /*
bla
bla
} else */ {
```
The problem is that the commented out `}` in front of the uncommented `{` caused
an <outdent> to be inserte between `else` and `{`.
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 =
bla.asInstanceOf
```
With indentation on, this is now equivalent to
```
def foo: T = {
bla.asInstanceof
}
```
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
avoided.
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 scala#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.
bishabosha
requested changes
Aug 29, 2019
There was a problem hiding this 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
println
println
else
println
printlnThis 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.
|
@bishabosha I found it. I'll add a fix. |
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.
bishabosha
approved these changes
Aug 29, 2019
42 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change significant indentation rules so that indented code can appear inside braces.