FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Log warnings and errors to stderr instead of stdout by jackkoenig · Pull Request #5213 · chipsalliance/chisel · GitHub

/ chisel Public

Log warnings and errors to stderr instead of stdout - #5213

Open
jackkoenig wants to merge 3 commits into
mainfrom
jackkoenig/errors-to-stderr
ConversationCommits (3)ChecksFiles changed
Open

Log warnings and errors to stderr instead of stdout#5213
jackkoenig wants to merge 3 commits into
mainfrom
jackkoenig/errors-to-stderr

Conversation

jackkoenig commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

This has bothered me for a very long time. I'm wondering if this is fine on a minor version or should wait for Chisel 8...

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • 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 add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • API deprecation
  • Bugfix

Desired Merge Strategy

  • Squash

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash) and clean up the commit message.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

jackkoenig added the Deprecation Deprecates an API, will be included in release notes label Feb 24, 2026
jackkoenig requested a review from seldridge February 24, 2026 22:41
seldridge approved these changes Feb 24, 2026
View reviewed changes

seldridge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This generally makes sense to me. I think it is defensible to send logs to stdout if the tool never uses stdout for anything. However, we should be using stdout for the output FIRRTL or Verilog.

This would make Chisel much closer to a command line generator than the file-based thing that it is now. 👍

Comment thread firrtl/src/main/scala/logger/Logger.scala
Comment on lines +218 to +219
case LogLevel.Error | LogLevel.Warn => state.errorStream
case _ => state.stream

seldridge Feb 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd say put everything into errorStream. This would level Info and Debug going to stdout right?

jackkoenig Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah currently I'm having it send Warn and Error to stderr and the others to stdout, your point is a good one though...

jackkoenig mentioned this pull request Mar 2, 2026
Open
4 of 32 tasks

This branch has not been deployed

No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

seldridge seldridge approved these changes

Assignees

No one assigned

Labels

Deprecation Deprecates an API, will be included in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL