klimek added a comment.
Note that I think we should add a regression test that will work with the
spelling loc but not with the expansion loc.
Something like:
#define A \
\
A
http://reviews.llvm.org/D12631
___
cfe-commits mailing list
cfe-c
angelgarcia updated this revision to Diff 34076.
angelgarcia added a comment.
Wooops, solved.
http://reviews.llvm.org/D12631
Files:
clang-tidy/modernize/LoopConvertUtils.cpp
clang-tidy/modernize/LoopConvertUtils.h
test/clang-tidy/modernize-loop-convert-extra.cpp
Index: test/clang-tidy/mo
alexfh accepted this revision.
alexfh added a reviewer: alexfh.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a comment.
Comment at: clang-tidy/modernize/LoopConvertUtils.h:211
@@ -210,3 +210,3 @@
/// \brief A class to encapsulate lowe
angelgarcia updated this revision to Diff 34054.
angelgarcia added a comment.
Yes, with SpellingLoc works as well, and it should be better.
http://reviews.llvm.org/D12631
Files:
clang-tidy/modernize/LoopConvertUtils.cpp
clang-tidy/modernize/LoopConvertUtils.h
test/clang-tidy/modernize-loo
klimek added inline comments.
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:463-470
@@ -462,1 +462,10 @@
+void ForLoopIndexUseVisitor::addUsage(const Usage &U) {
+ SourceLocation Begin = U.Range.getBegin();
+ if (Begin.isMacroID())
+Begin = Context->getSourceManage
angelgarcia updated this revision to Diff 34039.
angelgarcia marked 2 inline comments as done.
angelgarcia added a comment.
Ok! I wasn't sure, and a while would be correct in both cases :)
http://reviews.llvm.org/D12631
Files:
clang-tidy/modernize/LoopConvertUtils.cpp
clang-tidy/modernize/L
klimek added inline comments.
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:465
@@ +464,3 @@
+ SourceLocation Begin = U.Range.getBegin();
+ while (Begin.isMacroID())
+Begin = Context->getSourceManager().getExpansionLoc(Begin);
There should be no need
angelgarcia updated this revision to Diff 34035.
angelgarcia added a comment.
I think this solves the duplication problem. And I think is clearer and cheaper
than what I was trying to do before.
http://reviews.llvm.org/D12631
Files:
clang-tidy/modernize/LoopConvertUtils.cpp
clang-tidy/mode
angelgarcia added a comment.
Anyway, I just found that this needs a few more changes (it still does
duplicated replacementes inside some macros, and I have to find out why), so
don't bother with this for now.
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:465
@@ +464,3
alexfh added inline comments.
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:465
@@ +464,3 @@
+ if (UsageSet.insert(U).second) {
+Usages.push_back(U);
+return true;
Do you need both `Usages` and `UsageSet`?
Comment at: clang-tidy/
angelgarcia created this revision.
angelgarcia added a reviewer: klimek.
angelgarcia added subscribers: alexfh, cfe-commits.
The InitListExpr subtree is visited twice, this caused the check to do multiple
replacements. Added a set to avoid it.
http://reviews.llvm.org/D12631
Files:
clang-tidy/
11 matches
Mail list logo
