[Webkit-unassigned] [Bug 164061] [GTK][WPE] Initial implementation of JavaScriptCore glib bindings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=164061

--- Comment #23 from Michael Catanzaro <mcatanzaro at igalia.com> ---
(In reply to Carlos Garcia Campos from comment #22)
> > > Source/JavaScriptCore/API/glib/JSCClass.h:22
> > > +#if !defined(__JSC_H_INSIDE__) && !defined(JSC_COMPILATION)
> > > +#error "Only <jsc/jsc.h> can be included directly."
> > > +#endif
> > 
> > I think we should move this underneath the include guards. It's a
> > microoptimization, but it annoys me. We can change the existing public APIs
> > later.
> 
> This is what we do everywhere else, I think. This won't be included twice in
> any case, since it will fail, no?

It's a very very small microoptimization: if the file starts with the include guards, GCC will avoid opening the file repeatedly. We should change our existing headers. Yes, there's no behavioral difference.

> > > Source/JavaScriptCore/API/glib/JSCContext.h:47
> > > +typedef void (* JSCExceptionHandler) (JSCContext   *context,
> > 
> > JSCExceptionHandlerCallback?
> 
> Isn't Handler and Callback kind of redundant?

Kind of. This one is iffy IMO.

> > > Source/JavaScriptCore/API/glib/JSCDefines.h:43
> > > +#ifdef G_OS_WIN32
> > > +#    if defined(BUILDING_JavaScriptCore) || defined(STATICALLY_LINKED_WITH_JavaScriptCore)
> > > +#        define JSC_API __declspec(dllexport)
> > > +#    else
> > > +#        define JSC_API __declspec(dllimport)
> > > +#    endif
> > > +#else
> > > +#    define JSC_API __attribute__((visibility("default")))
> > > +#endif
> > 
> > Seems like the G_OS_WIN32 condition creates the false hope that this code
> > might be expected to work on Windows.
> 
> I don't see why not. Doesn't jsc compile in windows?

I guess it could be useful there... OK.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180317/8fea309d/attachment.html>