[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong added a comment. In D71018#1924604 , @martong wrote: > - Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc I realized that the mentioned functions are redundant, because we have a while loop which iterates over all related

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 250570. martong added a comment. - Fix the base commit of this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 250568. martong added a comment. - Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 Files:

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 249350. martong marked an inline comment as done. martong added a comment. - Handle AttributedTypeLoc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 Files:

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8132 + return VisitTypeSpecTypeLoc(FTS); +llvm_unreachable("Unimplemented TypeLoc visitor function"); + } balazske wrote: > This assert

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8132 + return VisitTypeSpecTypeLoc(FTS); +llvm_unreachable("Unimplemented TypeLoc visitor function"); + } This assert is new related to the old code. Are really all TypeLoc's are

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 ___ cfe-commits mailing list

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 249106. martong marked 10 inline comments as done. martong added a comment. - Remove default visit functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 Files:

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3281 + // Import the function parameters. + SmallVector Parameters; balazske wrote: > shafik wrote: > > I am curious, why move this chunk of code up? > See comments at line 8238? I

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong added a comment. Thanks guys for the review and the comments! :) I am going to address all of them, but only during next week, I am on vacation for the rest of this week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8133 + Error VisitTypedefTypeLoc(TypedefTypeLoc From) { +return VisitTypeSpecTypeLoc(From); + } teemperor wrote: > I know the ASTImporter is doing these reimplementation of the

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3281 + // Import the function parameters. + SmallVector Parameters; shafik wrote: > I am curious, why move this chunk of code up? See comments at line 8238? Comment

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

teemperor added a comment. My only problem is the reimplementation of the default visit methods with (from what I can tell) the same behavior as the default. If you delete all of these then that would also fix Shafik's point that all the custom dispatch code is technically untested in the unit

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

shafik added a comment. A few comments and I would like @teemperor to give this a look as well but it looks good to me. Comment at: clang/lib/AST/ASTImporter.cpp:3281 + // Import the function parameters. + SmallVector Parameters; I am curious, why move

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong added a comment. @shafik Ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong edited reviewers, added: teemperor; removed: a.sidorin. martong added a comment. Herald added a reviewer: a.sidorin. @teemperor , @shafik Guys, could you please take a look on this patch? I don't think that type locations could have any effects on LLDB, but one may never know, so, it is

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. The patch looks good. Thanks! I would prefer to see more tests for other TypeLoc cases, but I think they could be added in the following patches. @shafik Shafik, do you have any

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 238267. martong marked an inline comment as done. martong added a comment. - Move the setting of parameters of FunctionProtoTypeLoc to TypeLocImporter - Add tests for TypeLoc import in case of functions Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8043 + + Error VisitFunctionTypeLoc(FunctionTypeLoc From) { +auto To = ToL.castAs(); balazske wrote: > martong wrote: > > a_sidorin wrote: > >

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8043 + + Error VisitFunctionTypeLoc(FunctionTypeLoc From) { +auto To = ToL.castAs(); martong wrote: > a_sidorin wrote: > > Does this import interacts well with type loc import

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong marked an inline comment as done. martong added a comment. > Are you going to add any tests in the following patches? I don't have any tests for now, but in a later patch we can create them. Comment at: clang/lib/AST/ASTImporter.cpp:8043 + + Error

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

a_sidorin added a comment. Finally, this FIXME is addressed. Thanks! Are you going to add any tests in the following patches? It looks almost good to me, but I have a question inline. Comment at: clang/lib/AST/ASTImporter.cpp:7914 + +#define IMPORT_TYPE_LOC(NAME)

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 232336. martong added a comment. IMPORT -> IMPORT_TYPE_LOC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71018/new/ https://reviews.llvm.org/D71018 Files: clang/lib/AST/ASTImporter.cpp Index:

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7914 + +#define IMPORT(NAME) \ + if (auto ImpOrErr = Importer.Import(From.get##NAME())) \ I think this

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong created this revision. martong added reviewers: balazske, a_sidorin. Herald added subscribers: cfe-commits, teemperor, gamesh411, Szelethus, jfb, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. martong added a parent