0003822: [[gnu::fallthrough]] in source files (-Wunknown-attributes)

Notes
(0021417) WubTheCaptain (reporter) 2020-06-08 02:50 edited on: 2020-06-08 07:53	What should be done about this? 'https://en.cppreference.com/w/cpp/language/attributes/fallthrough [^]' 'https://clang.llvm.org/docs/AttributeReference.html#fallthrough [^]' I don't know why clang didn't like this vendor extension, maybe because ALE in NVim defaults to -std=c++14 (without GNU extensions). Anyway, [[fallthrough]] is valid since C++17. That's a far standards target at the moment (Doomseeker is targeting C++11). A comment / fallthrough / would be the most portable option, but I don't mind going with the C++17 [[fallthrough]] feature either.

(0021418) WubTheCaptain (reporter) 2020-06-08 03:37	'https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/ [^]'

(0021419) WubTheCaptain (reporter) 2020-06-08 03:40 edited on: 2020-06-08 03:40	'http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0068r0.pdf [^]' 'http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r1.pdf [^]'

(0021420) Blzut3 (administrator) 2020-06-08 04:20	Surprised Clang doesn't support the gnu version, but yes the proper course of action is to change this to the C++17 version now that it's standardized. Since unknown attributes are not an error, there's no issues with using this C++17 feature without requiring C++17.

(0021425) WubTheCaptain (reporter) 2020-06-08 06:32 edited on: 2020-06-08 06:40	Quote from Blzut3 Surprised Clang doesn't support the gnu version, I just found out it's not supported by clang 9.0, but it's supported by clang 10.0. My machine in OP was using clang 9.0 (Debian bullseye/sid). 'https://releases.llvm.org/9.0.0/tools/clang/docs/AttributeReference.html [^]' 'https://releases.llvm.org/10.0.0/tools/clang/docs/AttributeReference.html [^]' Quote from Blzut3 but yes the proper course of action is to change this to the C++17 version now that it's standardized. Since unknown attributes are not an error, there's no issues with using this C++17 feature without requiring C++17 I think you're correct and incorrect. Though I didn't test it, Intel's C++ compiler will treat unknown attributes as errors (I've read). Affects VSCode 2017 or 2019? (Fixed since April 2019?) N3337 draft for C++11 (page 168, dcl.attr.grammar, § 7.6.1) says: Quote from N3337 & N4140 For an attribute-token not specified in this International Standard, the behavior is implementation-defined. No word if it's not recognized, so compilers can either throw either an error, a warning, or just ignore it. Or do whatever. This remains so in N4140 (C++14 standard draft). In N4659 draft for C++17, this was changed (page 195, dcl.attr.grammar, § 10.6.1): Quote from N4659 For an attribute-token (including an attribute-scoped-token) not specified in this International Standard, the behavior is implementation-defined. Any attribute-token that is not recognized by the implementation is ignored. [Note: Each implementation should choose a distinctive name for the attribute-namespace in anattribute-scoped-token. — end note] So the unknown attributes are not an error only since C++17. I think we'd have to change CMake's CXX_STANDARD to target C++17 to ensure the behavior is correct in every case (with a standards compliant C++ compiler) to use [[fallthrough]]. Pedantically speaking. Or I could use / fallthrough / for the time being and leave CXX_STANDARD be C++11.

(0021426) WubTheCaptain (reporter) 2020-06-08 07:02	'http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0283r2.html [^]' and that MSVC (VSCode) issue:'https://developercommunity.visualstudio.com/content/problem/315417/error-c2429-on-fallthrough.html [^]'

(0021427) WubTheCaptain (reporter) 2020-06-08 07:34 edited on: 2020-06-08 07:43	I believe __attribute__((fallthrough)) was a GNU extension too (supported for C and C++98 since GCC 7, while [[gnu::fallback]] is a GNU extension for C++11 and C++14), so it's not proposed here. I'm not sure when exactly [[gnu::fallback]] was introduced, but I'd have to guess GCC 7. Relying on __cplusplus definition with the preprocessor was not reliable on MSVC:'https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ [^]' Nor on GCC (before GCC 4.7):'https://gcc.gnu.org/bugzilla/show_bug.cgi?id=1773 [^]' Plus, it'd be ugly to hack standards detection that way (and may cause problems in the future). So I'm only in for supporting one decision: Either the / fallthrough / comment style everywhere, or compiling against C++17 standard. So anyway, this affects (at least) GCC versions < 7, clang versions < 10, and Visual Studio 2019 Version < 16.1.

(0021428) WubTheCaptain (reporter) 2020-06-08 07:47 edited on: 2020-06-08 07:50	#if __has_cpp_attribute(fallthrough) [[fallthrough]] #else /* fallthrough */ #endif 🤪 I'd prefer just one way of doing it, not preprocessor heck. (Forget what I said about __cplusplus version checking, that's not reliable for feature compliance anyway.) And that hack doesn't check if __has_cpp_attribute is defined, so no thanks for more hacks. 'https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#testing-for-the-presence-of-an-attribute-__has_cpp_attribute [^]'

(0021433) Pol M (developer) 2020-06-08 13:03 edited on: 2020-06-08 13:08	Quote from Wub Though I didn't test it, Intel's C++ compiler will treat unknown attributes as errors (I've read). Affects VSCode 2017 or 2019? (Fixed since April 2019?) (standarized) attributes are intended to express intent both to the programmers and to the compiler. They are designed very explicitly to allow code with and without them to compile successfully and with the same behaviour. The compilers were technically on the wrong here. That said, intel's compiler and Visual Studio are not supported by the project, so we should only worry about gcc, mingw-w64 (which is basically gcc) and clang as it is pretty much gcc compatible. Quote from Wub I think we'd have to change CMake's CXX_STANDARD to target C++17 to ensure the behavior is correct in every case (with a standards compliant C++ compiler) to use [[fallthrough]]. Pedantically speaking. While not wrong, pretty much the course of action of any vendor is going to be to not error regardless the standard you use. If I recall correctly, I introduced this tag. We considered whether to break the laws a bit and just use [[fallthrough]], use a comment (which I opposed as I wanted something more formal) or use [[gnu::fallthrough]]. At the end, this last won as it was supported by gcc and mingw-w64 and that was the priority at the moment. I knew that would throw a warning in clang a that time, but since attributes doesn't change the code I considered it a small inconvenience in exchange for being able to compile with all the warnings enabled and being able to not get spammed with fallthrough warnings. I don't think it is a good idea to open the door to just a few C++>11 features as that just leads to extra warnings in compilers. That said, if we decide to move to C++17, I'll be happy to change them all to [[fallthrough]]

(0021438) WubTheCaptain (reporter) 2020-06-08 14:01 edited on: 2020-06-08 14:13	I'll leave the decision to you, Pol M. But I'll mention my preference and encouragement to be as portable and standards compliant as possible.

(0021441) WubTheCaptain (reporter) 2020-06-08 14:15	I will also mention the OpenBSD style(9) is to use a /* FALLTHROUGH */ comment (for C). 'https://man.openbsd.org/OpenBSD-6.6/style.9 [^]' Not entirely accurate because our code is C++, but compilers like GCC will recognize those comments to suppress the warnings.

(0021444) WubTheCaptain (reporter) 2020-06-08 14:41 edited on: 2020-06-08 14:45	0003806:0021443: Quote from Blzut3 Per the rule that we support Ubuntu releases until their EOL, we currently have to support GCC 5 which is only C++14. The GCC version may be of importance here.

(0021448) Blzut3 (administrator) 2020-06-08 21:17	Hmm, I don't recall the discussion on gnu::fallthrough vs fallthrough and it was added recently enough that I'm not entirely sure why we would have gone with the former. I'm fine with moving to the standard attribute and just letting GCC 5 throw a warning (or turn off that warning when compiling with older versions of GCC).

(0021464) Pol M (developer) 2020-06-15 00:25	Oh, if Blzut is ok with introducing a small C++17 thing, I won't block the way then.

Only registered users can voice their support. Click here to register, or here to log in.
Supporters:	No one explicitly supports this issue yet.
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2020-06-08 02:47	WubTheCaptain	New Issue
2020-06-08 02:50	WubTheCaptain	Note Added: 0021417
2020-06-08 02:50	WubTheCaptain	Assigned To	=> WubTheCaptain
2020-06-08 02:50	WubTheCaptain	Status	new => acknowledged
2020-06-08 03:37	WubTheCaptain	Note Added: 0021418
2020-06-08 03:40	WubTheCaptain	Note Added: 0021419
2020-06-08 03:40	WubTheCaptain	Note Edited: 0021419	View Revisions
2020-06-08 04:20	Blzut3	Note Added: 0021420
2020-06-08 06:32	WubTheCaptain	Note Added: 0021425
2020-06-08 06:32	WubTheCaptain	Status	acknowledged => confirmed
2020-06-08 06:35	WubTheCaptain	OS	=> Debian GNU/Linux
2020-06-08 06:35	WubTheCaptain	OS Version	=> bullseye/sid
2020-06-08 06:35	WubTheCaptain	Additional Information Updated	View Revisions
2020-06-08 06:38	WubTheCaptain	Note Edited: 0021425	View Revisions
2020-06-08 06:40	WubTheCaptain	Note Edited: 0021425	View Revisions
2020-06-08 07:02	WubTheCaptain	Note Added: 0021426
2020-06-08 07:34	WubTheCaptain	Note Added: 0021427
2020-06-08 07:35	WubTheCaptain	Note Edited: 0021427	View Revisions
2020-06-08 07:36	WubTheCaptain	Note Edited: 0021427	View Revisions
2020-06-08 07:40	WubTheCaptain	Note Edited: 0021427	View Revisions
2020-06-08 07:42	WubTheCaptain	Note Edited: 0021427	View Revisions
2020-06-08 07:43	WubTheCaptain	Note Edited: 0021427	View Revisions
2020-06-08 07:47	WubTheCaptain	Note Added: 0021428
2020-06-08 07:49	WubTheCaptain	Note Edited: 0021428	View Revisions
2020-06-08 07:50	WubTheCaptain	Note Edited: 0021428	View Revisions
2020-06-08 07:53	WubTheCaptain	Note Edited: 0021417	View Revisions
2020-06-08 13:03	Pol M	Note Added: 0021433
2020-06-08 13:08	Pol M	Note Edited: 0021433	View Revisions
2020-06-08 14:01	WubTheCaptain	Note Added: 0021438
2020-06-08 14:01	WubTheCaptain	Assigned To	WubTheCaptain => Pol M
2020-06-08 14:13	WubTheCaptain	Note Edited: 0021438	View Revisions
2020-06-08 14:13	WubTheCaptain	Note Edited: 0021438	View Revisions
2020-06-08 14:15	WubTheCaptain	Note Added: 0021441
2020-06-08 14:41	WubTheCaptain	Note Added: 0021444
2020-06-08 14:45	WubTheCaptain	Note Edited: 0021444	View Revisions
2020-06-08 21:17	Blzut3	Note Added: 0021448
2020-06-15 00:25	Pol M	Note Added: 0021464
2020-06-15 00:26	Pol M	Assigned To	Pol M =>
2020-06-15 00:27	Pol M	Status	confirmed => feedback
2020-06-16 07:36	WubTheCaptain	Status	feedback => confirmed
2020-06-16 07:38	WubTheCaptain	Assigned To	=> Blzut3

Relationships