Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Henri Sivonen]

On Tue, Aug 16, 2016 at 2:19 PM, Henri Sivonen  wrote:
>> For GSL polyfills, I think that we should continue to follow the MFBT
>> conventions set thus far and use Gecko style for naming.
>
> OK.

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1295611 to
request mozilla::Span in MFBT.

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Jim Blandy]

On Mon, Aug 15, 2016 at 11:59 AM, Bobby Holley 
wrote:

> On Mon, Aug 15, 2016 at 11:53 AM, Henri Sivonen 
> wrote:
>
>> What I'm asking is:
>>
>> When I take encoding_rs_cpp.h and adapt it to XPCOM/MFBT types for use
>> in Gecko, should this be
>> Encoding::for_label(const nsACString& label) // change only types that
>> need changing
>> or
>> Encoding::ForLabel(const nsACString& aLabel) // change naming style, too
>> ?
>>
>
>
> The latter, IMO.
>

I would agree.

The disadvantage is that people familiar with the Rust API would now have
to study the C++ API separately, because they're no longer trivially
related.

But you've already adapted the argument types to Gecko, and presumably made
other sorts of changes to provide an idiomatic C++ API. That's a good
choice, in my opinion, but it means you've already given up the prospect of
taking familiarity with the Rust API and applying it immediately to use the
C++ API.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Henri Sivonen]

On Aug 15, 2016 21:59, "Bobby Holley"  wrote:
>
> On Mon, Aug 15, 2016 at 11:53 AM, Henri Sivonen 
wrote:
>>
>> On Mon, Aug 15, 2016 at 6:45 PM, Jim Blandy  wrote:
>> > We're using Cheddar to produce C headers for our Rust mp4parse crate;
as far
>> > as I can see, Cheddar doesn't mangle Rust names.
>> >
>> > The Mozilla C++ style applies only to identifiers defined in Mozilla's
C++
>> > code base, not things that we merely use that are defined elsewhere.
When we
>> > use upstream code, we use its definitions in the form they're offered.
I
>> > think Rust code should be treated similarly to "upstream" code in that
>> > sense, and the C++ should use the Rust names unchanged.
>>
>> encoding_rs has three layers of API:
>>  1) Rust
>>  2) FFI/C
>>  3) C++ that wraps the C API so that it can be used in a C++-like way
>> with unique pointers doing the right thing.
>>
>> I think it's been already established that snake_case should be used
>> on layers 1 and 2. So on layer 1, there is Encoding::for_label(label:
>> &[u8]). On layer 2, there's encoding_for_label(const uint8_t* label,
>> size_t label_len). This is clear, and I don't want to reopen the
>> discussion on that.
>
>
> As mentioned we're doing something different for (2) with stylo, but I
guess the issue is that encoding_rs has non-Gecko consumers? That seems
like a reasonable argument for keeping the C FFI snake-cased.

Hopefully, it'll have non-Gecko consumers. Moreover, the only C++ code that
should be calling layer 2 is the implementation for layer 3, so whatever
naming layer 2 has shouldn't be of concern to Gecko C++.

>>
>> For non-Gecko uses,
>>
https://github.com/hsivonen/encoding_rs/blob/master/include/encoding_rs_cpp.h
>> has:
>> Encoding::for_label(gsl::cstring_span<> label)
>>
>> What I'm asking is:
>>
>> When I take encoding_rs_cpp.h and adapt it to XPCOM/MFBT types for use
>> in Gecko, should this be
>> Encoding::for_label(const nsACString& label) // change only types that
>> need changing
>> or
>> Encoding::ForLabel(const nsACString& aLabel) // change naming style, too
>> ?
>
>
> The latter, IMO.

Ok. :-/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Nathan Froyd]

On Mon, Aug 15, 2016 at 9:56 AM, Henri Sivonen  wrote:
> Relatedly, on the topic of MFBT Range and GSL, under the subject "C++
> Core Guidelines" Jim Blandy  wrote:
>> One of the main roles of MFBT is to provide polyfills for features
>> standardized in C++ that we can't use yet for toolchain reasons (remember
>> MOZ_OVERRIDE?); MFBT features get removed as we replace them with the
>> corresponding std thing.
>
> I'd have expected a polyfill that expects to be swapped out to use the
> naming of whatever it's polyfill for, except maybe for the namespace.
> Since MFBT has mozilla::UniquePtr instead of mozilla::unique_ptr, I
> had understood mozilla::UniquePtr as a long-term Gecko-specific
> implementation of the unique pointer concept as opposed to something
> that's expected to be replaced with std::unique_ptr as soon as
> feasible.
>
> Are we getting value out of going against the naming convention of the
> C++ standard library in order to enforce a Mozilla-specific naming
> style?

