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.

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

For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug) #9824

Merged
merged 1 commit into from Feb 23, 2022

Conversation

tribbloid
Copy link
Contributor

@tribbloid tribbloid commented Dec 8, 2021

-Vdebug-type-error prints a stacktrace if an error is discovered inside a typer context

-Yissue-debug has been removed because it has no effect on compiler's behavior

@scala-jenkins scala-jenkins added this to the 2.13.8 milestone Dec 8, 2021
@tribbloid
Copy link
Contributor Author

Oops, looks like my test condition is too rigorous (the stacktrace has to be identical every time), is there a way in DirectTest to make validation looser? (e.g. by only matching the first few rows)

@tribbloid tribbloid changed the title Add "-Ydebug-error" option and delete "-Yissue-debug" option to scala compiler Add "-Ydebug-error" option and delete "-Yissue-debug" option of scala compiler Dec 8, 2021
@tribbloid
Copy link
Contributor Author

muhaha, found the solution (filter: header). Should pass this time

@tribbloid
Copy link
Contributor Author

Nice, all passed. I only have one test case:

object Example
{
  val a: org.dummy.Dummy = ???
}

@lrytz @ira19921 : should I add more?

@som-snytt
Copy link
Contributor

som-snytt commented Dec 9, 2021

I have forgotten again how -Wconf works (sorry @lrytz !) but a different idea is to make tracing an action like "silent" so that I could specify a Wconf that shows a trace, -Wconf:cat=something:t would mean show me a compiler stack trace when that message is issued. Then an option -Werror-trace (which I have renamed, apologies in advance) would just augment the -Wconf config.

The problem is that warnings are issued lazily, so capturing stack trace context might require a few LOC.

Whether to use -Y or -W suggests asking if it is "private to EPFL" or "a useful tool for warning about things". I propose the latter. Maybe rewrite all the options as -U if they are useful (kidding of course!).

@lrytz
Copy link
Member

lrytz commented Dec 15, 2021

I don't think a compiler stack trace should be part of Wconf, it's something that should only be useful to compiler (and maybe plugin) devs. So in principle -Y seems good, though the other debug options we have are -V.

So I'd go for -Vdebug-typer-errors (debug-error seems a bit too generic).

@dwijnand dwijnand modified the milestones: 2.13.8, 2.13.9 Dec 15, 2021
@SethTisue SethTisue modified the milestones: 2.13.8, 2.13.9 Dec 15, 2021
@tribbloid
Copy link
Contributor Author

tribbloid commented Dec 15, 2021 via email

@lrytz
Copy link
Member

lrytz commented Dec 16, 2021

dotty is already using -Ydebug-error

I wasn't aware, thanks. Unfortunately there's still mismatch between the flags of Scala 2 and 3. A cleanup of Scala 2 flags (which introduced -V) happened after Scala 3 forked off, and that's still missing.

So either we align with Scala 3 but use the old scheme (-Y), or we align with the other Scala 2 flags and use the new scheme (-V). Since this flag is not intended for wide use or cross-building, I think we better do the latter.

@som-snytt
Copy link
Contributor

FWIW, my rule for -V: was it something a user might enable? -Vtyper? It's a category of verbose options to ask for more output, even if it's just to post it in a ticket. Then -Y is for behaviors.

@tribbloid
Copy link
Contributor Author

@lrytz sounds like a good idea. should I ask the feature owner from dotty team for their roadmap? (Or you have already done so)

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Jan 19, 2022
@SethTisue
Copy link
Member

I'm firmly in favor of the -V route here. Alignment can come later on the Scala 3 side.

@tribbloid Since it seems this will be merged, and it'll be in the release notes, can you write a nice user-facing PR description?

@tribbloid
Copy link
Contributor Author

@SethTisue sorry for the late reply. Yes I'll do BOTH immediately.

I trust your roadmap and coordination with the Scala 3 team.

@tribbloid tribbloid force-pushed the YDebugError/dev1 branch 2 times, most recently from 2242a7d to 3396718 Compare February 19, 2022 22:21
add a test to ensure that it works

remove an obsolete option
@tribbloid tribbloid changed the title Add "-Ydebug-error" option and delete "-Yissue-debug" option of scala compiler Remove "-Yissue-debug" and add "-Vdebug-type-error" compiler option, which print stacktrace if an error is discovered inside a typer context Feb 19, 2022
@tribbloid
Copy link
Contributor Author

