From dbe7c4ef52b0e0e93c1889ef61f800f93bd07947 Mon Sep 17 00:00:00 2001 From: John Bytheway <52664+jbytheway@users.noreply.github.com> Date: Tue, 7 Jun 2022 19:53:33 -0400 Subject: [PATCH 1/2] Enable bugprone-unhandled-self-assignment This check looks for suspicious assignment operators that might not correctly handle self-assignment. In practice it is a bit prone to false-positives, but we have so few assignment operators in general that's not a big deal. Also enable cert-oop54-cpp which is essentially the same check. Tweak its configuration so that it doesn't have more false positives. Suppress these warnings in one place and fix them in another. --- .clang-tidy | 8 ++++---- src/json.cpp | 1 + src/safe_reference.cpp | 6 ++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2997e067c1edd..d6734b4ba646b 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -51,8 +51,6 @@ readability-*,\ -bugprone-narrowing-conversions,\ -bugprone-redundant-branch-condition,\ -bugprone-signed-char-misuse,\ --bugprone-unhandled-self-assignment,\ --cert-oop54-cpp,\ -cert-str34-c,\ -clang-analyzer-core.CallAndMessage,\ -clang-analyzer-deadcode.DeadStores,\ @@ -86,9 +84,11 @@ WarningsAsErrors: '*' HeaderFilterRegex: '(src|test|tools).*' FormatStyle: none CheckOptions: - - key: readability-uppercase-literal-suffix.NewSuffixes - value: 'L;UL;LL;ULL' - key: cata-text-style.EscapeUnicode value: 0 + - key: cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField + value: true + - key: readability-uppercase-literal-suffix.NewSuffixes + value: 'L;UL;LL;ULL' # vim:tw=0 diff --git a/src/json.cpp b/src/json.cpp index 05c3cb199a39d..7b9e96e52e901 100644 --- a/src/json.cpp +++ b/src/json.cpp @@ -486,6 +486,7 @@ JsonArray::JsonArray( const JsonArray &ja ) final_separator = ja.final_separator; } +// NOLINTNEXTLINE(bugprone-unhandled-self-assignment,cert-oop54-cpp) JsonArray &JsonArray::operator=( const JsonArray &ja ) { jsin = ja.jsin; diff --git a/src/safe_reference.cpp b/src/safe_reference.cpp index 6dd4c6e6ee924..2264915a7b5a1 100644 --- a/src/safe_reference.cpp +++ b/src/safe_reference.cpp @@ -19,9 +19,11 @@ safe_reference_anchor::safe_reference_anchor( safe_reference_anchor && ) noexcep safe_reference_anchor() {} -safe_reference_anchor &safe_reference_anchor::operator=( const safe_reference_anchor & ) +safe_reference_anchor &safe_reference_anchor::operator=( const safe_reference_anchor &other ) { - impl = std::make_shared(); + if( this != &other ) { + impl = std::make_shared(); + } return *this; } From b8a698671c169b5e380ccfbf152aa42f016e1501 Mon Sep 17 00:00:00 2001 From: John Bytheway <52664+jbytheway@users.noreply.github.com> Date: Tue, 7 Jun 2022 19:56:16 -0400 Subject: [PATCH 2/2] Simplify JsonArray copy constructor Implement it in terms of its assignment operator to reduce code duplication. --- src/json.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/json.cpp b/src/json.cpp index 7b9e96e52e901..51311a47601dc 100644 --- a/src/json.cpp +++ b/src/json.cpp @@ -478,12 +478,7 @@ JsonArray::JsonArray( JsonIn &j ) JsonArray::JsonArray( const JsonArray &ja ) { - jsin = ja.jsin; - start = ja.start; - index = 0; - positions = ja.positions; - end_ = ja.end_; - final_separator = ja.final_separator; + *this = ja; } // NOLINTNEXTLINE(bugprone-unhandled-self-assignment,cert-oop54-cpp)