Skip to content

Hide stack traces behind -Ydebug #1057

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 4 commits into from
Feb 6, 2016

Conversation

smarter
Copy link
Member

@smarter smarter commented Feb 5, 2016

They're not very useful for end users and some tests like
tests/neg/selfreq.scala always print these exceptions which makes it
harder to read the test logs,

Also use Thread.dumpStack() instead of creating an Exception and calling
printStackTrace() on it.

Review by @odersky or @DarkDimius

They're not very useful for end users and some tests like
tests/neg/selfreq.scala always print these exceptions which makes it
harder to read the test logs,

Also use Thread.dumpStack() instead of creating an Exception and calling
printStackTrace() on it.
@smarter smarter force-pushed the fix/hide-stacktraces branch from 3fb8fb1 to 11df014 Compare February 5, 2016 01:05
@smarter
Copy link
Member Author

smarter commented Feb 5, 2016

Stack traces are useful documentation when something goes wrong. Users should paste them into their bugreports. Turning them off by default makes it harder to troubeshoot.

Look what happens when a user tries to compile tests/neg/selfreq.scala:
https://gist.github.com/anonymous/0f49ed5beb84d51f097a
200 lines of mostly useless output, the user will probably think that there is a bug in the compiler, even though this is just debug information.

I'm against disabling stacktraces in partest. They provide valuable information in case of failure.

Note that I'm not disabling stacktraces when the compiler fails with an exception, I'm only disabling stack traces that are explicitly printed in random places in the compiler with printStackTrace() or dumpStack(). In master currently we have 15 cases where this happen, 4 of these cases are hidden behind if (ctx.debug), why these 4? Don't you think that we should be consistent?

@odersky
Copy link
Contributor

odersky commented Feb 5, 2016

OK, I understand the reasoning that we want to hide these stacktraces by default because they do not signify real compiler crashes.

Can you try to do the whole test suite under -Ydebug? If that passes, we are good. If not, we need to either debug -Ydebug first, or else create a more specific option (-Ydebug:stacktraces?), or else
use a printer in Printers.scala for it. In fact it strikes me that we could generalize the cyclicErrors printer to all stack traces.

@smarter smarter force-pushed the fix/hide-stacktraces branch 2 times, most recently from 98cf50f to ee67cd9 Compare February 5, 2016 20:52
@smarter
Copy link
Member Author

smarter commented Feb 5, 2016

Can you try to do the whole test suite under -Ydebug? If that passes, we are good

This now works after dotty-staging@a6719ec and dotty-staging@ee67cd9

@DarkDimius
Copy link
Contributor

@smarter could you maintain dotty-staging@a6719ec under a -Ytrace-context-creation? It proved very useful while debugging typer&tree transforms.

This makes the compiler extremely slow. To store the trace, you now need
to pass -Ytrace-context-creation
Instead, a new setting called -Yplain-printer is used for this.
After this commit, we can now run all tests with -Ydebug (this was not
the case before because using the plain printer breaks -Ytest-pickler)
@smarter smarter force-pushed the fix/hide-stacktraces branch from ee67cd9 to b33babc Compare February 6, 2016 00:14
@smarter
Copy link
Member Author

smarter commented Feb 6, 2016

@DarkDimius : done!

@smarter
Copy link
Member Author

smarter commented Feb 6, 2016

/rebuild

odersky added a commit that referenced this pull request Feb 6, 2016
@odersky odersky merged commit d005613 into scala:master Feb 6, 2016
@allanrenucci allanrenucci deleted the fix/hide-stacktraces branch December 14, 2017 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants