This is interesting and can definitely enter 3.1. The only thing I would like to confirm are two.

1) If B is a subspace of A, having 'A B' as shared pools would actually behave the same as just 'B', right? In other words, A would be eliminated because of the topological sort.

2) Why do you remove pools that are superspaces of the class environment? What happens if you just search those twice? The answer might be related to item 1.

1) Yes, in your specific example; however, elimination is not done as pools are encountered, because we prefer left-to-right. For example, in the hierarchy:

X
 A
  B
 C

'B C' will sort to 'B A C X', whereas 'B C A' will sort to 'B C A X'.

2) It would be searching some pools too early. I wanted to allow importing of namespaces in other hierarchies, while not thereby forcing the early search of namespaces already imported by virtue of containing the importing class.

Arguments can be made to reduce or increase the elimination of shared pools. Both are discussed in the "Combination details" section, the specific option you mention being the subject of the second and third paragraphs.

thanks. i have now enough info to implement this in the C parser and make it the default. i don't how much of your code, especially #poolResolution, I will use, but the concepts will be there.

of course, feel free to beat me to it.

The attached patch makes TwistedPools the default pool search order outside the VM, and consequently changes your TwistedPools class to just use the default pool search order. All testcases still pass.

The code is heavily based on yours, with some refactoring because I wasn't afraid of touching base classes :-) and because the search order is implemented directly in Behavior (actually in Class).

I changed a couple of data structures. For the set of superspaces of this class and all the superclasses' environments, I used a Bag, which makes it easy to account for namespaces that are present multiple times. For the topological sort, I used two IdentitySets (grey/white, in three-color visit terminology) instead of a single dictionary.

I'll post the patch split in three to the ML too.

I added a fix for inherited class/shared pools and a test for it in c097acd. Also in c097acd is a new, failing test, fixed in b69098a, to eliminate only the direct superclass's namespaces from the namespace walk while searching pools. (See step #4 on PoolResolution).

> For the set of superspaces of this class and
> all the superclasses' environments

While it is convenient, it doesn't match the expectation of step #4, illustrated by the new test case, which is also why TwistedPools originally used superclass environment withAllSuperspaces asSet to make the sole set of namespace walk eliminations.

The unfortunate detail of this alternative, and thus a slight bug introduced in b69098a, is that it may answer some namespaces multiple times. OrderedSet did the right thing, so I could ignore the issue, but #allSharedPoolDictionariesDo: isn't necessarily building a set. Of course, the order is still right, so it doesn't affect a leftmost-first variable search at all.

thanks, I merged from you.

> The unfortunate detail of this alternative, and thus a slight bug
> introduced in b69098a, is that it may answer some namespaces
> multiple times.

Ah, later I realized that the Bag-based version in 274f63e too would duplicate some pools, in cases where a shared pool was used to force an early namespace import that would otherwise happen later anyway. So it looks like there are duplicates either way, unless anyone finds them bothersome.

ok, that's good. I could find in GCC a nice implementation of IdentitySet, and with that it was quite easy to implement TwistedPools in the VM. It is in my personal git repo.

Tests pass, but still, two more eyes can only do best. And there is also some pleaing for help:

/* Add POOLOOP and all of its superspaces to the list in the
   right order (Stephen, please help me... :-).  */

It's actually just a matter of copying from the right wiki page, if you don't have time I can do it.

Thanks again for noticing the need for this feature, for the precise description of the problem and (as usual) for the high quality of your code and your reviews.

I saw your merge; I'll wait a couple more days for a confirmation and then merge into master.

Yes, they were just a couple of bookkeeping things; I think it's ready for master.

committed then.