Re: [webkit-dev] maybe_unused vs UNUSED_PARAM

[Darin Adler via webkit-dev]

I support using standardized and widely enough available language features 
directly instead of through macros whenever we can. It’s similar to when we 
drop our own classes and use the ones from the C++ standard, which I think has 
been good for the project.

It’s also fine with me to use the style checker script to catch mistakes and 
help educate contributors about this once we’ve decided.

Not sure others agree with this, but I’d even support using global replace to 
move from old style to new style.

One nice thing about language features is we don’t have to be so careful about 
not using them in headers. We don’t want to pull in lots of our own macros into 
headers that are used outside the project.

It’s great for the long term health of the project to cut down the number of 
unique special things you need to know to understand and work with the code.

I have two thoughts about possible reasons for caution as we do these 
transitions:

Macros can be a flexible solution to cross-platform or cross-language issues. 
It would be a shame to move to a new language feature and discover only 
afterward that we as a project want to continue to support at least one 
compiler or platform that doesn’t have a good implementation yet, or support 
using some header from C, or that there was something else we can work around 
with macros. This would make me want to do some testing first on each of these 
before taking the plunge.

We should be open to the possibility that some of our macros may still be 
better solutions to a problem than directly using the new language feature. For 
example, it is possible that ASSERT_UNUSED is slightly better for its purpose 
than [[maybe_unused]] since it has a more specific purpose. Marking the 
argument [[maybe_unused]] when we specifically know it’s only used for 
assertions isn’t perfect. Of course, ASSERT_UNUSED doesn’t quite do what we’d 
want either, but it’s still more specific than just saying “maybe”. The unused 
argument warning macros aren’t superb right now. It’s almost always better to 
leave out argument names instead of using them, because then there is no 
“maybe” about it. Unfortunately, the unused argument warning is mostly not 
turned on outside JavaScriptCore and WebCore. Also, it’s easy to have stale 
“unused” markings on things that are actually always used, so we leave behind 
macros that are both no longer needed and subtly misleading. I’m pretty sure 
[[maybe_unused]] is nearly identical in its properties to UNUSED_PARAM, so none 
of this really seems like an argument against that. And I’m not sure 
ASSERT_UNUSED is actually good enough to keep, despite these considerations.

I agree with moving in the direction of using these.

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

Re: [webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

[Darin Adler via webkit-dev]

> On Nov 28, 2023, at 3:02 PM, Chris Dumez wrote:
> 
> FYI, my understanding is that the person gets a *green* checkmark when the 
> person is present in contributors.json (common case), even if not marked as a 
> reviewer in that file.

Does anyone know why we chose green for all contributors rather than green for 
reviewers?

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

Re: [webkit-dev] Smart Pointers Usage Guidelines

[Darin Adler via webkit-dev]

> On Aug 21, 2023, at 4:39 PM, Ryosuke Niwa  wrote:
> 
> Alternatively, we could add a new member function which returns CheckedPtr 
> like `pageChecked()`.

Yes, I think that would be a good approach that would complement the static 
checker.

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

Re: [webkit-dev] Smart Pointers Usage Guidelines

[Darin Adler via webkit-dev]

> On Aug 21, 2023, at 4:31 PM, Tim Horton via webkit-dev 
>  wrote:
> 
>> One subtle thing is that even when a member variable is already Ref / RefPtr 
>> / CheckedRef / CheckedPtr, we must create another one in stack as seen here:
>> https://commits.webkit.org/267108@main
> 
> (I asked rniwa to send this mail because this patch surprised me, so I hope 
> now we can chat about it).
> 
> The scope of this rule, and the … lack of elegance … at so many callsites 
> worries me a bit. If it’s possible to automate enforcement, that might help 
> with part of the problem, but it’s also just really not very pretty, and I 
> wonder if someone can come up with some clever alternative solution before we 
> go too far down this path (not me!).
> 
> Alternatively, it’s possible other people OK with this syntax/requirement and 
> I should just get over it. What do you all think?

I hope we can make this better by using a getter function that returns a 
CheckedPtr so instead of writing CheckedPtr { _page }, we would write page().

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

Re: [webkit-dev] jsc bus error EXC_BAD_ACCESS with jsc-only build on Mac

[Darin Adler via webkit-dev]

> On Mar 28, 2023, at 4:57 PM, Laurence Rowe via webkit-dev 
>  wrote:
> 
> I was hoping to try and wrap the jsc-only build into a rust crate since the 
> existing rust wrappers either include gtk dependencies or only wrap the lower 
> level api and link against the system libraries.

What’s the benefit here? How would this be better than linking against the 
JavaScriptCore framework?

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

Re: [webkit-dev] Smart Pointer Analysis Tool for WebKit

[Darin Adler via webkit-dev]

I am hoping this ends up being practical. In the CSS code I have recently 
worked on there are many short-lived references, and in some refactoring I was 
not able to change them all to smart pointers without a measurable performance 
degradation in Speedometer, so the code might need redesign to use different 
idioms.

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

Re: [webkit-dev] Compile times and class-scoped enums

[Darin Adler via webkit-dev]

> On Jan 20, 2023, at 9:50 AM, Jer Noble via webkit-dev 
>  wrote:
> 
> I attempted to address this in , 
> which (on this machine) reduces the total compile time of Document.h in the 
> WebCore project from about 1000s to about 200s.

That’s good.

> However, this change requires moving class-scoped enums out into the 
> namespace scope.

Seems worthwhile. Doesn’t seem to me like it would have far reaching effects.

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

Re: [webkit-dev] Stop Using Raw Pointers & References in New Code

[Darin Adler via webkit-dev]

I don’t think I can notice these patterns. I would be able to program this way 
if I had an analysis tool, but otherwise this simply seems too complicated. I 
can’t see the types of intermediate values to know if they are CheckedPtr or 
RefPtr or raw pointers or whatever. A variation on the first line in 
Element::clientLeft is a good example:

document()->updateLayoutIgnorePendingStylesheets();

That looks perfectly good to me, nothing makes me think “the result of document 
is an unsafe reference to a heap-allocated object and only trivial functions 
can be called on that”; I should not need to be an expert on each function to 
be able to tell if code is correct or not. How can we choose a programming 
style where something simple like that is subtly wrong and dangerous and 
requires reviewers to look out for it?

I understand that if properly supported by a tool we could adapt to this and 
write code a whole new way. Moving to this style without a tool would almost 
literally require me to stop working on WebKit because I couldn’t correctly 
write or review even a short function.

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

Re: [webkit-dev] Stop Using Raw Pointers & References in New Code

[Darin Adler via webkit-dev]

> On Jan 12, 2023, at 9:35 PM, Ryosuke Niwa  wrote:
> 
> One alternative is to make bar() return RefPtr although that would be a bit 
> heavy handed in the case of trivial function calls like this: 
> document().frame()->ownerElement()

I don’t quite follow. You just said that all arguments including this need to 
have RefPtr or something like it. What makes it OK to call ownerElement without 
a RefPtr? What is a “trivial function” and how can we tell which functions are 
the trivial ones?

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

Re: [webkit-dev] Stop Using Raw Pointers & References in New Code

[Darin Adler via webkit-dev]

> On Jan 12, 2023, at 3:35 PM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
>> On Jan 12, 2023, at 6:13 AM, Darin Adler  wrote:
>> 
>>> On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev 
>>>  wrote:
>>> 
>>> assuming every local variable / variable in stack is stored in a smart 
>>> pointer, function arguments are safe to be raw pointers / references via 
>>> transitive property
>> 
>> What about the case where the function argument is the return value from 
>> another function?
> 
> In those cases, the value should be stored in a local variable using a smart 
> pointer first.
> 
> So… instead of:
> foo(bar());
> 
> do:
> foo(RefPtr { bar() }.get());

This seems impractical. How will I remember to do this?

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

Re: [webkit-dev] Stop Using Raw Pointers & References in New Code

[Darin Adler via webkit-dev]

> On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> assuming every local variable / variable in stack is stored in a smart 
> pointer, function arguments are safe to be raw pointers / references via 
> transitive property

What about the case where the function argument is the return value from 
another function?

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

Re: [webkit-dev] GPU Process on all platforms eventually?

[Darin Adler via webkit-dev]

> On Nov 14, 2022, at 4:45 PM, Fujii Hironori via webkit-dev 
>  wrote:
> 
> When will we remove WebKit1?

Apple can’t remove WebKit1 from iOS or macOS while keeping old apps working, so 
we won’t be able to remove it any time soon as long as Apple is shipping new 
versions of WebKit. To cite one example, many older iOS apps use it for their 
advertising and I don’t think they would be OK if Apple made their ad revenue 
go to zero.

We could look for some creative way to make it easier to keep it working, 
perhaps.

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

Re: [webkit-dev] Nicks in contributors.json

[Darin Adler via webkit-dev]

> On Oct 24, 2022, at 1:57 PM, Alexey Proskuryakov via webkit-dev 
>  wrote:
> 
> 1. Finding people on Slack. For this, they would probably need to stay on 
> https://webkit.org/team.

If they were consistently the Slack nicknames. But they often aren’t! The 
nicknames currently in there don’t necessarily match the Slack ones and 
obviously don’t match the GitHub account names. I wish these were more explicit 
about what they are! I had no idea that they were the ones still used in 
bugs.webkit.org. I would love to see the list of what name each person used on 
what service, email, bugs.webkit.org , Slack, and 
GitHub, if there was some economical way of doing it.

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

Re: [webkit-dev] Remove the version labels in GitHub

[Darin Adler via webkit-dev]

> On Oct 22, 2022, at 12:11 PM, Caitlin Potter  wrote:
> 
> Would it make sense to, rather than referring to downstream browsers, have a 
> canonical WebKit version number that could be exposed in about:version or 
> something? A friendly semver-ish thing + short git hash could be a lot more 
> useful for doing things like bisecting.
> 
> I think part of the reason the version numbers havent been useful is they are 
> very detached from the actual open source product, and don't necessarily help 
> with triage or reproduction, or hint at a cause.

I personally am not looking to add something. I’d rather remove the thing we 
currently have that is not useful. I am definitely open to adding something 
later if we come up with something great.

It’s not clear how easy it would be to improve things by adding more than the 
git hashes and the commit.webkit.org increasing numbers we already have. On 
Apple’s platforms there are already both  Safari version numbers and WebKit 
version numbers and it’s not clear where an additional WebKit open source 
reference version number would go, and how to best express the effects of 
branching.

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

[webkit-dev] Remove the version labels in GitHub

[Darin Adler via webkit-dev]

Hi folks.

I understand it can be super critical to understand what version someone was 
testing when reporting a WebKit bug. But I, at least, haven’t found the version 
field at bugs.webkit.org useful in the 20+ years I have been working on this 
project.

Now we have that same noise in GitHub. All my pull requests have the label 
WebKit Nightly Build on them when I use the “git webkit pr” command to create 
them. And we have lots of labels with various version numbers, such as “Safari 
8” and “417.x".

Can we just get rid of all this? Is there anyone who’s finding these labels 
useful in GitHub?

I suggest we remove these labels from the project. No objections to leaving the 
version field in bugs.webkit.org for the time being, but eventually I think we 
should probably get rid of that too.

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

[webkit-dev] Gardening our pull requests

[Darin Adler via webkit-dev]

Hi folks.

I love to keep the WebKit project moving along, and one way that I personally 
like to contribute is helping get pull requests reviewed. I looked at the list 
of pull requests for the project, excluding closed requests, draft requests, 
and requests that have been reviewed:

https://github.com/WebKit/WebKit/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse+review%3Anone+sort%3Aupdated-asc

There are 113 of those right now.

It seems like we should convert many of these to drafts, close some others, and 
review most of the rest. I think it would make the project healthier if we 
didn’t have so many pull requests in an ambiguous state, including some that 
were last updated 4 months ago.

What do you all think? Do you own some of these? Would you be willing to 
convert ones that represent work in progress to drafts to clean up this list? 
Other ideas for making this work well?

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

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Darin Adler via webkit-dev]

> On Jun 6, 2022, at 1:09 PM, Olmstead, Don  wrote:
> 
> If it isn’t it should be considering how many times I’ve had a cq- come from 
> an AppleWin build that is in no way affected by my patch.

Yes, we have a problem with that AppleWin bot, and it’s one that bothers me 
quite a bit, but I don’t want to design our policy around that problem.

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

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Darin Adler via webkit-dev]

Yes, I don’t oppose adding another EWS bot, and I was not trying to argue 
against that proposal. I did intend to express my disagreement with many points 
made in follow-up replies that seem quite wrong to me.

Three other thoughts:

1) Even though I don’t object to adding a new bot, I will say that the idea 
that a single “non-unified” bot will add a lot of value at very little cost to 
the WebKit project doesn’t sound right to me. These arguments about how long 
things will take don’t seem correct. My experience is that it’s quite difficult 
to find, understand, and resolve errors seen in bot builds, far more difficult 
than resolving errors I can see locally as I make code changes. In my view 
every EWS bot we add helps weigh down the project, making each change more 
difficult.

2) In my contributions, I don’t just want to add missing includes, I want to 
remove unnecessary ones taking full advantage of forward declarations and 
moving code out of headers. Too many includes and too much dependency has a 
dramatic bad effect on the project, making colossal project build times even 
worse.

3) We had many, many problems with platform-specific include differences before 
we had unified builds, with frustrated contributors if they worked with any 
configuration that lacked an EWS bot. It may seem that the 
unified-build-specific problems are something fundamentally different, but this 
is not how I see it.

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

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Darin Adler via webkit-dev]

Here’s my view:

Long ago we agreed that we’ll ask WebKit contributors to keep builds working 
that have EWS bots, and not other configurations. As far as I can tell, nothing 
has changed that invalidates that strategy and we should stick with it.

I do not agree that the statement that “all projects must build under all 
supported configurations” applies to WebKit. We don’t even have a concept of 
“supported configurations” to build that policy on. This has not been a project 
goal in the past and I suggest we do not add this project goal.

We should continue to use the Early Warning System to define which 
configurations must be kept working by all contributors, with anything beyond 
being treated as a stretch goal.

And we should continue to accept patches to fix various configurations that 
people want to keep working that are not checked by EWS. But we absolutely 
should not ask contributors to keep all possible combinations working.

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

Re: [webkit-dev] WebKit Style: Whitespace for Objective-C protocols

[Darin Adler via webkit-dev]

I personally prefer id, and would be happy to standardize on 
that. I don’t really care that much about statistics about usage in our 
existing code.

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

Re: [webkit-dev] Great function for Cocoa platform, bridge_cast

[Darin Adler via webkit-dev]

I think of it as following the same naming pattern as downcast<> or 
static_cast<>, but you don’t have to specify a template argument since it 
infers it for you.

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

Re: [webkit-dev] Great function for Cocoa platform, bridge_cast

[Darin Adler via webkit-dev]

Just economy. There is no need for two different names. I personally like it 
this way, and have found it appealing when I used it. I think you should give 
it a try. We can certainly change the name later if this turns out to 
significantly improve things.

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

Re: [webkit-dev] Great function for Cocoa platform, bridge_cast

[Darin Adler via webkit-dev]

Oh, forgot to say, it’s in this header:

 #include 

Using functions like this one will help us get over the “bridge” to ARC on this 
project eventually.

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

[webkit-dev] Great function for Cocoa platform, bridge_cast

[Darin Adler via webkit-dev]

Hi folks.

For those of you doing work with Objective-C on Cocoa platforms, I want to draw 
your attention to a great new idiom. Back in October, David Kilzer added 
bridge_cast, a type-safe set of functions that convert an Objective-C pointer 
to a Core Foundation pointer or vice versa, preserving types across the 
toll-free bridging boundary. It’s better than traditional non-WTF idioms where 
you use casts that look like “(__bridge id)” because you don’t have to write 
the type, and the correct corresponding type is chosen for you.

When you have a CFStringRef and need to use it as an NSString key and value in 
an Objective-C dictionary, for example, the idiom would be:

bridge_cast(constantKey): bridge_cast(constantValue),

Rather than the old:

(__bridge id)constantKey: (__bridge id)constantValue,

It converts to NSString *, not id, which is better for many contexts, and good 
here, too. Since the function works in both directions, it will also turn an 
NSDictionary into a CFDictionaryRef. And it works on both raw pointers and on 
RetainPtr. I find it’s even more welcome to have something that can convert a 
RetainPtr into RetainPtr without danger of 
getting the reference counting wrong, doing the right thing in both ARC and 
non-ARC source files, and optimizing the move cases appropriately.

Please consider this instead of writing things like (CFStringRef)keyNS.get() 
because it’s easier to see it’s correct.

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

Re: [webkit-dev] OK to flatten WTF's header directory?

[Darin Adler via webkit-dev]

> On Feb 3, 2022, at 5:16 PM, Elliott Williams  wrote:
> 
> If what you’ve experienced is unique to WTF, I’d be suspicious of the 
> non-standard approach we’ve taken to copying WTF’s headers I mentioned above.

Great news. I hope this plan works out.

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

Re: [webkit-dev] OK to flatten WTF's header directory?

[Darin Adler via webkit-dev]

Long ago, I originally created the forwarding headers to bridge the gap between 
framework-style includes that those of us at Apple wanted to do, where headers 
are flattened and you write #include , and 
Unix-style installed libraries, where things are not flattened. I wanted us to 
be able to write include statements in WebKit in the traditional 
non-Apple-framework style so they would work on those other platforms, and the 
forwarding headers were originally just for the Apple platforms, making those 
includes work with the framework structure provided automatically by the Apple 
framework build system.

Those original forwarding headers were not copies of the headers, they were 
simply files “in the right place” with include statements in them. I’m not sure 
at what point along the way we started making copies of headers, and we may be 
solving other problems with that.

This proposal, to flatten the WTF headers, seems to be a step in of the 
opposite direction, making the non-Apple platforms do something akin to 
framework-style including. Not sure I understand the pros and cons are of that.

I am currently suffering when working on WebKit development because of the 
copies of headers made by the build system. I can’t count the number of times 
I’ve edited a WTF header and then later realized I had edited the copy, not the 
original. I would very much like a solution that resolved that problem. I use 
Xcode features, and it seems the copies are what Xcode thinks are the “real” 
headers.

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

Re: [webkit-dev] Proposal to change nested namespace indentation rule to match the existing code

[Darin Adler via webkit-dev]

> On Oct 22, 2021, at 12:33 AM, Kimmo Kinnunen via webkit-dev 
>  wrote:
> 
> 1) Match what seems to already be de-facto used in code.

Yes, does seem to be what we’re already doing. I like it. The rest of your 
arguments also seem good.

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

Re: [webkit-dev] Moving from WTF::Optional to std::optional

[Darin Adler via webkit-dev]

Thanks for pointing that out, Chris.

This advice goes beyond std::optional, by the way. For anything that we move 
from, there are two operations at are intended to be safe, from a C++ language 
and library design point of view: destroying the object and overwriting it by 
assigning a new value. If our code relies on doing anything else after the 
object is moved from, like examining the value after the old value is moved 
out, please use std::exchange to set the new value while moving the old value 
out. This even applies to large-seeming objects like HashMap, which I will note 
is not large: in release builds a HashMap is implemented as a single pointer to 
a structure on the heap and a new empty HashMap is a null pointer.

— Darin

Sent from my iPad

