[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-08 Thread Aleksei Sidorin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL331762: [ASTImporter] Properly import SourceLocations of Attrs (authored by a.sidorin, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-08 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl updated this revision to Diff 145660. r.stahl added a comment. Didn't see that overload, thanks! I need someone to commit this for me. https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-08 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Looks good! Comment at: lib/AST/ASTImporter.cpp:2650 + for (const auto *A : D->attrs()) +ToIndirectField->addAttr(Importer.Import(const_cast(A)));

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-08 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl added inline comments. Comment at: include/clang/AST/ASTImporter.h:137 +/// +/// \returns the equivalent attribute in the "to" context, or NULL if an +/// error occurred. a.sidorin wrote: > nullptr I tried to stay consistent with the other

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-08 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl updated this revision to Diff 145641. r.stahl marked 6 inline comments as done. https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp test/Import/attr/test.cpp Index: test/Import/attr/Inputs/S.cpp

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-07 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Sorry, two more nits. Comment at: include/clang/AST/ASTImporter.h:137 +/// +/// \returns the equivalent attribute in the "to" context, or NULL if an +/// error occurred. nullptr Comment at:

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-07 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Hi Rafael! Please find my comments inline. Comment at: lib/AST/ASTImporter.cpp:2650 + for (const auto *A : D->attrs()) +ToIndirectField->addAttr(Importer.Import(const_cast(A))); Could we just remove 'const' qualifier from `A`

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-07 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl updated this revision to Diff 145486. r.stahl edited the summary of this revision. r.stahl added a comment. full patch with test https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-05-03 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl added a comment. Maybe this is a user error of CrossTU, but it seemed to import a FuncDecl with attributes, causing the imported FuncDecl to have all those attributes twice. That's why I thought merging would maybe make sense. However I did not encounter any issue with the duplicate

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-04-27 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Hi all. I'm already here, invited by the Herald - just had no time to take a look (will change the script to add me as a reviewer). But thank you anyway! :) In the initial implementation

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-04-26 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a reviewer: a.sidorin. NoQ added a comment. +Alexey because he's the `ASTImporter` guy. Repository: rC Clang https://reviews.llvm.org/D46115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-04-26 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl added a comment. This is unfinished. Posting to make you aware of the issue. There are other occurances of imported attrs without importing the srcloc in: - VisitIndirectFieldDecl - VisitAttributedStmt So I was thinking this would suggest to add a public API `ASTImporter::Import(Attr

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

2018-04-26 Thread Rafael Stahl via Phabricator via cfe-commits
r.stahl created this revision. r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, martong, a.sidorin, rnkovacs. The ASTImporter was failing to import the SourceLocation field of Attrs. Repository: rC Clang