Skip to content

Fix debug print of QuoteMatcher to work on match failure #18023

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 1 commit into from
Jul 5, 2023
Merged

Fix debug print of QuoteMatcher to work on match failure #18023

merged 1 commit into from
Jul 5, 2023

Conversation

zeptometer
Copy link
Contributor

The current logic of debug print in QuoteMatcher is intended to show debug information when a pattern match fails. However, it does not work because notMatched invokes a global escape that skips debug-print logic.

This PR proposes to catch global escape when debug=1 and show debug information.

@nicolasstucki
Copy link
Contributor

We could enable debugging using a Ydebug-macros compiler flag. We would need to change

-  private inline val debug = false
+  private val debug = ctx.settings.YdebugMacros.value

and add val YdebugMacros to ScalaSettings.scala.

@zeptometer zeptometer force-pushed the fix-quotematcher-debugprint branch 3 times, most recently from 4ce87d9 to 8100200 Compare June 21, 2023 12:33
@nicolasstucki
Copy link
Contributor

The test failed due to #18033. We need to wait until this PR is merged to re-test.

nicolasstucki
nicolasstucki reviewed Jun 22, 2023
View reviewed changes
nicolasstucki
nicolasstucki reviewed Jun 22, 2023
View reviewed changes
@zeptometer zeptometer force-pushed the fix-quotematcher-debugprint branch 2 times, most recently from 00e85aa to 9e379d1 Compare June 23, 2023 05:41
@zeptometer zeptometer marked this pull request as ready for review June 26, 2023 07:04
@nicolasstucki nicolasstucki self-assigned this Jun 26, 2023
@nicolasstucki
Copy link
Contributor

Still requires the

 private val debug = ctx.settings.YdebugMacros.value

@zeptometer
Copy link
Contributor Author

For this, where can we get ctx?

private val debug = ctx.settings.YdebugMacros.value

@nicolasstucki
Copy link
Contributor

For this, where can we get ctx?

I overlooked that. We could make this change.

- object QuoteMatcher:
+ class QuoteMatcher(debug: Boolean):

And then, create the quote matcher using tx.settings.YdebugMacros.value.

@zeptometer
Copy link
Contributor Author

zeptometer commented Jul 5, 2023 •
edited
Loading

I made a change so that QuoteMatcher hold a value for debug print f744a13. Does it look good to you? @nicolasstucki
(if so, I'll squash commits)

nicolasstucki
nicolasstucki reviewed Jul 5, 2023
View reviewed changes
Copy link
Contributor

@nicolasstucki nicolasstucki 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/scala/quoted/runtime/impl/QuotesImpl.scala Outdated

extension [T](self: scala.quoted.Expr[T])
def show: String =
reflect.Printer.TreeCode.show(reflect.asTerm(self))

def matches(that: scala.quoted.Expr[Any]): Boolean =
QuoteMatcher.treeMatch(reflect.asTerm(self), reflect.asTerm(that)).nonEmpty
quoteMatcher.treeMatch(reflect.asTerm(self), reflect.asTerm(that)).nonEmpty
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
quoteMatcher.treeMatch(reflect.asTerm(self), reflect.asTerm(that)).nonEmpty
QuoteMatcher(yDebugMacros).treeMatch(reflect.asTerm(self), reflect.asTerm(that)).nonEmpty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For future reference, can I ask the reason for this change? Is it the case where creating QuoteMatcher instance for each method call is not very costful?

@zeptometer
Introduce a new compiler option -Ydebug-macros
be0c98a 
This compiler option shows debug information when quote pattern matching fails.
This commmit also fix a bug of current imiplementation of debug print
@zeptometer zeptometer force-pushed the fix-quotematcher-debugprint branch from 9729d35 to be0c98a Compare July 5, 2023 08:14
@zeptometer
Copy link
Contributor Author

Thank you! I've applied your suggestions and made a squashed commit.

@nicolasstucki nicolasstucki enabled auto-merge July 5, 2023 08:26
@nicolasstucki nicolasstucki merged commit 4e38a03 into scala:main Jul 5, 2023
@Kordyjan Kordyjan mentioned this pull request Nov 23, 2023
Merged
Kordyjan added a commit that referenced this pull request Dec 8, 2023
@Kordyjan
Backport "Fix debug print of QuoteMatcher to work on match failure" t…
aa5637f 
…o LTS (#19057)

Backports #18023 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
@Kordyjan Kordyjan added this to the 3.3.2 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolasstucki nicolasstucki nicolasstucki approved these changes

Assignees

@nicolasstucki nicolasstucki

Labels
None yet
Projects
None yet
Milestone
3.3.2
Development

Successfully merging this pull request may close these issues.
3 participants
@zeptometer @nicolasstucki @Kordyjan