Code structure bikeshed: Where should spec reference implementations live?

[Jakob Kummerow]

As we have discussed at various occasions recently, we generally want to move in the direction of having C++ implementations of spec-defined behavior. That raises the question of where this code should live.

As an example of the kind of code we're talking about, consider https://codereview.chromium.org/1368753003/diff/1/src/runtime/runtime-object.cc (don't panic, runtime-object.cc is not the intended final place for this code to live -- the very purpose of this thread is to figure out a better place.), but there are also other, existing examples (like various ToXXX conversion functions, a bunch of things spread across runtime-*.cc, the JS implementations littered with runtime calls that we want to replace, ...).

Options I can think of:

(1) Put everything into objects.cc. 

+ Makes a lot of sense for things like DefineOwnProperty_Array, which could just be a static function JSArray::DefineOwnProperty.

+ Is an easy approach in the sense of being consistent with existing code structure (is that a good thing?)

− It's not clear how this approach maps to non-HeapObjects like the new class PropertyDescriptor

− I like having some distinction between high-level spec-defined operations like "DefineOwnProperty" and low-level V8 implementation details like MigrateToMap -- installing both on the same class JSObject feels like a recipe for confusion.

− objects.h/.cc are too big as it is, IMHO (of course this point is moot if/when we split it up)

(2) Put everything in runtime-*.cc

+ Works, and there's plenty of precedent.

− AFAIK we have pretty wide consensus that that's not what we want.

− A concrete technical drawback is a lack of callability from other places.

(3) Create a new directory, put everything there.

+ All reference implementations would be in one place

+ Can use individual files for further grouping if desired. Is that desired? What file structure would be good?

+ Personally I think we need more separation of things anyway, this is a step in that direction

• next question: how to call that directory? src/spec/? src/es6/? /src/blue/? (blue sheds are nice)

− For some things it might be unclear where to put them; our "abstractions" are (necessarily?) leaky

− New thing to get used to; inconsistency while it's a work in progress

(4) Organize by spec chapter, e.g. put OrdinaryDefineOwnProperty into src/es2015/ch9/9.1.cc or somesuch

+ If applied consistently, makes it easy to find things that are already implemented, which avoids duplication

− the resulting grouping may or may not make sense (it's up to the spec)

− ugly

Personally I'm leaning towards some variant of (3), but I'm open to being convinced otherwise. (1) sounds like a temporary solution to me; why not go for a longer-term plan right away?

Thoughts? Other ideas? Indifference?

[Jochen Eisinger]

I think (3) would be nice. There's also a bunch of code in api.cc and execution.cc that could be moved there.

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Yang Guo]

another vote for (3).

[Andreas Rossberg]

Some quick thoughts.

Option (4) is a non-starter. Section numbers are not stable across spec releases.

Option (3) doesn't sound bad, although it's not quite clear what the criteria for putting something into objects vs runtime vs the new dir would be. If we introduced a third category, not only stuff from runtime but also significant parts of the logic in objects.cc should naturally move there. Sounds like a lot of work with unclear benefit.

Option (1) seems like the most adequate for now.

IMHO, thinking about a new directory structure is putting the cart before the horse at this point. It only is interesting as part of a broader strategy for splitting up objects.h/cc. I don't think we currently have any plausible plan for that.

/Andreas

[Benedikt Meurer]

AFAIK everyone is doing (1) currently, and so at least for now, which is kinda what we agreed on some time ago. I don't see why we have to change that now. I think (3) is essentially (2) with a different directory/base filename, and many people thought that (2) was bad, which AFAIR is why we are at (1) currently. I'm fine with moving objects.cc/.h somewhere else and/or split them up, but I think we should delay that for now. This will just be distracting for the work that has to happen in that area IMHO.

[Benedikt Meurer]

One important bit tho: The (static) methods should be somewhere close the classes they relate to, otherwise we just go back to runtime-*.cc.

[Jochen Eisinger]

dumping everything in objects.cc? really?

[Daniel Ehrenberg]

On Fri, Sep 25, 2015 at 2:58 AM, Jakob Kummerow <jkum...@chromium.org> wrote:
> As we have discussed at various occasions recently, we generally want to
> move in the direction of having C++ implementations of spec-defined
> behavior.

Was that the conclusion of the discussion? My understanding was that
*some* things would be in C++ and we would try to minimize the bad
parts of the previous approach, especially where it jumps back and
forth, but that some things still make sense in JavaScript. But
correct me if I'm wrong. I'm asking because I do work on JS natives
and I don't want to write code in the wrong form such that it has to
be rewritten some time soon.

(This doesn't affect the location discussion. I don't have a strong
opinion, except it'll be great if we can move to smaller files somehow
or other.)

[Jakob Kummerow]

To attempt a compromise:

How about installing the functions as static methods on JSReceiver, and putting the implementations into src/objects/jsreceiver.cc?

That would avoid dumping more stuff into objects.cc, and I think there is general agreement that objects.h/cc should be split into somewhat smaller chunks, so why not use this opportunity to start doing that. I wouldn't move existing code for now, just put the new code in the new file.