libcxx: Move #include_next <math.h> out of header guard in wrapper header.

[pcc]

Summary

Code on Windows expects to be able to do:

#define _USE_MATH_DEFINES
#include <math.h>

and receive the definitions of mathematical constants, even if <math.h>
has previously been included. To support this scenario, re-include
<math.h> every time the wrapper header is included.

[smeenai]

LGTM, though I'll wait a few days before accepting in case Eric and Marshall have comments.

libcxx/include/math.h

11

Could you add a comment explaining why this include is outside the header guards, since that's not standard?

Also, do you think it's worth adding a test for this? It would necessarily have to be Windows-only, and we don't have a Windows libc++ buildbot right now so it wouldn't catch regressions right away, but I think it'd be helpful for the future.

[pcc]

• Address review comments

[compnerd]

Yay for extensions.

[christof]

I've raised https://bugs.llvm.org/show_bug.cgi?id=36382 against this. I think the patch to fix it is trivial.

[mclow.lists]

A couple of points to consider here:

• "Windows users expect" is not a compelling argument.
• If done, this behavior should be windows-specific.
• You should include <__config> before anything else.

libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp

12 ↗

(On Diff #131523)

This test should use the libc++ macro, not the naked _MSC_VER

libcxx/include/math.h

 // -*- C++ -*-
 //===---------------------------- math.h ----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
+// This include lives outside the header guard in order to support an MSVC
+// extension which allows users to do:
+//
+// #define _USE_MATH_DEFINES
+// #include <math.h>
+//
+// and receive the definitions of mathematical constants, even if <math.h>
+// has previously been included.
+#include_next <math.h>
+
 #ifndef _LIBCPP_MATH_H
 #define _LIBCPP_MATH_H
 
 /*
     math.h synopsis
 
 Macros:
 

 */
 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
-#include_next <math.h>
-
 #ifdef __cplusplus
 
 // We support including .h headers inside 'extern "C"' contexts, so switch
 // back to C++ linkage before including these C++ headers.
 extern "C++" {
 
 #include <type_traits>
 #include <limits>

libcxx/test/libcxx/depr/depr.c.headers/math_h.sh.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// RUN: %compile -fsyntax-only
+
+#ifdef _MSC_VER
+
+#include <math.h>
+
+#define _USE_MATH_DEFINES
+#include <math.h>
+
+#ifndef M_PI
+#error M_PI not defined
+#endif
+
+#endif