This is an archive of the discontinued LLVM Phabricator instance.

Do not require __STDC_LIMIT_MACROS and others
ClosedPublic

Authored by chfast on Jun 21 2016, 5:08 AM.

Download Raw Diff

Details

Reviewers

jyknight
joerg

Commits

rGf4437e9b488d: Do not require __STDC_LIMIT_MACROS and others
rL273493: Do not require __STDC_LIMIT_MACROS and others

Summary

Do not require STDC_LIMIT_MACROS and STDC_CONSTANT_MACROS macros to be defined globally. They are not needed for C++11 compliant standard headers.

Diff Detail

Event Timeline

chfast updated this revision to Diff 61357.Jun 21 2016, 5:08 AM

chfast retitled this revision from to Do not require __STDC_LIMIT_MACROS and others.

chfast updated this object.

chfast added a subscriber: llvm-commits.

Is there a way to test this commit on buildbot before merging it?

No longer forcing them is fine, but no longer defining them is IMO asking for bootstrap problems without a good reason.

Not requiring the defines is good; removing the defines in the build files is not. We still want to be able to build out of the box on systems with this known brokenness.

cmake/modules/HandleLLVMOptions.cmake
555–557 ↗	(On Diff #61357)	Keep these.
utils/vim/vimrc
111 ↗	(On Diff #61357)	Probably leave this too.

In D21553#463304, @jyknight wrote:

Not requiring the defines is good; removing the defines in the build files is not. We still want to be able to build out of the box on systems with this known brokenness.

Can we identify such broken systems in the first place?

I do care about exported cmake variables, especially LLVM_DEFINITIONS and it currently looks like:

set(LLVM_DEFINITIONS "-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS")

I guess nobody knows any more why and where they are needed.

Leave defines config as it was before.

In D21553#464402, @chfast wrote:

In D21553#463304, @jyknight wrote:

Not requiring the defines is good; removing the defines in the build files is not. We still want to be able to build out of the box on systems with this known brokenness.

Can we identify such broken systems in the first place?

Looking around briefly at the libcs I have handy...
glibc stopped checking them in 2013, newlib dropped them in 2015, musl in 2013, bionic in 2014. (dunno release dates, those are commit dates). So things look pretty good there.

On the other hand, you have platforms like freebsd and netbsd, which apparently *still* haven't removed these #ifdefs from their stdint.h. So...yeah, seems like there's still a need for the defines, for now. Anyone over in BSD land paying attention?... /me pokes joerg. :)

Anyway, I think is it good point to at least separate the changes as you suggested.

Oops, forgot to say LGTM last time. I said it in my head. :)

This revision is now accepted and ready to land.Jun 22 2016, 11:44 AM

Closed by commit rL273493: Do not require __STDC_LIMIT_MACROS and others (authored by chfast). · Explain WhyJun 22 2016, 2:23 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents

Path

Size

include/

llvm/

Support/

DataTypes.h.cmake

15 lines

Diff 61529

include/llvm/Support/DataTypes.h.cmake

	Show First 20 Lines • Show All 49 Lines • ▼ Show 20 Lines
	#include <stdint.h>			#include <stdint.h>
	#else			#else
	#error "Compiler must provide an implementation of stdint.h"			#error "Compiler must provide an implementation of stdint.h"
	#endif			#endif
	#endif			#endif

	#ifndef _MSC_VER			#ifndef _MSC_VER

	/* Note that this header's correct operation depends on __STDC_LIMIT_MACROS			#if !defined(UINT32_MAX)
	being defined. We would define it here, but in order to prevent Bad Things			# error "The standard header <cstdint> is not C++11 compliant. Must #define "\
	happening when system headers or C++ STL headers include stdint.h before we			"__STDC_LIMIT_MACROS before #including Support/DataTypes.h"
	define it here, we define it on the g++ command line (in Makefile.rules). */
	#if !defined(__STDC_LIMIT_MACROS)
	# error "Must #define __STDC_LIMIT_MACROS before #including Support/DataTypes.h"
	#endif			#endif

	#if !defined(__STDC_CONSTANT_MACROS)			#if !defined(UINT32_C)
	# error "Must #define __STDC_CONSTANT_MACROS before " \			# error "The standard header <cstdint> is not C++11 compliant. Must #define "\
	"#including Support/DataTypes.h"			"__STDC_CONSTANT_MACROS before #including Support/DataTypes.h"
	#endif			#endif

	/* Note that <inttypes.h> includes <stdint.h>, if this is a C99 system. */			/* Note that <inttypes.h> includes <stdint.h>, if this is a C99 system. */
	#include <sys/types.h>			#include <sys/types.h>

	#ifdef _AIX			#ifdef _AIX
	// GCC is strict about defining large constants: they must have LL modifier.			// GCC is strict about defining large constants: they must have LL modifier.
	#undef INT64_MAX			#undef INT64_MAX
	▲ Show 20 Lines • Show All 62 Lines • Show Last 20 Lines