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

Cpp target: allow building runtime with newer C++ standards #3071

Merged
merged 4 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion runtime/Cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,13 @@ endif()

# Initialize CXXFLAGS.
if("${CMAKE_VERSION}" VERSION_GREATER 3.1.0)
set(CMAKE_CXX_STANDARD 11)
if(NOT DEFINED CMAKE_CXX_STANDARD)
# only set CMAKE_CXX_STANDARD if not already set
# this allows the standard to be set by the caller, for example with -DCMAKE_CXX_STANDARD:STRING=17
set(CMAKE_CXX_STANDARD 11)
endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
Copy link
Member

Choose a reason for hiding this comment

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

Why would you want to disable CXX extensions? Is there a rational behind this decision?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The runtime doesn't need/builds without GNU C++ Extensions.
(And that's a good thing, because e.g. Clang might not support (all) the GNU C++ extensions).
So I feel like it's good to make it explicit, but I'm ok with reverting the change if you want.

else()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -std=c++11")
Expand Down
7 changes: 3 additions & 4 deletions runtime/Cpp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ This folder contains the C++ runtime support for ANTLR. See [the canonical antl
## Authors and major contributors

ANTLR 4 is the result of substantial effort of the following people:

* [Terence Parr](http://www.cs.usfca.edu/~parrt/), parrt@cs.usfca.edu
ANTLR project lead and supreme dictator for life
[University of San Francisco](http://www.usfca.edu/)
* [Sam Harwell](http://tunnelvisionlabs.com/)
* [Sam Harwell](http://tunnelvisionlabs.com/)
Tool co-author, Java and C# target)

The C++ target has been the work of the following people:
Expand Down Expand Up @@ -40,6 +40,7 @@ The minimum C++ version to compile the ANTLR C++ runtime with is C++11. The supp
Include the antlr4-runtime.h umbrella header in your target application to get everything needed to use the library.

If you are compiling with cmake, the minimum version required is cmake 2.8.
By default, the libraries produced by the CMake build target C++11. If you want to target a different C++ standard, you can explicitly pass the standard - e.g. `-DCMAKE_CXX_STANDARD=17`.

#### Compiling on Windows with Visual Studio using he Visual Studio projects
Simply open the VS project from the runtime folder (VS 2013+) and build it.
Expand Down Expand Up @@ -69,5 +70,3 @@ If the CMake variable 'ANTLR4_INSTALL' is set, CMake Packages will be build and
They expose two packages: antlr4_runtime and antlr4_generator which can be referenced to ease up the use of the
ANTLR Generator and runtime.
Use and Sample can be found [here](cmake/Antlr4Package.md)


36 changes: 18 additions & 18 deletions runtime/Cpp/cmake/Antlr4Package.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@

## The `antlr4-generator` Package

To use the Package you must insert a
To use the Package you must insert a
```cmake
find_package(antlr4-generator REQUIRED)
```
line in your `CMakeList.txt` file.

The package exposes a function `antlr4_generate` that generates the required setup to call ANTLR for a
The package exposes a function `antlr4_generate` that generates the required setup to call ANTLR for a
given input file during build.

The following table lists the parameters that can be used with the function:

Argument# | Required | Default | Use
----------|-----------|---------|---
0 | Yes | n/a | Unique target name. It is used to generate CMake Variables to reference the various outputs of the generation
Expand Down Expand Up @@ -42,7 +42,7 @@ Output variable | Meaning
```cmake
# generate parser with visitor classes.
# put the classes in C++ namespace 'antlrcpptest::'
antlr4_generate(
antlr4_generate(
antlrcpptest_parser
${CMAKE_CURRENT_SOURCE_DIR}/TLexer.g4
LEXER
Expand All @@ -56,7 +56,7 @@ Output variable | Meaning

## The `antlr4-runtime` Package

To use the Package you must insert a
To use the Package you must insert a
```cmake
find_package(antlr4-runtime REQUIRED)
```
Expand Down Expand Up @@ -85,7 +85,7 @@ include_directories( ${ANTLR4_INCLUDE_DIR} )
add_dependencies( Parsertest antlr4_shared )

# add runtime to project link libraries
target_link_libraries( Parsertest PRIVATE
target_link_libraries( Parsertest PRIVATE
antlr4_shared)
```

Expand All @@ -94,22 +94,22 @@ target_link_libraries( Parsertest PRIVATE
# Bring in the required packages
find_package(antlr4-runtime REQUIRED)
find_package(antlr4-generator REQUIRED)

# Set path to generator
set(ANTLR4_JAR_LOCATION ${PROJECT_SOURCE_DIR}/thirdparty/antlr/antlr-4.9.1-complete.jar)

# generate lexer
antlr4_generate(
antlr4_generate(
antlrcpptest_lexer
${CMAKE_CURRENT_SOURCE_DIR}/TLexer.g4
LEXER
FALSE
FALSE
"antlrcpptest"
)

# generate parser
antlr4_generate(
antlr4_generate(
antlrcpptest_parser
${CMAKE_CURRENT_SOURCE_DIR}/TParser.g4
PARSER
Expand All @@ -119,18 +119,18 @@ target_link_libraries( Parsertest PRIVATE
"${ANTLR4_TOKEN_FILES_antlrcpptest_lexer}"
"${ANTLR4_TOKEN_DIRECTORY_antlrcpptest_lexer}"
)

# add directories for generated include files
include_directories( ${PROJECT_BINARY_DIR} ${ANTLR4_INCLUDE_DIR} ${ANTLR4_INCLUDE_DIR_antlrcpptest_lexer} ${ANTLR4_INCLUDE_DIR_antlrcpptest_parser} )

# add generated source files
add_executable( Parsertest main.cpp ${ANTLR4_SRC_FILES_antlrcpptest_lexer} ${ANTLR4_SRC_FILES_antlrcpptest_parser} )

# add required runtime library
add_dependencies( Parsertest antlr4_shared )
target_link_libraries( Parsertest PRIVATE

target_link_libraries( Parsertest PRIVATE
antlr4_shared)

```

4 changes: 4 additions & 0 deletions runtime/Cpp/cmake/ExternalAntlr4Cpp.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ if(ANTLR4_ZIP_REPOSITORY)
CMAKE_CACHE_ARGS
-DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
-DWITH_STATIC_CRT:BOOL=${ANTLR4_WITH_STATIC_CRT}
# -DCMAKE_CXX_STANDARD:STRING=17 # if desired, compile the runtime with a different C++ standard
# -DCMAKE_CXX_STANDARD:STRING=${CMAKE_CXX_STANDARD} # alternatively, compile the runtime with the same C++ standard as the outer project
INSTALL_COMMAND ""
EXCLUDE_FROM_ALL 1)
else()
Expand All @@ -104,6 +106,8 @@ else()
CMAKE_CACHE_ARGS
-DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
-DWITH_STATIC_CRT:BOOL=${ANTLR4_WITH_STATIC_CRT}
# -DCMAKE_CXX_STANDARD:STRING=17 # if desired, compile the runtime with a different C++ standard
Copy link
Member

Choose a reason for hiding this comment

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

This setting should not be done in the cmake file, but be set by the outer project which needs to build ANTLR4. As such it makes not so much sense to have this here, even if it is only a comment.

Additionally, it should be documented somewhere (readme).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, ExternalProject_Add() runs in a separate CMake instance, and does not use the same args as the outer project. So I've had to pass the build arguments explicitly here.
(The alternative seems too complicated: https://stackoverflow.com/a/48555098)

Either way, you're right, I have to update the readme - will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing the snippet above with -DCMAKE_CXX_STANDARD:STRING=${CMAKE_CXX_STANDARD} builds the runtime with the same C++ standard as the outer project. Currently, I've added it as a commented out line. Do you want me to un-comment it and make it the new default behaviour? I think the risk of breaking things is pretty small, because projects using the ExternalAntlr4Cpp build flow have a local copy of ExternalAntlr4Cpp.cmake in their source tree anyways.

# -DCMAKE_CXX_STANDARD:STRING=${CMAKE_CXX_STANDARD} # alternatively, compile the runtime with the same C++ standard as the outer project
INSTALL_COMMAND ""
EXCLUDE_FROM_ALL 1)
endif()
Expand Down
2 changes: 2 additions & 0 deletions runtime/Cpp/cmake/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ The ANTLR C++ runtime source is downloaded from GitHub by default. However, user

Visual C++ compiler users may want to additionally define `ANTLR4_WITH_STATIC_CRT` before including the file. Set `ANTLR4_WITH_STATIC_CRT` to true if ANTLR4 C++ runtime library should be compiled with `/MT` flag, otherwise will be compiled with `/MD` flag. This variable has a default value of `OFF`. Changing `ANTLR4_WITH_STATIC_CRT` after building the library may require reinitialization of CMake or `clean` for the library to get rebuilt.

You may need to modify your local copy of ExternalAntlr4Cpp.cpp to modify some build settings. For example, to specify the C++ standard to use when building the runtime, add `-DCMAKE_CXX_STANDARD:STRING=17` to `CMAKE_CACHE_ARGS`.

### Examples

To build and link ANTLR4 static library to a target one may call:
Expand Down
8 changes: 4 additions & 4 deletions runtime/Cpp/cmake/antlr4-generator.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function(antlr4_generate
else()
set(Antlr4_BuildListenerOption "-no-listener")
endif ()

if ( ( ARGC GREATER_EQUAL 5 ) AND ARGV4 )
set(Antlr4_BuildVisitorOption "-visitor")

Expand All @@ -101,15 +101,15 @@ function(antlr4_generate
else()
set(Antlr4_BuildVisitorOption "-no-visitor")
endif ()

if ( (ARGC GREATER_EQUAL 6 ) AND (NOT ${ARGV5} STREQUAL "") )
set(Antlr4_NamespaceOption "-package;${ARGV5}")

list( APPEND Antlr4_GeneratorStatusMessage " in Namespace ${ARGV5}" )
else()
set(Antlr4_NamespaceOption "")
endif ()

if ( (ARGC GREATER_EQUAL 7 ) AND (NOT ${ARGV6} STREQUAL "") )
set(Antlr4_AdditionalDependencies ${ARGV6})
else()
Expand Down Expand Up @@ -157,7 +157,7 @@ function(antlr4_generate

# export generated cpp files into list
foreach(generated_file ${Antlr4_GeneratedTargets})

if (NOT CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
set_source_files_properties(
${generated_file}
Expand Down