Re: [webkit-dev] [webkit-reviewers] usage of auto

[saam barati]

> On Jan 11, 2017, at 11:15 AM, JF Bastien  wrote:
> 
> Would it be helpful to focus on small well-defined cases where auto makes 
> sense, and progressively grow that list as we see fit?
> 
> 
> e.g. I think this is great:
> auto ptr = std::make_unique(bar);
> Proposed rule: if the type is obvious because it's on the line, then auto is 
> good.
> Similarly:
> auto i = static_cast(j);
> auto foo = make_foo();
> auto bar = something.get_bar(); // Sometimes, "bar" is obvious.
I'm not sure I agree with this style. There are times where the type of an auto 
variable is obvious-enough, but it's almost never more obvious than actually 
writing out the types. Writing out types, for my brain at least, almost always 
makes the code easier to understand. The most obvious place where I prefer auto 
over explicit types is when something has a lot of template bloat.

I feel like the places where auto makes the code better are limited, but places 
where auto makes the code more confusing, or requires me to spend more time 
figuring it out, are widespread. (Again, this is how my brain reads code.)

Also, I completely agree with Geoff that I use types to grep around the source 
code and to figure out what data structures are being used. If we used auto 
more inside JSC it would hurt my workflow for reading and understanding new 
code.

- Saam

> 
> 
> Range-based loops are a bit tricky. IMO containers with "simple" types are 
> good candidates for either:
> for (const auto& v : cont) { /* don't change v */ }
> for auto& v : cont) { /* change v */ }
> But what's "simple"? I'd say all numeric, pointer, and string types at least. 
> It gets tricky for more complex types, and I'd often rather have the type in 
> the loop. Here's more discussion on this, including a recommendation to use 
> auto&& on range-based loops! I think this gets confusing, and I'm not a huge 
> fan of r-value references everywhere.
> 
> 
> Here's another I like, which Yusuke pointed out a while ago (in ES6 Module's 
> implementation?):
> struct Foo {
>   typedef Something Bar;
>   // ...
>   Bar doIt();
> };
> auto Foo::doIt() -> Bar
> {
>   // ...
> }
> Why? Because Bar is scoped to Foo! It looks odd the first time, but I think 
> this is idiomatic "modern" C++.
> 
> 
> I also like creating unnamed types, though I know this isn't everyone's 
> liking:
> auto ohMy()
> {
>   struct { int a; float b; } result;
>   // ...
>   return result;
> }
> void yeah()
> {
>   auto myMy = ohMy();
>   dataLogLn(myMy.a, myMy.b);
> }
> I initially had that with consumeLoad, which returns a T as well as a 
> ConsumeDependency. I couldn't care less about the container for T and 
> ConsumeDependency, I just want these two values.
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Dan Bernstein]

> On Jan 11, 2017, at 5:43 PM, Maciej Stachowiak  wrote:
> 
> 
> To be clear, I think PAL is a great move. Details like the namespace, 
> placement of the code, and how includes look are all things that can be 
> changed after the fact.
> 
> That said, I think we need to ultimately consider our porting layer to be the 
> combination of PAL and WTF, so it is good for the two to be consistent. I 
> hope revisions along these lines can be considered in due course.

Indeed, some considerations relating to the Apple production build process led 
me to suggest that we avoid adding another subdirectory under Source. I believe 
that at some point in the future, those considerations will no longer hold. 
Even before then, while still keeping the PAL source code inside 
Source/WebCore, we should be able to take advantage of Xcode’s support for 
cross-project references and create a separate Xcode project for PAL (in a 
subdirectory of Source/WebCore), which will allow for better—or at least 
simpler—enforcement of the separation (and for an easier move to a top-level 
Source subdirectory in the future).

