Skip to content

Commit a72423e

Browse files
authored
Better grouping of explanations in error messages (#24155)
Group explanations that refer to the same Recorded entry with multiple strings together. Based on #24154
2 parents 9a46502 + 0bddba1 commit a72423e

Some content is hidden

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

46 files changed

+370
-435
lines changed

compiler/src/dotty/tools/dotc/cc/Capability.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,11 @@ object Capabilities:
205205
hiddenSet.adoptClassifier(cls)
206206
if freeze then isClassified = true
207207

208+
def ccOwnerStr(using Context): String =
209+
val owner = ccOwner
210+
if owner.name == nme.SKOLEM then i"a new instance of ${hiddenSet.owner}"
211+
else owner.show
212+
208213
def descr(using Context) =
209214
val originStr = origin match
210215
case Origin.InDecl(sym) if sym.exists =>

compiler/src/dotty/tools/dotc/cc/CaptureOps.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,7 @@ extension (tp: Type)
467467
acc(false, tp)
468468

469469
def refinedOverride(name: Name, rinfo: Type)(using Context): Type =
470-
RefinedType(tp, name,
471-
AnnotatedType(rinfo, Annotation(defn.RefineOverrideAnnot, util.Spans.NoSpan)))
470+
RefinedType.precise(tp, name, rinfo)
472471

473472
def dropUseAndConsumeAnnots(using Context): Type =
474473
tp.dropAnnot(defn.UseAnnot).dropAnnot(defn.ConsumeAnnot)

compiler/src/dotty/tools/dotc/cc/SepCheck.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ class SepCheck(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
358358
if shared.isEmpty then i"${CaptureSet(shared)}"
359359
else shared.nth(0) match
360360
case fresh: FreshCap =>
361-
if fresh.hiddenSet.owner.exists then i"{$fresh of ${fresh.hiddenSet.owner}}" else i"$fresh"
361+
val where = if ctx.settings.YccVerbose.value then "" else i" of ${fresh.ccOwnerStr}"
362+
i"{$fresh$where}"
362363
case _ =>
363364
i"${CaptureSet(shared)}"
364365

@@ -574,9 +575,9 @@ class SepCheck(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
574575
case Select(This(_), _) => tree
575576
case Select(prefix, _) => pathRoot(prefix)
576577
case _ => tree
577-
578+
578579
val rootSym = pathRoot(tree).symbol
579-
580+
580581
def findClashing(prevDefs: List[DefInfo]): Option[DefInfo] = prevDefs match
581582
case prevDef :: prevDefs1 =>
582583
if prevDef.symbol == rootSym then Some(prevDef)

compiler/src/dotty/tools/dotc/cc/Setup.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
273273
then
274274
val getterType =
275275
mapInferred(inCaptureRefinement = true)(tp.memberInfo(getter)).strippedDealias
276-
RefinedType(core, getter.name,
276+
RefinedType.precise(core, getter.name,
277277
CapturingType(getterType,
278278
CaptureSet.ProperVar(ctx.owner, isRefining = true)))
279279
.showing(i"add capture refinement $tp --> $result", capt)

compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,7 +1089,6 @@ class Definitions {
10891089
@tu lazy val UseAnnot: ClassSymbol = requiredClass("scala.caps.use")
10901090
@tu lazy val ReserveAnnot: ClassSymbol = requiredClass("scala.caps.reserve")
10911091
@tu lazy val ConsumeAnnot: ClassSymbol = requiredClass("scala.caps.internal.consume")
1092-
@tu lazy val RefineOverrideAnnot: ClassSymbol = requiredClass("scala.caps.internal.refineOverride")
10931092
@tu lazy val VolatileAnnot: ClassSymbol = requiredClass("scala.volatile")
10941093
@tu lazy val LanguageFeatureMetaAnnot: ClassSymbol = requiredClass("scala.annotation.meta.languageFeature")
10951094
@tu lazy val BeanGetterMetaAnnot: ClassSymbol = requiredClass("scala.annotation.meta.beanGetter")
@@ -1133,7 +1132,7 @@ class Definitions {
11331132

11341133
// Set of annotations that are not printed in types except under -Yprint-debug
11351134
@tu lazy val SilentAnnots: Set[Symbol] =
1136-
Set(InlineParamAnnot, ErasedParamAnnot, RefineOverrideAnnot, SilentIntoAnnot, UseAnnot, ConsumeAnnot)
1135+
Set(InlineParamAnnot, ErasedParamAnnot, SilentIntoAnnot, UseAnnot, ConsumeAnnot)
11371136

11381137
// A list of annotations that are commonly used to indicate that a field/method argument or return
11391138
// type is not null. These annotations are used by the nullification logic in JavaNullInterop to

compiler/src/dotty/tools/dotc/core/NamerOps.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ object NamerOps:
5252
*/
5353
def addParamRefinements(resType: Type, paramss: List[List[Symbol]])(using Context): Type =
5454
paramss.flatten.foldLeft(resType): (rt, param) =>
55-
if param.is(Tracked) then RefinedType(rt, param.name, param.termRef)
55+
if param.is(Tracked) then RefinedType.precise(rt, param.name, param.termRef)
5656
else rt
5757

5858
/** Split dependent class refinements off parent type. Add them to `refinements`,

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -914,12 +914,8 @@ object Types extends TypeUtils {
914914
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, jointInfo)
915915
}
916916
else
917-
val overridingRefinement = rinfo match
918-
case AnnotatedType(rinfo1, ann) if ann.symbol == defn.RefineOverrideAnnot => rinfo1
919-
case _ if pdenot.symbol.is(Tracked) => rinfo
920-
case _ => NoType
921-
if overridingRefinement.exists then
922-
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, overridingRefinement)
917+
if tp.isPrecise then
918+
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, rinfo)
923919
else
924920
val isRefinedMethod = rinfo.isInstanceOf[MethodOrPoly]
925921
val joint = CCState.withCollapsedFresh:
@@ -3259,6 +3255,12 @@ object Types extends TypeUtils {
32593255
else assert(refinedInfo.isInstanceOf[TypeType], this)
32603256
assert(!refinedName.is(NameKinds.ExpandedName), this)
32613257

3258+
/** If true we know that refinedInfo is always more precise than the info for
3259+
* field `refinedName` in parent, so no type intersection needs to be computed
3260+
* for the type of this field.
3261+
*/
3262+
def isPrecise: Boolean = false
3263+
32623264
override def underlying(using Context): Type = parent
32633265

32643266
private def badInst =
@@ -3270,6 +3272,7 @@ object Types extends TypeUtils {
32703272
(parent: Type = this.parent, refinedName: Name = this.refinedName, refinedInfo: Type = this.refinedInfo)
32713273
(using Context): Type =
32723274
if ((parent eq this.parent) && (refinedName eq this.refinedName) && (refinedInfo eq this.refinedInfo)) this
3275+
else if isPrecise then RefinedType.precise(parent, refinedName, refinedInfo)
32733276
else RefinedType(parent, refinedName, refinedInfo)
32743277

32753278
/** Add this refinement to `parent`, provided `refinedName` is a member of `parent`. */
@@ -3302,6 +3305,10 @@ object Types extends TypeUtils {
33023305
class CachedRefinedType(parent: Type, refinedName: Name, refinedInfo: Type)
33033306
extends RefinedType(parent, refinedName, refinedInfo)
33043307

3308+
class PreciseRefinedType(parent: Type, refinedName: Name, refinedInfo: Type)
3309+
extends RefinedType(parent, refinedName, refinedInfo):
3310+
override def isPrecise = true
3311+
33053312
object RefinedType {
33063313
@tailrec def make(parent: Type, names: List[Name], infos: List[Type])(using Context): Type =
33073314
if (names.isEmpty) parent
@@ -3311,6 +3318,10 @@ object Types extends TypeUtils {
33113318
assert(!ctx.erasedTypes)
33123319
unique(new CachedRefinedType(parent, name, info)).checkInst
33133320
}
3321+
3322+
def precise(parent: Type, name: Name, info: Type)(using Context): RefinedType =
3323+
assert(!ctx.erasedTypes)
3324+
unique(new PreciseRefinedType(parent, name, info)).checkInst
33143325
}
33153326

33163327
/** A recursive type. Instances should be constructed via the companion object.

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,11 +492,13 @@ class PlainPrinter(_ctx: Context) extends Printer {
492492
if c.hiddenSet.classifier == defn.AnyClass then ""
493493
else s" classified as ${c.hiddenSet.classifier.name.show}"
494494
def prefixTxt: Text = c.prefix match
495-
case NoPrefix | _: ThisType => ""
495+
case NoPrefix => ""
496+
case _: ThisType if !ccVerbose => ""
497+
case pre: TermRef if !ccVerbose && pre.name == nme.SKOLEM => ""
496498
case pre: SingletonType => toTextRef(pre) ~ "."
497499
case pre => toText(pre) ~ "."
498500
def core: Text =
499-
if ccVerbose then s"<fresh$idStr in ${c.ccOwner} hiding " ~ toTextCaptureSet(c.hiddenSet) ~ classified ~ ">"
501+
if ccVerbose then s"<fresh$idStr in ${c.ccOwnerStr} hiding " ~ toTextCaptureSet(c.hiddenSet) ~ classified ~ ">"
500502
else "cap"
501503
prefixTxt ~ core
502504
case tp: TypeProxy =>

compiler/src/dotty/tools/dotc/reporting/Message.scala

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ object Message:
142142
end record
143143

144144
/** Create explanation for single `Recorded` type or symbol */
145-
private def explanation(entry: AnyRef, key: String)(using Context): String =
145+
private def explanation(entry: AnyRef, keys: List[String])(using Context): String =
146146
def boundStr(bound: Type, default: ClassSymbol, cmp: String) =
147147
if (bound.isRef(default)) "" else i"$cmp $bound"
148148

@@ -178,7 +178,8 @@ object Message:
178178
s"is an unknown value of type ${tp.widen.show}"
179179
case ref: RootCapability =>
180180
val relation =
181-
if List("^", "=>", "?=>").exists(key.startsWith) then "refers to"
181+
if keys.length > 1 then "refer to"
182+
else if List("^", "=>", "?=>").exists(keys(0).startsWith) then "refers to"
182183
else "is"
183184
s"$relation ${ref.descr}"
184185
end explanation
@@ -207,16 +208,20 @@ object Message:
207208
res // help the inferencer out
208209
}.sortBy(_._1)
209210

210-
def columnar(parts: List[(String, String)]): List[String] = {
211+
def columnar(parts: List[(String, String)]): List[String] =
211212
lazy val maxLen = parts.map(_._1.length).max
212-
parts.map {
213-
case (leader, trailer) =>
214-
val variable = hl(leader)
215-
s"""$variable${" " * (maxLen - leader.length)} $trailer"""
216-
}
217-
}
218-
219-
val explainParts = toExplain.map { case (str, entry) => (str, explanation(entry, str)) }
213+
parts.map: (leader, trailer) =>
214+
val variable = hl(leader)
215+
s"""$variable${" " * (maxLen - leader.length)} $trailer"""
216+
217+
// Group keys with the same Recorded entry together. We can't use groupBy here
218+
// since we want to maintain the order in which entries first appear in the
219+
// original list.
220+
val toExplainGrouped: List[(Recorded, List[String])] =
221+
for entry <- toExplain.map(_._2).distinct
222+
yield (entry, for (key, e) <- toExplain if e == entry yield key)
223+
val explainParts = toExplainGrouped.map:
224+
(entry, keys) => (keys.mkString(" and "), explanation(entry, keys))
220225
val explainLines = columnar(explainParts)
221226
if (explainLines.isEmpty) "" else i"where: $explainLines%\n %\n"
222227
end explanations

library/src/scala/caps/package.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ object internal:
145145
* info from the parent type, so no intersection needs to be formed.
146146
* This could be useful for tracked parameters as well.
147147
*/
148+
@deprecated
148149
final class refineOverride extends annotation.StaticAnnotation
149150

150151
/** An annotation used internally for root capability wrappers of `cap` that

0 commit comments

Comments
 (0)