[PATCH] D47073: Document and Enforce new Host Compiler Policy

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

In D47073#1358641 <https://reviews.llvm.org/D47073#1358641>, @mehdi_amini wrote:

> In D47073#1358640 <https://reviews.llvm.org/D47073#1358640>, @jfb wrote:
>
> > In D47073#1358626 <https://reviews.llvm.org/D47073#1358626>, @mehdi_amini wrote:
> >
> > > In D47073#1358593 <https://reviews.llvm.org/D47073#1358593>, @jfb wrote:
> > >
> > > > OK I think we've had more than enough bikeshed, over months, and in-person at the LLVM dev meeting. This is good enough, we want to warn people for the LLVM 8 branch which is tomorrow. Let's get this checked in today. We'll start a discussion on LLVM dev when anything *actually* changes, so nobody needs to panic just yet.
> > >
> > >
> > > I strongly disagree with this right now: this is *not* OK to rush this as is.
> > >
> > > If you need something in the release, then it can be done in a much less controversial way.
> >
> >
> > What makes you say this is being rushed?
>
>
> The way you're trying to get this checked in with two reviewers requiring major changes that haven't been addressed.


`s/two/one/` : I've talked to Chandler already, and he was leaning on others (in large parts, me) to help Erich with the review. Further, Erich addressed your comments repeatedly. It seems like you're asking for more?

> The fact that the process of discussing which version of the compiler to bump to should happen *before* adding a check, and it hasn't happened for this patch which *actually* change the minimum versions.

It happened months ago in the LLVM dev RFC, and was re-discussed in-person at the dev meeting.

>> There's agreement that we'll move away from C++11, soon. We'd be irresponsible if we didn't tell developers who build LLVM. That's the only thing the patch does: tell people what we've agreed to do?
> 
> 
> 
> 1. I asked *multiple* times in this patch *why* and *how* these particular version of the compiler where picked. I still don't know why clang-3.5 would trigger a cmake error.

Read the email thread?

> 2. The two levels: `cmake warn` vs `cmake error` are *not* required to achieve what you're describing: i.e. warning people in LLVM 8 that LLVM 9 won't support their compiler. *One* level (cmake error) is plenty enough.

This is setting up something new. Yes, we could just have one level for now, but we'll add the second level as soon as we truly move off C++11 (this is clear in the policy!). The patch sets things up properly for the policy we're establishing. It's a boundary value problem, and like all good BVPs we're carving out a small boundary (here, at the initial impulse) to solve the wider problem. The policy is encoded in CMake.

> In D47073#1358066 <https://reviews.llvm.org/D47073#1358066>, @mehdi_amini wrote:
> 
>> In D47073#1358039 <https://reviews.llvm.org/D47073#1358039>, @jfb wrote:
>>
>> > Yes: we want to warn folks early. This is just warning them, and they can opt-out.
>>
>>
>> The current version of the patch would issue a fatal error when using a host clang 3.5, not just a warning 
>>  (note that clang-3.4 is supposed to support C++14)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47073/new/

https://reviews.llvm.org/D47073