a.sidorin added a comment.
Main revisions: https://reviews.llvm.org/rL292776,
https://reviews.llvm.org/rL292778. Sorry for not mentioning them in
Differential Revision.
Repository:
rL LLVM
https://reviews.llvm.org/D26753
___
cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292779: ASTImporter: quick test fix (authored by a.sidorin).
Changed prior to commit:
https://reviews.llvm.org/D26753?vs=79054=85332#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26753
Files:
a.sidorin added a comment.
I got it. I have hard-coded paths in CHECK-lines so these tests are passed on
my machine but not on other. Thank you Kareem!
https://reviews.llvm.org/D26753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
a.sidorin added a comment.
Kareem, I have re-checked it and I cannot see the failure. But I'm not going to
commit it until NY holidays end (and, anyway, I will not commit it if somebody
has failing tests). Could you describe your configuration?
https://reviews.llvm.org/D26753
a.sidorin added a comment.
I didn't notice this failure but I'll recheck this. Thank you Kareem!
https://reviews.llvm.org/D26753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
khazem added a comment.
Sorry for the late comment, but one of the tests that this introduces is
breaking on my end:
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/class-template-partial-spec.cpp:9:11:
error: expected string not found in input
// CHECK:
spyffe accepted this revision.
spyffe added a comment.
This revision is now accepted and ready to land.
Yeah, that test looks great! Thanks!
Comment at: lib/AST/ASTImporter.cpp:496
+return false;
+ if (DN1->isIdentifier())
+return
a.sidorin updated this revision to Diff 79054.
a.sidorin added a comment.
Add a simple test for import of complex `NestedNameSpecifierLoc`s.
https://reviews.llvm.org/D26753
Files:
include/clang/AST/TemplateBase.h
lib/AST/ASTImporter.cpp
a.sidorin added a comment.
Thank you! I'll update this review again when I have a test for
NestedNameSpecifierLocs.
Comment at: lib/AST/ASTImporter.cpp:458
+ }
+ return true;
+}
spyffe wrote:
> Is this really an appropriate default result? I would argue
a.sidorin updated this revision to Diff 78382.
a.sidorin added a comment.
Address review comments; fix tests.
https://reviews.llvm.org/D26753
Files:
include/clang/AST/TemplateBase.h
lib/AST/ASTImporter.cpp
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp
spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.
This looks amazing. I have a few minor quibbles and a testing concern, but
overall this looks like a great step forward! Thank you!
Comment at:
khazem added a comment.
Thanks very much for this patch! It certainly fixes the infinite recursion
issue on our codebase. It LGTM, but I'd like to add a test case before landing
it.
https://reviews.llvm.org/D26753
___
cfe-commits mailing list
a.sidorin created this revision.
a.sidorin added reviewers: spyffe, khazem.
a.sidorin added a subscriber: cfe-commits.
- Support template partial specialization
- Avoid infinite recursion in IsStructurallyEquivalent for TemplateArgument
with implementing IsStructurallyEquivalent for TemplateName
13 matches
Mail list logo
