@llvm/issue-subscribers-clang-tidy

Author: Valentyn Yukhymenko (BaLiKfromUA)

#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();
    }
}

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.

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

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!

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.

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.

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