bustub icon indicating copy to clipboard operation
bustub copied to clipboard

Add stream-style logging macros

Open dentiny opened this issue 1 year ago • 6 comments

Problem statement:

  • Existing logging and assertion is kind of awkward, for example, it doesn't accept stream-style operation, meanwhile it uses string as the macro parameter, which means we have to concatenate the message beforehand.
    • (minor) It might hurts performance, I see a lot of the message construction is made by operator+, which could create temporary strings
    • (major) it reduces logging util usability

In 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

dentiny avatar Dec 10 '24 09:12 dentiny

@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.

apavlo avatar Dec 10 '24 14:12 apavlo

Thanks @apavlo for taking a look! It would be nice to support CHECK_EQ-like syntax, which

  • Allows comparison logic
  • Prints out value for lhs and rhs if comparison logic fails
  • Code location (i.e. filename and line number) is included in the error message

For this PR, I mostly target at code simplicity w/o extra deps :)

dentiny avatar Dec 10 '24 17:12 dentiny

Curious if there're any updates on this PR? :eyes:

dentiny avatar Dec 15 '24 12:12 dentiny

@dentiny End of semester crunch. We will look at this soon.

apavlo avatar Dec 16 '24 16:12 apavlo

If you can fix the CI then I think this is fine to merge!

connortsui20 avatar May 13 '25 18:05 connortsui20

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.

dentiny avatar May 17 '25 21:05 dentiny