Page MenuHomeFreeBSD

bc: use __has_attribute to test attribute presence
ClosedPublic

Authored by rlibby on Dec 10 2020, 9:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 18 2024, 8:20 AM
Unknown Object (File)
Dec 20 2023, 6:37 AM
Unknown Object (File)
Sep 4 2023, 4:34 PM
Unknown Object (File)
May 20 2023, 10:59 AM
Unknown Object (File)
Apr 8 2023, 10:30 AM
Unknown Object (File)
Mar 22 2023, 7:49 AM
Subscribers

Details

Reviewers
se
Summary

Suppress empty declaration error on gcc versions 6 and lower, which
don't have the fallthrough attribute.

Test Plan
env MAKEOBJDIRPREFIX=/usr/obj/gcc6 CROSS_TOOLCHAIN=amd64-gcc make buildworld
env MAKEOBJDIRPREFIX=/usr/obj/clang make buildworld

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 35327
Build 32258: arc lint + arc unit

Event Timeline

This has an upstream but I've submitted the patch here because that seems to be what the upstream author has requested. Please let me know if we should engage upstream first.

https://git.yzena.com/gavin/bc#user-content-other-projects

I have updated the math/gh-bc port to version 3.2.4, which includes a slightly modified version of this change.
The author wanted to avoid problems with other compilers than gcc and clang, which might not support the __has_attribute(()) mechanism, and therefore has added further conditions to your patch.

Warner Losh has asked to avoid vendor imports until after the conversion to git, else I'd resolve this issue with a vendor import and MFV,
A patch to contrib/bc could be committed instead, if you consider the issue urgent enough to not allow waiting for 2 weeks.

If you consider committing a fix that urgent, then I'd ask you to use the following patch, in order to not cause a conflict in a later MFV (I cannot update the diff in this review to this version and did not want to create another review with this patch):

Index: contrib/bc/include/status.h
===================================================================
--- contrib/bc/include/status.h	(revision 368505)
+++ contrib/bc/include/status.h	(working copy)
@@ -166,7 +166,11 @@
 #endif // __STDC_VERSION__
 
 #if defined(__clang__) || defined(__GNUC__)
+#if defined(__has_attribute) && __has_attribute(fallthrough)
 #define BC_FALLTHROUGH __attribute__((fallthrough));
+#else // defined(__has_attribute) && __has_attribute(fallthrough)
+#define BC_FALLTHROUGH
+#endif // defined(__has_attribute) && __has_attribute(fallthrough)
 #else // defined(__clang__) || defined(__GNUC__)
 #define BC_FALLTHROUGH
 #endif //defined(__clang__) || defined(__GNUC__)

If you think that a commit can wait 2 weeks, then I'll import a new vendor version after the conversion of the src repository to git ...

This revision is now accepted and ready to land.Dec 11 2020, 8:05 PM

Yeah I think waiting is probably fine. This is one of a few things breaking gcc builds. Some ZFS issues are also causing trouble and I guess those may have the same hiccup with vendor imports.

Thanks.

@se Now that vendor workflows are open for git, can we get upstream's fix in? Or do you prefer I just do a direct commit of upstream's fix as discussed above?

@se Now that vendor workflows are open for git, can we get upstream's fix in? Or do you prefer I just do a direct commit of upstream's fix as discussed above?

I plan to do a vendor import and MFV, but have to understand and test the procedure first.
Please allow for 24 hours to perform the import (its past midnight, already, here ...)

Yep, just a friendly ping. I'm eager to finish the gcc work, but it is definitely not urgent. Thank you!