You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by Nick Wellnhofer <we...@aevum.de> on 2015/04/20 22:21:22 UTC
[lucy-dev] Refactoring CFCSymbol
Lucifers,
I recently discovered an old branch on my MacBook that I rebased and put on
GitHub:
https://github.com/nwellnhof/lucy-clownfish/tree/cfc_symbol
In this branch, I began to refactor or even eliminate the CFCSymbol class. The
original motivation came from thinking about how to reimplement the Clownfish
compiler as a Clownfish project itself.
The CFCSymbol struct in master looks like this:
struct CFCSymbol {
CFCBase base;
struct CFCParcel *parcel;
char *exposure;
char *class_name;
char *class_nickname;
char *micro_sym;
char *short_sym;
char *full_sym;
};
Some observations:
1. `micro_sym` is kind of a misnomer these days. It should simply be `name`.
2. `short_sym` and `full_sym` are cached values. They can be computed using
`name` and the class the symbol lives in. If this class is always passed as an
additional argument to the functions returning these values, `parcel` and
`class_nickname` become unnecessary, too.
3. `class_name` is used by classes to store the class name. But if you think
about it, classes should store the class name in the `name` (formerly
`micro_sym`) slot. `class_name` is also used by methods to keep track of the
class where they were defined first. But this is only needed for methods.
So a symbol is essentially just a name and an exposure specifier. This makes
it questionable whether CFCSymbol is useful as a base class at all. I lean
towards eliminating CFCSymbol completely.
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 12/05/2015 04:55, Marvin Humphrey wrote:
> As can be expected when there's this much churn, something's gone a little
> screwy. :) Final methods now alias to imaginary implementing functions.
Thanks, I just fixed this:
https://git-wip-us.apache.org/repos/asf?p=lucy-clownfish.git;a=commitdiff;h=d6276a5ffc4ebd9f477df12686d06985171bcac0
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Sat, May 9, 2015 at 10:51 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> I merged the branch as-is. Let's make further modifications in a new commit.
Excellent, thanks for the high-volume of work!
As can be expected when there's this much churn, something's gone a little
screwy. :) Final methods now alias to imaginary implementing functions.
Say that To_Host has a concrete implementation in Obj, and that we declare
Class final.
Old and correct:
#define CFISH_Class_To_Host(self) \
CFISH_Obj_To_Host_IMP((cfish_Obj*)self)
New and problematic:
#define CFISH_Class_To_Host(self) \
CFISH_Class_To_Host_IMP((cfish_Obj*)self)
I was able to put in a stopgap by only allowing the final-method optimization
when the method is fresh.
But all this would be a lot easier if we eliminate inheritance and limit
ourselves to interface polymorphism. :)
Marvin Humphrey
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 09/05/2015 17:40, Marvin Humphrey wrote:
> If you agree with these modificationss, we can either make them now or in a
> new commit after you finish the big merge. I'm wary of cascading changes when
> rebasing large branches with lots of conflicts.
+1
I merged the branch as-is. Let's make further modifications in a new commit.
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Sat, May 9, 2015 at 5:58 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> In the commit I mentioned, I removed the `parcel` field from `CFCSymbol` and
> `S_lazy_init_sig` can't call `CFCMethod_get_parcel` anymore.
Ah, now I get it. :)
> We could pass the parcel directly but I opted to pass the current class
> which also knows about the parcel.
I think it's easier to understand if we pass in the parcel. Thinking that the
specific invoker class is somehow relevant threw me off.
Also, if we continue to cache, we should keep a reference to the parcel
(or capture its name), and check on subsequent calls that the cached version
is valid.
If you agree with these modificationss, we can either make them now or in a
new commit after you finish the big merge. I'm wary of cascading changes when
rebasing large branches with lots of conflicts.
Marvin Humphrey
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 08/05/2015 16:24, Marvin Humphrey wrote:
> We use the Clownfish parcel to determine how types should be namespaced.
> When the parcel is clownfish, String is spelled `String`. When the parcel is
> lucy, String is spelled `clownfish.String`.
That's why `S_lazy_init_sig` needs to know the parcel of a method. In the
commit I mentioned, I removed the `parcel` field from `CFCSymbol` and
`S_lazy_init_sig` can't call `CFCMethod_get_parcel` anymore. That's why I
added the `invoker` parameter, so `S_lazy_init_sig` can call
`CFCClass_get_parcel` on the invoker class instead.
> There are two things that are weird about the patch.
>
> 1. The sig is cached, but should arguably change based on the `invoker`
> because that affects namespacing of types.
> 2. However, we'll only need the sig once: when the method is first declared.
> And when that happens, the parcel of the CFCMethod will be exactly the
> same as the parcel of the CFCClass invoker.
>
> So, I would argue that `CFCGoMethod_get_sig` does not need to be passed an
> `invoker`. Instead, its documentation should be clarified:
>
> /** Retrieve the Go interface method signature. Types will be namespaced
> * to the parcel in which the method is declared.
> */
>
> Does that make sense?
Yes, but `CFCGoMethod_get_sig` needs to know the current parcel. We could pass
the parcel directly but I opted to pass the current class which also knows
about the parcel.
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Thu, May 7, 2015 at 12:24 PM, Nick Wellnhofer <we...@aevum.de> wrote:
> On 07/05/2015 17:42, Marvin Humphrey wrote:
>>
>> This very large branch does not merge cleanly and is going to get harder
>> to
>> merge over time. Should we concentrate on getting it done and hold off on
>> other merges until it is in?
>
>
> I rebased the branch and started to merge it:
>
> https://github.com/nwellnhof/lucy-clownfish/commits/cfc_symbol_v3
>
> When rebasing one of the changes, I had to adjust some Go-related code:
>
>
> https://github.com/nwellnhof/lucy-clownfish/commit/ad6e08984c09aeb9664f44170135ea4fb5d9734a#diff-44d73b464e29b0aa45fd6058111dc60b
>
> Does this look OK?
What `CFCGoMethod_get_sig` does is return a signature for a Go interface
appropriate for a method declared in a Clownfish class. For example, Lucy's
TermQuery#Get_Field method yields the following signature:
GetField() clownfish.String
We use those signatures to build up Go interfaces for each class. Here's
some output taken from when we build the clownfish runtime...
type Obj interface {
Clone() Obj
Destroy()
Equals(Obj) bool
CompareTo(Obj) int32
GetClass() Class
GetClassName() String
IsA(Class) bool
ToString() String
ToI64() int64
ToF64() float64
ToBool() bool
Mimic(Obj)
TOPTR() uintptr
}
... and also from lucy:
type Query interface {
clownfish.Obj
MakeCompiler(Searcher, float32, bool) Compiler
SetBoost(float32)
GetBoost() float32
Serialize(OutStream)
Deserialize(InStream) Query
Dump() clownfish.Obj
Load(clownfish.Obj) clownfish.Obj
}
type TermQuery interface {
Query
GetField() clownfish.String
GetTerm() clownfish.Obj
}
Three items of note:
* The invocant is not part of the signature.
* Namespacing of types is a concern: `String` vs. `clownfish.String`
* Each method only appears *once*
We use the Clownfish parcel to determine how types should be namespaced.
When the parcel is clownfish, String is spelled `String`. When the parcel is
lucy, String is spelled `clownfish.String`.
There are two things that are weird about the patch.
1. The sig is cached, but should arguably change based on the `invoker`
because that affects namespacing of types.
2. However, we'll only need the sig once: when the method is first declared.
And when that happens, the parcel of the CFCMethod will be exactly the
same as the parcel of the CFCClass invoker.
So, I would argue that `CFCGoMethod_get_sig` does not need to be passed an
`invoker`. Instead, its documentation should be clarified:
/** Retrieve the Go interface method signature. Types will be namespaced
* to the parcel in which the method is declared.
*/
Does that make sense?
Marvin Humphrey
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 07/05/2015 17:42, Marvin Humphrey wrote:
> This very large branch does not merge cleanly and is going to get harder to
> merge over time. Should we concentrate on getting it done and hold off on
> other merges until it is in?
I rebased the branch and started to merge it:
https://github.com/nwellnhof/lucy-clownfish/commits/cfc_symbol_v3
When rebasing one of the changes, I had to adjust some Go-related code:
https://github.com/nwellnhof/lucy-clownfish/commit/ad6e08984c09aeb9664f44170135ea4fb5d9734a#diff-44d73b464e29b0aa45fd6058111dc60b
Does this look OK?
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Fri, Apr 24, 2015 at 4:00 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> OK, then I'll polish the patch set a little and merge the branch. A good way
> to test changes to CFC is to diff the generated output for the Clownfish
> runtime and Lucy.
This very large branch does not merge cleanly and is going to get harder to
merge over time. Should we concentrate on getting it done and hold off on
other merges until it is in?
Marvin Humphrey
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 24/04/2015 06:47, Marvin Humphrey wrote:
> I would like it if CFC stuck more closely to classic compiler design
> paradigms, and that includes using symbol table scoping instead of jamming
> such functionality into various type innards. It would also include
> implementing semantic analysis as parse-tree walking, which we don't really do
> at all. But all of these wishes are implementation details.
Such features would certainly help to better validate the input and create
better error messages. Another thing I'd like to see is to attach input line
numbers to symbols.
> +1 to merge the whole branch. I skimmed each patch -- these are big diffs,
> but I trust your attention to detail and I like the ideas behind the changes.
> Thanks for the big contribution!
OK, then I'll polish the patch set a little and merge the branch. A good way
to test changes to CFC is to diff the generated output for the Clownfish
runtime and Lucy.
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nathan Kurz <na...@verse.com>.
On Thu, Apr 23, 2015 at 9:47 PM, Marvin Humphrey <ma...@rectangular.com> wrote:
> +1 to merge the whole branch. I skimmed each patch -- these are big diffs,
> but I trust your attention to detail and I like the ideas behind the changes.
> Thanks for the big contribution!
Speaking from the cheap gallery seats, I'm impressed too. I haven't
been paying full attention to the process, but all the specific design
decisions that I did notice seemed astute. Thanks for keeping this
moving along!
--nate
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Wed, Apr 22, 2015 at 8:23 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> Well, a compiler for the Clownfish package has to support almost all
> Clownfish features. I don't think it would be very small, but look very much
> like CFC in it's current form.
It would only need one back end, and it might be able to share the lexer and
parser. But you're right that it would still be substantial. Let's leave
this idea aside.
>> The rationale for CFCSymbol as a superclass was that classes, inert
>> variables, inert functions, and methods (or at least their implementing
>> functions) all live in one global namespace as unique symbols.
>
> Do they? The way I see it, only classes live in a global namespace, and
> variables and callables (functions and methods) live in a class's namespace.
OK, sure -- I can see things that way. It's more Clownfish-centric than
C-centric.
These days, I might argue that parcels live in the global namespace, classes
live inside parcels, and callables live inside classes.
> I don't think that symbol tables would buy us much except for detecting name
> clashes between variables and callables.
I would like it if CFC stuck more closely to classic compiler design
paradigms, and that includes using symbol table scoping instead of jamming
such functionality into various type innards. It would also include
implementing semantic analysis as parse-tree walking, which we don't really do
at all. But all of these wishes are implementation details.
> Anyway, there are some commits in my cfc_symbol branch that I definitely
> want to merge:
>
> - micro_sym/macro_sym cleanup
> - CFCCallable base class
> - CFCMethod_is_fresh helper
+1 to merge the whole branch. I skimmed each patch -- these are big diffs,
but I trust your attention to detail and I like the ideas behind the changes.
Thanks for the big contribution!
Marvin Humphrey
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Nick Wellnhofer <we...@aevum.de>.
On 22/04/2015 02:59, Marvin Humphrey wrote:
> On Mon, Apr 20, 2015 at 1:21 PM, Nick Wellnhofer <we...@aevum.de> wrote:
>> The original motivation came from thinking about how to reimplement the
>> Clownfish compiler as a Clownfish project itself.
>
> I was thinking a bit more about this recently. I wonder if it makes sense to
> have a small compiler implemented in C -- just enough to compile the
> `clownfish` package, and then use that to bootstrap a full compiler
> implemented as a Clownfish project.
Well, a compiler for the Clownfish package has to support almost all Clownfish
features. I don't think it would be very small, but look very much like CFC in
it's current form.
> The rationale for CFCSymbol as a superclass was that classes, inert variables,
> inert functions, and methods (or at least their implementing functions) all
> live in one global namespace as unique symbols.
Do they? The way I see it, only classes live in a global namespace, and
variables and callables (functions and methods) live in a class's namespace.
That's why I wouldn't make CFCClass inherit from CFCSymbol.
> One way that CFC could be improved is to use a symbol table to manage scope
> and detect collisions.
>
> http://www.tutorialspoint.com/compiler_design/compiler_design_symbol_table.htm
>
> Even though the Clownfish header language allows very little nesting, in
> retrospect it would be better to use a symbol table instead of scattering the
> symbol scoping logic over various registries and type internals.
I don't think that symbol tables would buy us much except for detecting name
clashes between variables and callables.
Anyway, there are some commits in my cfc_symbol branch that I definitely want
to merge:
- micro_sym/macro_sym cleanup
- CFCCallable base class
- CFCMethod_is_fresh helper
Nick
Re: [lucy-dev] Refactoring CFCSymbol
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Mon, Apr 20, 2015 at 1:21 PM, Nick Wellnhofer <we...@aevum.de> wrote:
> The original motivation came from thinking about how to reimplement the
> Clownfish compiler as a Clownfish project itself.
I was thinking a bit more about this recently. I wonder if it makes sense to
have a small compiler implemented in C -- just enough to compile the
`clownfish` package, and then use that to bootstrap a full compiler
implemented as a Clownfish project.
> So a symbol is essentially just a name and an exposure specifier. This makes
> it questionable whether CFCSymbol is useful as a base class at all. I lean
> towards eliminating CFCSymbol completely.
I think this is OK. It's always interesting to see how someone else views the
same problem space from a different angle.
The rationale for CFCSymbol as a superclass was that classes, inert variables,
inert functions, and methods (or at least their implementing functions) all
live in one global namespace as unique symbols. But it turns out that in the
CFC codebase, all those items are rarely accessed using the CFCSymbol API.
One way that CFC could be improved is to use a symbol table to manage scope
and detect collisions.
http://www.tutorialspoint.com/compiler_design/compiler_design_symbol_table.htm
Even though the Clownfish header language allows very little nesting, in
retrospect it would be better to use a symbol table instead of scattering the
symbol scoping logic over various registries and type internals.
The symbol table could be a hash table mapping `char*` to CFCBase. It's not
necessary for the values in the symbol table to implement the CFCSymbol API.
Marvin Humphrey