Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 3 June 2016 at 15:54, Kurt Schwehr  wrote:
> I would make some of assertions (some just restating what Even wrote)
>
> * The cost to heap allocation is a fair question (but I would suggest we
> defer the work until required)
>
> * Any work done on performance critical subroutines should be done
> carefully.

I would precise:

Any major changes that are supposed to improve performance
should be first backed up by profiling results or complexity analysis
that clearly confirm promised gains.

Such results also could be used as not-slower-than reference
in later benchmarks.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

I would make some of assertions (some just restating what Even wrote)

* The cost to heap allocation is a fair question (but I would suggest we
defer the work until required)

* Any work done on performance critical subroutines should be done
carefully.

* It would help to have a standard comment that people can add to
empirically detected performance critical sections so that we reduce the
chance of performance regressions

* It would be nice to have a very simple performance check system, but that
seems like a separate discussion that will open the whole benchmarking
can-o-worms.

* It would be great to have a set of instructions on how to instrument GDAL
to collect metrics of runs and detect hot spots with open source tools.
It's been a long time since I've done anything like that with generally
available tools.  e.g. https://sourceware.org/binutils/docs/gprof/  And I
need to look more at the gcov output.  I see that mapserver has a start
https://trac.osgeo.org/mapserver/wiki/PerformanceTesting

* The gdal wiki needs to have more of
http://erouault.blogspot.com/2016/01/software-quality-improvements-in-gdal.html
blended in

* https://en.wikiquote.org/wiki/Donald_Knuth "premature optimization is the
root of all evil (or at least most of it) in programming" :)

* I would suggest that with any guideline there will almost always be the
occasional location where that guideline does not make sense.  In that
case, I would expect that a comment would go along with the exception
explaining why it is important.

* The cost of heap versus stack is not well done in a standalone test (but
there is value in standalone examples).  Pressures on caches, TLBs, etc are
difficult to replicate in simple examples.  Moving a large allocate to the
heap helps keep the rest of the stack fast.  Think about the pages required
for the stack with multiple cores and lots of threads.
http://unix.stackexchange.com/questions/128213/how-is-page-size-determined-in-virtual-address-space

* There are also places where heap allocation is just flat out banned after
a process is initialized.  e.g. daemons like gpsd or long running realtime
systems (your car's engine controllers, spacecraft control systems, etc.).
I would argue that it is never going to be a good idea to use GDAL inside
that kind of system / process

In terms of GDAL, I haven't seen any places where heap allocation of large
objects would like be an important fraction of runtime.  I'm sure they are
there, but I would bet they are rare.

On Thu, Jun 2, 2016 at 3:29 AM, Even Rouault 
wrote:

> Le jeudi 02 juin 2016 00:46:06, Kurt Schwehr a écrit :
> >
> https://docs.google.com/document/d/1O1B7LY13L532kXcYcB2EdO65m5LOCsqaqn5R9iJ
> > fSPU/pub
> >
> > The optimized stack .o is 1248 bytes and the on the heap vector is 1600
> > bytes with gcc 4.8.  The cost of either is pretty small.  So, if there
> are
> > 100 of these in gdal, we are talking about 30-60K of extra object file
> size
> > for all the cases in GDAL.
>
> Code size is one thing, but I'd be curious to know about the speed impacts
> due
> to the heap allocation. Like time several million iterations of each way
> (and
> make sure to do something non trivial with the arrray so that the compiler
> doesn't optimize the code away). If there's no significant difference,
> then fine.
> Otherwise we'd have to be careful when the replacements are done in a
> performance sensitive routine (my feeling is that there are not so many
> such
> places)
>
> >
> > For stack usage, start thinking about tons of threads spread out over
> lots
> > of cores and think about how systems are built...  Google Compute Engine
> > has 32 core machines.  If you want to get the most out your machines at
> > scale, you want a small stack.
> >
> > int anVals[256] = {};  Does initialize everything to the default value
> > (zeros), but doesn't solve the stack issue.
> >
> > Making a little class is yet another thing for people to learn when
> vector
> > looks to me to work quite well.
> >
> > On Mon, May 9, 2016 at 1:37 PM, Andrew Bell 
> >
> > wrote:
> > > On Mon, May 9, 2016 at 2:49 PM, Mateusz Loskot 
> wrote:
> > >> Point taken.
> > >>
> > >> Although the proposal looks OK, I'd suggest to check what
> > >> assembler code generates your favourite C++ toolkit,
> > >> or at least measured times for
> > >>
> > >> int anVals[256];
> > >> memset(anVals, 0, 256*sizeof(int));
> > >
> > > Are "we" doing memset anymore in these cases?
> > >
> > > int anVals[256] = {};
> > >
> > > seems preferable
> > >
> > >> vs
> > >>
> > >> std::vector oVals(256, 0);
> > >>
> > >> and compare with:
> > >>
> > >> std::vector oVals(256, 0);
> > >
> > > Think vector is a bad solution for something that's fixed.  Just write
> > > something.  But I already suggested that and wrote something... :)
> > >
> > > Do you know why they are wedded to a 16K stack?
> > >
> > > --
> > > Andrew Bell
> > > andrew.bell...@gmail.com
> > >
> > > ___
> > > gda

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 02 juin 2016 00:46:06, Kurt Schwehr a écrit :
> https://docs.google.com/document/d/1O1B7LY13L532kXcYcB2EdO65m5LOCsqaqn5R9iJ
> fSPU/pub
> 
> The optimized stack .o is 1248 bytes and the on the heap vector is 1600
> bytes with gcc 4.8.  The cost of either is pretty small.  So, if there are
> 100 of these in gdal, we are talking about 30-60K of extra object file size
> for all the cases in GDAL.

Code size is one thing, but I'd be curious to know about the speed impacts due 
to the heap allocation. Like time several million iterations of each way (and 
make sure to do something non trivial with the arrray so that the compiler 
doesn't optimize the code away). If there's no significant difference, then 
fine. 
Otherwise we'd have to be careful when the replacements are done in a 
performance sensitive routine (my feeling is that there are not so many such 
places)

> 
> For stack usage, start thinking about tons of threads spread out over lots
> of cores and think about how systems are built...  Google Compute Engine
> has 32 core machines.  If you want to get the most out your machines at
> scale, you want a small stack.
> 
> int anVals[256] = {};  Does initialize everything to the default value
> (zeros), but doesn't solve the stack issue.
> 
> Making a little class is yet another thing for people to learn when vector
> looks to me to work quite well.
> 
> On Mon, May 9, 2016 at 1:37 PM, Andrew Bell 
> 
> wrote:
> > On Mon, May 9, 2016 at 2:49 PM, Mateusz Loskot  wrote:
> >> Point taken.
> >> 
> >> Although the proposal looks OK, I'd suggest to check what
> >> assembler code generates your favourite C++ toolkit,
> >> or at least measured times for
> >> 
> >> int anVals[256];
> >> memset(anVals, 0, 256*sizeof(int));
> > 
> > Are "we" doing memset anymore in these cases?
> > 
> > int anVals[256] = {};
> > 
> > seems preferable
> > 
> >> vs
> >> 
> >> std::vector oVals(256, 0);
> >> 
> >> and compare with:
> >> 
> >> std::vector oVals(256, 0);
> > 
> > Think vector is a bad solution for something that's fixed.  Just write
> > something.  But I already suggested that and wrote something... :)
> > 
> > Do you know why they are wedded to a 16K stack?
> > 
> > --
> > Andrew Bell
> > andrew.bell...@gmail.com
> > 
> > ___
> > gdal-dev mailing list
> > gdal-dev@lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/gdal-dev

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

https://docs.google.com/document/d/1O1B7LY13L532kXcYcB2EdO65m5LOCsqaqn5R9iJfSPU/pub

The optimized stack .o is 1248 bytes and the on the heap vector is 1600
bytes with gcc 4.8.  The cost of either is pretty small.  So, if there are
100 of these in gdal, we are talking about 30-60K of extra object file size
for all the cases in GDAL.

For stack usage, start thinking about tons of threads spread out over lots
of cores and think about how systems are built...  Google Compute Engine
has 32 core machines.  If you want to get the most out your machines at
scale, you want a small stack.

int anVals[256] = {};  Does initialize everything to the default value
(zeros), but doesn't solve the stack issue.

Making a little class is yet another thing for people to learn when vector
looks to me to work quite well.

On Mon, May 9, 2016 at 1:37 PM, Andrew Bell 
wrote:

> On Mon, May 9, 2016 at 2:49 PM, Mateusz Loskot  wrote:
>
>>
>> Point taken.
>>
>> Although the proposal looks OK, I'd suggest to check what
>> assembler code generates your favourite C++ toolkit,
>> or at least measured times for
>>
>> int anVals[256];
>> memset(anVals, 0, 256*sizeof(int));
>>
>
> Are "we" doing memset anymore in these cases?
>
> int anVals[256] = {};
>
> seems preferable
>
>
>> vs
>>
>> std::vector oVals(256, 0);
>>
>> and compare with:
>>
>> std::vector oVals(256, 0);
>>
>
> Think vector is a bad solution for something that's fixed.  Just write
> something.  But I already suggested that and wrote something... :)
>
> Do you know why they are wedded to a 16K stack?
>
> --
> Andrew Bell
> andrew.bell...@gmail.com
>
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>



-- 
--
http://schwehr.org
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Andrew Bell]

On Mon, May 9, 2016 at 2:49 PM, Mateusz Loskot  wrote:

>
> Point taken.
>
> Although the proposal looks OK, I'd suggest to check what
> assembler code generates your favourite C++ toolkit,
> or at least measured times for
>
> int anVals[256];
> memset(anVals, 0, 256*sizeof(int));
>

Are "we" doing memset anymore in these cases?

int anVals[256] = {};

seems preferable


> vs
>
> std::vector oVals(256, 0);
>
> and compare with:
>
> std::vector oVals(256, 0);
>

Think vector is a bad solution for something that's fixed.  Just write
something.  But I already suggested that and wrote something... :)

Do you know why they are wedded to a 16K stack?

-- 
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 9 May 2016 at 18:02, Kurt Schwehr  wrote:
> Sounds like we are on the right track now.  Hopefully, the proposal is
> clearer after a few more changes just now.
>
> My concern is that you seem to taking the discuss as the proposal and not
> ready the proposal as stand alone: http://goo.gl/vuA3D6

Kurt,

Point taken.

Although the proposal looks OK, I'd suggest to check what
assembler code generates your favourite C++ toolkit,
or at least measured times for

int anVals[256];
memset(anVals, 0, 256*sizeof(int));

vs

std::vector oVals(256, 0);

and compare with:

std::vector oVals(256, 0);

It is an interesting experiment.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

>http://goo.gl/vuA3D6   (Especially after
> I fixed the "Status:"... which I just changed again). 

Looks good to me.

Perhaps 256 bytes as the threshold ? (a LUT of 256 byte elements would fit) By 
the way, is it a threshold per array or for all arrays of a given function ?

I can think of some *potentially* performance sensitive locations like 
RPCTransformPoint() that uses a double [20+1] array where, if it would be 
above the threshold, moving the array to a working structure 
(GDALRPCTransformInfo* in that instance), or to the upper layers if they call 
multiple times the subroutine, could both address the stack size and 
performance. But at that point it is hard to be able to predict that, so let's 
do no premature optimization.


-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

I intend to argue strongly against the many styles at a later date and in a
separate thread.

On Mon, May 9, 2016 at 9:21 AM, Mateusz Loskot  wrote:

> On 9 May 2016 at 16:47, Kurt Schwehr  wrote:
> > There are lots of implicit style things in GDAL that I have been
> > trying to bend my coding to, but I find clashing styles in the same file
> all
> > the time, so it is hard to get what I do to match.
>
> Just choose the one of the two you like more.
>
> > On Mon, May 9, 2016 at 5:59 AM, Mateusz Loskot 
> wrote:
> >> On 8 May 2016 at 07:01, Kurt Schwehr  wrote:
>
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

Please fork the 80 col style guide to a new thread.  Yes, there are lots of
issues with the coding style, but can we do one thing at a time?  If you
are really motivated, why not write a proposal like mine and talk through
the issues of different column constraints (there are good reasons for the
many different ways and related issues).  Then start a new thread with it.

On Mon, May 9, 2016 at 9:43 AM, Joaquim Luis  wrote:

