Re: Refactoring progress report

From: David Abrahams (gclbb-jamboost_at_[hidden])
Date: 2003-05-10 10:03:49

Rene Rivera <grafik666_at_[hidden]> writes:

> [2003-05-09] Ali Azarbayejani wrote:
>
>>David and I have been discussing a number of issues relating to
>>refactoring the BBv2 codebase and we thought it would be a good time
>>to summarize some of them so you can see what we're planning and have
>>an opportunity for early feedback.
>
> Some brief feedback...

Much appreciated.

>>o Dynamic Type Checking
>
> [snip]
>
>> Another related problem is that arguments are generally named to
>> reflect the *type* of the argument, often resulting in confusion as
>> to the *role* of the argument.
>>
>> The proposal is to introduce dynamic type checking. David can give
>> more details, but the resulting syntax will allow the confusing
>>
>> rule split ( property-set )
>> rule as-path ( properties * : feature-space ? )
>>
>> to be written more explicitly as something like 
>>
>> rule <property-list> path-to-list ( <property-path> input-path )
>> rule <property-path> list-to-path ( <property-list> input-list :
>> feature-space ? )
>>
>> or
>>
>> rule [property-list] path-to-list ( [property-path] input-path )
>> rule [property-path] list-to-path ( [property-list] input-list :
>> feature-space ? )
>>
> [snip]
>> This type of readability problem is widespread throughout the
>> system. The new syntax is backward compatible (arguments and rules
>> do not require types) and the syntax can be introduced for better
>> readability before actually implementing the dynamic type checking.
>> The dynamic type checking would work by registering a type-checking
>> function for each type. This would be done automatically for
>> classes. The actual type checking could be turned off to avoid any
>> possible performance hit.
>
> That argument sounds very much like the in/out arguments thread ;-)
> I'm not sure going to this much trouble to solve the "bad argument
> names" is worth it. But then again I haven't written as much OO/Jam
> code as others.

This isn't really about bad argument names: it is basically designed
to solve the same kinds of problems solved by the addition of
argument lists. Too many rules:

a. don't make their requirements apparent to the reader
b. don't enforce their requirements

b) can be solved by explicit checking within the rule body, though,
like checking for argument arity in classic Jam, nobody ever did it
because it was too cumbersome. a) is a much harder problem, and
can't be addressed by using a different coding style.

> neutral on this one

Well, it's already done for arguments ;-)

Haven't got return type checking working yet, though. I want to
discuss the "class" business a little first.

>>o Explicit Importing
>>
>> Currently "import foo" in a module doesn't guarantee that the module
>> is dependent upon foo and the absence of "import foo" doesn't
>> guarantee that the module is NOT dependent on foo. The latter is
>> because "import foo" in another module makes foo rules available
>> globally.
>>
>> This presents a problem for determining which modules depend on
>> which other modules.
>>
>> The solution is to modify "modules.import" to make foo rules
>> available only to the module that is importing them.
>>
>> This would allow a better visual understanding each module through
>> the import statements; would allow automatic determination of module
>> layering/dependency relationships, leading possibly to automatic
>> documentation of layering and automatic maintenance or enforcement
>> of layering and dependencies; and would reduce sloppy and
>> inconsistent coding practices.
>
> +1 from me on this. In fact that's how I use the import in the code I write,
> seems only natural. Additionaly, to me it seems that a more restrictive
> modules.import is what I would expect. Instead of making the rules visible
> in the importing module, the rules should only be visible in the importing
> scope. For example there are some places where we import inside a rule. It
> would make more sense to limit the rules to that scope only.

I think it will help to create a more-disciplined codebase.

