[llvm] db008af - [llvm] Repair the modules build with C++17

Thu Oct 6 10:03:11 PDT 2022

Hey Dave,

Apologies for missing this e-mail. This somehow got caught in a mail filter
and never made it to my inbox. I only saw the thread because someone else
pinged me directly about it. Not sure if there's a way in Gmail to debug
this after the fact, but I've made some changes to my mail rules
that should prevent this in the future.

As far as the change is concerned, I agree that it's unfortunate that we
need these workarounds. I suspect that it's a bug in the modules
implementation, but I have no evidence to back that up and I'm not familiar
enough with that part of clang to productively investigate that issue
myself. I've filed a radar on the folks that maintain that part of clang.
My motivation was limited to getting the LLDB bot building again after the
C++17 migration broke it.

We have (unfortunately) a bunch of workarounds in LLVM, often for quirks in
other compilers and older standard libraries. I don't think this is any
different except maybe that we have a little bit more agency here because
it's clang itself. To answer your question/concern: like those other
quirks, I wouldn't expect anyone to actively go out of their way to avoid
this construct. Like those other quirks, I think it's totally fair to deal
with those issues in a reactive matter, that is if we didn't fix the issue
ourselves already, which Adrian (CC'd) has been silently doing for the
majority of the modules issues.

On Mon, Oct 3, 2022 at 9:19 AM Philip Reames <listmail at philipreames.com>
wrote:

> Jonas,
>
> This thread has now been pinged 7 times over nearly two months. A basic
> expectation of all LLVM developers is to reply promptly to post commit
> review feedback.  Please reply to the original question on this thread.
> Please treat this matter as urgent.
>
> Philip
>

Hey Philip,

I'm sure this was well intended, but evidently I didn't see this thread or
I would've responded. An off-list ping or a DM on Discord would've gone a
long way.

Cheers,
Jonas

> On 10/3/22 09:04, David Blaikie via llvm-commits wrote:
> > Ping
> >
> > On Mon, Sep 26, 2022 at 8:41 AM David Blaikie <dblaikie at gmail.com>
> wrote:
> >> Ping
> >>
> >> On Mon, Sep 19, 2022 at 8:02 AM David Blaikie <dblaikie at gmail.com>
> wrote:
> >>> Ping
> >>>
> >>> On Mon, Sep 12, 2022 at 9:07 AM David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>> ping
> >>>>
> >>>> On Mon, Aug 22, 2022 at 12:52 PM David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>>> Ping
> >>>>>
> >>>>> On Mon, Aug 15, 2022 at 3:20 PM David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>>>> Ping
> >>>>>>
> >>>>>> On Tue, Aug 9, 2022 at 8:27 AM David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>>>>> These "I don't know why they work but they do" Workarounds
> continue to happen for modules & they're pretty unsatisfying.
> >>>>>>>
> >>>>>>> Any chance you could look into this a bit more to understand why
> they're broken/why this is the right fix? (partly because I expect we have
> quite a few range-based-for loops with initializer lists, and figuring out
> when some need explicit types and some don't seems like it'd be hard and
> confusing going forward)
> >>>>>>>
> >>>>>>> On Mon, Aug 8, 2022 at 3:04 PM Jonas Devlieghere via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >>>>>>>>
> >>>>>>>> Author: Jonas Devlieghere
> >>>>>>>> Date: 2022-08-08T15:04:46-07:00
> >>>>>>>> New Revision: db008af501534d4590542253ae3acf783986f5f7
> >>>>>>>>
> >>>>>>>> URL:
> https://github.com/llvm/llvm-project/commit/db008af501534d4590542253ae3acf783986f5f7
> >>>>>>>> DIFF:
> https://github.com/llvm/llvm-project/commit/db008af501534d4590542253ae3acf783986f5f7.diff
> >>>>>>>>
> >>>>>>>> LOG: [llvm] Repair the modules build with C++17
> >>>>>>>>
> >>>>>>>> I'm honestly not sure if this is a legitimate issue or not, but
> after
> >>>>>>>> switching from C++14 to C++17, the modules build started confusing
> >>>>>>>> arrays and initializer lists. Work around the issue by being
> explicit.
> >>>>>>>>
> >>>>>>>> Added:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>      llvm/include/llvm/ADT/STLExtras.h
> >>>>>>>>      llvm/include/llvm/Support/FormatProviders.h
> >>>>>>>>
> >>>>>>>> Removed:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> ################################################################################
> >>>>>>>> diff  --git a/llvm/include/llvm/ADT/STLExtras.h
> b/llvm/include/llvm/ADT/STLExtras.h
> >>>>>>>> index 8e18b6a95aac0..c56ca97856c2b 100644
> >>>>>>>> --- a/llvm/include/llvm/ADT/STLExtras.h
> >>>>>>>> +++ b/llvm/include/llvm/ADT/STLExtras.h
> >>>>>>>> @@ -725,8 +725,8 @@ class zip_shortest : public
> zip_common<zip_shortest<Iters...>, Iters...> {
> >>>>>>>>     template <size_t... Ns>
> >>>>>>>>     bool test(const zip_shortest<Iters...> &other,
> >>>>>>>>               std::index_sequence<Ns...>) const {
> >>>>>>>> -    return
> all_of(std::initializer_list<bool>{std::get<Ns>(this->iterators) !=
> >>>>>>>> -
> std::get<Ns>(other.iterators)...},
> >>>>>>>> +    return all_of(std::array{std::get<Ns>(this->iterators) !=
> >>>>>>>> +                             std::get<Ns>(other.iterators)...},
> >>>>>>>>                     identity<bool>{});
> >>>>>>>>     }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> diff  --git a/llvm/include/llvm/Support/FormatProviders.h
> b/llvm/include/llvm/Support/FormatProviders.h
> >>>>>>>> index 8101ed7968adb..7cfce29b134fc 100644
> >>>>>>>> --- a/llvm/include/llvm/Support/FormatProviders.h
> >>>>>>>> +++ b/llvm/include/llvm/Support/FormatProviders.h
> >>>>>>>> @@ -21,8 +21,8 @@
> >>>>>>>>   #include "llvm/Support/FormatVariadicDetails.h"
> >>>>>>>>   #include "llvm/Support/NativeFormatting.h"
> >>>>>>>>
> >>>>>>>> +#include <array>
> >>>>>>>>   #include <type_traits>
> >>>>>>>> -#include <vector>
> >>>>>>>>
> >>>>>>>>   namespace llvm {
> >>>>>>>>   namespace detail {
> >>>>>>>> @@ -369,7 +369,7 @@ template <typename IterT> class
> format_provider<llvm::iterator_range<IterT>> {
> >>>>>>>>         return Default;
> >>>>>>>>       }
> >>>>>>>>
> >>>>>>>> -    for (const char *D : {"[]", "<>", "()"}) {
> >>>>>>>> +    for (const char *D : std::array{"[]", "<>", "()"}) {
> >>>>>>>>         if (Style.front() != D[0])
> >>>>>>>>           continue;
> >>>>>>>>         size_t End = Style.find_first_of(D[1]);
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> llvm-commits mailing list
> >>>>>>>> llvm-commits at lists.llvm.org
> >>>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221006/6bd9e137/attachment.html>