Skip to content

Commit cc88aa5

Browse files
committed
fields phase
the info-transformed of constructors: - traits receive trait-setters for vals - classes receive fields & accessors for mixed in traits Constructors tree transformer - makes trees for decls added during above info transform - adds mixin super calls to the primary constructor ``` trait OneConcreteVal[T] { var x = 1 // : T = ??? def foo = x } trait OneOtherConcreteVal[T] { var y: T = ??? } class C extends OneConcreteVal[Int] with OneOtherConcreteVal[String] ``` we don't have a field -- only a getter -- so, where will we keep non-getter but-field annotations? mixin only deals with lazy accessors/vals do not used referenced to correlate getter/setter it messes with paramaccessor's usage, and we don't really need it make sure to clone info's, so we don't share symbols for method args this manifests itself as an exception in lambdalift, finding proxies Use NEEDS_TREES for all comms between InfoTransform and tree transform yep, need SYNTHESIZE_IMPL_IN_SUBCLASS distinguish accessors that should be mixed into subclass, and those that simply need to be implemented in tree transform, after info transform added the decl commit 4b4932e Author: Adriaan Moors <[email protected]> Date: 6 days ago do assignment to trait fields in AddInterfaces regular class vals get assignments during constructors, as before impl classes get accessors + assignments through trait setters in addinterfaces so that constructors acts on them there, and produced the init method in the required spot (the impl class) bootstrapped compiler needs new partest commit baf568d Author: Adriaan Moors <[email protected]> Date: 3 weeks ago produce identical bytecode for constant trait val getters I couldn't bring myself to emit the unused fields that we used to emit for constant vals, even though the getters immediately return the constant, and thus the field goes unused. In the next version, there's no need to synthesize impls for these in subclasses -- the getter can be implemented in the interface. commit b9052da Author: Lukas Rytz <[email protected]> Date: 3 weeks ago Fix enclosing method attribute for classes nested in trait fields Trait fields are now created as MethodSymbol (no longer TermSymbol). This symbol shows up in the `originalOwner` chain of a class declared within the field initializer. This promoted the field getter to being the enclosing method of the nested class, which it is not (the EnclosingMethod attribute is a source-level property). commit cf845ab Author: Adriaan Moors <[email protected]> Date: 3 weeks ago don't suppress field for unit-typed vals it affects the memory model -- even a write of unit to a field is relevant... commit 337a9dd Author: Adriaan Moors <[email protected]> Date: 4 weeks ago unit-typed lazy vals should never receive a field this need was unmasked by test/files/run/t7843-jsr223-service.scala, which no longer printed the output expected from the `0 to 10 foreach` Currently failing tests: - test/files/pos/t6780.scala - test/files/neg/anytrait.scala - test/files/neg/delayed-init-ref.scala - test/files/neg/t562.scala - test/files/neg/t6276.scala - test/files/run/delambdafy_uncurry_byname_inline.scala - test/files/run/delambdafy_uncurry_byname_method.scala - test/files/run/delambdafy_uncurry_inline.scala - test/files/run/delambdafy_uncurry_method.scala - test/files/run/inner-obj-auto.scala - test/files/run/lazy-traits.scala - test/files/run/reify_lazyunit.scala - test/files/run/showraw_mods.scala - test/files/run/t3670.scala - test/files/run/t3980.scala - test/files/run/t4047.scala - test/files/run/t6622.scala - test/files/run/t7406.scala - test/files/run/t7843-jsr223-service.scala - test/files/jvm/innerClassAttribute - test/files/specialized/SI-7343.scala - test/files/specialized/constant_lambda.scala - test/files/specialized/spec-early.scala - test/files/specialized/spec-init.scala - test/files/specialized/spec-matrix-new.scala - test/files/specialized/spec-matrix-old.scala - test/files/presentation/scope-completion-3 commit b1b4e5c Author: Adriaan Moors <[email protected]> Date: 4 weeks ago wip: lambdalift fix test/files/trait-defaults/lambdalift.scala works, but still some related tests failing (note that we can't use a bootstrapped compiler yet due to binary incompatibility in partest) commit eae7dac Author: Adriaan Moors <[email protected]> Date: 4 weeks ago update check now that trait vals can be concrete, use names not letters note the progression in a concrete trait val now being recognized as such ``` -trait T => true -method $init$ => false -value z1 => true -value z2 => true // z2 is actually concrete! ``` commit 6555c74 Author: Adriaan Moors <[email protected]> Date: 4 weeks ago bootstraps again by running info transform once per class... not sure how this ever worked, as separate compilation would transform a trait's info multiple times, resulting in double defs... commit 273cb20 Author: Adriaan Moors <[email protected]> Date: 6 weeks ago skip presuper vals in new encoding commit 728e71e Author: Adriaan Moors <[email protected]> Date: 6 weeks ago incoherent cyclic references between synthesized members must replace old trait accessor symbols by mixed in symbols in the infos of the mixed in symbols ``` trait T { val a: String ; val b: a.type } class C extends T { // a, b synthesized, but the a in b's type, a.type, refers to the original symbol, not the clone in C } ``` symbols occurring in types of synthesized members do not get rebound to other synthesized symbols package <empty>#4 { abstract <defaultparam/trait> trait N#7352 extends scala#22.AnyRef#2378 { <method> <deferred> <mutable> <accessor> <triedcooking> <sub_synth> def N$_setter_$self_$eq#15011(x$1#15012: <empty>#3.this.N#7352): scala#23.this.Unit#2340; <method> <deferred> <mutable> <accessor> <triedcooking> <sub_synth> def N$_setter_$n_$eq#15013(x$1#15014: N#7352.this.self#7442.type): scala#23.this.Unit#2340; <method> def /*N*/$init$scala#7441(): scala#23.this.Unit#2340 = { () }; <method> <deferred> <stable> <accessor> <triedcooking> <sub_synth> def self#7442: <empty>#3.this.N#7352; N#7352.this.N$_setter_$self_$eq#15011(scala#22.Predef#1729.$qmark$qmark$qmark#6917); <method> <deferred> <stable> <accessor> <sub_synth> def n#7443: N#7352.this.self#7442.type; N#7352.this.N$_setter_$n_$eq#15013(N#7352.this.self#7442) }; abstract class M#7353 extends scala#22.AnyRef#2378 { <method> <triedcooking> def <init>#13465(): <empty>#3.this.M#7353 = { M#7353.super.<init>scala#2719(); () }; <method> <deferred> <stable> <accessor> <triedcooking> def self#13466: <empty>#3.this.N#7352; <method> <deferred> <stable> <accessor> def n#13467: M#7353.this.self#13466.type }; class C#7354 extends M#7353 with <empty>#3.this.N#7352 { <method> <stable> <accessor> <triedcooking> def self#15016: <empty>#3.this.N#7352 = C#7354.this.self #15015; <triedcooking> private[this] val self #15015: <empty>#3.this.N#7352 = _; <method> <stable> <accessor> def n#15018: C#7354.this.self#7442.type = C#7354.this.n #15017; <triedcooking> private[this] val n #15017: C#7354.this.self#7442.type = _; <method> <mutable> <accessor> <triedcooking> def N$_setter_$self_$eq#15019(x$1#15021: <empty>#3.this.N#7352): scala#23.this.Unit#2340 = C#7354.this.self #15015 = x$1#15021; <method> <mutable> <accessor> <triedcooking> def N$_setter_$n_$eq#15022(x$1#15025: C#7354.this.self#7442.type): scala#23.this.Unit#2340 = C#7354.this.n #15017 = x$1#15025; <method> def <init>#14997(): <empty>#3.this.C#7354 = { C#7354.super.<init>#13465(); () } } } [running phase pickler on dependent_rebind.scala] [running phase refchecks on dependent_rebind.scala] test/files/trait-defaults/dependent_rebind.scala:16: error: overriding field n#15049 in trait N#7352 of type C#7354.this.self#15016.type; value n #15017 has incompatible type; found : => C#7354.this.self#7442.type (with underlying type => C#7354.this.self#7442.type) required: => N#7352.this.self#7442.type class C extends M with N ^ pos/t9111-inliner-workaround revealed need for: - override def transformInfo(sym: Symbol, tp: Type): Type = synthFieldsAndAccessors(tp) + override def transformInfo(sym: Symbol, tp: Type): Type = if (!sym.isJavaDefined) synthFieldsAndAccessors(tp) else tp commit b56ca2f Author: Adriaan Moors <[email protected]> Date: 6 weeks ago static forwarders & private[this] val in trait object Properties extends PropertiesTrait trait PropertiesTrait { // the protected member is not emitted as a static member of -- it works when dropping the access modifier // somehow, the module class member is considered deferred in BForwardersGen protected val propFilename: String = / } // [log jvm] No forwarder for 'getter propFilename' from Properties to 'module class Properties': false || m.isDeferred == true || false || false // the following method is missing compared to scalac // public final class Properties { // public static String propFilename() { // return Properties$.MODULE$.propFilename(); // } trait Chars { private[this] val char2uescapeArray = Array[Char]('\', 'u', 0, 0, 0, 0) } object Chars extends Chars // +++ w/reflect/scala/reflect/internal/Chars$.class // - private final [C scala83014char2uescapeArray constant fold in forwarder for backwards compat constant-typed final val in trait should yield impl method bean{setter,getter} delegates to setter/getter (commit 1655d1b)
1 parent a6cda80 commit cc88aa5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1012
-203
lines changed

src/compiler/scala/tools/nsc/Global.scala

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,10 +458,18 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
458458
val runsRightAfter = None
459459
} with ExtensionMethods
460460

461+
// phaseName = "fields"
462+
object fields extends {
463+
val global: Global.this.type = Global.this
464+
// somewhere between typers, before erasure (so we get bridges for getters) and pickler (separate compilation of trait with fields and subclass)
465+
val runsAfter = List("extmethods")
466+
val runsRightAfter = None
467+
} with Fields
468+
461469
// phaseName = "pickler"
462470
object pickler extends {
463471
val global: Global.this.type = Global.this
464-
val runsAfter = List("extmethods")
472+
val runsAfter = List("fields")
465473
val runsRightAfter = None
466474
} with Pickler
467475

@@ -605,7 +613,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
605613
* This implementation creates a description map at the same time.
606614
*/
607615
protected def computeInternalPhases(): Unit = {
608-
// Note: this fits -Xshow-phases into 80 column width, which it is
616+
// Note: this fits -Xshow-phases into 80 column width, which is
609617
// desirable to preserve.
610618
val phs = List(
611619
syntaxAnalyzer -> "parse source into ASTs, perform simple desugaring",
@@ -615,6 +623,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
615623
patmat -> "translate match expressions",
616624
superAccessors -> "add super accessors in traits and nested classes",
617625
extensionMethods -> "add extension methods for inline classes",
626+
fields -> "synthesize accessors and fields",
618627
pickler -> "serialize symbol tables",
619628
refChecks -> "reference/override checking, translate nested objects",
620629
uncurry -> "uncurry, translate function values to anonymous classes",

src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
155155

156156
def enclosingMethod(sym: Symbol): Option[Symbol] = {
157157
if (sym.isClass || sym == NoSymbol) None
158-
else if (sym.isMethod) {
158+
else if (sym.isMethod && !sym.isGetter) {
159159
if (doesNotExist(sym)) None else Some(sym)
160160
}
161161
else enclosingMethod(nextEnclosing(sym))

src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
646646
(((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModuleClass(sym))
647647
&& !sym.enclClass.isTrait
648648
&& !sym.isClassConstructor
649-
&& !sym.isMutable // lazy vals and vars both
649+
&& (!sym.isMutable || nme.isTraitSetterName(sym.name)) // lazy vals and vars and their setters cannot be final, but trait setters are
650650
)
651651

652652
// Primitives are "abstract final" to prohibit instantiation

src/compiler/scala/tools/nsc/transform/Fields.scala

Lines changed: 437 additions & 0 deletions
Large diffs are not rendered by default.

src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,10 @@ trait MethodSynthesis {
131131
val getterSym = getter.createAndEnterSymbol()
132132

133133
// Create the setter if necessary.
134-
if (getter.needsSetter)
135-
Setter(tree).createAndEnterSymbol()
134+
if (getter.needsSetter) Setter(tree).createAndEnterSymbol()
136135

137-
// If the getter's abstract the tree gets the getter's symbol,
138-
// otherwise, create a field (assume the getter requires storage).
136+
// If the getter's abstract, the tree gets the getter's symbol,
137+
// otherwise, create a field (we have to assume the getter requires storage for now).
139138
// NOTE: we cannot look at symbol info, since we're in the process of deriving them
140139
// (luckily, they only matter for lazy vals, which we've ruled out in this else branch,
141140
// and `doNotDeriveField` will skip them if `!mods.isLazy`)
@@ -304,46 +303,54 @@ trait MethodSynthesis {
304303

305304
result
306305
}
306+
307307
final def derive(initial: List[AnnotationInfo]): Tree = {
308308
validate()
309309

310310
// see scala.annotation.meta's package class for more info
311311
// Annotations on ValDefs can be targeted towards the following: field, getter, setter, beanGetter, beanSetter, param.
312312
// The defaults are:
313313
// - (`val`-, `var`- or plain) constructor parameter annotations end up on the parameter, not on any other entity.
314-
// - val/var member annotations solely end up on the underlying field.
314+
// - val/var member annotations solely end up on the underlying field, except in traits (@since 2.12),
315+
// where there is no field, and the getter thus holds annotations targetting both getter & field.
316+
// As soon as there is a field/getter (in subclasses mixing in the trait), we triage the annotations.
315317
//
316318
// TODO: these defaults can be surprising for annotations not meant for accessors/fields -- should we revisit?
317319
// (In order to have `@foo val X` result in the X getter being annotated with `@foo`, foo needs to be meta-annotated with @getter)
318320
val annotFilter: AnnotationInfo => Boolean = this match {
319321
case _: Param => annotationFilter(ParamTargetClass, defaultRetention = true)
320322
// By default annotations go to the field, except if the field is generated for a class parameter (PARAMACCESSOR).
321323
case _: Field => annotationFilter(FieldTargetClass, defaultRetention = !mods.isParamAccessor)
324+
case _: BaseGetter if owner.isTrait => annotationFilter(List(FieldTargetClass, GetterTargetClass), defaultRetention = true)
322325
case _: BaseGetter => annotationFilter(GetterTargetClass, defaultRetention = false)
323326
case _: Setter => annotationFilter(SetterTargetClass, defaultRetention = false)
324327
case _: BeanSetter => annotationFilter(BeanSetterTargetClass, defaultRetention = false)
328+
// TODO do bean getters need special treatment to collect field-targeting annotations in traits?
325329
case _: AnyBeanGetter => annotationFilter(BeanGetterTargetClass, defaultRetention = false)
326330
}
327331

328332
// The annotations amongst those found on the original symbol which
329333
// should be propagated to this kind of accessor.
330334
derivedSym setAnnotations (initial filter annotFilter)
331335

336+
if (derivedSym.isSetter && owner.isTrait && !isDeferred)
337+
derivedSym addAnnotation TraitSetterAnnotationClass
338+
332339
logDerived(derivedTree)
333340
}
334341
}
342+
335343
sealed trait DerivedGetter extends DerivedFromValDef {
336-
// A getter must be accompanied by a setter if the ValDef is mutable.
337344
def needsSetter = mods.isMutable
338345
}
339346
sealed trait DerivedSetter extends DerivedFromValDef {
340347
override def isSetter = true
341-
private def setterParam = derivedSym.paramss match {
348+
protected def setterParam = derivedSym.paramss match {
342349
case (p :: Nil) :: _ => p
343350
case _ => NoSymbol
344351
}
345352

346-
private def setterRhs = {
353+
protected def setterRhs = {
347354
assert(!derivedSym.isOverloaded, s"Unexpected overloaded setter $derivedSym for $basisSym in $enclClass")
348355
if (Field.noFieldFor(tree) || derivedSym.isOverloaded) EmptyTree
349356
else Assign(fieldSelection, Ident(setterParam))
@@ -390,6 +397,7 @@ trait MethodSynthesis {
390397
override def derivedSym = if (Field.noFieldFor(tree)) basisSym else basisSym.getterIn(enclClass)
391398
private def derivedRhs = if (Field.noFieldFor(tree)) tree.rhs else fieldSelection
392399

400+
// TODO: more principled approach -- this is a bit bizarre
393401
private def derivedTpt = {
394402
// For existentials, don't specify a type for the getter, even one derived
395403
// from the symbol! This leads to incompatible existentials for the field and
@@ -457,24 +465,31 @@ trait MethodSynthesis {
457465
def flagsMask = SetterFlags
458466
def flagsExtra = ACCESSOR
459467

468+
// TODO: double check logic behind need for name expansion in context of new fields phase
460469
override def derivedSym = basisSym.setterIn(enclClass)
461470
}
462471

463472
object Field {
464473
// No field for these vals (either never emitted or eliminated later on):
465474
// - abstract vals have no value we could store (until they become concrete, potentially)
466475
// - lazy vals of type Unit
467-
// - [Emitted, later removed during AddInterfaces/Mixins] concrete vals in traits can't have a field
468-
// - [Emitted, later removed during Constructors] a concrete val with a statically known value (Unit / ConstantType)
476+
// - concrete vals in traits don't yield a field here either (their getter's RHS has the initial value)
477+
// AddInterfaces will move the assignment to the constructor, abstracting over the field using the field setter,
478+
// and Fields will add a field to the class that mixes in the trait, implementing the accessors in terms of it
479+
// - [Emitted, later removed during Constructors] a concrete val with a statically known value (ConstantType)
469480
// performs its side effect according to lazy/strict semantics, but doesn't need to store its value
470481
// each access will "evaluate" the RHS (a literal) again
471482
// We would like to avoid emitting unnecessary fields, but the required knowledge isn't available until after typer.
472483
// The only way to avoid emitting & suppressing, is to not emit at all until we are sure to need the field, as dotty does.
473484
// NOTE: do not look at `vd.symbol` when called from `enterGetterSetter` (luckily, that call-site implies `!mods.isLazy`),
485+
// similarly, the `def field` call-site breaks when you add `|| vd.symbol.owner.isTrait` (detected in test suite)
474486
// as the symbol info is in the process of being created then.
475487
// TODO: harmonize tree & symbol creation
476-
// TODO: the `def field` call-site breaks when you add `|| vd.symbol.owner.isTrait` (detected in test suite)
477-
def noFieldFor(vd: ValDef) = vd.mods.isDeferred || (vd.mods.isLazy && isUnitType(vd.symbol.info))
488+
// the middle `&& !owner.isTrait` is needed after `isLazy` because non-unit-typed lazy vals in traits still get a field -- see neg/t5455.scala
489+
def noFieldFor(vd: ValDef) = vd.mods.isDeferred || (vd.mods.isLazy && !owner.isTrait && isUnitType(vd.symbol.info)) || (owner.isTrait && !traitFieldFor(vd))
490+
491+
// TODO: never emit any fields in traits -- only use getter for lazy/presuper ones as well
492+
private def traitFieldFor(vd: ValDef): Boolean = vd.mods.hasFlag(PRESUPER | LAZY)
478493
}
479494

480495
case class Field(tree: ValDef) extends DerivedFromValDef {
@@ -528,7 +543,10 @@ trait MethodSynthesis {
528543
}
529544
case class BooleanBeanGetter(tree: ValDef) extends BeanAccessor("is") with AnyBeanGetter { }
530545
case class BeanGetter(tree: ValDef) extends BeanAccessor("get") with AnyBeanGetter { }
531-
case class BeanSetter(tree: ValDef) extends BeanAccessor("set") with DerivedSetter
546+
case class BeanSetter(tree: ValDef) extends BeanAccessor("set") with DerivedSetter {
547+
// TODO: document, motivate
548+
override protected def setterRhs = Apply(Ident(tree.name.setterName), List(Ident(setterParam)))
549+
}
532550

533551
// No Symbols available.
534552
private def beanAccessorsFromNames(tree: ValDef) = {

src/compiler/scala/tools/nsc/typechecker/Namers.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ trait Namers extends MethodSynthesis {
117117
}
118118

119119
// All lazy vals need accessors, including those owned by terms (e.g., in method) or private[this] in a class
120-
def deriveAccessors(vd: ValDef) = vd.mods.isLazy || (owner.isClass && deriveAccessorsInClass(vd))
120+
def deriveAccessors(vd: ValDef) = (vd.mods.isLazy || owner.isTrait || (owner.isClass && deriveAccessorsInClass(vd)))
121121

122122
private def deriveAccessorsInClass(vd: ValDef) =
123-
!vd.mods.isPrivateLocal && // note, private[this] lazy vals do get accessors -- see outer disjunction of deriveAccessors
123+
!vd.mods.isPrivateLocal && // note, private[this] lazy vals do get accessors -- see outer disjunction of deriveAccessors
124124
!(vd.name startsWith nme.OUTER) && // outer accessors are added later, in explicitouter
125125
!isEnumConstant(vd) // enums can only occur in classes, so only check here
126126

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,9 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
423423
overrideError("cannot be used here - class definitions cannot be overridden")
424424
} else if (!other.isDeferred && member.isClass) {
425425
overrideError("cannot be used here - classes can only override abstract types")
426-
} else if (other.isEffectivelyFinal) { // (1.2)
426+
} else if (other.isEffectivelyFinal && !other.hasFlag(SYNTHESIZE_IMPL_IN_SUBCLASS)) { // (1.2)
427427
overrideError("cannot override final member")
428-
} else if (!other.isDeferredOrJavaDefault && !other.hasFlag(JAVA_DEFAULTMETHOD) && !member.isAnyOverride && !member.isSynthetic) { // (*)
428+
} else if (!other.isDeferredOrJavaDefault && !other.hasFlag(JAVA_DEFAULTMETHOD) && !member.isAnyOverride && !member.isSynthetic && !other.hasFlag(SYNTHESIZE_IMPL_IN_SUBCLASS)) { // (*)
429429
// (*) Synthetic exclusion for (at least) default getters, fixes SI-5178. We cannot assign the OVERRIDE flag to
430430
// the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
431431
if (isNeitherInClass && !(other.owner isSubClass member.owner))
@@ -465,7 +465,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
465465
overrideError("cannot be used here - only term macros can override term macros")
466466
} else {
467467
checkOverrideTypes()
468-
checkOverrideDeprecated()
468+
if (!other.hasFlag(SYNTHESIZE_IMPL_IN_SUBCLASS))
469+
checkOverrideDeprecated()
469470
if (settings.warnNullaryOverride) {
470471
if (other.paramss.isEmpty && !member.paramss.isEmpty) {
471472
reporter.warning(member.pos, "non-nullary method overrides nullary method")
@@ -1131,22 +1132,16 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
11311132
case _ =>
11321133
}
11331134

1134-
// SI-6276 warn for `def foo = foo` or `val bar: X = bar`, which come up more frequently than you might think.
1135-
def checkInfiniteLoop(valOrDef: ValOrDefDef) {
1136-
def callsSelf = valOrDef.rhs match {
1137-
case t @ (Ident(_) | Select(This(_), _)) =>
1138-
t hasSymbolWhich (_.accessedOrSelf == valOrDef.symbol)
1139-
case _ => false
1135+
// SI-6276 warn for trivial recursion, such as `def foo = foo` or `val bar: X = bar`, which come up more frequently than you might think.
1136+
// TODO: Move to abide rule. Also, this does not check that the def is final or not overridden, for example
1137+
def checkInfiniteLoop(sym: Symbol, rhs: Tree): Unit =
1138+
if (!sym.isValueParameter && sym.paramss.isEmpty) {
1139+
rhs match {
1140+
case t@(Ident(_) | Select(This(_), _)) if t hasSymbolWhich (_.accessedOrSelf == sym) =>
1141+
reporter.warning(rhs.pos, s"${sym.fullLocationString} does nothing other than call itself recursively")
1142+
case _ =>
1143+
}
11401144
}
1141-
val trivialInfiniteLoop = (
1142-
!valOrDef.isErroneous
1143-
&& !valOrDef.symbol.isValueParameter
1144-
&& valOrDef.symbol.paramss.isEmpty
1145-
&& callsSelf
1146-
)
1147-
if (trivialInfiniteLoop)
1148-
reporter.warning(valOrDef.rhs.pos, s"${valOrDef.symbol.fullLocationString} does nothing other than call itself recursively")
1149-
}
11501145

11511146
// Transformation ------------------------------------------------------------
11521147

@@ -1480,7 +1475,21 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
14801475
tree match {
14811476
case m: MemberDef =>
14821477
val sym = m.symbol
1483-
applyChecks(sym.annotations)
1478+
val allAnnots = sym.annotations
1479+
1480+
// drop field-targeting annotations from getters
1481+
// (in traits, getters must also hold annotations that target the underlying field,
1482+
// because the latter won't be created until the trait is mixed into a class)
1483+
// TODO do bean getters need special treatment to suppress field-targeting annotations in traits?
1484+
val annots =
1485+
if (sym.isGetter && sym.owner.isTrait) {
1486+
val annots = allAnnots filter AnnotationInfo.mkFilter(GetterTargetClass, defaultRetention = false)
1487+
// if (annots != allAnnots) println(s"filtering for $sym: $allAnnots to $annots")
1488+
sym setAnnotations annots
1489+
annots
1490+
} else allAnnots
1491+
1492+
applyChecks(annots)
14841493

14851494
def messageWarning(name: String)(warn: String) =
14861495
reporter.warning(tree.pos, f"Invalid $name message for ${sym}%s${sym.locationString}%s:%n$warn")
@@ -1661,9 +1670,13 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
16611670
sym resetFlag DEFERRED
16621671
transform(deriveDefDef(tree)(_ => typed(gen.mkSysErrorCall("native method stub"))))
16631672

1664-
case ValDef(_, _, _, _) | DefDef(_, _, _, _, _, _) =>
1673+
// NOTE: a val in a trait is now a DefDef, with the RHS being moved to an Assign in AddInterfaces
1674+
case tree: ValOrDefDef =>
16651675
checkDeprecatedOvers(tree)
1666-
checkInfiniteLoop(tree.asInstanceOf[ValOrDefDef])
1676+
1677+
if (!tree.isErroneous)
1678+
checkInfiniteLoop(tree.symbol, tree.rhs)
1679+
16671680
if (settings.warnNullaryUnit)
16681681
checkNullaryMethodReturnType(sym)
16691682
if (settings.warnInaccessible) {

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,13 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
13601360
notAllowed(s"redefinition of $name method. See SIP-15, criterion 4.")
13611361
else if (stat.symbol != null && stat.symbol.isParamAccessor)
13621362
notAllowed("additional parameter")
1363+
// concrete accessor (getter) in trait corresponds to a field definition (neg/anytrait.scala)
1364+
// TODO: only reject accessors that actually give rise to field (e.g., a constant-type val is fine)
1365+
else if (!isValueClass && stat.symbol.isAccessor && !stat.symbol.isDeferred)
1366+
notAllowed("field definition")
13631367
checkEphemeralDeep.traverse(rhs)
1368+
// for value class or "exotic" vals in traits
1369+
// (traits don't receive ValDefs for regular vals until fields phase -- well, except for early initialized/lazy vals)
13641370
case _: ValDef =>
13651371
notAllowed("field definition")
13661372
case _: ModuleDef =>
@@ -4213,7 +4219,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
42134219
// if (varsym.isVariable ||
42144220
// // setter-rewrite has been done above, so rule out methods here, but, wait a minute, why are we assigning to non-variables after erasure?!
42154221
// (phase.erasedTypes && varsym.isValue && !varsym.isMethod)) {
4216-
if (varsym.isVariable || varsym.isValue && phase.erasedTypes) {
4222+
if (varsym.isVariable || varsym.isValue && phase.assignsFields) {
42174223
val rhs1 = typedByValueExpr(rhs, lhs1.tpe)
42184224
treeCopy.Assign(tree, lhs1, checkDead(rhs1)) setType UnitTpe
42194225
}

src/library/scala/deprecatedInheritance.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package scala
1010

11+
import scala.annotation.meta._
12+
1113
/** An annotation that designates that inheriting from a class is deprecated.
1214
*
1315
* This is usually done to warn about a non-final class being made final in a future version.
@@ -19,4 +21,5 @@ package scala
1921
* @since 2.10
2022
* @see [[scala.deprecatedOverriding]]
2123
*/
24+
@getter @setter @beanGetter @beanSetter
2225
class deprecatedInheritance(message: String = "", since: String = "") extends scala.annotation.StaticAnnotation

src/library/scala/deprecatedOverriding.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package scala
1010

11+
import scala.annotation.meta._
12+
1113
/** An annotation that designates that overriding a member is deprecated.
1214
*
1315
* Overriding such a member in a sub-class then generates a warning.
@@ -17,4 +19,5 @@ package scala
1719
* @since 2.10
1820
* @see [[scala.deprecatedInheritance]]
1921
*/
22+
@getter @setter @beanGetter @beanSetter
2023
class deprecatedOverriding(message: String = "", since: String = "") extends scala.annotation.StaticAnnotation

0 commit comments

Comments
 (0)