Skip to content

Fix #825 #834

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

Merged
merged 7 commits into from
Oct 22, 2015
Merged

Fix #825 #834

merged 7 commits into from
Oct 22, 2015

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 21, 2015

Fixes #825 as one of many well-formedness conditions. Review by @smarter.

@smarter
Copy link
Member

smarter commented Oct 21, 2015

Otherwise LGTM.

ctx.error("implicit classes may not be toplevel", cdef.pos)
Nil
}
else if (mods is Case) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our coding style is } else if ... without a line break after the }.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I write newline } quite systematically now. Makes keywords align better.

On Wed, Oct 21, 2015 at 6:43 PM, Guillaume Martres <[email protected]

wrote:

In src/dotty/tools/dotc/ast/Desugar.scala
#834 (comment):

@@ -380,18 +380,22 @@ object desugar {
// synthetic implicit C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, ..., pMN: TMN): C[Ts] =
// new C[Ts](p11, ..., p1N) ... (pM1, ..., pMN) =
val implicitWrappers =

  •  if (mods is Implicit) {
    
  •    if (ctx.owner is Package)
    
  •      ctx.error("implicit classes may not be toplevel", cdef.pos)
    
  •    if (mods is Case)
    

- ctx.error("implicit classes may not case classes", cdef.pos)

  •  if (!mods.is(Implicit))
    
  •    Nil
    
  •  else if (ctx.owner is Package) {
    
  •    ctx.error("implicit classes may not be toplevel", cdef.pos)
    
  •    Nil
    
  •  }
    
  •  else if (mods is Case) {
    

I think our coding style is } else if ... without a line break after the }
.


Reply to this email directly or view it on GitHub
https://github.com/lampepfl/dotty/pull/834/files#r42649286.

Martin Odersky
EPFL

Supporess creation of implicit factory if it would be illegal.
Avoids accidental combinations of term and type flags.
Need to survive even if a value class does not have an
underlying type. Also: better diagnostics if sigName
goes wrong.
Enforces various restrictions of definitions.
Use more complicated control flow with returns instead.
sigName is likely performance critical, so some
trickiness is justified.
It's done anyway later in FirstTransform.
@odersky
Copy link
Contributor Author

odersky commented Oct 22, 2015

Fixed reviewers's comments and rebased to master.

odersky added a commit that referenced this pull request Oct 22, 2015
@odersky odersky merged commit 6b7c51c into scala:master Oct 22, 2015
@allanrenucci allanrenucci deleted the fix-#825 branch December 14, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants