search

kidoman / rays

Ray tracing based language benchmarks
https://kidoman.com/programming/go-getter.html
95 stars 23 forks source link

C++ version is also compilable with GCC 4.6 #12

Closed tkalbitz closed 10 years ago

tkalbitz commented 10 years ago

My old compiler don't understand this notation. This is also a good solution and works with old compilers.

t-mat commented 10 years ago

Does this PR mean to break prerequisite ?

I'm not offended and I think it's good PR for many peoples especially who use MSVC. But this subject seems to break prerequisite.

tkalbitz commented 10 years ago

Which prerequisite does the PR break? That the code use the latest gimmicks from C++0x or C++11? Then maybe. As stated here: http://en.cppreference.com/w/cpp/language/type_alias "using" is (more or less) an alias for typedef. There are no implications for speed or code correctness except that GCC 4.6 can compile the code.

My intention is not to break anything or offend anyone. I'm running an Ubuntu LTS with GCC 4.6 which does not understand the "using" statement and I needed a working C++ version.

t-mat commented 10 years ago

First of all, I'm not opposed to this PR. It's good one !

My point is your subject itself. Should we keep the code "compilable with GCC 4.6" ? How about Clang-x.y, icc-x.y, MSVC-x.y ? Basically we should follow the prerequisite in readme.md. If you don't have gcc 4.8.1 or later you should install it. But it seems some discussion is needed. I'm waiting for @kid0m4n's opinion.

Which prerequisite does the PR break?

"* gcc 4.8.1 or later" in readme.md.

That the code use the latest gimmicks from C++0x or C++11?

No, absolutely. And I agree with you that typedef is good for wider compiler support.

tkalbitz commented 10 years ago

My apologies for this misunderstanding. You're absolutely right with the prerequisite and the GCC version. Let's wait for @kid0m4n .

kidoman commented 10 years ago

Although we should aim and follow forward thinking, if such a simple change can get the code working on a wider range of compilers, it is a wise change

But its definitely a case-by-case decision

@t-mat The prerequisites were a way for me to make it easier for people to get started. We definitely need to start thinking of scenarios where the same code will get compiled using different compilers (like the very ones you mentioned.) I will open a issue to discuss this matter further

kidoman commented 10 years ago

Good to merge then?

tkalbitz commented 10 years ago

Fine for me.

t-mat commented 10 years ago

Nice decision. Thanks !