-
Notifications
You must be signed in to change notification settings - Fork 15
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
Switch default standard to C++20 (+gnu extensions) #26
base: main
Are you sure you want to change the base?
Conversation
I see it's already the case here... but why would NDK builds use a different default standard version than other platforms? That seems prone to confusing users. As a user I would expect bazel to be consistent across targets (even if it's consistently no opinion and punts to the compiler to decide, which is what ~every other build system I've used does). |
Hey @DanAlbert! Oh! On me if there's already another Bazel standard. I'd proposed this because I think I'd seen inconsistent settings already elsewhere and I figured most users would probably want to be up-to-date by default. 17 seemed like it was probably the latest when the rules were written, so I figured I'd give it an update. Anyway, zero pressure if you want to leave it--or remove it. Cheers, |
I don't know if there is another bazel standard (I'm not a bazel user, I just lurk here because I work on the NDK). If bazel doesn't aim to be consistent across targets this PR seems fine :) |
I remember from seeing you on other issues over there, like android/ndk#837 Hardly lurking! Thanks so much for all you do, and for caring enough to be here, too! |
Tagging @ahumesky, since it seems like we'll want him to make the directional call here. |
I guess a quick pitch for the latest would be: The benefit of having people be up to date by default is that they can use the broadest set of (stable) language features without having to do additional configuration. But that's no different than just recommending people use the latest stable version of anything that's (mostly) backwards compatible:) |
(@ahumesky, could we get your call on this? Totally find to close or to change over to the compiler default instead; just thought this might be the best default option for the reasons above.) |
As in #5, I'd like to propose that we enable C++20 by default, since I know the NDK compilers support it.
No pressure. It just seems like a good opportunity before the interface hardens.
Also, while we're all here: Is this code shared with blaze still (hopefully, so improvements are shared both ways)? If so maybe there's an equivalent of has_cxx17_headers that should be enabled? (From a quick search, that seemed to be a blaze-specific feature, since I didn't see the string in Bazel, but maybe that's wrong.)
[Self note: Maybe also strip X-ray section if no longer relevant. Seems unlikely people will be using this with NDK r15...]