[clang-tidy] `bugprone-unchecked-optional-access` false positive when analyzing optional field of object accessed via chain of getters

[BaLiKfromUA]

Problem

The next code snippet:

#include <optional>
#include <string>
#include <iostream>

class A {
private:
  std::optional<std::string> d_field;

public:
  const std::optional<std::string>& field() const {
        return d_field;
  } 
};

class B {
private:
    A d_a;

public:
   const A& a() const {
       return d_a;
   } 
};

void foo(const B& data) {
    if (data.a().field().has_value()) {
        std::cout << data.a().field().value();
    }
}

produces a false positive warning from clang-tidy:

clang-tidy (trunk) x86-64 clang 18.1.0 (Editor #1, Compiler #1)
-checks=bugprone-unchecked-optional-access
[<source>:27:22: warning: unchecked access to optional value [bugprone-unchecked-optional-access]](javascript:;)
   27 |         std::cout << data.a().field().value();
      |                      ^
1 warning generated.

The next workaround could solve this issue:

void bar(const B& data) {
    const std::optional<std::string>& field = data.a().field();
    if (field.has_value()) {
        std::cout << field.value();
    }
}

But it is easy to forget it, especially if such pattern is frequent in the codebase.

Compiler explorer link

Similar issues

I've checked existing open issues and one is similar-ish to mine -- #93194

Seems like it is fixed now and I cannot reproduce it on my side. I assume that my bug is connected to the "chaining" of calls.

Compiler explorer link

---

I am happy to contribute the fix to this issue if someone could give me initial suggestions about a proper solution and possible places to look :)

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Valentyn Yukhymenko (BaLiKfromUA)

### Problem

The next code snippet:

#include <optional>
#include <string>
#include <iostream>

class A {
private:
  std::optional<std::string> d_field;

public:
  const std::optional<std::string>& field() const {
        return d_field;
  } 
};

class B {
private:
    A d_a;

public:
   const A& a() const {
       return d_a;
   } 
};

void foo(const B& data) {
    if (data.a().field().has_value()) {
        std::cout << data.a().field().value();
    }
}

produces a false positive warning from clang-tidy:

clang-tidy (trunk) x86-64 clang 18.1.0 (Editor #<!-- -->1, Compiler #<!-- -->1)
-checks=bugprone-unchecked-optional-access
[&lt;source&gt;:27:22: warning: unchecked access to optional value [bugprone-unchecked-optional-access]](javascript:;)
   27 |         std::cout &lt;&lt; data.a().field().value();
      |                      ^
1 warning generated.

The next workaround could solve this issue:

void bar(const B& data) {
    const std::optional<std::string>& field = data.a().field();
    if (field.has_value()) {
        std::cout << field.value();
    }
}

But it is easy to forget it, especially if such pattern is frequent in the codebase.

Compiler explorer link

Similar issues

I've checked existing open issues and one is similar-ish to mine -- #93194

Seems like it is fixed now and I cannot reproduce it on my side. I assume that the bug is connected to the "chaining" of calls.

Compiler explorer link

---

I am happy to contribute the fix to this issue if someone could give me initial suggestions about a proper solution and possible places to look :)

[BaLiKfromUA]

Found a connected issue which has been solved recently -- #58510

@jvoung sorry for pinging, but could you please take a look at my issue when you have time?

Is it something that makes sense to fix as well? I believe that we could use the approach introduced in #112605 but we also need somehow to track the chain of const getters calls.

Thank you in advance!

[jvoung]

Hi, yes it may make sense to fix as well.

I believe the difference between your example and the other tested cases is that a() is returning a reference to something other than optional -- (A though it can reach an optional through a field/getter of A) .

For now, we had only cached if it is a

1. reference to an optional around here

   llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

   Line 558 in d578dbf

   // If the const method returns an optional or reference to an optional.

2. or a copy of an optional (that came up often in our samples where the copy is cheap -- e.g., optional<int>)
3. or if it is a pointer (to anything) or boolean.

Perhaps reference to "anything" is similar enough to pointer to anything, that we could expand. However, we probably shouldn't allow copy of anything. At least, it's a bit wasteful to be repeating a chain involving copies.

[BaLiKfromUA]

Thank you! I will try to extend your PR to see how it works!

Yeah, I was not planning to cache copies. I am interested only in const references.

[BaLiKfromUA]

@jvoung, I created a small PR, which passed my tests. Would appreciate some initial feedback!

[jvoung]

Thank you! Left some initial feedback