Skip to content

Incorrect handling of end <id> #12340

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
Glavo opened this issue May 5, 2021 · 7 comments
Closed

Incorrect handling of end <id> #12340

Glavo opened this issue May 5, 2021 · 7 comments

Comments

@Glavo
Copy link
Contributor

Glavo commented May 5, 2021

Compiler version

scala-3.0.1-RC1 (83acd4b)

Minimized code

This code shouldn't have been compiled, but it did:

class C {
}
end C

And automatic rewriting (-rewrite -no-indent) does not delete the end. For this code:

class C:
  val foo: Int = 0
end C

After compiling with scalac -rewrite -no-indent:

class C {
  val foo: Int = 0
}
end C
@Glavo Glavo added the itype:bug label May 5, 2021
@som-snytt
Copy link
Contributor

Braces and end markers are not mutually exclusive because:

Braces are not much better since a brace by itself also contains no information about what region is closed.

I don't have an opinion about behavior under -rewrite, but -endless would be a fun option. One could also pun about the "endless bikeshedding".

@tisonkun
Copy link

tisonkun commented May 6, 2021

Braces are not much better since a brace by itself also contains no information about what region is closed.

@som-snytt most of modern editor should be able to help you find the information about what region is closed, and fold it.

@noti0na1
Copy link
Member

noti0na1 commented May 6, 2021

@tisonkun It is difficult to balance how much information the source code carries and how much information the editor should display. Personally, I prefer using } over end when then the ending point is unclear.

@som-snytt
Copy link
Contributor

Sorry, I didn't cite the reference doc for end: https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html#the-end-marker

I understand the quote merely to imply that end is not intended as an alternative to brace, so I think the example ought to compile.

I agree there are varying preferences about the syntax. However, the greatest passions are reserved for comments, and while I am now a satisfied user of the end marker, I shall always recoil at the end comment:

class C {
  LOLOC()  // lots of lines of code
} // end C

It would be especially diabolical if the rewrite not only didn't remove the end marker, but put it behind a line comment after the brace.

@som-snytt
Copy link
Contributor

Actually the bug is that empty template is not patched:

class C:
end C

or equivalently, empty template with colon must be disallowed.

#12185 (comment)

Source must compile if (optional) end marker is deleted.

#12918 (comment)

@som-snytt
Copy link
Contributor

OK

package p {
}
end p

but not

package p:
end p

or

package p
end p

nor

package p
class C
end p

@som-snytt
Copy link
Contributor

This was fixed to make it a code comment in #12954 but PR was not linked.

I shall always recoil at the end comment

is obviously no longer the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants