This is an archive of the discontinued LLVM Phabricator instance.

[Support/Compiler.h] - Use gnu::fallthrough for LLVM_FALLTHROUGH when available.
ClosedPublic

Authored by grimar on May 10 2017, 6:15 AM.

Download Raw Diff

Details

Reviewers

bogner
chandlerc
• rafael
davide
ruiu
aaron.ballman

Commits

rG0c1f598cfa39: [Support/Compiler.h] - Use gnu::fallthrough for LLVM_FALLTHROUGH when available.
rL302878: [Support/Compiler.h] - Use gnu::fallthrough for LLVM_FALLTHROUGH when available.

Summary

I tried to compile LLD using GCC 7.1.0 and got warnings like
"warning: this statement may fall through [-Wimplicit-fallthrough=]"
(some more details are here: D32907)

GCC's __cplusplus value is 201402L by default, so macro expands to nothing,
though GCC 7 has support for [[fallthrough]].

I suggest to use gnu::fallthrough when it is available. That fixes issue I am observing.

Diff Detail

Repository: rL LLVM

Event Timeline

grimar created this revision.May 10 2017, 6:15 AM

This looks reasonable to me.

This revision is now accepted and ready to land.May 10 2017, 6:19 AM

grimar mentioned this in D32907: [ELF] - Fix warnings when LLD compiled using gcc 7.1.0.May 10 2017, 6:20 AM

grimar mentioned this in rL302666: [ELF] - Use LLVM_FALLTHROUGH in code..May 10 2017, 7:33 AM

Sure, but this is what I suggested in the first place, so I'll let other sign-off.

In D33036#750968, @davide wrote:

Sure, but this is what I suggested in the first place, so I'll let other sign-off.

Yeah, thanks for pointing on gnu::fallthrough !

ruiu added inline comments.May 10 2017, 11:51 AM

Compiler.h
236–237 ↗	(On Diff #98435)	I think you want to move this before !cplusplus to define LLVM_FALLTHROUGH even !cplusplus.

Addressed review comments.

grimar added inline comments.May 11 2017, 2:30 AM

Compiler.h

236–237 ↗

(On Diff #98435)

Looks reasonable, thanks.

Code probably could be something like next for clarity about workaround:

#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
#define LLVM_FALLTHROUGH [[fallthrough]]
#elif __has_cpp_attribute(gnu::fallthrough)
#define LLVM_FALLTHROUGH [[gnu::fallthrough]]
#elif __cplusplus && __has_cpp_attribute(clang::fallthrough)
// Workaround for llvm.org/PR23435, since clang 3.6 and below emit a spurious
// error when __has_cpp_attribute is given a scoped attribute in C mode.
#define LLVM_FALLTHROUGH [[clang::fallthrough]]
#else
#define LLVM_FALLTHROUGH
#endif

LGTM

Closed by commit rL302878: [Support/Compiler.h] - Use gnu::fallthrough for LLVM_FALLTHROUGH when available. (authored by grimar). · Explain WhyMay 12 2017, 12:06 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents

Path

Size

llvm/

trunk/

include/

llvm/

Support/

Compiler.h

2 lines

Diff 98729

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

	Show First 20 Lines • Show All 221 Lines • ▼ Show 20 Lines
	#define LLVM_ATTRIBUTE_RETURNS_NOALIAS __declspec(restrict)			#define LLVM_ATTRIBUTE_RETURNS_NOALIAS __declspec(restrict)
	#else			#else
	#define LLVM_ATTRIBUTE_RETURNS_NOALIAS			#define LLVM_ATTRIBUTE_RETURNS_NOALIAS
	#endif			#endif

	/// LLVM_FALLTHROUGH - Mark fallthrough cases in switch statements.			/// LLVM_FALLTHROUGH - Mark fallthrough cases in switch statements.
	#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)			#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
	#define LLVM_FALLTHROUGH [[fallthrough]]			#define LLVM_FALLTHROUGH [[fallthrough]]
				#elif __has_cpp_attribute(gnu::fallthrough)
				#define LLVM_FALLTHROUGH [[gnu::fallthrough]]
	#elif !__cplusplus			#elif !__cplusplus
	// Workaround for llvm.org/PR23435, since clang 3.6 and below emit a spurious			// Workaround for llvm.org/PR23435, since clang 3.6 and below emit a spurious
	// error when __has_cpp_attribute is given a scoped attribute in C mode.			// error when __has_cpp_attribute is given a scoped attribute in C mode.
	#define LLVM_FALLTHROUGH			#define LLVM_FALLTHROUGH
	#elif __has_cpp_attribute(clang::fallthrough)			#elif __has_cpp_attribute(clang::fallthrough)
	#define LLVM_FALLTHROUGH [[clang::fallthrough]]			#define LLVM_FALLTHROUGH [[clang::fallthrough]]
	#else			#else
	#define LLVM_FALLTHROUGH			#define LLVM_FALLTHROUGH
	▲ Show 20 Lines • Show All 257 Lines • Show Last 20 Lines