Skip to content

Add macros to check if logging at a given label is enabled #10768

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

Closed
wants to merge 2 commits into from
Closed

Add macros to check if logging at a given label is enabled #10768

wants to merge 2 commits into from

Conversation

Blei
Copy link
Contributor

@Blei Blei commented Dec 2, 2013

This is useful when the information that is needed to do useful logging
is expensive to produce.

@alexcrichton
Copy link
Member

I like the idea of having this, but I'm not sure that it's necessary to have a whole array of macros for all the logging levels. I think that it'd be nice to have something like:

// src/libstd/logging.rs
pub static DEBUG: u32 = 4;
pub static INFO: u32 = 3;
pub static WARN: u32 = 2;
pub static ERROR: u32 = 1;

// only one macro
macro_rules! log_enabled($lvl:expr) ( ... )

I would also rather not special-case the debug log level and instead clearly document that --cfg ndebug will disable all logging, but it will not affect the value of log_enabled!

information you would want to log is expensive to produce), you can use the
following macros:

* `log_enabled!(level)` - returns true iff logging of the given level is enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: if

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, sorry, I used iff to mean if and only if. Too much academia, API documentation is not the place for this :P

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't know that!

@Blei
Copy link
Contributor Author

Blei commented Dec 2, 2013

Pushed a new version


Note that when logging of level `DEBUG` is disabled at compile time,
`log_enabled!(DEBUG)` will still return `true`. If you want to make completely
sure, also test for `cfg!(not(ndebug))`.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, sorry about flip-flopping on this issue, but now that I actually read this in words it seems more awful then when I imagined this in my head.

That being said, I like only having one log_enabled! macro, and I think that we can just rely on LLVM optimizations to figure this out. The macro can be defined as:

macro_rules! log_enabled( ($lvl:expr) => (
  $lvl <= _log_level() && (log != 4 || cfg(not(ndebug)))
) )

Or at least I think I got the boolean logic there right

Verified

This commit was signed with the committer’s verified signature.
joboet Jonas Böttiger
This is useful when the information that is needed to do useful logging
is expensive to produce.
@Blei
Copy link
Contributor Author

Blei commented Dec 2, 2013

And another version :)

@Blei
Copy link
Contributor Author

Blei commented Dec 3, 2013

Test has been added

bors added a commit that referenced this pull request Dec 3, 2013
This is useful when the information that is needed to do useful logging
is expensive to produce.
@bors bors closed this Dec 3, 2013
@Blei Blei deleted the logging-enabled-macros branch December 4, 2013 07:01
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 20, 2023
[arithmetic_side_effects] Consider referenced allowed or hard-coded types

Fix rust-lang#10767

```
changelog: [`arithmetic_side_effects`]: Do not fire when dealing with allowed or hard-coded types that are referenced.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants