martong added a comment.

> I reverted this change because it breaks a bunch of lldb tests (on MacOS, and 
> probably on other platforms too). To be clear, part of the reason I'm 
> reacting strongly here is that this is not the first patch this has come up 
> on. I'm worried about the broader trend of landing patches to ASTImporter 
> without proper reviews and testing as I am of this very instance. I really 
> think you should consider asking reviews to somebody who's familiar with 
> clang and test lldb as it's the main client of this functionality we have in 
> tree.

@davide I am terribly sorry about this issue. I just applied this patch on my 
Linux box, and it did not failed those tests which failed on the green lab 
buildbots. Could you please refer to the Linux buildbots which failed? Seems 
like there is a significant difference between the testsuites on Linux and 
MacOS. Our primary target is Linux and we do run the LLDB tests before we 
accept any commit into our fork and only then we continue with upstreaming to 
Phabricator.

Our platform in our CI is Linux only, but we are in the middle of getting a 
hosted MacOS to support our CI. I do hope that in the near future there will be 
no such issues. In the meantime, is there a way for us to execute a green lab 
MacOS build bot with a specific patch before a commit? If that is not possible, 
then we can just promise that we will monitor your buildbots and we will do the 
revert.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53818



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

Reply via email to