> 
> Regards,
> Maciej
> 
>> On Jan 11, 2017, at 3:30 PM, Olmstead, Don > > wrote:
>> 
>> I was the one who did the WebCore::PAL namespace so I wanted to chime in on 
>> why I went that route. We at Sony are newcomers to pushing to trunk so my 
>> explanation might be entirely too idealistic but here goes.
>>  
>> I had thought of PAL as a library that is internal to WebCore that provides 
>> a clear porting layer. I would not expect anyone outside of WebCore to be 
>> linking to it. Because of that it was living inside Source/WebCore, and 
>> since it was setup that way having an internal namespace of WebCore::PAL 
>> made sense conceptually. Also in the future if PAL was successful I could 
>> see a WebKit2 equivalent.
>>  
>> Whatever the consensus is we’re looking forward to working on getting the 
>> PAL layer up and running. We’re working on rebooting our port so we’re in a 
>> good position to help build it out and do any refactoring to help create a 
>> clear layering. Having a clear porting layer, especially one with tests, is 
>> something we’re hoping will be beneficial to all the ports.
>>  
>> From: webkit-dev-boun...@lists.webkit.org 
>>  
>> [mailto:webkit-dev-boun...@lists.webkit.org 
>> ] On Behalf Of Maciej Stachowiak
>> Sent: Wednesday, January 11, 2017 2:05 PM
>> To: Antti Koivisto mailto:koivi...@iki.fi>>
>> Cc: Webkit Development List > >; mrobin...@igalia.com 
>> 
>> Subject: Re: [webkit-dev] WebCore/platform standalone library
>>  
>>  
>> These both sound right to me. 
>>  
>> More generally, I would expect that over time, PAL would likely become a 
>> peer project to WebCore instead of being inside it, much the same way WTF 
>> started inside JavaScriptCore and eventually moved outside it in the source 
>> tree. In the WTF case, it always had a separate top-level namespace.
>>  
>> On Jan 11, 2017, at 12:27 PM, Antti Koivisto > > wrote:
>>  
>> Why is the PAL namespace inside the WebCore namespace? Couldn't it just be a 
>> top-level namespace (even if it currently happens to live in the WebCore 
>> project)?
>>  
>> #include  would be more consistent with existing headers than 
>> .
>>  
>>  
>>   antti
>>  
>> On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield > > wrote:
>> After 18 months of no progress, Don Olmstead and I are getting the band back 
>> together!
>>  
>> We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 
>>  which incorporates feedback 
>> from many different stakeholders (and as such, the direction is a little 
>> different than where I was going with this in the beginning).
>>  
>> First of all, this isn’t a new project; instead, it’s a new target inside 
>> the WebCore project. The target creates a static library which gets linked 
>> into WebCore, which means that the enforcement mechanism can’t be done by 
>> the linker. Instead, the layering will be enforced by a Python script, 
>> triggered as an extra build step, which checks the symbol names inside the 
>> .a file as well as #include directives in source code.
>>  
>> We opted for WebCore to include files using “#include ” instead 
>> of just including Foo.h. Similarly, we are putting symbols inside the PAL 
>> namespace, which is a child of the WebCore namespace. Therefore, inside 
>> WebCore, you use PAL things by specifying “PAL::Foo”.
>>  
>> The first thing to move into PAL is the “crypto” subfolder, which is a good 
>> candidate because it’s small, simple, yet also has platform-dependent 
>> implementations.
>>  
>> We would love your feedback on this approach to help make the dream a 
>> reality!
>>  
>> 

Re: [webkit-dev] WebCore/platform standalone library

[Michael Catanzaro]

On Wed, 2017-01-11 at 23:30 +, Olmstead, Don wrote:
> I had thought of PAL as a library that is internal to WebCore that
> provides a clear porting layer. I would not expect anyone outside of
> WebCore to be linking to it.

So honestly, I don't think this is very realistic. Lots of stuff
currently under WebCore/platform needs to be used by WebKit and
WebKit2. ResourceHandle, for example. Or TextChecker. These are just
the very first two that come to mind. So if we want PAL to be
completely internal to WebCore, then we're not going to be able to get
rid of WebCore/platform, and then we'll have two different platform
abstraction libraries. That does not seem desirable. Moreover, the
criterion to determine whether a class should go in PAL or platform
would then be whether the stuff needs to be accessed from WebKit layers
or not. That doesn't seem useful.

So I think we should drop that goal. (Still really excited for PAL, of
course.)

On Wed, 2017-01-11 at 23:30 +, Olmstead, Don wrote:
> Whatever the consensus is we’re looking forward to working on getting
> the PAL layer up and running. We’re working on rebooting our port so
> we’re in a good position to help build it out and do any refactoring
> to help create a clear layering. Having a clear porting layer,
> especially one with tests, is something we’re hoping will be
> beneficial to all the ports.

This is exciting to hear too!

Michael
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Maciej Stachowiak]

To be clear, I think PAL is a great move. Details like the namespace, placement 
of the code, and how includes look are all things that can be changed after the 
fact.

That said, I think we need to ultimately consider our porting layer to be the 
combination of PAL and WTF, so it is good for the two to be consistent. I hope 
revisions along these lines can be considered in due course.

Regards,
Maciej

> On Jan 11, 2017, at 3:30 PM, Olmstead, Don  wrote:
> 
> I was the one who did the WebCore::PAL namespace so I wanted to chime in on 
> why I went that route. We at Sony are newcomers to pushing to trunk so my 
> explanation might be entirely too idealistic but here goes.
>  
> I had thought of PAL as a library that is internal to WebCore that provides a 
> clear porting layer. I would not expect anyone outside of WebCore to be 
> linking to it. Because of that it was living inside Source/WebCore, and since 
> it was setup that way having an internal namespace of WebCore::PAL made sense 
> conceptually. Also in the future if PAL was successful I could see a WebKit2 
> equivalent.
>  
> Whatever the consensus is we’re looking forward to working on getting the PAL 
> layer up and running. We’re working on rebooting our port so we’re in a good 
> position to help build it out and do any refactoring to help create a clear 
> layering. Having a clear porting layer, especially one with tests, is 
> something we’re hoping will be beneficial to all the ports.
>  
> From: webkit-dev-boun...@lists.webkit.org 
> [mailto:webkit-dev-boun...@lists.webkit.org] On Behalf Of Maciej Stachowiak
> Sent: Wednesday, January 11, 2017 2:05 PM
> To: Antti Koivisto 
> Cc: Webkit Development List ; 
> mrobin...@igalia.com
> Subject: Re: [webkit-dev] WebCore/platform standalone library
>  
>  
> These both sound right to me. 
>  
> More generally, I would expect that over time, PAL would likely become a peer 
> project to WebCore instead of being inside it, much the same way WTF started 
> inside JavaScriptCore and eventually moved outside it in the source tree. In 
> the WTF case, it always had a separate top-level namespace.
>  
> On Jan 11, 2017, at 12:27 PM, Antti Koivisto  > wrote:
>  
> Why is the PAL namespace inside the WebCore namespace? Couldn't it just be a 
> top-level namespace (even if it currently happens to live in the WebCore 
> project)?
>  
> #include  would be more consistent with existing headers than 
> .
>  
>  
>   antti
>  
> On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield  > wrote:
> After 18 months of no progress, Don Olmstead and I are getting the band back 
> together!
>  
> We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 
>  which incorporates feedback 
> from many different stakeholders (and as such, the direction is a little 
> different than where I was going with this in the beginning).
>  
> First of all, this isn’t a new project; instead, it’s a new target inside the 
> WebCore project. The target creates a static library which gets linked into 
> WebCore, which means that the enforcement mechanism can’t be done by the 
> linker. Instead, the layering will be enforced by a Python script, triggered 
> as an extra build step, which checks the symbol names inside the .a file as 
> well as #include directives in source code.
>  
> We opted for WebCore to include files using “#include ” instead of 
> just including Foo.h. Similarly, we are putting symbols inside the PAL 
> namespace, which is a child of the WebCore namespace. Therefore, inside 
> WebCore, you use PAL things by specifying “PAL::Foo”.
>  
> The first thing to move into PAL is the “crypto” subfolder, which is a good 
> candidate because it’s small, simple, yet also has platform-dependent 
> implementations.
>  
> We would love your feedback on this approach to help make the dream a reality!
>  
> Thanks,
> Myles and Don
>  
> On Mar 22, 2015, at 4:40 PM, Gavin Barraclough  > wrote:
>  
> On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak  > wrote:
>  
> Web Abstraction Toolbox (it’s hard to tell the difference between wat and WTF 
> sometimes…)
>  
> +1
>  
>  
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
>  
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
>  
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-

Re: [webkit-dev] WebCore/platform standalone library

[Olmstead, Don]

I was the one who did the WebCore::PAL namespace so I wanted to chime in on why 
I went that route. We at Sony are newcomers to pushing to trunk so my 
explanation might be entirely too idealistic but here goes.

I had thought of PAL as a library that is internal to WebCore that provides a 
clear porting layer. I would not expect anyone outside of WebCore to be linking 
to it. Because of that it was living inside Source/WebCore, and since it was 
setup that way having an internal namespace of WebCore::PAL made sense 
conceptually. Also in the future if PAL was successful I could see a WebKit2 
equivalent.

Whatever the consensus is we’re looking forward to working on getting the PAL 
layer up and running. We’re working on rebooting our port so we’re in a good 
position to help build it out and do any refactoring to help create a clear 
layering. Having a clear porting layer, especially one with tests, is something 
we’re hoping will be beneficial to all the ports.

From: webkit-dev-boun...@lists.webkit.org 
[mailto:webkit-dev-boun...@lists.webkit.org] On Behalf Of Maciej Stachowiak
Sent: Wednesday, January 11, 2017 2:05 PM
To: Antti Koivisto 
Cc: Webkit Development List ; mrobin...@igalia.com
Subject: Re: [webkit-dev] WebCore/platform standalone library


These both sound right to me.

More generally, I would expect that over time, PAL would likely become a peer 
project to WebCore instead of being inside it, much the same way WTF started 
inside JavaScriptCore and eventually moved outside it in the source tree. In 
the WTF case, it always had a separate top-level namespace.

On Jan 11, 2017, at 12:27 PM, Antti Koivisto 
mailto:koivi...@iki.fi>> wrote:

Why is the PAL namespace inside the WebCore namespace? Couldn't it just be a 
top-level namespace (even if it currently happens to live in the WebCore 
project)?

#include  would be more consistent with existing headers than 
.


  antti

On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield 
mailto:mmaxfi...@apple.com>> wrote:
After 18 months of no progress, Don Olmstead and I are getting the band back 
together!

We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 which 
incorporates feedback from many different stakeholders (and as such, the 
direction is a little different than where I was going with this in the 
beginning).

First of all, this isn’t a new project; instead, it’s a new target inside the 
WebCore project. The target creates a static library which gets linked into 
WebCore, which means that the enforcement mechanism can’t be done by the 
linker. Instead, the layering will be enforced by a Python script, triggered as 
an extra build step, which checks the symbol names inside the .a file as well 
as #include directives in source code.

We opted for WebCore to include files using “#include ” instead of 
just including Foo.h. Similarly, we are putting symbols inside the PAL 
namespace, which is a child of the WebCore namespace. Therefore, inside 
WebCore, you use PAL things by specifying “PAL::Foo”.

The first thing to move into PAL is the “crypto” subfolder, which is a good 
candidate because it’s small, simple, yet also has platform-dependent 
implementations.

We would love your feedback on this approach to help make the dream a reality!

Thanks,
Myles and Don

On Mar 22, 2015, at 4:40 PM, Gavin Barraclough 
mailto:barraclo...@apple.com>> wrote:

On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak 
mailto:m...@apple.com>> wrote:

Web Abstraction Toolbox (it’s hard to tell the difference between wat and WTF 
sometimes…)

+1


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Michael Catanzaro]

Really happy to see progress on this!

On Tue, 2017-01-10 at 21:24 -0800, Myles C. Maxfield wrote:
> First of all, this isn’t a new project; instead, it’s a new target
> inside the WebCore project. The target creates a static library which
> gets linked into WebCore, which means that the enforcement mechanism
> can’t be done by the linker. Instead, the layering will be enforced
> by a Python script, triggered as an extra build step, which checks
> the symbol names inside the .a file as well as #include directives in
> source code.

My preference remains to put PAL inside the toplevel Source/ directory
and make it a proper dependency of WebCore, because it seems far
superior to have the linker enforce layering. (A secondary concern is
that it will be nicer to say PAL::Foo rather than WebCore::PAL::Foo
when using PAL from the WebKit and WebKit2 layers.)

I appreciate that the decision to make it a target rather than a
dependency was driven by a desire to simplify the build system, and I
also appreciate that CMake and XCode are both complex beasts. But I
don't think this is the right approach. Consider that there is very
little material advantage to WebCore/PAL over the existing
WebCore/platform: we already have Tools/Scripts/check-for-platform-
layering-violations to check for platform layering violations, so the
only real advantage to adding PAL would be that PAL doesn't currently
have any such layering violations and so the layering enforcement
script could be run by the style checker. If that were the only goal,
then it would be simpler to keep WebCore/platform and just fix the
layering violations than to slowly move everything to a new directory.
I'd much rather let the linker handle layering enforcement; then there
is real value to splitting out PAL.

Michael
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Michael Catanzaro]

On Tue, 2017-01-10 at 21:28 -0800, Darin Adler wrote:
> Given that WTF chose , what is the reasoning for PAL
> choosing the all capital ?

I kinda prefer  myself, in order to parallel WTF. On the one
hand, it almost doesn't matter, but on the other it's one more rule to
have to memorize which includes must be lowercase and which must be
uppercase.

Michael
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Maciej Stachowiak]

These both sound right to me. 

More generally, I would expect that over time, PAL would likely become a peer 
project to WebCore instead of being inside it, much the same way WTF started 
inside JavaScriptCore and eventually moved outside it in the source tree. In 
the WTF case, it always had a separate top-level namespace.

> On Jan 11, 2017, at 12:27 PM, Antti Koivisto  wrote:
> 
> Why is the PAL namespace inside the WebCore namespace? Couldn't it just be a 
> top-level namespace (even if it currently happens to live in the WebCore 
> project)?
> 
> #include  would be more consistent with existing headers than 
> .
> 
> 
>   antti
> 
> On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield  > wrote:
> After 18 months of no progress, Don Olmstead and I are getting the band back 
> together!
> 
> We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 
>  which incorporates feedback 
> from many different stakeholders (and as such, the direction is a little 
> different than where I was going with this in the beginning).
> 
> First of all, this isn’t a new project; instead, it’s a new target inside the 
> WebCore project. The target creates a static library which gets linked into 
> WebCore, which means that the enforcement mechanism can’t be done by the 
> linker. Instead, the layering will be enforced by a Python script, triggered 
> as an extra build step, which checks the symbol names inside the .a file as 
> well as #include directives in source code.
> 
> We opted for WebCore to include files using “#include ” instead of 
> just including Foo.h. Similarly, we are putting symbols inside the PAL 
> namespace, which is a child of the WebCore namespace. Therefore, inside 
> WebCore, you use PAL things by specifying “PAL::Foo”.
> 
> The first thing to move into PAL is the “crypto” subfolder, which is a good 
> candidate because it’s small, simple, yet also has platform-dependent 
> implementations.
> 
> We would love your feedback on this approach to help make the dream a reality!
> 
> Thanks,
> Myles and Don
> 
>> On Mar 22, 2015, at 4:40 PM, Gavin Barraclough > > wrote:
>> 
>>> On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak >> > wrote:
>>> 
>>> Web Abstraction Toolbox (it’s hard to tell the difference between wat and 
>>> WTF sometimes…)
>> 
>> +1
>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> 
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev 
> 
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Michael Catanzaro]

Perhaps we need to be especially careful about replacing Refs and
RefPtrs with auto. It was mentioned that using typedefs to hide pointer
types is often the source of serious bugs. A similar rule could apply
to using auto with smart pointers.

On Wed, 2017-01-11 at 09:43 -0800, Darin Adler wrote:
> > On Jan 11, 2017, at 9:41 AM, Alexey Proskuryakov 
> > wrote:
> > 
> > In a way, these are read-time assertions.
> 
> Exactly. A type name is a read-time assertion of the specific type
> that a variable has and “auto” is a read-time assertion that the type
> of the variable is the same as the type of the expression on the
> right.
> 
> — Darin

That is a useful observation. Consider this simple scenario:

int i = returnsSomeIntType();
auto i = returnsSomeIntType();

Sometimes you want a variable that is the same type as the expression
on the right, and you are worried about possible bugs if the type of
the expression on the right changes. In my experience, this is the
typical case. Then auto is probably most appropriate. For instance, if
the function returns an int but in the future is changed to return an
int64_t, then you will be very happy that you chose to use auto, as it
avoids a truncation bug that would have been extremely difficult to
discover through code inspection. We face exactly this problem, for
example, in bug #165790, where the goal is to change the type of a
function from unsigned to size_t, but the function is named "length" so
it's virtually impossible to grep for call sites to update. (On the
other hand, you really always have to check call sites in such a
situation, because we have not always used auto.)

