This revision was automatically updated to reflect the committed changes.
Closed by commit rL266292: [ASTImporter] Implement some expression-related AST
node import. (authored by dergachev).
Changed prior to commit:
http://reviews.llvm.org/D14286?vs=53389&id=53692#toc
Repository:
rL LLVM
ht
spyffe accepted this revision.
spyffe added a comment.
This revision is now accepted and ready to land.
This all looks fine to me, and the LLDB test suite is happy also.
http://reviews.llvm.org/D14286
___
cfe-commits mailing list
cfe-commits@lists.l
Aleksei,
it looks like I might have made a bad assumption in my ImportArray – namely
that there were some entities that required arrays of things they’re
constructed with to be allocated in the ASTContext. Looking at the
constructors for those entities, it looks like most of them actually do t
spyffe added a subscriber: spyffe.
spyffe added a comment.
Aleksei,
it looks like I might have made a bad assumption in my ImportArray – namely
that there were some entities that required arrays of things they’re
constructed with to be allocated in the ASTContext. Looking at the
constructors
a.sidorin updated this revision to Diff 53389.
a.sidorin added a comment.
Update to current master;
Fix a memory leak introduced in cf8ccff5: an array is allocated in ImportArray
and never freed.
http://reviews.llvm.org/D14286
Files:
include/clang/AST/Type.h
lib/AST/ASTImporter.cpp
unitt
spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.
Overall this is a great improvement and I look forward to taking advantage of
this patch in LLDB!
I had one specific nit about `NullPtrTy`, and one more general comment about
your i
a.sidorin added a comment.
Is this patch OK to submit?
http://reviews.llvm.org/D14286
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
With the latest changes unit tests are successfully passed on Windows!
You can go forward in completing the fix.
Thanks,
--Serge
2016-03-18 21:44 GMT+06:00 Aleksei Sidorin :
> a.sidorin updated this revision to Diff 51027.
> a.sidorin added a comment.
>
> Serge, thank you for help!
> GNUNullExpr
a.sidorin updated this revision to Diff 51027.
a.sidorin added a comment.
Serge, thank you for help!
GNUNullExpr was 'long' on my platform. I weakened a matcher to check only if it
has integer type.
ParenListExpr issue was also successfully reproduced with
'fdelayed-template-parsing' and fixed.
sepavloff added a comment.
The last changes didn't help.
AST generated in `ImportGNUNullExpr` is:
TranslationUnitDecl 0xc46238 <>
…
| `-BuiltinType 0xc46290 'char'
`-FunctionDecl 0xc468a0 col:6 declToImport 'void
(void)'
`-CompoundStmt 0xc46948
`-GNUNullExpr 0xc46938 'in
a.sidorin updated this revision to Diff 50741.
a.sidorin added a comment.
An attempt to fix ParenListExpr test on Windows
http://reviews.llvm.org/D14286
Files:
include/clang/AST/Type.h
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/CMakeLists.txt
Index: unittes
sepavloff added a subscriber: sepavloff.
sepavloff added a comment.
On Windows (VS2015 update1) two unit tests fail:
[ RUN ] ImportExpr.ImportGNUNullExpr
input.cc:1:23: warning: expression result unused [-Wunused-value]
void declToImport() { __null; }
^~
G
On Windows (VS2015 update1) two unit tests fail:
```
[ RUN ] ImportExpr.ImportGNUNullExpr
input.cc:1:23: warning: expression result unused [-Wunused-value]
void declToImport() { __null; }
^~
G:\arbeit\llvm2\llvm\tools\clang\unittests\AST\ASTImporterTest.cpp(149):
err
a.sidorin added a comment.
Serge: BTW, tests for CXXBoolLiteralExpr are presented in this patch. There is
no test //exactly// for it but at least one test uses bool literal and matches
it successfully with cxxBoolLiteral() matcher.
http://reviews.llvm.org/D14286
a.sidorin removed rL LLVM as the repository for this revision.
a.sidorin updated this revision to Diff 50140.
a.sidorin added a comment.
Stylish fixes.
Serge: ASTMatcher review is complete so there should be no more build problems.
Could you try again?
Also, I'd prefer to add your changes in the
sepavloff added a comment.
I still cannot build project with your changes, now compiler cannot find symbol
`hasSubStmt`. When committing the change please make sure that all
prerequisites are committed and unit tests run successfully.
I would recommend you to take tests from http://reviews.llvm
a.sidorin added a comment.
Serge: Sorry, that's because I forgot to add a dependence. There should be a
patch implementing these matchers.
Repository:
rL LLVM
http://reviews.llvm.org/D14286
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
2016-02-19 20:52 GMT+06:00 Aleksei Sidorin :
> a.sidorin added a comment.
>
> Serge Pavlov: I'll enable tests from you patch (
> http://reviews.llvm.org/D14224) after all node types your patch supports
> will be supported. If you're agree, of course.
>
> Yes, sure, that would be nice!
I cannot c
a.sidorin added a comment.
Serge Pavlov: I'll enable tests from you patch
(http://reviews.llvm.org/D14224) after all node types your patch supports will
be supported. If you're agree, of course.
Repository:
rL LLVM
http://reviews.llvm.org/D14286
_
a.sidorin updated the summary for this revision.
a.sidorin set the repository for this revision to rL LLVM.
a.sidorin updated this revision to Diff 48487.
a.sidorin added a comment.
Herald added a subscriber: aemerson.
Add AST matcher-based unit tests; add some additional nodes to pass the tests.
sepavloff added a comment.
If you are going to make unit tests, consider alternative approach also. In
repository `test-suite` the directory `SingleSource/UnitTests` contains runtime
tests obtained from single file. You could modify Makefile that runs these
tests in such a way that each test ru
a.sidorin added a comment.
I understand your position. However, we cannot check that we don't import
`BreakStmt` as `ContinueStmt` using just ASTMerge without any verification. As
I understand, unit tests is what we need because we need to check that the
overall imported type/expression is cor
sepavloff added a comment.
Unit test, of course, checks import facility but i would say this is overkill.
You need to create tests for all nodes you implement in imported, this is huge
amount of work.
I would propose you to move tests from http://reviews.llvm.org/D14224 for nodes
that are impl
a.sidorin updated this revision to Diff 40775.
a.sidorin added a comment.
Add language-related arguments for compilation.
http://reviews.llvm.org/D14286
Files:
include/clang/ASTMatchers/ASTMatchers.h
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/CMakeLists.txt
a.sidorin removed rL LLVM as the repository for this revision.
a.sidorin updated this revision to Diff 40769.
a.sidorin added a comment.
Herald added a subscriber: klimek.
Seems like I have found a way to test ASTImporter. What about some unit-tests?
A sample test using AST matcher is attached. I
2015-11-06 20:28 GMT+06:00 Aleksei Sidorin :
> a.sidorin marked 7 inline comments as done.
> a.sidorin added a comment.
>
> Thank you for your comments. I leaved some replies and will update
> revision.
> Something about lacking tests.
>
> 1. We cannot check if expression import is correct until w
a.sidorin updated this revision to Diff 39517.
a.sidorin marked an inline comment as done.
a.sidorin added a comment.
Some issues pointed on review were fixed.
Repository:
rL LLVM
http://reviews.llvm.org/D14286
Files:
lib/AST/ASTImporter.cpp
Index: lib/AST/ASTImporter.cpp
a.sidorin marked 7 inline comments as done.
a.sidorin added a comment.
Thank you for your comments. I leaved some replies and will update revision.
Something about lacking tests.
1. We cannot check if expression import is correct until we implement
FunctionDecl body import. I was going to upstre
sepavloff added a comment.
The fix must contain tests.
Comment at: lib/AST/ASTImporter.cpp:35
@@ +34,3 @@
+void ImportMultipleItems(IIter Ibegin, IIter Iend, OIter Obegin) {
+ ASTImporter &_Importer = Importer;
+ std::transform(Ibegin, Iend, Obegin,
---
a.sidorin created this revision.
a.sidorin added a reviewer: sepavloff.
a.sidorin added a subscriber: cfe-commits.
a.sidorin set the repository for this revision to rL LLVM.
This patch implements some expression-related AST node import. This is the
first patch in a series.
Supported nodes:
GCCAs
30 matches
Mail list logo
