This revision was automatically updated to reflect the committed changes.
Closed by commit rL282486: Workaround ASTMatcher crashes. Added some more test
cases. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D24862?vs=72284&id=72640#toc
Repository:
rL LLVM
https://re
hokein accepted this revision.
hokein added a comment.
LGTM.
https://reviews.llvm.org/D24862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM, thank you!
Comment at: change-namespace/ChangeNamespace.cpp:279-281
@@ -276,3 +278,5 @@
Finder->addMatche
ioeric updated this revision to Diff 72284.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Addressed review comments.
https://reviews.llvm.org/D24862
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-
ioeric added a comment.
Thanks for the comments!
Comment at: change-namespace/ChangeNamespace.cpp:279
@@ -276,2 +278,3 @@
Finder->addMatcher(
- usingDecl(hasAnyUsingShadowDecl(IsInMovedNs)).bind("using_decl"), this);
+ usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(de
aaron.ballman added a comment.
In https://reviews.llvm.org/D24862#550665, @ioeric wrote:
> Sorry for the confusion. I guess I should've been clearer in the comments and
> patch summary... The changes would've been what we wanted even without the
> underlying bugs and would not be reverted after
ioeric added a comment.
In https://reviews.llvm.org/D24862#550654, @aaron.ballman wrote:
> In https://reviews.llvm.org/D24862#550646, @ioeric wrote:
>
> > Acked, and I totally agree with you :) It's just that the change in this
> > patch would still be valid after the underlying bugs are fixed,
ioeric updated this revision to Diff 72275.
ioeric added a comment.
- Update comment.
https://reviews.llvm.org/D24862
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/tool/ClangChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-nam
aaron.ballman added a comment.
In https://reviews.llvm.org/D24862#550646, @ioeric wrote:
> Acked, and I totally agree with you :) It's just that the change in this
> patch would still be valid after the underlying bugs are fixed, so I thought
> it was fine to fix those bugs after this.
I migh
ioeric updated this revision to Diff 72273.
ioeric added a comment.
- Update comments.
https://reviews.llvm.org/D24862
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/tool/ClangChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-na
ioeric added a comment.
In https://reviews.llvm.org/D24862#550637, @aaron.ballman wrote:
> In https://reviews.llvm.org/D24862#550628, @ioeric wrote:
>
> > In https://reviews.llvm.org/D24862#550615, @aaron.ballman wrote:
> >
> > > Should this perhaps be fixed in the AST matchers rather than in the
aaron.ballman added a comment.
In https://reviews.llvm.org/D24862#550628, @ioeric wrote:
> In https://reviews.llvm.org/D24862#550615, @aaron.ballman wrote:
>
> > Should this perhaps be fixed in the AST matchers rather than in the check
> > itself?
>
>
> I'll file bugs for the crashes, but the fi
ioeric added a comment.
In https://reviews.llvm.org/D24862#550615, @aaron.ballman wrote:
> Should this perhaps be fixed in the AST matchers rather than in the check
> itself?
I'll file bugs for the crashes, but the fix in this patch is not that "dirty" -
it simply changes the order of matcher
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added reviewers: klimek, sbenza.
aaron.ballman added a comment.
Should this perhaps be fixed in the AST matchers rather than in the check
itself?
https://reviews.llvm.org/D24862
___
cfe
ioeric updated this revision to Diff 72269.
ioeric added a comment.
- Update comments
https://reviews.llvm.org/D24862
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/tool/ClangChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-nam
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
- UsingDecl matcher crashed when `UsingShadowDecl` has no parent map.
Workaround by moving parent check into `UsingDecl`.
- FunctionDecl matcher crashed when there is a lambda defined in paramet
16 matches
Mail list logo
