diff --git a/.clang-tidy b/.clang-tidy index 4f1ac626be..642bcb8a96 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -71,6 +71,7 @@ Checks: "-*, bugprone-unhandled-self-assignment, bugprone-unique-ptr-array-mismatch, bugprone-unsafe-functions, + bugprone-use-after-move, bugprone-unused-raii, bugprone-unused-return-value, bugprone-unused-local-non-trivial-variable, @@ -107,9 +108,6 @@ Checks: "-*, # --- # checks that have some issues that need to be resolved: # -# bugprone-move-forwarding-reference, -# bugprone-use-after-move, -# # llvm-namespace-comment, # misc-const-correctness, # misc-include-cleaner, diff --git a/src/libxrpl/shamap/SHAMap.cpp b/src/libxrpl/shamap/SHAMap.cpp index 9d0bfcb625..d5fa67fdac 100644 --- a/src/libxrpl/shamap/SHAMap.cpp +++ b/src/libxrpl/shamap/SHAMap.cpp @@ -663,9 +663,10 @@ SHAMap::delItem(uint256 const& id) SHAMapNodeType type = leaf->getType(); - // What gets attached to the end of the chain - // (For now, nothing, since we deleted the leaf) - intr_ptr::SharedPtr prevNode; + using TreeNodeType = intr_ptr::SharedPtr; + + // What gets attached to the end of the chain (For now, nothing, since we deleted the leaf) + TreeNodeType prevNode; while (!stack.empty()) { @@ -674,7 +675,12 @@ SHAMap::delItem(uint256 const& id) stack.pop(); node = unshareNode(std::move(node), nodeID); - node->setChild(selectBranch(nodeID, id), std::move(prevNode)); + node->setChild( + selectBranch(nodeID, id), std::move(prevNode)); // NOLINT(bugprone-use-after-move) + + XRPL_ASSERT( + not prevNode, // NOLINT(bugprone-use-after-move) + "xrpl::SHAMap::delItem : prevNode should be nullptr after std::move"); if (!nodeID.isRoot()) { @@ -684,7 +690,9 @@ SHAMap::delItem(uint256 const& id) if (bc == 0) { // no children below this branch - prevNode.reset(); + // + // Note: This is unnecessary due to the std::move above but left here for safety + prevNode = TreeNodeType{}; } else if (bc == 1) { @@ -697,7 +705,7 @@ SHAMap::delItem(uint256 const& id) { if (!node->isEmptyBranch(i)) { - node->setChild(i, intr_ptr::SharedPtr{}); + node->setChild(i, TreeNodeType{}); break; } } diff --git a/src/test/basics/Buffer_test.cpp b/src/test/basics/Buffer_test.cpp index 56f440b993..675228c42f 100644 --- a/src/test/basics/Buffer_test.cpp +++ b/src/test/basics/Buffer_test.cpp @@ -106,18 +106,18 @@ struct Buffer_test : beast::unit_test::suite { // Move-construct from empty buf Buffer x; Buffer y{std::move(x)}; - BEAST_EXPECT(sane(x)); - BEAST_EXPECT(x.empty()); + BEAST_EXPECT(sane(x)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(x.empty()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(sane(y)); BEAST_EXPECT(y.empty()); - BEAST_EXPECT(x == y); + BEAST_EXPECT(x == y); // NOLINT(bugprone-use-after-move) } { // Move-construct from non-empty buf Buffer x{b1}; Buffer y{std::move(x)}; - BEAST_EXPECT(sane(x)); - BEAST_EXPECT(x.empty()); + BEAST_EXPECT(sane(x)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(x.empty()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(sane(y)); BEAST_EXPECT(y == b1); } @@ -129,8 +129,8 @@ struct Buffer_test : beast::unit_test::suite x = std::move(y); BEAST_EXPECT(sane(x)); BEAST_EXPECT(x.empty()); - BEAST_EXPECT(sane(y)); - BEAST_EXPECT(y.empty()); + BEAST_EXPECT(sane(y)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(y.empty()); // NOLINT(bugprone-use-after-move) } { // Move assign non-empty buf to empty buf @@ -140,8 +140,8 @@ struct Buffer_test : beast::unit_test::suite x = std::move(y); BEAST_EXPECT(sane(x)); BEAST_EXPECT(x == b1); - BEAST_EXPECT(sane(y)); - BEAST_EXPECT(y.empty()); + BEAST_EXPECT(sane(y)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(y.empty()); // NOLINT(bugprone-use-after-move) } { // Move assign empty buf to non-empty buf @@ -151,8 +151,8 @@ struct Buffer_test : beast::unit_test::suite x = std::move(y); BEAST_EXPECT(sane(x)); BEAST_EXPECT(x.empty()); - BEAST_EXPECT(sane(y)); - BEAST_EXPECT(y.empty()); + BEAST_EXPECT(sane(y)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(y.empty()); // NOLINT(bugprone-use-after-move) } { // Move assign non-empty buf to non-empty buf @@ -163,14 +163,14 @@ struct Buffer_test : beast::unit_test::suite x = std::move(y); BEAST_EXPECT(sane(x)); BEAST_EXPECT(!x.empty()); - BEAST_EXPECT(sane(y)); - BEAST_EXPECT(y.empty()); + BEAST_EXPECT(sane(y)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(y.empty()); // NOLINT(bugprone-use-after-move) x = std::move(z); BEAST_EXPECT(sane(x)); BEAST_EXPECT(!x.empty()); - BEAST_EXPECT(sane(z)); - BEAST_EXPECT(z.empty()); + BEAST_EXPECT(sane(z)); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(z.empty()); // NOLINT(bugprone-use-after-move) } } diff --git a/src/test/core/ClosureCounter_test.cpp b/src/test/core/ClosureCounter_test.cpp index f91cdbddf7..fc56d7f6c9 100644 --- a/src/test/core/ClosureCounter_test.cpp +++ b/src/test/core/ClosureCounter_test.cpp @@ -214,15 +214,14 @@ class ClosureCounter_test : public beast::unit_test::suite BEAST_EXPECT(strCounter.count() == 1); BEAST_EXPECT(wrapped); - // Make the string big enough to (probably) avoid the small string - // optimization. + // Make the string big enough to (probably) avoid the small string optimization. TrackedString strRValue("rvalue abcdefghijklmnopqrstuvwxyz"); TrackedString const result = (*wrapped)(std::move(strRValue)); // NOLINT(bugprone-unchecked-optional-access) BEAST_EXPECT(result.copies == 0); BEAST_EXPECT(result.moves == 1); BEAST_EXPECT(result.str == "rvalue abcdefghijklmnopqrstuvwxyz!"); - BEAST_EXPECT(strRValue.str.size() == 0); + BEAST_EXPECT(strRValue.str.size() == 0); // NOLINT(bugprone-use-after-move) } } diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 1fdf4e6db3..bd1867adb3 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -533,14 +533,14 @@ public: BEAST_EXPECT(*jt1.get() == 7); BEAST_EXPECT(!jt1.get()); JTx jt2(std::move(jt1)); - BEAST_EXPECT(!jt1.get()); - BEAST_EXPECT(!jt1.get()); + BEAST_EXPECT(!jt1.get()); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(!jt1.get()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(jt2.get()); BEAST_EXPECT(*jt2.get() == 7); BEAST_EXPECT(!jt2.get()); jt1 = std::move(jt2); - BEAST_EXPECT(!jt2.get()); - BEAST_EXPECT(!jt2.get()); + BEAST_EXPECT(!jt2.get()); // NOLINT(bugprone-use-after-move) + BEAST_EXPECT(!jt2.get()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(jt1.get()); BEAST_EXPECT(*jt1.get() == 7); BEAST_EXPECT(!jt1.get()); diff --git a/src/test/protocol/STObject_test.cpp b/src/test/protocol/STObject_test.cpp index 94472127f1..b809ffccad 100644 --- a/src/test/protocol/STObject_test.cpp +++ b/src/test/protocol/STObject_test.cpp @@ -351,7 +351,7 @@ public: Buffer b(1); BEAST_EXPECT(!b.empty()); st[sf4] = std::move(b); - BEAST_EXPECT(b.empty()); + BEAST_EXPECT(b.empty()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(Slice(st[sf4]).size() == 1); st[~sf4] = std::nullopt; BEAST_EXPECT(!~st[~sf4]); @@ -370,7 +370,7 @@ public: BEAST_EXPECT(!!~st[~sf5]); Buffer b(1); st[sf5] = std::move(b); - BEAST_EXPECT(b.empty()); + BEAST_EXPECT(b.empty()); // NOLINT(bugprone-use-after-move) BEAST_EXPECT(Slice(st[sf5]).size() == 1); st[~sf4] = std::nullopt; BEAST_EXPECT(!~st[~sf4]); diff --git a/src/tests/libxrpl/json/Value.cpp b/src/tests/libxrpl/json/Value.cpp index cbca7c88a8..a7b7387ba1 100644 --- a/src/tests/libxrpl/json/Value.cpp +++ b/src/tests/libxrpl/json/Value.cpp @@ -761,16 +761,16 @@ TEST(json_value, move) EXPECT_EQ(v1.asDouble(), 2.5); Json::Value v2 = std::move(v1); - EXPECT_FALSE(v1); + EXPECT_FALSE(v1); // NOLINT(bugprone-use-after-move) EXPECT_TRUE(v2.isDouble()); EXPECT_EQ(v2.asDouble(), 2.5); - EXPECT_NE(v1, v2); + EXPECT_NE(v1, v2); // NOLINT(bugprone-use-after-move) v1 = std::move(v2); EXPECT_TRUE(v1.isDouble()); EXPECT_EQ(v1.asDouble(), 2.5); - EXPECT_FALSE(v2); - EXPECT_NE(v1, v2); + EXPECT_FALSE(v2); // NOLINT(bugprone-use-after-move) + EXPECT_NE(v1, v2); // NOLINT(bugprone-use-after-move) } TEST(json_value, comparisons) @@ -1348,7 +1348,7 @@ TEST(json_value, memory_leak) // Note that the type() == nullValue check is implementation // specific and not guaranteed to be valid in the future. - EXPECT_EQ(temp.type(), Json::nullValue); + EXPECT_EQ(temp.type(), Json::nullValue); // NOLINT(bugprone-use-after-move) } }