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

Build.mk changes to allow the use of CPPFLAGS, CFLAGS and CXXFLAGS. #2065

Merged
merged 4 commits into from May 8, 2020

Conversation

slaff
Copy link
Contributor

@slaff slaff commented May 6, 2020

Before this PR it was impossible to set easily C only flag because CFLAGS was used to set both C and C++ flags.
With this PR the common C/C++ flags should be set using the CPPFLAGS, as in a standard makefile.
With this change CFLAGS will be further on used ONLY for C compilation flags.
This PR is part of the changes needed to allow the compilation of the upcoming ESP32 architecture.
With this PR CPPFLAGS, CFLAGS and CXXFLAGS are allowed only in the Sming/Arch/*/build.mk files and should not be used in a component or application makefile.

…d if the following is observed

- CPPFLAGS - common flags for C and C++
- CFLAGS - C only flags
- CXXFLAGS - C++ only flags

In addition some toolchains require special standard for their C or C++ headers.
Therefore every architecture should be responsible in the Arch/*/build.mk to set the C standard as a CFLAG.
@slaff slaff added this to the 4.1.1 milestone May 6, 2020
@slaff slaff requested a review from mikee47 May 6, 2020 08:32
@slaff slaff force-pushed the feature/cpp-c-cxx-flags branch from ff48223 to 14857ab Compare May 6, 2020 08:35
Copy link
Contributor

@mikee47 mikee47 left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of points.

@@ -4,7 +4,8 @@
#
##############

CFLAGS += -DARCH_ESP8266
CPPFLAGS += -DARCH_ESP8266
CFLAGS += -std=c11
Copy link
Contributor

Choose a reason for hiding this comment

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

Should C coding standard be set in main build.mk file, rather than repeating for arch. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should C coding standard be set in main build.mk file, rather than repeating for arch. ?

We could set a default one in the main build.mk file but allow every architecture to change it if needed. For Arch Esp32 we need c99 standard for the c files otherwise too may problems pop-up.

Comment on lines 642 to 644
.. envvar:: COMPONENT_CFLAGS

Used to provide **ONLY** C++ flags that are applied globally.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove - already listed above (lines 599 - 605).

@@ -116,7 +116,7 @@ CFLAGS_COMMON = \
-ffunction-sections
Copy link
Contributor

Choose a reason for hiding this comment

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

CFLAGS_COMMON can probably just be rolled into CPPFLAGS, it's not used anywhere else.

@slaff slaff merged commit 46437c8 into SmingHub:develop May 8, 2020
@slaff slaff removed the 3 - Review label May 8, 2020
@slaff slaff mentioned this pull request May 8, 2020
4 tasks
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.

None yet

3 participants