incubator-lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marvin Humphrey <mar...@rectangular.com>
Subject Re: [lucy-dev] Rethinking header file installation
Date Wed, 14 Mar 2012 22:17:55 GMT
On Wed, Mar 14, 2012 at 1:02 PM, Nick Wellnhofer <wellnhofer@aevum.de> wrote:
> On 12/03/2012 00:41, Marvin Humphrey wrote:
>>
>> Personally, I believe that these are all positive developments for the
>> project, so +1 to install the .cfh files instead of the autogenerated .h
>> files.
>
>
> OK, so I would propose to install them in:
>
>    catdir ( $ARCH_DIR, 'Clownfish', '_cfh' );

I have a mild preference for naming that directory "_include" as opposed to
"_cfh" for a couple reasons:

  1. That dir might wind up containing things other than .cfh files, such as
     .h files that have been pound-included by .cfh files (in a literal C
     section).
  2. The string "cfh" is adequate as a file extension (which has to be short),
     but in my opinion "_include" is more self-documenting as a directory name.

> Then Clownfish has to be extended to read files from multiple source
> directories. I'd propose to add an array of include directories to the
> arguments of CFCHierarchy_init and use them to build the hierarchy.

+1 in principle.

I'd advocate for a slightly different implementation: strip the "source"
argument out of CFCHierarchy_new() and CFCHierarchy_init(), and add two new
functions:

    /** Add a directory containing Clownfish source files to compile.
     */
    CFCHierarchy_add_source_dir(CFCHierarchy *self, const char *dir);

    /** Add an include directory containing interface-defining "header" files.
     */
    CFCHierarchy_add_include_dir(CFCHierarchy *self, const char *dir);

If you want, feel free to add "include" and "source" labeled parameters to the
Perl binding for new(), since constructors with lots of labeled params are a
common Perl idiom:

    my $hierarchy = Clownfish::CFC::Model::Hierarchy->new(
        dest    => $output_dir,   # maybe rename to "output"?
        source  => \@source_dirs,
        include => \@include_dirs,
    );

Behind the scenes, though, have new() invoke add_source_dir() and
add_include_dir() before returning.

I have two reasons for making this suggestion:

  * C functions with numerous arguments are not self-documenting and are hard
    to use.
  * It's not possible to map a Perl array to "const char**" automatically.
    You typically have to allocate memory and build up the array manually in
    the binding, which is messy, annoying, error-prone, and leaks memory when
    an exception is thrown.

> Parts of the code in Lucy::Build can then be reused to build the extension.
> This code could be factored out into a module like Clowfish::Build that is
> installed together with Clownfish.

I think we probably want to stay out of the Clownfish:: first-level namespace
as much as possible, because that's where all our basic object types are going
to go.  I realize I'm being a bit hypocritical here because I dumped CFC into
Clownfish::, but I figured that there would never be a collision.

Would "Clownfish::CFC::Build" be appropriate?  (We're using that privately
right now, but no big deal to vacate it.)

If not, my preference would be to correct my own mistake by figuring out where
else to put CFC than to compound that mistake by polluting Clownfish::
further.

Marvin Humphrey

Mime
View raw message