[lucy-issues] [jira] [Commented] (LUCY-233) Implement Clownfish Method class

["Marvin Humphrey (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCY-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13266838#comment-13266838 ] 

Marvin Humphrey commented on LUCY-233:
--------------------------------------

Looks good, +1 to commit!

Comments:

 * The METHOD_PTR patch looks fine.
 * I chuckled at the "Bootstrap VTables" hacks necessary when the the parcel
   prefix is "lucy_" (which will eventually change to "cfish_") so that
   VTable_init() could be generalized out of VTable_bootstrap().
 * Nice adding comments in the generated code.  That makes both the transpiler
   output and the sprintf format string easier to grok.
 * FWIW, the variable "x" which is "reserved for future use" in VTable can go
   away.  It's an artifact of when OFFSET vars had fixed values.
 * I think it makes sense for Method objects to become immutable, like
   VTables (modulo the mutable host object bug for VTables).  That would mean
   overriding Inc_RefCount() and Dec_RefCount() and leaking them
   intentionally.
 * It's a long-term concern that VTables now contain more elements that aren't
   immutable, e.g. VArrays.  Our goals for thread support are minimal -- we
   only support strongly divorced concurrency -- but if VTables and their
   components are not immutable, we'll have thread safety issues.  Perhaps we
   need classes like "FinalString", "FinalHash", etc. to support situations
   like this.
 * I notice that the Method takes ownership of the CharBuf "name" in
   Method_init() in the third patch.  What we do elsewhere is pass in a "const
   CharBuf*" and CB_Clone() it, so that the caller doesn't have to worry about
   whether they modify it.  The same thing applies to VTable_init() in patch
   4.
 * CB_newf(name) might be better written as CB_newf("%s", name), since the
   first arg is a format string.

                
> Implement Clownfish Method class
> --------------------------------
>
>                 Key: LUCY-233
>                 URL: https://issues.apache.org/jira/browse/LUCY-233
>             Project: Lucy
>          Issue Type: Task
>          Components: Clownfish
>            Reporter: Nick Wellnhofer
>         Attachments: 0001-Rename-METHOD-macro-to-METHOD_PTR.patch, 0002-Rework-VTable-bootstrapping.patch, 0003-Initial-implementation-of-Method-class.patch, 0004-Rework-VTable-method-initialization.patch
>
>
> As discussed on lucy-dev, it would be nice to have a Clownfish class for methods. The VTables would then contain a VArray of Methods which can be used to setup host language overrides and later to lookup offsets dynamically.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira