patchGNU Octave - Patches: patch #8906, configure check support for c11...

 
 

patch #8906: configure check support for c11 and c++11

Submitted by:  Carnë Draug <carandraug>
Submitted on:  Sun 07 Feb 2016 03:01:16 PM UTC  
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 18 Feb 2016 10:06:47 PM UTC, comment #16: 
Carnë Draug <carandraug>
Project Member
Thu 18 Feb 2016 06:00:24 PM UTC, comment #15: 

> should be in the configure.ac file. Probably no one will see it in the commit message. Similarly, you might add a note at the top of the .m4 files about the origin of those files.


Yeah, you're right that the comment should go in the code. I will add those comments and push this.

I do have a note about the origin of the macros but it's after the copyright notice just because I like to have the license as the very first block. I can move the that note up though.

Carnë Draug <carandraug>
Project Member
Thu 18 Feb 2016 05:56:04 PM UTC, comment #14: 

It looks OK to me except that the note about

These need to be done before AC_PROG_CPP and AC_PROG_CXXPP since the later will make use of the defined CXX and CC.

should be in the configure.ac file.  Probably no one will see it in the commit message.  Similarly, you might add a note at the top of the .m4 files about the origin of those files.

John W. Eaton <jwe>
Project Administrator
Thu 18 Feb 2016 05:30:28 PM UTC, comment #13: 

> Why does this macro use _AC_COMPILE_IFELSE instead of AC_COMPILE_IFELSE?


That's how it is in autoconf and I wanted to keep it as similar as possible to get no surprises once we can actually use the autoconf macros. The only difference in _AC_COMPILE_IFELSE is that it doesn't require a compiler to be selected yet.

> We normally omit the '()' when invoking autoconf macros that don't take arguments, so I see no reason to be different in this case.


Sure. I can make that change.

I found out one other change that is required. These calls must be made before AC_PROG_CXXCPP and AC_PROG_CCPP in order to include the right flags in the preprocessor.

I have attached a revised cset.

(file #36388)

Carnë Draug <carandraug>
Project Member
Thu 18 Feb 2016 05:03:57 PM UTC, comment #12: 

Why does this macro use _AC_COMPILE_IFELSE instead of AC_COMPILE_IFELSE?

We normally omit the '()' when invoking autoconf macros that don't take arguments, so I see no reason to be different in this case.

John W. Eaton <jwe>
Project Administrator
Thu 18 Feb 2016 04:33:00 PM UTC, comment #11: 

I included the source from autoconf, renamed the macros with the octave prefix to avoid any future clashes, and then the two lines required in configure.ac.

OCTAVE_PROG_CC_C11()
OCTAVE_PROG_CXX_CXX11()

It seems that such compiler flags are meant to go into CXX and CC (like AC_PROG_CC_99 does) so that's it's all that needs to be done.  This makes it optional.  If in the future we make it a requirement, both macros take a if-found and if-not-found blocks which we can use to error.

(file #36387)

Carnë Draug <carandraug>
Project Member
Thu 18 Feb 2016 12:34:14 PM UTC, comment #10: 

Turns out that autoconf will includes macros to do all of this:

  • AC_PROG_CC_C11: new macro, which AC_PROG_CC_STDC now defaults to (August 2012)

http://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=db36f6df60b6aa4e692be7163407ef83c1de1afb

  • AC_PROG_CXX: Add checks for C++11, C++98TR1 and C++98 (January 2013)

http://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=bd79b51000e2fe59368c93ff463adb59852ec6e7

But although this has been added to autoconf source tree ages ago, there hasn't been a new release that includes them.

Carnë Draug <carandraug>
Project Member
Wed 17 Feb 2016 01:49:08 PM UTC, comment #9: 

I have wrote a simple AX_CC_COMPILE_STDC based on the c++ version that I plan on submitting to autoconf archive.

(file #36363)

Carnë Draug <carandraug>
Project Member
Tue 16 Feb 2016 08:38:39 PM UTC, comment #8: 

Speaking with Mike Miller on IRC, we came up with the following solution:

octave_save_CXXFLAGS=$CXXFLAGS
AX_CXX_COMPILE_STDCXX([11], [ext], [optional])
CXX11_SWITCH=$(echo "$CXXFLAGS" | $SED "s,^$octave_save_CXXFLAGS ,,")
XTRA_CXXFLAGS="$XTRA_CXXFLAGS $CXX11_SWITCH"

This has the following properties:

  1. whatever CXXFLAGS we get at the start are used by AX_CXX_COMPILE_STDCXX (because they affect the result of the test, e.g., maybe there's no need to add any flag at all)
  2. whatever flag AX_CXX_COMPILE_STDCXX adds to CXXFLAGS is kept there for future configure checks
  3. only the new flags added by AX_CXX_COMPILE_STDCXX get added to XTRA_CXXFLAGS

If this is good, then I will work in a AX_CC_COMPILE_STDCC macro

Carnë Draug <carandraug>
Project Member
Mon 15 Feb 2016 09:53:41 AM UTC, comment #7: 

Sorry, I missed a line, should have been

octave_save_CXXFLAGS=$CXXFLAGS
CXXFLAGS=
AX_CXX_COMPILE_STDCXX([11], [ext], [optional])
XTRA_CXXFLAGS="$XTRA_CXXFLAGS $CXXFLAGS"
CXXFLAGS=$octave_save_CXXFLAGS

Mike Miller <mtmiller>
Project Administrator
Sat 13 Feb 2016 08:04:59 PM UTC, comment #6: 

> Something like this would be cleaner.


But won't CXXFLAGS already have other flags in there? Won't that copy the other non c++11 related flags? Also, shouldn't we keep the c++11 flags on CXXFLAGS for the rest of configure?

My understanding of the multiple flag-related variavles in configure.ac (just to make sure I understand it well, I may not):

1) CXXFLAGS starts with the user defined flags. Throughout configure, flags are added to CXXFLAGS as required, and used to continue the configure checks. At the end of configure, they are restored to their initial value and then appended to the end of flags actually used during compilation.

2) XTRA_CXXFLAGS start empty. Each time a configure check adds a new flag, it is also added here (as well as CXXFLAGS). In the end, these are the start of the flags used for compilation.

> I have no objection to using AX_CXX_COMPILE_STDCXX, but it doesn't handle the C11 option that you also wanted to add.


Yes. I was thinking of writing a AX_CXX_COMPILE_STDC that follows the same design. But wanted to know if that design was worth following for us.

> Most of the complexity of AX_CXX_COMPILE_STDCXX is checking various C++11 features to make sure the compiler actually does support all of C++11, do we care about that at this point?


Well, it's already there for the taking so why not use it? And it could also be used to check for c++14 later.

Carnë Draug <carandraug>
Project Member
Sat 13 Feb 2016 01:51:39 AM UTC, comment #5: 

Something like this would be cleaner.

octave_save_CXXFLAGS=$CXXFLAGS
AX_CXX_COMPILE_STDCXX([11], [ext], [optional])
XTRA_CXXFLAGS="$XTRA_CXXFLAGS $CXXFLAGS"
CXXFLAGS=$octave_save_CXXFLAGS

I have no objection to using AX_CXX_COMPILE_STDCXX, but it doesn't handle the C11 option that you also wanted to add.

Most of the complexity of AX_CXX_COMPILE_STDCXX is checking various C++11 features to make sure the compiler actually does support all of C++11, do we care about that at this point?

Mike Miller <mtmiller>
Project Administrator
Sat 13 Feb 2016 01:33:23 AM UTC, comment #4: 

A question on design of such macro. Is something like this acceptable?

AX_CXX_COMPILE_STDCXX([11], [ext], [optional])
if test x$ac_success = xyes; then
  ## CXXFLAGS will already have $switch
  XTRA_CXXFLAGS="$XTRA_CXXFLAGS $switch"
fi

I don't really like it because it relies on undocumented internals of the macro, but it's short, simple, and allow us to retain the existing macro from the autoconf archive. And maybe it's ok in a macro language.

If not, then I'm not sure how to do it. Such macro needs to try several flags and I'm not sure how to return which is the right flag to use. I can see that no existing macro changes XTRA_CXXFLAGS so I'm guessing I shouldn't do it here either.

Carnë Draug <carandraug>
Project Member
Fri 12 Feb 2016 11:24:57 PM UTC, comment #3: 

So I will work on a patch similar to ax_cxx_compile_stdcxx so it:

1) tries to build a program (maybe it doesn't need the c++11 flag (seems like gcc 6 sets c++14 by default for example.
2) if that doesn't work tries the flags in the same order
3) not declare any symbols

Should I copy the macro from autoconf archive and then make changes under the same macro name, or change the name to avoid confusions. I'll guess the later.

Also, should it go into acinclude (like all Octave macros) or into its own separate file (since it will be so heavily based on ax_cxx_compile_stdcxx ?

Carnë Draug <carandraug>
Project Member
Thu 11 Feb 2016 06:47:40 PM UTC, comment #2: 

To the statement "it's ok to use as long as it's not a requirement", I'd like to add that it is not OK to use C++11 for basic functionality that could be implemented without C++11 features.  So the change I proposed for acos is OK because it allows acos to work even if C11 and C++11 features are not available.  But that means more work for us now because to use the shiny new C++11 feature, we still need to provide a fallback method.

On the brighter side, I think we may be close to being able to just use at least some C++11 features without requiring fallback methods if they are supported by reasonably recent versions of GCC and Clang.  With mxe-octave, we have a way to build Octave with a current version of GCC when the system compiler is not sufficient.

Perhaps after 4.2 (or will it be 5.0?) we could start using C++11 features and if people want to build Octave with ten year old compilers, we can tell them that they either upgrade their compiler (possibly using mxe-octave to do it) or build an old version of Octave with their old compiler.

John W. Eaton <jwe>
Project Administrator
Tue 09 Feb 2016 11:00:10 PM UTC, comment #1: 

Have you looked at the autoconf-archive AX_CXX_COMPILE_STDCXX_11 macro? I don't think we should use it verbatim, since it also tests for a huge set of C++11 language features, but it has some ideas you might want to incorporate.

http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_cxx_compile_stdcxx.m4

E.g. you might want to loop over all possible flags in this order: -std=gnu++11, -std=gnu++0x, -std=c++11, -std=c++0x. They also suggest other flags for HP and Cray compilers (do we even support those?)

And the equivalent set for C11 would be: -std=gnu11, -std=c11, -std=gnu1x, -std=c1x. Do you want to fall back to flags to enable C99 if C11 is not supported?

The AC_SUBST are not needed for declaring new #define symbols, just AC_DEFINE is enough.

I'm not sure if we even want to add a conditional that says whether the compiler supports C11 or C++11 versus specific features, isn't this the same as checking for library versions that we've said we want to avoid in favor of feature tests?

For what you want to do, I would change this to just see if the compiler allows (one of) the flags to be passed and add it to the XTRA_ flags if so, but take away the HAVE_ symbols.

Then later if we decide we want to (conditionally) use specific library features, say std::atan(std::complex) or std::thread(), add a specific feature test for that function after the enabling compiler flag has already been tested for and used if possible.

Mike Miller <mtmiller>
Project Administrator
Sun 07 Feb 2016 03:01:16 PM UTC, original submission:  

It has been discussed a few times about Octave using C++11 and the decision has been that it's ok to use it as long as it's not a requirement.  However, we don't have a configure check for c++11 support.

This changes that, adding HAVE_C11 and HAVE_CXX11 symbols make it easier in the future.

There is a more complex macro on autoconf archive that actually tries several C++11 features and both gnu++11 and c++11 flags.  But I couldn't find one for C11 and we already had an octave macro to check for compiler flags.

Carnë Draug <carandraug>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #36388:  cxx11-c11-support-v2.cset added by carandraug (19KiB - application/octet-stream)
file #36387:  cxx11-c11-support.cset added by carandraug (19KiB - application/octet-stream)
file #36363:  ax_cc_compile_stdc.m4 added by carandraug (6KiB - application/x-m4)
file #36271:  c++11-support.cset added by carandraug (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by carandraug (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-26 mtmiller Carbon-CopyRemoved 80942 => -
    2016-02-18 carandraug StatusIn Progress => Done
        Open/ClosedOpen => Closed
    2016-02-18 carandraug Attached File- => Added cxx11-c11-support-v2.cset, #36388
    2016-02-18 carandraug Attached File- => Added cxx11-c11-support.cset, #36387
    2016-02-17 carandraug Attached File- => Added ax_cc_compile_stdc.m4, #36363
    2016-02-09 mtmiller StatusNone => In Progress
    2016-02-07 carandraug Attached File- => Added c++11-support.cset, #36271

    Back to the top


    Powered by Savane 3.5