Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Improvements to-Vimplicits; errors now show complete implicit search tree#9944

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
SethTisue merged 1 commit intoscala:2.13.xfromtribbloid:splain1Backport/dev1
Mar 13, 2023

Conversation

@tribbloid
Copy link

@tribbloidtribbloid commentedFeb 19, 2022•
edited
Loading

Backports bugfixes from splain 0.5.8 & 1.0.0

The new algorithm now build a tree instead of an indented list that records implicit search process. As a result, implicit terms being considered multiple times in a search will be displayed repeatedly.

E.g. for the following scala code:

object tpes{  trait I1  trait I2  trait I3  trait I4  trait I5  trait I6  trait I7  trait I8  trait I9}import tpes._object Tree{  implicit def i8(implicit p: I9): I8 = ???  implicit def i7(implicit p: I8): I7 = ???  implicit def i6a(implicit p: I7): I6 = ???  implicit def i6b(implicit p: I8): I6 = ???  implicit def i5(implicit p: I6): I5 = ???  implicit def i4(implicit p: I5): I4 = ???  implicit def i3a(implicit p: I4): I3 = ???  implicit def i3b(implicit p: I4): I3 = ???  implicit def i2(implicit p: I3): I2 = ???  implicit def i1a(implicit p: I2): I1 = ???  implicit def i1b(implicit p: I6): I1 = ???  implicitly[I1]}

The old error message can only show termi6b once, this is problematic asi6b: I6 was summoned for multiplep: I6 arguments:

newSource1.scala:28: error: implicit error;!I e: tpes.I1i1a invalid because!I p: tpes.I2――i2 invalid because  !I p: tpes.I3――――i3a invalid because    !I p: tpes.I4――――――i4 invalid because      !I p: tpes.I5――――――――i5 invalid because        !I p: tpes.I6――――――――――i6a invalid because          !I p: tpes.I7――――――――――――i7 invalid because            !I p: tpes.I8――――――――――――――i8 invalid because              !I p: tpes.I9――――――――――i6b invalid because          !I p: tpes.I8――――――――――――i8 invalid because            !I p: tpes.I9――――i3b invalid because    !I p: tpes.I4――――――i4 invalid because      !I p: tpes.I5――――――――i5 invalid because        !I p: tpes.I6――――――――――i6a invalid because          !I p: tpes.I7――――――――――――i7 invalid because            !I p: tpes.I8――――――――――――――i8 invalid because              !I p: tpes.I9i1b invalid because!I p: tpes.I6――i6a invalid because  !I p: tpes.I7――――i7 invalid because    !I p: tpes.I8――――――i8 invalid because      !I p: tpes.I9  implicitly[I1]            ^

after this patch, the error message becomes:

newSource1.scala:28: error: implicit error;!I e: tpes.I1i1a invalid because!I p: tpes.I2――i2 invalid because  !I p: tpes.I3――――i3a invalid because    !I p: tpes.I4――――――i4 invalid because      !I p: tpes.I5――――――――i5 invalid because        !I p: tpes.I6――――――――――i6a invalid because          !I p: tpes.I7――――――――――――i7 invalid because            !I p: tpes.I8――――――――――――――i8 invalid because              !I p: tpes.I9――――――――――i6b invalid because          !I p: tpes.I8――――――――――――i8 invalid because            !I p: tpes.I9――――i3b invalid because    !I p: tpes.I4――――――i4 invalid because      !I p: tpes.I5――――――――i5 invalid because        !I p: tpes.I6――――――――――i6a invalid because          !I p: tpes.I7――――――――――――i7 invalid because            !I p: tpes.I8――――――――――――――i8 invalid because              !I p: tpes.I9――――――――――i6b invalid because          !I p: tpes.I8――――――――――――i8 invalid because            !I p: tpes.I9i1b invalid because!I p: tpes.I6――i6a invalid because  !I p: tpes.I7――――i7 invalid because    !I p: tpes.I8――――――i8 invalid because      !I p: tpes.I9――i6b invalid because  !I p: tpes.I8――――i8 invalid because    !I p: tpes.I9  implicitly[I1]            ^

which shows multiple summoning attempts ofi6b correctly

@scala-jenkinsscala-jenkins added this to the2.13.9 milestoneFeb 19, 2022
@tribbloidtribbloid marked this pull request as draftFebruary 19, 2022 23:01
@SethTisue
Copy link
Member

@tribbloid interested in returning to this?

@tribbloid
Copy link
Author

tribbloid commentedApr 21, 2022•
edited by SethTisue
Loading

Of course. I'm just fixing a bug for splain 1.0

SethTisue and He-Pin reacted with thumbs up emoji

@SethTisueSethTisue modified the milestones:2.13.9,2.13.10Apr 25, 2022
@lrytzlrytz modified the milestones:2.13.10,2.13.11Sep 26, 2022
@SethTisue
Copy link
Member

@tribbloid interested in returning to this?

@tribbloid
Copy link
Author

Yes, don't want to drag it any longer, it will be 2.13.11 or monastery

SethTisue and mihaisoloi reacted with laugh emoji

@SethTisue
Copy link
Member

