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

[David Blaikie via llvm-commits]

On Thu, Oct 6, 2022 at 3:35 PM Adrian Prantl <aprantl at apple.com> wrote:
>
>
>
> > On Oct 6, 2022, at 3:07 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > On Thu, Oct 6, 2022 at 10:03 AM Jonas Devlieghere <jonas at devlieghere.com> wrote:
> >>
> >> 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.
> >
> > *fingers crossed* happy to help debug that if you like - but mostly
> > all I can probably offer is to send various test emails at points to
> > test your rules (presumably this email itself should qualify as such a
> > test... maybe?).
> >
> > But, yeah, I should escalate reviews via discord/discourse/chat/etc
> > moreso than I do. Sometimes I just get a bit lazy about it.
> >
> >> 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.
> >
> > Thanks! I'm glad there are some folks who might be able to take a look.
> >
> >> My motivation was limited to getting the LLDB bot building again after the C++17 migration broke it.
> >
> > Yeah, I appreciate the desire to keep things working, but worry about
> > keeping things working that we aren't maintaining/don't know why they
> > work/don't work the way they do.
> >
> >> 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.
> >
> > Yeah, I've often tried to poke at Adrian when he's committed changes
> > like this too - and I do think part of the difference is that we
> > maintain these things. If you want to keep a modules buildbot working
> > it's presumably because you care about modules working - so these bugs
> > presumably are part of the interest in keeping the bot working?
>
> We have two bots on green dragon that build with modules. The LLDB incremental bot and the Clang stage2 bot. The Clang stage2 bot mimics how clang is built at Apple when it ships with Xcode. The main reason why the LLDB bot is building with modules is performance, it used to result in a significant improvement in the number of builds / hour. Lately the difference doesn't seem to quite as large any more and I have not investigated if that's more due to changes in the sources we're compiling or in the compiler (which is confusingly the same thing ;-). Anyway, since the LLDB incremental bot runs a few hours before the Clang stage2 bot we usually find these problems first in LLDB and I usually just quietly fix up module maps and other smaller problems to keep it running.
> I always send (at least an internal) bug report to our Clang team when I find a problem that looks like a bug in the Clang frontend, but I also am under some pressure to put a workaround into place so we don't use coverage. Maybe we should just turn off modules on the LLDB bot and deal with the slightly slower turnaround times.

Glad to hear bugs are being filed at least - might be nice to have
those upstream to see if any progress is being made on them/help
out/etc.

(as much as I find it concerning that modules are being relied on but
not being debugged/maintained/fixed by the looks of it - I can't throw
stones here as Google's using modules internally without much
maintenance (Richard Smith having moved to focus on the Carbon work) -
though we haven't had to make patches like this one/haven't
encountered these sort of problems internally (admittedly we're not
using modules to build clang/llvm - only to build protobufs and other
low level libraries - so maybe we're not seeing the same sort of
interesting C++ idioms? Not sure))

> > Otherwise we could turn down the bot - I don't think many LLVM
> > developers are actually building with modules enabled, are they/is
> > that what the buildbot is there to support?
>
> We do build our production Clang with modules enabled, but that's primarily tested in the Clang stage2 bot.

Good to know!

- Dave