martong added a subscriber: a_sidorin. martong added a comment. > I think these changes make sense at a high level but I am not sure about the > refactoring strategy. I am especially concerned we may end up in place where > all the effected users of the API don't get updated and we are stuck with > this parallel API. > Tagging in @rsmith since he has reviewed a lot of recent changes involving > ASTImpoter that I have seen recently and he will have a better feeling for > how these types of refactoring on handled on the clang side. I am mostly > focused on the lldb side but care about the ASTImporter since we depend on it.
Hi @shafik , I completely understand your concern. We modify ASTImporter in order to make cross translation unit (CTU) analysis working on C++ projects. During these modifications we carefully try to keep LLDB functionality intact. This patch is one of the last patches of a refactor in ASTImporter about using `Error` and `Expected<T>` as return types. We need this in CTU analysis because - we want to distinguish between different kind of errors - internally in ASTImporter we'd like to enforce the checking whether there has been any error during the import of any subexpressions After this patch our next patch would rename these `Import_New` functions to `Import` and the old `Import` function implementations returning with a raw pointer would be deleted. This indeed would effect LLDB, thus we are going to create an LLDB patch too. We already have that LLDB change in our fork (https://github.com/Ericsson/lldb/commit/7085d20) and soon we will put that in Phabricator too. Now, my concern is that waiting for the approve from @rsmith could take several months since he is usually very overwhelmed. Unfortunately, we have several other changes which depend on this change, so this is a blocker for us. Also, I think that @a_sidorin has the greatest experience in ASTImporter code. Would it be okay for you to accept this patch once Alexei approved it? Repository: rC Clang https://reviews.llvm.org/D53751 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
