[TODO] Change name of 'Parrot_add_library_path' to 'Parrot_pf_add_library_path_from_cstring'

[allison]

The function 'Parrot_add_library_path' in src/library.c will be renamed to 'Parrot_lib_add_path_from_cstring'. A new function 'Parrot_lib_add_path' will be added that accepts a Parrot STRING.

The C string version will simply convert the C string to a Parrot STRING and call 'Parrot_lib_add_path'. (Currently, the first thing the C string version does is convert the C string to a Parrot STRING. It's a waste to have opcodes converting STRINGs to C strings, only to be immediately converted back to STRINGS.)

[coke]

Note: the function names in the subject and the body of this message differ; we should probably also consider 'Parrot_add_library_path_from_cstring' in this.

Passing back to allison for re-evaluation. We may need to open another ticket for other deprecation items.

[jkeenan]

There is an entry for this item in DEPRECATED.pod:

       Parrot_add_library_path [eligible in 1.1]
           Will be renamed to "Parrot_lib_add_path_from_cstring".

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

This suggests that we could work on this ticket right away. Am I correct in thinking that this will largely be a matter of changing all instances of Parrot_add_library_path to Parrot_lib_add_path_from_cstring -- much of which has already been done -- and then making all tests pass?

If so, then I will take the ticket and prepare a patch.

These are the files that would have to be touched:

$ fns . | xargs grep -l Parrot_add_library_path
./src/packfile.c
./src/library.c
./include/parrot/library.h
./compilers/imcc/main.c
./DEPRECATED.pod

Thank you very much.

kid51

[jkeenan]

Replying to jkeenan:

This suggests that we could work on this ticket right away. Am I correct in thinking that this will largely be a matter of changing all instances of Parrot_add_library_path to Parrot_lib_add_path_from_cstring -- much of which has already been done -- and then making all tests pass? If so, then I will take the ticket and prepare a patch.

I created the add_library_path_remove branch in SVN to work on this. I got as far as the attached patch, add_library_path.diff. At that point I began to get build warnings such as:

src/packfile.c:4776: warning: passing argument 2 of 'Parrot_add_library_path_from_cstring' from incompatible pointer type
src/packfile.c:4778: warning: passing argument 2 of 'Parrot_add_library_path_from_cstring' from incompatible pointer type

Subsequent discussion on IRC #parrot with Infinoid and NotFound suggested that more needed to be done for this TT than I originally understood:

09:21 NotFound "The C string version will simply convert the C string to a Parrot STRING and call 'Parrot_lib_add_path'."
09:21 NotFound So, the ticket ask for the two versions.
09:22 Infinoid Oh, I see.  So the callers of the old function (who are right to call the old function) should now call Parrot_lib_add_path(), instead
09:22 NotFound The naming used seem confusing, anyway.
09:22 kid51 So this is more than a simple "rip out this feature" deprecation.
09:22 Infinoid It's a "rename this function to that, and add this other function to make some of the callers cleaner"
09:23 NotFound Is renamimg a function and adding another.
09:23 Infinoid You've got the "this other function" part done

So, anyone who has a better understanding of these issues is welcome to take a crack at them in the add_library_path_remove branch.

Thank you very much.
kid51

[jkeenan]

Current state of work being done in the add_library_path_remove branch

[jkeenan]

Replying to jkeenan:

So, anyone who has a better understanding of these issues is welcome to take a crack at them in the add_library_path_remove branch.

Infinoid picked up the ball and the results were committed to trunk in r39431. No test failures due to these changes. I'll keep the ticket open through Tuesday for final comments.

Thank you very much.
kid51

[jkeenan]

There have been no complaints in the allocated time, so I'm closing this ticket.

Thank you very much.
kid51