But sometimes the type of the expression on the right is not so
important. For instance, I reviewed a patch recently where I discovered
a very subtle integer underflow bug. It was not caused by auto, so it's
not a perfect example, but in this case it was important to ensure
that, regardless of the type returned from the function, it needed to
be assigned to a variable of type int64_t to avoid underflow later on.
In this case, auto was an inferior option, as the only way to safely
use auto would have been this:

auto i = static_cast(returnsSomeIntType());

which is surely inferior to just not using auto:

int64_t i = returnsSomeIntType();

Michael

P.S.

Not very related: the underflow in question was of the form

int64 result = a - b()

where a was a size_t, and b() returned a size_t, and we wanted to
receive a negative result if b() was greater than a. This is such an
easy mistake to make and very difficult to spot. :/ It's worth a little
paranoia to consider that an attacker might try to intentionally insert
such a vulnerability.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Antti Koivisto]

Why is the PAL namespace inside the WebCore namespace? Couldn't it just be
a top-level namespace (even if it currently happens to live in the WebCore
project)?

#include  would be more consistent with existing headers than
.


  antti

On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield 
wrote:

> After 18 months of no progress, Don Olmstead and I are getting the band
> back together!
>
> We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 which
> incorporates feedback from many different stakeholders (and as such, the
> direction is a little different than where I was going with this in the
> beginning).
>
> First of all, this isn’t a new project; instead, it’s a new target inside
> the WebCore project. The target creates a static library which gets linked
> into WebCore, which means that the enforcement mechanism can’t be done by
> the linker. Instead, the layering will be enforced by a Python script,
> triggered as an extra build step, which checks the symbol names inside the
> .a file as well as #include directives in source code.
>
> We opted for WebCore to include files using “#include ” instead
> of just including Foo.h. Similarly, we are putting symbols inside the PAL
> namespace, which is a child of the WebCore namespace. Therefore, inside
> WebCore, you use PAL things by specifying “PAL::Foo”.
>
> The first thing to move into PAL is the “crypto” subfolder, which is a
> good candidate because it’s small, simple, yet also has platform-dependent
> implementations.
>
> We would love your feedback on this approach to help make the dream a
> reality!
>
> Thanks,
> Myles and Don
>
> On Mar 22, 2015, at 4:40 PM, Gavin Barraclough 
> wrote:
>
> On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak  wrote:
>
> Web Abstraction Toolbox (it’s hard to tell the difference between wat and
> WTF sometimes…)
>
>
> +1
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[JF Bastien]

Would it be helpful to focus on small well-defined cases where auto makes
sense, and progressively grow that list as we see fit?


e.g. I think this is great:

auto ptr = std::make_unique(bar);

Proposed rule: if the type is obvious because it's on the line, then auto
is good.
Similarly:

auto i = static_cast(j);

auto foo = make_foo();

auto bar = something.get_bar(); // Sometimes, "bar" is obvious.



Range-based loops are a bit tricky. IMO containers with "simple" types are
good candidates for either:

for (const auto& v : cont) { /* don't change v */ }
for auto& v : cont) { /* change v */ }

But what's "simple"? I'd say all numeric, pointer, and string types at
least. It gets tricky for more complex types, and I'd often rather have the
type in the loop. Here's more discussion on this
,
including a recommendation to use auto&& on range-based loops! I think this
gets confusing, and I'm not a huge fan of r-value references everywhere.


Here's another I like, which Yusuke pointed out a while ago (in ES6
Module's implementation?):

struct Foo {
  typedef Something Bar;
  // ...
  Bar doIt();
};
auto Foo::doIt() -> Bar
{
  // ...
}

Why? Because Bar is scoped to Foo! It looks odd the first time, but I think
this is idiomatic "modern" C++.


I also like creating unnamed types, though I know this isn't everyone's
liking:

auto ohMy()
{
  struct { int a; float b; } result;
  // ...
  return result;
}
void yeah()
{
  auto myMy = ohMy();
  dataLogLn(myMy.a, myMy.b);
}

I initially had that with consumeLoad
,
which returns a T as well as a ConsumeDependency. I couldn't care less
about the container for T and ConsumeDependency, I just want these two
values.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Konstantin Tokarev]

11.01.2017, 21:55, "Geoffrey Garen" :
> I’m open to auto, but skeptical.
>
> (1) Research-ability. I read a lot of code that is new to me, that I have 
> never written. I find type statements to be useful as documentation for where 
> to look for more information about how data structures and algorithms relate 
> to each other. Traversing a tree or list of data structures by type name 
> while reading code is a common task for me. But I can’t select and search for 
> auto, auto*, or auto&.
>
> (2) Mechanics. It’s totally true that if you loop over one collection and put 
> the values into another collection, subject to some filter, the types 
> involved aren’t really interesting to reviewing the filter. But if you want 
> to review low-level mechanics like object lifetime and thread safety, 
> suddenly the types are really important. I think the “I changed RefPtr to 
> auto and then crashed” examples in this thread are about mechanics.
>
> I find it strange that, as JavaScript authors adopt tools like TypeScript 
> because they find type-less programming to be unmaintainable, C++ authors 
> move toward type-less programming. Perhaps the grass is always greener on the 
> other side. Or perhaps we can strike a balance and put types where they help 
> us.
>
> Two arguments made in favor of auto don’t fully convince me:
>
> (1) You were always able to hide the types of certain expressions.
>
> I’m hoping the point of this argument is not to say “in for a penny, in for a 
> pound”, but rather to raise our awareness that we’ve been OK with certain 
> type-less expressions all along. I agree: I'm OK with hiding types in some 
> places, especially if surrounding code already provides some kind of type 
> foothold.

Please don't confuse dynamic typing and static typing with type deduction.

>
> At the same time, I sometimes break expressions into statements to improve 
> clarity and research-ability or to achieve certain mechanics.
>
> Maybe this answers Darin’s question:
>
>>  Here’s one thing to consider: Why is it important to see the type of 
>> foundSource->value, but not important to see the type of 
>> shifts.take(source)? In both cases they need to be Vector.
>
> In the cited code, ‘shifts’ is declared as HashMap>, 
> and HashMap ’take' is well understood to return its value type, so no further 
> comment about type is necessary for me on this line.
>
> Following this logic, I might be OK with eliding the type of 
> foundSource->value. But we’re starting to reach a limit. Each time we do this 
> we create an auto link in the type chain. If I have to trace back three autos 
> deep in order to understand a type, I’m gonna have a bad time. Each type 
> declaration is a stopping point for research. Therefore, I don’t prefer the 
> fully auto version of this code.
>
> Following this logic, I would be OK with const auto& a little later in this 
> algorithm:
>
>>  bool isRotate = false;
>>  for (const ShufflePair& pair : currentPairs) {
>>  if (pair.dst() == originalSrc) {
>>  isRotate = true;
>>  break;
>>  }
>>  }
>
> Might as well use const auto& for ‘pair’: The type of currentPairs is 
> well-documented locally. If currentPairs were a data member, and not declared 
> in the same screenful of code, I would want to write out a type for ‘pair’. 
> Otherwise, I would have to open another file to understand this line.
>
> But somebody said we don’t like const auto&? Is this a gotcha somehow?
>
> (2) You can’t be sure of the stated type, since a type conversion may have 
> happened.
>
> Knowing that a value is equivalent to a certain type still provides a 
> foothold for research and understanding mechanics.
>
> We only allow automatic type conversion when we consider types to be 
> reasonably equivalent. We consider it bad style to convert willy-nilly, and 
> we use the explicit keyword to avoid errant conversions. For example, we 
> don’t allow RefPtr to automatically convert to raw pointer.
>
> Geoff
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

-- 
Regards,
Konstantin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Geoffrey Garen]

I’m open to auto, but skeptical.

(1) Research-ability. I read a lot of code that is new to me, that I have never 
written. I find type statements to be useful as documentation for where to look 
for more information about how data structures and algorithms relate to each 
other. Traversing a tree or list of data structures by type name while reading 
code is a common task for me. But I can’t select and search for auto, auto*, or 
auto&.

(2) Mechanics. It’s totally true that if you loop over one collection and put 
the values into another collection, subject to some filter, the types involved 
aren’t really interesting to reviewing the filter. But if you want to review 
low-level mechanics like object lifetime and thread safety, suddenly the types 
are really important. I think the “I changed RefPtr to auto and then crashed” 
examples in this thread are about mechanics.

I find it strange that, as JavaScript authors adopt tools like TypeScript 
because they find type-less programming to be unmaintainable, C++ authors move 
toward type-less programming. Perhaps the grass is always greener on the other 
side. Or perhaps we can strike a balance and put types where they help us.

Two arguments made in favor of auto don’t fully convince me:

(1) You were always able to hide the types of certain expressions. 

I’m hoping the point of this argument is not to say “in for a penny, in for a 
pound”, but rather to raise our awareness that we’ve been OK with certain 
type-less expressions all along. I agree: I'm OK with hiding types in some 
places, especially if surrounding code already provides some kind of type 
foothold.

At the same time, I sometimes break expressions into statements to improve 
clarity and research-ability or to achieve certain mechanics.

Maybe this answers Darin’s question:

> Here’s one thing to consider: Why is it important to see the type of 
> foundSource->value, but not important to see the type of shifts.take(source)? 
> In both cases they need to be Vector.

In the cited code, ‘shifts’ is declared as HashMap>, 
and HashMap ’take' is well understood to return its value type, so no further 
comment about type is necessary for me on this line.

Following this logic, I might be OK with eliding the type of 
foundSource->value. But we’re starting to reach a limit. Each time we do this 
we create an auto link in the type chain. If I have to trace back three autos 
deep in order to understand a type, I’m gonna have a bad time. Each type 
declaration is a stopping point for research. Therefore, I don’t prefer the 
fully auto version of this code.

Following this logic, I would be OK with const auto& a little later in this 
algorithm:

> bool isRotate = false;
> for (const ShufflePair& pair : currentPairs) {
> if (pair.dst() == originalSrc) {
> isRotate = true;
> break;
> }
> }