>
>
> I like one statement per line because it helps walking through the code
>> with gdb; you can see the whole statement that is going to be executed.
>>
>
> Fully agree, and my example respects that.
>
>
> I also share the idea that seeing a whole subroutine or a logical part at
>> once helps understanding the code.
>>
>> Ari
>>
>> 09.05.2016, 18:14, Joaquim Luis kirjoitti:
>>
>>> Hi,
>>>
>>> There is one aspect of the coding style that I honestly do not
>>> understand. Why continuing to recommend the 80 chars line width? I don't by
>>> the readability argument, well on the contrary, if because of it the result
>>> is an excess 'verticalization' of the code it becomes much harder to read.
>>>
>>> I don't know for you guys but when I'm forced to permanently scroll up
>>> and down it becomes really much harder to study a code. For me the ideal is
>>> that a function holds in a single screen size, but ofc without piling up
>>> everything to reach that goal.
>>>
>>> A little example from gdaldem_lib.cpp (line 240)
>>>
>>> ** Current code *
>>>
>>> for ( i = 0; i < 2 && i < nYSize; i++)
>>> {
>>> if( GDALRasterIO(   hSrcBand,
>>> GF_Read,
>>> 0, i,
>>> nXSize, 1,
>>> pafThreeLineWin + i * nXSize,
>>> nXSize, 1,
>>> GDT_Float32,
>>> 0, 0) != CE_None )
>>> {
>>> eErr = CE_Failure;
>>> goto end;
>>> }
>>> }
>>> **
>>>
>>>  What I think would be reasonable
>>> 
>>>
>>> for (i = 0; i < 2 && i < nYSize; i++) {
>>> if (GDALRasterIO(hSrcBand, GF_Read, 0, i, nXSize, 1,
>>> pafThreeLineWin + i * nXSize, nXSize, 1,
>>>  GDT_Float32, 0, 0) != CE_None ) {
>>> eErr = CE_Failure;
>>> goto end;
>>> }
>>> }
>>> **
>>> 7 lines instead of 15, and is it less readable (in a text editor, not in a
>>> mail client)?
>>>
>>> Joaquim
>>>
>>> On 9 May 2016 at 09:15, Ari Jolma  wrote:

> 05.05.2016, 01:30, Kurt Schwehr kirjoitti:
>
> Hi all,
>
> If you've been watching the timeline on trac, you have probably seen a
> large
> number of cleanup CLs from me.  It's definitely past time to get some
> discussion going on these changes.  If the community likes these, we
> can add
> them to rfc8_devguide.
>
>
> It is very minimal right now. Good style guides tend to be quite large,
> e.g., https://google.github.io/styleguide/cppguide.html or
> http://geosoft.no/development/cppstyle.html
>

 Good C++ guidelines that fit on a single page is all you need
 these days, after Bjarne Stroustrup
 https://parasol.tamu.edu/people/bs/622-GP/C++11-style.pdf

 Best regards,

>>> ___
>>> gdal-dev mailing list
>>> gdal-dev@lists.osgeo.org
>>> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>>>
>>
>> ___
>> gdal-dev mailing list
>> gdal-dev@lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>>
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>



-- 
--
http://schwehr.org
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

All,

Just to be totally obvious and not meant to squash questions, comments,
concerns.  I'm trying to narrow the scope of this discussion.  http://goo.gl/vuA3D6   (Especially after
I fixed the "Status:"... which I just changed again).  My intent was to
have a super narrow focus to the proposal, but be complete thereby letting
the reader know that I'm aware of alternatives.  I thought that this change
would make the scope crystal clear (obvious I was wrong :)...

https://trac.osgeo.org/gdal/changeset/34177

unique_ptr was in there as an alternative.  The item you refer to was the
>>>side track<<< of C++11 in the mailing list.  It belongs as a separate
proposal dependent on a C++11 proposal.And on the side track of C++11
support: I think your concerns are very valid.  Let's keep those to the C++
language version thread.

Anything in this thread is a great discussion, but is NOT the proposal.
That document never said that it is a justification in any way for C++11.

I added a note at the top to clarify.  The what if section is just food for
thought / completeness.  I regret having it there in the beginning, but I
might as well leave it now that the confusion already happened.

I'm happy to clarify the text in the proposal or give you comment/suggest
access to the doc.


> > No!  Out of scope of this proposal.  (For another proposal, then yes).
>
> Again, I have seen arguments like this
> https://lists.osgeo.org/pipermail/gdal-dev/2016-May/044355.html
>
> But, OK, I assume it is no longer part of your proposal.
>

Never was part of the proposed solution for "Use vector(length,
initial_value) for local blocks of storage."


>
> >> That's all I can see from most of the content there.
> >> (The many fine-grained details are not critical to make the actual
> >> 'big' decisions, those will come into play later.)
> >>
> >> So, as I have already mentioned, great.
> >> Now, let's decide wheter we switch to C++11 to get the necessary tools
> or
> >> implement our own tools, e.g. as part of CPL.
> >
> > No.  Please do not do that with my proposal.  That is way way too far
> out of
> > scope of what I intended for *this* proposal.
>
> I'm not doing anything with it.
> I'm just asking questions and stating my understanding of your goal,
> some I might have got wrong.
>
>
Please just take the text in the doc as the proposal.  Unless I copy text
from the mailing list to the doc, I don't consider it a part of the
proposal.


> >  Can we start with just this simple proposal?
>
> Sure. I have got my issues cleared, thanks.
>

Great!  Thanks for helping me improve the doc.


>
> Best regards,
> --
> Mateusz Loskot, http://mateusz.loskot.net
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Joaquim Luis]

I like one statement per line because it helps walking through the code  
with gdb; you can see the whole statement that is going to be executed.


Fully agree, and my example respects that.

I also share the idea that seeing a whole subroutine or a logical part  
at once helps understanding the code.


Ari

09.05.2016, 18:14, Joaquim Luis kirjoitti:

Hi,

There is one aspect of the coding style that I honestly do not  
understand. Why continuing to recommend the 80 chars line width? I  
don't by the readability argument, well on the contrary, if because of  
it the result is an excess 'verticalization' of the code it becomes  
much harder to read.


I don't know for you guys but when I'm forced to permanently scroll up  
and down it becomes really much harder to study a code. For me the  
ideal is that a function holds in a single screen size, but ofc without  
piling up everything to reach that goal.


A little example from gdaldem_lib.cpp (line 240)

** Current code *

for ( i = 0; i < 2 && i < nYSize; i++)
{
if( GDALRasterIO(   hSrcBand,
GF_Read,
0, i,
nXSize, 1,
pafThreeLineWin + i * nXSize,
nXSize, 1,
GDT_Float32,
0, 0) != CE_None )
{
eErr = CE_Failure;
goto end;
}
}
**  

 What I think would be reasonable  



for (i = 0; i < 2 && i < nYSize; i++) {
if (GDALRasterIO(hSrcBand, GF_Read, 0, i, nXSize, 1,  
pafThreeLineWin + i * nXSize, nXSize, 1,

 GDT_Float32, 0, 0) != CE_None ) {
eErr = CE_Failure;
goto end;
}
}
**  
7 lines instead of 15, and is it less readable (in a text editor, not  
in a mail client)?


Joaquim


On 9 May 2016 at 09:15, Ari Jolma  wrote:

05.05.2016, 01:30, Kurt Schwehr kirjoitti:

Hi all,

If you've been watching the timeline on trac, you have probably seen  
a large

number of cleanup CLs from me.  It's definitely past time to get some
discussion going on these changes.  If the community likes these, we  
can add

them to rfc8_devguide.


It is very minimal right now. Good style guides tend to be quite  
large,

e.g., https://google.github.io/styleguide/cppguide.html or
http://geosoft.no/development/cppstyle.html


Good C++ guidelines that fit on a single page is all you need
these days, after Bjarne Stroustrup
https://parasol.tamu.edu/people/bs/622-GP/C++11-style.pdf

Best regards,

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev


___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Joaquim Luis]

I do all my programing in laptops, and yes I have a 15 inches HiRes screen  
but I also have my eyes, well, weakened (and 54 years old) so I make the  
fonts larger. Having to permanently scroll up and down is far worst than  
any perception purity.



There is one aspect of the coding style that I honestly do not  
understand.

Why continuing to recommend the 80 chars line width?


Perhaps, paraphrasing Kevlin Henney, because
not everyone falls in love with high-resolution screens
and not everyone can actually now project the whole of the Serengeti :)

[1] https://vimeo.com/97329157

Best regards,

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 9 May 2016 at 16:47, Kurt Schwehr  wrote:
> There are lots of implicit style things in GDAL that I have been
> trying to bend my coding to, but I find clashing styles in the same file all
> the time, so it is hard to get what I do to match.

Just choose the one of the two you like more.

> On Mon, May 9, 2016 at 5:59 AM, Mateusz Loskot  wrote:
>> On 8 May 2016 at 07:01, Kurt Schwehr  wrote:
>>
>> - make the stack/heap usage more configurable
>
>
> No.  Not configurable.  I propose to get large objects off the stack unless
> a particular case severely impacts performance.

Yes, I got it,
I meant coding/compile-time configurable, if necessary, with
standard container/custom allocator/traits, etc.
There could even be a #define to control that all current T buffer[12345]
become heap-based or stay stack-based buffers.
However, actual realisation does not matter at this point.

>> - use new C++ features to introduce widely recommended idioms
>
> No.  I do not propose using any C++ features new to GDAL.  This is just the
> old vector size with initializer constructor.

As one of your argument, I have seen RAII proposed.

>> > Please read the proposal as just looking at the possibilities.
>> > Please try not to care about C++ language version when
>> > trying to do a first round review of ideas.
>>
>> I have and as far as I can see, there are just two objectives:
>> 1. Make GDAL stack-friendly by preferring heap for arrays.
>> How? Use standard containers and smart pointers.
>
>
> That is 3 statements:
>
> a) Make GDAL stack-friendly by preferring heap for arrays.
>
> Yes.

OK,

> b)  How? Use standard containers
>
> Yes.  I propose this

OK

> c) and smart pointers
>
> No.  In this case, a smart pointer looks to be less attractive (to me).
>
>> 2. Introduce RAII across the whole codebase.
>> How? Use smart pointers and other related techniques.
>
>
> No!  Out of scope of this proposal.  (For another proposal, then yes).

Again, I have seen arguments like this
https://lists.osgeo.org/pipermail/gdal-dev/2016-May/044355.html

But, OK, I assume it is no longer part of your proposal.

>> That's all I can see from most of the content there.
>> (The many fine-grained details are not critical to make the actual
>> 'big' decisions, those will come into play later.)
>>
>> So, as I have already mentioned, great.
>> Now, let's decide wheter we switch to C++11 to get the necessary tools or
>> implement our own tools, e.g. as part of CPL.
>
> No.  Please do not do that with my proposal.  That is way way too far out of
> scope of what I intended for *this* proposal.

I'm not doing anything with it.
I'm just asking questions and stating my understanding of your goal,
some I might have got wrong.

>  Can we start with just this simple proposal?

Sure. I have got my issues cleared, thanks.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 9 May 2016 at 17:14, Joaquim Luis  wrote:
> Hi,
>
> There is one aspect of the coding style that I honestly do not understand.
> Why continuing to recommend the 80 chars line width?

Perhaps, paraphrasing Kevlin Henney, because
not everyone falls in love with high-resolution screens
and not everyone can actually now project the whole of the Serengeti :)

[1] https://vimeo.com/97329157

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Ari Jolma]

I like one statement per line because it helps walking through the code 
with gdb; you can see the whole statement that is going to be executed.


I also share the idea that seeing a whole subroutine or a logical part 
at once helps understanding the code.


Ari

09.05.2016, 18:14, Joaquim Luis kirjoitti:

Hi,

There is one aspect of the coding style that I honestly do not 
understand. Why continuing to recommend the 80 chars line width? I 
don't by the readability argument, well on the contrary, if because of 
it the result is an excess 'verticalization' of the code it becomes 
much harder to read.


I don't know for you guys but when I'm forced to permanently scroll up 
and down it becomes really much harder to study a code. For me the 
ideal is that a function holds in a single screen size, but ofc 
without piling up everything to reach that goal.


A little example from gdaldem_lib.cpp (line 240)

** Current code *

for ( i = 0; i < 2 && i < nYSize; i++)
{
if( GDALRasterIO(   hSrcBand,
GF_Read,
0, i,
nXSize, 1,
pafThreeLineWin + i * nXSize,
nXSize, 1,
GDT_Float32,
0, 0) != CE_None )
{
eErr = CE_Failure;
goto end;
}
}
** 




 What I think would be reasonable 



for (i = 0; i < 2 && i < nYSize; i++) {
if (GDALRasterIO(hSrcBand, GF_Read, 0, i, nXSize, 1, 
pafThreeLineWin + i * nXSize, nXSize, 1,

 GDT_Float32, 0, 0) != CE_None ) {
eErr = CE_Failure;
goto end;
}
}
** 



7 lines instead of 15, and is it less readable (in a text editor, not 
in a mail client)?


Joaquim


On 9 May 2016 at 09:15, Ari Jolma  wrote:

05.05.2016, 01:30, Kurt Schwehr kirjoitti:

Hi all,

If you've been watching the timeline on trac, you have probably seen 
a large

number of cleanup CLs from me.  It's definitely past time to get some
discussion going on these changes.  If the community likes these, we 
can add

them to rfc8_devguide.


It is very minimal right now. Good style guides tend to be quite large,
e.g., https://google.github.io/styleguide/cppguide.html or
http://geosoft.no/development/cppstyle.html


Good C++ guidelines that fit on a single page is all you need
these days, after Bjarne Stroustrup
https://parasol.tamu.edu/people/bs/622-GP/C++11-style.pdf

Best regards,

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev


___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Joaquim Luis]

