Bug 6403 – Windows patch: KJS_EXPORT for building DLLs

• Component: JavaScriptCore
• Status: RESOLVED INVALID

[Justin Haygood]

Changes from KJS to allow building a DLL that programs can link against.

Separated into 2 patches:

* One to add KJS_EXPORT
* Second to define KJS_EXPORT

[Justin Haygood]

Created attachment 5519 [details]
adds KJS_EXPORT to needed references

References located by attempting to link testkjs.exe to JavaScriptCore.dll and
adding KJS_EXPORT to things it says it needs to link to.

[Justin Haygood]

Created attachment 5520 [details]
Defines KJS_EXPORT 

I added this to fastmalloc.h because its really low level.

If there is a lower level header which is included by all headers, i can move
it there.

[Dave Hyatt]

Not sure, but maybe config.h would be more appropriate?

[Justin Haygood]

(In reply to comment #3)
> Not sure, but maybe config.h would be more appropriate?
> 

Is it included by all headers?

[Maks Orlovich]

Please consider marking the whole classes for export.  
http://www.cs.cornell.edu/~maksim/export.diff 
should be a good starting point 
 
 

[Justin Haygood]

Making this bug depend on Bug 6414 so that we can move KJS_EXPORT definition to
config.

Also, that diff is a good starting point.

Which classes should have the whole class exported?

JSObject & Interpreter are the biggest 2 that come to mind

[Justin Haygood]

Er, 6416

[Justin Haygood]

Created attachment 5726 [details]
KJS_EXPORT 

* Defines KJS_EXPORT in config.h (Requires JAVASCRIPTCORE_EXPORTS is defined by
the compiler if using Windows)
* If KJS_EXPORT isn't defined, it defines it to blank
* Exports the same classes KJS does
* Fixes a spelling error in one of the headers (error fixed in KJS's export
diff)
* Includes "nodes.h" in headers that require it (don't know why it didn't have
an error before now)
* On Windows only, makes sure the JSValue private declarations used only to
create a compiler error aren't there.. Exported classes require definitions.

[Darin Adler]

Comment on attachment 5726 [details]
KJS_EXPORT 

I don't think we want a FIXME in config.h for the possible future use of KJS_EXPORT for GCC visibility control; lets remove that for now.

The JSValue stuff should not be in an #if !WIN32 ifdef, because we do want those undefined copy constructor and assignment operators on Windows. Better would be to actually define them in value.cpp, but only when we are building a DLL on Windows. I don't think that WIN32 alone is good enough to indicate "building a DLL". For example, I know we might want to build a static library for Windows rather than a DLL, so we don't want ifdefs that assume "WIN32 == DLL".

Setting review- just because of the the value.h thing -- otherwise looks great, and about ready to land.

[Eric Seidel (no email)]

This bug hasn't been touched in nearly a year, and since then a working Windows port has been released.  I believe we can close this now.