Skip to content

Commit

Permalink
Bump CMAKE_CXX_STANDARD as 17 globally (#5612)
Browse files Browse the repository at this point in the history
### Description
<!-- - Describe your changes. -->
- Bump CMAKE_CXX_STANDARD as 17 globally (Windows and non-Windows).
- Update README.md accordingly.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Currently by default onnx uses CMAKE_CXX_STANDARD 17 on Windows, but by
contrast it uses CMAKE_CXX_STANDARD 14 on other platforms. It's a bit
confusing. Also it blocks onnx utilizing C++ 17 features. Trying this PR
to verify ONNX CI Pipelines with this update. If no one has other
concern, we will target next release (possibly 1.16) to include this
CMAKE_CXX_STANDARD bump. Users who build their onnx from their own are
still able to specify their own CMAKE_CXX_STANDARD version.

Anyone has any question/concern, feel free to chime in. Thanks!

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at a1c07ad</samp>

### Summary
🧹🔄📝

<!--
1. 🧹 - This emoji represents the simplification and cleanup of the CMake
logic for setting the C++ standard version.
2. 🔄 - This emoji represents the consistency and alignment of the C++
standard version across platforms and the possibility of overriding it
with a custom value.
3. 📝 - This emoji represents the documentation update in the README.md
file.
-->
Simplify and unify the C++ standard version setting for ONNX. Update the
`README.md` file accordingly.

> _`CMAKE_CXX_STANDARD`_
> _Simpler and consistent now_
> _Autumn of cleanup_

### Walkthrough
* Simplify CMake logic for setting C++ standard version
([link](https://github.com/onnx/onnx/pull/5612/files?diff=unified&w=0#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL58-R59))
* Update README documentation to match CMake changes
([link](https://github.com/onnx/onnx/pull/5612/files?diff=unified&w=0#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5L114-R114))

---------

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
  • Loading branch information
3 people committed Oct 26, 2023
1 parent 21bff4e commit b5e1378
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 8 deletions.
7 changes: 0 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,8 @@ else()
set(Protobuf_USE_STATIC_LIBS ON)
endif()

# Required to use C++17 or higher on Windows
# For other platforms, set C++14 as standard for the whole project
if(NOT DEFINED CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 14)
endif()
if(MSVC AND CMAKE_CXX_STANDARD LESS 17)
set(CMAKE_CXX_STANDARD 17)
elseif(CMAKE_CXX_STANDARD LESS 14)
set(CMAKE_CXX_STANDARD 14)
endif()

include(GNUInstallDirs)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ conda install -c conda-forge onnx

Before building from source uninstall any existing versions of onnx `pip uninstall onnx`.

c++17 or higher C++ compiler version is required to build ONNX from source on Windows. For other platforms, please use C++14 or higher versions.
c++17 or higher C++ compiler version is required to build ONNX from source. Still, users can specify their own `CMAKE_CXX_STANDARD` version for building ONNX.

If you don't have protobuf installed, ONNX will internally download and build protobuf for ONNX build.

Expand Down

0 comments on commit b5e1378

Please sign in to comment.