Hi,

There is one aspect of the coding style that I honestly do not understand.  
Why continuing to recommend the 80 chars line width? I don't by the  
readability argument, well on the contrary, if because of it the result is  
an excess 'verticalization' of the code it becomes much harder to read.


I don't know for you guys but when I'm forced to permanently scroll up and  
down it becomes really much harder to study a code. For me the ideal is  
that a function holds in a single screen size, but ofc without piling up  
everything to reach that goal.


A little example from gdaldem_lib.cpp (line 240)

** Current code *

for ( i = 0; i < 2 && i < nYSize; i++)
{
if( GDALRasterIO(   hSrcBand,
GF_Read,
0, i,
nXSize, 1,
pafThreeLineWin + i * nXSize,
nXSize, 1,
GDT_Float32,
0, 0) != CE_None )
{
eErr = CE_Failure;
goto end;
}
}
**


 What I think would be reasonable  



for (i = 0; i < 2 && i < nYSize; i++) {
if (GDALRasterIO(hSrcBand, GF_Read, 0, i, nXSize, 1,  
pafThreeLineWin + i * nXSize, nXSize, 1,

 GDT_Float32, 0, 0) != CE_None ) {
eErr = CE_Failure;
goto end;
}
}
**

7 lines instead of 15, and is it less readable (in a text editor, not in a  
mail client)?


Joaquim


On 9 May 2016 at 09:15, Ari Jolma  wrote:

05.05.2016, 01:30, Kurt Schwehr kirjoitti:

Hi all,

If you've been watching the timeline on trac, you have probably seen a  
large

number of cleanup CLs from me.  It's definitely past time to get some
discussion going on these changes.  If the community likes these, we  
can add

them to rfc8_devguide.


It is very minimal right now. Good style guides tend to be quite large,
e.g., https://google.github.io/styleguide/cppguide.html or
http://geosoft.no/development/cppstyle.html


Good C++ guidelines that fit on a single page is all you need
these days, after Bjarne Stroustrup
https://parasol.tamu.edu/people/bs/622-GP/C++11-style.pdf

Best regards,

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

I was trying to give people an overall sense of my goals for GDAL.

I'm sorry I added the context to the proposal that got the discussion off
the track I intended.  I am more interested in mechanics of the discussion
before hitting huge topics.  I love the discussion, but I feel like I
totally failed at trying to test strategy for the GDAL community to have
discussions and get some basics added to the style guide with some why
attached.  I think we have lots to discuss before tooling and newer
versions of C++.  There are lots of implicit style things in GDAL that I
have been trying to bend my coding to, but I find clashing styles in the
same file all the time, so it is hard to get what I do to match.

Just for this proposal... I do let myself get dragged a bit into the C++
version discussion a bit below to try to show that it is off topic of
>>>getting large objects off the stack<<<.

On Mon, May 9, 2016 at 5:59 AM, Mateusz Loskot  wrote:

> On 8 May 2016 at 07:01, Kurt Schwehr  wrote:
> >
> > Are meaning to say that my proposal is vague?
>
> Yes, I called that on the course of the discussion, it has become less and
> less
> clear to me: what is the actual goal?
>


> - make the stack/heap usage more configurable


No.  Not configurable.  I propose to get large objects off the stack unless
a particular case severely impacts performance.


>

- use new C++ features to introduce widely recommended idioms
>

No.  I do not propose using any C++ features new to GDAL.  This is just the
old vector size with initializer constructor.


> - make the code shorter, hence clearer/more readable/maintainable
>

No.  Shorter wasn't a goal for this one.  It's sorter because my proposed
solution combines the definition and the initialization so as to not have
an uninitialized variable after the definition until the memset.

- make the code more C++  than C by allowing wide use of C++ standard
> features
>

No.


> ...
> So, in order to achive the above, you are proposing switch to newer C++
> version?
>

No.  I was trying to be complete in going through all the options available
inside and out of GDAL.  Only if the community really wanted a feature from
C++ >= 11 would I consider the language version a part of this proposal.


>
> > Please read the proposal as just looking at the possibilities.
> > Please try not to care about C++ language version when
> > trying to do a first round review of ideas.
>
> I have and as far as I can see, there are just two objectives:
> 1. Make GDAL stack-friendly by preferring heap for arrays.
> How? Use standard containers and smart pointers.
>

That is 3 statements:

a) Make GDAL stack-friendly by preferring heap for arrays.

Yes.

b)  How? Use standard containers

Yes.  I propose this

c) and smart pointers

No.  In this case, a smart pointer looks to be less attractive (to me).


> 2. Introduce RAII across the whole codebase.
> How? Use smart pointers and other related techniques.
>

No!  Out of scope of this proposal.  (For another proposal, then yes).


>
> That's all I can see from most of the content there.
> (The many fine-grained details are not critical to make the actual
> 'big' decisions, those will come into play later.)
>
> So, as I have already mentioned, great.
> Now, let's decide wheter we switch to C++11 to get the necessary tools or
> implement our own tools, e.g. as part of CPL.
>

No.  Please do not do that with my proposal.  That is way way too far out
of scope of what I intended for *this* proposal.   Can we start with just
this simple proposal?


>
> I believe, much of the discussion would have gained more focus if the
> problem
> was stated like that.
>

Yeah, I missed the boat on focus.


>
> > On Fri, May 6, 2016 at 2:47 PM, Mateusz Loskot 
> wrote:
> >>
> >> On 6 May 2016 at 21:55, Kurt Schwehr  wrote:
> >> > My belief is that for this particular proposal, we should not have the
> >> > C++11/14 discussion unless the best overall solution requires a newer
> >> > version of C++.  The solution I propose to be the best works in C++03
> >> > and newer.
> >>
> >> Considering current codebase, GDAL is written in C and compiled in C++.
> >> The only major C++ feature used in GDAL is classes and polymorphism.
> >
> >
> > I agree that GDAL has a strong feel of C compiled with C++ right now.
>
> Since that is a fact, there is nothing to agree about.
> Please, notice, I'm not speaking here about my personal preferences,
> just the state of the art.
>
> > I disagree about C++ feature use.  I see algorithm, iostream, map, list,
> > sstream, a few exceptions, std::string (mostly in the form of CPLString),
> > vector, localization of variables, some templates, #define to static
> consts,
> > etc.
>
> In GDAL? e.g. streams are not used or they use is marginal.
>

find . -name \*cpp | xargs grep include | grep stream | wc -l
53

find . -name \*cpp | xargs grep -h include | grep stream | sort -u
#include 
#include 
#include "mrsidstream.h"
//#include 
#include 
#include 


>

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 9 May 2016 at 09:15, Ari Jolma  wrote:
> 05.05.2016, 01:30, Kurt Schwehr kirjoitti:
>
> Hi all,
>
> If you've been watching the timeline on trac, you have probably seen a large
> number of cleanup CLs from me.  It's definitely past time to get some
> discussion going on these changes.  If the community likes these, we can add
> them to rfc8_devguide.
>
>
> It is very minimal right now. Good style guides tend to be quite large,
> e.g., https://google.github.io/styleguide/cppguide.html or
> http://geosoft.no/development/cppstyle.html

Good C++ guidelines that fit on a single page is all you need
these days, after Bjarne Stroustrup
https://parasol.tamu.edu/people/bs/622-GP/C++11-style.pdf

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 8 May 2016 at 07:01, Kurt Schwehr  wrote:
>
> Are meaning to say that my proposal is vague?

Yes, I called that on the course of the discussion, it has become less and less
clear to me: what is the actual goal?
- make the stack/heap usage more configurable
- use new C++ features to introduce widely recommended idioms
- make the code shorter, hence clearer/more readable/maintainable
- make the code more C++  than C by allowing wide use of C++ standard features
...
So, in order to achive the above, you are proposing switch to newer C++ version?

> Please read the proposal as just looking at the possibilities.
> Please try not to care about C++ language version when
> trying to do a first round review of ideas.

I have and as far as I can see, there are just two objectives:
1. Make GDAL stack-friendly by preferring heap for arrays.
How? Use standard containers and smart pointers.
2. Introduce RAII across the whole codebase.
How? Use smart pointers and other related techniques.

That's all I can see from most of the content there.
(The many fine-grained details are not critical to make the actual
'big' decisions, those will come into play later.)

So, as I have already mentioned, great.
Now, let's decide wheter we switch to C++11 to get the necessary tools or
implement our own tools, e.g. as part of CPL.

I believe, much of the discussion would have gained more focus if the problem
was stated like that.

> On Fri, May 6, 2016 at 2:47 PM, Mateusz Loskot  wrote:
>>
>> On 6 May 2016 at 21:55, Kurt Schwehr  wrote:
>> > My belief is that for this particular proposal, we should not have the
>> > C++11/14 discussion unless the best overall solution requires a newer
>> > version of C++.  The solution I propose to be the best works in C++03
>> > and newer.
>>
>> Considering current codebase, GDAL is written in C and compiled in C++.
>> The only major C++ feature used in GDAL is classes and polymorphism.
>
>
> I agree that GDAL has a strong feel of C compiled with C++ right now.

Since that is a fact, there is nothing to agree about.
Please, notice, I'm not speaking here about my personal preferences,
just the state of the art.

> I disagree about C++ feature use.  I see algorithm, iostream, map, list,
> sstream, a few exceptions, std::string (mostly in the form of CPLString),
> vector, localization of variables, some templates, #define to static consts,
> etc.

In GDAL? e.g. streams are not used or they use is marginal.


>> If you ask any seasoned C programmer for opinion, I bet she will
>> consider GDAL codebase as clear and readable.
>> If you ask a C++ programmer, the answer might be different.
>>
>> IMO, I doubt any C++11/14 feature will increase the current code,
>> unless it is considered to ditch many of CPL features and replace
>> them with C++ standard features (string bashing operations, string
>> containers,
>> containers for other types, threading API, etc.).
>>
>
> "increase the current code" <- Did you mean increase the reliability?  Or
> size? Or complexity?

Sorry, sloppy typing. I meant code clarity, readability.

>> Certainly, any new line to be written may benefit from auto,
>> constexpr, using, nullptr, range loops - but for a "C with classes"
>> codebase as GDAL, it would be cosmetic improvement, if any.
>>
>
> I disagree that C++11/14 changes would be cosmetic.  But this is the wrong
> thread for this part of the discussion.

IMHO, you are missing one important point.
I'm not arguing that shifting from C with classes mode to C++11-17
is not going to increase software quality. Of course it is, if applied
properly and *at all*.

Have you considered current maintainers might not be willing to start using
the latest and the greatest C++11-17 features?
If they are not...changes will be rather cosmectic.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Ari Jolma]

05.05.2016, 01:30, Kurt Schwehr kirjoitti:

Hi all,

If you've been watching the timeline on trac, you have probably seen a 
large number of cleanup CLs from me.  It's definitely past time to get 
some discussion going on these changes.  If the community likes these, 
we can add them to rfc8_devguide 
.


It is very minimal right now. Good style guides tend to be quite large, 
e.g., https://google.github.io/styleguide/cppguide.html or 
http://geosoft.no/development/cppstyle.html


Is there something GDAL could reuse?

Ari

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

Mateusz,

Thanks for calling out the confusion!  Are meaning to say that my proposal

is
vague?  I rephrased the status to be more clear that the C++11/14 focus is
the GDAL community (AKA the majority of this thread), not the proposal.
Hopefully it is clearer now.  Please read the proposal as just looking at
the possibilities.  Please try not to care about C++ language version when
trying to do a first round review of ideas.

On Fri, May 6, 2016 at 2:47 PM, Mateusz Loskot  wrote:

> On 6 May 2016 at 21:55, Kurt Schwehr  wrote:
> > My belief is that for this particular proposal, we should not have the
> > C++11/14 discussion unless the best overall solution requires a newer
> > version of C++.  The solution I propose to be the best works in C++03 and
> > newer.
>
> Simply, the initial proposal was quite confusing.
>
> The 'title' said indicated it was "mostly focused on C++11/14 & C99/11",
> while the content mentioned mostly the std::vector as malloc'ed arrays
> replacement.
>

That was just a status I added after the fact.  I worded it poorly.


>
> Considering current codebase, GDAL is written in C and compiled in C++.
> The only major C++ feature used in GDAL is classes and polymorphism.
>
>
I agree that GDAL has a strong feel of C compiled with C++ right now.

I disagree about C++ feature use.  I see algorithm, iostream, map, list,
sstream, a few exceptions, std::string (mostly in the form of CPLString),
vector, localization of variables, some templates, #define to static
consts, etc.

I'm not sure if the facts mentioned in the proposal, namely
> "Want to focus on maintainability and readability of code",
> is meant to be a fact or a goal.
>

That would be a goal.  And readability is going somewhat be subjective.


> If you ask any seasoned C programmer for opinion, I bet she will
> consider GDAL codebase as clear and readable.
> If you ask a C++ programmer, the answer might be different.
>
> IMO, I doubt any C++11/14 feature will increase the current code,
> unless it is considered to ditch many of CPL features and replace
> them with C++ standard features (string bashing operations, string
> containers,
> containers for other types, threading API, etc.).
>
>
"increase the current code" <- Did you mean increase the reliability?  Or
size? Or complexity?


