-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Do not shadow LOCK's criticalblock variable for LOCK inside LOCK #8472
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
Conversation
utACK 33d15a3 |
More ACKs please. |
utACK 33d15a3 |
I'd probably use LINE which is defined in the standards, but in practice it probably doesn't matter. |
@luke-jr
Of course you can use LOCK2 then instead, but... Using |
@paveljanik just to clarify, I was being snarky there. I don't think that's a real worry. In fact, 2 locks on the same line would almost certainly be wrong, as they should be std::lock()ed for safety anyway. So either way is fine with me. utACK 33d15a3 |
@theuni Ah, OK. Then I think we SHOULD use the standard |
Using |
So let's go with |
Microsoft, gcc and clang implement We could even fall back to shadowing if it's not defined? |
Let's try it and see. |
…ide LOCK 33d15a3 Do not shadow LOCK's criticalblock variable for LOCK inside LOCK (Pavel Janík)
zcash: cherry picked from commit 33d15a3 zcash: bitcoin/bitcoin#8472
Bitcoin 0.14 locking PRs These are locking changes from upstream (bitcoin core) release 0.14, oldest to newest (when merged to the master branch). Each commit also includes a reference both to the PR and the upstream commit. - bitcoin/bitcoin#8472 - bitcoin/bitcoin#8606 - Excludes a lock move because we don't have bitcoin/bitcoin#7840 which this move was partially-reverting. - bitcoin/bitcoin#9230 - Only first commit (we don't have `LogTimestampStr` anymore). - bitcoin/bitcoin#9243 - Only the sixth commit, excluding `IsArgSet` locking (we haven't pulled that function in yet). - bitcoin/bitcoin#9626 - The cherry-picked commit does not match the upstream at all, but the resulting lock is useful. - bitcoin/bitcoin#9679 - bitcoin/bitcoin#9227 - bitcoin/bitcoin#9698 - Excludes change to `CConnman::PushMessage` in second commit (which we don't have yet). - bitcoin/bitcoin#9708 - bitcoin/bitcoin#9771
This is a followup to #8105, implements task 3 from it.
LOCK macro declares its variable
criticalblock
. When LOCK is used in nested block, again, the variable namedcriticalblock
is used. When compiling with-Wshadow
, this emits warning.This change renames the variable
criticalblock
tocriticalblockn
, where n is ordinal number of the variable in the compiled file. Macro__COUNTER__
is used for this. It is supported by gcc (https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html), clang (http://clang.llvm.org/docs/LanguageExtensions.html) and Visual Studio (https://msdn.microsoft.com/en-us/library/b0084kay.aspx).