From 7dad386f46d4565407cd80fedcd1b289a2f58ef2 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <tmpsantos@gmail.com>
Date: Thu, 12 Dec 2019 14:43:07 +0200
Subject: [tests] Fix -Werror=shadow issues in the unit tests

---
 test/actor/actor.test.cpp            | 137 ++++++++++++++---------------------
 test/actor/actor_ref.test.cpp        |  64 +++++++---------
 test/renderer/image_manager.test.cpp |   2 +-
 test/tile/tile_cache.test.cpp        |   6 +-
 4 files changed, 85 insertions(+), 124 deletions(-)

diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp
index 03eb9b8301..5b395074ed 100644
--- a/test/actor/actor.test.cpp
+++ b/test/actor/actor.test.cpp
@@ -14,32 +14,26 @@ using namespace mbgl;
 using namespace std::chrono_literals;
 
 TEST(Actor, Construction) {
-    struct Test {
-        Test(ActorRef<Test>, bool& constructed) {
-            constructed = true;
-        };
+    struct TestActor {
+        TestActor(ActorRef<TestActor>, bool& constructed) { constructed = true; };
     };
 
     bool constructed = false;
-    Actor<Test> test(Scheduler::GetBackground(), std::ref(constructed));
+    Actor<TestActor> test(Scheduler::GetBackground(), std::ref(constructed));
 
     EXPECT_TRUE(constructed);
 }
 
 TEST(Actor, Destruction) {
-    struct Test {
-        Test(ActorRef<Test>, bool& destructed_) : destructed(destructed_) {};
-        ~Test() {
-            destructed = true;
-        }
+    struct TestActor {
+        TestActor(ActorRef<TestActor>, bool& destructed_) : destructed(destructed_){};
+        ~TestActor() { destructed = true; }
 
         bool& destructed;
     };
 
     bool destructed = false;
-    {
-        Actor<Test> test(Scheduler::GetBackground(), std::ref(destructed));
-    }
+    { Actor<TestActor> test(Scheduler::GetBackground(), std::ref(destructed)); }
 
     EXPECT_TRUE(destructed);
 }
@@ -47,20 +41,15 @@ TEST(Actor, Destruction) {
 TEST(Actor, DestructionBlocksOnReceive) {
     // Destruction blocks until the actor is not receiving.
 
-    struct Test {
+    struct TestActor {
         std::promise<void> promise;
         std::future<void> future;
         std::atomic<bool> waited;
 
-        Test(ActorRef<Test>, std::promise<void> promise_, std::future<void> future_)
-            : promise(std::move(promise_)),
-              future(std::move(future_)),
-              waited(false) {
-        }
+        TestActor(ActorRef<TestActor>, std::promise<void> promise_, std::future<void> future_)
+            : promise(std::move(promise_)), future(std::move(future_)), waited(false) {}
 
-        ~Test() {
-            EXPECT_TRUE(waited.load());
-        }
+        ~TestActor() { EXPECT_TRUE(waited.load()); }
 
         void wait() {
             promise.set_value();
@@ -76,9 +65,9 @@ TEST(Actor, DestructionBlocksOnReceive) {
     std::promise<void> exitingPromise;
     std::future<void> exitingFuture = exitingPromise.get_future();
 
-    Actor<Test> test(Scheduler::GetBackground(), std::move(enteredPromise), std::move(exitingFuture));
+    Actor<TestActor> test(Scheduler::GetBackground(), std::move(enteredPromise), std::move(exitingFuture));
 
-    test.self().invoke(&Test::wait);
+    test.self().invoke(&TestActor::wait);
     enteredFuture.wait();
     exitingPromise.set_value();
 }
@@ -111,8 +100,8 @@ TEST(Actor, DestructionBlocksOnSend) {
         mapbox::base::WeakPtr<Scheduler> makeWeakPtr() override { return weakFactory.makeWeakPtr(); }
     };
 
-    struct Test {
-        Test(ActorRef<Test>) {}
+    struct TestActor {
+        TestActor(ActorRef<TestActor>) {}
         void message() {}
     };
 
@@ -123,14 +112,9 @@ TEST(Actor, DestructionBlocksOnSend) {
     std::future<void> exitingFuture = exitingPromise.get_future();
 
     auto scheduler = std::make_unique<TestScheduler>(std::move(enteredPromise), std::move(exitingFuture));
-    auto actor = std::make_unique<Actor<Test>>(*scheduler);
+    auto actor = std::make_unique<Actor<TestActor>>(*scheduler);
 
-    std::thread thread {
-        [] (ActorRef<Test> ref) {
-            ref.invoke(&Test::message);
-        },
-        actor->self()
-    };
+    std::thread thread{[](ActorRef<TestActor> ref) { ref.invoke(&TestActor::message); }, actor->self()};
 
     enteredFuture.wait();
     exitingPromise.set_value();
@@ -146,9 +130,8 @@ TEST(Actor, DestructionAllowedInReceiveOnSameThread) {
     // thread as receive(). This prevents deadlocks and
     // allows for self-closing actors
 
-    struct Test {
-
-        Test(ActorRef<Test>){};
+    struct TestActor {
+        TestActor(ActorRef<TestActor>){};
 
         void callMeBack(std::function<void ()> callback) {
             callback();
@@ -157,10 +140,10 @@ TEST(Actor, DestructionAllowedInReceiveOnSameThread) {
 
     std::promise<void> callbackFiredPromise;
     std::shared_ptr<Scheduler> retainer = Scheduler::GetBackground();
-    auto test = std::make_unique<Actor<Test>>(retainer);
+    auto test = std::make_unique<Actor<TestActor>>(retainer);
 
     // Callback (triggered while mutex is locked in Mailbox::receive())
-    test->self().invoke(&Test::callMeBack, [&]() {
+    test->self().invoke(&TestActor::callMeBack, [&]() {
         // Destroy the Actor/Mailbox in the same thread
         test.reset();
         callbackFiredPromise.set_value();
@@ -175,36 +158,31 @@ TEST(Actor, SelfDestructionDoesntCrashWaitingReceivingThreads) {
     // crash when a actor closes it's own mailbox from a
     // callback
 
-    struct Test {
-
-        Test(ActorRef<Test>){};
+    struct TestActor {
+        TestActor(ActorRef<TestActor>){};
 
         void callMeBack(std::function<void ()> callback) {
             callback();
         }
     };
 
-
     std::promise<void> actorClosedPromise;
 
-    auto closingActor = std::make_unique<Actor<Test>>(Scheduler::GetBackground());
-    auto waitingActor = std::make_unique<Actor<Test>>(Scheduler::GetBackground());
+    auto closingActor = std::make_unique<Actor<TestActor>>(Scheduler::GetBackground());
+    auto waitingActor = std::make_unique<Actor<TestActor>>(Scheduler::GetBackground());
 
     std::atomic<bool> waitingMessageProcessed {false};
 
     // Callback (triggered while mutex is locked in Mailbox::receive())
-    closingActor->self().invoke(&Test::callMeBack, [&]() {
-
+    closingActor->self().invoke(&TestActor::callMeBack, [&]() {
         // Queue up another message from another thread
         std::promise<void> messageQueuedPromise;
-        waitingActor->self().invoke(&Test::callMeBack, [&]() {
+        waitingActor->self().invoke(&TestActor::callMeBack, [&]() {
             // This will be waiting on the mutex in
             // Mailbox::receive(), holding a lock
             // on the weak_ptr so the mailbox is not
             // destroyed
-            closingActor->self().invoke(&Test::callMeBack, [&]() {
-                waitingMessageProcessed.store(true);
-            });
+            closingActor->self().invoke(&TestActor::callMeBack, [&]() { waitingMessageProcessed.store(true); });
             messageQueuedPromise.set_value();
         });
 
@@ -227,13 +205,11 @@ TEST(Actor, SelfDestructionDoesntCrashWaitingReceivingThreads) {
 TEST(Actor, OrderedMailbox) {
     // Messages are processed in order.
 
-    struct Test {
+    struct TestActor {
         int last = 0;
         std::promise<void> promise;
 
-        Test(ActorRef<Test>, std::promise<void> promise_)
-            : promise(std::move(promise_))  {
-        }
+        TestActor(ActorRef<TestActor>, std::promise<void> promise_) : promise(std::move(promise_)) {}
 
         void receive(int i) {
             EXPECT_EQ(i, last + 1);
@@ -247,26 +223,24 @@ TEST(Actor, OrderedMailbox) {
 
     std::promise<void> endedPromise;
     std::future<void> endedFuture = endedPromise.get_future();
-    Actor<Test> test(Scheduler::GetBackground(), std::move(endedPromise));
+    Actor<TestActor> test(Scheduler::GetBackground(), std::move(endedPromise));
 
     for (auto i = 1; i <= 10; ++i) {
-        test.self().invoke(&Test::receive, i);
+        test.self().invoke(&TestActor::receive, i);
     }
 
-    test.self().invoke(&Test::end);
+    test.self().invoke(&TestActor::end);
     endedFuture.wait();
 }
 
 TEST(Actor, NonConcurrentMailbox) {
     // An individual actor is never itself concurrent.
 
-    struct Test {
+    struct TestActor {
         int last = 0;
         std::promise<void> promise;
 
-        Test(ActorRef<Test>, std::promise<void> promise_)
-            : promise(std::move(promise_))  {
-        }
+        TestActor(ActorRef<TestActor>, std::promise<void> promise_) : promise(std::move(promise_)) {}
 
         void receive(int i) {
             EXPECT_EQ(i, last + 1);
@@ -281,31 +255,30 @@ TEST(Actor, NonConcurrentMailbox) {
 
     std::promise<void> endedPromise;
     std::future<void> endedFuture = endedPromise.get_future();
-    Actor<Test> test(Scheduler::GetBackground(), std::move(endedPromise));
+    Actor<TestActor> test(Scheduler::GetBackground(), std::move(endedPromise));
 
     for (auto i = 1; i <= 10; ++i) {
-        test.self().invoke(&Test::receive, i);
+        test.self().invoke(&TestActor::receive, i);
     }
 
-    test.self().invoke(&Test::end);
+    test.self().invoke(&TestActor::end);
     endedFuture.wait();
 }
 
 TEST(Actor, Ask) {
     // Asking for a result
 
-    struct Test {
-
-        Test(ActorRef<Test>) {}
+    struct TestActor {
+        TestActor(ActorRef<TestActor>) {}
 
         int doubleIt(int i) {
             return i * 2;
         }
     };
 
-    Actor<Test> test(Scheduler::GetBackground());
+    Actor<TestActor> test(Scheduler::GetBackground());
 
-    auto result = test.self().ask(&Test::doubleIt, 1);
+    auto result = test.self().ask(&TestActor::doubleIt, 1);
 
     ASSERT_TRUE(result.valid());
 
@@ -317,11 +290,10 @@ TEST(Actor, Ask) {
 TEST(Actor, AskVoid) {
     // Ask waits for void methods
 
-    struct Test {
+    struct TestActor {
         bool& executed;
 
-        Test(bool& executed_) : executed(executed_) {
-        }
+        TestActor(bool& executed_) : executed(executed_) {}
 
         void doIt() {
             executed = true;
@@ -329,9 +301,9 @@ TEST(Actor, AskVoid) {
     };
 
     bool executed = false;
-    Actor<Test> actor(Scheduler::GetBackground(), executed);
+    Actor<TestActor> actor(Scheduler::GetBackground(), executed);
 
-    actor.self().ask(&Test::doIt).get();
+    actor.self().ask(&TestActor::doIt).get();
     EXPECT_TRUE(executed);
 }
 
@@ -370,13 +342,10 @@ TEST(Actor, TwoPhaseConstruction) {
     // its actor in two parts so that the Thread<Object> instance can be created without waiting
     // for the target thread to be up and running.
 
-    struct Test {
-        Test(ActorRef<Test>, std::shared_ptr<bool> destroyed_)
-            : destroyed(std::move(destroyed_)) {};
+    struct TestActor {
+        TestActor(ActorRef<TestActor>, std::shared_ptr<bool> destroyed_) : destroyed(std::move(destroyed_)){};
 
-        ~Test() {
-            *destroyed = true;
-        }
+        ~TestActor() { *destroyed = true; }
 
         void callMe(std::promise<void> p) {
             p.set_value();
@@ -389,22 +358,22 @@ TEST(Actor, TwoPhaseConstruction) {
         std::shared_ptr<bool> destroyed;
     };
 
-    AspiringActor<Test> parent;
+    AspiringActor<TestActor> parent;
 
     auto destroyed = std::make_shared<bool>(false);
 
     std::promise<void> queueExecuted;
     auto queueExecutedFuture = queueExecuted.get_future();
 
-    parent.self().invoke(&Test::callMe, std::move(queueExecuted));
-    parent.self().invoke(&Test::stop);
+    parent.self().invoke(&TestActor::callMe, std::move(queueExecuted));
+    parent.self().invoke(&TestActor::stop);
 
     auto thread = std::thread([
         capturedArgs = std::make_tuple(destroyed),
         &parent
     ] () mutable {
         util::RunLoop loop(util::RunLoop::Type::New);
-        EstablishedActor<Test> test(loop, parent, capturedArgs);
+        EstablishedActor<TestActor> test(loop, parent, capturedArgs);
         loop.run();
     });
 
diff --git a/test/actor/actor_ref.test.cpp b/test/actor/actor_ref.test.cpp
index 221a220ed9..0f07b87a8f 100644
--- a/test/actor/actor_ref.test.cpp
+++ b/test/actor/actor_ref.test.cpp
@@ -11,16 +11,12 @@ TEST(ActorRef, CanOutliveActor) {
     // An ActorRef can outlive its actor. Doing does not extend the actor's lifetime.
     // Sending a message to an ActorRef whose actor has died is a no-op.
 
-    struct Test {
+    struct TestActorRef {
         bool& died;
 
-        Test(ActorRef<Test>, bool& died_)
-            : died(died_) {
-        }
+        TestActorRef(ActorRef<TestActorRef>, bool& died_) : died(died_) {}
 
-        ~Test() {
-            died = true;
-        }
+        ~TestActorRef() { died = true; }
 
         void receive() {
             FAIL();
@@ -29,20 +25,19 @@ TEST(ActorRef, CanOutliveActor) {
 
     bool died = false;
 
-    ActorRef<Test> test = [&] () {
-        return Actor<Test>(Scheduler::GetBackground(), std::ref(died)).self();
+    ActorRef<TestActorRef> test = [&]() {
+        return Actor<TestActorRef>(Scheduler::GetBackground(), std::ref(died)).self();
     }();
 
     EXPECT_TRUE(died);
-    test.invoke(&Test::receive);
+    test.invoke(&TestActorRef::receive);
 }
 
 TEST(ActorRef, Ask) {
     // Ask returns a Future eventually returning the result
 
-    struct Test {
-
-        Test(ActorRef<Test>) {}
+    struct TestActorRef {
+        TestActorRef(ActorRef<TestActorRef>) {}
 
         int gimme() {
             return 20;
@@ -53,32 +48,31 @@ TEST(ActorRef, Ask) {
         }
     };
 
-    Actor<Test> actor(Scheduler::GetBackground());
-    ActorRef<Test> ref = actor.self();
+    Actor<TestActorRef> actor(Scheduler::GetBackground());
+    ActorRef<TestActorRef> ref = actor.self();
 
-    EXPECT_EQ(20, ref.ask(&Test::gimme).get());
-    EXPECT_EQ(30, ref.ask(&Test::echo, 30).get());
+    EXPECT_EQ(20, ref.ask(&TestActorRef::gimme).get());
+    EXPECT_EQ(30, ref.ask(&TestActorRef::echo, 30).get());
 }
 
 TEST(ActorRef, AskVoid) {
     // Ask waits for void methods
-    
-    struct Test {
+
+    struct TestActorRef {
         bool& executed;
-        
-        Test(bool& executed_) : executed(executed_) {
-        }
-        
+
+        TestActorRef(bool& executed_) : executed(executed_) {}
+
         void doIt() {
             executed = true;
         }
     };
-    
+
     bool executed = false;
-    Actor<Test> actor(Scheduler::GetBackground(), executed);
-    ActorRef<Test> ref = actor.self();
-    
-    ref.ask(&Test::doIt).get();
+    Actor<TestActorRef> actor(Scheduler::GetBackground(), executed);
+    ActorRef<TestActorRef> ref = actor.self();
+
+    ref.ask(&TestActorRef::doIt).get();
     EXPECT_TRUE(executed);
 }
 
@@ -86,14 +80,12 @@ TEST(ActorRef, AskOnDestroyedActor) {
     // Tests behavior when calling ask() after the
     // Actor has gone away. Should set a exception_ptr.
 
-    struct Test {
+    struct TestActorRef {
         bool& died;
 
-        Test(ActorRef<Test>, bool& died_) : died(died_) {}
+        TestActorRef(ActorRef<TestActorRef>, bool& died_) : died(died_) {}
 
-        ~Test() {
-            died = true;
-        }
+        ~TestActorRef() { died = true; }
 
         int receive() {
             return 1;
@@ -101,12 +93,12 @@ TEST(ActorRef, AskOnDestroyedActor) {
     };
     bool died = false;
 
-    auto actor = std::make_unique<Actor<Test>>(Scheduler::GetBackground(), died);
-    ActorRef<Test> ref = actor->self();
+    auto actor = std::make_unique<Actor<TestActorRef>>(Scheduler::GetBackground(), died);
+    ActorRef<TestActorRef> ref = actor->self();
 
     actor.reset();
     EXPECT_TRUE(died);
 
-    auto result = ref.ask(&Test::receive);
+    auto result = ref.ask(&TestActorRef::receive);
     EXPECT_ANY_THROW(result.get());
 }
diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp
index d1d0a83c44..a891f48b48 100644
--- a/test/renderer/image_manager.test.cpp
+++ b/test/renderer/image_manager.test.cpp
@@ -78,7 +78,7 @@ TEST(ImageManager, RemoveReleasesBinPackRect) {
 
 class StubImageRequestor : public ImageRequestor {
 public:
-    StubImageRequestor(ImageManager& imageManager) : ImageRequestor(imageManager) {}
+    StubImageRequestor(ImageManager& imageManager_) : ImageRequestor(imageManager_) {}
 
     void onImagesAvailable(ImageMap icons, ImageMap patterns, std::unordered_map<std::string, uint32_t> versionMap, uint64_t imageCorrelationID_) final {
         if (imagesAvailable && imageCorrelationID == imageCorrelationID_) imagesAvailable(icons, patterns, versionMap);
diff --git a/test/tile/tile_cache.test.cpp b/test/tile/tile_cache.test.cpp
index e64ad0bb79..7a89ece756 100644
--- a/test/tile/tile_cache.test.cpp
+++ b/test/tile/tile_cache.test.cpp
@@ -47,11 +47,11 @@ public:
 
 class VectorTileMock : public VectorTile {
 public:
-    VectorTileMock(const OverscaledTileID& id,
-                   std::string sourceID,
+    VectorTileMock(const OverscaledTileID& id_,
+                   std::string sourceID_,
                    const TileParameters& parameters,
                    const Tileset& tileset)
-        : VectorTile(id, sourceID, parameters, tileset) {
+        : VectorTile(id_, sourceID_, parameters, tileset) {
         renderable = true;
     }
 };
-- 
cgit v1.2.1