Re: [Opencxx-users] future directions

[Grzegorz J. <ja...@ac...>]

Hi,

Stefan Seefeld wrote:
> Grzegorz Jakacki wrote:
> 
>> Currently each C++ constructs is represented by a bunch of Ptree 
>> nodes, e.g.:
>>
>>     if (C) T else E
>>
>> is something like
>>
>>     PtreeIfStatement
>>      /          \
>>     C          NonLeaf
>>                 /    \
>>                T     NonLeaf
>>                       /  \
>>                      E    NULL
>>
>> [in fact it may be even more complicated]
>>
>> PtreeIfStatement is derived from NonLeaf; NonLeaf has obvious binary 
>> ctor. All derived classes generally also have binary ctors which just 
>> forward to NonLeaf's ctor.
>>
>> In particular currently there is no function which would build the 
>> above topology given C, T and E. Similarily, there is no function that 
>> would destroy it given a pointer to PtreeIfStatement.
>>
>> I was looking for a place to put this functionality in Node<>-less 
>> scenario. What I was trying to convey was that this function does not 
>> belong as a ctor in PtreeIfStatement, because:
>>
>> (1) PtreeIfStatement would need to have two ctors:
>>     * legacy ctor forwarding to NonLeaf's ctor
>>     * new ctor building auxiliary NonLeaf nodes
> 
> 
> indeed, but why is this a problem ? The idea would be to slowly phase out
> the legacy constructor (which is right now only used by the parser anyways,
> right ?)
> And I don't see a problem having two separate constructors, one operating
> on Ptree pointers, the other on the typed counterpart.

I am not sure if you get the exact picture. The "untyped" constructor 
does not construct the NonLeaf auxiliary nodes. It is just:


     PtreeIfStatement(Ptree* p, Ptree* q) : NonLeaf(p, q) {}

If we have another, "typed" constructor like this one:

     PtreeIfStatement(
         PtreeExprStatement* cond
       , PtreeExprStatement *th
       , PtreeExprStatement* el
     )
       : NonLeaf(cond, new NonLeaf(th, new NonLeaf(el, NULL)))
     {}

Then, as you noticed, there is a question of who owns auxiliary
NonLeaf nodes (and, consequently, if ~PtreeIfStatement should delete them).

> 
>> (2) PtreeIfStatement's dtor would need to know somehow
>>     which ctor was used; if the legacy one, then it should
>>     just destroy *this; if the new one, then it should
>>     also destroy auxiliary NonLeaf nodes. I don't want
>>     to go there.
> 
> 
> yeah, sounds messy. But why should the IfStatement care how
> it was constructed ?

Because "untyped" ctor does not create auxiliary NonLeaf nodes,
and "typed" does.

> Doesn't this reveal a more fundamental
> problem ? You seem to assume that the legacy constructor
> constructs an IfStatement that doesn't own its children, while
> the other does. 

The point is that legacy IfStatement constructor does not create
the representation of if-statement. In particular its arguments are
not "condition", "then-part" and "else-part". Representation of 
if-statement consists of three nodes (PtreeIfStatement node and
two "glue" NonLeaf nodes) and legacy PTreeIfStatement ctor constructs
only one of them.

> That sounds indeed like a bad idea. Either Ptrees
> own their children, or they don't. 

I agree. However I am not sure what you mean by "child". What is a 
"child" of PtreeIfStatement node? Do you consider "else-part" to be a 
"child" of PtreeIfStatement node?

It is also not clear to me if you would like to keep the existing 
representation of if-statement as three physical nodes or if you 
eventually would like to modify PtreeIfStatement so that it becomes

struct PtreeIfStatement
{
     ...
     PtreeExprStatement *cond_, *then_, *else_;
};

?

I proposed NodePtr<> iface to mask the fact that one logical AST node
(e.g. if-statement) is in fact represented by several physical objects.
The client of NodePtr<> would not be aware of this fact. NodePtr<> 
provides uniform interface for creating, destroying and manipulating 
such "logical objects".