> Certainly, any new line to be written may benefit from auto,
> constexpr, using, nullptr, range loops - but for a "C with classes"
> codebase as GDAL, it would be cosmetic improvement, if any.
>
>
I disagree that C++11/14 changes would be cosmetic.  But this is the wrong
thread for this part of the discussion.


> Surely, the override and final will help compiler to catch potential bugs.
> Also, C++11/14 compilation mode is generally stricter, so it might
> help to catch bugs.
>
> Simply, the whole proposal is becoming vague.
>
> Best regards,
> --
> Mateusz Loskot, http://mateusz.loskot.net
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Tim Keitt]

On Fri, May 6, 2016 at 2:47 PM, Kurt Schwehr  wrote:

> For stack ranking, see the "alternatives" section of this (and I did not
> include my preferred std::vector solution in the list):


That seems like a reasonable solution. The function/method call interface
is where I would begin substituting STL for C equivalents if it were up to
me. Once those are set, then you can refactor as needed behind the API.

I was not recommending autoptr, I just did not go look up the replacements.
Also, much of C++11 can be accomplished using Boost as many of the features
originated there as libraries for C++03. A non-trivial dependency to be
sure.

THK

http://www.keittlab.org/
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Tim Keitt]

http://www.keittlab.org/

On Fri, May 6, 2016 at 2:47 PM, Mateusz Loskot  wrote:

> On 6 May 2016 at 21:55, Kurt Schwehr  wrote:
> > My belief is that for this particular proposal, we should not have the
> > C++11/14 discussion unless the best overall solution requires a newer
> > version of C++.  The solution I propose to be the best works in C++03 and
> > newer.
>
> Simply, the initial proposal was quite confusing.
>
> The 'title' said indicated it was "mostly focused on C++11/14 & C99/11",
> while the content mentioned mostly the std::vector as malloc'ed arrays
> replacement.
>
> Considering current codebase, GDAL is written in C and compiled in C++.
> The only major C++ feature used in GDAL is classes and polymorphism.
>
> I'm not sure if the facts mentioned in the proposal, namely
> "Want to focus on maintainability and readability of code",
> is meant to be a fact or a goal.
> If you ask any seasoned C programmer for opinion, I bet she will
> consider GDAL codebase as clear and readable.
> If you ask a C++ programmer, the answer might be different.
>
> IMO, I doubt any C++11/14 feature will increase the current code,
> unless it is considered to ditch many of CPL features and replace
> them with C++ standard features (string bashing operations, string
> containers,
> containers for other types, threading API, etc.).
>

That's exactly my preference. For those of us who have to remember too many
things, using the standard library is a godsend because it is exceptionally
well documented online and used in many places.


>
> Certainly, any new line to be written may benefit from auto,
> constexpr, using, nullptr, range loops - but for a "C with classes"
> codebase as GDAL, it would be cosmetic improvement, if any.
>

It is true that unless you refactor along the lines of Boost, then these
features are not used as effectively.


>
> Surely, the override and final will help compiler to catch potential bugs.
> Also, C++11/14 compilation mode is generally stricter, so it might
> help to catch bugs.
>

My experience with C++11 is that it is vastly less error prone and more
readable. But of course there is a big cost to rewriting code, so I
understand the conundrum.

THK


>
> Simply, the whole proposal is becoming vague.
>
> Best regards,
> --
> Mateusz Loskot, http://mateusz.loskot.net
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 6 May 2016 at 21:55, Kurt Schwehr  wrote:
> My belief is that for this particular proposal, we should not have the
> C++11/14 discussion unless the best overall solution requires a newer
> version of C++.  The solution I propose to be the best works in C++03 and
> newer.

Simply, the initial proposal was quite confusing.

The 'title' said indicated it was "mostly focused on C++11/14 & C99/11",
while the content mentioned mostly the std::vector as malloc'ed arrays
replacement.

Considering current codebase, GDAL is written in C and compiled in C++.
The only major C++ feature used in GDAL is classes and polymorphism.

I'm not sure if the facts mentioned in the proposal, namely
"Want to focus on maintainability and readability of code",
is meant to be a fact or a goal.
If you ask any seasoned C programmer for opinion, I bet she will
consider GDAL codebase as clear and readable.
If you ask a C++ programmer, the answer might be different.

IMO, I doubt any C++11/14 feature will increase the current code,
unless it is considered to ditch many of CPL features and replace
them with C++ standard features (string bashing operations, string containers,
containers for other types, threading API, etc.).

Certainly, any new line to be written may benefit from auto,
constexpr, using, nullptr, range loops - but for a "C with classes"
codebase as GDAL, it would be cosmetic improvement, if any.

Surely, the override and final will help compiler to catch potential bugs.
Also, C++11/14 compilation mode is generally stricter, so it might
help to catch bugs.

Simply, the whole proposal is becoming vague.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Andrew Bell]

On Fri, May 6, 2016 at 3:55 PM, Kurt Schwehr  wrote:

> I was not intending for C++11 to be a big part of the discussion as it is
> a much more complicated topic, but I do appreciate the discussion.  I
> picked the stack + memset -> std::vector(nSize, initialValue) to do first
> because I thought it was a simpler issue than most and we could use it to
> figure out how to go about these sorts of discussions.  It's not a show
> stoper, but it is a real need.
>
> At this point, I think it would be good if we could pause the C++11
> discussion for a bit and back up to the original question.
>
> I'd like to ask folks a couple of things:
>
> - If you could stack rank (all or some) the options assuming that C++14
> was available
> - Is your top pick far better than the rest or just a little better?
> - Are there any options that you think should not be used in GDAL?
> - Are there any reasons to be for or against any particular alternative
> that we need to call out?
>

Doing a heap-based array is simple.  You can pretty-much copy the
std::array code from the standard library and introduce the indirection you
require.  It's better than a vector for the reasons you already mentioned.
Something that looks and acts like an array is better than a vector because
it makes the use clear.  If you have C++ 11 you can simply wrap
std::array.  C++03 has value initialization for arrays: new int[10]();, so
that's another alternative.

-- 
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

For stack ranking, see the "alternatives" section of this (and I did not
include my preferred std::vector solution in the list):

https://docs.google.com/document/d/1Y9flzxj3Uz1vTEPCBmlswgi470m8i-oepGutVkbowfc/pub

BTW, Someone pointed out that I originally listed autoptr (which just won't
work) when I mean to say scoped_ptr.h.  I chatted with the original author
of scoped_ptr last year and have permission to share the latest version
from inside Google should that somehow become the solution of choice for
any particular issue, but I really think we should avoid scoped_ptr when
unique_ptr exists, is a standard and is more robust.  std::vector was not
the first solution I came up with for this issue.

On Fri, May 6, 2016 at 2:37 PM, Tim Keitt  wrote:

>
>
> http://www.keittlab.org/
>
> On Fri, May 6, 2016 at 1:55 PM, Kurt Schwehr  wrote:
>
>> I was not intending for C++11 to be a big part of the discussion as it is
>> a much more complicated topic, but I do appreciate the discussion.  I
>> picked the stack + memset -> std::vector(nSize, initialValue) to do first
>> because I thought it was a simpler issue than most and we could use it to
>> figure out how to go about these sorts of discussions.  It's not a show
>> stoper, but it is a real need.
>>
>> At this point, I think it would be good if we could pause the C++11
>> discussion for a bit and back up to the original question.
>>
>> I'd like to ask folks a couple of things:
>>
>> - If you could stack rank (all or some) the options assuming that C++14
>> was available
>>
>
> What are the options and for what purpose? (I have not followed the thread
> in detail.) I'd like to see pointer types banished or if necessary wrapped
> with std::auto_ptr (or whichever variant makes sense).
>
>
>> - Is your top pick far better than the rest or just a little better?
>> - Are there any options that you think should not be used in GDAL?
>> - Are there any reasons to be for or against any particular alternative
>> that we need to call out?
>>
>
> Always prefer vanilla standard library over others.
>
>
>>
>> My belief is that for this particular proposal, we should not have the
>> C++11/14 discussion unless the best overall solution requires a newer
>> version of C++.  The solution I propose to be the best works in C++03 and
>> newer.
>>
>
> I have longed for GDAL to be a modern C++ library that follows the STL, so
> any move in that direction would be terrific in my opinion. It would be
> nice to have the latest and greatest C++ bits, but I would be very happy
> with C++03. I strongly suggest maximizing use of the STL to avoid
> reinventing the wheel. If you need custom allocation, use a custom
> allocator with an STL data structure. I would also recommend judicious use
> of Boost where appropriate.
>
> THK
>
>
>>
>>
>> On Thu, May 5, 2016 at 2:25 PM, Even Rouault 
>> wrote:
>>
>>>
>>> > I misinterpreted this thread because of the title.  It seems this isn't
>>> > really about code style or C++ 11.
>>>
>>> I agree this thread mixes different topics, which causes some confusion.
>>> The
>>> word C++11 appearing in a doc has created some passion, whereas it is
>>> probably
>>> not the heart of the subject.
>>>
>>> Kurt has contributed to a lot of cleanup in the code base over the last
>>> months, mostly in applying stricter formatting rules, bool'ifying,
>>> const'ifying when possible, moving variable declarations close to their
>>> initialization, etc.. (Kurt, correct me if I'm wrong.), also addressing a
>>> bunch of Covertiy Scan warnings, etc.
>>>
>>> I've actually encouraged him to share his thoughts regarding cleanups
>>> with the
>>> rest of the community so that we can decide which are worth to be rules
>>> that
>>> apply to the project as a whole, and if some tooling is needed to
>>> automate/enforce them.
>>>
>>> > It's really about Google wanting to use
>>> > heap over stack because of its particular use of the library.  There
>>> is no
>>> > need for C++ 11 for that and it would seem that doing a compile-time
>>> > policy-based array isn't too hard (proof of concept below).  Perhaps
>>> Google
>>> > could flush something like this out to its liking and replace current
>>> stack
>>> > allocations that cause it issues.
>>>
>>> This use case is probably shared by other actors that use GDAL for
>>> massive
>>> data processing, and if the solution to reduce stack usage doesn't hurt
>>> more
>>> mundane use cases, we probably don't need the #ifdef complication.
>>>
>>> --
>>> Spatialys - Geospatial professional services
>>> http://www.spatialys.com
>>>
>>
>>
>>
>> --
>> --
>> http://schwehr.org
>>
>> ___
>> gdal-dev mailing list
>> gdal-dev@lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>>
>
>


-- 
--
http://schwehr.org
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Tim Keitt]

http://www.keittlab.org/

On Fri, May 6, 2016 at 1:55 PM, Kurt Schwehr  wrote:

> I was not intending for C++11 to be a big part of the discussion as it is
> a much more complicated topic, but I do appreciate the discussion.  I
> picked the stack + memset -> std::vector(nSize, initialValue) to do first
> because I thought it was a simpler issue than most and we could use it to
> figure out how to go about these sorts of discussions.  It's not a show
> stoper, but it is a real need.
>
> At this point, I think it would be good if we could pause the C++11
> discussion for a bit and back up to the original question.
>
> I'd like to ask folks a couple of things:
>
> - If you could stack rank (all or some) the options assuming that C++14
> was available
>

What are the options and for what purpose? (I have not followed the thread
in detail.) I'd like to see pointer types banished or if necessary wrapped
with std::auto_ptr (or whichever variant makes sense).


> - Is your top pick far better than the rest or just a little better?
> - Are there any options that you think should not be used in GDAL?
> - Are there any reasons to be for or against any particular alternative
> that we need to call out?
>

Always prefer vanilla standard library over others.


>
> My belief is that for this particular proposal, we should not have the
> C++11/14 discussion unless the best overall solution requires a newer
> version of C++.  The solution I propose to be the best works in C++03 and
> newer.
>

I have longed for GDAL to be a modern C++ library that follows the STL, so
any move in that direction would be terrific in my opinion. It would be
nice to have the latest and greatest C++ bits, but I would be very happy
with C++03. I strongly suggest maximizing use of the STL to avoid
reinventing the wheel. If you need custom allocation, use a custom
allocator with an STL data structure. I would also recommend judicious use
of Boost where appropriate.

THK


>
>
> On Thu, May 5, 2016 at 2:25 PM, Even Rouault 
> wrote:
>
>>
>> > I misinterpreted this thread because of the title.  It seems this isn't
>> > really about code style or C++ 11.
>>
>> I agree this thread mixes different topics, which causes some confusion.
>> The
>> word C++11 appearing in a doc has created some passion, whereas it is
>> probably
>> not the heart of the subject.
>>
>> Kurt has contributed to a lot of cleanup in the code base over the last
>> months, mostly in applying stricter formatting rules, bool'ifying,
>> const'ifying when possible, moving variable declarations close to their
>> initialization, etc.. (Kurt, correct me if I'm wrong.), also addressing a
>> bunch of Covertiy Scan warnings, etc.
>>
>> I've actually encouraged him to share his thoughts regarding cleanups
>> with the
>> rest of the community so that we can decide which are worth to be rules
>> that
>> apply to the project as a whole, and if some tooling is needed to
>> automate/enforce them.
>>
>> > It's really about Google wanting to use
>> > heap over stack because of its particular use of the library.  There is
>> no
>> > need for C++ 11 for that and it would seem that doing a compile-time
>> > policy-based array isn't too hard (proof of concept below).  Perhaps
>> Google
>> > could flush something like this out to its liking and replace current
>> stack
>> > allocations that cause it issues.
>>
>> This use case is probably shared by other actors that use GDAL for massive
>> data processing, and if the solution to reduce stack usage doesn't hurt
>> more
>> mundane use cases, we probably don't need the #ifdef complication.
>>
>> --
>> Spatialys - Geospatial professional services
>> http://www.spatialys.com
>>
>
>
>
> --
> --
> http://schwehr.org
>
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

I was not intending for C++11 to be a big part of the discussion as it is a
much more complicated topic, but I do appreciate the discussion.  I picked
the stack + memset -> std::vector(nSize, initialValue) to do first because
I thought it was a simpler issue than most and we could use it to figure
out how to go about these sorts of discussions.  It's not a show stoper,
but it is a real need.

At this point, I think it would be good if we could pause the C++11
discussion for a bit and back up to the original question.

I'd like to ask folks a couple of things:

- If you could stack rank (all or some) the options assuming that C++14 was
available
- Is your top pick far better than the rest or just a little better?
- Are there any options that you think should not be used in GDAL?
- Are there any reasons to be for or against any particular alternative
that we need to call out?

My belief is that for this particular proposal, we should not have the
C++11/14 discussion unless the best overall solution requires a newer
version of C++.  The solution I propose to be the best works in C++03 and
newer.


On Thu, May 5, 2016 at 2:25 PM, Even Rouault 
wrote:

>
> > I misinterpreted this thread because of the title.  It seems this isn't
> > really about code style or C++ 11.
>
> I agree this thread mixes different topics, which causes some confusion.
> The
> word C++11 appearing in a doc has created some passion, whereas it is
> probably
> not the heart of the subject.
>
> Kurt has contributed to a lot of cleanup in the code base over the last
> months, mostly in applying stricter formatting rules, bool'ifying,
> const'ifying when possible, moving variable declarations close to their
> initialization, etc.. (Kurt, correct me if I'm wrong.), also addressing a
> bunch of Covertiy Scan warnings, etc.
>
> I've actually encouraged him to share his thoughts regarding cleanups with
> the
> rest of the community so that we can decide which are worth to be rules
> that
> apply to the project as a whole, and if some tooling is needed to
> automate/enforce them.
>
> > It's really about Google wanting to use
> > heap over stack because of its particular use of the library.  There is
> no
> > need for C++ 11 for that and it would seem that doing a compile-time
> > policy-based array isn't too hard (proof of concept below).  Perhaps
> Google
> > could flush something like this out to its liking and replace current
> stack
> > allocations that cause it issues.
>
> This use case is probably shared by other actors that use GDAL for massive
> data processing, and if the solution to reduce stack usage doesn't hurt
> more
> mundane use cases, we probably don't need the #ifdef complication.
>
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
>



-- 
--
http://schwehr.org
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Jeff McKenna]

On 2016-05-06 11:38 AM, Mateusz Loskot wrote:

On 6 May 2016 at 16:20, Joaquim Luis  wrote:

On Fri, 06 May 2016 15:00:07 +0100, Even Rouault
 wrote:


- ECW SDK 5.2.1: available for VS2010, VS2012, VS2013



I got information that the next SDK 5.3 should support VS2012, 2013 and
2015
(so no more 2010) + GCC 5



And one can also build SDK 3.3 with all of those compilers (I did).


I do it too, works well indeed.



same.

-jeff



--
Jeff McKenna
MapServer Consulting and Training Services
http://www.gatewaygeomatics.com/




___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 6 May 2016 at 16:20, Joaquim Luis  wrote:
> On Fri, 06 May 2016 15:00:07 +0100, Even Rouault
>  wrote:
>
>>> - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
>>
>>
>> I got information that the next SDK 5.3 should support VS2012, 2013 and
>> 2015
>> (so no more 2010) + GCC 5
>
>
> And one can also build SDK 3.3 with all of those compilers (I did).

I do it too, works well indeed.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Joaquim Luis]

On Fri, 06 May 2016 15:00:07 +0100, Even Rouault  
 wrote:



- ECW SDK 5.2.1: available for VS2010, VS2012, VS2013


I got information that the next SDK 5.3 should support VS2012, 2013 and  
2015

(so no more 2010) + GCC 5


And one can also build SDK 3.3 with all of those compilers (I did).
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

> - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013

I got information that the next SDK 5.3 should support VS2012, 2013 and 2015 
(so no more 2010) + GCC 5

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

> I misinterpreted this thread because of the title.  It seems this isn't
> really about code style or C++ 11.  

I agree this thread mixes different topics, which causes some confusion. The 
word C++11 appearing in a doc has created some passion, whereas it is probably 
not the heart of the subject.

Kurt has contributed to a lot of cleanup in the code base over the last 
months, mostly in applying stricter formatting rules, bool'ifying, 
const'ifying when possible, moving variable declarations close to their 
initialization, etc.. (Kurt, correct me if I'm wrong.), also addressing a 
bunch of Covertiy Scan warnings, etc.

I've actually encouraged him to share his thoughts regarding cleanups with the 
rest of the community so that we can decide which are worth to be rules that 
apply to the project as a whole, and if some tooling is needed to 
automate/enforce them.

> It's really about Google wanting to use
> heap over stack because of its particular use of the library.  There is no
> need for C++ 11 for that and it would seem that doing a compile-time
> policy-based array isn't too hard (proof of concept below).  Perhaps Google
> could flush something like this out to its liking and replace current stack
> allocations that cause it issues. 

This use case is probably shared by other actors that use GDAL for massive 
data processing, and if the solution to reduce stack usage doesn't hurt more 
mundane use cases, we probably don't need the #ifdef complication.

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Andrew Bell]

On Thu, May 5, 2016 at 10:19 AM, Kurt Schwehr  wrote:

> Thanks all for your comments!  I will try to write up a similar proposal
> for C++11 based on what people have written.
>
> This local stack storage change is for a real need, so it's probably good
> that I add a use case that explains why this change is important.  And I
> >>believe<< (but am not 100% sure) that using std::vector is the strongest
> solution for the majority of instances of large buffers on the stack in
> gdal.
>
> Use case:  (note that I can only share generals)
>
> At Google, we have jobs that use GDAL running with thousands of cores and
> >20 threads per process that run for days.  The overhead of having to allow
> for bumping the stack size adds substantial overhead to all those jobs.
> GDAL is the only thing driving the requirement for the larger stack.  For
> people using GDAL on a desktop machine, the larger stack is unlikely to be
> a real issue.  For anyone running large multithreaded jobs in the cloud,
> the large stack is a real cost issue.  The overhead of vector and heap
> allocation in all these cases I've looked at in GDAL so far is far smaller
> than the cost from the stack size overhead.
>

I misinterpreted this thread because of the title.  It seems this isn't
really about code style or C++ 11.  It's really about Google wanting to use
heap over stack because of its particular use of the library.  There is no
need for C++ 11 for that and it would seem that doing a compile-time
policy-based array isn't too hard (proof of concept below).  Perhaps Google
could flush something like this out to its liking and replace current stack
allocations that cause it issues.  C++ 11 and code style seem another
conversation.

==

#include 

struct Stack
{};

struct Heap
{};

template 
struct Arr
{
};

template 
struct Arr
{
Arr() : m_instance(new TYPE[SIZE ? SIZE : 1])
{}

~Arr()
{
delete [] m_instance;
}

TYPE *m_instance;

TYPE& operator[](std::size_t idx)
{ return *(m_instance + idx); }
};

template 
struct Arr
{
TYPE m_instance[SIZE ? SIZE : 1];

TYPE& operator[](std::size_t idx)
{ return m_instance[idx]; }
};


#ifdef HEAPALLOC
template 
struct Array : public Arr
{};
#else
template 
struct Array : public Arr
{};
#endif

int main()
{
Array a;

a[10] = 15;
std::cout << "A[10] = " << a[10] << "!\n";

return 0;
}

-- 
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Ray Gardener]

How about C++11 threads?

Ray


On 5/5/2016, Thursday 12:43 PM, Mark Coletti wrote:
On Thu, May 5, 2016 at 11:15 AM, Even Rouault 
mailto:even.roua...@spatialys.com>> wrote:


[...]
That would be interesting if you (or anyone) could list which
C++11 features would be killer features
to justify the upgrade to C++11 vs the potential pains that such a
move might cause (especially
as I raised in other emails, given the fact that GDAL uses a
number of other libs and is used by a number
of other libs) [...]


This is an off the cuff list of C++-11 niceties, and by no means 
exhaustive:


- std::move()
- *lambda expressions*
- auto keyword
- native smart pointer support, such as std::unique_ptr and 
std::shared_ptr (though there's always the Boost fallback)

- concurrency API
- nullptr

Also, it's been many, many years since I last looked at GDAL/OGR 
source.  When I did I noted a lot of C-style I/O calls instead of 
using C++ equivalents.  Is that still the case?  If so, that's one 
area ripe for refactoring.


Cheers,

Mark
--
mcole...@gmail.com 



___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev



___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mark Coletti]

On Thu, May 5, 2016 at 11:15 AM, Even Rouault 
wrote:

> [...]
> That would be interesting if you (or anyone) could list which C++11
> features would be killer features
> to justify the upgrade to C++11 vs the potential pains that such a move
> might cause (especially
> as I raised in other emails, given the fact that GDAL uses a number of
> other libs and is used by a number
> of other libs) [...]


This is an off the cuff list of C++-11 niceties, and by no means exhaustive:

- std::move()
- *lambda expressions*
- auto keyword
- native smart pointer support, such as std::unique_ptr and std::shared_ptr
(though there's always the Boost fallback)
- concurrency API
- nullptr


Also, it's been many, many years since I last looked at GDAL/OGR source.
When I did I noted a lot of C-style I/O calls instead of using C++
equivalents.  Is that still the case?  If so, that's one area ripe for
refactoring.

Cheers,

Mark
-- 
mcole...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[David Strip]

  
  
On 5/5/2016 9:00 AM, Kurt Schwehr
  wrote:

Thanks!  I've integrated your derived class in the
  alternates section and Even's response about commenting on resize
  into the drawbacks line
  
  
  Can you provide (on the list would be best) a bit more on why
the vector suggestion makes the it less tempting to do pointer
tricks?


First I want to make clear that use of std::vector doesn't prevent
using pointer tricks. At least for me (and many others I've worked
with) the fact that something has been defined as a std::vector
creates a mental barrier to using an interface other than that
declared in std::vector. To a large degree the strength of this
barrier will reflect the programmer's feelings towards
object-orientation and all that goes with it. If you're an
old-school C programmer who thinks all this new C++ "O-O crap" is
for sissies, then the first thing you'll do is grab the address of
element zero and use all the nasty, obscure coding style you've
always used. (Are my prejudices showing? And for what it's worth, I
certainly qualify as old school by virtue of age, but like to think
I've learned something along the way.) If, on the other hand, you
actually program in C++ (as opposed to using the C++ compiler on C
code), you'll look at the object as a vector and use the interface.

Here's what I mean by "pointer tricks" - 

Imagine you have to do some operations on the diagonal of a square
matrix. Consider the readability of the two snippets:

// I'm assuming we defined our array as a vector of
  vectors.
  for (int i = 0; i < matrix_size;  ++i)
  {
      do something(the_matrix[i][i]); 
  }


vs.

//Since I would never do this myself, a "real C
  programmer" would almost certainly write this differently, esp.
  the test for the end of the loop, but you get the idea
  for (int * i = the_matrix; i < sizeof(the_matrix) +
  the_matrix;  i += matrix_size + 1))
  {
      do_something(i);
  }


naming the loop var as diag_element would help, but in the first
example it's clear that do_something() is operating on a diagonal
element. In the second example it's not clear what you're operating
on at all. 

  

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Damian Dixon]

I'm currently stuck on the following compilers:

   - Visual Studio 2010 SP1
   - GCC 4.7

C++11 is only partially supported with these compilers.

Regards
Damian


On 4 May 2016 at 23:51, Mateusz Loskot  wrote:

> On 4 May 2016 at 23:30, Kurt Schwehr  wrote:
>
>>
>> To start off the conversation, I wrote up a doc on changing large C
>> arrays on the stack to std::vectors to get this data off of the stack and
>> to simplify initialization.
>>
>
>
> Since many, if not most, of the ideas rely on availability of the C++11
> features,
> it might be a good idea to first agree on C++11 as the lowest
> required C++ compilation mode.
>
> Let's poll if there are any users who require C++03 at all.
>
> Best regards,
> --
> Mateusz Loskot, http://mateusz.loskot.net
>
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Damian Dixon]

In the VS World...

If you delete C/C++ memory/objects in a DLL that has been compiled with the
same VS compiler you are ok.

If you attempt to delete a memory/object in DLL/application compiled with a
different version of the VS compiler all bets are off.

For example:

Allocate an object in VS2010 compiled code delete in VS2015 compiled code,
you will likely crash.


You will have to distribute the runtime distribution DLLs for the versions
of VS that you used or your 3rd party DLLs used.

Regards
Damian


On 5 May 2016 at 10:28, Even Rouault  wrote:

> Le jeudi 05 mai 2016 11:09:48, Mateusz Loskot a écrit :
> > On 5 May 2016 at 01:45, Even Rouault  wrote:
> > > Of the potential issues with requiring C++11, I can think of OSGeo4W.
> It
> > > is mostly(completely?) built with Visual Studio 2010. And from
> > > https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of
> C++11
> > > is only partial in VS 2010
> >
> > VS2013 is the lowest version sensible to consider regarding C++11
> support.
> >
> > > On the other hand regarding dependencies of GDAL, the binary propritary
> > > SDKs with a C++ API could be a problem, although they will likely move
> > > on too. - FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
> > > - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
> >
> > AFAICT, GDAL built using VS2015 links against the 5.2.1 version fine.
>
> Do you link against the VS2013 .lib ?
> >
> > > - MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
> >
> > Similarly to the ECW above, version 9.1.0 links fine too.
> >
> > > But I'm not sure about the compatibility of C++11 build against
> non-C++11
> > > builds in the VS realm : can a GDAL C++11 build link against a library
> > > built without C++11 enabled ? Will not there be ABI problems ?
> >
> > Although mixing C run-time libraries is a bad idea generally,
> > kosher C APIs should be fine.
>
> I'm not too worried about C ABI. My mention of C++03 ABI vs C++11 ABI was
> for
> the dependencies with those SDK that we use through their C++ API. In GCC
> world, with same compiler but with different -std=... flags, there have
> been
> troubles ( https://gcc.gnu.org/wiki/Cxx11AbiCompatibility ,
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html ), so
> I
> was wondering how the situation is in the VS world.
> Actually since I mention this, the issue does apply to Linux too. Looking
> at
> the C++11 Linux build (
>
> https://github.com/rouault/gdal_coverage/blob/trunk_gcc4.8_stdc11/.travis.yml
> ) , I see I couldn't build against FileGDB with -std=c++11. This was a
> compilation issue with its headers, rather than a link/runtime issue.
>
>
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev
>
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Andrew Bell]

On Thu, May 5, 2016 at 10:15 AM, Even Rouault 
wrote:

>
> > As for the notion of replacing some allocations with vectors, well,
> great.
> > But the GDAL codebase is creaky in lots of places and to get too
> particular
> > about the advantage/disadvantage of vectors seems a little silly.  At
> some
> > point it's just reasonable to say that you're only going to support
> > compilers/systems made after some date/version.  People who don't want to
> > upgrade a system aren't harmed.  They always have access to an older
> > version of the library.
>
> (Sometimes they want the latest version of the lib for a particular
> feature on a ancient system.)
>

Yes, but I'm unsympathetic.  This is free software.  To make the system
less maintainable for developers often working for free and more difficult
for many other users for the sake of a few outliers is to me unappealing.
If people want a feature integrated into some old system, they can
certainly hire someone to do just that.

-- 
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

Thanks all for your comments!  I will try to write up a similar proposal
for C++11 based on what people have written.

I've integrated (and still working on) a number of the points from the
discussion so far on the stack storage issue into the doc and added a
section titled "Impact of the proposed change"

This local stack storage change is for a real need, so it's probably good
that I add a use case that explains why this change is important.  And I
>>believe<< (but am not 100% sure) that using std::vector is the strongest
solution for the majority of instances of large buffers on the stack in
gdal.

Use case:  (note that I can only share generals)

At Google, we have jobs that use GDAL running with thousands of cores and
>20 threads per process that run for days.  The overhead of having to allow
for bumping the stack size adds substantial overhead to all those jobs.
GDAL is the only thing driving the requirement for the larger stack.  For
people using GDAL on a desktop machine, the larger stack is unlikely to be
a real issue.  For anyone running large multithreaded jobs in the cloud,
the large stack is a real cost issue.  The overhead of vector and heap
allocation in all these cases I've looked at in GDAL so far is far smaller
than the cost from the stack size overhead.
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 05 mai 2016 16:49:37, Andrew Bell a écrit :
> On Thu, May 5, 2016 at 8:26 AM, Even Rouault 
> 
> wrote:
> > Might be a little trickier with GCC, particularly with the new GCC 5
> > C++11 *ABI* and the dual ABI thing
> > (https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html),
> > but if
> > I understand well, if people use the same GCC version and do not play
> > with _GLIBCXX_USE_CXX11_ABI, linking code compiled with different
> > -std=C++?? setting
> > should work.
> > 
> > I've confirmed it with the following experiment:
> > 
> > $ cat a.cpp
> > #include 
> > 
> > std::string foo()
> > {
> > 
> >   return std::string("foo");
> > 
> > }
> > 
> > With gcc 5.2, both
> > g++ -std=c++11 -fPIC -shared a.cpp -o liba.so
> > g++ -std=c++03 -fPIC -shared a.cpp -o liba.so
> > result in the following symbol to be exported:
> > _Z3fooB5cxx11v
> > 
> > whereas if you add -D_GLIBCXX_USE_CXX11_ABI=0, the symbol name is
> > _Z3foov, which is the same as if you compile with GCC 4.X
> > 
> > So the potential problems would be when linking a GCC 5 compiled GDAL
> > (whether
> > it be C++03 or C++11) against a GCC 4.X compiled lib (think of one of the
> > SDK), so this might already occur currently. Or reversely, if an
> > application/library compiled with GCC 4.X would link against GDAL
> > compiled with GCC 5.X
> 
> ABI compatibility is a mess: https://isocpp.org/files/papers/n4028.pdf
> 
> With that in mind, what you do inside your own codebase isn't much of an
> issue and I'm guessing that even if GDAL wants to be as accommodating as
> possible, maintainers of dependent libraries probably won't take such
> care.  In the end, if you're doing development and building things
> yourself, you need to make sure you use the same compiler and switches for
> everything if you're providing a C++ interface.  Is this a pain?  Yes.  Is
> it necessary?  Yes.
> 
> As for the notion of replacing some allocations with vectors, well, great.
> But the GDAL codebase is creaky in lots of places and to get too particular
> about the advantage/disadvantage of vectors seems a little silly.  At some
> point it's just reasonable to say that you're only going to support
> compilers/systems made after some date/version.  People who don't want to
> upgrade a system aren't harmed.  They always have access to an older
> version of the library. 

(Sometimes they want the latest version of the lib for a particular feature on 
a ancient system.)

> There is just so much cruft in GDAL that could
> benefit from use of the standard library and C++ 11 that I don't know where
> to start.

That would be interesting if you (or anyone) could list which C++11 features 
would be killer features
to justify the upgrade to C++11 vs the potential pains that such a move might 
cause (especially
as I raised in other emails, given the fact that GDAL uses a number of other 
libs and is used by a number
of other libs)


> That said, changing code that works just because you like this or that
> language feature always presents an opportunity for bugs to creep in.

I agree completely with that :-) Especially for parts of the code base that are
not or poorly tested :
http://rawgit.com/rouault/gdalautotest-coverage-results/master/coverage_html/index.html
 


-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 5 May 2016 at 16:31, Even Rouault  wrote:
> Le jeudi 05 mai 2016 16:14:34, David Strip a écrit :
>> On 5/4/2016 4:30 PM, Kurt Schwehr wrote:
>>
>> Drawbacks:
>>
>>
>> It is possible to change the size of the vector later on in the code
>> Vector has some storage overhead and bookkeeping that has to be done (but
>> often the compiler can probably optimize away most of that). TODO:
>> References that explain this? Resizing the array could break anything that
>> was using a C style array pointer to the vector’s data
>>
>>  Drawbacks one and three can be eliminated by deriving a class from vector
>> that hides resize, so there really is only the single drawback of storage
>> overhead and bookkeeping, which are often minor.
>
> I'd not find it very attractive to create our own class just to prevent
> resizing for such a simple thing as an array of integers... "std::vector
> oVals(256, 0)" is very readable. If it must not be resized for some reason, a
> comment should do.

An assertion, you mean :)

> One thing to keep in mind however is the potential impact of heap allocation
> in performance sensitive code paths.

Indeed.
I would divide Kurt's proposal to several phases, roughly:
1. Switch to C++11
2. Refactor: clean-up, clarify, simplify, make it less error prone
using C++11 idioms and features (i.e. int[256] => std::array)
3. Update coding guidelines
4. Measure and optimise using C++11 features with potential
functional/behavioural changes (yes, switch from stack to heap
allocation drags a behavioural change, e.g. exceptions).

Although it looks like a long and boring process, points 2 and 4 are
optional, IMO.
However, mixing plain refactoring with functional and behaviour
changes is a bad idea.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Andrew Bell]

On Thu, May 5, 2016 at 8:26 AM, Even Rouault 
wrote:

>
> Might be a little trickier with GCC, particularly with the new GCC 5 C++11
> *ABI* and the dual ABI thing
> (https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html),
> but if
> I understand well, if people use the same GCC version and do not play with
> _GLIBCXX_USE_CXX11_ABI, linking code compiled with different -std=C++??
> setting
> should work.
>
> I've confirmed it with the following experiment:
>
> $ cat a.cpp
> #include 
>
> std::string foo()
> {
>   return std::string("foo");
> }
>
> With gcc 5.2, both
> g++ -std=c++11 -fPIC -shared a.cpp -o liba.so
> g++ -std=c++03 -fPIC -shared a.cpp -o liba.so
> result in the following symbol to be exported:
> _Z3fooB5cxx11v
>
> whereas if you add -D_GLIBCXX_USE_CXX11_ABI=0, the symbol name is _Z3foov,
> which is the same as if you compile with GCC 4.X
>
> So the potential problems would be when linking a GCC 5 compiled GDAL
> (whether
> it be C++03 or C++11) against a GCC 4.X compiled lib (think of one of the
> SDK), so this might already occur currently. Or reversely, if an
> application/library compiled with GCC 4.X would link against GDAL compiled
> with GCC 5.X
>

ABI compatibility is a mess: https://isocpp.org/files/papers/n4028.pdf

With that in mind, what you do inside your own codebase isn't much of an
issue and I'm guessing that even if GDAL wants to be as accommodating as
possible, maintainers of dependent libraries probably won't take such
care.  In the end, if you're doing development and building things
yourself, you need to make sure you use the same compiler and switches for
everything if you're providing a C++ interface.  Is this a pain?  Yes.  Is
it necessary?  Yes.

As for the notion of replacing some allocations with vectors, well, great.
But the GDAL codebase is creaky in lots of places and to get too particular
about the advantage/disadvantage of vectors seems a little silly.  At some
point it's just reasonable to say that you're only going to support
compilers/systems made after some date/version.  People who don't want to
upgrade a system aren't harmed.  They always have access to an older
version of the library.  There is just so much cruft in GDAL that could
benefit from use of the standard library and C++ 11 that I don't know where
to start.

That said, changing code that works just because you like this or that
language feature always presents an opportunity for bugs to creep in.

-- 
Andrew Bell
andrew.bell...@gmail.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 05 mai 2016 16:14:34, David Strip a écrit :
> On 5/4/2016 4:30 PM, Kurt Schwehr wrote:
> 
> 
> 
> 
> 
> 
> 
> Drawbacks:
> 
> 
> It is possible to change the size of the vector later on in the code
> Vector has some storage overhead and bookkeeping that has to be done (but
> often the compiler can probably optimize away most of that). TODO:
> References that explain this? Resizing the array could break anything that
> was using a C style array pointer to the vector’s data
> 
>  Drawbacks one and three can be eliminated by deriving a class from vector
> that hides resize, so there really is only the single drawback of storage
> overhead and bookkeeping, which are often minor.

I'd not find it very attractive to create our own class just to prevent 
resizing for such a simple thing as an array of integers... "std::vector 
oVals(256, 0)" is very readable. If it must not be resized for some reason, a 
comment should do.

One thing to keep in mind however is the potential impact of heap allocation 
in performance sensitive code paths.


-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[David Strip]

  
  
On 5/4/2016 4:30 PM, Kurt Schwehr
  wrote:
  


  


Drawbacks:


  
It is possible to change the size of the vector later on in the code
  
  
Vector has some storage overhead and bookkeeping that has to be done (but often the compiler can probably optimize away most of that). TODO: References that explain this?
  
  
Resizing the array could break anything that was using a C style array pointer to the vector’s data
  

  

Drawbacks one and three can be eliminated by deriving a class from
vector that hides resize, so there really is only the single
drawback of storage overhead and bookkeeping, which are often minor.

