From ba22e52f4bfb727161128904e726f37def797b62 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 9 Oct 2016 14:53:30 -0700 Subject: [PATCH] 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 --- folly/Foreach.h | 27 +++++++++++++++++---------- folly/test/ForeachTest.cpp | 12 ++++++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/folly/Foreach.h b/folly/Foreach.h index d362c0ba..84663ef3 100644 --- a/folly/Foreach.h +++ b/folly/Foreach.h @@ -29,13 +29,13 @@ * and everything is taken care of. * * 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 * evaluated multiple times, the macro defines one do-nothing if * statement to inject the Boolean variable FOR_EACH_state1, and then a * for statement that is executed only once, which defines the variable - * FOR_EACH_state2 holding a rvalue reference to the container being - * iterated. The workhorse is the last loop, which uses the just defined + * FOR_EACH_state2 holding an rvalue reference to the container being + * iterated. The workhorse is the last loop, which uses the just-defined * rvalue reference FOR_EACH_state2. * * The state variables are nested so they don't interfere; you can use @@ -47,18 +47,25 @@ */ #include +#include + +/* + * 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: * 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) \ - if (bool FOR_EACH_state1 = false) {} else \ - for (auto && FOR_EACH_state2 = (c); \ - !FOR_EACH_state1; FOR_EACH_state1 = true) \ - for (auto i = FOR_EACH_state2.begin(); \ - i != FOR_EACH_state2.end(); ++i) +#define FOR_EACH(i, c) \ + if (bool _FE_ANON(s1_) = false) {} else \ + for (auto && _FE_ANON(s2_) = (c); \ + !_FE_ANON(s1_); _FE_ANON(s1_) = true) \ + for (auto i = _FE_ANON(s2_).begin(); \ + i != _FE_ANON(s2_).end(); ++i) /* * Similar to FOR_EACH, but iterates the container backwards by diff --git a/folly/test/ForeachTest.cpp b/folly/test/ForeachTest.cpp index 9904084d..d7de3117 100644 --- a/folly/test/ForeachTest.cpp +++ b/folly/test/ForeachTest.cpp @@ -40,6 +40,18 @@ TEST(Foreach, ForEachRvalue) { 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) { std::map testMap; testMap["abc"] = 1; -- 2.34.1