Description
In commit 8575a52, the -Wno-expansion-to-defined was introduced, apparently to prevent a ton of warnings generated by the Atmel CMSIS header files (initially reported here: #290 (comment)):
#if SAM4_SERIES
^~~~~~~~~~~~~~
/home/matthijs/.arduino15/packages/arduino/tools/CMSIS-Atmel/1.2.0/CMSIS/Device/ATMEL/sam.h:563:1: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
I just tested without this flag (SAMD core 1.8.8, with CMSIS 1.2.0), and it seems the warnings are still there.
Since this warning does point to actual non-portable code, it would be nicer if it was not suppressed, but fixed in the CMSIS instead.
The actual problem seems to be that they use a macro that contains defined()
, e.g. something like #define BAR defined(FOO)
and then check that later (#if BAR
). In the Atmel CMSIS code, this is a bit more complex, because they use a part_is_defined
intermediate macro: https://github.com/arduino/ArduinoModule-CMSIS-Atmel/blob/46ab1021146152a64caf1ddbb837d8181b8faa35/CMSIS-Atmel/CMSIS/Device/ATMEL/sam.h#L32
E.g.:
#define part_is_defined(part) (defined(__ ## part ## __))
#define SAMG55J1 ( \
part_is_defined( SAMG55J19 ) )
I think the portable way to write this, is to write:
#if defined(SAMG55J19)
#define SAMG55J1 1
#endif
This might also need an else to define it as 0, but that's not needed if the macro is only used from the preprocessor I think. There's 150 instances of this macro, but all in the same file, so might be fixable automatically.
I also looked to see if newer versions of the Atmel CMSIS library still have this, but couldn't quite find this particular library with this layout. I looked inside the "ASF standalone framework 3.48", but that seemingly contains the same header files and components, but there is no "sam.h" that figures out what header file to include, only e.g. "samd21.h" that figures out which of the SAMD21 headers to include. To use those, I think we should let our own headers decide which family header to include exactly. I also looked at the Microchips packs, which is pretty much the same thing, but downloadable per family.
So if we'd upgrade this CMSIS stuff, I guess we could just pick out the families we need and include the right one directly.
Thinking on this, we could even do that now already and bypass the problematic sam.h
header entirely. Looking at boards.txt, all supported boards are SAMD21 anyway, so the below diff actually fixes this warning right now (while I was there, I also replaced the ""
with <>
in the includes, which is not actualy needed for this fix, but is a bit more correct):
diff --git a/cores/arduino/Arduino.h b/cores/arduino/Arduino.h
index c374c96..a40a5e4 100644
--- a/cores/arduino/Arduino.h
+++ b/cores/arduino/Arduino.h
@@ -45,7 +45,7 @@ extern "C"{
#endif // __cplusplus
// Include Atmel headers
-#include "sam.h"
+#include <samd.h>
#include "wiring_constants.h"
diff --git a/cores/arduino/SERCOM.h b/cores/arduino/SERCOM.h
index 6f855af..cbc3a05 100644
--- a/cores/arduino/SERCOM.h
+++ b/cores/arduino/SERCOM.h
@@ -19,7 +19,7 @@
#ifndef _SERCOM_CLASS_
#define _SERCOM_CLASS_
-#include "sam.h"
+#include <samd.h>
#define SERCOM_FREQ_REF 48000000
#define SERCOM_NVIC_PRIORITY ((1<<__NVIC_PRIO_BITS) - 1)
diff --git a/cores/arduino/WVariant.h b/cores/arduino/WVariant.h
index bbe2e0c..d44a0e2 100644
--- a/cores/arduino/WVariant.h
+++ b/cores/arduino/WVariant.h
@@ -19,7 +19,7 @@
#pragma once
#include <stdint.h>
-#include "sam.h"
+#include <samd.h>
#ifdef __cplusplus
extern "C" {
diff --git a/cores/arduino/cortex_handlers.c b/cores/arduino/cortex_handlers.c
index a910d08..e125b6e 100644
--- a/cores/arduino/cortex_handlers.c
+++ b/cores/arduino/cortex_handlers.c
@@ -16,7 +16,7 @@
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include <sam.h>
+#include <samd.h>
#include <variant.h>
#include <stdio.h>
diff --git a/cores/arduino/startup.c b/cores/arduino/startup.c
index d66bfa8..3f990ab 100644
--- a/cores/arduino/startup.c
+++ b/cores/arduino/startup.c
@@ -16,7 +16,7 @@
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include "sam.h"
+#include <samd.h>
#include "variant.h"
#include <stdio.h>
diff --git a/cores/arduino/wiring_private.h b/cores/arduino/wiring_private.h
index ce64e2d..1393237 100644
--- a/cores/arduino/wiring_private.h
+++ b/cores/arduino/wiring_private.h
@@ -27,7 +27,7 @@ extern "C" {
#endif
// Includes Atmel CMSIS
-#include "sam.h"
+#include <samd.h>
#include "wiring_constants.h"
diff --git a/platform.txt b/platform.txt
index 0a860df..7e6f881 100644
--- a/platform.txt
+++ b/platform.txt
@@ -28,8 +28,8 @@ version=1.8.5
compiler.warning_flags=-w
compiler.warning_flags.none=-w
compiler.warning_flags.default=
-compiler.warning_flags.more=-Wall -Wno-expansion-to-defined
-compiler.warning_flags.all=-Wall -Wextra -Wno-expansion-to-defined
+compiler.warning_flags.more=-Wall
+compiler.warning_flags.all=-Wall -Wextra
# EXPERIMENTAL feature: optimization flags
# - this is alpha and may be subject to change without notice