Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove -Wno-expansion-to-defined #557

Merged

Conversation

matthijskooijman
Copy link
Collaborator

By including a different CMSIS header, this warning no longer needs to be supressed.

This fixes #556.

@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b209_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Remove -Wno-expansion-to-defined  #557
  5. Select one of the boards under SAMD Pull Request Remove -Wno-expansion-to-defined  #557 in Tools->Board menu
  6. Compile/Upload as usual

@facchinm
Copy link
Member

LGTM! @cmaglie are you ok in merging it?

@aentinger
Copy link
Contributor

@matthijskooijman care to rebase on master? Since we've got the CI build now I'd run the CI build using your change. If it greenlights then I see no reason for not merging.

The sam.h file uses some non-portable macros that raise a warning in
newer gcc version. This warning was supressed in commit 8575a52 (Add
-Wno-expansion-to-defined compile warning flag), but this is not ideal.

However, since the only thing sam.h does is figure out what CPU is
selected and include the right family header, and we always use SAMD21
CPUs, the only thing sam.h does is include samd.h.

So we can easily bypass then and include samd.h directly.

This fixes the first part of arduino#556.
Now that we no longer include sam.h, this warning is no longer triggered
in normal builds, so there is no longer a need to supress it.

This fixes arduino#556.
@matthijskooijman matthijskooijman force-pushed the remove-no-expansion-to-defined branch from 04339d6 to 32f7ac6 Compare November 26, 2020 13:59
@matthijskooijman
Copy link
Collaborator Author

Just rebased. Haven't done any testing after rebasing, but let's see what the CI says :-)

@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b210_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Remove -Wno-expansion-to-defined  #557
  5. Select one of the boards under SAMD Pull Request Remove -Wno-expansion-to-defined  #557 in Tools->Board menu
  6. Compile/Upload as usual

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you @matthijskooijman 🚀

@aentinger aentinger merged commit 56e13aa into arduino:master Nov 26, 2020
@matthijskooijman matthijskooijman deleted the remove-no-expansion-to-defined branch July 4, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix warnings and remove -Wno-expansion-to-defined
4 participants