Ticket #266 (closed todo: fixed)

Opened 6 years ago

Last modified 6 years ago

Packfile API cleanup: rename Parrot_readbc to Parrot_pbc_read, _loadbc => _pbc_load

Reported by: rurban Owned by: jkeenan
Priority: minor Milestone:
Component: core Version:
Severity: low Keywords:
Cc: Language:
Patch status: applied Platform:

Description (last modified by rurban) (diff)

Attached is a patch which:

- renames Parrot_readbc to Parrot_pbc_read (as heard on IRC)
- renames Parrot_loadbc to Parrot_pbc_load
- adds a third debug argument to Parrot_pbc_read which is stored in

the returned pf->options member, to be able to react in parrot and in other pbc utils differently.
Used for debugging pbc transformers.

- macrofy TRACE_PACKFILE printing (debug levels 1, 2 and 3)

Rationale: Debugging the pbc reader requires rebuilding libparrot and pdump, but this affects parrot also. parrot should not debug-print pbc transformers while testing. 64-bit pbc unpacking has still broken alignments, 64bit BE and 12-byte floats are untested, so there's still a way to go with this debugging stuff.

Also included is the TT #258 TODO patch "pcb_disassemble options -[bho?], enable roundtrips". I can apply them seperately.

The patch needs a reviewer.

Attachments

tt266+258-todo-read_pbc.patch Download (42.2 KB) - added by rurban 6 years ago.
tt266+todo-read_pbc.patch Download (36.5 KB) - added by rurban 6 years ago.

Change History

  Changed 6 years ago by rurban

  • owner set to rurban
  • status changed from new to assigned
  • description modified (diff)

  Changed 6 years ago by rurban

  • description modified (diff)
  • summary changed from Packfile cleanup: rename Parrot_readbc to Parrot_read_pbc, _loadbc => _load_pbc to Packfile API cleanup: rename Parrot_readbc to Parrot_read_pbc, _loadbc => _load_pbc

  Changed 6 years ago by rurban

  • description modified (diff)

  Changed 6 years ago by particle

close, but the naming convention is to put the subsystem first, so the functions should be named like Parrot_pbc_{read,load} instead.

~jerry

  Changed 6 years ago by rurban

  • description modified (diff)

Changed 6 years ago by rurban

  Changed 6 years ago by rurban

  • summary changed from Packfile API cleanup: rename Parrot_readbc to Parrot_read_pbc, _loadbc => _load_pbc to Packfile API cleanup: rename Parrot_readbc to Parrot_pbc_read, _loadbc => _pbc_load

Changed 6 years ago by rurban

  Changed 6 years ago by rurban

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

Tested on irc by Infinoid.

Added proper deprecation.

Applied with r36680.

  Changed 6 years ago by jkeenan

  • status changed from closed to reopened
  • resolution fixed deleted

follow-up: ↓ 11   Changed 6 years ago by jkeenan

  • status changed from reopened to new
  • priority changed from normal to minor
  • severity changed from medium to low
  • owner changed from rurban to jkeenan

I was looking at DEPRECATED.pod tonight and noticed that it still contains this notice:

       Parrot_readbc, Parrot_loadbc. [eligible in 1.1]
           They have been renamed to "Parrot_pbc_read", "Parrot_pbc_load".
           "Parrot_pbc_read" has one third int argument.

           <https://trac.parrot.org/parrot/ticket/266>

I grepped the repository and found that the only usages of Parrot_readbc and Parrot_loadbc were in this part of include/parrot/embed.h:

     87 /* Deprecated */
     88 
     89 /* with 0.9.1 TT #266 */
     90 #define Parrot_readbc(i, c)  Parrot_pbc_read((i), (c), 0)
     91 #define Parrot_loadbc(i, pf) Parrot_pbc_load((i), (pf))
     92 

... which was added by rurban as part of r36680.

Now, since we are past 1.1, I believe that we should be able to remove this code as well, correct? If so, I will do so and then remove the notice from DEPRECATED.pod.

Thank you very much.

kid51

  Changed 6 years ago by jkeenan

  • status changed from new to assigned

in reply to: ↑ 9   Changed 6 years ago by jkeenan

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

Replying to jkeenan:

Now, since we are past 1.1, I believe that we should be able to remove this code as well, correct? If so, I will do so and then remove the notice from DEPRECATED.pod.

On #parrot, tene++ confirmed my hunch. So I removed the remaining code in r39418.

Thank you very much.
kid51

  Changed 6 years ago by jkeenan

  • patch changed from new to applied
Note: See TracTickets for help on using tickets.