Keeping the Gecko naming scheme avoids unwanted name conflicts versus
the standard library, and makes it a bit clearer in code where
prefixes are not present that something Gecko-ish is being used.  The
latter is helpful for things that are named similarly to the standard,
but differ dramatically (mozilla::Vector, mozilla::IsPod).  Removing
the polyfills to use something more standardized requires only
sed/perl-style renaming (mostly).  Manual effort to adjust includes
would be necessary whether we chose Gecko style or standard library
style.

> I suggest that we start allowing snake_case C++ in m-c so that C++
> wrappers for the C interfaces to Rust code can be named with mere
> copy-paste of the Rust method names and so that we don't need to
> change naming style of GSL stuff regardless of whether what's in the
> tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
> compilers) of GSL or GSL itself.

I don't follow the argument here for Rust names.  I think it's
reasonable that if one needs to call FFI functions in Rust directly,
then we should use whatever names the Rust library chose for its FFI
interface.  That policy is no different than what we have today for
third-party libraries.  But if one is going to write wrappers around
those FFI functions (resp. third-party libraries), then it seems
equally reasonable that those wrappers should follow Gecko
conventions, and not the conventions of whatever code they are
wrapping.  Again, this is no different than what we have today for
third-party libraries.

For GSL polyfills, I think that we should continue to follow the MFBT
conventions set thus far and use Gecko style for naming.  But that is
partly skepticism about how much in GSL will actually get used and/or
how quickly GSL would get standardized and provided by our compiler
vendors.

-Nathan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Bobby Holley]

On Mon, Aug 15, 2016 at 11:53 AM, Henri Sivonen 
wrote:

> On Mon, Aug 15, 2016 at 6:45 PM, Jim Blandy  wrote:
> > We're using Cheddar to produce C headers for our Rust mp4parse crate; as
> far
> > as I can see, Cheddar doesn't mangle Rust names.
> >
> > The Mozilla C++ style applies only to identifiers defined in Mozilla's
> C++
> > code base, not things that we merely use that are defined elsewhere.
> When we
> > use upstream code, we use its definitions in the form they're offered. I
> > think Rust code should be treated similarly to "upstream" code in that
> > sense, and the C++ should use the Rust names unchanged.
>
> encoding_rs has three layers of API:
>  1) Rust
>  2) FFI/C
>  3) C++ that wraps the C API so that it can be used in a C++-like way
> with unique pointers doing the right thing.
>
> I think it's been already established that snake_case should be used
> on layers 1 and 2. So on layer 1, there is Encoding::for_label(label:
> &[u8]). On layer 2, there's encoding_for_label(const uint8_t* label,
> size_t label_len). This is clear, and I don't want to reopen the
> discussion on that.
>

As mentioned we're doing something different for (2) with stylo, but I
guess the issue is that encoding_rs has non-Gecko consumers? That seems
like a reasonable argument for keeping the C FFI snake-cased.


>
> For non-Gecko uses,
> https://github.com/hsivonen/encoding_rs/blob/master/
> include/encoding_rs_cpp.h
> has:
> Encoding::for_label(gsl::cstring_span<> label)
>
> What I'm asking is:
>
> When I take encoding_rs_cpp.h and adapt it to XPCOM/MFBT types for use
> in Gecko, should this be
> Encoding::for_label(const nsACString& label) // change only types that
> need changing
> or
> Encoding::ForLabel(const nsACString& aLabel) // change naming style, too
> ?
>

The latter, IMO.


>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Henri Sivonen]

On Mon, Aug 15, 2016 at 6:45 PM, Jim Blandy  wrote:
> We're using Cheddar to produce C headers for our Rust mp4parse crate; as far
> as I can see, Cheddar doesn't mangle Rust names.
>
> The Mozilla C++ style applies only to identifiers defined in Mozilla's C++
> code base, not things that we merely use that are defined elsewhere. When we
> use upstream code, we use its definitions in the form they're offered. I
> think Rust code should be treated similarly to "upstream" code in that
> sense, and the C++ should use the Rust names unchanged.

encoding_rs has three layers of API:
 1) Rust
 2) FFI/C
 3) C++ that wraps the C API so that it can be used in a C++-like way
with unique pointers doing the right thing.

I think it's been already established that snake_case should be used
on layers 1 and 2. So on layer 1, there is Encoding::for_label(label:
&[u8]). On layer 2, there's encoding_for_label(const uint8_t* label,
size_t label_len). This is clear, and I don't want to reopen the
discussion on that.

For non-Gecko uses,
https://github.com/hsivonen/encoding_rs/blob/master/include/encoding_rs_cpp.h
has:
Encoding::for_label(gsl::cstring_span<> label)

What I'm asking is:

When I take encoding_rs_cpp.h and adapt it to XPCOM/MFBT types for use
in Gecko, should this be
Encoding::for_label(const nsACString& label) // change only types that
need changing
or
Encoding::ForLabel(const nsACString& aLabel) // change naming style, too
?

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Bobby Holley]

I agree with Jim that the introduction of Rust code doesn't fundamentally
alter the dynamic of our style conventions. We're not going to get
consistency with all upstream projects and bound languages no matter what
convention we pick, and changing mozilla style would be hugely disruptive.

Regarding which style to pick for the boundary:

Rust already uses CamelCase types. In Stylo, we're using CamelCase
functions for our FFI bindings, along with a Gecko_ or Servo_ prefix to
indicate on which side of the boundary the function is defined. This makes
sense for a variety of reasons:
* The binding generator we have goes from .h => .rs, not the other way
around. So the code we hand-write is mostly-idiomatic code, while the
generated code is not (which is preferable to the alternatively).
* Invoking C++ functions from rust is already clunky because you have to
use unsafe { }, so it doesn't hurt much for the function naming to be
non-idiomatic there.
* Rust/Servo have good tooling already (tidy) to enforce stylistic
consistency, with explicit granular opt-outs. This means that non-idiomatic
style is less likely to unintentionally spread in Rust than in C++.
* We can represent most C++ types in Rust but not vice-versa. This (along
with the need for custom linkage) means that Rust APIs already need a lot
of special stuff to be usable from C++, so the naming convention is not an
onerous requirement.
* Similar to the above, we end up using lots of non-primitive C++ types as
non-opaque in Rust, but non-primitive Rust types are pretty much always
opaque in C++. This means that the C++ style convention has more momentum
at the boundary.

http://searchfox.org/mozilla-central/source/layout/style/ServoBindings.h
https://github.com/servo/servo/blob/master/ports/geckolib/gecko_bindings/bindings.rs
https://github.com/servo/servo/blob/master/ports/geckolib/wrapper.rs
https://github.com/servo/servo/blob/master/ports/geckolib/glue.rs

bholley

On Mon, Aug 15, 2016 at 8:46 AM, Jim Blandy  wrote:

> tl;dr: There's a reason it's called "mangling"...
>
> On Mon, Aug 15, 2016 at 8:45 AM, Jim Blandy  wrote:
>
> > I suggest that we start allowing snake_case C++ in m-c so that C++
> >> wrappers for the C interfaces to Rust code can be named with mere
> >> copy-paste of the Rust method names and so that we don't need to
> >> change naming style of GSL stuff regardless of whether what's in the
> >> tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
> >> compilers) of GSL or GSL itself.
> >>
> >
> > Has anyone suggested mangling C++ bindings for Rust definitions into
> > Mozilla C++ style, making them different from the Rust names? I'm
> opposed!
> > :)
> >
> > In the most general case, cross-language adapters must mangle the names
> > they deal with. For example, XPIDL has to affix something to IDL
> attribute
> > names when naming the C++ accessors, and "Get" and "Set" prefixes aren't
> > worse than anything else. And maybe the languages just have different
> > identifier syntaxes, so mangling is necessary simply to produce
> well-formed
> > code. But when an unmangled conversion is possible, that seems clearly
> > preferable. We're using Cheddar to produce C headers for our Rust
> mp4parse
> > crate; as far as I can see, Cheddar doesn't mangle Rust names.
> >
> > The Mozilla C++ style applies only to identifiers defined in Mozilla's
> C++
> > code base, not things that we merely use that are defined elsewhere. When
> > we use upstream code, we use its definitions in the form they're
> offered. I
> > think Rust code should be treated similarly to "upstream" code in that
> > sense, and the C++ should use the Rust names unchanged.
> >
> > It's true that the benefit from a convention increases the more
> > consistently it's applied, but as long as we want to use upstream code
> > bases, we must work in a multi-style world, so universal conformance is
> > just never going to happen. In that light, the C++ standard and GSL are
> > just two more cases of an upstream project not matching Mozilla style.
> >
> > You don't quite spell it out out, but I feel like you're hoping to argue
> > that the C and C++ standard libraries' use of snake_case suggests that
> it's
> > somehow acceptable, or ought to be acceptable, for Mozilla C++
> definitions
> > too. But these are mostly arbitrary decisions, and our well-established
> > arbitrary decision is that that's not acceptable.
> >
> >
> > On Mon, Aug 15, 2016 at 6:56 AM, Henri Sivonen 
> > wrote:
> >
> >> We've already established that Rust code in m-c will use the Rust
> >> coding style, so we'll have snake_case methods and functions in Rust
> >> code in m-c. Also, Rust code that exposes an FFI interface typically
> >> does so with snake_case functions, which look natural both for Rust
> >> and for C as C style is influenced by the C standard library.
> >>
> >> When a Rust library provides a C++ interface, the C++ 

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Jim Blandy]

tl;dr: There's a reason it's called "mangling"...

On Mon, Aug 15, 2016 at 8:45 AM, Jim Blandy  wrote:

