MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003822Doomseeker[All Projects] Cleanuppublic2020-06-08 02:472020-06-16 07:38
ReporterWubTheCaptain 
Assigned ToBlzut3 
PrioritynormalSeveritytextReproducibilityhave not tried
StatusconfirmedResolutionopen 
PlatformOSDebian GNU/LinuxOS Versionbullseye/sid
Product Version1.3.1 
Target VersionFixed in Version 
Summary0003822: [[gnu::fallthrough]] in source files (-Wunknown-attributes)
Descriptionclang++ compiler doesn't like this GNU extension when compiling, ignores it and throws a warning.
Quote from :ALEInfo
(executable check - success) clang++
(finished - exit code 0) ['/bin/mksh', '-c', '''clang++'' -S -x c++ -fsyntax-only -iquote ''/home/wub/.local/src/doomseeker/doomseeker/src/core'' 
-DINSTALL_LIBDIR=\"lib\" -DINSTALL_PREFIX=\"/usr/local\" -DQT_CORE_LIB -DQT_GUI_LIB 
-DQT_MULTIMEDIA_LIB -DQT_
NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DQT_XML_LIB -Ddoomseeker_EXPORTS -I/home/wub/.local/src/doomseeker/doomseeker/build/src/core/doomseeker_autogen/include 
-I/home/wub/.local/src/doomseeker/doomseeker/src/core/gui -I/home/wub/.local/src/doomseeker
/doomseeker/build/src/core -I/home/wub/.local/src/doomseeker/doomseeker/src/core -I/home/wub/.local/src/doomseeker/doomseeker/src/wadseeker/.. 
-isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem 
/usr/in
clude/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ 
-isystem /usr/include/x86_64-linux-gnu/qt5/QtMultimedia -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork 
-isyst
em /usr/include/x86_64-linux-gnu/qt5/QtXml -fPIC -std=gnu++11 -std=c++14 -Wall - < ''/tmp/nvimfVec8g/2/scanner.cpp''']

<<<OUTPUT STARTS>>>
<stdin>:550:7: warning: unknown attribute 'fallthrough' ignored [-Wunknown-attributes]
                                [[gnu::fallthrough]];
                                  ^
1 warning generated.
<<<OUTPUT ENDS>>>
Steps To Reproduce
$ grep -ir "fallthrough" src/
src/wadseeker/zip/unzip.cpp:                            [[gnu::fallthrough]];
src/core/joincommandlinebuilder.cpp:            [[gnu::fallthrough]];
src/core/refresher/refresher.cpp:                       [[gnu::fallthrough]];
src/core/scanner.cpp:                           [[gnu::fallthrough]];
Additional Informationclang 9.0; see note 0003822:0021425
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2020-06-08 02:47WubTheCaptainNew Issue
2020-06-08 02:50WubTheCaptainNote Added: 0021417
2020-06-08 02:50WubTheCaptainAssigned To => WubTheCaptain
2020-06-08 02:50WubTheCaptainStatusnew => acknowledged
2020-06-08 03:37WubTheCaptainNote Added: 0021418
2020-06-08 03:40WubTheCaptainNote Added: 0021419
2020-06-08 03:40WubTheCaptainNote Edited: 0021419bug_revision_view_page.php?bugnote_id=21419#r13164
2020-06-08 04:20Blzut3Note Added: 0021420
2020-06-08 06:32WubTheCaptainNote Added: 0021425
2020-06-08 06:32WubTheCaptainStatusacknowledged => confirmed
2020-06-08 06:35WubTheCaptainOS => Debian GNU/Linux
2020-06-08 06:35WubTheCaptainOS Version => bullseye/sid
2020-06-08 06:35WubTheCaptainAdditional Information Updatedbug_revision_view_page.php?rev_id=13166#r13166
2020-06-08 06:38WubTheCaptainNote Edited: 0021425bug_revision_view_page.php?bugnote_id=21425#r13168
2020-06-08 06:40WubTheCaptainNote Edited: 0021425bug_revision_view_page.php?bugnote_id=21425#r13169
2020-06-08 07:02WubTheCaptainNote Added: 0021426
2020-06-08 07:34WubTheCaptainNote Added: 0021427
2020-06-08 07:35WubTheCaptainNote Edited: 0021427bug_revision_view_page.php?bugnote_id=21427#r13171
2020-06-08 07:36WubTheCaptainNote Edited: 0021427bug_revision_view_page.php?bugnote_id=21427#r13172
2020-06-08 07:40WubTheCaptainNote Edited: 0021427bug_revision_view_page.php?bugnote_id=21427#r13173
2020-06-08 07:42WubTheCaptainNote Edited: 0021427bug_revision_view_page.php?bugnote_id=21427#r13174
2020-06-08 07:43WubTheCaptainNote Edited: 0021427bug_revision_view_page.php?bugnote_id=21427#r13175
2020-06-08 07:47WubTheCaptainNote Added: 0021428
2020-06-08 07:49WubTheCaptainNote Edited: 0021428bug_revision_view_page.php?bugnote_id=21428#r13177
2020-06-08 07:50WubTheCaptainNote Edited: 0021428bug_revision_view_page.php?bugnote_id=21428#r13178
2020-06-08 07:53WubTheCaptainNote Edited: 0021417bug_revision_view_page.php?bugnote_id=21417#r13180
2020-06-08 13:03Pol MNote Added: 0021433
2020-06-08 13:08Pol MNote Edited: 0021433bug_revision_view_page.php?bugnote_id=21433#r13184
2020-06-08 14:01WubTheCaptainNote Added: 0021438
2020-06-08 14:01WubTheCaptainAssigned ToWubTheCaptain => Pol M
2020-06-08 14:13WubTheCaptainNote Edited: 0021438bug_revision_view_page.php?bugnote_id=21438#r13190
2020-06-08 14:13WubTheCaptainNote Edited: 0021438bug_revision_view_page.php?bugnote_id=21438#r13191
2020-06-08 14:15WubTheCaptainNote Added: 0021441
2020-06-08 14:41WubTheCaptainNote Added: 0021444
2020-06-08 14:45WubTheCaptainNote Edited: 0021444bug_revision_view_page.php?bugnote_id=21444#r13194
2020-06-08 21:17Blzut3Note Added: 0021448
2020-06-15 00:25Pol MNote Added: 0021464
2020-06-15 00:26Pol MAssigned ToPol M =>
2020-06-15 00:27Pol MStatusconfirmed => feedback
2020-06-16 07:36WubTheCaptainStatusfeedback => confirmed
2020-06-16 07:38WubTheCaptainAssigned To => Blzut3

Notes
(0021417)
WubTheCaptain   
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   
2020-06-08 03:37   
'https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/ [^]'
(0021419)
WubTheCaptain   
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   
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   
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   
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   
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   
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   
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   
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   
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   
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   
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   
2020-06-15 00:25   
Oh, if Blzut is ok with introducing a small C++17 thing, I won't block the way then.