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

[Greg Chicares]

On 2017-01-13 11:39, Greg Chicares wrote:
[...]
> By now I would probably have found the problem if I had followed
> my inclination to revert changes selectively, iteratively rebuilding
> and retesting. There are 49 changed files, but many of the
> corresponding object files aren't even linked into the binaries that
> fail, and others are less likely candidates because their unit tests
> pass, so this isn't necessarily too laborious. I'll send this email
> off now and then try that after the morning's dog routine.

Here's a simple, robust, effective technique that runs unattended and
works quickly enough.

First, disable wine's crash dialog, because until we close it manually
it stalls the loop we're going to write. See:
  https://wiki.winehq.org/FAQ#How_do_I_disable_the_GUI_crash_dialog.3F
I'm not inclined to install 'winetricks', so...

/opt/lmi/src/lmi[0]$cat >../wine_dlg_off.reg <<\EOF
heredoc> [HKEY_CURRENT_USER\Software\Wine\WineDbg]
heredoc> "ShowCrashDialog"=dword:00000000
heredoc> EOF
/opt/lmi/src/lmi[0]$cat >../wine_dlg_on.reg <<\EOF
[HKEY_CURRENT_USER\Software\Wine\WineDbg]
"ShowCrashDialog"=dword:00000001
EOF
/opt/lmi/src/lmi[0]$regedit ../wine_dlg_off.reg 

(I'll have to remember to turn that thing back on later.)

Now behold the beauty of organized brute force:

/opt/lmi/src/lmi[0]$for z in file0.cpp file1.cpp ; \
  do echo REVERT $z >>../log; git checkout -- $z; \
  make $coefficiency system_test testdecks=/opt/lmi/test/REDACTED.cns >>../log 
2>&1; \
done

[Backslashes inserted manually--hope I didn't mangle it.]

Replace 'file0.cpp file1.cpp' with any list of files you like;
they'll be reverted in the order given.

(Did I mention that this runs unattended?) Inspect '../log': go to
the bottom, which shows success if the offending changes have been
reverted; then search upwards for "REVERT" to find the problem.

In this case, it's this:

https://github.com/vadz/lmi/commit/d1fd3ae2e222c6d7a9edd1c9124ef2935e2918a5.patch
         // Calculate duration when the youngest life matures.
         int MaxYr = 0;
-        for(i = cell_values.begin(); i != cell_values.end(); ++i)
+        for(auto& cell_value: cell_values)
             {
-            (*i)->InitializeLife(*run_basis);
+            cell_value->InitializeLife(run_basis);
             MaxYr = std::max(MaxYr, (*i)->GetLength());
             }

where the (*i) on the last substantial line should have been changed.

I had quite a lot of trouble figuring that out because I was instead
looking at:

https://github.com/vadz/lmi/commit/6afb131f2d5d16aadb3b00dfc852d0cfba6949b6.patch
         for(auto& cell_value: cell_values)
             {
             cell_value->InitializeLife(run_basis);
-            MaxYr = std::max(MaxYr, (*i)->GetLength());
+            MaxYr = std::max(MaxYr, cell_value->GetLength());
             }

where Vadim manually corrected the problem...and I was wondering how
'patch' could have failed to apply that, without giving any warning.

I had thought that the first patch, being mostly generated by clang-tidy,
would be the safer of the two. I didn't realize that the second corrects
the first (at least in this hunk) as well as introducing more changes.

I even remembered to turn the wine crash dialog back on:
  /opt/lmi/src/lmi[0]$regedit ../wine_dlg_on.reg