[PATCH] D66195: Move to C++14

[JF Bastien via Phabricator via llvm-commits]

jfb marked 3 inline comments as done.
jfb added a comment.

In D66195#1630743 <https://reviews.llvm.org/D66195#1630743>, @dexonsmith wrote:

> In D66195#1629514 <https://reviews.llvm.org/D66195#1629514>, @hans wrote:
>
> > In D66195#1629329 <https://reviews.llvm.org/D66195#1629329>, @jfb wrote:
> >
> > > In D66195#1629120 <https://reviews.llvm.org/D66195#1629120>, @hans wrote:
> > >
> > > > Perhaps add a note in docs/ReleaseNotes.rst as well?
> > >
> > >
> > > I can, but that seems odd: aren’t release notes for users of LLVM, not developers? In other words, what would someone reading this note take away from it, and do based on reading it?
> >
> >
> > I think the releases are also used by folks doing various kinds of out-of-tree LLVM stuff, and this change might let them use C++14 in their code by default.
>
>
> I agree with Hans that there should be an LLVM release note.  Another action for an LLVM user (who, to be clear, is a developer of an LLVM-based compiler) would be to update their host toolchain before trying to migrate to the newer LLVM version.


Our process <http://llvm.org/docs/DeveloperPolicy.html#toolchain> already handles this:

> Once the RFC reaches consensus, update the CMake toolchain version checks as well as the getting started guide. We want to soft-error when developers compile LLVM. We say “soft-error” because the error can be turned into a warning using a CMake flag. This is an important step: LLVM still doesn’t have code which requires the new toolchains, but it soon will. If you compile LLVM but don’t read the mailing list, we should tell you!

So it's the soft-error that tells developers what to do, not release notes. I can certainly add something, but it doesn't seem to be the right audience. If we do want something in the release notes, I think we should amend the process document to say so, and it should be done when we change the toolchain requirements, not when we change the language version.



================
Comment at: llvm/trunk/include/llvm/Support/type_traits.h:195-199
+#if defined(__cplusplus) || defined(_MSC_VER)
 #define LLVM_IS_FINAL(Ty) std::is_final<Ty>()
 #elif __has_feature(is_final) || LLVM_GNUC_PREREQ(4, 7, 0)
 #define LLVM_IS_FINAL(Ty) __is_final(Ty)
 #endif
----------------
dexonsmith wrote:
> It seems like we don't need `LLVM_IS_FINAL` anymore, given that `#if defined(__cplusplus)` looks like it would always be true for this header.  At least we can remove the `#if` guard.
I did it separately yesterday, see llvm.org/r368910
I wanted to keep this change small... and it still had some explosions :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66195