Closed mtei closed 4 years ago
I can confirm that I'm having trouble compiling some AVR boards with LTO_ENABLE = yes
on the latest master.
It appears that these boards don't define the macros conditionally, i.e. they don't check for LTO first.
Since this is a common occurrence across many keyboards, keymaps and userspaces, the only sensible solution, in my opinion, is to remove the macro definitions from tmk_core/common.mk
.
I seem to have misread the contents of quantum/template/avr/config.h.
In quantum/template/avr/config.h, NOACTION* is defined when LINK_TIME_OPTIMIZATION_ENABLE is not defined.
In tmk_core/common.mk b/tmk_core/common.mk, the LINK_TIME_OPTIMIZATION_ENABLE and NOACTION* are all defined.
As a result, NOACTION* always seems to be defined.
There's still a problem with that. Not all boards, userspaces or keymaps are going to have the #ifndef LINK_TIME_OPTIMIZATION_ENABLE
check around #define NO_ACTION_{FUNCTION,MACRO}
— and it would be unreasonable to expect them to have it.
Defining NO_ACTION_{FUNCTION,MACRO}
in tmk_core/common.mk
is a bad solution for this reason. At the time of processing the makefile, there's no way of knowing or checking whether some user code down the line defines those same macros. Making the check mandatory would not only make keyboard/user code less elegant, but it would also make the usage of NO_ACTION_{FUNCTION,MACRO}
asymmetric compared to other NO_ACTION_*
macros.
For this reason, I think a better solution would be to remove the defines from tmk_core/common.mk
:
diff --git a/tmk_core/common.mk b/tmk_core/common.mk
index 4d4272d26..3d0b83a01 100644
--- a/tmk_core/common.mk
+++ b/tmk_core/common.mk
@@ -162,8 +162,6 @@ ifeq ($(strip $(LINK_TIME_OPTIMIZATION_ENABLE)), yes)
endif
EXTRAFLAGS += -flto
TMK_COMMON_DEFS += -DLINK_TIME_OPTIMIZATION_ENABLE
- TMK_COMMON_DEFS += -DNO_ACTION_MACRO
- TMK_COMMON_DEFS += -DNO_ACTION_FUNCTION
endif
# Search Path
and instead conditionally define NO_ACTION_{MACRO,FUNCTION}
in tmk_core/common/action.h
:
diff --git a/tmk_core/common/action.h b/tmk_core/common/action.h
index dd22023f9..17f770fa0 100644
--- a/tmk_core/common/action.h
+++ b/tmk_core/common/action.h
@@ -28,6 +28,15 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
extern "C" {
#endif
+#ifdef LINK_TIME_OPTIMIZATION_ENABLE
+# ifndef NO_ACTION_MACRO
+# define NO_ACTION_MACRO
+# endif
+# ifndef NO_ACTION_FUNCTION
+# define NO_ACTION_FUNCTION
+# endif
+#endif
+
/* tapping count and state */
typedef struct {
bool interrupted : 1;
This solution avoids all of the above problems, and further allows the LTO check to be removed from quantum/template/avr/config.h
.
Tagging @qmk/collaborators
Here's a branch where I have implemented the change:
https://github.com/vomindoraan/qmk_firmware/tree/no_action_macro_function
The following keyboards can be used for testing:
claw44
amj96
georgi
scarletbandana
tetris
pearl
uzu42
clueboard/66_hotswap
jc65/v32a
kbdfans/kbd6x
converter/usb_usb/ble
handwired/xealous/rev1
sentraq/s60_x/default
On master, these don't compile if LTO_ENABLE = yes
is set. With this change, they compile correctly.
It appears that NO_ACTION_MACRO and NO_ACTION_FUNCTION have been defined twice. Why is that?
The following changes have been made to #5674
Nevertheless, the following further changes have been made in #7211