Another benefit of the proposed approach is that it makes it less
tempting to use inscrutable pointer tricks for referencing array
entries.
  

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 05 mai 2016 14:22:03, Jeff McKenna a écrit :
> On 2016-05-05 9:10 AM, Jeff McKenna wrote:
> > On 2016-05-05 5:14 AM, Dmitry Baryshnikov wrote:
> >> Hi,
> >> 
> >> I think it's time to go forward and shift to C++11 in i.e. GDAL 2.2 and
> >> drop old staff as it was with Windows mobile support, VB, etc.
> >> Yes, it may be some regression in ABI, but this is less evil than
> >> support lot of ancient compilers.
> >> 
> >> During our work on cmake build system for GDAL I faced that some
> >> dependency library needs VC2013SP2 on Windows - i.e liblzma, as it needs
> >> C99.
> >> We worked around it by including some logic to cmake, that if VC
> >> compiler is less than specific version the support of dependent features
> >> will be disabled.
> >> 
> >> By the way, the repo with cmake for GDAL is here:
> >> https://github.com/nextgis-extra/lib_gdal
> >> The ubuntu ppa of libgdal2 build using cmake:
> >> https://launchpad.net/~nextgis/+archive/ubuntu/ppa/+packages
> >> Windows builds also using cmake: http://nextgis.ru/en/borsch/
> > 
> > About a month ago I made the hard decision to upgrade my entire MS4W
> > build environment from VC 2008 to 2015.  Driving this is that PHP 7 only
> > supports 2015.
> > 
> > The only GDAL dependent library not supported for VC 2015 is FileGDB
> > (I've seen an Esri developer state that 2015 SDK will be released with
> > their next FileGDB API upgrade).
> > 
> > MrSID 9.5.1 SDK works well with 2015.
> > 
> > I didn't try an ECW 5.x SDK, but the old 3.3 SDK works well with 2015.
> > 
> > Short story is I support future GDAL builds requiring a more recent
> > Visual Studio compiler (2013+), as other projects are already doing so.
> > 
> > For those packagers cringing in the thought of upgrading their whole
> > stack: join the club :)
> 
> I should also note that for compiling Python 3.5.x VC 2015 is the
> supported environment.  Yet another GDAL dependent library that favors
> an upgraded compiler on Windows.

Thanks for your feedback. Although I'd note that upgrading compiler version is 
one thing, and enabling C++11 mode *might* be another, particularly if there's 
a mix of C++ libs linking against each other. But from what I can read, it 
seems that with Visual Studio you don't have the equivalent of -std=c++?? of 
GCC, so if things are compiled with the same VS version, that should be OK.

Might be a little trickier with GCC, particularly with the new GCC 5 C++11 
*ABI* and the dual ABI thing 
(https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html), but if 
I understand well, if people use the same GCC version and do not play with 
_GLIBCXX_USE_CXX11_ABI, linking code compiled with different -std=C++?? setting 
should work.

I've confirmed it with the following experiment:

$ cat a.cpp 
#include 

std::string foo()
{
  return std::string("foo");
}

With gcc 5.2, both
g++ -std=c++11 -fPIC -shared a.cpp -o liba.so
g++ -std=c++03 -fPIC -shared a.cpp -o liba.so
result in the following symbol to be exported:
_Z3fooB5cxx11v

whereas if you add -D_GLIBCXX_USE_CXX11_ABI=0, the symbol name is _Z3foov, 
which is the same as if you compile with GCC 4.X

So the potential problems would be when linking a GCC 5 compiled GDAL (whether 
it be C++03 or C++11) against a GCC 4.X compiled lib (think of one of the 
SDK), so this might already occur currently. Or reversely, if an 
application/library compiled with GCC 4.X would link against GDAL compiled 
with GCC 5.X

Even

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Joaquim Luis]

I'm also updating my builds to support 2015 for a future inclusion in GMT.
There is, however, a bit storm ahead. Files created with HDF5.10 break  
compatibility with older versions and can't be read. Put netCDF in this  
bag as well. See GMT's #3495 for a recent case.


http://gmt.soest.hawaii.edu/boards/1/topics/3495



On 2016-05-05 5:14 AM, Dmitry Baryshnikov wrote:

Hi,

I think it's time to go forward and shift to C++11 in i.e. GDAL 2.2 and
drop old staff as it was with Windows mobile support, VB, etc.
Yes, it may be some regression in ABI, but this is less evil than
support lot of ancient compilers.

During our work on cmake build system for GDAL I faced that some
dependency library needs VC2013SP2 on Windows - i.e liblzma, as it needs
C99.
We worked around it by including some logic to cmake, that if VC
compiler is less than specific version the support of dependent features
will be disabled.

By the way, the repo with cmake for GDAL is here:
https://github.com/nextgis-extra/lib_gdal
The ubuntu ppa of libgdal2 build using cmake:
https://launchpad.net/~nextgis/+archive/ubuntu/ppa/+packages
Windows builds also using cmake: http://nextgis.ru/en/borsch/


About a month ago I made the hard decision to upgrade my entire MS4W  
build environment from VC 2008 to 2015.  Driving this is that PHP 7 only  
supports 2015.


The only GDAL dependent library not supported for VC 2015 is FileGDB  
(I've seen an Esri developer state that 2015 SDK will be released with  
their next FileGDB API upgrade).


MrSID 9.5.1 SDK works well with 2015.

I didn't try an ECW 5.x SDK, but the old 3.3 SDK works well with 2015.

Short story is I support future GDAL builds requiring a more recent  
Visual Studio compiler (2013+), as other projects are already doing so.


For those packagers cringing in the thought of upgrading their whole  
stack: join the club :)


-jeff




___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Jeff McKenna]

On 2016-05-05 9:10 AM, Jeff McKenna wrote:

On 2016-05-05 5:14 AM, Dmitry Baryshnikov wrote:

Hi,

I think it's time to go forward and shift to C++11 in i.e. GDAL 2.2 and
drop old staff as it was with Windows mobile support, VB, etc.
Yes, it may be some regression in ABI, but this is less evil than
support lot of ancient compilers.

During our work on cmake build system for GDAL I faced that some
dependency library needs VC2013SP2 on Windows - i.e liblzma, as it needs
C99.
We worked around it by including some logic to cmake, that if VC
compiler is less than specific version the support of dependent features
will be disabled.

By the way, the repo with cmake for GDAL is here:
https://github.com/nextgis-extra/lib_gdal
The ubuntu ppa of libgdal2 build using cmake:
https://launchpad.net/~nextgis/+archive/ubuntu/ppa/+packages
Windows builds also using cmake: http://nextgis.ru/en/borsch/


About a month ago I made the hard decision to upgrade my entire MS4W
build environment from VC 2008 to 2015.  Driving this is that PHP 7 only
supports 2015.

The only GDAL dependent library not supported for VC 2015 is FileGDB
(I've seen an Esri developer state that 2015 SDK will be released with
their next FileGDB API upgrade).

MrSID 9.5.1 SDK works well with 2015.

I didn't try an ECW 5.x SDK, but the old 3.3 SDK works well with 2015.

Short story is I support future GDAL builds requiring a more recent
Visual Studio compiler (2013+), as other projects are already doing so.

For those packagers cringing in the thought of upgrading their whole
stack: join the club :)



I should also note that for compiling Python 3.5.x VC 2015 is the 
supported environment.  Yet another GDAL dependent library that favors 
an upgraded compiler on Windows.


-jeff




--
Jeff McKenna
MapServer Consulting and Training Services
http://www.gatewaygeomatics.com/




___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Jeff McKenna]

On 2016-05-05 5:14 AM, Dmitry Baryshnikov wrote:

Hi,

I think it's time to go forward and shift to C++11 in i.e. GDAL 2.2 and
drop old staff as it was with Windows mobile support, VB, etc.
Yes, it may be some regression in ABI, but this is less evil than
support lot of ancient compilers.

During our work on cmake build system for GDAL I faced that some
dependency library needs VC2013SP2 on Windows - i.e liblzma, as it needs
C99.
We worked around it by including some logic to cmake, that if VC
compiler is less than specific version the support of dependent features
will be disabled.

By the way, the repo with cmake for GDAL is here:
https://github.com/nextgis-extra/lib_gdal
The ubuntu ppa of libgdal2 build using cmake:
https://launchpad.net/~nextgis/+archive/ubuntu/ppa/+packages
Windows builds also using cmake: http://nextgis.ru/en/borsch/


About a month ago I made the hard decision to upgrade my entire MS4W 
build environment from VC 2008 to 2015.  Driving this is that PHP 7 only 
supports 2015.


The only GDAL dependent library not supported for VC 2015 is FileGDB 
(I've seen an Esri developer state that 2015 SDK will be released with 
their next FileGDB API upgrade).


MrSID 9.5.1 SDK works well with 2015.

I didn't try an ECW 5.x SDK, but the old 3.3 SDK works well with 2015.

Short story is I support future GDAL builds requiring a more recent 
Visual Studio compiler (2013+), as other projects are already doing so.


For those packagers cringing in the thought of upgrading their whole 
stack: join the club :)


-jeff




--
Jeff McKenna
MapServer Consulting and Training Services
http://www.gatewaygeomatics.com/




___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 5 May 2016 at 11:28, Even Rouault  wrote:
> Le jeudi 05 mai 2016 11:09:48, Mateusz Loskot a écrit :
>> On 5 May 2016 at 01:45, Even Rouault  wrote:
>> > Of the potential issues with requiring C++11, I can think of OSGeo4W. It
>> > is mostly(completely?) built with Visual Studio 2010. And from
>> > https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11
>> > is only partial in VS 2010
>>
>> VS2013 is the lowest version sensible to consider regarding C++11 support.
>>
>> > On the other hand regarding dependencies of GDAL, the binary propritary
>> > SDKs with a C++ API could be a problem, although they will likely move
>> > on too. - FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
>> > - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
>>
>> AFAICT, GDAL built using VS2015 links against the 5.2.1 version fine.
>
> Do you link against the VS2013 .lib ?

Yes, namely these:

ls -1 ERDAS_ECW_JPEG_2000\lib\vc120\Win32
NCSEcw.lib
NCSEcwd.lib
NCSEcwS.lib
NCSEcwSd.lib

>> > - MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
>>
>> Similarly to the ECW above, version 9.1.0 links fine too.
>>
>> > But I'm not sure about the compatibility of C++11 build against non-C++11
>> > builds in the VS realm : can a GDAL C++11 build link against a library
>> > built without C++11 enabled ? Will not there be ABI problems ?
>>
>> Although mixing C run-time libraries is a bad idea generally,
>> kosher C APIs should be fine.
>
> I'm not too worried about C ABI. My mention of C++03 ABI vs C++11 ABI was for
> the dependencies with those SDK that we use through their C++ API

Visual C++ does not promise ABI compatibility for anything but a
"portable type" [1]
- a C built-in type or a struct that contains only C built-in types

Mixing binaries built with different compiler versions or even
compiler/linker settings
is not recommended [2]. It might or might not work :)

[1] https://msdn.microsoft.com/en-us/library/hh438475.aspx
[2] https://msdn.microsoft.com/en-us/library/bb531344.aspx

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 05 mai 2016 11:09:48, Mateusz Loskot a écrit :
> On 5 May 2016 at 01:45, Even Rouault  wrote:
> > Of the potential issues with requiring C++11, I can think of OSGeo4W. It
> > is mostly(completely?) built with Visual Studio 2010. And from
> > https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11
> > is only partial in VS 2010
> 
> VS2013 is the lowest version sensible to consider regarding C++11 support.
> 
> > On the other hand regarding dependencies of GDAL, the binary propritary
> > SDKs with a C++ API could be a problem, although they will likely move
> > on too. - FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
> > - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
> 
> AFAICT, GDAL built using VS2015 links against the 5.2.1 version fine.

Do you link against the VS2013 .lib ?
> 
> > - MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
> 
> Similarly to the ECW above, version 9.1.0 links fine too.
> 
> > But I'm not sure about the compatibility of C++11 build against non-C++11
> > builds in the VS realm : can a GDAL C++11 build link against a library
> > built without C++11 enabled ? Will not there be ABI problems ?
> 
> Although mixing C run-time libraries is a bad idea generally,
> kosher C APIs should be fine.

I'm not too worried about C ABI. My mention of C++03 ABI vs C++11 ABI was for 
the dependencies with those SDK that we use through their C++ API. In GCC 
world, with same compiler but with different -std=... flags, there have been 
troubles ( https://gcc.gnu.org/wiki/Cxx11AbiCompatibility , 
https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html ), so I 
was wondering how the situation is in the VS world.
Actually since I mention this, the issue does apply to Linux too. Looking at 
the C++11 Linux build ( 
https://github.com/rouault/gdal_coverage/blob/trunk_gcc4.8_stdc11/.travis.yml 
) , I see I couldn't build against FileGDB with -std=c++11. This was a 
compilation issue with its headers, rather than a link/runtime issue.


-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 5 May 2016 at 01:45, Even Rouault  wrote:
> Of the potential issues with requiring C++11, I can think of OSGeo4W. It is
> mostly(completely?) built with Visual Studio 2010. And from
> https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11 is
> only partial in VS 2010

VS2013 is the lowest version sensible to consider regarding C++11 support.

