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

Add missing -std=c++11 flag when using non-appleclang compiler on macosx #2168

Merged
merged 4 commits into from
Nov 8, 2018
Merged

Add missing -std=c++11 flag when using non-appleclang compiler on macosx #2168

merged 4 commits into from
Nov 8, 2018

Conversation

ajaypanyala
Copy link
Contributor

When using non-appleclang compiler on macosx, i.e when using upstream (llvm.org) Clang,

if("${CMAKE_VERSION}" VERSION_GREATER 3.1.0)
  set(CMAKE_CXX_STANDARD 11)
  set(CMAKE_CXX_STANDARD_REQUIRED ON)
 ....

does not add -std=c++11 to the list of compile flags when building the Cpp runtime sources causing the build to fail. This happens when I am using any cmake version >= 3.1.

An alternative fix to the one suggested in this PR is to cmake_minimum_required (VERSION 3.0) instead of 2.8. Not sure why exactly, but the setting minimum version to 3.0 fixes the missing c++11 flag problem (i.e. I would not need to explicitly add the flag as done in this PR).

Copy link
Member

@mike-lischke mike-lischke left a comment

Choose a reason for hiding this comment

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

OK, let's go with that explicit flag then. No need to raise the required CMake version. Please remove that min requirement line.

@ajaypanyala
Copy link
Contributor Author

Removed the comment. Thanks!

@ajaypanyala
Copy link
Contributor Author

@mike-lischke Could you please restart the one testcase that failed. From the build log, it's not clear why it fails.

@parrt parrt merged commit 441c407 into antlr:master Nov 8, 2018
@parrt parrt added this to the 4.7.2 milestone Nov 8, 2018
@ajaypanyala ajaypanyala deleted the cppruntime-non-appleclang-macosx branch November 8, 2018 21:56
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.

3 participants