On Wed, Jul 21, 2021 at 1:18 AM Miklos Vajna <
vmi...@vmiklos.hu> wrote:
>
> On Tue, Jul 20, 2021 at 09:50:13AM -0700, Lei Zhang <
the...@chromium.org> wrote:
> > Taking a quick look, most of that patch looks good for upstreaming,
> > except removing constexpr from third_party/base/span.h. That is likely
> > an issue with GCC. In the long run, when C++20 usage is allowed, we'll
> > just switch to std::span which has a constexpr ctor.
>
> Sure, I only focused on the missing includes, the rest is a random
> collection of local workarounds to please our older baseline. :-)
>
> I've put <
https://pdfium-review.googlesource.com/c/pdfium/+/83210> up
> for review regarding the includes.
Thanks. That's in the CQ. I think it is mostly because the files in
question use memset(). For my local GCC build, I have
use_custom_libcxx=false set in my GN args, but for some reason I don't
see the same compile errors.
> > In the bug tracker, we have
https://crbug.com/pdfium/956 open for
> > having continuous testing with GCC. I do build with GCC locally every
> > so often and fix compile errors as they come up. One open question
> > here is what version of GCC to support. Which version does LibreOffice
> > use?
>
> GCC 7.0.0 is the libreoffice baseline (see
> <
https://cgit.freedesktop.org/libreoffice/core/tree/README.md#n45>), but
> in general, it's not my intention to force this baseline on any
> dependency we bundle. It's just this longer list of missing includes is
> something that can be potentially problematic to maintain downstream,
> and looked like an "it worked by accident before" situation, so good to
> fix.
Good to know. Locally I have GCC 8.x, 9.x, and 10.x, with 10.x being
the default and the one I test with.