Thread: Re: [brlcad-devel] [brlcad-commits] SF.net SVN: brlcad:[57869] brlcad/trunk/misc/CMake/CheckCInline
Open Source Solid Modeling CAD
Brought to you by:
brlcad
From: Christopher S. M. <br...@ma...> - 2013-09-24 18:40:36
|
Tom, I fixed the issue that was probably causing the inline check to misbehave for you. Would you delete cache and give it another try? It should now fail 'inline' and pass '__inline__' as intended. Tested your new BRLCAD_ENABLE_STRICT_C89=ON build mode, and it seemed to do the trick right. The problem was actually build files in src/other getting loaded and run before our -std= checks, causing the check to end up with the wrong result. Cheers! Sean p.s. That BRLCAD_ENABLE_STRICT_C89 flag needs to be documented in the INSTALL file, especially if it's going to be in the summary output. What do you think about a generalized ENABLE_POSIX_COMPLIANT flag name that can be c89 now and c99/c04/whatever later? On Sep 24, 2013, at 12:51 PM, br...@us... wrote: Revision: 57869 http://sourceforge.net/p/brlcad/code/57869 Author: brlcad Date: 2013-09-24 16:51:08 +0000 (Tue, 24 Sep 2013) Log Message: ----------- reverting r57867 for a variety of reasons: this module should not rely on BRLCAD_* logic (it's implemented as a proper CMake extension), should be testing 'inline' first or we can end up with different inline behavior on c99 compilers, and this change didn't update the documentation that was specific on the ordering and reasoning. the problem is that this needs to be tested *after* any std flags are set to ensure 'inline' will correctly fail the test. Revision Links: -------------- http://sourceforge.net/p/brlcad/code/57867 Modified Paths: -------------- brlcad/trunk/misc/CMake/CheckCInline.cmake Modified: brlcad/trunk/misc/CMake/CheckCInline.cmake =================================================================== --- brlcad/trunk/misc/CMake/CheckCInline.cmake 2013-09-24 16:45:11 UTC (rev 57868) +++ brlcad/trunk/misc/CMake/CheckCInline.cmake 2013-09-24 16:51:08 UTC (rev 57869) @@ -65,16 +65,8 @@ # initialize to empty set(${RESULT} "") - # provision for strict C89 testing - if (NOT BRLCAD_ENABLE_STRICT_C89) - # C99 C89 other - set(INLINE_STRINGS "inline" "__inline__" "__inline") - else() # C89 other - set(INLINE_STRINGS "__inline__" "__inline") - endif() |
From: Tom B. <tom...@gm...> - 2013-09-24 18:49:37
|
On Tue, Sep 24, 2013 at 1:40 PM, Christopher Sean Morrison <br...@ma...> wrote: > Tom, > > I fixed the issue that was probably causing the inline check to misbehave > for you. Would you delete cache and give it another try? It should now > fail 'inline' and pass '__inline__' as intended. Great, thanks for cleaning up the cow dung! > Tested your new > BRLCAD_ENABLE_STRICT_C89=ON build mode, and it seemed to do the trick right. Good to hear. > p.s. That BRLCAD_ENABLE_STRICT_C89 flag needs to be documented in the > INSTALL file, especially if it's going to be in the summary output. Will do. > What do > you think about a generalized ENABLE_POSIX_COMPLIANT flag name that can be > c89 now and c99/c04/whatever later? Sounds good to me, but shouldn't it be "ENABLE_C_POSIX_COMPLIANCE?" Then, when we decide on what it should be, we can define a "ENABLE_CXX_POSIX_COMPLIANCE" option. Best, -Tom |
From: Christopher S. M. <br...@ma...> - 2013-09-24 19:18:38
|
On Sep 24, 2013, at 02:48 PM, Tom Browder <tom...@gm...> wrote: Sounds good to me, but shouldn't it be "ENABLE_C_POSIX_COMPLIANCE?" Then, when we decide on what it should be, we can define a "ENABLE_CXX_POSIX_COMPLIANCE" option. I was thinking just one flag to mean either/both, similar to how ENABLE_STRICT basically means "use -Werror everywhere we can" but it applies to C/C++ code and has to be disabled in some dirs where it's a work in progress, etc. It would be a flag to force some level of standards compliance greater than our default, and we could use that flag to help us keep pressing forward no matter what standard or code (language) we're chasing next. It would sort of mean "compile even more strict / portable than our default", and what that actually means would necessarily change over time as we fix up sections of the code. Cheers! Sean |
From: Tom B. <tom...@gm...> - 2013-09-24 19:25:00
|
On Tue, Sep 24, 2013 at 2:18 PM, Christopher Sean Morrison <br...@ma...> wrote: > On Sep 24, 2013, at 02:48 PM, Tom Browder <tom...@gm...> wrote:> > Sounds good to me, but shouldn't it be "ENABLE_C_POSIX_COMPLIANCE?" > Then, when we decide on what it should be, we can define a > "ENABLE_CXX_POSIX_COMPLIANCE" option. ... > I was thinking just one flag to mean either/both, similar to how > ENABLE_STRICT basically means "use -Werror everywhere we can" but it applies Gotcha--WILCO. -Tom |