From e910dd4bdc9db80cb412a5c6d4af8c6f9faa5591 Mon Sep 17 00:00:00 2001 From: Giuseppe Ottaviano Date: Wed, 28 Dec 2016 20:18:43 -0800 Subject: [PATCH] Avoid shadowing warnings in SYNCHRONIZED Summary: If two `SYNCHRONIZED` blocks are nested the internal `SYNCHRONIZED_*` variables can be compatible shadows (`SYNCHRONIZED_state` always is), so GCC will issue a warning with `-Wshadow-compatible-local`. This diff disambiguates the variable names for each `SYNCHRONIZED` block (as long as they appear on different lines). Reviewed By: yfeldblum, philippv Differential Revision: D4371263 fbshipit-source-id: b467a1a2651667c679382a1cc1eaa28f7ee4e6b3 --- folly/Synchronized.h | 60 ++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/folly/Synchronized.h b/folly/Synchronized.h index 34dd6624..5f369dad 100644 --- a/folly/Synchronized.h +++ b/folly/Synchronized.h @@ -1292,6 +1292,15 @@ void swap(Synchronized& lhs, Synchronized& rhs) { lhs.swap(rhs); } +/** + * Disambiguate the name var by concatenating the line number of the original + * point of expansion. This avoids shadowing warnings for nested + * SYNCHRONIZEDs. The name is consistent if used multiple times within + * another macro. + * Only for internal use. + */ +#define SYNCHRONIZED_VAR(var) FB_CONCATENATE(SYNCHRONIZED_##var##_, __LINE__) + /** * SYNCHRONIZED is the main facility that makes Synchronized * helpful. It is a pseudo-statement that introduces a scope where the @@ -1318,31 +1327,31 @@ void swap(Synchronized& lhs, Synchronized& rhs) { FOLLY_MSVC_DISABLE_WARNING(4458) /* declaration hides member */ \ FOLLY_MSVC_DISABLE_WARNING(4459) /* declaration hides global */ \ FOLLY_GCC_DISABLE_NEW_SHADOW_WARNINGS \ - if (bool SYNCHRONIZED_state = false) { \ + if (bool SYNCHRONIZED_VAR(state) = false) { \ } else \ - for (auto SYNCHRONIZED_lockedPtr = \ + for (auto SYNCHRONIZED_VAR(lockedPtr) = \ (FB_VA_GLUE(FB_ARG_2_OR_1, (__VA_ARGS__))).operator->(); \ - !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) \ for (auto& FB_VA_GLUE(FB_ARG_1, (__VA_ARGS__)) = \ - *SYNCHRONIZED_lockedPtr.operator->(); \ - !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) \ + *SYNCHRONIZED_VAR(lockedPtr).operator->(); \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) \ FOLLY_POP_WARNING #define TIMED_SYNCHRONIZED(timeout, ...) \ - if (bool SYNCHRONIZED_state = false) { \ + if (bool SYNCHRONIZED_VAR(state) = false) { \ } else \ - for (auto SYNCHRONIZED_lockedPtr = \ + for (auto SYNCHRONIZED_VAR(lockedPtr) = \ (FB_VA_GLUE(FB_ARG_2_OR_1, (__VA_ARGS__))).timedAcquire(timeout); \ - !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) \ for (auto FB_VA_GLUE(FB_ARG_1, (__VA_ARGS__)) = \ - (!SYNCHRONIZED_lockedPtr \ + (!SYNCHRONIZED_VAR(lockedPtr) \ ? nullptr \ - : SYNCHRONIZED_lockedPtr.operator->()); \ - !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) + : SYNCHRONIZED_VAR(lockedPtr).operator->()); \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) /** * Similar to SYNCHRONIZED, but only uses a read lock. @@ -1366,15 +1375,16 @@ void swap(Synchronized& lhs, Synchronized& rhs) { * different data). Synchronization is done in increasing address of * object order, so there is no deadlock risk. */ -#define SYNCHRONIZED_DUAL(n1, e1, n2, e2) \ - if (bool SYNCHRONIZED_state = false) { \ - } else \ - for (auto SYNCHRONIZED_ptrs = acquireLockedPair(e1, e2); \ - !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) \ - for (auto& n1 = *SYNCHRONIZED_ptrs.first; !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) \ - for (auto& n2 = *SYNCHRONIZED_ptrs.second; !SYNCHRONIZED_state; \ - SYNCHRONIZED_state = true) +#define SYNCHRONIZED_DUAL(n1, e1, n2, e2) \ + if (bool SYNCHRONIZED_VAR(state) = false) { \ + } else \ + for (auto SYNCHRONIZED_VAR(ptrs) = acquireLockedPair(e1, e2); \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) \ + for (auto& n1 = *SYNCHRONIZED_VAR(ptrs).first; !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) \ + for (auto& n2 = *SYNCHRONIZED_VAR(ptrs).second; \ + !SYNCHRONIZED_VAR(state); \ + SYNCHRONIZED_VAR(state) = true) } /* namespace folly */ -- 2.34.1