[PATCH] D53751: [ASTImporter] Added Import functions for transition to new API.

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 27 12:31:56 PST 2018


aaron.ballman added a comment.

In D53751#1301551 <https://reviews.llvm.org/D53751#1301551>, @shafik wrote:

> 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.


I didn't actually see this comment get addressed other than to say it won't be a problem in practice, which I'm not certain I agree with. Was there a reason why this got commit before finding out if the reviewer with the concern agrees with your rationale? FWIW, I share the concern that having parallel APIs for any length of time is a dangerous thing. Given that "almost ready to go" is not "ready to go" but there's not an imminent release, I don't understand the rush to commit this.

When is the renaming and removal of the old API expected take place? Days? Weeks? Months?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53751





More information about the cfe-commits mailing list