Move convertDictionary<>() template specializations to the header in order to: - Allow them to be used from custom bindings code. - Prepare for moving dictionaries to their own IDL files (Bug 162912).
Created attachment 290643 [details] Patch
Created attachment 290645 [details] Patch
Do the implementations really have to go in the header? Can't we just forward declare what we want to call?
(In reply to comment #3) > Do the implementations really have to go in the header? Can't we just > forward declare what we want to call? Your comment only applies to jsStringWithCache() right? All other functions are templated so I believe they will need to be defined in the header if we want them to be callable from other compilation units. I do think we can move the definition of jsStringWithCache() to the cpp though and only keep its declaration in the header.
(In reply to comment #4) > (In reply to comment #3) > > Do the implementations really have to go in the header? Can't we just > > forward declare what we want to call? > > Your comment only applies to jsStringWithCache() right? All other functions > are templated so I believe they will need to be defined in the header if we > want them to be callable from other compilation units. > > I do think we can move the definition of jsStringWithCache() to the cpp > though and only keep its declaration in the header. Although I don't know about the performance impact of doing so given that the function was likely being inlined.
Created attachment 290665 [details] Patch
I have moved jsStringWithCache() implementation back to the cpp. I haven't tried instantiating the template functions in the header and moving the template specializations to the cpp yet.
(In reply to comment #7) > I haven't > tried instantiating the template functions in the header and moving the > template specializations to the cpp yet. The thing is that I worry it will lead to linking errors when actually calling the specialization from another compilation unit (which we do not do yet so I won't notice) or about getting weird compilation trouble on Windows. When I have tried tricks like this in the past, it usually hasn't gone well :/ Do you mind if we try doing so in a follow-up, once we actually start sharing dictionaries (i.e. after Bug 161932 is fixed)?
(In reply to comment #8) > (In reply to comment #7) > > I haven't > > tried instantiating the template functions in the header and moving the > > template specializations to the cpp yet. > > The thing is that I worry it will lead to linking errors when actually > calling the specialization from another compilation unit (which we do not do > yet so I won't notice) or about getting weird compilation trouble on Windows. > > When I have tried tricks like this in the past, it usually hasn't gone well > :/ > > Do you mind if we try doing so in a follow-up, once we actually start > sharing dictionaries (i.e. after Bug 161932 is fixed)? Oh, it looks like it is easier than I thought. I'll try and do it in this patch.
Created attachment 290688 [details] Patch
Comment on attachment 290688 [details] Patch Clearing flags on attachment: 290688 Committed r206812: <http://trac.webkit.org/changeset/206812>
All reviewed patches have been landed. Closing bug.