-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 errors with './bootstrap-configure --with-device-layer=linux' o… #1442
Build errors with './bootstrap-configure --with-device-layer=linux' o… #1442
Conversation
// Parentheses used to fix clang parsing issue with these declarations | ||
friend CHIP_ERROR(::chip::Platform::PersistedStorage::Read(::chip::Platform::PersistedStorage::Key key, uint32_t & value)); | ||
friend CHIP_ERROR(::chip ::Platform::PersistedStorage::Write(::chip::Platform::PersistedStorage::Key key, uint32_t value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we use the CHIP_ERROR( syntax, would it work in GCC?
needing an ifdef sounds odd ... is C++ not standard?
there seems to be an extra space in the namespace for Write
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof. Can we make a macro to address portability issues like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC/Clang will be both happy with parentheses. The issue being some zealous flags that will complains with ConfigurationManager.h:141:22: error: unnecessary parentheses in declaration of 'Read' [-Werror=parentheses]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworked the parentheses so CI is happy, Clang is happy and I hope you will be happy too :)
// Parentheses used to fix clang parsing issue with these declarations | ||
friend CHIP_ERROR(::chip::Platform::PersistedStorage::Read(::chip::Platform::PersistedStorage::Key key, uint32_t & value)); | ||
friend CHIP_ERROR(::chip ::Platform::PersistedStorage::Write(::chip::Platform::PersistedStorage::Key key, uint32_t value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof. Can we make a macro to address portability issues like this?
src/include/platform/internal/GenericPlatformManagerImpl_POSIX.ipp
Outdated
Show resolved
Hide resolved
8e416bd
to
32b5fd7
Compare
…n macOS with clang
32b5fd7
to
60964e2
Compare
…n macOS with clang
Problem
While doing
./bootstrap-configure --with-device-layer=linux
andmake
the build fails. Note that while device-layer says Linux there is not really anything specific there.Also I add to move the template instantiation at the bottom of the file otherwise I was unable to run the platform tests (`make -C src/platform/tests check) because of linking errors.