I hear that a choice cell just opened up at the monastery ;-)

@tribbloid
Copy link
Author

NOOOO you can't just drag me in without me submitting it!

Seriously when will be the feature freeze date? I'll just need a week to wrap it up, with all test cases

@SethTisue
Copy link
Member

when will be the feature freeze date

we're not exactly sure. it's not imminent. but we're coming up on the 4 month mark since 2.13.10, and it would be highly unusual of us to let more than 6 months pass, so...

you're safe for at least 2 or 3 weeks, say. after that, can't predict.

tribbloid reacted with thumbs up emoji

@tribbloid
Copy link
Author

Backport almost finished and passed partest locally. Let's wait for the CI

Still, this doesn't rule out the monastery as there may be numerous problems in downstream testing, format compliance & release schedule.

@SethTisue
Copy link
Member

(CI likes the last commit, but you'll need to rebase such that every commit passes — that's the "combined" check.)

tribbloid reacted with thumbs up emoji

tribbloid pushed a commit to tribbloid/scala that referenced this pull requestFeb 14, 2023
@tribbloid
Copy link
Author

Sorry for being slow, Travis CI is getting a bit squishy, got "504 Gateway Time-out" in my last run.

@SethTisue
Copy link
Member

SethTisue commentedFeb 14, 2023•
edited
Loading

Don't worry about the Travis-CI failure — it's unrelated to your changes. I've openedscala/scala-dev#832 on it

Let us know if you consider this ready for review

tribbloid reacted with heart emoji

@tribbloid
Copy link
Author

thanks a lot! Will remove "Draft" tag once I'm comfortable

@tribbloid
Copy link
Author

Finished reviewing, the only issue left is formatting difference. Is there a formatter configuration I could use to enforce it automatically?

tribbloid pushed a commit to tribbloid/scala that referenced this pull requestFeb 16, 2023
@tribbloidtribbloid changed the title[work in progress, DO NOT MERGE] Backport bugfixes from splain 0.5.8-SNAPSHOT & 1.0.0Backport bugfixes from splain 0.5.8-SNAPSHOT & 1.0.0Feb 16, 2023
@tribbloidtribbloid marked this pull request as ready for reviewFebruary 16, 2023 22:37
@SethTisue
Copy link
Member

We don't have a repo-wide style, no. We try to match a file's existing style when tweaking existing code, prioritizing minimal/reviewable diffs. But if you're really tearing something up you can suit yourself as long as it's within the (admittedly nebulous) boundaries of common, consensus style.

tribbloid reacted with thumbs up emoji

@SethTisue
Copy link
Member

Oh, one thing we'll want before merge is to have the PR title and description be your best effort at documenting these changes from the user's point of view.

tribbloid reacted with thumbs up emoji

@tribbloidtribbloid changed the titleBackport bugfixes from splain 0.5.8-SNAPSHOT & 1.0.0Backport bugfixes from splain 0.5.8 & 1.0.0, errors from-VImplicits now show complete implicit search treeFeb 20, 2023
@tribbloid
Copy link
Author

Oh, one thing we'll want before merge is to have the PR title and description be your best effort at documenting these changes from the user's point of view.

I believe it has been done.@SethTisue Can we start the review if it is up to your standard?

@tribbloid
Copy link
Author

This patch only synchronise behaviour & test cases of all features of splain-plugin before 1.x. New features are not included

tek reacted with thumbs up emoji

@SethTisueSethTisue changed the titleBackport bugfixes from splain 0.5.8 & 1.0.0, errors from-VImplicits now show complete implicit search treeImprovements to-Vimplicits; errors now show complete implicit search treeFeb 28, 2023
@SethTisue
Copy link
Member

SethTisue commentedFeb 28, 2023•
edited
Loading

I revised the PR title and description a bit further, but I think the PR description could use further work, both for reviewers and for end users. Can you expand it to show an example of the improved behavior? "Suppose we're debugging the following error. The following implicit search tree is now produced, and it shows that the problem is X", something like that maybe?

tribbloid reacted with thumbs up emoji

@tribbloid
Copy link
Author

sounds good, updated

SethTisue reacted with thumbs up emoji

@tribbloid
Copy link
Author

Any plan to integrate it before the deadline?

@SethTisueSethTisue added the prio:hihigh priority (used only by core team, only near release time) labelMar 8, 2023
@SethTisue
Copy link
Member

I don't anticipate any trouble with this making 2.13.11

tribbloid reacted with thumbs up emojitribbloid reacted with heart emoji

@SethTisueSethTisue merged commitf3bfc05 intoscala:2.13.xMar 13, 2023
@SethTisue
Copy link
Member

thank you!

tribbloid reacted with laugh emoji

@SethTisueSethTisue added release-notesworth highlighting in next release notes and removed prio:hihigh priority (used only by core team, only near release time) labelsMay 3, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

release-notesworth highlighting in next release notes

Projects

None yet

Milestone

2.13.11

Development

Successfully merging this pull request may close these issues.

4 participants

@tribbloid@SethTisue@lrytz@scala-jenkins
[8]ページ先頭
©2009-2025 Movatter.jp