Skip to content

Test compilation of decompiled code #4266

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
May 9, 2018
Merged

Test compilation of decompiled code #4266

merged 9 commits into from
May 9, 2018

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Apr 6, 2018
@nicolasstucki nicolasstucki force-pushed the compile-decompiled-code branch 3 times, most recently from b08eff1 to 4382eae Compare April 8, 2018 09:39
@nicolasstucki nicolasstucki changed the title WIP Compile decompiled code Test compilation of decompiled code Apr 8, 2018
@nicolasstucki
Copy link
Contributor Author

Currently it only tests compilation of decompiled code for which a XYZ.decompiled file exists.

@nicolasstucki nicolasstucki removed their assignment Apr 8, 2018
@nicolasstucki
Copy link
Contributor Author

This PR also contains a partial fix for module object decompilation.

@nicolasstucki nicolasstucki force-pushed the compile-decompiled-code branch 2 times, most recently from 97ad093 to b231f93 Compare April 8, 2018 13:16
allanrenucci
allanrenucci previously approved these changes Apr 9, 2018
View reviewed changes
Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala Outdated
@@ -631,7 +631,8 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
val tparamsTxt = withEnclosingDef(constr) { tparamsText(tparams) }
val primaryConstrs = if (constr.rhs.isEmpty) Nil else constr :: Nil
val prefix: Text =
if (vparamss.isEmpty || primaryConstrs.nonEmpty) tparamsTxt
if (constr.symbol.owner.is(Module)) " "
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be the empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To print object Foo { .. } instead of object Foo{ ... }.

Copy link
Contributor

Choose a reason for hiding this comment

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

But in puzzle.decompiled I see two spaces after object Test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally fixed this one detail

compiler/src/dotty/tools/dotc/decompiler/Main.scala
new TASTYDecompiler
}

override def setup(args0: Array[String], rootCtx: Context): (List[String], Context) = {
var args = args0.filter(a => a != "-decompile")
args = if (args.contains("-from-tasty")) args else "-from-tasty" +: args
if (!args.contains("-from-tasty")) args = "-from-tasty" +: args
if (args.contains("-d")) args = "-color:never" +: args
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we print colors in the output file it is impossible to read (and probably compile) the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we do this even if args does not contain -d?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If args does not contain -d the output is printed on the terminal. There it is fine and nicer to have colors. Also copy-pasting from the terminal usually removes the colors.

compiler/src/dotty/tools/dotc/printing/DecompilerPrinter.scala Outdated
@@ -55,7 +55,11 @@ class DecompilerPrinter(_ctx: Context) extends RefinedPrinter(_ctx) {
}

override protected def toTextTemplate(impl: Template, ofNew: Boolean = false): Text = {
val impl1 = impl.copy(parents = impl.parents.filterNot(_.symbol.maybeOwner == defn.ObjectClass))
def filter(sym: Symbol): Boolean = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give a better name (e.g. isSyntheticParent)

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala Outdated
@@ -642,7 +643,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
val selfText = {
val selfName = if (self.name == nme.WILDCARD) keywordStr("this") else self.name.toString
(selfName ~ optText(self.tpt)(": " ~ _) ~ " =>").close
} provided !self.isEmpty
} provided (!self.isEmpty && !constr.symbol.owner.is(Module))
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this should go into RefinedPrinter. You could reproduce this behavior in DecompilerPrinter by replacing the self type with an EmptyTree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will move it to DecompilerPrinter

compiler/test/dotty/tools/vulpix/ParallelTesting.scala Outdated
def compileTastyInDir(f: String, flags0: TestFlags, blacklist: Set[String] = Set.empty)(
implicit testGroup: TestGroup): (CompilationTest, CompilationTest, CompilationTest) = {
def compileTastyInDir(f: String, flags0: TestFlags, blacklist: Set[String], recompileBlacklist: Set[String])(
implicit testGroup: TestGroup): (CompilationTest, CompilationTest, CompilationTest, CompilationTest) = {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably create the class TastyCompilationTest and replace the returned Tuple4

@nicolasstucki nicolasstucki dismissed allanrenucci’s stale review April 9, 2018 11:02

Needs a new review after the changes

@nicolasstucki nicolasstucki force-pushed the compile-decompiled-code branch from e0c7506 to 6971c0d Compare May 7, 2018 06:16
@allanrenucci allanrenucci force-pushed the compile-decompiled-code branch from 176c787 to 5f5f474 Compare May 8, 2018 16:57
@allanrenucci
Move more logic from RefinedPrinter to DecompilerPrinter
3301cf4 
RefinedPrinter cannot make use of symbols in order to be able to print
after all phases (e.g. Parser).
@allanrenucci allanrenucci force-pushed the compile-decompiled-code branch from 5f5f474 to 3301cf4 Compare May 8, 2018 17:33
allanrenucci
allanrenucci approved these changes May 9, 2018
View reviewed changes
Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

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

LGTM. @nicolasstucki please review last commit

@nicolasstucki
Copy link
Contributor Author

Changes by @allanrenucci LGTM

@nicolasstucki nicolasstucki merged commit d109c0b into scala:master May 9, 2018
@allanrenucci allanrenucci deleted the compile-decompiled-code branch May 9, 2018 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allanrenucci allanrenucci allanrenucci approved these changes

Assignees

@allanrenucci allanrenucci

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nicolasstucki @allanrenucci