◐ Shell
reader mode source ↗
Skip to content

avoid direct usage of std::cout#499

Open
firewave wants to merge 1 commit into
cppcheck-opensource:masterfrom
firewave:ostream
Open

avoid direct usage of std::cout#499
firewave wants to merge 1 commit into
cppcheck-opensource:masterfrom
firewave:ostream

Conversation

@firewave

@firewave firewave commented Aug 15, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

danmar
danmar reviewed Aug 22, 2025
View reviewed changes

@danmar danmar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hide comment

hmm these functions are for usage in the debugger. will I be able to call these during debugging? I guess std::cout symbol is not available while I debug inside the simplecpp.

We don't need to have these functions in release builds? would that solve your itch?

@firewave

firewave commented Aug 22, 2025

Copy link
Copy Markdown
Collaborator Author

Not directly specifying any stream is good style as you do not want some "random" print something. It would also allow to use it in more contexts.

Not sure about calling it directly from the debugger since I rarely do that. I thought about adding a default parameter but I did not want to have the <iostream> include in the header.

danmar
danmar approved these changes Aug 23, 2025
View reviewed changes

@danmar danmar left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hide comment

I can still call it directly in the debugger:

(gdb) call rawtok->printAll(std::cout)

so well this complicates debugging but it works.

jcfr
jcfr reviewed Aug 26, 2025
View reviewed changes

@jcfr jcfr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Integrating the proposed API while maintaining the possibility to just call the function seems like a reasonable trade-off to finalize this.

@firewave

firewave commented Aug 26, 2025

Copy link
Copy Markdown
Collaborator Author

Integrating the proposed API while maintaining the possibility to just call the function seems like a reasonable trade-off to finalize this.

Like I said that would introduce the <iostream> header which it is one of the heavier headers that and as it is just debug code that seems unnecessary to spill into user code.

Also we could use default parameters which would require less code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danmar danmar danmar approved these changes

+1 more reviewer

@jcfr jcfr jcfr left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@firewave @danmar @jcfr