Bump CMAKE_CXX_STANDARD as 17 globally #5612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
jcwchen
added
CI pipelines
justinchuby
approved these changes
Sep 29, 2023
jcwchen
commented
Sep 29, 2023
xadupre
approved these changes
Oct 2, 2023
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know!. |
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
isdanni
pushed a commit
to isdanni/onnx
that referenced
this pull request
Nov 2, 2023
### 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
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!
🤖 Generated by Copilot at a1c07ad
Summary
🧹🔄📝
Simplify and unify the C++ standard version setting for ONNX. Update the
README.mdfile accordingly.Walkthrough