llvm dev - May 2017 - When to use auto instead of iterator/const_iterator?

Hi All,

  While reading LLVM source code, sometimes I am wondering when should we
use auto instead of iterator/const_iterator.
I want to use the patch [1] I sent before as an example. Could someone give
me advice/guideline here? Also, I have another
question. Sometimes the for-loop uses const_iterator, say

  for (SUnit::const_succ_iterator I = SU->Succs.begin(), E
SU->Succs.end();

         I != E; ++I)

    if (I->isCtrl())

      NumberDeps++;

Can we rewrite above code as,

  for (auto &Succ : SU->Succs)

    if (Succ.isCtrl())

      NumberDeps++;
Or do we need to use `const auto &` instead, like this

  for (
​const ​
auto &Succ : SU->Succs)

    if (Succ.isCtrl())

      NumberDeps++;


Thanks.


[1] https://reviews.llvm.org/D32682

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Homepage: https://people.cs.nctu.edu.tw/~chenwj
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170502/89ac5d06/attachment.html>

As for:
- range based for: Use them wherever possible instead of the iterators. There
are some cases however where you cannot avoid the iterators (for example if you
want to call `iter = container.erase(iter)` in a loop)
- Avoid `auto` unless the type is obviously clear from the context:
> On May 2, 2017, at 6:36 AM, 陳韋任 via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi All,
> 
>   While reading LLVM source code, sometimes I am wondering when should we
use auto instead of iterator/const_iterator.
> I want to use the patch [1] I sent before as an example. Could someone give
me advice/guideline here? Also, I have another
> question. Sometimes the for-loop uses const_iterator, say
These are topics discussed in the developer policy:
http://llvm.org/docs/DeveloperPolicy.html
<http://llvm.org/docs/DeveloperPolicy.html>

While reading LLVM sourcecode keep in mind that sometimes the sourcecode is
older than some rules in the policy and might violate them. If you write new
code however it should adhere to the policy.
>   for (SUnit::const_succ_iterator I = SU->Succs.begin(), E =
SU->Succs.end();
> 
>          I != E; ++I)
> 
>     if (I->isCtrl())
> 
>       NumberDeps++;
> 
> 
This should work just as well with a range based for, so you should prefer that
over the iterator!
> Can we rewrite above code as,
> 
>   for (auto &Succ : SU->Succs)
> 
>     if (Succ.isCtrl())
> 
> 
>       NumberDeps++;
> 
> Or do we need to use `const auto &` instead, like this
>   for (
> 
> ​const ​auto &Succ : SU->Succs)
> 
>     if (Succ.isCtrl())
> 
> 
>       NumberDeps++; 
This should be written as:

for (const SDep &Succ : SU->Succs) {
  if (Succ.isCtrl())
    ++NUmberDeps;
}

And I think the LLVM community never agreed whether you need {} around the for
in a case like this. Some add the {} here, some people don't. Attempts to
standardize this as in https://reviews.llvm.org/D26943
<https://reviews.llvm.org/D26943> did not reach agreement.

- Matthias
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170502/90aff236/attachment.html>

On Tue, May 2, 2017 at 9:24 AM Matthias Braun via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
> This should be written as:
>
> for (const SDep &Succ : SU->Succs) {
>   if (Succ.isCtrl())
>     ++NUmberDeps;
> }
>
You may even wish to write this as:

NumberDeps = llvm::count_if(SU->Succs, [](const SDep &S) { return
S.isCtrl(); });
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170502/2e344b08/attachment.html>