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=72640#toc
Repository:
rL LLVM
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
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:
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,
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
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:
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
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
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
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
___
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:
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
13 matches
Mail list logo
