[PATCH] D16751: Enable constexpr on Visual Studio 2015, add support for two equivalent attributes
Aaron Ballman via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 3 07:58:35 PST 2016
aaron.ballman added inline comments.
Comment at: C:/LLVM/llvm/include/llvm/Support/Compiler.h:95
@@ -94,3 +94,3 @@
-#if __has_feature(cxx_constexpr) || defined(__GXX_EXPERIMENTAL_CXX0X__)
+#if __has_feature(cxx_constexpr) || defined(__GXX_EXPERIMENTAL_CXX0X__) || LLVM_MSC_PREREQ(1900)
# define LLVM_CONSTEXPR constexpr
> aaron.ballman wrote:
> > I think this is reasonable, but we may still get some failures depending on usage. MSVC's constexpr support is drastically improved in MSVC 2015, but is not C++14 constexpr, it's C++11 constexpr.
> That's a valid concern, but everything built fine.
> ...so I guess LLVM doesn't yet use C++14 `constexpr`?
Probably not. I don't think this concern is a reason to *not* enable this functionality if our usages currently work as expected.
Comment at: C:/LLVM/llvm/include/llvm/Support/Compiler.h:128
@@ -127,1 +127,3 @@
+#define LLVM_ATTRIBUTE_UNUSED_RESULT _Check_return_
> rnk wrote:
> > This is provided by sal.h, right? You should probably add an include like this at the top:
> > #ifdef _MSC_VER
> > #include <sal.h>
> > #endif
> It is provided by `sal.h`, but it's like `size_t`: you generally don't need to `#include <stddef.h>` to use `size_t`, or `#include <cstddef>`, for `std::size_t` - they're usually included by default or built in.
> If you want to be super strict about standards conformance - which is a totally noble goal - then lemme know and I will go ahead and actually `#include <sal.h>`.
Since Compiler.h is a stand-alone header file, I think it should include sal.h. Good catch, @rnk!
More information about the llvm-commits