> You want to pick an
> example where you can without additional round-trips take the pattern
> displayed in that response and map it back onto your actual codebase. I
> hope — but am not confident! — that your new example clearly
> demonstrates the problem you're asking about, and that therefore my
> solution will help you.
 
This is not about solving a problem - my code already works. This mail list is to propose and discuss the additions to the C++ standard. I wrote a lot of code for ASTs, and variants generally seem a good fit for AST nodes. However, I noticed that the problem I described in the original post is rather common and very annoying - one tends to write a big chunk of the visitor, then at some point the node's alternative needs changing, so one has to go back and uglify the code using some workaround. It's so-o-o tempting to use a cast!

I think adding the variant_from_alternative function to the standard is the best solution. Of course, there are workarounds, and I already used them in my code. However, these workarounds result in less-than-elegant and sometimes fragile code (as illustrated in my example).


> To be clear: ASTNode is a value type, and so the only way to accumulate this
> vector of value-semantic subtrees is by copying subtrees out of the original
> tree, or by dismantling the original tree, is that right?

Yes, it's a value type with efficient move - essentially, one can move a tree by copying only its root - the children nodes are then stolen.


> At second glance, I assume that you're dismantling the given tree. This also
> means that you expect never to be in a situation where you have two
> interesting subexpressions S and T, and S is a subtree of T. For example, in (x
> + (y*z)) it is never the case that both (y*z) and z are interesting. Right?
 
Correct.


> Anyway, I'm still inclined to build up a vector of reference_wrappers (or
> pointers) to pieces of the given original tree, and then allow the caller to
> pilfer out of any-or-all of them after visitation is done. That just seems
> simpler to reason about, compared to dismantling the tree on the fly.

I'd argue it's not simpler:
  * you just add an extra post-processing loop, leaving the code otherwise very similar;
  * you create references to tree nodes (which are otherwise uniquely owned);
  * the references are used far away from the point of their creation;
  * as the object is being dismantled, you need to care about the validity of references - risking UB.
 
> Right. This breakage will happen regardless of whether you move(n) or
> move(v). So you've got a logic bug here; no amount of physical refactoring
> will solve the logic issue. You'll have to redo your algorithm so that
> semantically the algorithm no longer needs to refer to a node after it's been
> dismantled.

The point here is that it's not just any odd logic bug - it's a bug that can be attributed to breaking the unique ownership contract, and this defect is *inherent* in special_visit function when the variant is an rvalue. If you have a function
 void f(string&& s1, string&& s2)
it would be surprising if s1 and s2 refer to the same object (even if one of the references is changed to &). (The natural interpretation here is that, say, s1 uniquely owns the object, so you can steal things from it without anyone else (e.g. s2) noticing.) I'd say calling such code "fragile" would be an understatement.

Contrast this situation with variant_from_alternative: one has only one reference, so side-effects are unsurprising:

     void operator()(Variable&& v){
                useful.push_back(variant_from_alternative<ASTNode>(move(v)));
                // fiddle(context,v); // it's clear that v can no longer be used, as the above line moves it
     }

It seems using special_visit function when the variant is an rvalue is much more dangerous than variant_from_alternative, and also less elegant (due to an extra parameter).



> I continue to suggest using a vector of references/pointers to
> ASTNodes so that the original tree remains intact for the entire visitation
> process.

I must confess I resorted to something like this a few times in the past, but for the purposes of this discussion, it's a workaround on top of a workaround. This also focuses too much on the pecularities of the example rather than on the main issues.