Skip to content

clang-tidy bugprone-unchecked-optional-access assumes unstable function results when it's impossible #58510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tsdgeos opened this issue Oct 20, 2022 · 2 comments
Assignees

Comments

@tsdgeos
Copy link

tsdgeos commented Oct 20, 2022

It will complain in code like

#include <optional>
#include <string>

class FontInfo
{
public:
    FontInfo() = default;

    const std::optional<std::string> &getName() const { return name; };

private:
    std::optional<std::string> name;

};

int main(int argc, char **)
{
    FontInfo fi;
    printf("HAS NAME %s\n", fi.getName() ? fi.getName()->c_str() : "[none]");
}

because it has the rule that consecutive calls to the same function might return different values, but that's simply not possible given how the two getName calls are one after the other and they are just returning a const & to a member in a const method, no?

@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2022

@llvm/issue-subscribers-clang-tidy

@jvoung jvoung self-assigned this Sep 27, 2024
jvoung added a commit to jvoung/llvm-project that referenced this issue Oct 3, 2024
…r methods

By caching const accessor methods we can sometimes treat method call
results as stable (e.g., for issue
llvm#58510).
Users can clear the cache when a non-const method is called that may
modify the state of an object.

This is represented as mixin.
It  will be used in a follow on patch to change
bugprone-unchecked-optional-access's lattice from
NoopLattice to CachedConstAccessorsLattice<NoopLattice>,
along with some additional transfer functions.
jvoung added a commit that referenced this issue Oct 16, 2024
…111006)

By caching const accessor methods we can sometimes treat method call
results as stable (e.g., for issue
#58510). Users can clear the
cache when a non-const method is called that may modify the state of an
object.
This is represented as mixin.

It will be used in a follow on patch to change
bugprone-unchecked-optional-access's lattice from NoopLattice to
CachedConstAccessorsLattice<NoopLattice>, along with some additional
transfer functions.
jvoung added a commit to jvoung/llvm-project that referenced this issue Oct 16, 2024
Treat calls to zero-param const methods as having stable return values
(with a cache) to address issue llvm#58510. The cache is invalidated when
non-const methods are called. This uses the infrastructure from PR llvm#111006.

For now we cache methods returning:
- ref to optional
- optional by value
- booleans

We can extend that to pointers to optional in a next change.
jvoung added a commit that referenced this issue Oct 22, 2024
…ess (#112605)

Treat calls to zero-param const methods as having stable return values
(with a cache) to address issue #58510. The cache is invalidated when
non-const methods are called. This uses the infrastructure from PR
#111006.

For now we cache methods returning:
- ref to optional
- optional by value
- booleans

We can extend that to pointers to optional in a next change.
jvoung added a commit to jvoung/llvm-project that referenced this issue Oct 28, 2024
…ecked-optional-access

Previously, we covered returning refs, or copies of optional, and bools.
Now cover returning pointers (to any type).
This is useful for cases like operator-> of smart pointers.
Addresses more of issue llvm#58510
jvoung added a commit that referenced this issue Oct 28, 2024
…ecked-optional-access (#113922)

Previously, we covered returning refs, or copies of optional, and bools.
Now cover returning pointers (to any type).
This is useful for cases like operator-> of smart pointers.
Addresses more of issue #58510
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
…ecked-optional-access (llvm#113922)

Previously, we covered returning refs, or copies of optional, and bools.
Now cover returning pointers (to any type).
This is useful for cases like operator-> of smart pointers.
Addresses more of issue llvm#58510
@jvoung
Copy link
Contributor

jvoung commented Nov 5, 2024

ClangTidy trunk should no longer be warning on this https://godbolt.org/z/rMGnM3eh8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants