yaxunl added inline comments.

================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:763
+
+    // Create an intermediate temporary file for reading the bundles.
+    TempFileHandlerRAII TempFiles;
----------------
tra wrote:
> Having to create a temporary file in order to *list* content of the bundle 
> strikes me as rather odd.
> It looks like in order to list the content we actually do bundle unpacking, 
> printing bundled content in the process, and discard the results afterwards. 
> Is that so?
> 
> Perhaps it would be better to refactor the code a bit and separate iteration 
> over the bundle from what each iteration does.
> E.g. make a function `forEachBundledFile(input, lambda)` and then pass a 
> function that writes things out for normal operations and a function which 
> just prints the info in case of `--list`. It may simplify the code a bit as 
> right now you have to copy/paste the loops iterating over ReadBundleStart.
done. thanks.


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

https://reviews.llvm.org/D92954

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to