[PATCH] D16751: Enable constexpr on Visual Studio 2015, add support for two equivalent attributes

[Aaron Ballman via llvm-commits]

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
----------------
ariccio wrote:
> 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 @@
+#elif defined(_MSC_VER)
+#define LLVM_ATTRIBUTE_UNUSED_RESULT _Check_return_
 #else
----------------
ariccio wrote:
> 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!


http://reviews.llvm.org/D16751