Skip to content

Commit

Permalink
fix(libutil/posix-source-accessor.cc): get rid of use-after-move bug
Browse files Browse the repository at this point in the history
Naming class member variables the same as constructor arguments is a very
slippery slope because of how member variable names get resolved. Compiler
is not very helpful here and we need static analysis to forbid this kind of
stuff.

The following example illustrates the cause quite well:

```cpp

struct B {
    B(int) {}
};

struct A {
    A(int b): b([&](){
        return b;
        static_assert(std::is_same_v<decltype(b), int>);
    }()) {
       static_assert(std::is_same_v<decltype(b), int>);
    }
    void member() {
        static_assert(std::is_same_v<decltype(b), B>);
    }
    B b;
};

int main() {
    A(1).member();
}
```

From N4861 6.5.1 Unqualified name lookup:

> In all the cases listed in [basic.lookup.unqual], the scopes are searched
> for a declaration in the order listed in each of the respective categories;
> name lookup ends as soon as a declaration is found for the name.
> If no declaration is found, the program is ill-formed.

In the affected code there was a use-after-move for all accesses in the constructor
body, but this UB wasn't triggered.

These types of errors are trivial to catch via clang-tidy's [clang-analyzer-cplusplus.Move].

(cherry picked from commit 3e0129c)
  • Loading branch information
xokdvium authored and mergify[bot] committed Nov 8, 2024
1 parent 6e095dd commit c556c20
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/libutil/posix-source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

namespace nix {

PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && root)
: root(std::move(root))
PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && argRoot)
: root(std::move(argRoot))
{
assert(root.empty() || root.is_absolute());
displayPrefix = root.string();
Expand Down

0 comments on commit c556c20

Please sign in to comment.