[PATCH] D133705: [HIP] Fix unbundling archive

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:2907-2908
+        // which are not object files. Files with extension ".lib" is classified
+        // as TY_Object but they are actually archives, therefore should not be
+        // unbundled.
+        const StringRef LibFileExt = ".lib";
----------------
yaxunl wrote:
> tra wrote:
> > This is confusing. I do not understand how/why `therefore should not be unbundled` is inferred from `they are actually archives`.
> > The patch description says that not unbundling the archives is that the patch is intended to fix.  The tests below with `MSVC` label show that we do unbundle the inputs with `.lib` extension.
> > 
> > Should this comment be fixed/updated?  What do I miss? 
> Here the driver is trying to unbundle bundled objects. This is different from unbundle archives. clang classifies '.lib' files as objects. If we do not exclude '.lib' files here, they will be incorrectly unbundled as objects here. The unbundling of '.lib' as archives should be done at other places.
> 
> Since this comment is confusing, I will change it to:
> 
> 'therefore should not be unbundled as objects here. They will be handled at other places.'
Thank you for the explanation. It makes more sense to me now. 


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

https://reviews.llvm.org/D133705