> I suggest that we start allowing snake_case C++ in m-c so that C++
>> wrappers for the C interfaces to Rust code can be named with mere
>> copy-paste of the Rust method names and so that we don't need to
>> change naming style of GSL stuff regardless of whether what's in the
>> tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
>> compilers) of GSL or GSL itself.
>>
>
> Has anyone suggested mangling C++ bindings for Rust definitions into
> Mozilla C++ style, making them different from the Rust names? I'm opposed!
> :)
>
> In the most general case, cross-language adapters must mangle the names
> they deal with. For example, XPIDL has to affix something to IDL attribute
> names when naming the C++ accessors, and "Get" and "Set" prefixes aren't
> worse than anything else. And maybe the languages just have different
> identifier syntaxes, so mangling is necessary simply to produce well-formed
> code. But when an unmangled conversion is possible, that seems clearly
> preferable. We're using Cheddar to produce C headers for our Rust mp4parse
> crate; as far as I can see, Cheddar doesn't mangle Rust names.
>
> The Mozilla C++ style applies only to identifiers defined in Mozilla's C++
> code base, not things that we merely use that are defined elsewhere. When
> we use upstream code, we use its definitions in the form they're offered. I
> think Rust code should be treated similarly to "upstream" code in that
> sense, and the C++ should use the Rust names unchanged.
>
> It's true that the benefit from a convention increases the more
> consistently it's applied, but as long as we want to use upstream code
> bases, we must work in a multi-style world, so universal conformance is
> just never going to happen. In that light, the C++ standard and GSL are
> just two more cases of an upstream project not matching Mozilla style.
>
> You don't quite spell it out out, but I feel like you're hoping to argue
> that the C and C++ standard libraries' use of snake_case suggests that it's
> somehow acceptable, or ought to be acceptable, for Mozilla C++ definitions
> too. But these are mostly arbitrary decisions, and our well-established
> arbitrary decision is that that's not acceptable.
>
>
> On Mon, Aug 15, 2016 at 6:56 AM, Henri Sivonen 
> wrote:
>
>> We've already established that Rust code in m-c will use the Rust
>> coding style, so we'll have snake_case methods and functions in Rust
>> code in m-c. Also, Rust code that exposes an FFI interface typically
>> does so with snake_case functions, which look natural both for Rust
>> and for C as C style is influenced by the C standard library.
>>
>> When a Rust library provides a C++ interface, the C++ interface is
>> built on top of the C/FFI interface. Per above, the Rust and C layers
>> use snake_case for methods/functions.
>>
>> As it happens, the C++ standard library also uses snake_case, so for a
>> C++ interface to a Rust library outside of the Gecko context, it's not
>> unnatural to use snake_case methods on the C++ layer, too. Like this:
>> https://github.com/hsivonen/encoding_rs/blob/master/include/
>> encoding_rs_cpp.h
>>
>> Since Gecko has does not use C++ standard-library strings and, at
>> least currently, does not use GSL, a slightly different C++ wrapper is
>> called for in the Gecko case.
>>
>> But should such a wrapper just use XPCOM nsACString and MFBT Range or
>> should it also change the names of the methods to follow Gecko case
>> rules?
>>
>> Relatedly, on the topic of MFBT Range and GSL, under the subject "C++
>> Core Guidelines" Jim Blandy  wrote:
>> > Given GSL's pedigree, I was assuming that we'd bring it in-tree and
>> switch
>> > out MFBT facilities with the corresponding GSL things as they became
>> > available.
>> >
>> > One of the main roles of MFBT is to provide polyfills for features
>> > standardized in C++ that we can't use yet for toolchain reasons
>> (remember
>> > MOZ_OVERRIDE?); MFBT features get removed as we replace them with the
>> > corresponding std thing.
>>
>> I'd have expected a polyfill that expects to be swapped out to use the
>> naming of whatever it's polyfill for, except maybe for the namespace.
>> Since MFBT has mozilla::UniquePtr instead of mozilla::unique_ptr, I
>> had understood mozilla::UniquePtr as a long-term Gecko-specific
>> implementation of the unique pointer concept as opposed to something
>> that's expected to be replaced with std::unique_ptr as soon as
>> feasible.
>>
>> Are we getting value out of going against the naming convention of the
>> C++ standard library in order to enforce a Mozilla-specific naming
>> style?
>>
>> I suggest that we start allowing snake_case C++ in m-c so that C++
>> wrappers for the C interfaces to Rust code can be named with mere
>> copy-paste of the Rust method names and so that we don't need to
>> 

Re: snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Jim Blandy]

>
> I suggest that we start allowing snake_case C++ in m-c so that C++
> wrappers for the C interfaces to Rust code can be named with mere
> copy-paste of the Rust method names and so that we don't need to
> change naming style of GSL stuff regardless of whether what's in the
> tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
> compilers) of GSL or GSL itself.
>

Has anyone suggested mangling C++ bindings for Rust definitions into
Mozilla C++ style, making them different from the Rust names? I'm opposed!
:)

