From 4a11a612724408149cb1d3b55e759f5b8a09bccd Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 10 Apr 2024 11:56:00 +0200 Subject: [PATCH 1/3] Load docstings lazily from TASTy Now we are always able to load docstings from TASTy, even if `-Yread-docs` is not set. The `-Yread-docs` flag loads the doc strings eagerly, as it did before. Fixes #20106 --- compiler/src/dotty/tools/dotc/Driver.scala | 2 +- .../tools/dotc/config/ScalaSettings.scala | 2 +- .../src/dotty/tools/dotc/core/Comments.scala | 21 +++++++++++++++---- .../tools/dotc/core/tasty/TreePickler.scala | 7 ++++--- .../tools/dotc/core/tasty/TreeUnpickler.scala | 14 +++++++------ .../dotc/decompiler/IDEDecompilerDriver.scala | 1 - .../dotc/interactive/InteractiveDriver.scala | 1 - .../dotty/tools/dotc/transform/Pickler.scala | 1 - .../src/dotty/tools/repl/ReplDriver.scala | 1 - .../dotc/core/tasty/CommentPicklingTest.scala | 4 ---- .../scaladoc/snippets/SnippetCompiler.scala | 2 +- .../tasty/inspector/TastyInspector.scala | 2 +- .../tasty/inspector/TastyInspector.scala | 1 - tests/run-macros/i20106.check | 16 ++++++++++++++ tests/run-macros/i20106/Macro_1.scala | 12 +++++++++++ tests/run-macros/i20106/Main_1.scala | 14 +++++++++++++ tests/run-macros/i20106/Test_2.scala | 19 +++++++++++++++++ 17 files changed, 94 insertions(+), 26 deletions(-) create mode 100644 tests/run-macros/i20106.check create mode 100644 tests/run-macros/i20106/Macro_1.scala create mode 100644 tests/run-macros/i20106/Main_1.scala create mode 100644 tests/run-macros/i20106/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/Driver.scala b/compiler/src/dotty/tools/dotc/Driver.scala index ae2219a4f049..2dcd91d35bbb 100644 --- a/compiler/src/dotty/tools/dotc/Driver.scala +++ b/compiler/src/dotty/tools/dotc/Driver.scala @@ -79,7 +79,7 @@ class Driver { Positioned.init(using ictx) inContext(ictx) { - if !ctx.settings.YdropComments.value || ctx.settings.YreadComments.value then + if !ctx.settings.YdropComments.value then ictx.setProperty(ContextDoc, new ContextDocstrings) val fileNamesOrNone = command.checkUsage(summary, sourcesRequired)(using ctx.settings)(using ctx.settingsState) fileNamesOrNone.map { fileNames => diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index fc7e61c8ec71..52e320a2f64d 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -394,7 +394,7 @@ private sealed trait YSettings: val YcheckReentrant: Setting[Boolean] = BooleanSetting(ForkSetting, "Ycheck-reentrant", "Check that compiled program does not contain vars that can be accessed from a global root.") val YdropComments: Setting[Boolean] = BooleanSetting(ForkSetting, "Ydrop-docs", "Drop documentation when scanning source files.", aliases = List("-Ydrop-comments")) val YcookComments: Setting[Boolean] = BooleanSetting(ForkSetting, "Ycook-docs", "Cook the documentation (type check `@usecase`, etc.)", aliases = List("-Ycook-comments")) - val YreadComments: Setting[Boolean] = BooleanSetting(ForkSetting, "Yread-docs", "Read documentation from tasty.") + val YreadDocsEagerly: Setting[Boolean] = BooleanSetting(ForkSetting, "Yread-docs", "Read documentation eagerly from TASTy") val YforceSbtPhases: Setting[Boolean] = BooleanSetting(ForkSetting, "Yforce-sbt-phases", "Run the phases used by sbt for incremental compilation (ExtractDependencies and ExtractAPI) even if the compiler is ran outside of sbt, for debugging.") val YdumpSbtInc: Setting[Boolean] = BooleanSetting(ForkSetting, "Ydump-sbt-inc", "For every compiled foo.scala, output the API representation and dependencies used for sbt incremental compilation in foo.inc, implies -Yforce-sbt-phases.") val YcheckAllPatmat: Setting[Boolean] = BooleanSetting(ForkSetting, "Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm).") diff --git a/compiler/src/dotty/tools/dotc/core/Comments.scala b/compiler/src/dotty/tools/dotc/core/Comments.scala index 92160c97973d..1c40581e3835 100644 --- a/compiler/src/dotty/tools/dotc/core/Comments.scala +++ b/compiler/src/dotty/tools/dotc/core/Comments.scala @@ -24,18 +24,31 @@ object Comments { */ class ContextDocstrings { - private val _docstrings: MutableSymbolMap[Comment] = MutableSymbolMap[Comment](512) // FIXME: 2nd [Comment] needed or "not a class type" + private val docstrings: MutableSymbolMap[Comment | CommentLoader] = MutableSymbolMap[Comment | CommentLoader](512) // FIXME: 2nd [Comment] needed or "not a class type" + + private class CommentLoader(val load: () => Option[Comment]) val templateExpander: CommentExpander = new CommentExpander - def docstrings: ReadOnlyMap[Symbol, Comment] = _docstrings + def docstring(sym: Symbol): Option[Comment] = + docstrings.get(sym).flatMap { + case comment: Comment => Some(comment) + case commentLoader: CommentLoader => + val commentOpt = commentLoader.load() + commentOpt match + case Some(comment) => docstrings.update(sym, comment) + case None => docstrings.remove(sym) + commentOpt + } - def docstring(sym: Symbol): Option[Comment] = _docstrings.get(sym) + def registerLoader(sym: Symbol, loader: () => Option[Comment]): Unit = + docstrings.update(sym, CommentLoader(loader)) def addDocstring(sym: Symbol, doc: Option[Comment]): Unit = - doc.foreach(d => _docstrings.update(sym, d)) + doc.foreach(d => docstrings.update(sym, d)) } + /** * A `Comment` contains the unformatted docstring, it's position and potentially more * information that is populated when the comment is "cooked". diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index 0a8669292a74..08550fa483ee 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -366,9 +366,10 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) { if sym.is(Method) && sym.owner.isClass then profile.recordMethodSize(sym, (currentAddr.index - addr.index) max 1, mdef.span) for docCtx <- ctx.docCtx do - val comment = docCtx.docstrings.lookup(sym) - if comment != null then - docStrings(mdef) = comment + docCtx.docstring(sym) match + case Some(comment) => + docStrings(mdef) = comment + case _ => } def pickleParam(tree: Tree)(using Context): Unit = { diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 5f04418bbe7f..c32faa6458e1 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -981,13 +981,15 @@ class TreeUnpickler(reader: TastyReader, if !sym.isType && !sym.is(ParamAccessor) then sym.info = ta.avoidPrivateLeaks(sym) - if (ctx.settings.YreadComments.value) { - assert(ctx.docCtx.isDefined, "`-Yread-docs` enabled, but no `docCtx` is set.") - commentUnpicklerOpt.foreach { commentUnpickler => + commentUnpicklerOpt.foreach { commentUnpickler => + def loadComment() = val comment = commentUnpickler.commentAt(start) - ctx.docCtx.get.addDocstring(tree.symbol, comment) - tree.setComment(comment) - } + tree.setComment(comment) // TODO should this be set? + comment + if ctx.settings.YreadDocsEagerly.value then + ctx.docCtx.get.addDocstring(tree.symbol, loadComment()) + else + ctx.docCtx.get.registerLoader(tree.symbol, loadComment) } tree.setDefTree diff --git a/compiler/src/dotty/tools/dotc/decompiler/IDEDecompilerDriver.scala b/compiler/src/dotty/tools/dotc/decompiler/IDEDecompilerDriver.scala index c1bd6b6778fd..4e80b3fb7ab1 100644 --- a/compiler/src/dotty/tools/dotc/decompiler/IDEDecompilerDriver.scala +++ b/compiler/src/dotty/tools/dotc/decompiler/IDEDecompilerDriver.scala @@ -19,7 +19,6 @@ class IDEDecompilerDriver(val settings: List[String]) extends dotc.Driver { private val myInitCtx: Context = { val rootCtx = initCtx.fresh.addMode(Mode.Interactive | Mode.ReadPositions) - rootCtx.setSetting(rootCtx.settings.YreadComments, true) rootCtx.setSetting(rootCtx.settings.YretainTrees, true) rootCtx.setSetting(rootCtx.settings.fromTasty, true) val ctx = setup(settings.toArray :+ "dummy.scala", rootCtx).get._2 diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index 8f42c62cb3b0..1229aae62d22 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -34,7 +34,6 @@ class InteractiveDriver(val settings: List[String]) extends Driver { val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive) rootCtx.setSetting(rootCtx.settings.YretainTrees, true) rootCtx.setSetting(rootCtx.settings.YcookComments, true) - rootCtx.setSetting(rootCtx.settings.YreadComments, true) val ctx = setup(settings.toArray, rootCtx) match case Some((_, ctx)) => ctx case None => rootCtx diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 3a4212547d16..a904721ce799 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -203,7 +203,6 @@ class Pickler extends Phase { super.runOn(units) if ctx.settings.YtestPickler.value then val ctx2 = ctx.fresh - .setSetting(ctx.settings.YreadComments, true) .setSetting(ctx.settings.YshowPrintErrors, true) testUnpickler( using ctx2 diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index f8bba2f59fe1..34c13f815695 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -83,7 +83,6 @@ class ReplDriver(settings: Array[String], private def initialCtx(settings: List[String]) = { val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions | Mode.Interactive) rootCtx.setSetting(rootCtx.settings.YcookComments, true) - rootCtx.setSetting(rootCtx.settings.YreadComments, true) setupRootCtx(this.settings ++ settings, rootCtx) } diff --git a/compiler/test/dotty/tools/dotc/core/tasty/CommentPicklingTest.scala b/compiler/test/dotty/tools/dotc/core/tasty/CommentPicklingTest.scala index db58ff36ac42..4c0b3793cacf 100644 --- a/compiler/test/dotty/tools/dotc/core/tasty/CommentPicklingTest.scala +++ b/compiler/test/dotty/tools/dotc/core/tasty/CommentPicklingTest.scala @@ -108,10 +108,6 @@ class CommentPicklingTest { } private class UnpicklingDriver extends Driver { - override def initCtx = - val ctx = super.initCtx.fresh - ctx.setSetting(ctx.settings.YreadComments, true) - ctx def unpickle[T](args: Array[String], files: List[File])(fn: (List[tpd.Tree], Context) => T): T = { implicit val ctx: Context = setup(args, initCtx).map(_._2).getOrElse(initCtx) diff --git a/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompiler.scala b/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompiler.scala index b47b15676c57..9ac5fc34c28b 100644 --- a/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompiler.scala +++ b/scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompiler.scala @@ -31,7 +31,7 @@ class SnippetCompiler( rootCtx.setSetting(rootCtx.settings.experimental, true) rootCtx.setSetting(rootCtx.settings.YretainTrees, true) rootCtx.setSetting(rootCtx.settings.YcookComments, true) - rootCtx.setSetting(rootCtx.settings.YreadComments, true) + rootCtx.setSetting(rootCtx.settings.YreadDocsEagerly, true) rootCtx.setSetting(rootCtx.settings.color, "never") rootCtx.setSetting(rootCtx.settings.XimportSuggestionTimeout, 0) diff --git a/scaladoc/src/scala/tasty/inspector/TastyInspector.scala b/scaladoc/src/scala/tasty/inspector/TastyInspector.scala index 03b3aadedc4d..237c169929b9 100644 --- a/scaladoc/src/scala/tasty/inspector/TastyInspector.scala +++ b/scaladoc/src/scala/tasty/inspector/TastyInspector.scala @@ -103,7 +103,7 @@ object TastyInspector: reset() val ctx2 = ctx.fresh .addMode(Mode.ReadPositions) - .setSetting(ctx.settings.YreadComments, true) + .setSetting(ctx.settings.YreadDocsEagerly, true) new TASTYRun(this, ctx2) new InspectorDriver diff --git a/tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala b/tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala index ea3f0a95dded..5c3da941c53f 100644 --- a/tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala +++ b/tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala @@ -100,7 +100,6 @@ object TastyInspector: reset() val ctx2 = ctx.fresh .addMode(Mode.ReadPositions) - .setSetting(ctx.settings.YreadComments, true) new TASTYRun(this, ctx2) new InspectorDriver diff --git a/tests/run-macros/i20106.check b/tests/run-macros/i20106.check new file mode 100644 index 000000000000..25102b18110e --- /dev/null +++ b/tests/run-macros/i20106.check @@ -0,0 +1,16 @@ +docs from same compilation run: +Some(/** + * my doc string for Main + * + * @param tracks track listing + * */) +docs loaded from tasty: +Some(/** + * my doc string for Main + * + * @param tracks track listing + * */) +docs from same compilation run: +Some(/** + * Doc of Test + * */) diff --git a/tests/run-macros/i20106/Macro_1.scala b/tests/run-macros/i20106/Macro_1.scala new file mode 100644 index 000000000000..32bdfb233ba2 --- /dev/null +++ b/tests/run-macros/i20106/Macro_1.scala @@ -0,0 +1,12 @@ +import scala.quoted.* + +object Doc { + inline def of[A]: Option[String] = ${ ofImpl[A] } + + def ofImpl[A: Type](using Quotes): Expr[Option[String]] = { + import quotes.reflect.* + + val symbol = TypeRepr.of[A].typeSymbol + Expr(symbol.docstring) + } +} diff --git a/tests/run-macros/i20106/Main_1.scala b/tests/run-macros/i20106/Main_1.scala new file mode 100644 index 000000000000..72ba2dc59905 --- /dev/null +++ b/tests/run-macros/i20106/Main_1.scala @@ -0,0 +1,14 @@ +/** + * my doc string for Main + * + * @param tracks track listing + * */ + +class Main(tracks: String) + +object Main { + def printdoc(): Unit = { + val docString = Doc.of[Main] + println(docString) + } +} diff --git a/tests/run-macros/i20106/Test_2.scala b/tests/run-macros/i20106/Test_2.scala new file mode 100644 index 000000000000..5e6954c2d7c1 --- /dev/null +++ b/tests/run-macros/i20106/Test_2.scala @@ -0,0 +1,19 @@ +/** + * Doc of Test + * */ +class Test + +object Test { + def main(args: Array[String]): Unit = { + println("docs from same compilation run:") + Main.printdoc() + + println("docs loaded from tasty:") + val docString = Doc.of[Main] + println(docString) + + println("docs from same compilation run:") + val docStringTest = Doc.of[Test] + println(docStringTest) + } +} From cb531f74d38627a8b1bb50b0ebc012b8b0e0daa2 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 11 Apr 2024 09:46:34 +0200 Subject: [PATCH 2/3] Do not set doc string on tree when unpickling Reading it from the tree would be unreliable as doc might not have been loaded yet. Doc stings should be accessed from the `ContextDocstrings`. --- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index c32faa6458e1..10d2036a5ba9 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -982,10 +982,7 @@ class TreeUnpickler(reader: TastyReader, sym.info = ta.avoidPrivateLeaks(sym) commentUnpicklerOpt.foreach { commentUnpickler => - def loadComment() = - val comment = commentUnpickler.commentAt(start) - tree.setComment(comment) // TODO should this be set? - comment + def loadComment() = commentUnpickler.commentAt(start) if ctx.settings.YreadDocsEagerly.value then ctx.docCtx.get.addDocstring(tree.symbol, loadComment()) else From 0dd8b0230eb08ce597a35ec5adac3ff40f41903a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 11 Apr 2024 11:22:39 +0200 Subject: [PATCH 3/3] Optimize `addDocstring` --- compiler/src/dotty/tools/dotc/core/Comments.scala | 4 ++-- compiler/src/dotty/tools/dotc/core/Definitions.scala | 2 +- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Docstrings.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Comments.scala b/compiler/src/dotty/tools/dotc/core/Comments.scala index 1c40581e3835..0a48d74c05cb 100644 --- a/compiler/src/dotty/tools/dotc/core/Comments.scala +++ b/compiler/src/dotty/tools/dotc/core/Comments.scala @@ -44,8 +44,8 @@ object Comments { def registerLoader(sym: Symbol, loader: () => Option[Comment]): Unit = docstrings.update(sym, CommentLoader(loader)) - def addDocstring(sym: Symbol, doc: Option[Comment]): Unit = - doc.foreach(d => docstrings.update(sym, d)) + def addDocstring(sym: Symbol, doc: Comment): Unit = + docstrings.update(sym, doc) } diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 932a7d72d33e..3099d167c7e7 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -2169,7 +2169,7 @@ class Definitions { @tu lazy val LazyValsControlState = requiredClass(s"$LazyValsModuleName.LazyValControlState") def addSyntheticSymbolsComments(using Context): Unit = - def add(sym: Symbol, doc: String) = ctx.docCtx.foreach(_.addDocstring(sym, Some(Comment(NoSpan, doc)))) + def add(sym: Symbol, doc: String) = ctx.docCtx.foreach(_.addDocstring(sym, Comment(NoSpan, doc))) add(AnyClass, """/** Class `Any` is the root of the Scala class hierarchy. Every class in a Scala diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 10d2036a5ba9..f37861b35341 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -984,7 +984,7 @@ class TreeUnpickler(reader: TastyReader, commentUnpicklerOpt.foreach { commentUnpickler => def loadComment() = commentUnpickler.commentAt(start) if ctx.settings.YreadDocsEagerly.value then - ctx.docCtx.get.addDocstring(tree.symbol, loadComment()) + loadComment().foreach(doc => ctx.docCtx.get.addDocstring(tree.symbol, doc)) else ctx.docCtx.get.registerLoader(tree.symbol, loadComment) } diff --git a/compiler/src/dotty/tools/dotc/typer/Docstrings.scala b/compiler/src/dotty/tools/dotc/typer/Docstrings.scala index 33ef3e85e14e..2cc24416233a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Docstrings.scala +++ b/compiler/src/dotty/tools/dotc/typer/Docstrings.scala @@ -43,7 +43,7 @@ object Docstrings { } val commentWithUsecases = expanded.copy(usecases = typedUsecases) - docCtx.addDocstring(sym, Some(commentWithUsecases)) + docCtx.addDocstring(sym, commentWithUsecases) commentWithUsecases } } @@ -52,7 +52,7 @@ object Docstrings { val tplExp = docCtx.templateExpander tplExp.defineVariables(sym) val newComment = comment.expand(tplExp.expandedDocComment(sym, owner, _)) - docCtx.addDocstring(sym, Some(newComment)) + docCtx.addDocstring(sym, newComment) newComment } diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 4831c49f91bb..6dcf6160f0d7 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -501,7 +501,7 @@ class Namer { typer: Typer => def setDocstring(sym: Symbol, tree: Tree)(using Context): Unit = tree match { case t: MemberDef if t.rawComment.isDefined => - ctx.docCtx.foreach(_.addDocstring(sym, t.rawComment)) + ctx.docCtx.foreach(_.addDocstring(sym, t.rawComment.get)) case t: ExtMethods => for meth <- t.methods.find(_.span.point == sym.span.point) do setDocstring(sym, meth)