[llvm] r278907 - [LTO] Introduce an Output class to wrap the output stream creation (NFC)

[Teresa Johnson via llvm-commits]

I tried it and fixed a few issues, along with merging with the recent
caching support I added. However it is still giving an error. The
reason is that with the caching support we need to capture OS which is
a unique_ptr in the lambda. This fails because we can't capture the
move-only unique_ptr. I tried an initialized lambda capture, but that
is C++14 only. What is the cleanest way to get this to work with
C++11?

Current diff with your patch (that isn't compiling due to capture of
OS near the end).

diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp
index 0423ce1..b336974 100644
--- a/tools/gold/gold-plugin.cpp
+++ b/tools/gold/gold-plugin.cpp
@@ -639,7 +639,7 @@ getOutputStream(SmallString<128> InFilename, bool
TempOutFile,
   int FD;
   if (TempOutFile) {
     std::error_code EC =
-        sys::fs::createTemporaryFile("lto-llvm", "o", NewFilename, FD);
+        sys::fs::createTemporaryFile("lto-llvm", "o", FD, NewFilename);
     if (EC)
       message(LDPL_FATAL, "Could not create temporary file: %s",
               EC.message().c_str());
@@ -651,7 +651,9 @@ getOutputStream(SmallString<128> InFilename, bool
TempOutFile,
     NewFilename += utostr(TaskID);
   std::error_code EC;
   auto OS = llvm::make_unique<raw_fd_ostream>(NewFilename, EC,
sys::fs::F_None);
-  check(EC, Path);
+  if (EC)
+    message(LDPL_FATAL, "Could not open file '%s': %s", NewFilename.str(),
+            EC.message().c_str());
   return std::move(OS);
 }

@@ -807,11 +809,11 @@ static ld_plugin_status allSymbolsReadHook() {

     IsTemporary[Task] = !SaveTemps;
     if (options::cache_dir.empty())
-      return llvm::make_unique<LTOOutput>(std::move();
+      return llvm::make_unique<LTOOutput>(std::move(OS));

     return llvm::make_unique<CacheObjectOutput>(
-        options::cache_dir,
[OutputName](std::unique_ptr<MemoryBuffer> Buffer) {
-          *LTOOutput(OutputName).getStream() << Buffer->getBuffer();
+        options::cache_dir, [OS](std::unique_ptr<MemoryBuffer> Buffer) {
+          *LTOOutput(std::move(OS)).getStream() << Buffer->getBuffer();
         });
   };


On Tue, Aug 23, 2016 at 3:04 PM, Mehdi Amini via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>> On Aug 23, 2016, at 3:03 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>>
>>
>>> On Aug 23, 2016, at 1:38 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>
>>>> I’ll give a shot.
>>>> Why is it a problem in this case?
>>>
>>> I don't think there is a practical problem is here, it is just not as
>>> obviously correct/safe.
>>>
>>> Also, one of the long term ideas I used to have was to change the api
>>> to completely hide the name, that would allow us to use O_TMPFILE when
>>> available, fixing the annoying issue that we leave temporary files on
>>> disk with the program "crashes" (like when restarting it in a
>>> debugger).
>>
>> OK!
>>
>> Tentative patch:
>>
>>
>
>
>
>>
>> I don’t have Gold to be able to check it though…
>>
>> —
>> Mehdi
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413