Skip to content

Conversation

@theamirocohen
Copy link
Contributor

Implement STATIC_ASSERT macro to catch errors at compile time.

* is known from c++11 while mbed-os compiles with c++98
*/
#define UVISOR_STATIC_ASSERT(cond, msg)
#ifdef __cplusplus
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use a different ifdef here.

Consider checking for C11 (__STDC_VERSION) first, and then falling back to other implementations.

#define UVISOR_STATIC_ASSERT(cond, msg) typedef char STATIC_ASSERT_##msg[(cond)?1:-1]
#else
#define UVISOR_STATIC_ASSERT(cond, msg) _Static_assert(cond, #msg)
#endif/*__CPP__*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove /*__CPP__*/ comment. It's annoying to maintain and doesn't help much, since the ifdef is so small.

@@ -1 +1 @@
0ac4c38729d2602bff96d68983a7790143210912 No newline at end of file
19ac0f1047d9bb7223fb8854f531968a8fe55d7b No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to update uvisor-test.txt? What modifications to uvisor-test do you need to support UVISOR_STATIC_ASSERT?

Copy link

Choose a reason for hiding this comment

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

this is an update to the head of the master branch of uvisor-tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Please only update when needed.

  • If you've made recent changes to the tests, also make a PR to uvisor to update the pointer.
  • If you need changes in the tests for work on uvisor, update tests and then update your PR to uvisor to update the pointer in the same commit to uvisor that needs the change.

@Patater
Copy link
Contributor

Patater commented Aug 8, 2017

The fallback implementation is not working when UVISOR_STATIC_ASSERT is used as a global statement, as in

#define UVISOR_BOX_RPC_GATEWAY_SYNC(box_name, gw_name, fn_name, fn_ret, ...) \
. Please revise the implementation so that it additionally works in this case.

@alzix
Copy link
Contributor

alzix commented Aug 10, 2017

please squash the "fix code review" commit

#define UVISOR_STATIC_ASSERT(cond, msg) typedef char STATIC_ASSERT_##msg[(cond)?1:-1]
#endif
#else
#define UVISOR_STATIC_ASSERT(cond, msg) _Static_assert(cond, #msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use elseif to eliminate excessive nesting. Also, check that C is new enough for _Static_assert, and otherwise fallback to the typedef-based implementation.

@theamirocohen theamirocohen force-pushed the static_assert branch 2 times, most recently from bb1cd23 to 595b91b Compare August 13, 2017 05:32
#define UVISOR_STATIC_ASSERT(cond, msg)
* The implementations differ due to compilation differences, C++ static_assert
* is known from C++11 (__cplusplus > 199711L) while mbed-os compiles with c++98,
* and C _Static_assert is known grom GCC version 4.6.0. */
Copy link
Contributor

Choose a reason for hiding this comment

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

- grom
+ from

@theamirocohen theamirocohen force-pushed the static_assert branch 3 times, most recently from 17ac946 to 1e7fb3d Compare August 14, 2017 13:07
Implement STATIC_ASSERT macro to catch errors at compile time.
@theamirocohen theamirocohen changed the title Implement a STATIC_ASSERT macro for uVisor Implement a STATIC_ASSERT macro Aug 14, 2017
@Patater Patater merged commit 4df46fb into ARMmbed:master Aug 14, 2017
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.

4 participants