[clang] modules: Fix callback argument thinko

[urnathan]

Summary

VisbleModuleSet::setVisible takes a callback, to inform of modules
being made (transitively) visible. However, we were calling it as
'Vis(M)' from a recursive lambda, where 'M' is a capture of
setVisible's M, module parameter. Thus we can invoke the callback
multiple times, passing the same value to it each time.

Everywhere else in the lambda, we refer to V.M of the lambda's
Visiting parameter. We should be doing so for the callback. Thus
we'll pass the outermost module on the outermost recursive call, and
as we descend the imports, we'll pass each import to the callback.

AFAICT, there's no real use of the callback, which is probably why this has not been detected.

[iains]

LGTM (the CI test fail seems spurious/unrelated, and does not repeat for me on macOS)

I guess there are not (m)any uses of the callback as you say - or the uses have not thrown up a failed test case.

clang/lib/Basic/Module.cpp

std::function<void(Visiting)> VisitModule = [&](Visiting V) {
     // Nothing to do for a module that's already visible.
     unsigned ID = V.M->getVisibilityID();
     if (ImportLocs.size() <= ID)
       ImportLocs.resize(ID + 1);
     else if (ImportLocs[ID].isValid())
       return;
 
     ImportLocs[ID] = Loc;
-    Vis(M);
+    Vis(V.M);
 
     // Make any exported modules visible.
     SmallVector<Module *, 16> Exports;
     V.M->getExportedModules(Exports);
     for (Module *E : Exports) {
       // Don't import non-importable modules.
       if (!E->isUnimportable())
         VisitModule({E, &V});