Ticket #872 (closed deprecation: fixed)

Opened 5 years ago

Last modified 5 years ago

PASM1 compiler and PDB_compile function

Reported by: coke Owned by: whiteknight
Priority: minor Milestone:
Component: none Version: 1.4.0
Severity: low Keywords:
Cc: Language:
Patch status: Platform:

Description (last modified by coke) (diff)

.sub 'a' :main
  $P0 = compreg 'PASM1'
  $S0 = "say 'hi'\n"
  $P1 = $P0($S0)
  $P1()
.end

BT from gcc:

#0  0x00000000 in ?? ()
#1  0xb7c485b6 in Parrot_invokecc_p (cur_opcode=0x80f8260, interp=0x804f040)
    at src/ops/core.ops:504
#2  0xb7cfbcd0 in runops_slow_core (interp=0x804f040, pc=0x80f8260)
    at src/runcore/cores.c:462
#3  0xb7cfa8ce in runops_int (interp=0x804f040, offset=0)
    at src/runcore/main.c:987
#4  0xb7cd3155 in runops (interp=0x804f040, offs=0) at src/call/ops.c:119
#5  0xb7cd3564 in runops_args (interp=0x804f040, sub=0x80c34d8, obj=0x80b4450,
    meth_unused=0x0, sig=0xb7f1dcb3 "vP",
    ap=0xbfebd02c "À4\f\bxÐë¿\2009÷·\220ò\004\bÀ4\f\bØ4\f\b\030lô·xÐë¿¿.ï·@ð\004\b\001") at src/call/ops.c:269
#6  0xb7cd4416 in Parrot_runops_fromc_args (interp=0x804f040, sub=0x80c34d8,
    sig=0xb7f1dcb3 "vP") at src/call/ops.c:338
#7  0xb7cb1907 in Parrot_runcode (interp=0x804f040, argc=1, argv=0xbfebd1a8)
    at src/embed.c:1021
#8  0xb7ef2ebf in imcc_run_pbc (interp=0x804f040, obj_file=0, output_file=0x0,
    argc=1, argv=0xbfebd1a8) at compilers/imcc/main.c:801
#9  0xb7ef3abc in imcc_run (interp=0x804f040, sourcefile=0xbfebd41a "foo.pir",
    argc=1, argv=0xbfebd1a8) at compilers/imcc/main.c:1092
#10 0x08048978 in main (argc=1, argv=0xbfebd1a8) at src/main.c:60

Change History

  Changed 5 years ago by coke

A reasonable response would be to immediately deprecate this compiler. Near as I can tell, it's completely untested, and documented only in passing in a chapter of the draft book.

  Changed 5 years ago by coke

  • owner set to NotFound

NotFound suggested on IRC that we simply delete this compiler, as it is untested and broken. I agree.

If it /never/ worked, there's no point in keeping it; easiest way to check this (given lack of tests, see TT #672), is to verify that it also failed in Parrot 1.0.0; I think if we do that, then we can safely remove it now.

  Changed 5 years ago by NotFound

Tested with  https://svn.parrot.org/parrot/tags/RELEASE_1_0_0 and it segfaults the same way.

  Changed 5 years ago by NotFound

I suggest that together with the PASM1 pseudo compiler we also delete the function PDB_compiler declared in include/parrot/debugger.h whose only usages are to implement PASM1 and the debugger comand eval, which also doesn't work. And make that command just emit an "unimplemented" message.

  Changed 5 years ago by NotFound

  • priority changed from major to minor
  • severity changed from medium to low
  • summary changed from segfault with PASM1 compiler to Deprecate PASM1 compiler and PDB_compile function

In yesterday's #ps it has been decide to deprecate this functionality and throw exceptions in attempts of usage. Deprecation note added and code fixes done in r40408

follow-up: ↓ 7   Changed 5 years ago by coke

Removing the implementation of something entirely and throwing an exception when its used isn't deprecation.

We should either:

1) leave it alone until post 2.0 on the off chance that someone was using it in a way that worked for them (even though no one has been able to provide code that might demonstrate its usage, I'm willing to believe it exists.)

Or

2) delete it entirely, on the assumption that it never worked.

Or

3) emit a warning when it is used, but otherwise leave the implementation alone. This can be tied to parrot's warning feature, as deprecated opcodes do.

The current solution, removing the guts and throwing an exception, means that anyone who was potentially using it can't; This has effectively already removed the feature from parrot, and if we're going to do that, let's just do it.

If instead we're trying to warn people, #3 is preferred.

in reply to: ↑ 6   Changed 5 years ago by whiteknight

  • description modified (diff)

Replying to coke:

Removing the implementation of something entirely and throwing an exception when its used isn't deprecation. We should either: 1) leave it alone until post 2.0 on the off chance that someone was using it in a way that worked for them (even though no one has been able to provide code that might demonstrate its usage, I'm willing to believe it exists.) Or 2) delete it entirely, on the assumption that it never worked. Or 3) emit a warning when it is used, but otherwise leave the implementation alone. This can be tied to parrot's warning feature, as deprecated opcodes do. The current solution, removing the guts and throwing an exception, means that anyone who was potentially using it can't; This has effectively already removed the feature from parrot, and if we're going to do that, let's just do it. If instead we're trying to warn people, #3 is preferred.

  Changed 5 years ago by coke

  • owner changed from NotFound to coke
  • description modified (diff)

Moving whiteknight's comment into a comment and replacing the original ticket description.

+1 from me. What happened to this compiler, ripping out all it's functionally and replacing it with an exception, doesn't really satisfy the deprecation policy, but then again keeping in a feature that doesn't work for the sake of keeping it not working doesn't really do so either.

I don't think we've really given it enough thought, but "features" that didn't work at the time that the deprecation policy was adopted should really be grandfathered in as being "experimental" features. They didn't work, there wasn't an expectation that they would "continue" to work, etc. Better to just remove this and be done with it.

  Changed 5 years ago by whiteknight

  • status changed from new to closed
  • resolution set to fixed

removed in r44130.

  Changed 5 years ago by coke

  • status changed from closed to reopened
  • resolution fixed deleted

from DEPRECATED.pod:

=item PDB_compile and PDB_eval [eligible in 2.1]

The function PDB_compile doesn't work since some time, and his implementation
was a hack. His functionality may be reimplemented in another way. Until the
end of deprecation cycle it just throws an exception.

The PDB_eval function, that used in the debugger to support the eval
command, was using PDB_compile. His functionality must be reimplemented
in another way and with a name that follows current conventions, in the
meantime and until the end of the deprecation cycle it just shows a
diagnostic message.

These functions are still present in r44134 - reopening.

  Changed 5 years ago by coke

  • status changed from reopened to new
  • owner changed from coke to whiteknight
  • type changed from bug to deprecated
  • summary changed from Deprecate PASM1 compiler and PDB_compile function to PASM1 compiler and PDB_compile function

  Changed 5 years ago by whiteknight

  • status changed from new to closed
  • resolution set to fixed

The PASM1 compreg and the PDB_compile functions were deprecated and removed. I'm closing the ticket.

The PDB_eval function is *not* slated for removal and I have not removed it. The deprecation notice suggests that this function (which has been throwing an UNIMPLEMENTED exception) needs to eventually be implemented in a different way since PASM1 no longer exists.

The status of the PDB_eval function will be tracked in #1466

Note: See TracTickets for help on using tickets.