[clang-tidy] [NFC] Potential dereference of nullptr.

[zahiraam]

The static analyzer we use internally complains about potential dereference of nullptr for Found. I think both Found and Member can't be null here (please confirm). I have added assertions.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Zahira Ammarguellat (zahiraam)

Changes

The static analyzer we use internally complains about potential dereference of nullptr for Found. I think both Found and Member can't be null here (please confirm). I have added assertions.

---

Full diff: https://github.com/llvm/llvm-project/pull/143145.diff

1 Files Affected:

• (modified) clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp (+2)

diff --git a/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp
index 4aba5831e6772..23de8d971898e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp
@@ -50,6 +50,8 @@ void MisleadingSetterOfReferenceCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *Found = Result.Nodes.getNodeAs<CXXMethodDecl>("bad-set-function");
   const auto *Member = Result.Nodes.getNodeAs<FieldDecl>("member");
+  assert(Found != nullptr);
+  assert(Member != nullptr);
 
   diag(Found->getBeginLoc(),
        "function '%0' can be mistakenly used in order to change the "

[vbvictor]

I think we generally discussed such changes in #138283,
CC @nicovank.

Maybe we need to come to consensus in clang-tidy whether we should write assert() after Result.Nodes.getNodeAs<>. (There are many-many places that has assert and that doesn't have assert)

Personally, I don't have a strong opinion on such changes, so from me LGTM without explicit approval, please wait for a second opinion.

[carlosgalvezp]

I think this is the correct thing to do. It's a programming error if these pointers are null, and so assertions are the correct tool for this.

Other checks do if (const auto* Found = ...), but that's not correct, since it silences programming errors.