[PATCH] D47698: [ASTImporter] import macro source locations

This revision was automatically updated to reflect the committed changes. r.stahl marked an inline comment as done. Closed by commit rC336269: [ASTImporter] import macro source locations (authored by r.stahl, committed by ). Changed prior to commit:

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl marked an inline comment as done. r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc()); +SourceLocation ToExLocS =

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 152633. r.stahl marked 5 inline comments as done. r.stahl added a comment. improved code quality; added nested macro test. it "works", but is disabled because it revealed another bug: the function end location is not imported. will send a patch

[PATCH] D47698: [ASTImporter] import macro source locations

a.sidorin added a comment. Hi Rafael, I think the patch is great. But, honestly, I have never dealt with SourceLocation machinery closely, so some things are a bit unclear to me. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo =

[PATCH] D47698: [ASTImporter] import macro source locations

a.sidorin added a comment. Hi Rafael, I apologize for the delay in review and hope to get ASTImporter patches reviewed on this weekend. Comment at: lib/AST/ASTImporter.cpp:7054 + // Map the FileID for to the "to" source manager. FileID ToID; 'for to'

[PATCH] D47698: [ASTImporter] import macro source locations

martong added a comment. In https://reviews.llvm.org/D47698#1140629, @thakis wrote: > This code is live when reading pchs, correct? Does this have any measurable > perf impact on deserializing pchs for, say, Cocoa.h or Windows.h? No. The deserialization of a PCH is handled in a completely

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added a comment. In https://reviews.llvm.org/D47698#1140629, @thakis wrote: > This code is live when reading pchs, correct? Does this have any measurable > perf impact on deserializing pchs for, say, Cocoa.h or Windows.h? I don't know for sure, but it should be used - yes. I have not

[PATCH] D47698: [ASTImporter] import macro source locations

thakis added a comment. This code is live when reading pchs, correct? Does this have any measurable perf impact on deserializing pchs for, say, Cocoa.h or Windows.h? Repository: rC Clang https://reviews.llvm.org/D47698 ___ cfe-commits mailing

[PATCH] D47698: [ASTImporter] import macro source locations

balazske added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:1534 +} +TEST_P(ASTImporterTestBase, ImportSourceLocs) { + Decl *FromTU = getTuDecl( This test causes every case for expansion (macro, macro arg) to be executed at import?

[PATCH] D47698: [ASTImporter] import macro source locations

martong added reviewers: balazske, xazax.hun. martong added subscribers: balazske, xazax.hun. martong added a comment. Adding @balazske and @xazax.hun as reviewers. I think if it gets one more approve then we could merge. I'd like to speed up the things here ... we can't expect Aleksei to

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 152436. r.stahl marked 5 inline comments as done. r.stahl added a comment. addressed review comment, but there is nothing like FullSourceRange to improve everything Repository: rC Clang https://reviews.llvm.org/D47698 Files: lib/AST/ASTImporter.cpp

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc()); +SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());

[PATCH] D47698: [ASTImporter] import macro source locations

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. This patch is really useful and LGTM! Just found some minor things. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 151138. r.stahl added a comment. remove stray include Repository: rC Clang https://reviews.llvm.org/D47698 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added a comment. The split-token case should be covered by this, because it takes the correct TokenLen and the isTokenRange flag. Other than that the split-token ExpansionInfo is indistinguishable. What about loaded source locations? Do they need to be handled specifically or does

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl created this revision. r.stahl added reviewers: a.sidorin, klimek, martong. Herald added subscribers: cfe-commits, rnkovacs. Implement full import of macro expansion info with spelling and expansion locations. Repository: rC Clang https://reviews.llvm.org/D47698 Files: