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

Raising compiler minimum requirement to use some C++11 features? #4537

Closed
ocornut opened this issue Sep 13, 2021 · 16 comments
Closed

Raising compiler minimum requirement to use some C++11 features? #4537

ocornut opened this issue Sep 13, 2021 · 16 comments

Comments

@ocornut
Copy link
Owner

ocornut commented Sep 13, 2021

I am considering upping the minimum compiler requirements to use some C++ features.
Right now I'm mostly interested in using range-based for loops (for (SomeType* node : containers) {})

The minimum requirement would be raised to:

You can upvote this and I do expect a majority of people to be ok with this, but mostly I am interested in seeing if there are any strong argument AGAINST this.

(PS: we will keep avoiding the use of any C++ standard library header file. Only looking to use C++ core language feature)

@bkaradzic
Copy link
Contributor

No objection! You could also bump it to C++14 (a lot of #define could be changed to be constexpr, and similar with a lot of inline functions) since it's been 5 years since standard is released, all compilers caught up with it, and there is enough knowledge about which newly released features are worth using.

@dos1
Copy link
Contributor

dos1 commented Sep 13, 2021

but mostly I am interested in seeing if there are any strong argument AGAINST this

There's ABI compatibility to consider, but from a quick glance it doesn't seem like there's something relevant to ImGui there: https://gcc.gnu.org/wiki/Cxx11AbiCompatibility

Also, not sure how strong that argument is, but checking the platforms I build my stuff for it seems like Maemo 5 would be affected as that has GCC 4.2.1 in its SDK and so far ImGui worked fine there. I'd probably be fine with disabling ImGui support on that platform though. Bumping to C++14 would be more problematic.

@jeremyong-az
Copy link

jeremyong-az commented Sep 13, 2021

I would actually consider going past C++11 as well, frankly to C++17 (gives you structured bindings, and even some optimization opportunities like guaranteed copy elision, etc.). Given that you aren't using any STL structures (that may have broken ABI compatibility), it still doesn't restrict someone from compiling their own code with a lower standard version but using your interfaces.

(note: Amazon employee, but this is a personal opinion)

@bkaradzic
Copy link
Contributor

@dos1

Also, not sure how strong that argument is, but checking the platforms I build my stuff for it seems like Maemo 5 would be affected as that has GCC 4.2.

C'mon, even when it was produced N900 wasn't very popular. Maemo was abandoned 10 years ago...

Maybe it would be the best is to create another official release pre-update to C++11 or above. And then mark it clearly so people who use it on obsolete platforms could just update to that version.

@phaseq
Copy link

phaseq commented Sep 14, 2021

I'm part of a team that maintains software for some widely-deployed CNC machines. The controllers of many of those machines run on Windows CE/ARM, and the newest compiler for that platform is VS2008. We use ImGui for a debug interface.

It is unlikely that those machines disappear in the next 10 years, since they have an average lifetime of >20 years. For me it would be fine to stick to an older version of ImGui though. I think it would be helpful to document the release where support was dropped.

@iUltimateLP
Copy link

For me, personally, C++11 is fine. I am working with the Unreal Engine, which itself uses C++11, so anything past that wouldn't work out for me - and newer C++ versions didn't add anything useful anyway imo.

@jeremyong-az
Copy link

jeremyong-az commented Sep 14, 2021

For me, personally, C++11 is fine. I am working with the Unreal Engine, which itself uses C++11, so anything past that wouldn't work out for me - and newer C++ versions didn't add anything useful anyway imo.

This is not true. Unreal Engine has used C++14 for a while with an option to enable 17. From my memory of the period where I worked with the UE codebase, plenty of features in post-C++11 standards were used (e.g. capture initialization). Also, bear in mind that internal usage of C++17 doesn't preclude usage of the library from code that is C++14 or C++11 even.

@ocornut
Copy link
Owner Author

ocornut commented Sep 14, 2021

Thank you for your feedback!

Likely I am going to mark 1.85 as "last version supporting VS2008/VS2010" and then upgrade afterwards.

I can't provide long term maintenance of old tech (especially as none of the corporate users who may be affected are contributing to the project). It's just healthier moving onward and I've confirmed it with specific users aside of this thread.

@mnesarco
Copy link

It would be great to have constexpr constructors for ImVec2 and ImVec4.

@rokups
Copy link
Contributor

rokups commented Sep 16, 2021

Note that we can support constexpr without increasing standard requirement to C++14 by using preprocessor. People using sufficiently new standard version would automatically take advantage of that, while people stuck with older standard version would not feel a difference.

@metarutaiga
Copy link

VC2008 is need to modify some code because the stdint.h is not available.

And the uint32_t need to change to unsigned int.

imgui/imgui.cpp

Line 1654 in 677fe33

static const uint32_t mins[] = { 0x400000, 0, 0x80, 0x800, 0x10000 };

imgui/imgui.cpp

Lines 1672 to 1675 in 677fe33

*out_char = (uint32_t)(s[0] & masks[len]) << 18;
*out_char |= (uint32_t)(s[1] & 0x3f) << 12;
*out_char |= (uint32_t)(s[2] & 0x3f) << 6;
*out_char |= (uint32_t)(s[3] & 0x3f) << 0;

Otherwise, I played the older Visual C++ Compiler 2008 for RISC, I compiled imgui is succeed.

734720 Sep 30 22:01 imgui.Release.mips16.dll
715264 Sep 30 22:01 imgui.Release.sh4.dll
814592 Sep 30 22:00 imgui.Release.thumb.dll
716800 Sep 30 22:12 imgui.Release.x86.dll

@bkaradzic
Copy link
Contributor

@metarutaiga You should use msinttypes: https://github.com/chemeris/msinttypes when using VS2008.

@TerensTare
Copy link

Not sure if C++11 is adapted yet or it's out of plans for now (didn't find any note on the release notes of v1.85), but I just wanted to ask for making the enums scoped when that happens (ie. enum class instead of just enum and drop the prefix for each enum entry). I know this is huge and affects every possible codebase out there using ImGui, but I believe this improves code readability a lot (eg. ImGuiWindowFlags_NoDocking vs. ImGuiWindowFlags::NoDocking). Furthermore this removes the need for the analogue typedefs (eg. typedef int ImGuiWindowFlags).
One thing to consider is that bit operators (|, &, etc) have to be overloaded for each scoped enum but I believe it is worth the effort.

ocornut added a commit that referenced this issue Jul 6, 2022
…ditions) (#4537)

Possible since we are now C++11 + fix warning in GetNavInputAmount().
@joshnatis
Copy link

Hey, I think you guys have some out-of-date docs that say Dear IMGUI doesn't require C++11 on https://imgui-test.readthedocs.io/en/latest/:

Why using C++ (as opposed to C)?
Dear ImGui takes advantage of a few C++ languages features for convenience but nothing anywhere Boost-> insanity/quagmire. Dear ImGui does NOT require C++11 so it can be used with most old C++ compilers

I'm not sure what the source is for this site, I think it used to be based on the README but it's out of date?

@ocornut
Copy link
Owner Author

ocornut commented Aug 8, 2022 via email

ocornut added a commit that referenced this issue Sep 20, 2022
…ls to be named in debuggers. (#4921, #4537)

May affect binding generators.
BramblePie pushed a commit to BramblePie/imgui that referenced this issue Oct 26, 2022
…ls to be named in debuggers. (ocornut#4921, ocornut#4537)

May affect binding generators.
BramblePie pushed a commit to BramblePie/imgui that referenced this issue Oct 26, 2022
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
…ditions) (ocornut#4537)

Possible since we are now C++11 + fix warning in GetNavInputAmount().
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
…ls to be named in debuggers. (ocornut#4921, ocornut#4537)

May affect binding generators.
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
@ocornut
Copy link
Owner Author

ocornut commented Aug 28, 2023

FYI i switched various loops to use range-based for with 82d177c
I made some disassembly and made measurement in MSVC both Debug and Release modes, and found that in both cases the code seemed faster + the change allowed to remove the assert in operator[] so it feels advantageous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests