tra added inline comments.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:176
+ /// List bundle IDs in \a Input.
+ virtual Error listBundleIDs(MemoryBuffer &Input) {
+ if (Error Err = ReadHeader(Input))
----------------
Now that listBundleIDs is only used by ` ListBundleIDsInFile()`, perhaps it
could all be simplified and moved out of the class.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:882
+// List bundle IDs. Return true if an error was found.
+static Error ListBundleIDsInFile() {
+ // Open Input file.
----------------
I'd pass InputFileNames as an argument. Makes it easier to tell what the
function needs.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1031-1035
+ Error = true;
+ reportError(createStringError(
+ errc::invalid_argument,
+ "for the --outputs option: must be specified at least once!"));
+ }
----------------
Does it make sense to continue once we know that CLI options are wrong?
If we just early-exit with an error that may help simplifying the code below a
bit.
================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1047
+ errc::invalid_argument, "-unbundle and -list cannot be used
together"));
+ } else if (ListBundleIDs) {
+ if (InputFileNames.size() != 1) {
----------------
Perhaps we can separate list option processing from bundle/unbundle?
I think if we could do something like this it would be more readable:
```
if (ListBundleIDs) {
if (Unbundle) {
error...
exit.
}
... other list-specific checks...
ListBundleIDsInFile(InputFileNames)
exit 0;
}
// complicated bundle/unbundle logic can proceed without having to bother about
`list` option.
```
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
