jhuber6 added inline comments.

================
Comment at: clang/test/Driver/linker-wrapper-libs.c:27
 //
 // Check that we extract a static library defining an undefined symbol.
 //
----------------
tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > How does this test test the functionality of the undefined symbol? E.g. 
> > > how does it fail now, before the patch?
> > > 
> > > Is there an explicit check we could to do to make sure things work as 
> > > intended as opposed to "there's no obvious error" which may also mean "we 
> > > forgot to process *undefined.bc".
> > Yeah, I wasn't sure how to define a good test for this. The problem I 
> > encountered before making this patch was that having another file that used 
> > an undefined symbol would override the `NewSymbol` check and then would 
> > prevent it from being extracted. So this checks that case.
> AFAICT, with -DUNDEFINED, the file would have only `extern int sym;`. CE says 
> suggests that it produces an embty bitcode file: 
> https://godbolt.org/z/EY9a8Pfeb
> 
> What exactly is supposed to be in the `*.undefined.bc` ?  If it's intended to 
> have an undefined reference to `sym` you need to add some sort of a reference 
> to it. 
> 
Good catch, forgot about that. It's why the other use of `extern sym` returns 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151839

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

Reply via email to