I see several aesthetic/ergonomic reasons to prefer using typedefs everywhere, inside and outside the engine:
- When we are not in namespace js or JS, I think everyone should be able to agree that JS::Handle<JS::Value> is definitely too verbose. (We have this problem in SM headers too.) - It is the general SM and, more broadly, C++ style to immediately typedef any template-id that has more than a few (or even one) use, even for short template-ids. Perhaps for this reason the < > look noisy to my eyes when reading code, like someone should have refactored it to use a typedef. - Longer typenames are more annoying to type. I know, we read 100x more than write, but we write *a lot* of HandleFoo and I don't see the argument that they are harder to read, as discussed below. - Longer typenames cause function declarations to wrap to multiple lines which look mildly sloppier (and we have tons of Handles in signatures). - Super-common names get shortened (take 'cx', 'rt' or, back in the day, 'fp') and Handles are super-common. I'd even be happy to see HandleObj/HandleStr, but I won't push my luck :) Since these are only aesthetics, a good meaty counter-argument would trump them, but I just don't see it: - Of course we'll continue to have the templates in the headers (we kinda have to), so the 1% generic programming Handle<T> use case will continue to work. - On the "Handle<JSObject*> is more explicit / easier to understand / one less indirection" argument, I think we need to consider the cases of programmer using handles: A. The programmer already understands SM handles: this argument doesn't apply. B. The programmer is new to GC and handles: finding the definition is the least hard thing (ctags or mxr will take you there immediately); it's not like JS::Handle is some simple template utility that can be easily understood from it's interface. Oh no no, it's all SFINAE and CRTP and template specialization and the only way you are going to know what to do with it is to read the big beefy comment at the top of that file and/or look at some other code using Handles/Rooteds. Any indirection introduced by the typedef is insignificant in the overall process of learning how to correctly use handles, and then, once you have, you're in case A. C. The programmer is familiar with a different GC handle syntax, say V8s: this is a bit unfortunate, but I expect this set of people is small (and I assume figuring it out will take all of 5 minutes). - The you-can't-forward-declare-typedefs problem. I think we can address this as Brendan pointed out by forward declaring Handle and putting the HandleX typedefs in jspubtd.h (perhaps with a comment above saying "See js/public/RootingAPI.h"). Lastly, I don't see a good reason to have SM-internal Handle style differ from the mozilla-wide style. I think two different styles would, if anything, hurt understandability for mozilla hackers venturing into SM code. Also, as bholley pointed out, the JS::Handle<JS::Value> situation is worse. Does anyone see any flagrant misunderstandings or omissions in the above reasoning? If not, it would, as Till originally pointed out, be really good to have a single unified style, and I'd like the typedefs to be that style (except in cases of metaprogrammatic use of Handle<T>). Cheers, Luke ----- Original Message ----- > On Thu, Jun 20, 2013 at 7:21 AM, Till Schneidereit > <[email protected]> wrote: > > > > We still don't have a clear line on how to name handles, both in- and > > outside of the engine. > > > > Options: > > 1. don't use any typedefs at all, so always use (js:: and JS::)Handle<Foo*> > > 2. change the template to the form Handle<Foo>, roughly matching what v8 > > does > > 3. use typedefs everywhere, so always use HandleFoo > > This thread died, unresolved. We basically need Luke, as module > owner, to make a decision. > > I can offer one new data point. I've been working on jsapi.h include > minimization (bug 908050). That's pushed me into the option 1 camp, > because having to deal with the typedefs complicates things -- i.e. > forward declarations for Handle<T> aren't enough; you have to include > js/RootingAPI.h everywhere as well. > > Nick > _______________________________________________ > dev-tech-js-engine-internals mailing list > [email protected] > https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals > _______________________________________________ dev-tech-js-engine-internals mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
