Re: [webkit-dev] Unified source builds and adding or removing files...

[Tim Horton]

We already had this discussion; see the thread titled "Unified sources have 
broken our #include hygiene"

> On Mar 19, 2019, at 3:48 PM, Said Abou-Hallawa  wrote:
> 
> I have been working on patches that require adding and removing cpp files 
> from WebCore/Sources.txt. Almost every time I add or remove a file, I hit 
> undefined symbol compilation error in some unrelated source or header file. 
> Because a group of source files are compiled in one unified source file, some 
> dependencies get hidden.  The same symbol is defined in another source or 
> header file. Once sources are moved to different unified sources, the problem 
> gets uncovered.
> 
> For example the file svg/graphics/filters/filters/SVGFEImage.h uses the class 
> TreeScope without forward declaring it or including its header file. Oddly 
> the source file svg/graphics/filters/filters/SVGFEImage.cpp compiles in the 
> trunk right now. If a file is added to or removed from WebCore/Sources.txt 
> such that this source is moved to another unified source, the compiler will 
> give an error that TreeScope is not defined.
> 
> Another example is  > which fixes a compilation error 
> on GTK port. The same file was compiling fine on macOS but it failed on GTK.
> 
> Can we fix this issue? The fixes for such errors look very mysterious in the 
> patches. It can also cause build breaks because the ports do not compile the 
> same files.
> 
> One naive solution is to have the EWS bots compile without the unified source 
> builds. In this case, all the header and source files must have the required 
> forward declaration and/or the header file inclusion. So adding or removing 
> files should not affect the ability to compile any other source file.
> 
> Thanks,
> Said
> 
> ___
> 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] Unified source builds and adding or removing files...

[Said Abou-Hallawa]

I have been working on patches that require adding and removing cpp files from 
WebCore/Sources.txt. Almost every time I add or remove a file, I hit undefined 
symbol compilation error in some unrelated source or header file. Because a 
group of source files are compiled in one unified source file, some 
dependencies get hidden.  The same symbol is defined in another source or 
header file. Once sources are moved to different unified sources, the problem 
gets uncovered.

For example the file svg/graphics/filters/filters/SVGFEImage.h uses the class 
TreeScope without forward declaring it or including its header file. Oddly the 
source file svg/graphics/filters/filters/SVGFEImage.cpp compiles in the trunk 
right now. If a file is added to or removed from WebCore/Sources.txt such that 
this source is moved to another unified source, the compiler will give an error 
that TreeScope is not defined.

Another example is  which fixes a 
compilation error on GTK port. The same file was compiling fine on macOS but it 
failed on GTK.

Can we fix this issue? The fixes for such errors look very mysterious in the 
patches. It can also cause build breaks because the ports do not compile the 
same files.

One naive solution is to have the EWS bots compile without the unified source 
builds. In this case, all the header and source files must have the required 
forward declaration and/or the header file inclusion. So adding or removing 
files should not affect the ability to compile any other source file.

Thanks,
Said

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

Re: [webkit-dev] -Wpessimizing-move and -Wredundant-move

[Michael Catanzaro]

Several more return WTFMove() cases were committed just between 
yesterday and today. Please be careful. :)


On Tue, Mar 19, 2019 at 8:01 AM, Emilio Cobos 
=?iso-8859-1?q?=C1lvarez?=  wrote:
In Gecko, when I switched from mozilla::Move to std::move [1], I had 
to

disable the warning and fix all of them in a followup, since clang
didn't use to warn about them.


We used to have WTF::move defined as a function, but now it's a #define 
to ensure std::move gets called directly. I think that's what Andy 
meant when he says "WTFMove was written to accommodate these warnings."


Michael


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

Re: [webkit-dev] -Wpessimizing-move and -Wredundant-move

[Emilio Cobos Álvarez]

On 19/03/2019 00:56, Michael Catanzaro wrote:
> On Mon, Mar 18, 2019 at 4:43 PM, Andy Estes  wrote:
>> FWIW, Apple’s ports use the equivalent clang warning for pessimizing
>> and redundant moves, and we cleaned up a bunch of these mistakes in
>> our ports a few years ago. Hopefully you aren’t finding any of these
>> mistakes in shared code.
> 
> There are a lot, actually. Maybe clang only has -Wpessimizing-move
> (which caused relatively few warnings) and not -Wredundant-move (which
> caused very many). I'm not actually sure what the difference is; in both
> cases, a local variable is cast to rvalue reference via std::move before
> it is returned.

IIRC, clang only warns when using std::move, not a similarly defined
function (like WTFMove, presumably).

In Gecko, when I switched from mozilla::Move to std::move [1], I had to
disable the warning and fix all of them in a followup, since clang
didn't use to warn about them.

If gcc is able to warn more generally, that may explain what you're seeing.

 -- Emilio

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1465585
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] -Wpessimizing-move and -Wredundant-move

[Tomas Popela]

Ah, good to know! Thank you Michael :)

Tom

On Tue, Mar 19, 2019 at 1:42 PM Michael Catanzaro 
wrote:

> On Tue, Mar 19, 2019 at 12:57 AM, Tomas Popela 
> wrote:
> > If a note from
> >
> https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66
> > applies, then you can't fix it until we support Ubuntu 14.04 (due to
> > its old gcc version):
> >
> > Turns out the
> > std::move can only be dropped if the compiler has a fix for CWG1579.
> > For GCC
> > that's the case starting with GCC 5.1
>
> Currently we require GCC 6 to build, and if we follow our dependencies
> policy we'll be able to require GCC 7 next month, so that should be no
> problem.
>
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] -Wpessimizing-move and -Wredundant-move

[Michael Catanzaro]

On Tue, Mar 19, 2019 at 12:57 AM, Tomas Popela  
wrote:
If a note from 
https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66 
applies, then you can't fix it until we support Ubuntu 14.04 (due to 
its old gcc version):


Turns out the
std::move can only be dropped if the compiler has a fix for CWG1579.  
For GCC

that's the case starting with GCC 5.1


Currently we require GCC 6 to build, and if we follow our dependencies 
policy we'll be able to require GCC 7 next month, so that should be no 
problem.



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