[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 28 14:48:42 PST 2018


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





More information about the cfe-commits mailing list