Omit unnecessary clearing of cached base_scope values

3 files changed, 13 insertions, 2 deletions

diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index 7944294..7b465b4 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -1072,11 +1072,17 @@ namespace build2
       //    number of files (targets) and a large number of load phase
       //    switches (due to directory buildfiles).
       //
+      // Thinking some more on this, we shouldn't need to do this since such
+      // loads can (or at least should) only perform "island appends" see
+      // comment on context::phase for details.
+      //
+#if 0
       if (*r)
       {
         for (const unique_ptr<target>& t: ctx.targets)
           t->base_scope_.store (nullptr, memory_order_relaxed);
       }
+#endif
     }
 
     //text << this_thread::get_id () << " phase switch  "
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 5cc2115..9083556 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -248,7 +248,12 @@ namespace build2
     // "islands" are identified by the load_generation number (0 for the
     // initial/serial load). It is incremented in case of a phase switch and
     // can be stored in various "nodes" to verify modifications are only done
-    // "within the islands".
+    // "within the islands". Another example of invalidation would be
+    // insertion of a new scope "under" an existing target thus changing its
+    // scope hierarchy (and potentially even its base scope). This would be
+    // bad because we may have made decisions based on the original hierarchy,
+    // for example, we may have queried a variable which in the new hierarchy
+    // would "see" a new value from the newly inserted scope.
     //
     run_phase phase = run_phase::load;
     size_t load_generation = 0;
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 86f5e4b..9dc2be6 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -761,7 +761,7 @@ namespace build2
     // single invocation. How can we pick the scope that is "ours", for some
     // definition of "ours"?
     //
-    // The current think is that a project can be "associated" with other
+    // The current thinking is that a project can be "associated" with other
     // projects: its sub-projects and imported projects (it doesn't feel like
     // its super-projects should be in this set, but maybe). And "ours" could
     // mean belonging to one of the associated projects. This feels correct