In the most general case, cross-language adapters must mangle the names
they deal with. For example, XPIDL has to affix something to IDL attribute
names when naming the C++ accessors, and "Get" and "Set" prefixes aren't
worse than anything else. And maybe the languages just have different
identifier syntaxes, so mangling is necessary simply to produce well-formed
code. But when an unmangled conversion is possible, that seems clearly
preferable. We're using Cheddar to produce C headers for our Rust mp4parse
crate; as far as I can see, Cheddar doesn't mangle Rust names.

The Mozilla C++ style applies only to identifiers defined in Mozilla's C++
code base, not things that we merely use that are defined elsewhere. When
we use upstream code, we use its definitions in the form they're offered. I
think Rust code should be treated similarly to "upstream" code in that
sense, and the C++ should use the Rust names unchanged.

It's true that the benefit from a convention increases the more
consistently it's applied, but as long as we want to use upstream code
bases, we must work in a multi-style world, so universal conformance is
just never going to happen. In that light, the C++ standard and GSL are
just two more cases of an upstream project not matching Mozilla style.

You don't quite spell it out out, but I feel like you're hoping to argue
that the C and C++ standard libraries' use of snake_case suggests that it's
somehow acceptable, or ought to be acceptable, for Mozilla C++ definitions
too. But these are mostly arbitrary decisions, and our well-established
arbitrary decision is that that's not acceptable.

On Mon, Aug 15, 2016 at 6:56 AM, Henri Sivonen  wrote:

> We've already established that Rust code in m-c will use the Rust
> coding style, so we'll have snake_case methods and functions in Rust
> code in m-c. Also, Rust code that exposes an FFI interface typically
> does so with snake_case functions, which look natural both for Rust
> and for C as C style is influenced by the C standard library.
>
> When a Rust library provides a C++ interface, the C++ interface is
> built on top of the C/FFI interface. Per above, the Rust and C layers
> use snake_case for methods/functions.
>
> As it happens, the C++ standard library also uses snake_case, so for a
> C++ interface to a Rust library outside of the Gecko context, it's not
> unnatural to use snake_case methods on the C++ layer, too. Like this:
> https://github.com/hsivonen/encoding_rs/blob/master/include/
> encoding_rs_cpp.h
>
> Since Gecko has does not use C++ standard-library strings and, at
> least currently, does not use GSL, a slightly different C++ wrapper is
> called for in the Gecko case.
>
> But should such a wrapper just use XPCOM nsACString and MFBT Range or
> should it also change the names of the methods to follow Gecko case
> rules?
>
> Relatedly, on the topic of MFBT Range and GSL, under the subject "C++
> Core Guidelines" Jim Blandy  wrote:
> > Given GSL's pedigree, I was assuming that we'd bring it in-tree and
> switch
> > out MFBT facilities with the corresponding GSL things as they became
> > available.
> >
> > One of the main roles of MFBT is to provide polyfills for features
> > standardized in C++ that we can't use yet for toolchain reasons (remember
> > MOZ_OVERRIDE?); MFBT features get removed as we replace them with the
> > corresponding std thing.
>
> I'd have expected a polyfill that expects to be swapped out to use the
> naming of whatever it's polyfill for, except maybe for the namespace.
> Since MFBT has mozilla::UniquePtr instead of mozilla::unique_ptr, I
> had understood mozilla::UniquePtr as a long-term Gecko-specific
> implementation of the unique pointer concept as opposed to something
> that's expected to be replaced with std::unique_ptr as soon as
> feasible.
>
> Are we getting value out of going against the naming convention of the
> C++ standard library in order to enforce a Mozilla-specific naming
> style?
>
> I suggest that we start allowing snake_case C++ in m-c so that C++
> wrappers for the C interfaces to Rust code can be named with mere
> copy-paste of the Rust method names and so that we don't need to
> change naming style of GSL stuff regardless of whether what's in the
> tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
> compilers) of GSL or GSL itself.
>
> Of course, we won't change all existing code to snake_case at the 

snake_case C++ in m-c (was: Re: C++ Core Guidelines)

[Henri Sivonen]

We've already established that Rust code in m-c will use the Rust
coding style, so we'll have snake_case methods and functions in Rust
code in m-c. Also, Rust code that exposes an FFI interface typically
does so with snake_case functions, which look natural both for Rust
and for C as C style is influenced by the C standard library.

When a Rust library provides a C++ interface, the C++ interface is
built on top of the C/FFI interface. Per above, the Rust and C layers
use snake_case for methods/functions.

As it happens, the C++ standard library also uses snake_case, so for a
C++ interface to a Rust library outside of the Gecko context, it's not
unnatural to use snake_case methods on the C++ layer, too. Like this:
https://github.com/hsivonen/encoding_rs/blob/master/include/encoding_rs_cpp.h

Since Gecko has does not use C++ standard-library strings and, at
least currently, does not use GSL, a slightly different C++ wrapper is
called for in the Gecko case.

But should such a wrapper just use XPCOM nsACString and MFBT Range or
should it also change the names of the methods to follow Gecko case
rules?