>>o Class Declaration
>>
>> Each class declaration requires a "rule" declaration followed by a
>> "class" declaration, making it difficult to see at a glance whether
>> a rule is a regular rule or a class definition. Short of a core jam
>> modification to allow a more obvious declaration, it may be possible
>> to reduce confusion by allowing the placing of the "class"
>> declaration just before the "rule" declaration. David plans to
>> explore this possibility. We're not sure there is an easy solution.
>
> Or just going more simply and commenting the rule as:
>
> # Class something-or-other....
> #...
> rule something-or-other...
>
> But since we are considering language changes... If you wan't a more
> immediate indication that it's a class why not use the dynamic type syntax
> as above:
>
> rule [class] something-or-other ( ...
>
> After all this is the construction rule, so in a sense it does return a
> class.

We can do that, but we'd really like to have a syntax which allows
you to declare the bases and everything all in one shot. Of course,
it is possible to enter [class] in the typecheck module with a null
check just to allow the above syntax to work in addition.

> If David does manage to make it possible to move the declaration before the
> rule, please warn me because the doc module depends on the current ordering.

Hmm, that's an interesting thought. Maybe it would be enough to have
a builtin rule which allows you to set a "rule definition hook". Then
an invocation of "class" would install a hook which would do the job
which "class" currently does as soon as the definition of the
following rule is processed.

class x : y z ; # installs rule definition hook
rule [class] x ( ... )
{
...
}

It's still quite redundant, but it does mean that the existing class
module works with very little modification. Other syntaxes I have
considered center around something like this:

module x : class y z
{
rule __init__ ( whatever ) { ... }
rule method1 ( whatever ) { ... }
}

Note the explicitly-declared __init__ function. It's not clear just
what it would take to make that work.

Another option:

module [ class x : y z ]
{
rule __init__ ( whatever ) { ... }
rule method1 ( whatever ) { ... }
}

This would use a similar technique to the first option, installing a
module-definition hook instead of rule-definition. It has some
advantages in that [ class ] can choose to define a qualified module
name based on the caller's module.

Hmm, a hybrid approach might be:

rule [ class x : y z ] ( init-params )
{
... init code ...
rule method1 ( whatever ) { ... }
}

This one could work in similar ways, installing a rule-definition
hook; Although the previous approach has lots of appeal, I think
this one might be the most expedient.

By the way, Rene, I'd have to hack the grammer a bit for that one;
I've been doing that a bit recently and I find the current build
scripts don't rebuild jamgram.y/jamgram.c on demand as I expected.
Also, even though I have the tools in my path on NT, I can't seem to
get yacc to run at all unless I'm running a Cygwin build.

> [snip]
>
>>o Option plug-ins
>>
>> Module "bootstrap.jam" is conceptually low-level, but depends upon
>> "doc.jam", which results in a great deal of unwanted dependency on
>> text-processing modules. This is a minor architectural problem.
>>
>> Actually, bootstrap.jam depends only on the parsing of command line
>> options representing help requests. One way to decouple bootstrap
>> from doc and provide a general mechanism for extending command line
>> options is to provide a plug-in architecture for command-line
>> options. The "help" module would use the option plug-in technique
>> to express that it wants to handle certain command line options and
>> then exit. Other modules might do the same, or handle command line
>> options and continue. Thus, doc is decoupled from bootstrap and we
>> have a new command-line-option extension feature.
>
> Ok at this point I'm getting confused about your use of "depends" (and it's
> variants). Yes bootstrap depends on doc, but not just to parse the command
> line. More precisely it depends on it to interpret the command line. I'm not
> sure how a plugin concept can help. If bootstrap "depends" on
> command-line-options and "depends" on doc, how is that different than the
> current situation?
>
> I'm afraid you'll have to be more descriptive about this one.

The idea is to do something like this:

bootstrap.jam scans the command-line, picking out any arguments that
look like

--([^-]*)(.*)
^^^^^ ^^
key: $1 $2

for each one that it finds, it looks for a module named options.$1 (I
am assuming a hierarchical module directory structure, but you could
do that today with a file called options.help.jam today, for example).

If it finds such a module, it is loaded. If the module contains a
rule named $2, it is invoked with the part of the command-line
starting with the argument following the option. Otherwise, if the
module contains a rule named __default__ (or something), it is invoked
with the part of the command-line starting with the option.

The ability to break dependency clashes is nice, but the big advantage
of a scheme like this is that it allows us to non-intrusively drop in
the ability to handle new options which affect startup behavior,
without having to explicitly add "import" calls for new option
processors.

>>o Layers (tools, build, core)
>
> [snip]
>
>> We propose eventual re-structuring of the BBv2 directory to have
>> three main subdirectories "tools", "build", and "core" containing
>> the functionality described above, after refactoring. This would
>> result in import statements like:
>>
>> import tools/gcc ;
>> import build/type ;
>> import core/os ;

I was actually in favor of

import tools.gcc ;
import build.type ;
import core.os ;

...but as they say in California, "Whateverrrr". Certainly the slash
syntax is easier, and it doesn't rule out '.' in module names.

>> We realize the proposed names result in source-tree paths like
>> "boost/tools/build/tools" and "boost/tools/build/build", which is
>> kind of ugly. Any suggestions?
>
> How does this interact with the BOOST_BUILD_PATH?

Simple: if you import 

tools/gcc

and BOOST_BUILD_PATH is

foo bar baz

SEARCH on the included file is:

foo/tools/gcc.jam bar/tools/gcc.jam baz/tools/gcc.jam

> Of source if we normalized the directories to the rest of Boost that
> would be:
>
> boost/tools/build/src/tools
> boost/tools/build/src/build
> boost/tools/build/src/core
>
> Doesn't seem all that ugly with the "src" divider in there :-\
>
> Of course along with that I would also normalize the "jam_src" directory
> right out of there into a:
>
> boost/tools/bjam/src
> ...etc...

So bjam isn't part of Boost.Build anymore? I can live with that.

>>o Refactoring feature.jam, property.jam
>
> [snip]
>
> Yea better names are good ;->
>
>>o Refactoring classes
>>
>> We propose that each class have its own file.
>>
>> This will have implications for module dependencies, because current
>> implementations are rather sloppy about what depends on what.
>> Classes depend on functions in the module they exist in and other
>> functions in the module depend on the class. These circular
>> dependencies are probably not necessary. Re-factoring will
>> highlight more clearly where circular dependencies exist. Trying to
>> eliminate circular dependencies will probably lead to a clearer
>> design and better concepts.
>
> I just don't see how this will be all that beneficial, but again I
> haven't looked at code as much as others. Just having good names for
> classes would go a long way. But the general idea of less classes
> per file seems like it would help in clarifying interfaces.

Yeah, I agree on all counts. I was a little surprised to read that I
was proposing a "one file per class rule" ;-), though I think it's
probably a very good idea in most cases.

I think Ali was probably confused about a different suggestion I made,
which was that it feels like there might be some advantage to having a
way of defining modules which act like classes in and of themselves.
But it was a vague suggestion.

>>o Generate Process
>
> [snip]

Workin' on it.

>>o Project-root/Jamfile
>>
>> The requirement of having a "project-root.jam" file in addition to a
>> "Jamfile" for every project means that each simple single-Jamfile
>> project requires an additional file.
>
> Not true. The only requirement is that there be at least "one"
> project-root.jam. For example boost should only have one project-root.jam...
> At the logical BOOST_ROOT/project-root.jam.

Yes, that's precisely what we're saying. Ali's usage model is not one
which needs or wants subprojects most of the time, and the arrangement
where each project can stand on its own is a very important model.

>> In fact, the Jamfile is the unnecessary one of the two, which leads
>> us to propose that the file performing the function of the current
>> "project-root.jam" be called "Jamfile", allowing simple projects to
>> exist with a single Jamfile. 

Actually, I'm surprised again. I talked about this idea and one other
with Volodya, and agreed that the other idea was the one to go with
[see below **]. I talked this over with Ali, but maybe our wires got
crossed.

>> The current function of "Jamfile" is really as a sub-Jamfile and
>> should be called something else, but we're not sure quite how to
>> rename.
>>
>> project-root.jam -> Jamfile, Jamroot ??
>> Jamfile -> SubJamfile ??
>
> No. There is no requirement that there be a Jamfile in the directory where
> project-root.jam is located. If you don't have buildable targets at your
> project-root than you should not have a Jamfile there.

That's not the point. The point is that I should be able to make a
standalone project with a single build description file.

> And now that I look at the "Jamfile.v2" in the boost_root, it should be
> renamed to project-root.jam. Of course we can only do this when BBv2 is
> fully adopted, as it clashes with the current project-root.jam. Or perhaps
> they can be merged with the use of a version check?

** The "right" approach, IMO, is to allow project-root.jam to fulfill
the functions of a top-level Jamfile if no such Jamfile exists. I'm
not sure how much infrastructure is required to implement that; maybe
it already works (though I doubt it).

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com