Enable constexpr on Visual Studio 2015, add support for two equivalent attributes

[ariccio]

Summary

I'm not sure who owns Compiler.h, so I'll add people from the revision log. If it's not correct, please add the correct subscriber?

This patch enables constexpr on Visual Studio 2015 by adding || LLVM_MSC_PREREQ(1900) to the preprocessor #if statement. Since VS2013 doesn't support constexpr, that's purposely excluded. The LLVM_CONSTEXPR macro is used in ~25 places.

I also added the MSVC/SAL equivalent of:

• __attribute__((__warn_unused_result__)) as an LLVM_ATTRIBUTE_UNUSED_RESULT definition
• __attribute__((returns_nonnull)) as an LLVM_ATTRIBUTE_RETURNS_NONNULL definition

...in case anybody ever decides to run /analyze on LLVM (probably myself, if anybody)

[ariccio]

Everything SEEMS good, although I need to investigate two new failing tests?

(some time later)

I filed a bug. https://llvm.org/bugs/show_bug.cgi?id=26399 Everything should be good.

[aaron.ballman]

Generally LGTM, but adding @rnk and @majnemer for an extra opinion on MSVC's constexpr-readiness.

C:/LLVM/llvm/include/llvm/Support/Compiler.h

95 ↗

(On Diff #46478)

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.

[rnk]

C:/LLVM/llvm/include/llvm/Support/Compiler.h

128 ↗

(On Diff #46478)

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

[ariccio]

Grr. I need to "submit" comments.

C:/LLVM/llvm/include/llvm/Support/Compiler.h

128 ↗

(On Diff #46478)

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>.

[ariccio]

Responded to single comment.

C:/LLVM/llvm/include/llvm/Support/Compiler.h

95 ↗

(On Diff #46478)

That's a valid concern, but everything built fine.

...so I guess LLVM doesn't yet use C++14 constexpr?

[aaron.ballman]

C:/LLVM/llvm/include/llvm/Support/Compiler.h

95 ↗

(On Diff #46478)

Probably not. I don't think this concern is a reason to *not* enable this functionality if our usages currently work as expected.

128 ↗

(On Diff #46478)

Since Compiler.h is a stand-alone header file, I think it should include sal.h. Good catch, @rnk!

[ariccio]

Added #include <sal.h>.

[ariccio]

C:/LLVM/llvm/include/llvm/Support/Compiler.h

128 ↗

(On Diff #46478)

Ok, will do.

128 ↗

(On Diff #46478)

Will upload patch as:

#if defined(_MSC_VER)
#include <sal.h>
#endif

[ariccio]

Forgot to mark three things as done.

[ariccio]

Bump?

[aaron.ballman]

LGTM, thank you!

[ariccio]

Pardon my ignorance, but what exactly does This revision is now accepted and ready to land. mean? I assume it's waiting on something?

No, I don't mean literally.

[aaron.ballman]

In D16751#347027, @ariccio wrote:

Pardon my ignorance, but what exactly does This revision is now accepted and ready to land. mean? I assume it's waiting on something?

No, I don't mean literally.

It means that the revision is no longer in review status and you are welcome to commit the patch to trunk. The next step is for you (or someone else) to commit these changes, and then you should watch for build bot failures (in IRC is preferred, though you will get an email from most bots if your patch is in a batch of commits that causes a failure), and watch for any post-commit review feedback (which generally comes in on either this thread, or the thread started by the automated commit email).

[mehdi_amini]

In D16751#347196, @aaron.ballman wrote:

In D16751#347027, @ariccio wrote:

Pardon my ignorance, but what exactly does This revision is now accepted and ready to land. mean? I assume it's waiting on something?

No, I don't mean literally.

It means that the revision is no longer in review status and you are welcome to commit the patch to trunk. The next step is for you (or someone else) to commit these changes, and then you should watch for build bot failures (in IRC is preferred, though you will get an email from most bots if your patch is in a batch of commits that causes a failure), and watch for any post-commit review feedback (which generally comes in on either this thread, or the thread started by the automated commit email).

To be more explicit: if you don't have commit access, usually you need to ask here for someone to commit it for you.
To get commit access, see: http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

[ariccio]

Nit, it says:

(authored by rnk)

...eh? rnk committed it, not authored it?

[rnk]

Yeah, unfortunately SVN just has committer information, not actual info about who wrote the patch, unlike git. Our policy has been to add a line like "Patch by Hacker Jenny" when committing on behalf of others, so people looking for authorship should know to look for that. If you click through to rL260410 you can see it says "Patch by Alexander Riccio".

llvm/trunk/include/llvm/Support/Compiler.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_COMPILER_H
 #define LLVM_SUPPORT_COMPILER_H
 
 #include "llvm/Config/llvm-config.h"
 
+#if defined(_MSC_VER)
+#include <sal.h>
+#endif
+
 #ifndef __has_feature
 # define __has_feature(x) 0
 #endif
 
 #ifndef __has_extension
 # define __has_extension(x) 0
 #endif
 

 /// This can be used to provide lvalue/rvalue overrides of member functions.
 /// The rvalue override should be guarded by LLVM_HAS_RVALUE_REFERENCE_THIS
 #if LLVM_HAS_RVALUE_REFERENCE_THIS
 #define LLVM_LVALUE_FUNCTION &
 #else
 #define LLVM_LVALUE_FUNCTION
 #endif
 
-#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
 #else
 # define LLVM_CONSTEXPR
 #endif
 
 /// LLVM_LIBRARY_VISIBILITY - If a class marked with this attribute is linked
 /// into a shared library, then the class should be private to the library and
 /// not accessible from outside it.  Can also be used to mark variables and

 #if __has_attribute(used) || LLVM_GNUC_PREREQ(3, 1, 0)
 #define LLVM_ATTRIBUTE_USED __attribute__((__used__))
 #else
 #define LLVM_ATTRIBUTE_USED
 #endif
 
 #if __has_attribute(warn_unused_result) || LLVM_GNUC_PREREQ(3, 4, 0)
 #define LLVM_ATTRIBUTE_UNUSED_RESULT __attribute__((__warn_unused_result__))
+#elif defined(_MSC_VER)
+#define LLVM_ATTRIBUTE_UNUSED_RESULT _Check_return_
 #else
 #define LLVM_ATTRIBUTE_UNUSED_RESULT
 #endif
 
 // Some compilers warn about unused functions. When a function is sometimes
 // used or not depending on build settings (e.g. a function only called from
 // within "assert"), this attribute can be used to suppress such warnings.
 //

 #elif defined(_MSC_VER)
 #define LLVM_ATTRIBUTE_NORETURN __declspec(noreturn)
 #else
 #define LLVM_ATTRIBUTE_NORETURN
 #endif
 
 #if __has_attribute(returns_nonnull) || LLVM_GNUC_PREREQ(4, 9, 0)
 #define LLVM_ATTRIBUTE_RETURNS_NONNULL __attribute__((returns_nonnull))
+#elif defined(_MSC_VER)
+#define LLVM_ATTRIBUTE_RETURNS_NONNULL _Ret_notnull_
 #else
 #define LLVM_ATTRIBUTE_RETURNS_NONNULL
 #endif
 
 /// \macro LLVM_ATTRIBUTE_RETURNS_NOALIAS Used to mark a function as returning a
 /// pointer that does not alias any other valid pointer.
 #ifdef __GNUC__
 #define LLVM_ATTRIBUTE_RETURNS_NOALIAS __attribute__((__malloc__))