aaron.ballman added inline comments.
================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:95-96
+ const MatchFinder::MatchResult &Result) {
+ if (Result.Context->getDiagnostics().hasUncompilableErrorOccurred())
+ return;
+
----------------
hokein wrote:
> aaron.ballman wrote:
> > We don't usually add this test in to our check calls; why are you adding it
> > here?
> It will prevent some unexpected things happened when the translation unit
> fails to compile.
>
> We had a few experiences before. We encountered some misbehavior of some
> checks (e.g. https://reviews.llvm.org/rL294578) with a non-compilable TU, and
> then we added this statement to avoid the misbehavior.
>
I think that this should be moved up into whatever layer calls check(), because
it does not seem like the checks are actually valid if the TU has had a fatal
error. However, that doesn't have to be part of this patch.
================
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) {
----------------
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).
https://reviews.llvm.org/D31757
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
