-
Notifications
You must be signed in to change notification settings - Fork 888
Revert back from C/C++17 to C/C++11 #3095
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
Conversation
There are older toolchains that don't support C++17 and are still used in shipping products. Rewrite hexdump.cc to not use C++17 features (std::byte and Class Template Argument Deduction). Also remove forcing of -std=libc for xtensa targets, which was added because stdlib isn't supported with -std=c++17. BUG=410831256
@ddavis-2015 It seems like these are really the only files that are needing C++17? The makefile tests are all passing with C++11, do you know what else you would have expected to break? |
We may not want to revert back to C++11 and commit to C++11 compliance going forward. That decision is not only about the code uniquely written for TFLM; we have dependencies outside our control---upstream TFLite from which we merge code regularly, libraries like flatbuffers, gemmlowp , ruy, nnlib, etc.
We took the decision to be a C++17 project in 2023 in #2060, to address an issue #2058 merging C++17 code from TFLite. See the extensive discussion of whether to switch in that issue. Makefile-based builds were caught up in #2648. Issue #2650 has some discussion. In any event, a commitment to C++11 would be a major shift of policy. It would have project-wide implications for what features we can use in our own code, and what workarounds we're committing to implement when our dependencies use non-C++11 features. |
There are older toolchains that don't support C++17 and are still used in shipping products.
Rewrite hexdump.cc to not use C++17 features (std::byte and Class Template Argument Deduction). Also remove forcing of -std=libc for xtensa targets, which was added because stdlib isn't supported with -std=c++17.
BUG=410831256