logging: Replace LogPrint macros with regular functions #14209
Merged
Conversation
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
maflcko
force-pushed
the
Mf1809-logFun
branch
from
fa35dc5 to
fae3fbd
Compare
Member
Author
|
This should only affect test builds, so tagged with refactoring. |
scravy
approved these changes
Sep 12, 2018
Member
Author
|
|
Ruteri
approved these changes
Sep 12, 2018
Ruteri
left a comment
Ruteri
left a comment
There was a problem hiding this comment.
Since this works mostly with strings and builtins it could benefit from perfect forwarding. I took a look a the tinyformat and it looks doable in something like 2 hours (we would need to fork it).
Gnappuraz
approved these changes
Sep 12, 2018
Contributor
Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Sep 12, 2018
ryanofsky
approved these changes
Sep 12, 2018
Contributor
|
utACK fae3fbd Very nice! |
Member
|
nice, replacing macros with functions is unambiguously good
This has been tried before, I don't think it was as easy as suggested. But you're welcome to try!—please do file changes to tinyformat upstream at https://github.com/c42f/tinyformat |
laanwj
added a commit
that referenced
this pull request
Sep 13, 2018
fae3fbd logging: Replace LogPrint macros with regular functions (MarcoFalke) Pull request description: It is not possible to run the full test suite when configured with `--enable-lcov`, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9) Fix this instead by replacing the macros with functions. Tree-SHA512: 101aa4f4a3ffcefc38faf70c9d3deb5fc63e0b11ca54a164d0463931c79eaf53ab0b0c6ae92a45355574e3b1d2c32233874a6b24293e7a09d188fc6698e212a5
scravy
added a commit
to dtr-org/unit-e
that referenced
this pull request
Apr 3, 2019
Signed-off-by: Julian Fleischer <[email protected]>
jkczyz
added a commit
to jkczyz/bitcoin
that referenced
this pull request
Oct 22, 2019
Calling LogPrint with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining LogPrint as a macro prevents this unnecessary expression evaluation. This is a partial revert of bitcoin#14209. The decision to revert is discussed in bitcoin#16688, which adds verbose logging for validation event notification.
maflcko
pushed a commit
that referenced
this pull request
Nov 1, 2019
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of #14209. The decision to revert is discussed in #16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Nov 1, 2019
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of bitcoin#14209. The decision to revert is discussed in bitcoin#16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
sidhujag
pushed a commit
to syscoin-core/syscoin
that referenced
this pull request
Nov 10, 2020
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of bitcoin#14209. The decision to revert is discussed in bitcoin#16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Jun 16, 2021
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Jun 24, 2021
PastaPastaPasta
added a commit
to dashpay/dash
that referenced
this pull request
Jun 25, 2021
merge bitcoin#14209, bitcoin#13878, bitcoin#14192, bitcoin#17086: logging and filesystem updates
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 11, 2021
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of bitcoin#14209. The decision to revert is discussed in bitcoin#16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Sep 12, 2021
…egular function 14f2239 [Util] Replace LogPrintf (but not LogPrint) macro with regular function (random-zebra) Pull request description: This commit combines together bitcoin#14209 [MarcoFalke] and bitcoin#17218 [jkczyz] - The first one replaces `LogPrintf` and `LogPrint` macros with functions: > It is not possible to run the full test suite when configured with --enable-lcov, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9) > > Fix this instead by replacing the macros with functions. - The second one restores the `LogPrint` macro: > Calling LogPrint with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining LogPrint as a macro prevents this unnecessary expression evaluation. > > This is a partial revert of 14209. The decision to revert is discussed in 16688, which adds verbose logging for validation event notification. ACKs for top commit: furszy: ACK 14f2239 Fuzzbawls: ACK 14f2239 Tree-SHA512: bbbe475aae784e18acaf8f4f1eae8f6114f9ddb5eff460225fd0a1d40f999826bac99fb63f1359d71d03c6694d9c56fa697bfdee546188fe4fdf1a2f3de3eb22
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 15, 2021
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of bitcoin#14209. The decision to revert is discussed in bitcoin#16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is not possible to run the full test suite when configured with
--enable-lcov, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9)Fix this instead by replacing the macros with functions.