[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked an inline comment as done.
arphaman added inline comments.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:76
+CachedFileSystemEntry
+CachedFileSystemEntry::createDirectoryEntry(llvm::vfs::Status Stat) {
+  assert(Stat.isDirectory() && "not a directory!");
----------------
aganea wrote:
> arphaman wrote:
> > aganea wrote:
> > > `llvm::vfs::Status &&Stat` to avoid a copy?
> > The copy should already be avoided, as I move the argument when passing in, and when it's assigned to the result.
> If `Stat` is not a rvalue reference, wouldn't the `std::move` in the call site end up as a copy? See [[ https://godbolt.org/z/Rr7cdM | this ]]. If you change `int test(A a)` to `int test(A &&a)` you can see the difference in the asm output. However if the function is inlined, the extra copy will probably be optimized out. Not a big deal - the OS calls like stat() will most likely dominate here.
Isn't the difference in the output caused by the details of the calling convention (pass the structure on the stack by value)? The move constructor should still be called for the stat, it will not copy its members. We can optimize this though and pass by ref directly, I agree, so let me do that.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63907/new/

https://reviews.llvm.org/D63907