Skip to content
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

Better stack trace #599

Merged
merged 151 commits into from
Jan 4, 2024
Merged

Better stack trace #599

merged 151 commits into from
Jan 4, 2024

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented Sep 27, 2023

No description provided.

@adpi2 adpi2 force-pushed the better-stack-trace branch 2 times, most recently from d37c377 to 9e45477 Compare October 5, 2023 09:10
@adpi2 adpi2 mentioned this pull request Oct 12, 2023
Open
iusildra
iusildra reviewed Oct 14, 2023
View reviewed changes
Copy link
Contributor

@iusildra iusildra left a comment

Choose a reason for hiding this comment

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

That's a big one 👏
Just to be sure, you use SAM for Single Abstract Method ?

...ckler/src/main/scala/ch/epfl/scala/debugadapter/internal/javareflect/JavaReflectLoader.scala Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/jdi/JdiMethod.scala Outdated Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/jdi/JdiReferenceType.scala Show resolved Hide resolved
...s/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/BinarySymbol.scala Outdated Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/binary/Symbol.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
iusildra
iusildra reviewed Oct 31, 2023
View reviewed changes
Copy link
Contributor

@iusildra iusildra left a comment

Choose a reason for hiding this comment

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

Getting better! Was it able to find some of the "not found" methods ?

I wonder if it's possible to group all the case classes by "kind" or by a property (maybe with several traits?) to reduce the number of case. There seems to be some patterns but can't tell if it's just a particular case or not

modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/binary/Method.scala Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/binary/Symbol.scala Outdated Show resolved Hide resolved
...ickler/src/main/scala/ch/epfl/scala/debugadapter/internal/javareflect/JavaReflectClass.scala Outdated Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/jdi/JdiReferenceType.scala Outdated Show resolved Hide resolved
modules/unpickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Patterns.scala Outdated
method match
case ByNameArgProxy() => None
case DefaultArg(_) => None
case _ => method.extractFromDecodedNames("(.+)\\$\\d+".r)(_(0))
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 use triple quotes, so you don't need to put this much \ 😀

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Unpickler.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Formatter.scala Outdated Show resolved Hide resolved
...npickler/src/main/scala/ch/epfl/scala/debugadapter/internal/stacktrace/Scala3Formatter.scala Outdated
Comment on lines 48 to 66
case BinaryMethod(_, sym) => formatName(sym)
case BinaryLocalLazyInit(_, sym) => formatName(sym).dot("<lazy init>")
case BinaryLazyInit(_, sym) => formatName(sym).dot("<lazy init>")
case BinaryTraitParamAccessor(_, sym) => formatName(sym)
case BinaryMixinForwarder(_, target) => formatName(target).dot("<mixin forwarder>")
case BinaryTraitStaticForwarder(target) => formatName(target).dot("<static forwarder>")
case _: BinaryOuter => "<outer>"
case BinarySuperArg(_, init, _) => formatName(init).dot("<super arg>")
case BinaryLiftedTry(_, _) => "<try>"
case _: BinaryByNameArg => "<by-name arg>"
case BinaryMethodBridge(target, _) => formatName(target).dot("<bridge>")
case BinaryAnonOverride(_, overridden, _) => formatName(overridden)
case BinaryStaticForwarder(_, target, _) => formatName(target).dot("<static forwarder>")
case _: BinaryDeserializeLambda => "$deserializeLambda$"
case BinarySetter(_, sym, _) => if sym.isMethod then formatName(sym) else formatName(sym) + "_="
case BinarySuperAccessor(_, sym, _) => formatName(sym).dot("<super>")
case BinarySpecializedMethod(_, sym) => formatName(sym).dot("<specialized>")
case BinaryInlineAccessor(_, target) => s"<inline ${formatOwner(target).dot(formatName(target))}>"
case BinaryAdaptedFun(target) => formatName(target).dot("<adapted>")
Copy link
Contributor

Choose a reason for hiding this comment

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

How about having a field stackTraceInfo = "<adapted" | "<lazy init>" |... in BinaryMethodSymbol
Maybe with some trait LocalMethodSymbol and XXXMethodSymbol you could do something like

case m: LocalMethodSymbol => formatQualifiedName(m) // <=> formatQualifiedName(TermSymbol, String)
case m: XXXMethodSymbol => formatQualifiedName(m) // <=> formatQualifiedName(BinaryClassSymbol, TermSymbol, String)
case m: // other special cases such as BinaryOuter

@adpi2
Use source lines to disambiguate classes
8c28208
@adpi2
Reduce ambiguity of local symbols by filtering out enclosing symbols
d9edb66
@adpi2
fix collectLocalMethods
f714d9f
@adpi2
Introduce JavaReflectLoader to load source lines lazily
8e458a5
@adpi2
refacto
6a6c0a5
@adpi2
Trim inline source lines
c7ecef4
@adpi2
bump tasty-query
ee48d04
@adpi2
refacto
29909ca
@adpi2
Add support for synthetic companion class
a8ba129
@adpi2
Add support for adapted anon fun
907ad64
@adpi2
Fix compilation + add test on adapted anon fun
ea3064e
@adpi2
Bump tasty-query to fix many not-found
e2467f4
@adpi2
Handle superArg methods
1437239
@adpi2
Remove inlined lines from local methods
71b2637
@adpi2
Inlined local def can have more precise type
8994fde
@adpi2
Fix decoding name of local def
b72fcb1
@adpi2
Find methods which return a context function
6ee443c
@adpi2
Find trait param accessors
e35bd1e
@adpi2
Find lifted try
40a3634
@adpi2
traverse all tree of ClassDef
bfb07c3
@adpi2
fix tests
39e7fbf
@adpi2
Find by-name arg
8bf92fc
@adpi2
Bump TASTy Query to fix sig of local module classes
72cb57e
@adpi2
Check return type of non-method symbols
5c6db9c
@adpi2 adpi2 force-pushed the better-stack-trace branch 6 times, most recently from b259168 to 0991ca3 Compare January 3, 2024 14:57
@adpi2 adpi2 force-pushed the better-stack-trace branch 8 times, most recently from ec0fd47 to 1e76685 Compare January 4, 2024 12:44
@adpi2 adpi2 marked this pull request as ready for review January 4, 2024 13:41
@adpi2 adpi2 merged commit 20ff51a into scalacenter:main Jan 4, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iusildra iusildra iusildra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adpi2 @iusildra @sjrd