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

Compiling ... with a well-defined C++ version #11

Closed
tadu opened this issue Jul 22, 2020 · 7 comments
Closed

Compiling ... with a well-defined C++ version #11

tadu opened this issue Jul 22, 2020 · 7 comments

Comments

@tadu
Copy link

tadu commented Jul 22, 2020

The source currently uses very, very recent C++ constructs, requiring really, really new compilers. I'm not sure -std=c++2a actually means a released C++ version. To allow other people to compile this, I'd reduce the requirements to C++17. This is provided by clang++-6.0 and g++-8 from Ubuntu 18. Patch attached, including simple Makefile.

@tadu
Copy link
Author

tadu commented Jul 22, 2020

diff-reduce-to-c++17.txt

@bepaald
Copy link
Owner

bepaald commented Jul 30, 2020

The source currently uses very, very recent C++ constructs, requiring really, really new compilers. I'm not sure -std=c++2a actually means a released C++ version.

The current, stable, released version of gcc is 10.2. The c++2a flag was already present in gcc 8, in fact, it has by now been deprecated and replaced with std=c++20, I just keep it at 2a for compatibility with gcc 9, the previous version of gcc which was released over 14 moths ago and I am also still supporting.

Generally I really dislike having to change my code to accommodate people who can't be bothered to update their software. I also have no love for distro's with lazy updating policies. If people prefer running those non-updating distro's I think they should be prepared to compile necessary software (newer gcc versions) themselves and not make it the developer's problem.

Sorry for the ranty reply, I just wanted to have said that.

But, having said that, I am strongly considering this and leaning towards also supporting gcc 8 for a little while longer. I will push some changes in the near future to make it work.

I'm not taking the Makefile just yet, but I will take a look at it to try and learn from it (I don't understand Makefiles at all, I have no experience with them). The currently provided buildscript is auto-generated, and I'll see if I can get my build-tool to also auto-generate a Makefile, the possibility of a parallel build is very appealing.

Thanks!

@bepaald
Copy link
Owner

bepaald commented Jul 31, 2020

Done! Should be resolved with fbea5b1

@bepaald bepaald closed this as completed Jul 31, 2020
@tadu
Copy link
Author

tadu commented Aug 1, 2020

| Sorry for the ranty reply, I just wanted to have said that.

In general I am with you that people should use proper tools. Also, this is your private project, so of course it is your choice to require what you want. Even requiring a compiler using a compiler flag that selects an unfinished C++ standard. Or expecting users to compile a compiler first that your bleeding edge distro happens to be using. Or expecting users to install a docker image with the right compiler to be able to compile a simple project. When fixing the code is easier than compiling a compiler, or installing a docker image.

However, if you goal is to make it not unnecessarily complicated for other people to build your code, then supporting compiler versions in common use proves helpful. There were only two kinds of issues not covered by C++17 (which is in Ubuntu 18.04 LTS and other distros): range based loops with additional initializers (which is a bit of syntactic suger that causes no pain not to use for a while longer - and using #ifdefs to compile it only when supported is making it even less sugarly); and the starts_with() method (which you hid in a #define). Is that worth the hassle using C++20?

Thanks for making it work for older gcc. What is bad in your fix, though, is that it is dependant on certain compiler versions. Test features, not browsers^h^h^h^h^h^h^h^hcompilers. It would be better to check __cplusplus version:

cplusplus.txt

So #ifdef __cplusplus >= 201709L would be better. Or simply #ifdef __cplusplus >= 202002L for C++20 syntax.

Using __cplusplus has the advantage that clang++-10 -std=c++17 works, too.

Regarding the Makefile: I made it a bit more complex by putting object files into a separate directory per compiler; that is not really necessary. A more simple version, using variable names from GNU make default rules, could be this:

Makefile.txt

@bepaald
Copy link
Owner

bepaald commented Aug 17, 2020

Or expecting users to compile a compiler first that your bleeding edge distro happens to be using. Or expecting users to install a docker image with the right compiler to be able to compile a simple project. When fixing the code is easier than compiling a compiler, or installing a docker image.

Obviously I do not expect people to compile a compiler. Also I do not consider current, stable, released versions of software 'bleeding edge', it was never my intention for people to install development or beta versions. I even supported the previous released versions of gcc and clang (9), how can that be called bleeding edge? 'Fixing code' implies broken code, this was not broken. And I doubt changing the code was much easier than installing an up-to-date compiler, even for Ubuntu users (https://askubuntu.com/questions/1192955/how-to-install-g-10-on-ubuntu-18-04).

Obviously, when someone is happy with their old distribution, and don't want to update, that is up to them. Most users will not be compiling their own software so the gcc version does not matter, they might not even have a compiler installed at all. If however you insist on compiling your own software (especially current, in-development software like this) and then decide to stick with clang 6 for example, that does not seem like a well thought out decision to me. If you don't use the internet, your browser version doesn't matter, but a heavy internet user going through the mozilla archives to look for firefox 3.5 (because it's more stable?) and then complain that websites don't look right seems nuts in my opinion.

Anyway - even though I don't fully agree with it - I obviously do see your point, which is why I have now got about six different compilers installed and the code will compile with gcc 8 and clang 6 (at least), I hope that is good enough for everyone. If I hadn't already written the starts_with type code, I might have held off on putting it in, but now that it's there I just can not get myself to go back to the horrible looking, inefficient substr(0, string.length()) == string. Also the ugliness of the preprocessor #if's will remind me to remove that stuff sometime in the future.

Thanks for making it work for older gcc. What is bad in your fix, though, is that it is dependant on certain compiler versions. Test features, not browsers^h^h^h^h^h^h^h^hcompilers. It would be better to check __cplusplus version:

Yeah, that is a lot nicer. I have just pushed a change for that.

Regarding the Makefile: I made it a bit more complex by putting object files into a separate directory per compiler; that is not really necessary. A more simple version, using variable names from GNU make default rules, could be this:

Thanks! I've been messing around with your Makefile, and looking at stackoverflow and many other makefile on github to get some version checking. Currently I came up with this

Makefile.txt

It seems this makefile will compile the program with gcc 8-10 and clang 6-10 without warnings, do you have any comments on it? It will be a while before I actually add it to the project as I will first get my build tool to output this format so I don't have to maintain it manually.

Thanks!

@tadu
Copy link
Author

tadu commented Aug 17, 2020

I think we disagree on some fundamental points. I think it isn't worth the hassle to be able to use more efficient code on c++20 for a tool that you use only once in a long while, and that it compiles at all is more important, while you need to force c++20 whenever you can. I also do not believe that, with the existance of the ancient autotools and the more modern cmake, any other hand grown tool on top of make is needed. I admit I made my Makefile a bit unnecessary complicated before because I wanted to compile in a separate directory, but I think simplicity is king here. Your Makefile now overwrites settings given on the command line to make; it essentially does compiler detection (which auto* and cmake would do). That might make sense in a build script, but not in a Makefile. But that's just my 2¢ (you asked for it), don't listen to it.

Given that the README now gives everybody a command line to compile it even with c++17 (thanks for that), there's ultimately no strict requirement for a Makefile anymore - my and hopefully other's itch to make it compile is scratched by now. I don't see any point to argue any further. Do as you must do.

@bepaald
Copy link
Owner

bepaald commented Aug 30, 2020

Ok, I had a feeling I was stepping outside the scope of normal makefile functions, I think I'll just leave it for now. I might expand the options in the script a bit in the future if that's a better place for this sort of thing.

Thanks for the feedback!

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

No branches or pull requests

2 participants