src/pmc/oplib.pmc, src/pmc/resizeablestringarray.pmc: Deprecate duplicated VTABLE functions in

[jkeenan]

In r48932, NotFound added a long overdue check for duplicated VTABLE functions -- a check which, when applied, turned up these two duplicates:

Index: /trunk/src/pmc/oplib.pmc
===================================================================
--- /trunk/src/pmc/oplib.pmc	(revision 48923)
+++ /trunk/src/pmc/oplib.pmc	(revision 48932)
@@ -117,8 +117,4 @@
     }
 
-    VTABLE INTVAL get_integer() {
-        return STATICSELF.elements();
-    }
-
     METHOD op_family(STRING *shortname)
     {
Index: /trunk/src/pmc/resizablestringarray.pmc
===================================================================
--- /trunk/src/pmc/resizablestringarray.pmc	(revision 46300)
+++ /trunk/src/pmc/resizablestringarray.pmc	(revision 48932)
@@ -303,23 +303,4 @@
 /*
 
-=item C<PMC *shift_pmc()>
-
-Removes and returns the first element in the array.
-
-=cut
-
-*/
-
-    VTABLE PMC *shift_pmc() {
-        STRING * const strval = SELF.shift_string();
-        PMC    * const value  = Parrot_pmc_new(INTERP, enum_class_String);
-
-        VTABLE_set_string_native(INTERP, value, strval);
-
-        return value;
-    }
-
-/*
-
 =item C<INTVAL shift_integer()>

However, pmichaud noted that Rakudo had inadvertently relied on the bug and that, though he could easily correct the problem in Rakudo, this should have been covered by a deprecation policy.

So I am reverting that commit, changing the die at line 74 of lib/Parrot/Pmc2c/PMC.pm to a warn, and posting in DEPRECATED.pod.

The next supported release is Oct 19 2010 and we can rip out the duplicated functions immediately thereafter.

[jkeenan]

Corrected 'Description'.

Reversion and change from die to warn performed in r48935.

[NotFound]

I can understand the reasons to want a deprecation for the error.

But in the name of sanity, what is the reason to keep the duplicated vtable functions in the PMC files? Just to hear me cry?

[jkeenan]

Replying to NotFound:

I can understand the reasons to want a deprecation for the error. But in the name of sanity, what is the reason to keep the duplicated vtable functions in the PMC files? Just to hear me cry?

The duplication of the vtable functions was erroneous, but, as discussed on #parrot 2010-09-11, it was also the thing that needed to be put through the deprecation cycle. I took on that responsibility; hence this ticket. When touched, the code will emit warnings. After Oct 19, I'll change the warn to a die, clear the item from DEPRECATION.pod and close the ticket.

Thank you very much.

kid51

[jonathan]

20:07 <@jnthn> As far as I understand, the detection is totally fine (in fact,
               good). However, pmichaud++ felt that dying rather than just
               warning required a deprecation cycle.
20:08 <@jnthn> (I helped pm to track down and remove the duplicated v-table
               method we had somewhere in Rakudo. It's good to detect them -
               our duplication was a straightforward mistake.)
20:09 <@jnthn> oh wth
20:09 <@jnthn> The ticket is a complete mis-understanding.
20:09 <@jnthn> The duplicates serve no useful purpose and should be tossed.
20:09 <@NotFound> Sigh... Again, the duplicated vtable functions don't get
                  compiled, they aren't put in the generated C file. Look at
http://tapir2.ro.vutbr.cz/cover/cover-results/48955/c_cover/src-pmc-oplib-pmc.html
20:09 <@jnthn> There's no way we could have depended on them.
20:10 <@jnthn> It was just a simple "die vs warn" issue.
20:10 <@jnthn> NotFound++ is right here.

So, we warn rather than die for now. But the dead code that was detected can stay removed - it was dead code. :-)

[plobsing]

Replying to jkeenan:

Replying to NotFound:

I can understand the reasons to want a deprecation for the error. But in the name of sanity, what is the reason to keep the duplicated vtable functions in the PMC files? Just to hear me cry?

The duplication of the vtable functions was erroneous, but, as discussed on #parrot 2010-09-11, it was also the thing that needed to be put through the deprecation cycle. I took on that responsibility; hence this ticket. When touched, the code will emit warnings. After Oct 19, I'll change the warn to a die, clear the item from DEPRECATION.pod and close the ticket.

The duplicate vtable functions were being ignored and didn't occur in the final PMC object code. It is therefore impossible that they were part of any user visible functionality. By definition, they do not fall under the deprecation policy.

What pmichaud argued falls under the deprecation policy is the behaviour of pmc2c, changes to which had caused rakudo build failures.

The vtable changes were unfortunately caught up in the revert due to being in the same commit. They should be reapplied ASAP because they add sanity at no cost.

Thank you very much. kid51

[pmichaud]

So, we warn rather than die for now. But the dead code that was detected can stay removed - it was dead code. :-)

That's all I was pointing out -- that if pmc2c dies when it encounters a duplicate vtable then it breaks language build systems (such as Rakudo's) that were inadvertently relying on the previous behavior. In no way was I claiming that the duplicate vtable entries themselves needed to be restored to the source, or that Rakudo or anything else could be relying on them.

[jkeenan]

Corrected reversion committed in r48985. Please review.

Thank you very much.

kid51

[NotFound]

Warnings are no longer shown. Deprecation notice looks fine for me.

[jkeenan]

Change 'warn' to 'die' and remove item from DEPRECATION.pod

[jkeenan]

Please see attachment tt1785_final.diff. Assuming I understand the issues remaining in this ticket, these changes are the last we need to perform to complete a scheduled deprecation. I will apply the patch in 1-2 days unless someone objects.

Thank you very much.

kid51

[NotFound]

Fine for me

[jkeenan]

Replying to jkeenan:

I will apply the patch in 1-2 days unless someone objects.

Applied in r49628. Closing ticket.