[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

This revision was automatically updated to reflect the committed changes. Closed by commit rL355332: [ASTImporter] Handle built-in when importing SourceLocation and FileID (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 ___ cfe-commits mailing

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

shafik added a comment. @teemperor ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

balazske added a comment. Looks good now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

shafik added a comment. In D58743#1413249 , @lebedev.ri wrote: > Is there a test missing here? This origin of this fix is the LLDB expression parsing issue. We were not able to reduce to a test we could put in `ASTImpoterTest.cpp` but we have a LLDB

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

shafik updated this revision to Diff 188753. shafik marked 7 inline comments as done. shafik added a comment. Addressed comments on formatting and missing changes to the `Import_New` version of the code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

balazske added inline comments. Comment at: include/clang/AST/ASTImporter.h:342 // FIXME: Remove this version. -FileID Import(FileID); +FileID Import(FileID, bool isBuiltin=false); teemperor wrote: > `IsBuiltin`, not `isBuiltin` `IsBuiltin =

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

martong accepted this revision. martong added a reviewer: balazske. martong added a subscriber: balazske. martong added a comment. Shafik, this looks good to me, once teemperor's comments are addressed. Note, I added @balazske as a reviewer, he recently worked with importing the FileIDs, he may

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

lebedev.ri added a comment. Is there a test missing here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I think the idea of the patch is right. Not sure tough if having Import take a second parameter is consistent with the other Import functions (and if that even matters).

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

shafik created this revision. shafik added reviewers: martong, a.sidorin, teemperor, aaron.ballman. Herald added subscribers: jdoerfert, rnkovacs. Currently when we see a built-in we try and import the include location. Instead what we do now is find the buffer like we do for the invalid case