Might as well use const auto& for ‘pair’: The type of currentPairs is 
well-documented locally. If currentPairs were a data member, and not declared 
in the same screenful of code, I would want to write out a type for ‘pair’. 
Otherwise, I would have to open another file to understand this line.

But somebody said we don’t like const auto&? Is this a gotcha somehow?

(2) You can’t be sure of the stated type, since a type conversion may have 
happened.

Knowing that a value is equivalent to a certain type still provides a foothold 
for research and understanding mechanics.

We only allow automatic type conversion when we consider types to be reasonably 
equivalent. We consider it bad style to convert willy-nilly, and we use the 
explicit keyword to avoid errant conversions. For example, we don’t allow 
RefPtr to automatically convert to raw pointer.

Geoff
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Myles C. Maxfield]

On Jan 10, 2017, at 9:28 PM, Darin Adler  wrote:

>> On Jan 10, 2017, at 9:24 PM, Myles C. Maxfield  wrote:
>> 
>> We opted for WebCore to include files using “#include ” instead 
>> of just including Foo.h.
> 
> Will this work without ForwardingHeaders on macOS and iOS?

I think this requires more investigation to answer. 

> 
> Given that WTF chose , what is the reasoning for PAL choosing the 
> all capital ?

Beauty. Do you think this was a bad call?

> 
>> Similarly, we are putting symbols inside the PAL namespace, which is a child 
>> of the WebCore namespace. Therefore, inside WebCore, you use PAL things by 
>> specifying “PAL::Foo”.
> 
> Given that WebCore is built on top of PAL, it seems a little strange to put 
> PAL “inside” WebCore, but I guess OK.

Yeah, this was driven by the decision to make it a target rather than a 
project. Putting a target of a project outside the project itself seems worse 
than putting a dependency inside the project.

> 
> — Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCore/platform standalone library

[Myles C. Maxfield]

> On Jan 10, 2017, at 11:08 PM, Carlos Garcia Campos  
> wrote:
> 
>> El mar, 10-01-2017 a las 21:24 -0800, Myles C. Maxfield escribió:
>> After 18 months of no progress, Don Olmstead and I are getting the
>> band back together!
> 
> Great news. 
> 
>> We’ve uploaded a patch
>> to https://bugs.webkit.org/show_bug.cgi?id=143358 which incorporates
>> feedback from many different stakeholders (and as such, the direction
>> is a little different than where I was going with this in the
>> beginning).
>> 
>> First of all, this isn’t a new project; instead, it’s a new target
>> inside the WebCore project. The target creates a static library which
>> gets linked into WebCore, which means that the enforcement mechanism
>> can’t be done by the linker. Instead, the layering will be enforced
>> by a Python script, triggered as an extra build step, which checks
>> the symbol names inside the .a file as well as #include directives in
>> source code.
> 
> So, will it be possible to link to PAL but not the rest of WebCore?

Yes. This is the testing strategy. A goal is to be able to unit-test PAL 
without a layout test.

> 
>> We opted for WebCore to include files using “#include ”
>> instead of just including Foo.h. Similarly, we are putting symbols
>> inside the PAL namespace, which is a child of the WebCore namespace.
>> Therefore, inside WebCore, you use PAL things by specifying
>> “PAL::Foo”.
> 
> And WebCore::PAL::Foo when used outside WebCore namespace, right?

✅

> 
>> The first thing to move into PAL is the “crypto” subfolder, which is
>> a good candidate because it’s small, simple, yet also has platform-
>> dependent implementations.
>> 
>> We would love your feedback on this approach to help make the dream a
>> reality!
> 
> I'm not sure I agree with the approach because I don't know the reasons
> to make it a target inside WebCore, could you elaborate more on that,
> please?

Build systems are already very complicated; we want fewer projects, not more of 
them. Also there's so complexity with ForwardingHeaders, which I'd like to 
avoid. We can achieve our objectives in the most simple way possible; no 
project is necessary. If, one day, we want PAL to be promoted to a project, we 
can do that then.

> 
>> Thanks,
>> Myles and Don
>> 
>>> On Mar 22, 2015, at 4:40 PM, Gavin Barraclough >> om> wrote:
>>> 
 On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak 
 wrote:
 
 Web Abstraction Toolbox (it’s hard to tell the difference between
 wat and WTF sometimes…)
>>> 
>>> +1
>>> 
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Filip Pizlo]

I'm only arguing for why using auto would be bad in the code snippet that we 
were talking about.

My views regarding auto in other code are not strong.  I only object to using 
auto when it is dropping useful information.

-Filip



> On Jan 11, 2017, at 9:15 AM, Darin Adler  wrote:
> 
> OK, you didn’t convince me but I can see that your opinions here are strongly 
> held!
> 
> — Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Darin Adler]

> On Jan 11, 2017, at 9:41 AM, Alexey Proskuryakov  wrote:
> 
> In a way, these are read-time assertions.

Exactly. A type name is a read-time assertion of the specific type that a 
variable has and “auto” is a read-time assertion that the type of the variable 
is the same as the type of the expression on the right.

— Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Alexey Proskuryakov]

There are two considerations which make me skeptical that auto is a good thing.

