Justin Bogner via llvm-dev
2016-Mar-19 00:12 UTC
[llvm-dev] Should we enable -Wrange-loop-analysis? (Was: [llvm] r261524 - Fix some abuse of auto...)
This is a pretty nice warning. Should we enable it for LLVM's build when the host compiler supports it? Benjamin Kramer via llvm-commits <llvm-commits at lists.llvm.org> writes:> Author: d0k > Date: Mon Feb 22 07:11:58 2016 > New Revision: 261524 > > URL: http://llvm.org/viewvc/llvm-project?rev=261524&view=rev > Log: > Fix some abuse of auto flagged by clang's -Wrange-loop-analysis. > > Modified: > llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp > llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp > llvm/trunk/tools/sancov/sancov.cc > > Modified: llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp > URL: > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp?rev=261524&r1=261523&r2=261524&view=diff > =============================================================================> --- llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp (original) > +++ llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp Mon Feb 22 07:11:58 2016 > @@ -119,7 +119,7 @@ class PPCBoolRetToInt : public FunctionP > Promotable.insert(P); > > SmallVector<const PHINode *, 8> ToRemove; > - for (const auto &P : Promotable) { > + for (const PHINode *P : Promotable) { > // Condition 2 and 3 > auto IsValidUser = [] (const Value *V) -> bool { > return isa<ReturnInst>(V) || isa<CallInst>(V) || isa<PHINode>(V) || > @@ -146,7 +146,7 @@ class PPCBoolRetToInt : public FunctionP > Promotable.erase(User); > ToRemove.clear(); > > - for (const auto &P : Promotable) { > + for (const PHINode *P : Promotable) { > // Condition 4 and 5 > const auto &Users = P->users(); > const auto &Operands = P->operands(); > @@ -199,11 +199,11 @@ class PPCBoolRetToInt : public FunctionP > // Presently, we only know how to handle PHINode, Constant, and Arguments. > // Potentially, bitwise operations (AND, OR, XOR, NOT) and sign extension > // could also be handled in the future. > - for (const auto &V : Defs) > + for (Value *V : Defs) > if (!isa<PHINode>(V) && !isa<Constant>(V) && !isa<Argument>(V)) > return false; > > - for (const auto &V : Defs) > + for (Value *V : Defs) > if (const PHINode *P = dyn_cast<PHINode>(V)) > if (!PromotablePHINodes.count(P)) > return false; > @@ -214,7 +214,7 @@ class PPCBoolRetToInt : public FunctionP > ++NumBoolCallPromotion; > ++NumBoolToIntPromotion; > > - for (const auto &V : Defs) > + for (Value *V : Defs) > if (!BoolToIntMap.count(V)) > BoolToIntMap[V] = translate(V); > > > Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=261524&r1=261523&r2=261524&view=diff > =============================================================================> --- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original) > +++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Mon Feb 22 07:11:58 2016 > @@ -473,7 +473,7 @@ void MemorySSA::verifyDomination(Functio > if (!MD) > continue; > > - for (const auto &U : MD->users()) { > + for (User *U : MD->users()) { > BasicBlock *UseBlock; > // Things are allowed to flow to phi nodes over their predecessor edge. > if (auto *P = dyn_cast<MemoryPhi>(U)) { > > Modified: llvm/trunk/tools/sancov/sancov.cc > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/sancov/sancov.cc?rev=261524&r1=261523&r2=261524&view=diff > =============================================================================> --- llvm/trunk/tools/sancov/sancov.cc (original) > +++ llvm/trunk/tools/sancov/sancov.cc Mon Feb 22 07:11:58 2016 > @@ -364,7 +364,7 @@ static void getObjectCoveragePoints(cons > if (SanCovAddrs.empty()) > Fail("__sanitizer_cov* functions not found"); > > - for (const auto Section : O.sections()) { > + for (object::SectionRef Section : O.sections()) { > if (Section.isVirtual() || !Section.isText()) // llvm-objdump does the same. > continue; > uint64_t SectionAddr = Section.getAddress(); > > > _______________________________________________ > llvm-commits mailing list > llvm-commits at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
Mehdi Amini via llvm-dev
2016-Mar-19 00:18 UTC
[llvm-dev] Should we enable -Wrange-loop-analysis? (Was: [llvm] r261524 - Fix some abuse of auto...)
What does this warning look like (and what cases does it catch)? Also does it have false positive? -- Mehdi> On Mar 18, 2016, at 5:12 PM, Justin Bogner via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > This is a pretty nice warning. Should we enable it for LLVM's build when > the host compiler supports it? > > Benjamin Kramer via llvm-commits <llvm-commits at lists.llvm.org> writes: >> Author: d0k >> Date: Mon Feb 22 07:11:58 2016 >> New Revision: 261524 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=261524&view=rev >> Log: >> Fix some abuse of auto flagged by clang's -Wrange-loop-analysis. >> >> Modified: >> llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp >> llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp >> llvm/trunk/tools/sancov/sancov.cc >> >> Modified: llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp?rev=261524&r1=261523&r2=261524&view=diff >> =============================================================================>> --- llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp (original) >> +++ llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp Mon Feb 22 07:11:58 2016 >> @@ -119,7 +119,7 @@ class PPCBoolRetToInt : public FunctionP >> Promotable.insert(P); >> >> SmallVector<const PHINode *, 8> ToRemove; >> - for (const auto &P : Promotable) { >> + for (const PHINode *P : Promotable) { >> // Condition 2 and 3 >> auto IsValidUser = [] (const Value *V) -> bool { >> return isa<ReturnInst>(V) || isa<CallInst>(V) || isa<PHINode>(V) || >> @@ -146,7 +146,7 @@ class PPCBoolRetToInt : public FunctionP >> Promotable.erase(User); >> ToRemove.clear(); >> >> - for (const auto &P : Promotable) { >> + for (const PHINode *P : Promotable) { >> // Condition 4 and 5 >> const auto &Users = P->users(); >> const auto &Operands = P->operands(); >> @@ -199,11 +199,11 @@ class PPCBoolRetToInt : public FunctionP >> // Presently, we only know how to handle PHINode, Constant, and Arguments. >> // Potentially, bitwise operations (AND, OR, XOR, NOT) and sign extension >> // could also be handled in the future. >> - for (const auto &V : Defs) >> + for (Value *V : Defs) >> if (!isa<PHINode>(V) && !isa<Constant>(V) && !isa<Argument>(V)) >> return false; >> >> - for (const auto &V : Defs) >> + for (Value *V : Defs) >> if (const PHINode *P = dyn_cast<PHINode>(V)) >> if (!PromotablePHINodes.count(P)) >> return false; >> @@ -214,7 +214,7 @@ class PPCBoolRetToInt : public FunctionP >> ++NumBoolCallPromotion; >> ++NumBoolToIntPromotion; >> >> - for (const auto &V : Defs) >> + for (Value *V : Defs) >> if (!BoolToIntMap.count(V)) >> BoolToIntMap[V] = translate(V); >> >> >> Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=261524&r1=261523&r2=261524&view=diff >> =============================================================================>> --- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original) >> +++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Mon Feb 22 07:11:58 2016 >> @@ -473,7 +473,7 @@ void MemorySSA::verifyDomination(Functio >> if (!MD) >> continue; >> >> - for (const auto &U : MD->users()) { >> + for (User *U : MD->users()) { >> BasicBlock *UseBlock; >> // Things are allowed to flow to phi nodes over their predecessor edge. >> if (auto *P = dyn_cast<MemoryPhi>(U)) { >> >> Modified: llvm/trunk/tools/sancov/sancov.cc >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/sancov/sancov.cc?rev=261524&r1=261523&r2=261524&view=diff >> =============================================================================>> --- llvm/trunk/tools/sancov/sancov.cc (original) >> +++ llvm/trunk/tools/sancov/sancov.cc Mon Feb 22 07:11:58 2016 >> @@ -364,7 +364,7 @@ static void getObjectCoveragePoints(cons >> if (SanCovAddrs.empty()) >> Fail("__sanitizer_cov* functions not found"); >> >> - for (const auto Section : O.sections()) { >> + for (object::SectionRef Section : O.sections()) { >> if (Section.isVirtual() || !Section.isText()) // llvm-objdump does the same. >> continue; >> uint64_t SectionAddr = Section.getAddress(); >> >> >> _______________________________________________ >> llvm-commits mailing list >> llvm-commits at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Justin Bogner via llvm-dev
2016-Mar-19 00:35 UTC
[llvm-dev] Should we enable -Wrange-loop-analysis?
Mehdi Amini <mehdi.amini at apple.com> writes:> What does this warning look like (and what cases does it catch)? Also > does it have false positive?I ran it on LLVM and clang and it only caught real (albeit minor) problems. Here, someone wrote & instead of * by mistake: SelectionDAGBuilder.cpp:2382:22: error: loop variable 'U' is always a copy because the range of type 'llvm::iterator_range<llvm::Value::user_iterator_impl<const llvm::User> >' does not return a reference [-Werror,-Wrange-loop-analysis] for (const auto &U : User->users()) { ^ SelectionDAGBuilder.cpp:2382:10: note: use non-reference type 'const llvm::User *' for (const auto &U : User->users()) { ^~~~~~~~~~~~~~~ Here, we're copying this struct for no good reason (someone missed a *): LoopLoadElimination.cpp:439:47: error: loop variable 'Cand' of type 'const (anonymous namespace)::StoreToLoadForwardingCandidate' creates a copy from type 'const (anonymous namespace)::StoreToLoadForwardingCandidate' [-Werror,-Wrange-loop-analysis] for (const StoreToLoadForwardingCandidate Cand : StoreToLoadDependences) { ^ LoopLoadElimination.cpp:439:10: note: use reference type 'const (anonymous namespace)::StoreToLoadForwardingCandidate &' to prevent copying for (const StoreToLoadForwardingCandidate Cand : StoreToLoadDependences) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~> -- > Mehdi > > On Mar 18, 2016, at 5:12 PM, Justin Bogner via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > This is a pretty nice warning. Should we enable it for LLVM's build when > the host compiler supports it? > > Benjamin Kramer via llvm-commits <llvm-commits at lists.llvm.org> writes: >> Author: d0k >> Date: Mon Feb 22 07:11:58 2016 >> New Revision: 261524 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=261524&view=rev >> Log: >> Fix some abuse of auto flagged by clang's -Wrange-loop-analysis. >> >> Modified: >> llvm/trunk/lib/Target/PowerPC/PPCBoolRetToInt.cpp >> llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp >> llvm/trunk/tools/sancov/sancov.cc