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

Support C++ in app-layer #962

Merged
merged 6 commits into from
Feb 23, 2022
Merged

Support C++ in app-layer #962

merged 6 commits into from
Feb 23, 2022

Conversation

jonasdn
Copy link
Contributor

@jonasdn jonasdn commented Feb 23, 2022

  • Kbuild: Add support for C++ (.cc and .cpp) source
  • app-layer: Add C++ example app

Fixes: #960

scripts/Makefile.build Outdated Show resolved Hide resolved
scripts/Makefile.lib Outdated Show resolved Hide resolved
@ntamas
Copy link
Contributor

ntamas commented Feb 23, 2022

It works! At least with the changes I outlined above. The part that was not working is that I was adding an extra include folder to the build in Kbuild with subdir-ccflags-y += -I$(srctree)/app/vendor/some-library/include and it was not picked up until I switched to _c_flags.

Thanks for the quick fix!

@jonasdn
Copy link
Contributor Author

jonasdn commented Feb 23, 2022

Thank you for testing it out! Could you try with the last commit: "Kbuild: Use cxx_flags for C++" ? To see if it solves your last issue.

scripts/Makefile.build Outdated Show resolved Hide resolved
@ntamas
Copy link
Contributor

ntamas commented Feb 23, 2022

Yes, it works now! One tiny nitpick is that the output now shows CC something.o for C compilations but arm-none-eabi-g++ something.o for C++ files (and for some reason it includes the full path), but I couldn't figure out where this output is coming from. It doesn't bother me, but if you are super pedantic, this is the only issue left :)

@jonasdn
Copy link
Contributor Author

jonasdn commented Feb 23, 2022

Yes, it works now! One tiny nitpick is that the output now shows CC something.o for C compilations but arm-none-eabi-g++ something.o for C++ files (and for some reason it includes the full path), but I couldn't figure out where this output is coming from. It doesn't bother me, but if you are super pedantic, this is the only issue left :)

Thanks!

It was a typo $(CXX) vs CXX, fixed in latest commit.

 CXX     /home/jonasdn/sandbox/kbuild-firmware/examples/app_hello_world-cpp/src/hello_world.o

Copy link
Member

@tobbeanton tobbeanton 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!

@jonasdn
Copy link
Contributor Author

jonasdn commented Feb 23, 2022

Yes, it works now! One tiny nitpick is that the output now shows CC something.o for C compilations but arm-none-eabi-g++ something.o for C++ files (and for some reason it includes the full path), but I couldn't figure out where this output is coming from. It doesn't bother me, but if you are super pedantic, this is the only issue left :)

Thanks!

It was a typo $(CXX) vs CXX, fixed in latest commit.

 CXX     /home/jonasdn/sandbox/kbuild-firmware/examples/app_hello_world-cpp/src/hello_world.o

The full-path I think is an artifact of the oot build.

@ntamas
Copy link
Contributor

ntamas commented Feb 23, 2022

Before you guys merge this, here's a proposed patch to the build system that gets rid of some annoying clang warnings when building Kbuild itself on macOS:

diff --git a/tools/kbuild/Makefile.kbuild b/tools/kbuild/Makefile.kbuild
index 017ad491..fd7586c6 100644
--- a/tools/kbuild/Makefile.kbuild
+++ b/tools/kbuild/Makefile.kbuild
@@ -217,12 +217,17 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 HOSTCC       = gcc
 HOSTCXX      = g++
 HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O2
-HOSTCFLAGS  += -fomit-frame-pointer -Wno-format-overflow -std=gnu89
+HOSTCFLAGS  += -fomit-frame-pointer -std=gnu89
 HOSTCXXFLAGS = -O2

 ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
+# These flags are clang-only
 HOSTCFLAGS  += -Wno-unused-value -Wno-unused-parameter \
-               -Wno-missing-field-initializers -fno-delete-null-pointer-checks
+               -Wno-missing-field-initializers -fno-delete-null-pointer-checks \
+               -Wno-format-security
+else
+# These flags are gcc-only
+HOSTCFLAGS  += -Wno-format-overflow
 endif

 # Decide whether to build built-in, modular, or both.

(Sent this earlier on Discord).


Edit: this is on an Apple M1 CPU with clang 13, not sure whether it appears on Intel Macs.

@jonasdn
Copy link
Contributor Author

jonasdn commented Feb 23, 2022

Before you guys merge this, here's a proposed patch to the build system that gets rid of some annoying clang warnings when building Kbuild itself on macOS:

diff --git a/tools/kbuild/Makefile.kbuild b/tools/kbuild/Makefile.kbuild
index 017ad491..fd7586c6 100644
--- a/tools/kbuild/Makefile.kbuild
+++ b/tools/kbuild/Makefile.kbuild
@@ -217,12 +217,17 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 HOSTCC       = gcc
 HOSTCXX      = g++
 HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O2
-HOSTCFLAGS  += -fomit-frame-pointer -Wno-format-overflow -std=gnu89
+HOSTCFLAGS  += -fomit-frame-pointer -std=gnu89
 HOSTCXXFLAGS = -O2

 ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
+# These flags are clang-only
 HOSTCFLAGS  += -Wno-unused-value -Wno-unused-parameter \
-               -Wno-missing-field-initializers -fno-delete-null-pointer-checks
+               -Wno-missing-field-initializers -fno-delete-null-pointer-checks \
+               -Wno-format-security
+else
+# These flags are gcc-only
+HOSTCFLAGS  += -Wno-format-overflow
 endif

 # Decide whether to build built-in, modular, or both.

(Sent this earlier on Discord).

Edit: this is on an Apple M1 CPU with clang 13, not sure whether it appears on Intel Macs.

Thanks I will test this a bit and add as a separate PR!

@jonasdn jonasdn merged commit 308c13c into master Feb 23, 2022
@jonasdn jonasdn deleted the jonasdn/960 branch February 23, 2022 13:23
@knmcguire knmcguire added this to the next-release milestone Mar 21, 2022
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.

Kbuild: adding C++ support?
4 participants