> On Jun 1, 2021, at 10:01 PM, Chris Dumez  wrote:
> 
> Hi,
> 
> Another thing Darin didn’t mention but I think people should be careful about:
> The move constructor for std::optional does not clear the is-set flag (while 
> the one for WTF::Optional did).
> 
> As a result, you will be having a very bad time if you do a use-after-move of 
> a std::optional. Please make sure to use std::exchange() instead of WTFMove() 
> if you want to leave to std::optional in a clean state for reuse later.
> 
> Chris Dumez
> 
>>> On Jun 1, 2021, at 8:54 PM, Darin Adler via webkit-dev 
>>>  wrote:
>>> 
>> Hi folks.
>> 
>> We’re getting rid of the WTF::Optional class template, because, hooray, 
>> std::optional is supported quite well by all the C++17 compilers we use, and 
>> we don’t have to keep using our own special version. Generally we don’t want 
>> to reimplement the C++ standard library when there is not a significant 
>> benefit, and this is one of those times.
>> 
>> Here are a few considerations:
>> 
>> 1) Since https://commits.webkit.org/238290@main, if you use Optional<> by 
>> mistake instead of std::optional<>, your code won’t compile. (Unless you are 
>> writing code for ANGLE, which has its own separate Optional<>.)
>> 
>> 2) If you want to use std::optional, include the C++ standard header, 
>> , or something that includes it. In a lot of cases, adding an 
>> include will not be required since it’s included by widely-used headers like 
>> WTFString.h and Vector.h, so if you include one of those are covered. 
>> Another way to think about this is that if your base class already uses 
>> std::optional, then you don’t need to include it.
>> 
>> 3) Once the patch in https://bugs.webkit.org/show_bug.cgi?id=226437 lands, 
>> includes of  won’t forward declare optional, and includes of 
>>  won’t do anything at all.
>> 
>> — 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

[webkit-dev] Moving from WTF::Optional to std::optional

[Darin Adler via webkit-dev]

Hi folks.

We’re getting rid of the WTF::Optional class template, because, hooray, 
std::optional is supported quite well by all the C++17 compilers we use, and we 
don’t have to keep using our own special version. Generally we don’t want to 
reimplement the C++ standard library when there is not a significant benefit, 
and this is one of those times.

Here are a few considerations:

1) Since https://commits.webkit.org/238290@main, if you use Optional<> by 
mistake instead of std::optional<>, your code won’t compile. (Unless you are 
writing code for ANGLE, which has its own separate Optional<>.)

2) If you want to use std::optional, include the C++ standard header, 
, or something that includes it. In a lot of cases, adding an include 
will not be required since it’s included by widely-used headers like 
WTFString.h and Vector.h, so if you include one of those are covered. Another 
way to think about this is that if your base class already uses std::optional, 
then you don’t need to include it.

3) Once the patch in https://bugs.webkit.org/show_bug.cgi?id=226437 lands, 
includes of  won’t forward declare optional, and includes of 
 won’t do anything at all.

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

Re: [webkit-dev] Let's use -Werror on EWS?

[Darin Adler via webkit-dev]

Sorry, I should clarify.

Apple’s ports of WebKit already use -Werror and always have, everywhere, not 
just on EWS. Since day one.

I do not know why we do not already use -Werror on GTK and WPE and I support 
using it there after fixing all the warnings.

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

Re: [webkit-dev] Let's use -Werror on EWS?

[Darin Adler via webkit-dev]

I’m a big fan of -Werror. Back when WebKit started, it was controversial to use 
it at Apple.

But we can’t just turn on -Werror until after we fix all the warnings! Who will 
do that project.

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

Re: [webkit-dev] New EWS Non-Unified builder

[Darin Adler via webkit-dev]

OK. I acknowledge my view on this is different from the others commenting here, 
perhaps because of different ideas about what is hard and requires expertise. I 
won’t stand in the way of changes you want to make. You know my view now.

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

Re: [webkit-dev] New EWS Non-Unified builder

[Darin Adler via webkit-dev]

> On Apr 29, 2021, at 9:06 PM, Tim Horton via webkit-dev 
>  wrote:
> 
> it is definitely highly annoying

It’s possible that where my thinking differs from others on this is that I 
don’t think shifting annoying work from one set of commits (the ones adding a 
new file) to a different set (the ones unknowingly adding need for a new 
include for non-unified builds) is a significant improvement. Adding more EWS 
bubbles has a cost.

Keep in mind that I am just as passionate as the next person about getting 
includes “right”. I’m just not sure that this would be a step in the right 
direction.

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

Re: [webkit-dev] New EWS Non-Unified builder

[Darin Adler via webkit-dev]

> On Apr 29, 2021, at 9:01 PM, Tim Horton  wrote:
> 
> This is a huge problem for people on the Apple platforms using the Xcode 
> projects. Nearly every time someone adds a file they have to add random 
> headers to unrelated code.

OK, sorry, I must be out of touch with the rest of you about how difficult this 
is. I’ve done that many times, and it was always easy for me.

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

Re: [webkit-dev] New EWS Non-Unified builder

[Darin Adler via webkit-dev]

> On Apr 29, 2021, at 12:04 PM, Ross Kirsling via webkit-dev 
>  wrote:
> 
> Yeah, I think it's important to clarify that nobody is "using 
> non-Unified-Source building for their development", at least to my knowledge. 
> Being broken by the shifting sands of unified sources is an everybody problem 
> (or at the very least an "everybody that builds via CMake problem", which is 
> ultimately an everybody problem).

How is this a bigger problem in CMake than for people on the Apple platforms 
who are using Xcode projects? Can we create greater parity between the two?

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

Re: [webkit-dev] New EWS Non-Unified builder

[Darin Adler via webkit-dev]

Given the issue is that there are people that are using non-Unified-Source 
building for their development, the best fix is to add post-commit and EWS 
builders for one of those platforms. I do not support the idea of adding an 
additional builder just to “keep non-Unified-Source building” if no 
actively-supported platform is not choosing that build style.

Specifically, if Sony people are most affected by this, I suggest we find a way 
to add Sony PlayStation post-commit and EWS builders.

I am not convinced that we should add some kind of abstract “correct 
compilation” that is a separate builder.

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

Re: [webkit-dev] Mailman web interface missing

[Darin Adler via webkit-dev]

> On Apr 26, 2021, at 1:03 PM, Adrian Perez de Castro via webkit-dev 
>  wrote:
> 
> - https://lists.webkit.org/listinfo/webkit-wpe/ results in “Not Found”
> - https://lists.webkit.org/admindb/webkit-wpe/ results in “403 Forbidden”

I took a look and it seems that the URLs have simply changed, perhaps as a 
result of an update:

https://lists.webkit.org/mailman/listinfo/webkit-wpe
https://lists.webkit.org/mailman/admin/webkit-wpe 


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

Re: [webkit-dev] GitHub Account and Commit Attribution

[Darin Adler via webkit-dev]

> On Dec 17, 2020, at 8:47 AM, Jonathan Bedard via webkit-dev 
>  wrote:
> 
> Something we’ve just learned about commit attribution and GitHub is that 
> adding an email to your GitHub account may not attribute commits that were 
> pushed to a repository before you added the email. There were a few issues 
> with history as it stands now, and I will be pushing up the final version of 
> history in the next few days.
> 
> What this means for you now is that it is time to set-up your GitHub account 
> if you have not already and add any emails you used to commit to WebKit to 
> your account (GitHub allows multiple emails to be associated with a single 
> account).

Do GitHub’s privacy settings matter? My GitHub account now has da...@apple.com 
associated with it, but I have "Keep my email addresses private” checked in 
GitHub settings. Does that matter?

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

Re: [webkit-dev] Making WTF::StringImpl and WTF::AtomString thread-safe

[Darin Adler via webkit-dev]

> On Dec 9, 2020, at 1:02 PM, Geoff Garen via webkit-dev 
>  wrote:
> 
>> - Make FontCache thread-safe, but do it via introducing a completely
>> separate thread-safe AtomString type and leave the current one as it is
>> (I don't have a good grasp of how difficult this would actually be)
> 
> I had to chuckle at this point because the obvious name for this new 
> thread-safe AtomString class would be AtomicString, the prior name of 
> AtomString.

If we make a separate thread-safe code path, I’m not sure we need to create a 
variant of AtomString at all.

AtomString optimizes string comparisons (by paying up front every time you 
construct one with a hash table lookup) and memory use (by sharing the same 
memory for all equal strings), but there’s no reason we can’t compare two 
strings without using AtomString. I’m doubting that once we figure out what 
we’re trying to do that we’ll need AtomString.

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

Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

[Darin Adler]

> On Oct 20, 2020, at 4:08 PM, Sam Weinig  wrote:
> 
> For the ones conditionalizing adding to ADDITIONAL_BINDING_IDLS, those IDLs 
> should just have the appropriate Conditional=* extended attribute added, then 
> they can be included unconditionally in the makefile.
> 
> For the ones conditionalizing adding go USER_AGENT_STYLE_SHEETS, again, there 
> doesn’t seem a real reason to keep that. The sheets are all preprocessed 
> anyway, so we can just move those #ifdefs into the files.
> 
> I filed https://bugs.webkit.org/show_bug.cgi?id=218001 
>  for this and will get that 
> done regardless.

Some of these also make it possible to build without WebKitAdditions. We may 
need some different approach to that.

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

Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

[Darin Adler]

> On Oct 20, 2020, at 3:09 PM, Sam Weinig  wrote:
> 
> For the Platform.h issue, I think we could probably engineer things in the 
> short term to have a script phase that produces a Defines.txt that the other 
> script phases and build rules depend on.

We definitely can, and it’s simpler than how we do it now. I have a patch that 
does this for the makefile partly done and set aside. My approach was to revise 
each script, one at a time, to take a file with the defines rather than 
requiring they be passed on the command line. It doesn’t work for the parts of 
the makefile itself that depend directly on checking the defines (grep for 
findstring.*FEATURE_AND_PLATFORM_DEFINES to see those).

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

Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

[Darin Adler]

It would be wonderful to drastically cut down the repeated mentions of the same 
filenames. People not on Xcode platforms probably focus primarily on Xcode 
projects for this frustration. But I find it ridiculous how many places I have 
to add things. For example, to add AbstractRange I had to add filenames to:

Shared

- Sources.txt: Two source files and one IDL-generated file.

CMake:

- CMakeLists.txt: IDL file.
- Headers.cmake: One source file and one IDL-generated file.

Xcode:

- DerivedSources.make: IDL file
- WebCore.xcodeproj/project.pbxproj: IDL file, two source files, and Two 
IDL-generated files.

And also, updating these checked-in generated files (that we’d need regardless 
of build system, due to Apple build requirements):

- DerivedSources-input.xcfilelist
- DerivedSources-output.xcfilelist

It seems like we should have a goal that you should not have to list 
IDL-generated file names. That will be hard to accomplish in Xcode, though. 
Maybe the reason we haven’t been so ambitious about getting this right for 
CMake is that it’s so unreachable a goal for Xcode, given that we want the 
files to show up in a clean way in the IDE?

And it would be good to find a way to share the lists of IDL files like we do 
the list of source files. That seems relevant to this discussion of changing 
DerivedSources.make.

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

Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

[Darin Adler]

Oops, I was in the middle of writing this and hit send by mistake
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Reducing / removing use of Makefile based DerivedSources.make

[Darin Adler]

Please note that this is a Cocoa-only, Xcode-only conversation.

As Sam pointed out, there’s a whole different “will you please switch to CMake” 
conversation. That’s not an easy switch to make for programmers working on the 
Apple platforms, but we might do it at some point if we can work out the issues 
Sam mentioned.

> On Oct 17, 2020, at 10:00 AM, Sam Weinig  wrote:
> 
> While I can’t recall the initial reasons for using Makefiles for derived 
> sources, over the years there have been a number of advantages to it that I 
> do know of. One clear advantage, that is no longer applicable, was code 
> sharing, as earlier in the project, at least the Apple Windows port did 
> utilize these Makefiles.

Pretty sure I was the one who did this. And I do recall the initial reasons. 
There were only two.

1) Code sharing

I believed at the time that the rules for building derived sources were tricky 
and hard to get right, and proposed that we would use the 
lowest-common-denominator build system, make, across all platforms. This 
strategy didn’t work because right from the beginning people working with 
various build systems used copies of the derived sources rules that I wrote 
rather than invoking make, and never tried sharing code.

I think this was a missed opportunity to share complicated code, but it 
happened a very long time ago, and I made my peace with it back then.

2) Xcode build system deficiencies

At that time, Xcode’s build system had too many problems for these kinds of 
complex rules. Many years later, that may no longer be true.

Since we’re not even trying to do the code sharing, I think it’s fine to stop 
using makefiles now.

If we’re deeply committed to switching to CMake at some point in future, one 
way to take a step in that direction would be to return to the original code 
sharing goal, and use CMake for derived sources, so we have code sharing. I 
don’t know how modular CMake is and how practical it is to use it for part, but 
not all, of the build process with Xcode. People using CMake for everything 
might be frustrated with the constraints this could place on how CMake is used.

I think an important third goal that I did not have at the start, but have now, 
is the one Ryosuke mentioned:

3) Minimize the number of places files are listed, so it’s easy to add, remove, 
and rename files of various types.

It would be wonderful to drastically cut down the repeated mentions of the same 
filenames. People not on Xcode platforms probably focus primarily on Xcode 
projects for this frustration. But I find it ridiculous how many places I have 
to add things. For example, to add AbstractRange I had to add filenames to:

Shared:

Sources.txt

CMake:

CMakeLists.txt
Headers.cmake

Xcode:

WebCore.xcodeproj/project.pbxproj

And also, the checked-in generated files (that we’d need regardless of build 
systems, due to Apple build requirements):

DerivedSources-input.xcfilelist
DerivedSources-output.xcfilelist

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

Re: [webkit-dev] Trailing space changes in "expected.txt" files

[Darin Adler]

> On Sep 26, 2020, at 8:13 PM, Darin Adler  wrote:
> 
> by rebasing and either regenerating the expected.txt files hand editing.

… resolve the conflicts by rebasing and either regenerating the expected.txt 
files with run-webkit-tests or by hand editing the expected.txt files.

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

[webkit-dev] Trailing space changes in "expected.txt" files

[Darin Adler]

Hi folks.

This afternoon, I changed dumpAsText so it strips trailing spaces.

https://trac.webkit.org/changeset/267640

As part of that change, run-webkit-tests temporarily will ignore trailing 
spaces in expected.txt files, so they can be checked in with, or without, the 
spaces. Over the next few days, I’ll be checking in new versions of 
expected.txt files with the spaces at the ends of lines stripped. Once we have 
all the spaces stripped, then run-webkit-tests will go back to being strict 
about this again.

This affects you if you have a patch that changes an expected.txt file; the 
space changes might lead to patch conflicts. If you do encounter conflicts, you 
can resolve them by rebasing and either regenerating the expected.txt files 
hand editing. Any newly-regenerated files won’t have any trailing spaces, and 
it’s best if newly checked in files do not have trailing spaces.

If you run into any trouble with this, please get in touch.

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

Re: [webkit-dev] Smart Pointer Analysis Tool for WebKit

[Darin Adler]

> On Sep 23, 2020, at 12:18 PM, Ryosuke Niwa  wrote:
> 
> There are quite a few cases where data members are references but then those 
> can also be replaced by a simple member function which retrieves the value of 
> the smart pointer member variable and returns a reference.

I think this should be an explicit recommendation in the project of refactoring 
to follow these rules.

> For now, a trivial function is defined as a member function defined in the 
> class declaration whose definition simply returns a member variable (the 
> result of get() or a copy if the member variable is a smart pointer).

That seems like a rule that’s too narrow. I would not want a function to become 
non-trivial just because I moved it from being inline within the class 
definition to an inline below the class definition in the same header.

This rule worries me a lot right now; it seems like it could result in an 
explosion of local variable copies of arguments.

> We probably also need to figure out a way to exempt all lambda functions that 
> never get stored anywhere. We have a bunch of helper functions like WTF::map 
> which just calls lambdas on each item while iterating over an array, etc... 
> and there is no need to create a separate Ref / RefPtr in those cases since 
> lambdas are never stored and re-used later.

Does seem important. I am pretty sure I have seen this concept in other 
languages. We often try to use const Function& for one type of lambda argument 
and Function&& for the other type, but that’s far from complete.

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

Re: [webkit-dev] Smart Pointer Analysis Tool for WebKit

[Darin Adler]

> On Sep 23, 2020, at 10:32 AM, Darin Adler  wrote:
> 
>> On Sep 16, 2020, at 11:32 PM, Ryosuke Niwa > <mailto:rn...@webkit.org>> wrote:
>> Every data member to a ref counted object must use either Ref, RefPtr, or 
>> WeakPtr. webkit.NoUncountedMemberChecker 
>> <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checkers.rst#id17>My
>>  only worry here is performance. Do we know yet if we can afford it?
> 
> The worst case here is Ref, which is much worse than a reference since we end 
> up having to use -> instead of . everywhere and you can’t see that there is 
> no null involved.

I don’t mean performance worst case. I meant “outside of performance concerns, 
the worst downgrade of our programming idioms is Ref”.
>> Every ref counted base class, if it has a derived class, must define a 
>> virtual destructor. webkit.RefCntblBaseVirtualDtor 
>> <https://github.com/llvm/llvm-project/blob/master/clang/docs/analyzer/checkers.rst#id16>The
>>  style system has an optimization that intentionally violates this for 
>> performance reasons (StyleRuleBase).

So are we saying we must not do that optimization, or will we have a way to 
suppress the warning for this?

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

Re: [webkit-dev] Smart Pointer Analysis Tool for WebKit

[Darin Adler]

> On Sep 16, 2020, at 11:32 PM, Ryosuke Niwa  wrote:
> Every data member to a ref counted object must use either Ref, RefPtr, or 
> WeakPtr. webkit.NoUncountedMemberChecker 
> My
>  only worry here is performance. Do we know yet if we can afford it?

The worst case here is Ref, which is much worse than a reference since we end 
up having to use -> instead of . everywhere and you can’t see that there is no 
null involved.
> Every ref counted base class, if it has a derived class, must define a 
> virtual destructor. webkit.RefCntblBaseVirtualDtor 
> The
>  style system has an optimization that intentionally violates this for 
> performance reasons (StyleRuleBase).
> Every ref counted object passed to a non-trivial function as an argument 
> (including "this" pointer) should be stored as a Ref or RefPtr in the 
> caller’s local scope unless it's an argument to the caller itself by 
> transitive property [1]. alpha.webkit.UncountedCallArgsChecker 
> What
>  is a non-trivial function?
> Every ref counted object must be captured using Ref or RefPtr for lambda. 
> webkit.UncountedLambdaCapturesChecker 
> Ref,
>  RefPtr, or WeakPtr, right?

Same concern about Ref vs references.
> Local variables - we’re still working on this 
> (https://reviews.llvm.org/D83259 )
I am looking forward to learning more about the proposal here.

Same concern about Ref vs. references.

I really want to see before/after for some non-trivial source files with 
significant problems; where will this drive the most change and what will 
things look like after?

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

[webkit-dev] Could use some help from people who care about the WinCairo port

[Darin Adler]

The patch for this bug

https://bugs.webkit.org/show_bug.cgi?id=216428 


is failing to build on WinCairo because of some kind of precompiled header 
problem. I could use some help figuring out what I did wrong.

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

Re: [webkit-dev] PSA: Bit fields won't be packed on Windows if you mix types

[Darin Adler]

> On Sep 3, 2020, at 11:14 PM, Fujii Hironori  wrote:
> 
> BTW, I don't like to idea adding a new rule, but keeping old style code. It 
> introduces divergence between the guideline and actual code. Do we really 
> need a new rule that one doesn’t necessary have to follow?

I understand that you don’t like this.

But all of our style rules are like this. We don’t apply them across our entire 
project at the moment we institute them. They guide the future of WebKit. So 
this is not a valid argument against adding a new rule.

You can help us apply a given rule to existing code if you feel strongly in a 
particular case, and in some cases it might be practical to quickly apply it 
everywhere relevant. Maybe in this case it is.

But that’s not the basis for deciding whether to make it a rule.

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

Re: [webkit-dev] Feedback on Blink's text fragment directive proposal

[Darin Adler]

> On Aug 31, 2020, at 9:33 AM, David Bokan  wrote:
> 
> We've made lots of improvements to the spec, notably around issues raised in 
> Mozilla's standards-position 
> .

Do you think those improvements address Maciej’s concerns from last December? 
Since you didn’t say that explicitly I was wondering what your take on that was.

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

Re: [webkit-dev] Plugin process

[Darin Adler]

> On Jul 20, 2020, at 7:35 AM, Michael Catanzaro  wrote:
> 
> Then we can make the plugin process specific to PLATFORM(COCOA)

Side note: It will be specific to PLATFORM(MAC).

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

> On Jul 16, 2020, at 3:33 PM, Adrian Perez de Castro  wrote:
> 
> enabling/disabling features changes the set of source files to
> build, which results in different source files being #included from each
> unifier source compilation unit.

Let’s stop doing it that way. Just compile the files and have #if inside the 
file.

I removed all conditional source files from the Xcode build system. Let's do 
the same for the CMake build system.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

> On Jul 16, 2020, at 2:28 PM, Adrian Perez de Castro  wrote:
> 
> It is not feasible to test unified builds for all the possible combinations of
> target architectures and set of features enabled at build time (there is a
> combinatorial explosion of configurations). Keeping non-unified builds in a
> working state guarantees that regular unified builds will keep working no
> matter what the build configuration is.

But it doesn’t.

Non-unified builds don’t make it possible to have arbitrary sets of features 
enabled at build time. We break that all the time and it’s only fixed when 
someone tests that arbitrary set of features. Headers are only a small part of 
that issue.

I’d go as far as saying it’s not a project goal to always build with arbitrary 
sets of features enabled.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

> On Jul 16, 2020, at 12:54 PM, Kirsling, Ross  wrote:
> 
> Non-unified build fixes are done to support *all* platforms, because sudden 
> unification shifts can happen anywhere.

I’m not sure that benefit is worth the additional code churn. I understand that 
it’s frustrating to run into a problem when unification shifts, say when you 
are adding a source file. I believe we have made changes to unification since 
the earliest version that reduce how often unification shifts.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

> On Jul 14, 2020, at 2:38 PM, Simon Fraser  wrote:
> 
> Could someone educate me about ? When should I use this 
> instead of individual wtf headers?

Forward.h is analogous to forward-declaring a class ('class IntPoint;' instead 
of ‘#include “IntPoint.h”'), but it works for many often-used classes and class 
templates in the WTF namespace, including class templates that would be 
difficult to correctly forward-declare due to their many arguments, such as 
WTF::Vector. And it includes “using WTF::String” and the like, as well, to 
import WTF namespace things into the global namespace.

We can use it any time we need a forward-declaration, not an entire definition, 
of one of the items. For example, to compile a header that just takes and 
returns String objects, we only need a forward declaration of String. The 
easiest way to correctly do that is to include . Including 
 pulls in a lot more. For the specific case of String, I think 
you might be able to go further and write this instead:

namespace WTF {
class String;
}
using WTF::String;

But I have never tried it, and there might be some problem with that.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

We need to be cautious about adding header includes to other headers. Adding 
includes to .cpp files is likely fine and not a deeply consequential decision, 
but adding to headers is something that can have a massive effect on build 
times over time.

> On Jul 13, 2020, at 10:44 PM, hironori.fu...@sony.com wrote:
>  <>Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
> (264331 => 264332)
> 
> --- trunk/Source/WebCore/platform/graphics/ColorSerialization.h   
> 2020-07-14 05:17:20 UTC (rev 264331)
> +++ trunk/Source/WebCore/platform/graphics/ColorSerialization.h   
> 2020-07-14 05:44:25 UTC (rev 264332)
> @@ -25,6 +25,8 @@
>  
>  #pragma once
>  
> +#include 
This change is wrong. The header to include here is Forward.h, not WTFString.h. 
Not super-harmful since WTFString.h is already so widely included, but we need 
to be really cautious in headers.

> Modified: trunk/Source/WebCore/svg/SVGParserUtilities.h (264331 => 264332)
> 
> --- trunk/Source/WebCore/svg/SVGParserUtilities.h 2020-07-14 05:17:20 UTC 
> (rev 264331)
> +++ trunk/Source/WebCore/svg/SVGParserUtilities.h 2020-07-14 05:44:25 UTC 
> (rev 264332)
> @@ -24,6 +24,7 @@
>  #include "ParsingUtilities.h"
>  #include 
>  #include 
> +#include 
Same mistake.

I’d really like to come up with some other system for reviewing adding headers 
to *headers*.

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

Re: [webkit-dev] Platform.h vs. makefiles

[Darin Adler]

> On May 10, 2020, at 10:07 PM, Maciej Stachowiak  wrote:
> 
> I think the best way to configure WebKit is to use a separate data file, 
> neither a header nor a makefile or similar, that defines the options in a 
> single place with clear syntax. Then everything else is generated from that. 
> Such a system could cover runtime flags as well, which are even more 
> scattered around the project than compile-time flags.

Sounds OK. I worry a little about defining yet another language, but otherwise 
I do find this appealing. Perhaps some people would say that 
FeatureDefines.props from cmake could be that file?

Not sure literally a single data file is the best way to do this across 
multiple platforms/ports. I think PlatformEnableCocoa.h shows us that breaking 
this down can be helpful.

One file that has the master list of all the settings, and all the default 
values. Then other files that contain overlays for each port/configuration 
where they are different from the default.

My worry is that it could become complicated, like our TestExpectations files, 
which were once simple.

> Moving logic from build files to the header is probably a move in the right 
> direction, though of course it carries risk, particularly for less tested 
> configurations.

Yes, I’m not suggesting rushing to do it all at once in a mass change.

But for new things especially on Apple’s Cocoa platforms, I’d like to avoid 
FeatureDefines.xcconfig and see new things in the PlatformEnableCocoa.h header 
file instead. Unless the defines need to affect the build system and not just 
the C++ code, I think the header file is superior.

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

[webkit-dev] Platform.h vs. makefiles

[Darin Adler]

Hi folks.

The Platform.h configuration file family has been useful for WebKit for a long 
time. We created it to try to organize configuration options in WebKit so the 
would not be spread out through the whole project.

One way to look at these, particularly the ENABLE options, is as a set of 
configuration options that let each consumer of the WebKit source code create a 
unique flavor of WebKit with the particular features they want enabled turned 
on and others turned off. Another is to think of this as a mechanism for 
keeping decisions made by the WebKit contributors organized and easy to 
understand.

If these truly are WebKit consumer options, then it’s important they be set as 
part of the build process. The macros can be defined with a build and 
configuration system outside WebKit, and the Platform.h headers should 
interpret those values correctly.

On the other hand, if we are just trying to keep our decisions straight, then 
it would be best if the logic for what is on and what is off by in the header 
files, written with preprocessor macro logic, and not spread across multiple 
make files and scripts.

Thus I think the pattern on macOS, for example, of setting these in .xcconfig 
files doesn’t make a lot of sense. I think the .xcconfig files should compute 
the things that need to be determined about the build environment, but the 
configuration decisions should be in files like PlatformHaveCocoa.h, for 
example.

I think the guideline should be like this:

All code to compute configuration should be in the Platform.h files, not in 
makefiles, with only the following exceptions:

1) Options like ENABLE(XXX) that some ports wish to offer as options to people 
building WebKit can have overridden values in makefiles. But even these options 
should have sensible defaults in the Platform.h headers that express the 
current status of the port from the point of view of the port’s maintainers. 
Ideally we’d find a way to not repeat these default settings twice.

2) In some cases, the build machinery needs to contribute to things like 
feature detection. So on some platforms, things like HAVE(READLINE) would be 
set correctly by the build system.

Options intended to be set by the environment would carefully be wrapped in 
#ifndef.

But other options, which simply express relationships between configuration 
elements, are designed to be set by Platform.h and not overridden by the 
environment, and so they would not be wrapped in #ifndef. Many HAVE options and 
most USE options would fall into this category.

What do you all think?

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

Re: [webkit-dev] Introducing a minimum ICU version for WebKit

[Darin Adler]

Here’s my take:

The source code we check into the WebKit repository is there for the 
convenience of the people *contributing* to WebKit, and is need not be the sole 
input when building and packaging WebKit for distribution.

Including ICU sources in a GTK WebKit tarball would not necessarily require 
checking ICU source code into the WebKit repository.

Everything else being equal, adding more files to the WebKit repository does 
make things less convenient for contributors. I would like us to do what we can 
to cut down the files, including things like unused PNG files in the 
LayoutTests directories, any source code that was needed at one time but is not 
needed in practice any more, including source code in the ThirdParty directory.

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

Re: [webkit-dev] Accidental binary bloating via C/C++ class/struct + Objective-C

[Darin Adler]

> On Jan 13, 2020, at 5:52 PM, Yusuke Suzuki  wrote:
> 
> We can purge type-encoding-string if we use Objective-C NS_DIRECT feature 
> (which makes Objective-C function as C function calling convention, removing 
> metadata).
> However, this does not work universally: with NS_DIRECT, Objective-C override 
> does not work. This means we need to be extra-careful when using it.

Yes, we need to be careful, but NS_DIRECT is likely to have more benefit than 
just binary shrinking, and it should do even more binary shrinking than hiding 
types from Objective-C. Use of NS_DIRECT will likely help performance, 
sometimes in a measurable way.

However, besides the risk of making something “non-overridable”, I think it’s 
only available in new versions of the clang compiler.

> So, as a simple, but effective work-around, in the above patch, we introduced 
> NakedRef / NakedPtr. This is basically raw pointer / raw reference to 
> T, with a wrapper class.
> This leverages the behavior of Objective-C compiler’s mechanism “one-level 
> deep type information collection”. Since NakedRef / NakedPtr introduces 
> one-level deep field,
> Objective-C compiler does not collect the type information of T if 
> NakedPtr is included in the fields / signatures, while the compiler 
> collects information when T* is used.

Very exciting. Does this cover all the cases we care about? Does come up for 
types that are not references or pointers? Maybe we can pass arguments by const 
reference? What about return values?

> ## Future work
> 
> We would like to avoid including such types accidentally in Objective-C. We 
> should introduce build-time hook script which detects such a thing.
> I uploaded the PoC script in https://bugs.webkit.org/show_bug.cgi?id=205968 
> , and I’m personally planning 
> to introduce such a hook into a part of build process.

Beautiful. Well worth doing.

Thanks!

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

Re: [webkit-dev] Introducing WTF::makeUnique and WTF::makeUniqueWithoutFastMallocCheck

[Darin Adler]

> On Aug 23, 2019, at 7:26 AM, Antti Koivisto  wrote:
> 
> Could WTF::makeUnique simply use FastMalloc by default? We could then remove 
> most of these messy annotations.
> 
> This would require replacing std::unique_ptr with a type that knows how to 
> free the objects correctly (bring back OwnPtr!) but that doesn't seem like a 
> big deal. It wouldn't play well with mixed use of OwnPtr and new/delete but 
> that should be avoided in any case.

I also suggested this, and you can see Yusuke’s response here 
.

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

Re: [webkit-dev] Moving to Python 3

[Darin Adler]

> On Jul 16, 2019, at 12:46 PM, Alexey Proskuryakov  wrote:
> 
> - They shouldn't make it excessively difficult to do WebKit engineering on 
> older versions of macOS.
> 
> "Excessively" is not clearly defined, but it seems obvious that there is a 
> tradeoff between tools work difficulty, and difficulty for engineers who go 
> back to older macOS versions (and implicitly, difficulty for people who set 
> up bots, QA engineers, and others involved).
> 
> I don't think that anyone ever suggested that it will be up to each engineer 
> to figure out the best way to install Python 3.

Lets keep in mind our strategy to keep development of WebKit on macOS easy. We 
want to preserve this. The steps (assuming git) are:

https://webkit.org/build-tools/
• download and install Xcode from Apple
% xcode-select --install

https://webkit.org/getting-the-code/
% git clone git://git.webkit.org/WebKit.git WebKit
% Tools/Scripts/webkit-patch setup-git-clone

https://webkit.org/building-webkit/
% build-webkit --debug

https://webkit.org/running-webkit/
% run-safari

We’d really like to keep it to a small number of steps. Having to download and 
install anything else would be a significant additional step.

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

Re: [webkit-dev] Incremental unified builds on macOS

[Darin Adler]

It’s unlikely the problem is due to unified builds, since those are just source 
files with includes in them.

But I have encountered problems like this before; sometimes they are things 
affecting lots of people and I’ve been able to fix them. When trying to 
understand causes of this kind of problem in the past, an Xcode build system 
feature that Dan Bernstein told me about was really useful. To use it you can 
type this command:

% defaults write com.apple.dt.Xcode ExplainWhyBuildCommandsAreRun -bool YES

Then rebuild and the log will contain additional information about each build 
step.

Good luck and please let us know what you find!

— Darin

PS: That should work for now since WebKit still uses the “legacy Xcode build 
system” but I hear it won’t work once we move to the modern build system.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Requesting feedback about EWS comments on Bugzilla bugs

[Darin Adler]

> On Jun 15, 2019, at 9:13 PM, Aakash Jain  wrote:
> 
> 1) Do not upload archive (for layout-test-results) on bugzilla, instead 
> upload it to another server, unzip it and post a link to the results.html.
> Pros:
> a) Engineers won't have to download the attachment, unzip it, look for 
> failures, and then delete it from their disk. They can simply click the url 
> to view the results. 
> b) This approach will also reduce 2 comments per failure to 1 comment. 
> Currently there are two comments per failure, one for failure details, second 
> for bugzilla attachment.

Great improvement to do this. The most confusing thing about build bot comments 
is all the “creation of attachments” extra text with things like “attachment 
number” and “patch".

However, it’s really nice that I can download a directory full of test results 
easily. I’d like to see the EWS website still have that feature.

> 4) When a patch becomes 'obsolete', tag the corresponding EWS comments as 
> 'obsolete', so that they will be hidden.

Incredibly valuable.

> 5) Do not comment on bugzilla bug at all

I think this makes sense. I don’t see a reason that test results need to be 
comments. I think the “red bubble” in EWS already calls someone’s attention to 
failures.

If we want to augment it, we should think of what we are aiming at. I do find 
it useful to see which tests are failing, and when I click on the red bubble I 
don’t see that information. I have to click once to see the “log of activities” 
then click on “results”, then see a confusing giant file with lots of other 
information. At the bottom of that file the one thing I want to know.

A better hierarchy is to put that “what new tests are failing” summary right t 
the top and let the logs be fallbacks, not the primary place to see the 
features.

> instead send email to the author of the patch.

Why? I don’t think this should send any emails at all, unless the person 
requested it.

> Pros: less noisy, also this will allow to include more detailed information 
> about the failure in email.

I think the more detailed information should be on the webpage, not in an email.

> Cons: reviewers would have to click status-bubbles to see the failures, 
> failure information is not immediately present in the comments.

I think we should start with this approach, eliminating the comments entirely.

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

Re: [webkit-dev] Changing the svn commit hook to allow tabs for tests.

[Darin Adler]

> On May 10, 2019, at 1:13 PM, Keith Miller  wrote:
> 
> I’m not sure I know what you mean by allow a whole-directory exception. Do 
> you mean a top level directory? Or some kind of parameter we pass to the hook 
> to ignore some directory for that run?

I meant that we could add something the pre-commit hook could see in Subversion 
that would create an exception for a whole directory, rather than something 
inside the hook itself. Perhaps a specially named file, or a Subversion 
attribute on a specially named file, or something more clever. If Subversion 
had attributes on directories, it could be that.

> you can’t commit with git-svn as it doesn’t support svn properties (or at 
> least I wasn’t able to figure it out)

Ah, that’s a big blocker if lots of people are using git-svn — I certainly use 
it.

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

Re: [webkit-dev] Changing the svn commit hook to allow tabs for tests.

[Darin Adler]

> On May 10, 2019, at 1:00 PM, Keith Miller  wrote:
> 
> I don’t know if this is possible but it would be great if some 
> sub-directories could be excluded from the no-tabs pre-commit hook.

Maybe we can rewrite the pre-commit hook to allow a whole-directory exception. 
Ideally I’d prefer not to hardcode directories.

> it’s pretty inconvenient to add the svn attribute that allows tabs every time 
> I update the tests

Does it really have to be inconvenient? Can we make script that does this and 
check it in so anyone can run it? Or build it into webkit-patch or whatever 
tool you already use?

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

Re: [webkit-dev] Spam and indexing

[Darin Adler]

Should we post instructions somewhere for people dealing with spam? I believe 
the instructions are:

1) Look up the email address of the account that posted the spam and disable it 
first, so spammers don’t get email about other steps. Do this by clicking on 
Administration, Users, finding the user and putting the word “Spam” into the 
disable text.

2) Move any spam bugs into the Spam component.

3) Mark any spam comments as Private and also add the tag “spam”.

But maybe there’s more to it than that. For example, can someone without 
administration privileges do the right thing? Should we make a small tool to 
make this easier to do correctly?

I like the idea of having instructions so this isn’t oral tradition.

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

Re: [webkit-dev] Code Style: Opinion on returning void

[Darin Adler]

I tried not to weigh in on this, but my view on the materials we should use to 
build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and 
terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I 
don’t like about using “return” with a call to a void-returning function.

- You can’t use this idiom when you want to ignore the result of a function, 
only if the function actually returns void. Often functions are designed with 
ignorable and often ignored return values. For example, it’s common to call 
something that might fail and have a good reason to ignore whether it failed or 
not. The idiom we are discussing requires treating those functions differently 
from ones that return void. If you refactor so that a function now returns an 
ignorable return value you need to visit all the call sites using return and 
change them into the two line form.

- It works for return, but not for break. I’ve seen at least one case where 
someone filled a switch statement with return statements instead of break 
statements so they could use this more-terse form. Now if we want to add one 
line of code after that switch, we need to convert all those cases into 
multi-line statements with break.

- Unless it’s mandatory, it’s a case where two programmers can make different 
style choices. If both forms are acceptable, then it introduces a little bit of 
disorder into our code.

One thing I like about it is that since “pass along the return value from this 
inner function to this outer function” can be written this way, the code can 
then survive refactorings where both the inner and outer functions might gain 
or lose the return value.

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

[webkit-dev] Rename AtomicString to AtomString

[Darin Adler]

So, did we reach consensus that we should rename AtomicString to AtomString?

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

Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

[Darin Adler]

Vector’s inline capacity feature was originally created as an alternative to 
variable length arrays for most of the purposes people would want to put them.

Imagine, for example, that you need a variable length buffer of characters that 
is almost always going to be less then 32 bytes. You write this:

 Vector buffer;

You can then grow the buffer to whatever size you need. If it’s less than 32 
bytes then it uses space on the stack, and if more than 32 bytes it uses space 
on the heap.

The reason this is better than variable length arrays is that stack size often 
has a inflexible total limit, so it’s not OK in general to use an arbitrary 
amount of stack. The vector inline capacity allows us to easily use up to a 
fixed amount of stack, but then still correctly handle unusually large requests 
by spilling go the heap.

For that WebGL2 code the idiom would be something like this:

Vector parameters;
parameters.grow(numValues);

Not sure we have good Vector inline capacity documentation, but that’s a basic 
primer.

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

Re: [webkit-dev] the name "AtomicString"

[Darin Adler]

Seems to me we could fix the current problem by renaming from AtomicString to 
AtomString without causing any new problem.

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

Re: [webkit-dev] 'final' class specifier and 'final' method specifier

[Darin Adler]

Let’s be clear about what we are discussing.

The choice is not be between “final” and “override”.

The choice is between “final override”, “override final”, and “final” for 
functions which are both overrides and final.

— Darin

Sent from my iPhone

> On Dec 19, 2018, at 12:27 PM, Michael Catanzaro  wrote:
> 
>> On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev  
>> wrote:
>> Adding override to method which already has final specifier doesn't affect 
>> anything,
>> because both final and override may ony be used on virtual methods
> 
> FWIW I prefer override because it's much more clear what that keyword is used 
> for.
> 
> Michael
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] 'final' class specifier and 'final' method specifier

[Darin Adler]

> On Dec 19, 2018, at 1:52 AM, Fujii Hironori  wrote:
> 
> I'd like to change this because 'final' doesn't necessarily imply
> 'override'. See the following stackoverflow:
> https://stackoverflow.com/questions/29412412/does-final-imply-override

I’d be happy to require both final and override if there was any benefit to 
doing so.

I read it fairly carefully, and I don’t think it says anything meaningful for 
us. Maybe we can find a real example in WebKit code where there’s a “final” and 
if we added “override”, it would have caught a mistake that “final” won’t catch?

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

Re: [webkit-dev] the name "AtomicString"

[Darin Adler]

The name “AtomicString” was inspired by the term of art, “atom”, traditionally 
used in at least some programming language implementations for what I see now 
is often called interned strings. You’ll see a mention of that term in the 
article https://en.wikipedia.org/wiki/String_interning in the context of ML 
along with some other terms used for this such as “symbol”.

I’ve gotten used to the name AtomicString over the years, but I wouldn’t 
strongly object to changing it if other programmers are often confused by it’s 
similarity to the term “atomic operations”.

A mild objection I have to the term “interned string” is that the term 
“interned” is not really a normal English word; I wasn’t familiar with that 
jargon when we named AtomicString and I am still not entirely thrilled with it. 
I think that specific term comes from the LISP intern function and is familiar 
to programmers largely because of its use in Java, .NET, and some other modern 
programming languages and libraries; I had encountered the technique many times 
over the years without ever hearing the word “interning” and don’t find the 
jargon entirely logical.

Some people might suggest using the term “flyweight string” instead 
https://en.wikipedia.org/wiki/Flyweight_pattern and I’m not sure which I’d 
prefer. Maybe there’s another obvious name?

Apparently in Delphi’s DWScript they called it “unified string” 
https://www.delphitools.info/2013/06/17/string-unification/ but in the article 
I cited they are chided for not calling it an interned string.

— Darin

PS: I do love loudly declaiming, “Atomic string, the string of tomorrow!” but I 
can get over it.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCorePrefix.h vs. config.h

[Darin Adler]

> On Dec 9, 2018, at 10:34 PM, Fujii Hironori  wrote:
> 
> MSVC has /FI option.
> 
>   /FI (Name Forced Include File) | Microsoft Docs
>   
> https://docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017
> 
> Unfortunately, it seems that MSVC's precompiled header needs to be included 
> explicitly.
> 
>   /Yu (Use Precompiled Header File) | Microsoft Docs
>   
> https://docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017

So this seems like the main potential obstacle. We can force an include of a 
prefix header, or precompile a header, but not both, with the Microsoft 
compiler.

If we deal with this, then I think we could get rid of “config.h”; every other 
platform can handle a prefix header.

Did I understand that right?

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

Re: [webkit-dev] WebCorePrefix.h vs. config.h

[Darin Adler]

OK, here’s my answer after thinking on it a bit:

Best would be to eliminate “config.h”: Change “config.h” into an empty file 
first, then remove all “config.h” includes, and then remove the file. But to do 
that, we need to make sure every build system for WebKit supports prefix 
headers. I don’t know how close to that we are. Maybe close? How can we quickly 
find out?

Lacking that, I think we can and should change “config.h” so it’s just an 
include of “WebCorePrefix.h”, or the other way around. I think it would be 
valuable to keep the feature where we try to catch cases where we forget to 
include “config.h”, on the platforms that use a prefix header, for the benefit 
of the platforms that do not. That might mean small complexity remains and one 
file won’t literally just be a trivial include of the other.

I suppose it’s also important to verify that there is no benefit to 
precompiling less than all of what “config.h” includes.

— Darin

PS: I don’t think we know that there is only one configuration that needs 
“config.h”. That second code snippet from your original message, Alexey, was 
only relevant for platforms that are trying to support macOS without prefix 
headers, and there could be any number of non-macOS cases. (And that include 
seems like a relatively recent change done by someone who didn’t fully 
understand the original scheme.)
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebCorePrefix.h vs. config.h

[Darin Adler]

> On Dec 8, 2018, at 2:56 PM, Darin Adler  wrote:
> 
>> On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov  wrote:
>> 
>> only keep config.h just to include WebCorePrefix for the lone build scenario 
>> where that's needed, and to undef new/delete?
> 
> I think the answer likely lies here: What is this lone build scenario where 
> config.h is needed?

I guess it’s CMake with Unix makefiles. Question for our CMake on Unix experts: 
Can we get CMake with Unix makefiles to support prefix headers?

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

Re: [webkit-dev] WebCorePrefix.h vs. config.h

[Darin Adler]

Useful background exists in Wikipedia: 
https://en.wikipedia.org/wiki/Prefix_header 
 and 
https://en.wikipedia.org/wiki/Precompiled_header 
 are relevant.

Alexey, perhaps you know all of this already, but here’s how I understand the 
intention behind having both of these files.

The “WebCorePrefix.h” file is a prefix header. We put things here that we want 
defined everywhere in the project.

The “config.h” file is a workaround for build systems that don’t support a 
prefix header. It’s inspired by the “config.h” files used in build systems 
based on autoconf, and was originally intended to keep the WebKit project 
compatible with them.

The file is unnecessary for builds systems like Xcode that support prefix 
headers.

Once we decided to use “config.h” as a “pseudo prefix header” we still decided 
to have a real prefix header under the Xcode build system, at least so we could 
precompile it. Ideally, under the Xcode build system, including “config.h” 
should have no effect at all other than participating in the check to help us 
ensure we didn’t forget to include it for the benefit of other build systems. 
We wanted to cleverly devise the contents of the prefix header so it’s easy to 
catch a mistake where we forget to include “config.h” somewhere; we’d like to 
get compile errors if we forget in most cases.

Maybe we don’t need both of these any more. Two possibilities occur to me:

- If we don’t need to support systems without support for prefix headers, we 
can eliminate “config.h”, which would be nice since we can streamline all our 
source files by removing the include for it.

- If we can get fast compilation without precompiling a header, then we don’t 
need to use a prefix header.

However, if we need support for systems without prefix headers and we need to 
use a prefix header for precompilation, then I do think we need to keep both of 
these. Their names or contents could change, but I think would still need two 
separate headers.

It would be great to “purify” any strange properties that these headers have 
accumulated. There should not be meaningful content repeated in both places. 
Ideally, content that needs to be included everywhere would be in a single 
place, perhaps a third header appropriately included by these two, and the 
prefix header and “pseudo prefix header” would just contain the tricks used to 
check for their proper use.

A corollary to this is that we should resist the urge to put platform-specific 
things into the prefix header just because it happens to be used on those 
platforms and no others. So perhaps there are Cocoa-specific things in 
WebCorePrefix.h that should instead be in a common place shared by both.

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

Re: [webkit-dev] WebCorePrefix.h vs. config.h

[Darin Adler]

> On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov  wrote:
> 
> only keep config.h just to include WebCorePrefix for the lone build scenario 
> where that's needed, and to undef new/delete?

I think the answer likely lies here: What is this lone build scenario where 
config.h is needed?

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

Re: [webkit-dev] Clarifying feature flags

[Darin Adler]

A while ago Kentaro Hara did some IDL keyword cleanup.

One of the best things he did was to make a single file with a list of all the 
keywords. In that case he was able to set things up so that if the file was 
inaccurate the build would fail; not sure if that’s practical for the various 
feature flags, but it’s helpful to have a single place. Here’s a link to an 
interesting WebKit mailing list message about one of the steps in that 
>.

I’d love us to find a similar way to “manage” and make it so someone can look 
over the entire list of feature flags and conditionals so we can spot problems 
easily.

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

Re: [webkit-dev] Rename wtf/unicode/UTF8.h

[Darin Adler]

> On Oct 31, 2018, at 7:52 AM, Konstantin Tokarev  wrote:
> 
> 31.10.2018, 05:18, "Fujii Hironori"  >:
>> wtf/unicode/UTF8.h is conflicting with ICU header in MSVC builds. I'd like 
>> to rename wtf/unicode/UTF8.h to wtf/unicode/WTFUTF8.h.
>> Any suggestion?
> 
> What about Unicode.h or UnicodeHelpers.h? UTF8.h deals with UTF16 as well


While I don’t love other of those names, I do like the idea of avoiding awkward 
“WTF” in the filename.

I don’t think it’s right to say “deals with UTF16”; this header contains only 
functions about dealing with UTF-8 and converting UTF-8 to and from other 
encodings (and yet those other encoding include UTF-16).

With a few seconds thought I am thinking that maybe UTF8Conversion.h or 
UTF8Transcoding.h are possible better ideas for new names. Neither is 
completely accurate. If we were going to add the word “helpers” than I would 
say UTF8Helpers.h, but I really don’t like those kinds of word in header names 
(“utilities”, “helpers”, “functions”, “classes”).

A separate issue once we rename: the header is also pretty old and crufty. 
Eventually we might want to remove or refine the functions in here. Not sure 
how widely they are used.

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

Re: [webkit-dev] node-jsc: A node.js port to the JavaScriptCore engine and iOS

[Darin Adler]

> On Sep 23, 2018, at 1:34 PM, Koby Boyango  wrote:
> 
> I will merge your changes to my fork

Would you be willing to focus on upstreaming first, instead? That would also 
get you the latest improvements, but in a more sustainable way.

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

Re: [webkit-dev] Offline Assembler build step always computes hashes even when nothing changes

[Darin Adler]

I don’t know. 

Sent from my iPhone

> On Sep 17, 2018, at 7:49 AM, Filip Pizlo  wrote:
> 
> 
> 
>> On Sep 16, 2018, at 8:48 PM, Darin Adler  wrote:
>> 
>>> On Sep 16, 2018, at 5:59 PM, Filip Pizlo  wrote:
>>> 
>>> Which offline assembler build step are you referring to?
>> 
>> The one that is the “Offline Assembler” target in Xcode, which runs this 
>> command:
>> 
>> ruby JavaScriptCore/offlineasm/asm.rb 
>> JavaScriptCore/llint/LowLevelInterpreter.asm 
>> "${BUILT_PRODUCTS_DIR}/JSCLLIntOffsetsExtractor” LLIntAssembly.h
>> 
>> For a “nothing rebuild” of all of WebKit and all of Safari for iOS on my 
>> iMac, it takes about 10 seconds out of a 30 second total “build" time.
>> 
>> Looking more carefully at the build log now, it seems that recompiling 
>> LLIntOffsetExtractor.cpp is also taking multiple seconds. Not executing 
>> generate_offset_extractor.rb, but compiling the output.
> 
> Does every build that you do rebuild LLIntOffsetExtractor.cpp?  Including a 
> clean build?
> 
> -Filip
> 
>> 
>> — Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Offline Assembler build step always computes hashes even when nothing changes

[Darin Adler]

> On Sep 16, 2018, at 5:59 PM, Filip Pizlo  wrote:
> 
> Which offline assembler build step are you referring to?

The one that is the “Offline Assembler” target in Xcode, which runs this 
command:

ruby JavaScriptCore/offlineasm/asm.rb 
JavaScriptCore/llint/LowLevelInterpreter.asm 
"${BUILT_PRODUCTS_DIR}/JSCLLIntOffsetsExtractor” LLIntAssembly.h

For a “nothing rebuild” of all of WebKit and all of Safari for iOS on my iMac, 
it takes about 10 seconds out of a 30 second total “build" time.

Looking more carefully at the build log now, it seems that recompiling 
LLIntOffsetExtractor.cpp is also taking multiple seconds. Not executing 
generate_offset_extractor.rb, but compiling the output.

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

[webkit-dev] Offline Assembler build step always computes hashes even when nothing changes

[Darin Adler]

I noticed that the “Offline Assembler” build step was taking between 5 and 30 
seconds every time I build. Really stands out in incremental builds. I realized 
that this step does not do any dependency analysis. Every time, it builds a 
hash of the input to see if it needs to recompute the assembly.

That’s probably not the best pattern; normally we like to use file modification 
dates to avoid doing any work when files haven’t changed.

Is there someone who can help me fix this so we get faster incremental builds?

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

Re: [webkit-dev] Unified sources have broken our #include hygiene

[Darin Adler]

> On Sep 1, 2018, at 5:14 PM, Simon Fraser  wrote:
> 
> Unified sources allow you to get away without #including all the files you 
> need in a .cpp file, because some earlier file in the group has probably 
> already included them for you.
> 
> This means that when you add a new file to the build, and the unified sources 
> get shuffled around, you end up with a long list of build breakages, some 
> platform-specific, that you can only fix by repeating EWS trials.

Yes.

We knew this was going to happen when evaluated the proposal before we switched 
to unified sources. I believe you can find some discussion of it in webkit-dev.

> How can we solve this?

I don’t think we should try to solve it.

To me at the moment, this seems to be a minor irritation. Even without unified 
sources, it’s common to get includes wrong for platforms other than the one you 
are testing on and find this out only when building on EWS.

I would be OK having an EWS server that builds various platforms without 
unified sources, but while it might help it might also hurt, adding more EWS 
results to interpret for each patch, and also finding theoretical problems that 
don’t affect any platform.

If there’s a tool that can figure out what files need to be included by 
analyzing source code, and that works well enough to be practical, I’d love to 
arrange it so that we can use that instead of having programmers have to write 
their own include statements. I’ve long been interested in 
> and 
wondered whether we could use it, but it sort of does the opposite, and the 
“multiple platforms” problem could well make even that tool impractical for us.

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

Re: [webkit-dev] Freenode spam counter-measure

[Darin Adler]

> On Aug 6, 2018, at 5:22 AM, Konstantin Tokarev  wrote:
> 
>>>  On Aug 5, 2018, at 2:38 AM, Philippe Normand  wrote:
>>> 
>>>  Can one of the #webkit admin please set the +r mode on? This would help 
>>> reducing spam. Only messages from registered nicks would come through.
>> 
>> I tried this by typing this:
>> 
>> /msg ChanServ set #webkit restricted on
> 
> This command works differently from +r: it forbids join for people who are 
> not in access list, instead of simply forbidding unregistered users.

What’s the ChanServ function for setting +r?

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

Re: [webkit-dev] Freenode spam counter-measure

[Darin Adler]

> On Aug 5, 2018, at 2:38 AM, Philippe Normand  wrote:
> 
> Can one of the #webkit admin please set the +r mode on? This would help 
> reducing spam. Only messages from registered nicks would come through.

I tried this by typing this:

/msg ChanServ set #webkit restricted on

But ChanServ replied:

ChanServ: You are not authorized to perform this command.

I wonder who is authorized!?

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

Re: [webkit-dev] Proposal: Not supporting x86 w/o SSE2

[Darin Adler]

Sounds good.

There are clients of WebKit outside web browsing. A significant client like 
that on Windows at Apple is iTunes. I checked 
 and Windows versions of iTunes require 
a processor with support for SSE2, so clarifying WebKit’s lack of support won’t 
be a problem there.

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

Re: [webkit-dev] Objective-C code in libwebrtc already assuming ARC?

[Darin Adler]

> On Jun 6, 2018, at 9:39 AM, Dan Bernstein  wrote:
> 
> libwebrtc.xcconfig sets CLANG_ENABLE_OBJC_ARC to YES for the libwebrtc 
> target. Since RTCVideoCodecH264.mm is part of that target the file is 
> compiled with ARC.

OK, great. That’s what I missed.

I’ll get rid of the stray -fobjc-arc in 
ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj for 
voice_processing_audio_unit.mm as a cleanup step.

Should we also set it in ThirdParty/libwebrtc/Configurations/Base.xcconfig or 
is there a good argument against doing that? I am considering setting it there 
and removing it from libwebrtc.xcconfig since eventually all the Base.xcconfig 
will have it. Or maybe we generally keep all the Base.xcconfig files in sync 
with each other?

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

Re: [webkit-dev] Objective-C code in libwebrtc already assuming ARC?

[Darin Adler]

> On Jun 6, 2018, at 9:28 AM, Darin Adler  wrote:
> 
> best keep working with manual retain and release

best kept working

> a non-tribal amount of code that already seems to assume ARC

a non-trivial amount

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

[webkit-dev] Objective-C code in libwebrtc already assuming ARC?

[Darin Adler]

Hi folks.

As some of you have probably noticed, I’ve begun making changes with the goal 
of preparing us to move most Objective-C code in WebKit to ARC. In doing so, I 
have been exploring the existing code in the tree and the various projects in 
our source tree. The nine projects with configuration files that I have looked 
at are:

JavaScriptCore, ANGLE, libwebrtc, WebCore, WebCore/PAL, WebInspectorUI, WebKit, 
WebKitLegacy/mac, and bmalloc.

These projects all currently have CLANG_ENABLE_OBJC_WEAK in their configuration 
files, and moving them to ARC involves adding CLANG_ENABLE_OBJC_ARC, and then 
doing lots of other work to ensure the projects still work properly, including 
possibly turning off ARC for certain source files that are best keep working 
with manual retain and release. Some of them such as ANGLE, bmalloc, and 
WebInspectorUI, have no Objective-C code, or almost none, so they should be 
easy to “convert".

But when investigating libwebrtc I discovered a non-tribal amount of code that 
already seems to assume ARC but to be compiled with ARC disabled. One example 
is these two methods:

-[RTCVideoEncoderFactoryH264 supportedCodecs]
-[RTCVideoDecoderFactoryH264 supportedCodecs]

If we are compiling this without ARC and executing calls to these methods, then 
I think we will have storage leaks.

As far as I can tell, the only source file in libwebrtc that is currently 
compiled with ARC enabled is voice_processing_audio_unit.mm but perhaps I am 
overlooking something.

Do we need to turn ARC on for the entire libwebrtc project?

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

Re: [webkit-dev] Build issues due to anonymous namespace

[Darin Adler]

> On Dec 3, 2017, at 12:16 PM, Filip Pizlo  wrote:
> 
> That also means not using static, for the same reason. FWIW, I think it’s a 
> good idea.

Maybe.

There is definitely no benefit in wrapping a class, structure, or type 
definition in an anonymous namespace. My comment was specifically about a 
structure.

For functions and global variables, static or anonymous namespace does have one 
remaining benefit. It’s a way to communicate to the compiler’s warning system 
and the -Wmissing-declarations warning that the function does not need to have 
been previously declared in a header. So, for WebKit, these would not mean “I 
can rely on the fact that this name is private to this file”. They would mean 
“I only intend to use this in this file and so did not declare it in a header 
and so please don’t warn me about the fact that I did not”.

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

Re: [webkit-dev] Build issues due to anonymous namespace

[Darin Adler]

I think it’s also worthwhile to remove the anonymous namespace wrapping each of 
these DestroyFunc structures when renaming them. Generally speaking, anonymous 
namespace doesn’t work when compilation units are arbitrary, since they say 
“limit this to one compilation unit”. So I’m not sure we should ever use them 
any more.

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

Re: [webkit-dev] Unified sources build errors when adding new source files

[Darin Adler]

Oh, I see. Errors in EWS in that bug. Looking.

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

Re: [webkit-dev] Unified sources build errors when adding new source files

[Darin Adler]

> On Nov 21, 2017, at 6:54 AM, Javier Fernandez  wrote:
> 
> It builds perfectly with the no-unify tag, but I produces totally
> unrelated errors when using the unified building. I wonder whether my
> patch could make those errors arise because of altering the building
> units (8 files, as far as I know) when adding the mentioned new entry to
> the WebCore/Source.txt file.

Seems likely. We could help you more if you gave specifics on the errors.

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

[webkit-dev] EWS queues seem stuck

[Darin Adler]

A couple of my patches have been sitting around all day and some of the EWS 
servers still say they are 15 patches behind. Are they stuck? Can someone bring 
them back to life?

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

Re: [webkit-dev] Tests failing because WPT images not served at correct URL?

[Darin Adler]

Youenn is helping me figure this out. My original guess about why it’s failing 
was probably wrong. You can follow along here 
>.

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