DONE. Hope the new PR title meets your standard.

I still have 2 questions:

  1. In ScalaSettings, Should the withAbbreviation "-Ydebug-type-error" be added?
  2. In ScalaOptionParser.booleanSettingNames, only the token "-Ydebug" is defined, should it also include "-Ydebug-tasty" which is another abbreviation?

@SethTisue SethTisue changed the title Remove "-Yissue-debug" and add "-Vdebug-type-error" compiler option, which print stacktrace if an error is discovered inside a typer context For troubleshooting compiler, remove "-Yissue-debug" add "-Vdebug-type-error" Feb 20, 2022
@SethTisue SethTisue changed the title For troubleshooting compiler, remove "-Yissue-debug" add "-Vdebug-type-error" For troubleshooting compiler, remove -Yissue-debug add -Vdebug-type-error Feb 20, 2022
@SethTisue SethTisue changed the title For troubleshooting compiler, remove -Yissue-debug add -Vdebug-type-error For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug) Feb 20, 2022
@SethTisue
Copy link
Member

Can you edit the PR description to indicate why -Yissue-debug was removed?

@tribbloid
Copy link
Contributor Author

tribbloid commented Feb 20, 2022 via email

@tribbloid tribbloid changed the title For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug) For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug option because it has no effect on compiler's behaviour) Feb 20, 2022
@tribbloid
Copy link
Contributor Author

@SethTisue thanks a lot, hope it looks ready now

@SethTisue
Copy link
Member

SethTisue commented Feb 23, 2022

I think we've been making edits at cross-purposes because I haven't been precise in my wording. I'm not sure what the official terminology is, but to me the "PR description" is the top comment, separately from the PR's "title".

We like to keep the title short and sweet, and then explain further in the description.

@SethTisue SethTisue changed the title For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug option because it has no effect on compiler's behaviour) For troubleshooting compiler, add -Vdebug-type-error (and remove -Yissue-debug) Feb 23, 2022
@SethTisue SethTisue merged commit 99de462 into scala:2.13.x Feb 23, 2022
@SethTisue
Copy link
Member

SethTisue commented Feb 23, 2022

I think the title is now good, but the description could use more explanation and perhaps an example. Imagine you're a random Scala programmer and you've never heard of this new flag before and you have no idea whether you even need to know about it, and when you might want to use it, and what it will do if enabled.

Who is this new flag useful for? Is it only useful to compiler hackers? If so, you should say that (and we probably won't release-note it). Note that "Inside a typer context" isn't meaningful to the average Scala programmer, that's compiler-hacker talk.

The PR description should explain all of that, at least briefly. You can go on at length at you want, but the minimum we're looking for is a paragraph or two that answers these questions.

If it's only useful to compiler hackers, less documentation is needed. But if it's sometimes useful to end users, it's important to write something that is clear for that audience.

@som-snytt
Copy link
Contributor

I guess the difference between -V and -Y is that Seth goads us into producing doc for -V.

@tribbloid
Copy link
Contributor Author

Sorry hope it is not too late to change the PR description (wasn't able to find that option in github web UI).

@som-snytt I already have a short description in the code:

  val VdebugTypeError = BooleanSetting("-Vdebug-type-error", "Print the stack trace when any error is caught.") withAbbreviation "-Ydebug-type-error"

Other descriptions in the sourcecode are identical to the doc at https://docs.scala-lang.org/overviews/compiler-options/index.html. SO I assume that the later was compiled from the former. I wasn't able to successfully Google any longer version. So at this moment I'm going to assume that it will only exist in the PR or the release note

@SethTisue
Copy link
Member

hope it is not too late to change the PR description

not too late! you should see a pencil icon, like this:

Screen Shot 2022-02-23 at 1 29 43 PM

if you don't, that could be because I'm running Refined GitHub, so I'm never sure whether what I'm seeing was tweaked by that or not. but even if you don't see the pencil icon, there should be an "Edit" option in the little "..." menu next to it

So at this moment I'm going to assume that it will only exist in the PR or the release note

in the long run, hopefully we'll copy the PR description into the compiler options doc, but our automation for updating that doc isn't working currently, as per scala/docs.scala-lang#1711, so at the moment there's no point in doing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
6 participants