-
Notifications
You must be signed in to change notification settings - Fork 613
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
Fix stack trace trimming across Driver/ChiselStage #1771
Conversation
Any Driver work here is going to conflict with #1730, any thoughts @seldridge and @sequencer? |
I think if Driver will be dropped at last. So let's get #1730 merged first? |
4d109e2
to
c843f6e
Compare
69103c6
to
5f6620d
Compare
There are some weird interactions with capturing |
5f6620d
to
65460ef
Compare
Manual backport to 3.4.x is here: #1773. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic, I have a few comments I'd like answered but the tests are great and I love the simplified and unified functionality
65460ef
to
4ff512e
Compare
Ready for re-review. Also, the backport (#1773) should be reviewed, too. I'm currently maintaining the backport and then cherry-picking onto master for this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice work.
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Fix bug in stack trace trimming behavior. Now, the following is what happens: 1. The Builder, if catching accumulated errors, will now throw a ChiselException with a Scala-trimmed Stack trace. Previously, this would throw the full excpetion. 2. The Elaborate phase handles stack trace trimming. By default, any Throwable thrown during elaboration will have its stack trace *mutably* trimmed and is rethrown. A logger.error is printed stating that there was an error during elaboration and how the user can turn on the full stack trace. If the --full-stacktrace option is on, then the Throwable is not caught and only the first logger.error (saying that elaboration failed) will be printed. 3. ChiselStage (the class), ChiselStage$ (the object), and ChiselMain all inherit the behavior of (2). Mutable stack trace trimming behavior is moved into an implicit class (previously this was defined on ChiselException only) so this can be applied to any Throwable. No StageErrors are now thrown anymore. However, StageErrors may still be caught by ChiselMain (since it is a StageMain). Testing is added for ChiselMain, ChiselStage, and ChiselStage$ to test all this behavior. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
1e92374
to
edfbcad
Compare
Fix bug in stack trace trimming behavior. Now, the following is what
happens:
ChiselException with a Scala-trimmed Stack trace. Previously, this
would throw the full excpetion.
Throwable thrown during elaboration will have its stack
trace mutably trimmed and is rethrown. A logger.error is printed
stating that there was an error during elaboration and how the user
can turn on the full stack trace. If the --full-stacktrace option
is on, then the Throwable is not caught and only the first
logger.error (saying that elaboration failed) will be printed.
all inherit the behavior of (2).
Mutable stack trace trimming behavior is moved into an implicit
class (previously this was defined on ChiselException only) so this
can be applied to any Throwable.
No StageErrors are now thrown anymore. However, StageErrors may still
be caught by ChiselMain (since it is a StageMain).
Testing is added for ChiselMain, ChiselStage, and ChiselStage$ to test
all this behavior.
Fixes #1763. (I think...)
Contributor Checklist
docs/src
?Type of Improvement
API Impact
None for actual user-facing API. Some exception behavior is changed.
Backend Code Generation Impact
None.
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
Please Merge
?