The justification is correct based on my research into this problem, and the code changes themselves look correct. I cannot think of any reason why re-stating the alignment will matter in GCC mode, so I think this is OK.

This seems innocuous to me. I don't see any problem with it, but please don't commit until the others have had time to look.

LGTM, thanks for doing this!

In D54653#1368973, @lebedev.ri wrote:

@erichkeane are you still going to land this?
If you don't want to bother with updating my tests, want me to take over so this doesn't get abandoned?

@rnk and @aaron.ballman, does this look appropriate here?

In rL351495#306965, @erichkeane wrote:

Taking a look now, thank you for letting me know!

Taking a look now, thank you for letting me know!

I think this is a good start. That said, this is also a bug-fix (not exactly NFC), since it adds the Clang check to AppleClang.

This patch has run its course.

In D47073#1359273, @chandlerc wrote:

In D47073#1359266, @erichkeane wrote:

For what it's worth, the stricter version of this patch received quite overwhelming support in May. We delayed it due to Google's limitations and yet we still don't seem to have made progress. This current patch is a result of a continued set of compromises to try to satisfy a few in the vocal minority. I'd suggest that we all agree to have an ACTUAL decision made on llvm-dev, rather than this constant state of consensus derailed by vocal minority.

This *patch* does not matter to Google in any real sense that I'm aware of.... *All* of what I wrote in my latest comment is about this patch, and has nothing to do with the overall strategy of the LLVM project. And that feedback was given as a very active individual contributor to the project.

Does Google care a great deal about the overall state of host compiler support and C++ version support of LLVM? Yes, it does. We expressed that, and so did others. That was done on the -dev list as well as in the BoF. I'm not really interested in re-arguing it here.

But please try to take my feedback on this *patch* as just that. It is code review and feedback on the specific changes you are making. I'm genuinely trying to help get the CMake and documentation implementation *right* and useful for the project and the community, not for Google. I'm sorry that you're frustrated, I really am. But I am actually trying to help, and comments like this, IMO, are not constructive.

For what it's worth, the stricter version of this patch received quite overwhelming support in May. We delayed it due to Google's limitations and yet we still don't seem to have made progress. This current patch is a result of a continued set of compromises to try to satisfy a few in the vocal minority. I'd suggest that we all agree to have an ACTUAL decision made on llvm-dev, rather than this constant state of consensus derailed by vocal minority.

Update branch date to match the 7.0.0 from 8.0.0 branch date.

Fixed @lebedev.ri s comments.

Added Policy guidance for the change process to CheckCompilerVersion.cmake.

Fixing my awful inattention to spelling in a few places, moving the AppleClang code to be more clear for future updates.

Few more fixes suggested by @jfb . Also, added AppleClang to the check.

Change language to make this a 'guideline' instead of a hardline policy.

Reworded a thing. Still needs direction feedback for 8.0.

In D56407#1349672, @aaron.ballman wrote:

I think this is a good approach to solving the problem, so let's go this route.

Bumping again!

As stated in CFE commits (in response to Richard's comments):

Should catch me up on all comments except @zturner's llvm-undname feature request :)

@jfb encouraged me to ping this, whats the current status/thoughts on this?

Is there any feedback here? Am I just completely incorrect in how I tried to fix this?

As mentioned, the @rsmith comments that I thought were doable without feedback.

I got a couple of @rsmith's requests done. Most importantly I suspect is MultiVersionFuncs, though NotForDefinition vs ForDefinition is perhpas something you'll find important.

Thanks @aaron.ballman . This should fix all of your complaints.