xiaoxiang781216 commented on a change in pull request #1321:
URL: https://github.com/apache/incubator-nuttx/pull/1321#discussion_r448336994



##########
File path: include/nuttx/compiler.h
##########
@@ -255,15 +255,6 @@
 #  undef  CONFIG_PTR_IS_NOT_INT
 #endif
 
-/* GCC supports inlined functions for C++ and for C version C99 and above */
-
-#  if defined(__cplusplus) || (defined(__STDC_VERSION__) && __STDC_VERSION__ 
>= 199901L)

Review comment:
       Since inline doesn't work well in the old compiler, @davids5 here is the 
patch which add CONFIG_HAVE_INLINE:
   
https://github.com/apache/incubator-nuttx/commit/3b53cd1e578de1e6bddfae605452a77323e5a70b
   You can see each place which reference CONFIG_HAVE_INLINE have to provide 
two versions: one is inline function and another is macro which make the simple 
thing complex(@yamt and I have the same feeling). The better approach is:
   1.Remove CONFIG_HAVE_INLINE
   2.Implement the function as a normal(no inline) function as much as possible
   3.For the simple and performance critical function:
     a.Implement as the macro if it is an internal function
     b.C standard function(like isspace) provide two versions:
        1.Inline version if ```__cpluscplus__``` define
        2.Macro verson if ```__cpluscplus__``` not define
   Note: 3.b is required because the c++ library often write "using ::isspace" 
which is incompatible with macro.
   The approach is better because:
   1.Only need provide one version(either normal function or macro) in the most 
case
   2.Have to provide two version(inline function and macro) for the C defined 
and performance critical function.
   The case 2 happen seldomly and even in this case we can directly use 
```__cpluscplus__ ``` because inline is the required feature for c++ compiler.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to