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

Add JsonProtocol.serializeRecover #2227

Merged
merged 1 commit into from
May 14, 2021
Merged

Conversation

jackkoenig
Copy link
Contributor

This function will safely wrap any unserializeable annotations in
UnserializeableAnnotations so that they can be safely serialized to JSON
for logging.

Follow on to #1885, makes -ll trace much more useful if you happen to have 1 unserializable annotation making your whole log worthless.

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • [NA] Did you update the FIRRTL spec to include every new feature/behavior?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • new feature/API

API Impact

There is a new public method, JsonProtocol.serializeRecover which will always succeed because it wraps any unserializable annotations in UnserializableAnnotations with the error and .toString of the underlying Annotation as contents.

Backend Code Generation Impact

None

Desired Merge Strategy

  • Squash

Release Notes

  • Add new JsonProtocol.serializeRecover which wraps unserializable annotations in UnserializableAnnotations so that a valid JSON is always created.
  • Use this new API in --log-level trace so that the logged annotations are always useful, even when there are unserializable annotations

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (1.2.x, 1.3.0, 1.4.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

This function will safely wrap any unserializeable annotations in
UnserializeableAnnotations so that they can be safely serialized to JSON
for logging.
@jackkoenig jackkoenig added this to the 1.3.x milestone May 14, 2021
@jackkoenig jackkoenig added the Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. label May 14, 2021
@mergify mergify bot merged commit c2d72fd into master May 14, 2021
mergify bot pushed a commit that referenced this pull request May 14, 2021
This function will safely wrap any unserializeable annotations in
UnserializeableAnnotations so that they can be safely serialized to JSON
for logging.

(cherry picked from commit c2d72fd)

# Conflicts:
#	src/main/scala/firrtl/Compiler.scala
#	src/main/scala/firrtl/annotations/JsonProtocol.scala
#	src/test/scala/firrtlTests/annotationTests/JsonProtocolSpec.scala
mergify bot pushed a commit that referenced this pull request May 14, 2021
This function will safely wrap any unserializeable annotations in
UnserializeableAnnotations so that they can be safely serialized to JSON
for logging.

(cherry picked from commit c2d72fd)
@mergify mergify bot added the Backported This PR has been backported to marked stable branch label May 14, 2021
mergify bot added a commit that referenced this pull request May 14, 2021
This function will safely wrap any unserializeable annotations in
UnserializeableAnnotations so that they can be safely serialized to JSON
for logging.

(cherry picked from commit c2d72fd)

Co-authored-by: Jack Koenig <koenig@sifive.com>
@jackkoenig jackkoenig deleted the log-unserializeable-annos branch May 24, 2021 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported This PR has been backported to marked stable branch Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants