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

aaron.ballman added a comment. In D53751#1311037 , @martong wrote: > > 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

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

shafik added a comment. In D53751#1311037 , @martong wrote: > > 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

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

martong added a comment. > 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?

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

balazske added a comment. It is really "ugly" thing to have a Import_New and a Import for the same functionality. Without these a single large patch would have been needed for clang and another smaller one for LLDB that must be committed "at the same time". Now 1 dependent patch is under

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

aaron.ballman added a comment. In 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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL347685: [ASTImporter] Added Import functions for transition to new API. (authored by balazske, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

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

balazske updated this revision to Diff 175529. balazske added a comment. - Corrected Import_New(const Attr *) - Added missing Import_New with Selector and DeclarationName. - Split long lines. - Split long lines (ASTImporter.cpp). Rebase to newest master. Repository: rC Clang CHANGES SINCE

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

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53751/new/ https://reviews.llvm.org/D53751

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

balazske updated this revision to Diff 175212. balazske marked an inline comment as done. balazske added a comment. - Split long lines (ASTImporter.cpp). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53751/new/ https://reviews.llvm.org/D53751 Files:

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

balazske updated this revision to Diff 175207. balazske added a comment. - Split long lines. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53751/new/ https://reviews.llvm.org/D53751 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp Index:

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

a_sidorin added a subscriber: spyffe. a_sidorin added a comment. Hi Balazs, The changes look mostly fine to me. I think they can be accepted after some minor stylish fixes. Hi Shafik, > I think these changes make sense at a high level but I am not sure about the > refactoring strategy. I am

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

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

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

shafik added subscribers: rsmith, shafik. shafik added a comment. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. 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

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

balazske updated this revision to Diff 171292. balazske added a comment. - Added missing Import_New with Selector and DeclarationName. Repository: rC Clang https://reviews.llvm.org/D53751 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp Index: lib/AST/ASTImporter.cpp

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

balazske updated this revision to Diff 171278. balazske added a comment. - Corrected Import_New(const Attr *) Repository: rC Clang https://reviews.llvm.org/D53751 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp Index: lib/AST/ASTImporter.cpp

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

balazske created this revision. Herald added subscribers: cfe-commits, Szelethus, martong, dkrupp. Herald added a reviewer: a.sidorin. These Import_New functions should be used in the ASTImporter, and the old Import functions should not be used. Later the Import_New should be renamed to Import