[PATCH] D43511: [dsymutil] Be smarter in caching calls to realpath

[Frederic Riss via Phabricator via llvm-commits]

friss added inline comments.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:122
+    sys::path::append(ResolvedPath, FileName);
+    return StringRef(ResolvedPath);
+  }
----------------
aprantl wrote:
> would it be more efficient to instead return the SmallString<256> directly, so it gets allocated by the caller and we avoid the copy upon return? I haven't looked at what the caller is doing with it.
Returning a StringRef to a local variable is just confusing (now I see that this actually gets converted into a std::string so it's not incorrect, but it's still confusing). I would pass that StringPool to the resolver instead and intern the resolved path there. Then you can return just a StringRef to the permanent storage.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:1945-1947
+            // FIXME: Passing U.getOrigUnit().getCompilationDir()
+            // instead of "" would allow more uniquing, but for now, do
+            // it this way to match dsymutil-classic.
----------------
You can drop that backward compatibility stuff at the same time.


https://reviews.llvm.org/D43511