klimek added inline comments.
================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:556-557
@@ -555,1 +555,4 @@
DerefByValue = true;
+ // We can assume that we have at least one Usage, because
+ // usagesAreConst() returned false.
+ QualType Type = Usages[0].Expression->getType().getCanonicalType();
----------------
I'd add (non-const):
... we have at least one (non-const) Usage...
================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:658-663
@@ -646,7 +657,8 @@
+ IsTriviallyCopyable = BeginPointeeType.isTriviallyCopyableType(*Context);
} else {
// Check for qualified types to avoid conversions from non-const to const
// iterator types.
if (!Context->hasSameType(CanonicalInitVarType, CanonicalBeginType))
return;
}
----------------
Can you add a comment what FixerKind is in the else, so it's obvious why we
don't want to do anything else in here?
================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:667
@@ -654,1 +666,3 @@
+ Nodes.getNodeAs<QualType>(DerefByValueResultName);
+ DerefByValue = DerefByValueType != nullptr;
if (!DerefByValue) {
----------------
Btw, upstream style is to just use implicit null checks; probably not something
you wrote, but good to change while we're at it.
================
Comment at: clang-tidy/modernize/LoopConvertCheck.h:28-44
@@ -27,18 +27,19 @@
private:
void doConversion(ASTContext *Context, const VarDecl *IndexVar,
const VarDecl *MaybeContainer, StringRef ContainerString,
const UsageResult &Usages, const DeclStmt *AliasDecl,
bool AliasUseRequired, bool AliasFromForInit,
const ForStmt *TheLoop, bool ContainerNeedsDereference,
- bool DerefByValue, bool DerefByConstRef);
+ bool DerefByValue, bool IsTriviallyCopyable,
+ bool DerefByConstRef);
StringRef checkRejections(ASTContext *Context, const Expr *ContainerExpr,
const ForStmt *TheLoop);
void findAndVerifyUsages(ASTContext *Context, const VarDecl *LoopVar,
const VarDecl *EndVar, const Expr *ContainerExpr,
const Expr *BoundExpr,
bool ContainerNeedsDereference, bool DerefByValue,
- bool DerefByConstRef, const ForStmt *TheLoop,
- LoopFixerKind FixerKind);
+ bool IsTriviallyCopyable, bool DerefByConstRef,
+ const ForStmt *TheLoop, LoopFixerKind FixerKind);
std::unique_ptr<TUTrackingInfo> TUInfo;
----------------
I find it somewhat distressing that the variable order is completely different.
Perhaps it's time to pull out a struct that encapsulates the specific
attributes of the loop (the bools)?
http://reviews.llvm.org/D12675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
