Re: Non-header-only headers shared between SpiderMonkey and the rest of Gecko
On Tue, Aug 6, 2019 at 8:54 PM Kris Maglione wrote: > > On Tue, Aug 06, 2019 at 10:56:55AM +0300, Henri Sivonen wrote: > > Do we have some #ifdef for excluding parts of mfbt/ when mfbt/ is being used > > in a non-SpiderMonkey/Gecko context? > > #ifdef MOZ_HAS_MOZGLUE Thanks. This appears to be undefined for gtests that call in to libxul code. (Is that intentional?) It appears that MOZILLA_INTERNAL_API is needed for covering gtests that call into libxul code. As far as I can tell, after https://bugzilla.mozilla.org/show_bug.cgi?id=1572364 , ".cpp that links with jsrust_shared" is detected by: #if defined(MOZILLA_INTERNAL_API) || \ (defined(MOZ_HAS_MOZGLUE) && defined(ENABLE_WASM_CRANELIFT)) Does that look right? I verified this experimentally by checking that the above evaluates to false when compiling --enable-application=tools/update-packaging or --enable-application=memory. (Despite advice to the contrary, I still think it's important for discoverability to put my code in mfbt/TextUtils.h and having it disabled in contexts that don't link with jsrust_shared. It would be bad if e.g. mozilla::IsAscii taking a single char and mozilla::IsAscii taking mozilla::Span weren't discoverable in the same place. Or even worse if mozilla::IsAscii taking '\0'-terminated const char* and mozilla::IsAscii taking mozilla::Span weren't discoverable in the same place.) -- Henri Sivonen hsivo...@mozilla.com ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Non-header-only headers shared between SpiderMonkey and the rest of Gecko
On Tue, Aug 06, 2019 at 10:56:55AM +0300, Henri Sivonen wrote: Do we have some #ifdef for excluding parts of mfbt/ when mfbt/ is being used in a non-SpiderMonkey/Gecko context? #ifdef MOZ_HAS_MOZGLUE ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Non-header-only headers shared between SpiderMonkey and the rest of Gecko
On Tue, Aug 6, 2019 at 10:15 AM Henri Sivonen wrote: > In general, it seems problematic to organize headers based on whether > they have associated .cpp or crate code. I'd expect developers to look > for stuff under mfbt/ instead of some place else, since developers > using the header shouldn't have to know if the implementation is > header-only or not. :-( Notably, in my case the functions would logically belong in Utf8.h (which already has Utf8.cpp under mfbt/) and in TextUtils.h. What should my r+ expectations be if I put the entry points there despite the code requiring linking with Rust crates that SpiderMonkey (and, therefore, Gecko) depends on? Do we have some #ifdef for excluding parts of mfbt/ when mfbt/ is being used in a non-SpiderMonkey/Gecko context? -- Henri Sivonen hsivo...@mozilla.com ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Non-header-only headers shared between SpiderMonkey and the rest of Gecko
On Mon, Aug 5, 2019 at 4:14 PM Gabriele Svelto wrote: > On 05/08/19 12:04, Henri Sivonen wrote: > > I has come to my attention that that putting non-header-only code > > under mfbt/ is something we're trying to get away from: > > https://bugzilla.mozilla.org/show_bug.cgi?id=1554062 > > > > Do we have an appropriate place for headers that declare entry points > > for non-header-only functionality (in my case, backed by Rust code) > > and that depend on types declared in headers that live under mfbt/ and > > that need to be available both to SpiderMonkey and the rest of Gecko? > > IIRC we have some stuff like that under mozglue/misc. The TimeStamp > class for example is used in both Gecko and SpiderMonky, has > platform-dependent C++ implementations (also linking to external > libraries) and uses MFBT headers. Is mozglue only for Gecko and SpiderMonkey and not anything else? (I.e. not crash reporter or anything else that doesn't link the Rust crates that SpiderMonkey links?) In general, it seems problematic to organize headers based on whether they have associated .cpp or crate code. I'd expect developers to look for stuff under mfbt/ instead of some place else, since developers using the header shouldn't have to know if the implementation is header-only or not. :-( -- Henri Sivonen hsivo...@mozilla.com ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Non-header-only headers shared between SpiderMonkey and the rest of Gecko
I has come to my attention that that putting non-header-only code under mfbt/ is something we're trying to get away from: https://bugzilla.mozilla.org/show_bug.cgi?id=1554062 Do we have an appropriate place for headers that declare entry points for non-header-only functionality (in my case, backed by Rust code) and that depend on types declared in headers that live under mfbt/ and that need to be available both to SpiderMonkey and the rest of Gecko? (So far, shipping headers that depend on types that come from mfbt/ inside the related crates.io crate has been suggested, but it seems weird to ship Gecko-specific code via crates.io and Gecko developers probably aren't looking for mfbt type-aware C++ API headers under third_party/rust/.) -- Henri Sivonen hsivo...@mozilla.com ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
