aaron.ballman added inline comments.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:109-111
+ auto AllVectorVarRefs = utils::decl_ref_expr::allDeclRefExprs(
+ *VectorVarDecl, *LoopParent, *Result.Context);
+ for (const auto *Ref : AllVectorVarRefs) {
----------------
hokein wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > I'm not certain what types are being used here. Can you turn
> > > `AllVectorVarRefs` into something with an explicit type so that I can
> > > know what `Ref`'s type is?
> > I may not have been clear -- I don't mean that the variable name should
> > contain type information, I mean that the type should not be automatically
> > deduced. We only use `auto` when the type is spelled explicitly in the
> > initialization or is otherwise obvious from context (like range-based for
> > loops).
> I'd prefer to use `auto` to initialize `AllVectorVarRefExprs`, as its type is
> `SmallPtrSet<const DeclRefExpr *, 16>`, which is a long and noisy name. Using
> `auto` can increases readability here.
It doesn't increase readability when someone reviewing the code has no idea
what types are involved and has to ask you for clarification. `auto` should
only be used when the type is obvious from the context or spelling the type is
*really* noisy. A single level of template goop is a very small amount of noise.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:116
+ *VectorVarDecl, *LoopParent, *Result.Context);
+ for (const DeclRefExpr* RefExpr : AllVectorVarRefExprs) {
+ // Skip cases where there are usages (defined as DeclRefExpr that refers to
----------------
Formatting (and you can use `auto` here once you correct the type above).
https://reviews.llvm.org/D31757
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