1. There are many smart pointer types in C++, and ignoring pointer types is 
very error prone. Others have mentioned std::optional, and mistakes being made 
with RefPtrs. I even saw a case where a review comment that suggested switching 
to auto, but it would have introduced memory corruption if followed.

Some specific cases of this may become irrelevant as PassRefPtr goes away, but 
I don't think that there is any expectation that smart pointers in general are 
going away.

2. I also find that types in code are an important part of documenting how it 
is supposed to work. In a way, these are read-time assertions. An assertion can 
be wrong and they are compiled out in release builds, yet they prove to be 
highly valuable anyway. Similarly, a type can be wrong, but it tells me as the 
reader what the author thinks their code is doing, and lets me focus on other 
parts of it for the first pass at least.

A very similar kind of type agnostic coding has always been used in templates, 
and it feels like a well established belief that it takes engineers with high 
levels of expertise to write generic code in templates. And if it's harder, I 
don't see why using these techniques all over the place is beneficial.

- Alexey


> 11 янв. 2017 г., в 9:15, Darin Adler  написал(а):
> 
> OK, you didn’t convince me but I can see that your opinions here are strongly 
> held!
> 
> — Darin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Darin Adler]

OK, you didn’t convince me but I can see that your opinions here are strongly 
held!

— Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Filip Pizlo]

On Jan 10, 2017, at 23:49, Darin Adler  wrote:

>> On Jan 10, 2017, at 10:17 PM, Filip Pizlo  wrote:
>> 
>>while (Arg src = worklist.pop()) {
>>HashMap>::iterator iter = 
>> mapping.find(src);
>>if (iter == mapping.end()) {
>>// With a shift it's possible that we previously built 
>> the tail of this shift.
>>// See if that's the case now.
>>if (verbose)
>>dataLog("Trying to append shift at ", src, "\n");
>>currentPairs.appendVector(shifts.take(src));
>>continue;
>>}
>>Vector pairs = WTFMove(iter->value);
>>mapping.remove(iter);
>> 
>>for (const ShufflePair& pair : pairs) {
>>currentPairs.append(pair);
>>ASSERT(pair.src() == src);
>>worklist.push(pair.dst());
>>}
>>}
> 
> Here is the version I would write in my favored coding style:
> 
>while (auto source = workList.pop()) {
>auto foundSource = mapping.find(source);
>if (foundSource == mapping.end()) {
>// With a shift it's possible that we previously built the tail of 
> this shift.
>// See if that's the case now.
>if (verbose)
>dataLog("Trying to append shift at ", source, "\n");
>currentPairs.appendVector(shifts.take(source));
>continue;
>}
>auto pairs = WTFMove(foundSource->value);
>mapping.remove(foundSource);
>for (auto& pair : pairs) {
>currentPairs.append(pair);
>ASSERT(pair.source() == source);
>workList.push(pair.destination());
>}
>}
> 
> You argued that specifying the type for both source and for the iterator 
> helps reassure you that the types match. I am not convinced that is an 
> important interesting property unless the code has types that can be 
> converted, but with conversions that we must be careful not to do. If there 
> was some type that could be converted to Arg or that Arg could be converted 
> to that was a dangerous possibility, then I grant you that, although I still 
> prefer my version despite that.

It would take me longer to understand your version. The fact that the algorithm 
is working on Args is the first thing I'd want to know when reading this code, 
and with your version I'd have to spend some time to figure that out. It 
actually quite tricky to infer that it's working with Args, so this would be a 
time-consuming puzzle. 

So, if the code was changed in this manner then I'd want it changed back 
because this version would make my job harder. 

I get that you don't need or want the type to understand this code. But I want 
the type to understand this code because knowing that it works with Args makes 
all the difference for me. 

I also get that conversions are possible and so the static assertion provided 
by a type annotation is not as smart as it would be in some other languages. 
But this is irrelevant to me. I would want to know that source is an Arg and 
pair is a ShufflePair regardless of whether this information was checked by the 
compiler. In this case, putting the information in a type means that it is 
partly checked. You could get it wrong and still have compiling code but that's 
unlikely. And for what it's worth, my style is to prefer explicit constructors 
unless I have a really good reason for implicit precisely because I like to 
limit the amount of implicit conversions that are possible. I don't think you 
can implicitly convert Arg to anything. 

> 
> You also said that it’s important to know that the type of foundSource->value 
> matches the type of pairs. I would argue that’s even clearer in my favored 
> style, because we know that auto guarantees that are using the same type for 
> both, although we don’t state explicitly what that type is.

I agree with you that anytime you use auto, it's possible to unambiguously 
infer what the type would have been if you think about it.

I want to reduce the amount of time I spend reading code because I read a lot 
of it. It would take me longer to read the version with auto because knowing 
the types is a huge hint about the code's intent and in your version I would 
have to pause and think about it to figure out the types.

> 
> Here’s one thing to consider: Why is it important to see the type of 
> foundSource->value, but not important to see the type of shifts.take(source)? 
> In both cases they need to be Vector.

Your version does not annotate the type at all. I would have to guess that the 
pair is a ShufflePair and that pairs is a Vector of ShufflePairs. I agree that 
probably anyone working on WebKit would be smart enough to solve this puzzle. I 
don't want my job to involve solving more puzzles than it already does. 

The point of saying the type explicit