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:
https://reviews.llvm.org/D46115?v
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
test/Import/attr/t
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)));
--
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 des
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
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: lib/AST/ASTIm
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` t
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
test/Import/attr/test.cpp
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 att
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
(https://raw.githubusercontent.com/haoNoQ/clang/summary-ipa-draft/lib/AST/ASTImpo
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
http://lists.llvm.org/cgi-bin/mailman/l
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 *
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
https://reviews.llvm.org/D461
13 matches
Mail list logo