[PATCH] D92954: [clang-offload-bundler] Add option -list

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 9 19:40:01 PST 2020


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



More information about the cfe-commits mailing list