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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:188
+
+      if (Error Err = Func())
+        return Err;
----------------
tra wrote:
> Now, if we could save the triple in a `BundleInfo` when it's parsed, and pass `BundleInfo` to `Func()` that would make the iterator more useful.
> 
done


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:320-321
       ReadChars += TripleSize;
+      if (ListBundleIDs)
+        llvm::outs() << Triple << '\n';
 
----------------
tra wrote:
> I'm still not quite happy with the fact that the listing is interleaving with reading the bundle.
> I think the code would benefit from further refactoring that would separate bundle reading from what we do with the bundles we've read.
> 
good point. this can be moved to the lambda without incurring significant overhead.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:641
+    // start of each bundle which is done by forEachBundle.
+    return forEachBundle(Input, []() { return Error::success(); });
+  }
----------------
tra wrote:
> Once BundleInfo carries the triple, `ListBundleIDs` could then be changed to print the bundle info and moved into the FileHandler class:
> 
> ```
>     if (Error Err = ReadHeader(Input))
>         return Err;
> 
>     return forEachBundle(Input, [](BundleInfo bundle) { llvm::outs() << bundle.triple << '\n'});
> ```
> 
> All other printouts of the triple should no longer be necessary.
done


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

https://reviews.llvm.org/D92954