Relatedly, on the topic of MFBT Range and GSL, under the subject "C++
Core Guidelines" Jim Blandy  wrote:
> Given GSL's pedigree, I was assuming that we'd bring it in-tree and switch
> out MFBT facilities with the corresponding GSL things as they became
> available.
>
> One of the main roles of MFBT is to provide polyfills for features
> standardized in C++ that we can't use yet for toolchain reasons (remember
> MOZ_OVERRIDE?); MFBT features get removed as we replace them with the
> corresponding std thing.

I'd have expected a polyfill that expects to be swapped out to use the
naming of whatever it's polyfill for, except maybe for the namespace.
Since MFBT has mozilla::UniquePtr instead of mozilla::unique_ptr, I
had understood mozilla::UniquePtr as a long-term Gecko-specific
implementation of the unique pointer concept as opposed to something
that's expected to be replaced with std::unique_ptr as soon as
feasible.

Are we getting value out of going against the naming convention of the
C++ standard library in order to enforce a Mozilla-specific naming
style?

I suggest that we start allowing snake_case C++ in m-c so that C++
wrappers for the C interfaces to Rust code can be named with mere
copy-paste of the Rust method names and so that we don't need to
change naming style of GSL stuff regardless of whether what's in the
tree is a Mozilla polyfill for GSL, a third-party polyfill (for legacy
compilers) of GSL or GSL itself.

Of course, we won't change all existing code to snake_case at the same
time, and the mix of Mozilla C++ case and snake_case would be somewhat
aesthetically offensive. But does maintaining a house style that
differs from e.g. the standard library of C++ itself provide us more
value that more direct copypasteability would?

(FWIW, as precedent, in the HTML parser, the names of methods that are
translated from Java use Java naming conventions. Since that code goes
through a translator anyway, as opposed to be manual copypasta, I
offered to make the translator change the method naming to Mozilla C++
style but was told not to bother.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Jim Blandy]

Given GSL's pedigree, I was assuming that we'd bring it in-tree and switch
out MFBT facilities with the corresponding GSL things as they became
available.

One of the main roles of MFBT is to provide polyfills for features
standardized in C++ that we can't use yet for toolchain reasons (remember
MOZ_OVERRIDE?); MFBT features get removed as we replace them with the
corresponding std thing. Why would Range vs. GSL span be any different?


On Fri, Jul 15, 2016 at 3:44 AM, Henri Sivonen  wrote:

> On Thu, Mar 24, 2016 at 6:01 PM, Jeff Muizelaar 
> wrote:
> > On Wed, Jan 6, 2016 at 7:15 AM, Henri Sivonen 
> wrote:
> >> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
> >>> For those who are interested in this, there's a bug to consider
> integrating
> >>> the Guidelines Support Library (GSL) into the tree:
> >>>
> >>> https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
> >>
> >> This bug appears to have stalled.
> >>
> >> What should my expectations be regarding getting an equivalent of (at
> >> least single-dimensional) GSL span (formerly array_view;
> >> conceptually Rust's slice) into MFBT?
> >
> > Something like this already exits: mfbt/Range.h
>
> And we also have
> https://dxr.mozilla.org/mozilla-central/source/gfx/src/ArrayView.h
> whose comments say to prefer Range.
>
> ArrayView as well as GSL span use pointer and length while Range uses
> pointer and pointer past end.
>
> Are we happy enough with Range to the point where Range should be
> promoted in the codebase where the Core Guidelines would recommend
> span?
>
> (What to call it is, of course, a total bikeshed, but when the Core
> Guidelines are happening near the C++ standardization source of
> authority, it seems rather NIH-y to call it something other than
> "span" even if there are still compiler compat reasons [are there?]
> not to use Microsoft's span.h outright.)
>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Henri Sivonen]

On Thu, Mar 24, 2016 at 6:01 PM, Jeff Muizelaar  wrote:
> On Wed, Jan 6, 2016 at 7:15 AM, Henri Sivonen  wrote:
>> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
>>> For those who are interested in this, there's a bug to consider integrating
>>> the Guidelines Support Library (GSL) into the tree:
>>>
>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
>>
>> This bug appears to have stalled.
>>
>> What should my expectations be regarding getting an equivalent of (at
>> least single-dimensional) GSL span (formerly array_view;
>> conceptually Rust's slice) into MFBT?
>
> Something like this already exits: mfbt/Range.h

And we also have
https://dxr.mozilla.org/mozilla-central/source/gfx/src/ArrayView.h
whose comments say to prefer Range.

ArrayView as well as GSL span use pointer and length while Range uses
pointer and pointer past end.

Are we happy enough with Range to the point where Range should be
promoted in the codebase where the Core Guidelines would recommend
span?

(What to call it is, of course, a total bikeshed, but when the Core
Guidelines are happening near the C++ standardization source of
authority, it seems rather NIH-y to call it something other than
"span" even if there are still compiler compat reasons [are there?]
not to use Microsoft's span.h outright.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Jeff Muizelaar]

On Wed, Jan 6, 2016 at 7:15 AM, Henri Sivonen  wrote:
> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
>> For those who are interested in this, there's a bug to consider integrating
>> the Guidelines Support Library (GSL) into the tree:
>>
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
>
> This bug appears to have stalled.
>
> What should my expectations be regarding getting an equivalent of (at
> least single-dimensional) GSL span (formerly array_view;
> conceptually Rust's slice) into MFBT?

Something like this already exits: mfbt/Range.h

-Jeff
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Henri Sivonen]

On Wed, Jan 13, 2016 at 12:53 AM, Ehsan Akhgari  wrote:
> I'll investigate integrating gsl-lite into m-c.

Thank you.

On Mon, Jan 11, 2016 at 10:03 PM, Brian Smith  wrote:
> mozilla::pkix::Input/Reader will never throw an exception or abort the
> process; instead it always returns an explicit success/failure result. It
> seems GSL will either abort or throw an exception in many situations. Since
> aborting is terrible and exceptions are not allowed in Gecko code, it seems
> Input/Reader is safer.

For uses where bound checks are merely a seat belt to avoid remote
code execution or memory disclosure vulnerabilities in case of a
programming error, aborting the process in probably better than asking
the caller to have error handling code that hopefully never runs.

(Out-of-bounds access of slices on the Rust side of FFI panics anyway.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Ehsan Akhgari]

On 2016-01-06 7:15 AM, Henri Sivonen wrote:

On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:

For those who are interested in this, there's a bug to consider integrating
the Guidelines Support Library (GSL) into the tree:

https://bugzilla.mozilla.org/show_bug.cgi?id=1208262


This bug appears to have stalled.

What should my expectations be regarding getting an equivalent of (at
least single-dimensional) GSL span (formerly array_view;
conceptually Rust's slice) into MFBT?


I'll investigate integrating gsl-lite into m-c.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Brian Smith]

Henri Sivonen  wrote:

> On Wed, Jan 6, 2016 at 9:27 PM, Brian Smith  wrote:
> > Henri Sivonen  wrote:
> >>
> >> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
> >> > For those who are interested in this, there's a bug to consider
> >> > integrating
> >> > the Guidelines Support Library (GSL) into the tree:
> >> >
> >> > https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
> >>
> >> This bug appears to have stalled.
> >>
> >> What should my expectations be regarding getting an equivalent of (at
> >> least single-dimensional) GSL span (formerly array_view;
> >> conceptually Rust's slice) into MFBT?
> >>
> >> > On 30/09/2015 22:00, Botond Ballo wrote:
> >> >> The document is a work in progress, still incomplete in many places.
> >> >> The initial authors are Bjarne Stroustrup and Herb Sutter, two
> members
> >> >> of the C++ Standards Committee, and they welcome contributions via
> >> >> GitHub to help complete and improve it.
> >>
> >> In their keynotes, a template called array_buffer was mentioned. What
> >> happened to it? array_buffer was supposed to be array_view
> >> (since renamed to span) plus an additional size_t communicating
> >> current position in the buffer. Surprisingly, Core Guidelines has an
> >> example of reading up to n items into span but the example doesn't
> >> show how the function would signal how many bytes between 0 and n it
> >> actually read, so the Guidelines themselves don't seem to give a
> >> proper answer to signaling how many items of a span a function read or
> >> wrote.
> >
> >
> > This functionality already exists--in a safer form than the Core C++
> > form--in Gecko: mozilla::pkix::Input and mozilla::pkix::Reader.
>
> I admit I'm not familiar with the nuances of either GSL span or
> mozilla::pkix::Input. What makes the latter safer?
>

mozilla::pkix::Input/Reader will never throw an exception or abort the
process; instead it always returns an explicit success/failure result. It
seems GSL will either abort or throw an exception in many situations. Since
aborting is terrible and exceptions are not allowed in Gecko code, it seems
Input/Reader is safer.

The documentation for the Rust version of Input/Reader [1] attempts to
explain more of the benefits of the Input/Reader approach. The one in
*ring* is better than the one in mozilla::pkix in quite a few respects, but
the idea is mostly the same.


> mozilla::pkix::Input seems to be read-only. I'm looking for both
> read-only and writable spans.
>

That's something Input/Reader doesn't do, because it is focused exclusively
on parsing (untrusted) input.

[1] https://briansmith.org/rustdoc/ring/input/index.html

Cheers,
Brian
-- 
https://briansmith.org/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Henri Sivonen]

On Wed, Jan 6, 2016 at 9:27 PM, Brian Smith  wrote:
> Henri Sivonen  wrote:
>>
>> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
>> > For those who are interested in this, there's a bug to consider
>> > integrating
>> > the Guidelines Support Library (GSL) into the tree:
>> >
>> > https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
>>
>> This bug appears to have stalled.
>>
>> What should my expectations be regarding getting an equivalent of (at
>> least single-dimensional) GSL span (formerly array_view;
>> conceptually Rust's slice) into MFBT?
>>
>> > On 30/09/2015 22:00, Botond Ballo wrote:
>> >> The document is a work in progress, still incomplete in many places.
>> >> The initial authors are Bjarne Stroustrup and Herb Sutter, two members
>> >> of the C++ Standards Committee, and they welcome contributions via
>> >> GitHub to help complete and improve it.
>>
>> In their keynotes, a template called array_buffer was mentioned. What
>> happened to it? array_buffer was supposed to be array_view
>> (since renamed to span) plus an additional size_t communicating
>> current position in the buffer. Surprisingly, Core Guidelines has an
>> example of reading up to n items into span but the example doesn't
>> show how the function would signal how many bytes between 0 and n it
>> actually read, so the Guidelines themselves don't seem to give a
>> proper answer to signaling how many items of a span a function read or
>> wrote.
>
>
> This functionality already exists--in a safer form than the Core C++
> form--in Gecko: mozilla::pkix::Input and mozilla::pkix::Reader.

I admit I'm not familiar with the nuances of either GSL span or
mozilla::pkix::Input. What makes the latter safer?

mozilla::pkix::Input seems to be read-only. I'm looking for both
read-only and writable spans.

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Henri Sivonen]

On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
> For those who are interested in this, there's a bug to consider integrating
> the Guidelines Support Library (GSL) into the tree:
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=1208262

This bug appears to have stalled.

What should my expectations be regarding getting an equivalent of (at
least single-dimensional) GSL span (formerly array_view;
conceptually Rust's slice) into MFBT?

> On 30/09/2015 22:00, Botond Ballo wrote:
>> The document is a work in progress, still incomplete in many places.
>> The initial authors are Bjarne Stroustrup and Herb Sutter, two members
>> of the C++ Standards Committee, and they welcome contributions via
>> GitHub to help complete and improve it.

In their keynotes, a template called array_buffer was mentioned. What
happened to it? array_buffer was supposed to be array_view
(since renamed to span) plus an additional size_t communicating
current position in the buffer. Surprisingly, Core Guidelines has an
example of reading up to n items into span but the example doesn't
show how the function would signal how many bytes between 0 and n it
actually read, so the Guidelines themselves don't seem to give a
proper answer to signaling how many items of a span a function read or
wrote.

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Brian Smith]

Henri Sivonen  wrote:

> On Thu, Oct 1, 2015 at 9:58 PM, Jonathan Watt  wrote:
> > For those who are interested in this, there's a bug to consider
> integrating
> > the Guidelines Support Library (GSL) into the tree:
> >
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1208262
>
> This bug appears to have stalled.
>
> What should my expectations be regarding getting an equivalent of (at
> least single-dimensional) GSL span (formerly array_view;
> conceptually Rust's slice) into MFBT?
>
> > On 30/09/2015 22:00, Botond Ballo wrote:
> >> The document is a work in progress, still incomplete in many places.
> >> The initial authors are Bjarne Stroustrup and Herb Sutter, two members
> >> of the C++ Standards Committee, and they welcome contributions via
> >> GitHub to help complete and improve it.
>
> In their keynotes, a template called array_buffer was mentioned. What
> happened to it? array_buffer was supposed to be array_view
> (since renamed to span) plus an additional size_t communicating
> current position in the buffer. Surprisingly, Core Guidelines has an
> example of reading up to n items into span but the example doesn't
> show how the function would signal how many bytes between 0 and n it
> actually read, so the Guidelines themselves don't seem to give a
> proper answer to signaling how many items of a span a function read or
> wrote.
>

This functionality already exists--in a safer form than the Core C++
form--in Gecko: mozilla::pkix::Input and mozilla::pkix::Reader.

Cheers,
Brian
-- 
https://briansmith.org/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ Core Guidelines

[Jonathan Watt]

For those who are interested in this, there's a bug to consider integrating the 
Guidelines Support Library (GSL) into the tree:


https://bugzilla.mozilla.org/show_bug.cgi?id=1208262

On 30/09/2015 22:00, Botond Ballo wrote:

Hi folks,

I wanted to draw your attention to a new project underway in the C++
standards community.

It's a document called "C++ Core Guidelines" [1], intended to be a
collection of widely applicable C++ best practices, that can serve as
a template on which C++ projects can base their own, more specific
guidelines.

The document is a work in progress, still incomplete in many places.
The initial authors are Bjarne Stroustrup and Herb Sutter, two members
of the C++ Standards Committee, and they welcome contributions via
GitHub to help complete and improve it.

At some point (probably some time later when the guidelines are more
complete), we may want to see if there's anything in there that we'd
like to lift into our own Mozilla guidelines.

Cheers,
Botond

[1] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform