gobo-eiffel / gobo

The Gobo Eiffel Project provides the Eiffel community with free and portable Eiffel tools and libraries.
https://sourceforge.net/projects/gobo-eiffel/
Other
59 stars 24 forks source link

Old code does not compile anymore #22

Closed manuseiffel closed 8 years ago

manuseiffel commented 8 years ago

We used to have a lot of code using internal cursor to navigate XM_NODEs. With the latest version of gobo it does not work as those were removed. Is this expected?

By the way, on github we cannot see the history of files after the move, we only see the commit pertaining the move.

ebezault commented 8 years ago

The main modifications have been made in July 2015 to make the library compilable in void-safe mode. I don't see any removal of internal cursor routines in class XM_NODE. What are the routines which are missing in your compilation?

I'm not using Github to browse the history, but SourceTree. It has a check-box to follow file renaming.

manuseiffel commented 8 years ago

I'm facing issues with EiffelBuild. Some classes that do not compile anymore are: https://svn.eiffel.com/viewvc/eiffelstudio/trunk/Src/build/XML/gb_xml_utilities.e?annotate=92483 https://svn.eiffel.com/viewvc/eiffelstudio/trunk/Src/build/Constants/gb_constants_handler.e?annotate=92483

We use to iterate on XM_ELEMENT nodes and perform some updates/removals.

I do not use git so I'm stuck with the web interface.

manuseiffel commented 8 years ago

To be more precise, we are lacking the following features on XM_ELEMENT:

manuseiffel commented 8 years ago

I looked at the diffs between the version we used to have and the new one. XM_COMPOSITE used to inherit from XM_LINKED_LIST, not anymore. My guess is that all the missing features were coming from this class.

ebezault commented 8 years ago

I found the same thing when looking at the history. Now we don't inherit from XM_LINKED_LIST, we have an attribute 'children: XM_LINKED_LIST [XM_NODE]'. My guess is that the inheritance has been removed because there were some attribute initialization problems with the cursor inherited from XM_LINKED_LIST. I think I can add back the missing routines in XM_ELEMENT (some of them don't really make sense in XM_DOCUMENT). But first I will try to add the inheritance to XM_LINKED_LIST back in XM_COMPOSITE in order to see what was the problem when compiling in void-safe mode.

ebezault commented 8 years ago

I confirm that inheriting from XM_LINKED_LIST does not work in void-safe mode when compiling XM_DOCUMENT.make_with_root_named (where 'Current' is passed as argument both when creating the internal cursor within 'list_make' and when creating 'root_element').

manuseiffel commented 8 years ago

Is there a reason for not exporting children? I know this is not ideal but it would help migrate the code as most code can be updated to use cursors and the problematic ones would use children.

ebezault commented 8 years ago

It's because one should use 'force_last' (which calls 'before_addition'), not 'children.force_last' directly.

ebezault commented 8 years ago

I think I will add the inheritance back, and change 'root_element' in XM_DOCUMENT to be a function, which would use 'internal_root_element: detachable XM_ELEMENT' if not Void, and create a new one otherwise.

ebezault commented 8 years ago

Oh no, I will not do that. In XM_ELEMENT 'children' is a list of XM_ELEMENT_NODE and in XM_DOCUMENT it is a list of XM_DOCUMENT_NODE. I cannot do that with inheritance.

ebezault commented 8 years ago

My impression is that if you use the external cursor (using 'new_cursor'), then the only routines that you are missing are 'wipe_out' and 'delete'. Would that be OK if I add only these two in XM_COMPOSITE? Or is there some reason to imperatively use internal cursor?

manuseiffel commented 8 years ago

It was more for convenience. If you stop the iteration mid-way, do you still have to do something with the cursor?

ebezault commented 8 years ago

Yes, you have to call 'l_cursor.go_after' to make it available for the GC. Never mind, I'll add all the routines that you listed above.

manuseiffel commented 8 years ago

Great. Thanks.

I would rather user the external cursor in most cases so I will also start modifying the code to have less dependence on the internal cursors. By the way, do you have plan to inherit from ITERABLE to benefit from the new across loop?

ebezault commented 8 years ago

The Gobo containers already inherit from ITERABLE.

manuseiffel commented 8 years ago

I tried iterating first on XM_ELEMENT and it did not work, so I tried on {XM_ELEMENT}.new_cursor but it failed too. So the containers do inherit from ITERABLE, but not the cursors.

Ultimately, I was looking for ITERABLE on XM_COMPOSITE.

ebezault commented 8 years ago

I only did the DS classes. XM classes are not mine. I already spent too much time converting them to void-safe mode. As for cursors inheriting from ITERABLE, why not. But it's weird. It's not the cursor which is iterable, but the container it traverses. If cursors should be iterable as well, then why not make ITERATION_CURSOR inherit from ITERABLE? It would make things clearer for the library implementers.

ebezault commented 8 years ago

The missing routines have been added. The ITERABLE part will be implemented tomorrow.

ebezault commented 8 years ago

Coming back to ITERABLE cursors, what is the reason for making INDEXABLE_ITERATION_CURSOR iterable, but not TABLE_ITERATION_CURSOR nor REPEATABLE.

ebezault commented 8 years ago

The ITERABLE part is now committed.

manuseiffel commented 8 years ago

ITERATION_CURSOR used to inherit from ITERABLE but people working on verification work at ETH pushed for simplifying to the bare minimum. I would make it ITERABLE again but need to check if there are any trace of the rationale. Commit removing this support was https://svn.eiffel.com/viewvc/eiffelstudio?view=revision&revision=91929