Re: [PATCH] Set -std=c++11 flag for g++

2016-06-22 Thread Thiago Macieira
On quarta-feira, 22 de junho de 2016 22:47:23 PDT Anton Lundin wrote:
> @@ -73,6 +73,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 ")
>  elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
> +   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
>  elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")

You want to set it for AppleClang and Intel branches too. Do we support 
building with MSVC at all?

Also note that GCC 6 defaults to C++14, so effectively this change 
"downgrades" the default support. But that might be a good idea, to make 
people using the latest GCC not add features that older compilers can't grok.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PATCH] Set -std=c++11 flag for g++

2016-06-22 Thread Dirk Hohndel

> On Jun 22, 2016, at 2:49 PM, Thiago Macieira  wrote:
> 
> On quarta-feira, 22 de junho de 2016 22:47:23 PDT Anton Lundin wrote:
>> @@ -73,6 +73,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
>>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 ")
>> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
>>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
>> +   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
>> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")
> 
> You want to set it for AppleClang and Intel branches too. Do we support 
> building with MSVC at all?

No. MinGW, gcc and clang

> Also note that GCC 6 defaults to C++14, so effectively this change 
> "downgrades" the default support. But that might be a good idea, to make 
> people using the latest GCC not add features that older compilers can't grok.

yes.

/D
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PATCH] Set -std=c++11 flag for g++

2016-06-22 Thread Thiago Macieira
On quarta-feira, 22 de junho de 2016 14:59:49 PDT Dirk Hohndel wrote:
> > On Jun 22, 2016, at 2:49 PM, Thiago Macieira  wrote:
> > 
> > On quarta-feira, 22 de junho de 2016 22:47:23 PDT Anton Lundin wrote:
> >> @@ -73,6 +73,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL
> >> "AppleClang")
> >> 
> >>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 ")
> >> 
> >> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> >> 
> >>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
> >> 
> >> +   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
> >> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")
> > 
> > You want to set it for AppleClang and Intel branches too. Do we support
> > building with MSVC at all?
> 
> No. MinGW, gcc and clang

Ok, so the "Intel" branch above must be on Mac and Linux, which means it takes 
the same syntax as GCC and Clang.

If it were on Windows, the syntax would be different, as it tries to be 
compatible with MS's compiler.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PATCH] Set -std=c++11 flag for g++

2016-06-23 Thread Anton Lundin
On 22 June, 2016 - Thiago Macieira wrote:

> On quarta-feira, 22 de junho de 2016 14:59:49 PDT Dirk Hohndel wrote:
> > > On Jun 22, 2016, at 2:49 PM, Thiago Macieira  wrote:
> > > 
> > > On quarta-feira, 22 de junho de 2016 22:47:23 PDT Anton Lundin wrote:
> > >> @@ -73,6 +73,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL
> > >> "AppleClang")
> > >> 
> > >>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 ")
> > >> 
> > >> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> > >> 
> > >>set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
> > >> 
> > >> +   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
> > >> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")
> > > 
> > > You want to set it for AppleClang and Intel branches too. Do we support
> > > building with MSVC at all?
> > 
> > No. MinGW, gcc and clang
> 
> Ok, so the "Intel" branch above must be on Mac and Linux, which means it 
> takes 
> the same syntax as GCC and Clang.
> 
> If it were on Windows, the syntax would be different, as it tries to be 
> compatible with MS's compiler.
> 

Okay. So we're almost back to what it looked before 4ddf4f6d ?


Dirk: Are you how okay with c++11 code being allowed in Subsurface or
at least to allow us to use Qt5.7?


//Anton


-- 
Anton Lundin+46702-161604
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PATCH] Set -std=c++11 flag for g++

2016-06-23 Thread Dirk Hohndel

> On Jun 23, 2016, at 12:20 PM, Anton Lundin  wrote:
> 
> On 22 June, 2016 - Thiago Macieira wrote:
> 
>> On quarta-feira, 22 de junho de 2016 14:59:49 PDT Dirk Hohndel wrote:
 On Jun 22, 2016, at 2:49 PM, Thiago Macieira  wrote:
 
 On quarta-feira, 22 de junho de 2016 22:47:23 PDT Anton Lundin wrote:
> @@ -73,6 +73,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL
> "AppleClang")
> 
>   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 ")
> 
> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> 
>   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
> 
> +   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
> elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")
 
 You want to set it for AppleClang and Intel branches too. Do we support
 building with MSVC at all?
>>> 
>>> No. MinGW, gcc and clang
>> 
>> Ok, so the "Intel" branch above must be on Mac and Linux, which means it 
>> takes 
>> the same syntax as GCC and Clang.
>> 
>> If it were on Windows, the syntax would be different, as it tries to be 
>> compatible with MS's compiler.
>> 
> 
> Okay. So we're almost back to what it looked before 4ddf4f6d ?
> 
> 
> Dirk: Are you how okay with c++11 code being allowed in Subsurface or
> at least to allow us to use Qt5.7?

No, careful. I’m grudgingly accepting the fact that we need to compile with
the -std=c++11 flag in order to build against Qt5.7. I am NOT ok with 
including C++11 features in the code. I’d prefer my code to be readable. 
Or to be more explicit - I don’t understand how the new features work, I 
can’t parse the syntax, and if there is one thing that I have learned over 
the past year it’s that I’m on my own when it comes to maintaining this. 
So code that I don’t understand is not welcome.

/D
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PATCH] Set -std=c++11 flag for g++

2016-06-23 Thread Thiago Macieira
On quinta-feira, 23 de junho de 2016 12:42:36 PDT Dirk Hohndel wrote:
> No, careful. I’m grudgingly accepting the fact that we need to compile with
> the -std=c++11 flag in order to build against Qt5.7. I am NOT ok with 
> including C++11 features in the code. I’d prefer my code to be readable. 
> Or to be more explicit - I don’t understand how the new features work, I 
> can’t parse the syntax, and if there is one thing that I have learned over 
> the past year it’s that I’m on my own when it comes to maintaining this. 
> So code that I don’t understand is not welcome.

Of course. Unreadable code is never welcome and "unreadable" is quite 
subjective.

We're having similar discussions in Qt itself as to how much of C++11 is 
acceptable. For example, the "Almost Always Auto" (AAA) suggestions have been 
denied and we'll continue to name our variables with the correct type, with a 
few exceptions.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface