Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
On Mon, Sep 12, 2022 at 12:17 PM David Blaikie wrote: > > On Sat, Sep 10, 2022 at 3:01 PM Kazu Hirata wrote: > > > > Thank you Aaron and David for your inputs. > > > > First and foremost, I apologize if I made your job harder by increasing the > > number of commits you have to peel to get to the real author. > > > > I hear that we are moving toward github pull requests. A casual search > > tells me that there are some add-ons to integrate clang-tidy into the code > > review platform, so I am hoping we can use something like that to get each > > patch right first time. > > > > Going forward, I'll take git churn and the difficulty of backsliding as big > > factors in doing future clenaups. For example, it's probably a good idea > > to delete a function that hasn't been used for many years (excluding dump > > functions and such). Library standardization (like the recent removal of > > llvm::array_lengthof in favor of std::size) is less good in terms of git > > churn, but it's very unlikely for somebody to re-introduce > > llvm::array_lengthof. > > I think API deprecations (where the API can be completely removed > eventually, and marked as deprecated/all in-tree usage removed within > O(weeks/months)) especially for cases like the core/support libraries > with relatively many uses, and relatively small APIs are great - if we > get into the territory of naming convention cleanup, that gets more > debatable because there's wide APIs with many naming violations and > then we need more community discussion about what direction we're > going (there are multiple lingering naming conventions, some > discussions about moving to different ones in the future, so maybe > churn to meet the current letter of the style guide would be better > spent after changing the style guide with those directions in mind, > etc). +1 > For stylistic things like range-based-for conversion, const auto*, etc > - yeah, there's some wriggle room depending on how uncontentious the > conversion is, I think. Agreed. > (there's also some way to mark certain changes as ignorable by git? > Maybe using that more frequently would help lower the cost of these > sort of changes - broader discussion on discourse about ways to > enable/lower the cost of these sort of changes would probably be good > - I think as much as we can make these sort of changes cheaper/less > problematic, to make them more encouraged, is a really good thing to > do) Strong +1! I want to find a way to encourage cleanups as much as we reasonably can. I think using something like this: https://akrabat.com/ignoring-revisions-with-git-blame/ is probably worth exploring. ~Aaron > > > > > Thanks, > > > > Kazu Hirata > > > > > > On Fri, Sep 9, 2022 at 5:27 AM Aaron Ballman wrote: > >> > >> On Thu, Sep 8, 2022 at 12:37 PM David Blaikie wrote: > >> > > >> > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes > >> > across the LLVM project for a while now, most, at least I think, are > >> > quite welcome (things like switching to range-based-for, std > >> > algorithms over llvm ones, llvm algorithms over manually written > >> > loops, etc). But yeah, there's some threshold below which the churn > >> > might not be worth the benefit - especially if the change doesn't come > >> > along with tooling to enforce the invariant is maintained in the > >> > future (if it's easy to make mistakes like this one - we'll regress it > >> > and need to do cleanup again in the future) > >> > >> Thanks for speaking up, because I also waffled a bit on whether I > >> called this out or not. :-) > >> > >> > Also, for this particular one, I wonder if in some cases this sort of > >> > automatic transformation isn't ideal - if something is a pointer, but > >> > that's an implementation detail, rather than an intentional feature of > >> > an API (eg: the pointer-ness might be hidden behind a typedef and used > >> > as an opaque handle, without any dereferencing, etc) > >> > >> Agreed. > >> > >> > I think it'd be really good to have some discussion on discourse about > >> > if/how some of these cleanups could be formed into a way to > >> > enforce/encourage the invariant to be maintained going forward - > >> > clang-tidy (assuming that's the basis for the tooling Kazu's using to > >> > make these changes in the first place) integration into the LLVM build > >> > in some way, etc. > >> > >> I think that's a good idea! We want to encourage cleanups, but we > >> don't want to encourage churn, and I think it's somewhat subjective > >> where to draw that line. Having some more community awareness around > >> that would be beneficial. I'm especially interested in how we balance > >> between making incremental style improvements to the project and > >> keeping our git blame logs useful. I'm seeing a lot more git blames > >> that require several steps to get to an interesting commit because of > >> the number of NFCs and reverts/recommits. Unfortunately, the tooling > >> around
Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
On Sat, Sep 10, 2022 at 3:01 PM Kazu Hirata wrote: > > Thank you Aaron and David for your inputs. > > First and foremost, I apologize if I made your job harder by increasing the > number of commits you have to peel to get to the real author. > > I hear that we are moving toward github pull requests. A casual search tells > me that there are some add-ons to integrate clang-tidy into the code review > platform, so I am hoping we can use something like that to get each patch > right first time. > > Going forward, I'll take git churn and the difficulty of backsliding as big > factors in doing future clenaups. For example, it's probably a good idea to > delete a function that hasn't been used for many years (excluding dump > functions and such). Library standardization (like the recent removal of > llvm::array_lengthof in favor of std::size) is less good in terms of git > churn, but it's very unlikely for somebody to re-introduce > llvm::array_lengthof. I think API deprecations (where the API can be completely removed eventually, and marked as deprecated/all in-tree usage removed within O(weeks/months)) especially for cases like the core/support libraries with relatively many uses, and relatively small APIs are great - if we get into the territory of naming convention cleanup, that gets more debatable because there's wide APIs with many naming violations and then we need more community discussion about what direction we're going (there are multiple lingering naming conventions, some discussions about moving to different ones in the future, so maybe churn to meet the current letter of the style guide would be better spent after changing the style guide with those directions in mind, etc). For stylistic things like range-based-for conversion, const auto*, etc - yeah, there's some wriggle room depending on how uncontentious the conversion is, I think. (there's also some way to mark certain changes as ignorable by git? Maybe using that more frequently would help lower the cost of these sort of changes - broader discussion on discourse about ways to enable/lower the cost of these sort of changes would probably be good - I think as much as we can make these sort of changes cheaper/less problematic, to make them more encouraged, is a really good thing to do) > > Thanks, > > Kazu Hirata > > > On Fri, Sep 9, 2022 at 5:27 AM Aaron Ballman wrote: >> >> On Thu, Sep 8, 2022 at 12:37 PM David Blaikie wrote: >> > >> > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes >> > across the LLVM project for a while now, most, at least I think, are >> > quite welcome (things like switching to range-based-for, std >> > algorithms over llvm ones, llvm algorithms over manually written >> > loops, etc). But yeah, there's some threshold below which the churn >> > might not be worth the benefit - especially if the change doesn't come >> > along with tooling to enforce the invariant is maintained in the >> > future (if it's easy to make mistakes like this one - we'll regress it >> > and need to do cleanup again in the future) >> >> Thanks for speaking up, because I also waffled a bit on whether I >> called this out or not. :-) >> >> > Also, for this particular one, I wonder if in some cases this sort of >> > automatic transformation isn't ideal - if something is a pointer, but >> > that's an implementation detail, rather than an intentional feature of >> > an API (eg: the pointer-ness might be hidden behind a typedef and used >> > as an opaque handle, without any dereferencing, etc) >> >> Agreed. >> >> > I think it'd be really good to have some discussion on discourse about >> > if/how some of these cleanups could be formed into a way to >> > enforce/encourage the invariant to be maintained going forward - >> > clang-tidy (assuming that's the basis for the tooling Kazu's using to >> > make these changes in the first place) integration into the LLVM build >> > in some way, etc. >> >> I think that's a good idea! We want to encourage cleanups, but we >> don't want to encourage churn, and I think it's somewhat subjective >> where to draw that line. Having some more community awareness around >> that would be beneficial. I'm especially interested in how we balance >> between making incremental style improvements to the project and >> keeping our git blame logs useful. I'm seeing a lot more git blames >> that require several steps to get to an interesting commit because of >> the number of NFCs and reverts/recommits. Unfortunately, the tooling >> around viewing git blames of large files (like Clang tends to have) >> makes these sorts of commits surprisingly painful when you do have to >> dig to see where changes came from. (So I find myself having far less >> concern when TransGCAttrs.cpp (~350LoC) gets a cleanup like this >> compared to SemaExpr.cpp (~21kLoC), which suggests to me we should >> maybe strongly consider splitting more of these massive files up so >> that churn is less painful.) >> >> > & yeah,
Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
On Sat, Sep 10, 2022 at 6:01 PM Kazu Hirata wrote: > > Thank you Aaron and David for your inputs. > > First and foremost, I apologize if I made your job harder by increasing the > number of commits you have to peel to get to the real author. No worries at all! It's all a tradeoff. :-) > I hear that we are moving toward github pull requests. A casual search tells > me that there are some add-ons to integrate clang-tidy into the code review > platform, so I am hoping we can use something like that to get each patch > right first time. That would be lovely (though we could do the same with our current precommit CI pipeline; there's no need for us to wait for a switch to GitHub). > Going forward, I'll take git churn and the difficulty of backsliding as big > factors in doing future clenaups. For example, it's probably a good idea to > delete a function that hasn't been used for many years (excluding dump > functions and such). Library standardization (like the recent removal of > llvm::array_lengthof in favor of std::size) is less good in terms of git > churn, but it's very unlikely for somebody to re-introduce > llvm::array_lengthof. Thanks! If we're going to err on one side or the other, I think it's better to err on the side that leaves us with the best code base because that's what everyone interacts with the most. But I also like David's idea of starting a community discussion to see where folks want to draw the line these days. Perhaps the only kind of sweeping changes we want to avoid are whitespace/line ending/formatting changes and we're fine with other kinds of sweeping improvements. ~Aaron > > Thanks, > > Kazu Hirata > > > On Fri, Sep 9, 2022 at 5:27 AM Aaron Ballman wrote: >> >> On Thu, Sep 8, 2022 at 12:37 PM David Blaikie wrote: >> > >> > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes >> > across the LLVM project for a while now, most, at least I think, are >> > quite welcome (things like switching to range-based-for, std >> > algorithms over llvm ones, llvm algorithms over manually written >> > loops, etc). But yeah, there's some threshold below which the churn >> > might not be worth the benefit - especially if the change doesn't come >> > along with tooling to enforce the invariant is maintained in the >> > future (if it's easy to make mistakes like this one - we'll regress it >> > and need to do cleanup again in the future) >> >> Thanks for speaking up, because I also waffled a bit on whether I >> called this out or not. :-) >> >> > Also, for this particular one, I wonder if in some cases this sort of >> > automatic transformation isn't ideal - if something is a pointer, but >> > that's an implementation detail, rather than an intentional feature of >> > an API (eg: the pointer-ness might be hidden behind a typedef and used >> > as an opaque handle, without any dereferencing, etc) >> >> Agreed. >> >> > I think it'd be really good to have some discussion on discourse about >> > if/how some of these cleanups could be formed into a way to >> > enforce/encourage the invariant to be maintained going forward - >> > clang-tidy (assuming that's the basis for the tooling Kazu's using to >> > make these changes in the first place) integration into the LLVM build >> > in some way, etc. >> >> I think that's a good idea! We want to encourage cleanups, but we >> don't want to encourage churn, and I think it's somewhat subjective >> where to draw that line. Having some more community awareness around >> that would be beneficial. I'm especially interested in how we balance >> between making incremental style improvements to the project and >> keeping our git blame logs useful. I'm seeing a lot more git blames >> that require several steps to get to an interesting commit because of >> the number of NFCs and reverts/recommits. Unfortunately, the tooling >> around viewing git blames of large files (like Clang tends to have) >> makes these sorts of commits surprisingly painful when you do have to >> dig to see where changes came from. (So I find myself having far less >> concern when TransGCAttrs.cpp (~350LoC) gets a cleanup like this >> compared to SemaExpr.cpp (~21kLoC), which suggests to me we should >> maybe strongly consider splitting more of these massive files up so >> that churn is less painful.) >> >> > & yeah, adding the `const` too if/when that's relevant would've >> > improved the quality/value/justification for a cleanup change like >> > this. >> >> It also would have matched our coding style. (We document it somewhat >> poorly as an example showing "observe" and "change", but reviewers who >> call for cleanups with auto are pretty consistent about asking to make >> qualifiers explicit.) >> >> ~Aaron >> >> > >> > On Sun, Sep 4, 2022 at 5:58 AM Aaron Ballman via cfe-commits >> > wrote: >> > > >> > > FWIW, sweeping NFC changes like this cause a fair amount of code churn >> > > (which makes tools like git blame a bit harder to use) for a >> > > relatively
Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
On Thu, Sep 8, 2022 at 12:37 PM David Blaikie wrote: > > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes > across the LLVM project for a while now, most, at least I think, are > quite welcome (things like switching to range-based-for, std > algorithms over llvm ones, llvm algorithms over manually written > loops, etc). But yeah, there's some threshold below which the churn > might not be worth the benefit - especially if the change doesn't come > along with tooling to enforce the invariant is maintained in the > future (if it's easy to make mistakes like this one - we'll regress it > and need to do cleanup again in the future) Thanks for speaking up, because I also waffled a bit on whether I called this out or not. :-) > Also, for this particular one, I wonder if in some cases this sort of > automatic transformation isn't ideal - if something is a pointer, but > that's an implementation detail, rather than an intentional feature of > an API (eg: the pointer-ness might be hidden behind a typedef and used > as an opaque handle, without any dereferencing, etc) Agreed. > I think it'd be really good to have some discussion on discourse about > if/how some of these cleanups could be formed into a way to > enforce/encourage the invariant to be maintained going forward - > clang-tidy (assuming that's the basis for the tooling Kazu's using to > make these changes in the first place) integration into the LLVM build > in some way, etc. I think that's a good idea! We want to encourage cleanups, but we don't want to encourage churn, and I think it's somewhat subjective where to draw that line. Having some more community awareness around that would be beneficial. I'm especially interested in how we balance between making incremental style improvements to the project and keeping our git blame logs useful. I'm seeing a lot more git blames that require several steps to get to an interesting commit because of the number of NFCs and reverts/recommits. Unfortunately, the tooling around viewing git blames of large files (like Clang tends to have) makes these sorts of commits surprisingly painful when you do have to dig to see where changes came from. (So I find myself having far less concern when TransGCAttrs.cpp (~350LoC) gets a cleanup like this compared to SemaExpr.cpp (~21kLoC), which suggests to me we should maybe strongly consider splitting more of these massive files up so that churn is less painful.) > & yeah, adding the `const` too if/when that's relevant would've > improved the quality/value/justification for a cleanup change like > this. It also would have matched our coding style. (We document it somewhat poorly as an example showing "observe" and "change", but reviewers who call for cleanups with auto are pretty consistent about asking to make qualifiers explicit.) ~Aaron > > On Sun, Sep 4, 2022 at 5:58 AM Aaron Ballman via cfe-commits > wrote: > > > > FWIW, sweeping NFC changes like this cause a fair amount of code churn > > (which makes tools like git blame a bit harder to use) for a > > relatively small improvement to code readability, which is why our > > developer policy asks that you "Avoid committing formatting- or > > whitespace-only changes outside of code you plan to make subsequent > > changes to." In the future, I'd caution against doing such large-scale > > sweeping NFC changes outside of areas you're actively working on > > unless there's some wider discussion with the community first. That > > said, all of your changes are all improvements, so thank you for them! > > > > Some of the changes you made would have ideally made it more clear > > when the deduced type is a pointer to a const object instead of hiding > > the qualifier behind the deduction. I've pointed out a couple such > > places below, but don't feel obligated to go back and change anything > > unless you're going to be touching other code in the area. > > > > ~Aaron > > > > > > On Sun, Sep 4, 2022 at 2:27 AM Kazu Hirata via cfe-commits > > wrote: > > > > > > > > > Author: Kazu Hirata > > > Date: 2022-09-03T23:27:27-07:00 > > > New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05 > > > > > > URL: > > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05 > > > DIFF: > > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff > > > > > > LOG: [clang] Qualify auto in range-based for loops (NFC) > > > > > > Added: > > > > > > > > > Modified: > > > clang/lib/ARCMigrate/ObjCMT.cpp > > > clang/lib/ARCMigrate/TransGCAttrs.cpp > > > clang/lib/AST/ASTContext.cpp > > > clang/lib/AST/ASTImporter.cpp > > > clang/lib/AST/Decl.cpp > > > clang/lib/AST/DeclObjC.cpp > > > clang/lib/AST/ODRHash.cpp > > > clang/lib/AST/OpenMPClause.cpp > > > clang/lib/AST/StmtProfile.cpp > > > clang/lib/AST/Type.cpp > > > clang/lib/Analysis/CFG.cpp > > > clang/lib/Analysis/ThreadSafetyCommon.cpp > > >
Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes across the LLVM project for a while now, most, at least I think, are quite welcome (things like switching to range-based-for, std algorithms over llvm ones, llvm algorithms over manually written loops, etc). But yeah, there's some threshold below which the churn might not be worth the benefit - especially if the change doesn't come along with tooling to enforce the invariant is maintained in the future (if it's easy to make mistakes like this one - we'll regress it and need to do cleanup again in the future) Also, for this particular one, I wonder if in some cases this sort of automatic transformation isn't ideal - if something is a pointer, but that's an implementation detail, rather than an intentional feature of an API (eg: the pointer-ness might be hidden behind a typedef and used as an opaque handle, without any dereferencing, etc) I think it'd be really good to have some discussion on discourse about if/how some of these cleanups could be formed into a way to enforce/encourage the invariant to be maintained going forward - clang-tidy (assuming that's the basis for the tooling Kazu's using to make these changes in the first place) integration into the LLVM build in some way, etc. & yeah, adding the `const` too if/when that's relevant would've improved the quality/value/justification for a cleanup change like this. On Sun, Sep 4, 2022 at 5:58 AM Aaron Ballman via cfe-commits wrote: > > FWIW, sweeping NFC changes like this cause a fair amount of code churn > (which makes tools like git blame a bit harder to use) for a > relatively small improvement to code readability, which is why our > developer policy asks that you "Avoid committing formatting- or > whitespace-only changes outside of code you plan to make subsequent > changes to." In the future, I'd caution against doing such large-scale > sweeping NFC changes outside of areas you're actively working on > unless there's some wider discussion with the community first. That > said, all of your changes are all improvements, so thank you for them! > > Some of the changes you made would have ideally made it more clear > when the deduced type is a pointer to a const object instead of hiding > the qualifier behind the deduction. I've pointed out a couple such > places below, but don't feel obligated to go back and change anything > unless you're going to be touching other code in the area. > > ~Aaron > > > On Sun, Sep 4, 2022 at 2:27 AM Kazu Hirata via cfe-commits > wrote: > > > > > > Author: Kazu Hirata > > Date: 2022-09-03T23:27:27-07:00 > > New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05 > > > > URL: > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05 > > DIFF: > > https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff > > > > LOG: [clang] Qualify auto in range-based for loops (NFC) > > > > Added: > > > > > > Modified: > > clang/lib/ARCMigrate/ObjCMT.cpp > > clang/lib/ARCMigrate/TransGCAttrs.cpp > > clang/lib/AST/ASTContext.cpp > > clang/lib/AST/ASTImporter.cpp > > clang/lib/AST/Decl.cpp > > clang/lib/AST/DeclObjC.cpp > > clang/lib/AST/ODRHash.cpp > > clang/lib/AST/OpenMPClause.cpp > > clang/lib/AST/StmtProfile.cpp > > clang/lib/AST/Type.cpp > > clang/lib/Analysis/CFG.cpp > > clang/lib/Analysis/ThreadSafetyCommon.cpp > > clang/lib/CodeGen/CGBlocks.cpp > > clang/lib/CodeGen/CGCall.cpp > > clang/lib/CodeGen/CGClass.cpp > > clang/lib/CodeGen/CGDebugInfo.cpp > > clang/lib/CodeGen/CGDeclCXX.cpp > > clang/lib/CodeGen/CGExpr.cpp > > clang/lib/CodeGen/CGObjCGNU.cpp > > clang/lib/CodeGen/CGObjCMac.cpp > > clang/lib/CodeGen/CodeGenFunction.cpp > > clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp > > clang/lib/CodeGen/SwiftCallingConv.cpp > > clang/lib/Driver/Compilation.cpp > > clang/lib/Driver/Driver.cpp > > clang/lib/Driver/ToolChains/Clang.cpp > > clang/lib/Driver/ToolChains/CommonArgs.cpp > > clang/lib/Driver/ToolChains/Gnu.cpp > > clang/lib/Driver/ToolChains/HIPAMD.cpp > > clang/lib/Format/Format.cpp > > clang/lib/Frontend/FrontendActions.cpp > > clang/lib/Index/USRGeneration.cpp > > clang/lib/Sema/IdentifierResolver.cpp > > clang/lib/Sema/Sema.cpp > > clang/lib/Sema/SemaCodeComplete.cpp > > clang/lib/Sema/SemaDecl.cpp > > clang/lib/Sema/SemaDeclAttr.cpp > > clang/lib/Sema/SemaDeclCXX.cpp > > clang/lib/Sema/SemaDeclObjC.cpp > > clang/lib/Sema/SemaExpr.cpp > > clang/lib/Sema/SemaExprCXX.cpp > > clang/lib/Sema/SemaInit.cpp > > clang/lib/Sema/SemaLambda.cpp > > clang/lib/Sema/SemaLookup.cpp > > clang/lib/Sema/SemaObjCProperty.cpp > > clang/lib/Sema/SemaOpenMP.cpp > > clang/lib/Sema/SemaOverload.cpp > > clang/lib/Sema/SemaTemplateDeduction.cpp > > clang/lib/Sema/SemaTemplateInstantiateDecl.cpp > >
Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
FWIW, sweeping NFC changes like this cause a fair amount of code churn
(which makes tools like git blame a bit harder to use) for a
relatively small improvement to code readability, which is why our
developer policy asks that you "Avoid committing formatting- or
whitespace-only changes outside of code you plan to make subsequent
changes to." In the future, I'd caution against doing such large-scale
sweeping NFC changes outside of areas you're actively working on
unless there's some wider discussion with the community first. That
said, all of your changes are all improvements, so thank you for them!
Some of the changes you made would have ideally made it more clear
when the deduced type is a pointer to a const object instead of hiding
the qualifier behind the deduction. I've pointed out a couple such
places below, but don't feel obligated to go back and change anything
unless you're going to be touching other code in the area.
~Aaron
On Sun, Sep 4, 2022 at 2:27 AM Kazu Hirata via cfe-commits
wrote:
>
>
> Author: Kazu Hirata
> Date: 2022-09-03T23:27:27-07:00
> New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
>
> URL:
> https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
> DIFF:
> https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff
>
> LOG: [clang] Qualify auto in range-based for loops (NFC)
>
> Added:
>
>
> Modified:
> clang/lib/ARCMigrate/ObjCMT.cpp
> clang/lib/ARCMigrate/TransGCAttrs.cpp
> clang/lib/AST/ASTContext.cpp
> clang/lib/AST/ASTImporter.cpp
> clang/lib/AST/Decl.cpp
> clang/lib/AST/DeclObjC.cpp
> clang/lib/AST/ODRHash.cpp
> clang/lib/AST/OpenMPClause.cpp
> clang/lib/AST/StmtProfile.cpp
> clang/lib/AST/Type.cpp
> clang/lib/Analysis/CFG.cpp
> clang/lib/Analysis/ThreadSafetyCommon.cpp
> clang/lib/CodeGen/CGBlocks.cpp
> clang/lib/CodeGen/CGCall.cpp
> clang/lib/CodeGen/CGClass.cpp
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/lib/CodeGen/CGDeclCXX.cpp
> clang/lib/CodeGen/CGExpr.cpp
> clang/lib/CodeGen/CGObjCGNU.cpp
> clang/lib/CodeGen/CGObjCMac.cpp
> clang/lib/CodeGen/CodeGenFunction.cpp
> clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> clang/lib/CodeGen/SwiftCallingConv.cpp
> clang/lib/Driver/Compilation.cpp
> clang/lib/Driver/Driver.cpp
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/lib/Driver/ToolChains/CommonArgs.cpp
> clang/lib/Driver/ToolChains/Gnu.cpp
> clang/lib/Driver/ToolChains/HIPAMD.cpp
> clang/lib/Format/Format.cpp
> clang/lib/Frontend/FrontendActions.cpp
> clang/lib/Index/USRGeneration.cpp
> clang/lib/Sema/IdentifierResolver.cpp
> clang/lib/Sema/Sema.cpp
> clang/lib/Sema/SemaCodeComplete.cpp
> clang/lib/Sema/SemaDecl.cpp
> clang/lib/Sema/SemaDeclAttr.cpp
> clang/lib/Sema/SemaDeclCXX.cpp
> clang/lib/Sema/SemaDeclObjC.cpp
> clang/lib/Sema/SemaExpr.cpp
> clang/lib/Sema/SemaExprCXX.cpp
> clang/lib/Sema/SemaInit.cpp
> clang/lib/Sema/SemaLambda.cpp
> clang/lib/Sema/SemaLookup.cpp
> clang/lib/Sema/SemaObjCProperty.cpp
> clang/lib/Sema/SemaOpenMP.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/lib/Sema/SemaTemplateDeduction.cpp
> clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
> clang/lib/Serialization/ASTReader.cpp
> clang/lib/Serialization/ASTWriterDecl.cpp
> clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
> clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
> clang/lib/StaticAnalyzer/Core/CallEvent.cpp
> clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
> clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
> clang/lib/Tooling/Tooling.cpp
>
> Removed:
>
>
>
>
> diff --git a/clang/lib/ARCMigrate/ObjCMT.cpp
> b/clang/lib/ARCMigrate/ObjCMT.cpp
> index 26f751b77f86a..fe0ce4c5cdc3a 100644
> --- a/clang/lib/ARCMigrate/ObjCMT.cpp
> +++ b/clang/lib/ARCMigrate/ObjCMT.cpp
> @@ -792,7 +792,7 @@ static bool UseNSOptionsMacro(Preprocessor ,
> ASTContext ,
>bool PowerOfTwo = true;
>bool AllHexdecimalEnumerator = true;
>uint64_t MaxPowerOfTwoVal = 0;
> - for (auto Enumerator : EnumDcl->enumerators()) {
> + for (auto *Enumerator : EnumDcl->enumerators()) {
> const Expr *InitExpr = Enumerator->getInitExpr();
> if (!InitExpr) {
>PowerOfTwo = false;
>
> diff --git a/clang/lib/ARCMigrate/TransGCAttrs.cpp
> b/clang/lib/ARCMigrate/TransGCAttrs.cpp
> index 99a61e0842a76..b94aee2de573e 100644
> --- a/clang/lib/ARCMigrate/TransGCAttrs.cpp
> +++ b/clang/lib/ARCMigrate/TransGCAttrs.cpp
> @@ -158,7 +158,7 @@ class GCAttrsCollector : public
> RecursiveASTVisitor {
> if (!D)
>
[clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)
Author: Kazu Hirata
Date: 2022-09-03T23:27:27-07:00
New Revision: b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
URL:
https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05
DIFF:
https://github.com/llvm/llvm-project/commit/b7a7aeee90cffefd0f73b8d9f44ab4d1d5123d05.diff
LOG: [clang] Qualify auto in range-based for loops (NFC)
Added:
Modified:
clang/lib/ARCMigrate/ObjCMT.cpp
clang/lib/ARCMigrate/TransGCAttrs.cpp
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/Type.cpp
clang/lib/Analysis/CFG.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
clang/lib/CodeGen/SwiftCallingConv.cpp
clang/lib/Driver/Compilation.cpp
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/HIPAMD.cpp
clang/lib/Format/Format.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Index/USRGeneration.cpp
clang/lib/Sema/IdentifierResolver.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
clang/lib/Tooling/Tooling.cpp
Removed:
diff --git a/clang/lib/ARCMigrate/ObjCMT.cpp b/clang/lib/ARCMigrate/ObjCMT.cpp
index 26f751b77f86a..fe0ce4c5cdc3a 100644
--- a/clang/lib/ARCMigrate/ObjCMT.cpp
+++ b/clang/lib/ARCMigrate/ObjCMT.cpp
@@ -792,7 +792,7 @@ static bool UseNSOptionsMacro(Preprocessor , ASTContext
,
bool PowerOfTwo = true;
bool AllHexdecimalEnumerator = true;
uint64_t MaxPowerOfTwoVal = 0;
- for (auto Enumerator : EnumDcl->enumerators()) {
+ for (auto *Enumerator : EnumDcl->enumerators()) {
const Expr *InitExpr = Enumerator->getInitExpr();
if (!InitExpr) {
PowerOfTwo = false;
diff --git a/clang/lib/ARCMigrate/TransGCAttrs.cpp
b/clang/lib/ARCMigrate/TransGCAttrs.cpp
index 99a61e0842a76..b94aee2de573e 100644
--- a/clang/lib/ARCMigrate/TransGCAttrs.cpp
+++ b/clang/lib/ARCMigrate/TransGCAttrs.cpp
@@ -158,7 +158,7 @@ class GCAttrsCollector : public
RecursiveASTVisitor {
if (!D)
return false;
-for (auto I : D->redecls())
+for (auto *I : D->redecls())
if (!isInMainFile(I->getLocation()))
return false;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 52e7eeca665ab..20fcc8fea4b79 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7568,7 +7568,7 @@ std::string ASTContext::getObjCEncodingForBlock(const
BlockExpr *Expr) const {
// FIXME: There might(should) be a better way of doing this computation!
CharUnits PtrSize = getTypeSizeInChars(VoidPtrTy);
CharUnits ParmOffset = PtrSize;
- for (auto PI : Decl->parameters()) {
+ for (auto *PI : Decl->parameters()) {
QualType PType = PI->getType();
CharUnits sz = getObjCEncodingTypeSize(PType);
if (sz.isZero())
@@ -7583,7 +7583,7 @@ std::string ASTContext::getObjCEncodingForBlock(const
BlockExpr *Expr) const {
// Argument types.
ParmOffset = PtrSize;
- for (auto PVDecl : Decl->parameters()) {
+ for (auto *PVDecl : Decl->parameters()) {
QualType PType = PVDecl->getOriginalType();
if (const auto *AT =
dyn_cast(PType->getCanonicalTypeInternal())) {
@@ -7612,7 +7612,7 @@
