-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add stream-style logging macros #786
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
base: master
Are you sure you want to change the base?
Conversation
src/include/common/macros.h
Outdated
// A macro which checks `expr` value and performs assert. | ||
// Different from `BUSTUB_ASSERT`, it takes stream-style parameters. | ||
#define BUSTUB_ASSERT_AND_LOG(expr) \ | ||
if (bool val = (expr); !val) LogFatalStream { \ |
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.
If you think necessary, I could add a Unlikely
function, which improves branch prediction, since I suspect all assertions sits on critical path.
@dentiny Thanks for sending this. We will take a look at it. It might make more sense to use a third-party header-only logger. |
Thanks @apavlo for taking a look! It would be nice to support
For this PR, I mostly target at code simplicity w/o extra deps :) |
fd83a95
to
34d5495
Compare
Curious if there're any updates on this PR? 👀 |
@dentiny End of semester crunch. We will look at this soon. |
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 looks fine to me. Sorry for not reviewing this last semester! I guess this will be up to the future TAs to actually merge but LGTM
If you can fix the CI then I think this is fine to merge! |
Thank you @connortsui20 for the review! I think the code is benign so I simply suppress the linter warnings. |
Problem statement:
operator+
, which could create temporary stringsIn this PR, I propose to add a new macro, which combines both assertion and logging with stream style operation.
How I tested:
https://onlinegdb.com/PZiB5VT2A