llvm / llvm/llvm-project

[clang-tidy]: `bugprone-unchecked-optional-access` false-positive when modifying optional field via setter method (by reference)

Open
#161,005 4 comments 0 reactions 0 assignees View on GitHub
clang-tidy clang:dataflow false-positive
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

Hi folks!

Recently, I found an interesting bug related to accessing an optional field by reference via a method.

Initially, I thought that it might be in the same nature as https://github.com/llvm/llvm-project/pull/128437, but now I think it's much more complicated.

Let's assume we have next code:
```cpp
class A {
private:
std::optional d_field;

public:
const std::optional& getField() const {
return d_field;
}

std::optional& setField() {
return d_field;
}

private:
// no violations
void do_something_with_optional() {
if (!d_field.has_value()) {
d_field = 42;
}

std::cout << d_field.value();
}
};
```

Code below triggers warnings from check even though it's technically similar to `A::do_something_with_optional()`:
```cpp
// false positive
void foo1(A data) {
if (!data.getField().has_value()) {
data.setField() = 42;
}

std::cout << data.getField().value();
}
```

It also can be reproduced even by code like this:
```cpp
// false positive
void foo2(A data) {
data.setField() = 42;
std::cout << data.getField().value();
}
```

## My understanding why it doesn't work

IIRC the implementation of a given check, we don't analyze the access to non-const methods, even if they return optional field by reference. Plus, any access to them invalidate the stored state of related optional field that we have seen before.

Probably the fix is not simple since we cannot just rely on the method name (as I did in my previous fix), we need to understand that it returns exactly the same optional field as was modified before.

I believe the proper fix also might address the issue like https://github.com/llvm/llvm-project/issues/151287 when we call non-const method which doesn't modify optional data, but maybe I am wrong :)

**I am happy to explore the solution but need an initial guidance.**

Thanks!

[Compiler explorer link with my experiments](https://godbolt.org/z/Kqoj73v1d)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.