[PATCH] D70870: [IR] Use a reference in a range-based for
Mark de Wever via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 1 11:49:10 PST 2019
Mordante marked an inline comment as done.
Mordante added inline comments.
================
Comment at: llvm/lib/IR/Attributes.cpp:728
- for (const auto I : *this) {
+ for (const auto &I : *this) {
if (!I.isStringAttribute()) {
----------------
fhahn wrote:
> It might help a bit more with readability of the code (here and other places in the patch) if you expand the type instead of auto, if the type is not too long (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable)
According the that link it's common to use `auto` in a range-based for loop. However I agree using `*this` makes it a bit odd. I'd like the opinion of the reviewers to see how they feel about the usage of `auto` here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70870/new/
https://reviews.llvm.org/D70870
More information about the llvm-commits
mailing list