> (Well, a third possibility
> would be to use ref counting, in which case nobody would own,
> but instead allocate a reference).
> 
>>> Is there some other instance
>>> beside the parser that will create ptree nodes ? 
>>
>>
>>
>> My primary motivation for developing OpenC++ are applications to 
>> refactorng, so the answer is YES, I would like to be able to 
>> manipulate AST and create new nodes.
> 
> 
> Sure. So everybody would use the new constructor, and as soon as the
> existing code moves to that, we could remove the legacy constructor.

Removing legacy ctors will be difficult. You once asked why and I was 
not clear about it, now I think I am. See Parser::rIfStatement(). It 
builds representation of if-statement and it takes 12 LOC and 8 function 
calls just to build the topology, excluding logic necessary to call 
parser for subexpressions and figure out if ELSE is present. Removing 
legacy ctor means replacing this 12 LOC and 8 calls with one call to a 
new ctor. Similar transformation would need to be done in roughly 100 
functions in parser. This seems like a lot of work and a lot of new bugs 
in code that is reasonably stable and works. Why do it?

> 
>>  > Will a Node<> ever
>>
>>> own the ptree it wraps ?
>>
>>
>>
>> Good question. No. At least I did not have it in mind.
> 
> 
> There are a couple of questions hiding here: Right now a Leaf node
> points to some memory associated with the input stream. That makes
> it easy to re-serialize the whole file just from the ptree. But
> what happens if you insert a new leaf ptree ? What memory would
> it refer to ? 
> Who would destroy it ? 

Now: it does not matter, it can refer to any c_string that is left 
allocated, because everything is GC-ed.

Future (pick one):

   * have special kinds of Leafs that store std::string internally
     and use this string for deserialization;
   * have "string manager" that owns inserted pieces
   * other?...

 > How's this solved right now ?

GC.

Best regards
Grzegorz

PS: I am applying your patch to rel_2_8. Thanks!

>>> Now if I get synopsis to process the boost.org code on all platforms, 
>>> I'm ready
>>> to ship :-)
>>
>> This is impressive. Boost is a heavy workout. Congratulations!!! I 
>> would like to put it up as a news on the website; will something like 
>> this do?:
>>
>>     Synopsis 0.7, source documentation tool based on
>>     OpenC++ library, now parses all Boost code!
> 
> 
> Sure, as soon as I'm done with the release :-)
> 
>> I branched rel_2_8, I will see how RH works, possibly apply patches in 
>> this branch and release, so HEAD is open for commits if you would like 
>> to commit your recent changes. Also if you want you can get yourself a 
>> sandbox branch.
> 
> 
> synopsis is my sandbox :-)
> 
> ---
> 
> Attached is a little patch that I just applied to opencxx trunk. With
> that opencxx passes all tests on fedora core 2 with gc disabled. I'd
> suggest to apply it to the 2.8 branch, too.
> 
> There are some other improvements I may backport from synopsis, but
> these are merely features, not bug fixes, so they are less critical.
> For example today I fixed synopsis to accept the ternary condition
> operator in a template parameter expression such as
> 
> foobar<is_const ? 1 : 2>();
> 
> More on that stuff later...
> 
> Regards,
>         Stefan
> 
> 
> ------------------------------------------------------------------------
> 
> Index: opencxx/parser/Lex.cc
> ===================================================================
> RCS file: /cvsroot/opencxx/opencxx/opencxx/parser/Lex.cc,v
> retrieving revision 1.1.2.1
> diff -u -r1.1.2.1 Lex.cc
> --- opencxx/parser/Lex.cc	27 May 2004 03:26:07 -0000	1.1.2.1
> +++ opencxx/parser/Lex.cc	8 Jul 2004 02:16:53 -0000
> @@ -807,6 +807,7 @@
>      { "__attribute__", token(ATTRIBUTE) },
>      { "__const",	token(CONST) },
>      { "__extension__", token(EXTENSION) },
> +    { "__inline", token(INLINE) },
>      { "__inline__", token(INLINE) },
>      { "__noreturn__", token(Ignore) }, 
>      { "__restrict", token(Ignore) },