> On the other hand regarding dependencies of GDAL, the binary propritary SDKs
> with a C++ API could be a problem, although they will likely move on too.
> - FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
> - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013

AFAICT, GDAL built using VS2015 links against the 5.2.1 version fine.

> - MrSID SDK: I didn't check. Perhaps Kirk can tell us ?

Similarly to the ECW above, version 9.1.0 links fine too.

> But I'm not sure about the compatibility of C++11 build against non-C++11
> builds in the VS realm : can a GDAL C++11 build link against a library built
> without C++11 enabled ? Will not there be ABI problems ?

Although mixing C run-time libraries is a bad idea generally,
kosher C APIs should be fine.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Peter Halls]

I am a humble geologist, rather than a Computer Scientist, and do not
pretend to understand all the ins-and-outs of this type of discussion - and
hence read, in the hopes of learning, but otherwise keep quiet!

I suspect that I am also something of a 'throw-back' in that I continue to
use Simula as my primary programming language.  Today's Simula compilers
are 'cross-compilers', in that they generate code in c, which is then
compiled and linked with an appropriate c compiler (it actually makes
providing software for multiple platforms a lot easier ...).

Having said that, my only concern with the use of (C++) Vector structures,
rather than c Arrays, would be the mappability required between application
language and implementation language.  If the C++ structures are universal,
fine.  If not, then either interface code (needing maintenance) or c
alternatives would be needed.  My example is based on Simula: it would be
interesting to know how Java or Python might interact.

Best wishes,

Peter


On 5 May 2016 at 00:45, Even Rouault  wrote:

> Le jeudi 05 mai 2016 00:51:28, Mateusz Loskot a écrit :
> > On 4 May 2016 at 23:30, Kurt Schwehr  wrote:
> > > To start off the conversation, I wrote up a doc on changing large C
> > > arrays on the stack to std::vectors to get this data off of the stack
> > > and to simplify initialization.
> >
> > Since many, if not most, of the ideas rely on availability of the C++11
> > features,
> > it might be a good idea to first agree on C++11 as the lowest
> > required C++ compilation mode.
> >
> > Let's poll if there are any users who require C++03 at all.
>
> I think there are different topics in what Kurt proposes :
> - style and other changes that are not bound to a particular compiler
> version
> - changes potentially dependant of the C++ version
>
> Of the potential issues with requiring C++11, I can think of OSGeo4W. It is
> mostly(completely?) built with Visual Studio 2010. And from
> https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11
> is
> only partial in VS 2010
>
> Regarding the current use of VS2010 in OSGeo4W, this is discussed here :
> https://lists.osgeo.org/pipermail/discuss/2016-February/015658.html
>
> Potentially we could imagine having GDAL compiled with a later VS version
> and
> have the rest using VS2010, since most (all?) other software in OSGeo4W
> use it
> probably through its C API. Hum, actually that must not be true for OTB
> that
> uses the C++ API I think. Perhaps Jürgen has something to add regarding
> this
> compiler issue ?
>
> On the other hand regarding dependencies of GDAL, the binary propritary
> SDKs
> with a C++ API could be a problem, although they will likely move on too.
> - FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
> - ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
> - MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
> But I'm not sure about the compatibility of C++11 build against non-C++11
> builds in the VS realm : can a GDAL C++11 build link against a library
> built
> without C++11 enabled ? Will not there be ABI problems ?
>
> Even
>
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
> ___
> gdal-dev mailing list
> gdal-dev@lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/gdal-dev




-- 

Peter J Halls, PhD Student, Post-war Reconstruction and Development Unit
(PRDU),
  University of York

Snail mail: PRDU, Derwent College, University of York,
Heslington, York YO10 5DD
This message has the status of a private and personal communication

___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Dmitry Baryshnikov]

Hi,

I think it's time to go forward and shift to C++11 in i.e. GDAL 2.2 and 
drop old staff as it was with Windows mobile support, VB, etc.
Yes, it may be some regression in ABI, but this is less evil than 
support lot of ancient compilers.


During our work on cmake build system for GDAL I faced that some 
dependency library needs VC2013SP2 on Windows - i.e liblzma, as it needs 
C99.
We worked around it by including some logic to cmake, that if VC 
compiler is less than specific version the support of dependent features 
will be disabled.


By the way, the repo with cmake for GDAL is here: 
https://github.com/nextgis-extra/lib_gdal
The ubuntu ppa of libgdal2 build using cmake: 
https://launchpad.net/~nextgis/+archive/ubuntu/ppa/+packages

Windows builds also using cmake: http://nextgis.ru/en/borsch/

Please note, that most but not all drivers are available now (for 
details see readme in repo) and only python bindings.


Best regards,
Dmitry

05.05.2016 02:45, Even Rouault пишет:

Le jeudi 05 mai 2016 00:51:28, Mateusz Loskot a écrit :

On 4 May 2016 at 23:30, Kurt Schwehr  wrote:

To start off the conversation, I wrote up a doc on changing large C
arrays on the stack to std::vectors to get this data off of the stack
and to simplify initialization.

Since many, if not most, of the ideas rely on availability of the C++11
features,
it might be a good idea to first agree on C++11 as the lowest
required C++ compilation mode.

Let's poll if there are any users who require C++03 at all.

I think there are different topics in what Kurt proposes :
- style and other changes that are not bound to a particular compiler version
- changes potentially dependant of the C++ version

Of the potential issues with requiring C++11, I can think of OSGeo4W. It is
mostly(completely?) built with Visual Studio 2010. And from
https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11 is
only partial in VS 2010

Regarding the current use of VS2010 in OSGeo4W, this is discussed here :
https://lists.osgeo.org/pipermail/discuss/2016-February/015658.html

Potentially we could imagine having GDAL compiled with a later VS version and
have the rest using VS2010, since most (all?) other software in OSGeo4W use it
probably through its C API. Hum, actually that must not be true for OTB that
uses the C++ API I think. Perhaps Jürgen has something to add regarding this
compiler issue ?

On the other hand regarding dependencies of GDAL, the binary propritary SDKs
with a C++ API could be a problem, although they will likely move on too.
- FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
- ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
- MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
But I'm not sure about the compatibility of C++11 build against non-C++11
builds in the VS realm : can a GDAL C++11 build link against a library built
without C++11 enabled ? Will not there be ABI problems ?

Even



___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Even Rouault]

Le jeudi 05 mai 2016 00:51:28, Mateusz Loskot a écrit :
> On 4 May 2016 at 23:30, Kurt Schwehr  wrote:
> > To start off the conversation, I wrote up a doc on changing large C
> > arrays on the stack to std::vectors to get this data off of the stack
> > and to simplify initialization.
> 
> Since many, if not most, of the ideas rely on availability of the C++11
> features,
> it might be a good idea to first agree on C++11 as the lowest
> required C++ compilation mode.
> 
> Let's poll if there are any users who require C++03 at all.

I think there are different topics in what Kurt proposes :
- style and other changes that are not bound to a particular compiler version
- changes potentially dependant of the C++ version

Of the potential issues with requiring C++11, I can think of OSGeo4W. It is 
mostly(completely?) built with Visual Studio 2010. And from 
https://msdn.microsoft.com/en-us/library/hh567368.aspx , support of C++11 is 
only partial in VS 2010

Regarding the current use of VS2010 in OSGeo4W, this is discussed here : 
https://lists.osgeo.org/pipermail/discuss/2016-February/015658.html

Potentially we could imagine having GDAL compiled with a later VS version and 
have the rest using VS2010, since most (all?) other software in OSGeo4W use it 
probably through its C API. Hum, actually that must not be true for OTB that 
uses the C++ API I think. Perhaps Jürgen has something to add regarding this 
compiler issue ?

On the other hand regarding dependencies of GDAL, the binary propritary SDKs 
with a C++ API could be a problem, although they will likely move on too. 
- FileGDB SDK 1.4: available for VS2010, VS2012, VS2013
- ECW SDK 5.2.1: available for VS2010, VS2012, VS2013
- MrSID SDK: I didn't check. Perhaps Kirk can tell us ?
But I'm not sure about the compatibility of C++11 build against non-C++11 
builds in the VS realm : can a GDAL C++11 build link against a library built 
without C++11 enabled ? Will not there be ABI problems ?

Even

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Re: [gdal-dev] Starting a discussion on style and coding guidelines

[Mateusz Loskot]

On 4 May 2016 at 23:30, Kurt Schwehr  wrote:

>
> To start off the conversation, I wrote up a doc on changing large C arrays
> on the stack to std::vectors to get this data off of the stack and to
> simplify initialization.
>


Since many, if not most, of the ideas rely on availability of the C++11
features,
it might be a good idea to first agree on C++11 as the lowest
required C++ compilation mode.

Let's poll if there are any users who require C++03 at all.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

[gdal-dev] Starting a discussion on style and coding guidelines

[Kurt Schwehr]

Hi all,

If you've been watching the timeline on trac, you have probably seen a
large number of cleanup CLs from me.  It's definitely past time to get some
discussion going on these changes.  If the community likes these, we can
add them to rfc8_devguide .
We also need to consider tools that help committers push code towards what
we want and keep it there once it conforms to those guides (e.g.
clang-format with a custom GDAL specification)  If you haven't see one of
the changes I've committed yet, here is an example to check out... this
change to geotiff.cpp is likely the biggest I will ever do.

https://trac.osgeo.org/gdal/changeset/34130/

I personally think the goals of style / coding guidelines are:

0. Ensure that GDAL is around for a long time to come
1. Make the code easier to read (which particular patterns to use and
consistent formatting)
2. When the code has trouble or data is bad, fail with error messages
rather than crash
3. Make it easier for debuggers to present sensible state
4. Help static (clang static analyzer, Coverity, etc. ) and dynamic (e.g.
fuzzers) analyzers perform better
5. Make it easier for authors to contribute code that plays well with the
rest of system with the least possible effort

To start off the conversation, I wrote up a doc on changing large C arrays
on the stack to std::vectors to get this data off of the stack and to
simplify initialization.  I've attached the doc inline, but it is also
available here:

goo.gl/vuA3D6

The full url is:
https://docs.google.com/document/d/1Y9flzxj3Uz1vTEPCBmlswgi470m8i-oepGutVkbowfc/pub

It would be great to get people to have at commenting on this proposal.  If
you all like this kind of thing, we can more of them and work them into a
form that extends RFC 8.  Please don't be shy in commenting any way you
feel is constructive on this doc.  We can discuss on this list or if people
really want, I can give individuals comment access to the Google doc.

cheers,
-kurt


Use vector(length, initial_value) for local blocks of storage.

Author: Kurt Schwehr schw...@google.com / schw...@gmail.com (goatbar)

Date: 2016-May-04

Consider:

  int anVals[256];

  memset(anVals, 0, 256*sizeof(int));

This is less than optimal because:


   -

   It puts 1K or 2K on the stack, some of us (who can have huge numbers of
   threads) try to keep stack usage to 16K per thread
   -

   You have two places that need to know the size
   -

   The variable exists uninitialized between the definition and
   initialization
   -

   memset is a C API, which is able to do fewer sanity checks
   -

   No bounds checking


In GDAL, there are >700 stack allocations of more than 100 items and > 150
of 1000 or more items.  There are 877 calls to memset in the C++ code.

Proposed solution:

 https://trac.osgeo.org/gdal/changeset/34177

 std::vector oVals(256, 0);

Benefits:


   -

   It can pretty much be used like a C array,  e.g., ++anVals[index]; works
   -

   Using vector gets most of the memory off of the stack and into the heap
   helping threaded programs
   -

   The values are all initialized when anVals is created
   -

   If functions need to get a pointer to a C style array, you can call
   Foo(&anVals[0])
   -

   Compilers can optimize the standard containers.
   -

   Bounds checking is possible without having to resort to ASAN (which not
   all compilers have) or Valgrind


Drawbacks:


   -

   It is possible to change the size of the vector later on in the code
   -

   Vector has some storage overhead and bookkeeping that has to be done
   (but often the compiler can probably optimize away most of that). TODO:
   References that explain this?
   -

   Resizing the array could break anything that was using a C style array
   pointer to the vector’s data


Alternatives

Might be better or worse in different ways:


   -

   Use std::fill instead of memset
   -

   Declare and pass around the array size as a constant, e.g., const int
   knValsSize = 256.
   -

   CPL_ARRAY_SIZE to only have one 256 around
   -

   Use CPLCalloc to put it on the heap and initialize
   -

   Use new[] to put it on the heap
   -

   Use std::array .  Introduced
   in C++ 11, so currently disallowed.  Still puts the payload on the stack.
   -

   Use std::unique_ptr 
   to put it on the heap. Introduced in C++ 11, so currently disallowed
   -

   Add autoptr to GDAL to put it on the heap.  autoptr is very similar to
   unique_ptr, but works with C++03
   -

   Use some other library like boost.  That would require a major change to
   GDALs build requirements.
   -

   int anVals[256] = { 0 };  Only initializes the first element or you must
   specify all 256 elements.
   -

   Make our own simple class that handles allocation on the heap and cleans
   up in the destructor and provides operator[]


Key GDAL facts to keep in mind: