Skip to content
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

Use explicit flag for the specific version of c++ wer'e targeting. #9231

Merged
merged 2 commits into from Jul 13, 2020

Conversation

omoerbeek
Copy link
Member

@omoerbeek omoerbeek commented Jun 15, 2020

Short description

Been running this on my test OpenBSD/arm64 box for a while.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@@ -19,7 +19,7 @@ AM_SILENT_RULES([yes])
AC_CANONICAL_HOST
# Add some default CFLAGS and CXXFLAGS, can be appended to using the environment variables
CFLAGS="-g -O2 -Wall -Wextra -Wshadow -Wno-unused-parameter -Wmissing-declarations -Wredundant-decls $CFLAGS"
CXXFLAGS="-g -O2 -Wall -Wextra -Wshadow -Wno-unused-parameter -Wmissing-declarations -Wredundant-decls $CXXFLAGS"
CXXFLAGS="-std=c++11 -g -O2 -Wall -Wextra -Wshadow -Wno-unused-parameter -Wmissing-declarations -Wredundant-decls $CXXFLAGS"
Copy link
Collaborator

@zeha zeha Jun 15, 2020

Choose a reason for hiding this comment

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

isn't this what AX_CXX_COMPILE_STDCXX_11 is supposed to do/avoid? having both sounds bad to me, but i'm not an autoconf expert.

Copy link
Member Author

@omoerbeek omoerbeek Jun 15, 2020

Choose a reason for hiding this comment

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

Hmmm, lemme doublecheck the flags actually added.

Copy link
Contributor

@pieterlexis pieterlexis Jun 15, 2020

Choose a reason for hiding this comment

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

afaik, AX_CXX_COMPILE_STDCXX_11 checks if the flag is needed or not to support C++11 features. However, if your compiler default is to use C++14, it will happily run the compiler without -std=c++11, allowing the (accidental) use of newer C++ features

Copy link
Member Author

@omoerbeek omoerbeek Jun 15, 2020

Choose a reason for hiding this comment

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

Using the AX_CXX_COMPILE_STDCXX_11 we know the compiler supports c++11, but there are two issues:

  • an extended mode might be used. This can be fixed by using [noext] instead of [ext].
  • Depending on the compiler, it might support a newer version of c++. One of the goals of this commit is to detect (accidental) use of features > c++11. Using AX_CXX_COMPILE_STDCXX_11 does not do that.

Copy link
Collaborator

@zeha zeha Jun 15, 2020

Choose a reason for hiding this comment

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

Right. Maybe AX_CXX_COMPILE_STDCXX_11 can then just go away. (I hope compilers abort if they do not understand -std=c++11.

Copy link
Member

@rgacogne rgacogne left a comment

Looks good to me. I think we should keep AX_CXX_COMPILE_STDCXX_11 because otherwise the output message from a failing configure does not clearly state that the compiler doesn't support C++11.

@omoerbeek omoerbeek merged commit ef0e872 into PowerDNS:master Jul 13, 2020
29 checks passed
@omoerbeek omoerbeek deleted the explicit-cxx-version branch Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants