lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] [PATCH] C++ m11n: range-based for loops


From: Vadim Zeitlin
Subject: Re: [lmi] [PATCH] C++ m11n: range-based for loops
Date: Fri, 13 Jan 2017 16:32:49 +0100

On Fri, 13 Jan 2017 13:53:45 +0000 Greg Chicares <address@hidden> wrote:

[... debugging discussion mostly snipped ...]
GC> Maybe I have gdb in this chroot:
GC>   $whence gdb
GC> No, I don't; and no 'i686-w64-mingw32-gdb' either.

 But why not? It's part of gdb-mingw-w64 package which can be easily
installed and I had no trouble using it in remote debugging mode.

[... skipping to successful conclusion ...]
GC> In this case, it's this:
GC> 
GC> 
https://github.com/vadz/lmi/commit/d1fd3ae2e222c6d7a9edd1c9124ef2935e2918a5.patch
GC>          // Calculate duration when the youngest life matures.
GC>          int MaxYr = 0;
GC> -        for(i = cell_values.begin(); i != cell_values.end(); ++i)
GC> +        for(auto& cell_value: cell_values)
GC>              {
GC> -            (*i)->InitializeLife(*run_basis);
GC> +            cell_value->InitializeLife(run_basis);
GC>              MaxYr = std::max(MaxYr, (*i)->GetLength());
GC>              }
GC> 
GC> where the (*i) on the last substantial line should have been changed.
GC> 
GC> I had quite a lot of trouble figuring that out because I was instead
GC> looking at:
GC> 
GC> 
https://github.com/vadz/lmi/commit/6afb131f2d5d16aadb3b00dfc852d0cfba6949b6.patch
GC>          for(auto& cell_value: cell_values)
GC>              {
GC>              cell_value->InitializeLife(run_basis);
GC> -            MaxYr = std::max(MaxYr, (*i)->GetLength());
GC> +            MaxYr = std::max(MaxYr, cell_value->GetLength());
GC>              }
GC> 
GC> where Vadim manually corrected the problem...and I was wondering how
GC> 'patch' could have failed to apply that, without giving any warning.
GC> 
GC> I had thought that the first patch, being mostly generated by clang-tidy,
GC> would be the safer of the two. I didn't realize that the second corrects
GC> the first (at least in this hunk) as well as introducing more changes.

 Sorry, I'm afraid I mangled my commits here :-( Rerunning clang-tidy on
just group_values.cpp now I see that it didn't touch this loop at all
(probably because the iterator variable scope is not limited to the loop)
and so the change above must have been done manually by me, yet somehow
half-committed as part of the automatic changes. Really sorry for wasting
your time, it would probably be better to apply both commits before testing
further as if I made such a mistake here, I could have done it elsewhere as
well.

VZ


reply via email to

[Prev in Thread] Current Thread [Next in Thread]