Commit ba22e52f authored by Jim Meyering's avatar Jim Meyering Committed by Facebook Github Bot

folly/Foreach.h: allow FOR_EACH to be nested with no shadowing

Summary:
Prior to this change, any nested use of FOR_EACH would
induce a shadowed declaration for each of the two local
state variables it declares.

This makes the names of those variables __LINE__-dependent,
so that there is no shadowing, as long as each nested use
is on a different line.

This also adds a new test that (prior to this change) would fail to
compile with an option like -Werror=shadow-compatible-local.

Since this change relies on cpp token concatenation, I have included
The fix defines a new helper macro, _FE_ANON, to derive each new variable
name. I wondered whether to do this for every other FOR_* macro here, but
since so far, I have encountered more than 10 cases of nested FOR_EACH
uses in a large corpus, but no nesting of any other FOR_* macro, I am
content to do it only for this one.

Reviewed By: yfeldblum

Differential Revision: D3992956

fbshipit-source-id: f26fba89bc661bb9d22747dec0acdcf8c648fb83
parent 9f8538ee
...@@ -29,13 +29,13 @@ ...@@ -29,13 +29,13 @@
* and everything is taken care of. * and everything is taken care of.
* *
* The implementation is a bit convoluted to make sure the container is * The implementation is a bit convoluted to make sure the container is
* only evaluated once (however, keep in mind that c.end() is evaluated * evaluated only once (however, keep in mind that c.end() is evaluated
* at every pass through the loop). To ensure the container is not * at every pass through the loop). To ensure the container is not
* evaluated multiple times, the macro defines one do-nothing if * evaluated multiple times, the macro defines one do-nothing if
* statement to inject the Boolean variable FOR_EACH_state1, and then a * statement to inject the Boolean variable FOR_EACH_state1, and then a
* for statement that is executed only once, which defines the variable * for statement that is executed only once, which defines the variable
* FOR_EACH_state2 holding a rvalue reference to the container being * FOR_EACH_state2 holding an rvalue reference to the container being
* iterated. The workhorse is the last loop, which uses the just defined * iterated. The workhorse is the last loop, which uses the just-defined
* rvalue reference FOR_EACH_state2. * rvalue reference FOR_EACH_state2.
* *
* The state variables are nested so they don't interfere; you can use * The state variables are nested so they don't interfere; you can use
...@@ -47,18 +47,25 @@ ...@@ -47,18 +47,25 @@
*/ */
#include <type_traits> #include <type_traits>
#include <folly/Preprocessor.h>
/*
* Form a local variable name from "FOR_EACH_" x __LINE__, so that
* FOR_EACH can be nested without creating shadowed declarations.
*/
#define _FE_ANON(x) FB_CONCATENATE(FOR_EACH_, FB_CONCATENATE(x, __LINE__))
/* /*
* Shorthand for: * Shorthand for:
* for (auto i = c.begin(); i != c.end(); ++i) * for (auto i = c.begin(); i != c.end(); ++i)
* except that c is only evaluated once. * except that c is evaluated only once.
*/ */
#define FOR_EACH(i, c) \ #define FOR_EACH(i, c) \
if (bool FOR_EACH_state1 = false) {} else \ if (bool _FE_ANON(s1_) = false) {} else \
for (auto && FOR_EACH_state2 = (c); \ for (auto && _FE_ANON(s2_) = (c); \
!FOR_EACH_state1; FOR_EACH_state1 = true) \ !_FE_ANON(s1_); _FE_ANON(s1_) = true) \
for (auto i = FOR_EACH_state2.begin(); \ for (auto i = _FE_ANON(s2_).begin(); \
i != FOR_EACH_state2.end(); ++i) i != _FE_ANON(s2_).end(); ++i)
/* /*
* Similar to FOR_EACH, but iterates the container backwards by * Similar to FOR_EACH, but iterates the container backwards by
......
...@@ -40,6 +40,18 @@ TEST(Foreach, ForEachRvalue) { ...@@ -40,6 +40,18 @@ TEST(Foreach, ForEachRvalue) {
EXPECT_EQ(0, n); EXPECT_EQ(0, n);
} }
TEST(Foreach, ForEachNested) {
const std::string hello = "hello";
size_t n = 0;
FOR_EACH(i, hello) {
FOR_EACH(j, hello) {
++n;
}
}
auto len = hello.size();
EXPECT_EQ(len * len, n);
}
TEST(Foreach, ForEachKV) { TEST(Foreach, ForEachKV) {
std::map<std::string, int> testMap; std::map<std::string, int> testMap;
testMap["abc"] = 1; testMap["abc"] = 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment