Skip to content

-Ylog:X now only log phase X, use -Ylog:X+ to also log phase X+1 #308

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 1 commit into from
Dec 18, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/dotty/tools/dotc/config/CompilerCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ object CompilerCommand extends DotClass {
|Where multiple values are accepted, they should be comma-separated.
| example: -Xplugin:plugin1,plugin2
|<phases> means one or a comma-separated list of:
| (partial) phase names, phase ids, phase id ranges, or the string "all".
| - (partial) phase names with an optional "+" suffix to include the next phase
| - the string "all"
| example: -Xprint:all prints all phases.
| example: -Xprint:expl,24-26 prints phases explicitouter, closelim, dce, jvm.
| example: -Xprint:-4 prints only the phases up to typer.
|
| example: -Xprint:front,mixin prints the frontend and mixin phases.
| example: -Ylog:erasure+ logs the erasure phase and the phase after the erasure phase.
| This is useful because during the tree transform of phase X, we often
| already are in phase X+1.
""".stripMargin.trim + "\n"

def shortUsage = s"Usage: $cmdName <options> <source files>"
Expand Down
19 changes: 13 additions & 6 deletions src/dotty/tools/dotc/core/Decorators.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,22 @@ object Decorators {
def show(implicit ctx: Context) = text.mkString(ctx.settings.pageWidth.value)
}

/** Implements a test whether a list of strings representing phases contains
* a given phase. The test returns true if the given phase starts with
* one of the names in the list of strings, or if the list of strings
* contains "all".
/** Test whether a list of strings representing phases contains
* a given phase. See [[config.CompilerCommand#explainAdvanced]] for the
* exact meaning of "contains" here.
*/
implicit class PhaseListDecorator(val names: List[String]) extends AnyVal {
def containsPhase(phase: Phase): Boolean = phase match {
def containsPhase(phase: Phase): Boolean = phase match {
case phase: TreeTransformer => phase.transformations.exists(trans => containsPhase(trans.phase))
case _ => names exists (n => n == "all" || phase.phaseName.startsWith(n))
case _ =>
names exists { name =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the logic can be simplified and made clearer as follows:

case _ => 
    names exists { n =>
       n == "all" || {
          val strippedName = name.stripSuffix("+")
          val indicatedPhase = if (name == strippedName) phase else phase.prev
          indicatedPhase.phaseName.startsWith(strippedName)
      }
   }  

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that doesn't work, if we have a "+" we want to match on both phase and phase.prev, we could use a list but that seems overkill, what do you think of:

case _ => 
    names exists { n =>
       n == "all" || {
          val strippedName = name.stripSuffix("+")
          val logNextPhase = name ne strippedName
          phase.phaseName.startsWith(strippedName) ||
            (logNextPhase && phase.prev.phaseName.startsWith(strippedName))
      }
   }

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I thought of a different spec. But yours makes more sense, I think. The change looks good.

name == "all" || {
val strippedName = name.stripSuffix("+")
val logNextPhase = name ne strippedName
phase.phaseName.startsWith(strippedName) ||
(logNextPhase && phase.prev.phaseName.startsWith(strippedName))
}
}
}
}

Expand Down
12 changes: 4 additions & 8 deletions src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,12 @@ trait Reporting { this: Context =>
def incompleteInputError(msg: String, pos: SourcePosition = NoSourcePosition)(implicit ctx: Context): Unit =
reporter.incomplete(new Error(msg, pos))(ctx)

/** Log msg if current phase or its precedessor is mentioned in
* settings.log.
* The reason we also pick the predecessor is that during the
* tree transform of phase X, we often are already in phase X+1.
* It's convenient to have logging work independently of whether
* we have advanced the phase or not.
/** Log msg if settings.log contains the current phase.
* See [[config.CompilerCommand#explainAdvanced]] for the exact meaning of
* "contains" here.
*/
def log(msg: => String): Unit =
if (this.settings.log.value.containsPhase(phase) ||
this.settings.log.value.containsPhase(phase.prev))
if (this.settings.log.value.containsPhase(phase))
echo(s"[log ${ctx.phasesStack.reverse.mkString(" -> ")}] $msg")

def debuglog(msg: => String): Unit =
Expand Down