Header inclusion guards

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
Hi,

As it was discussed and agreed off-list, I've started refactoring
existing inclusion guards. I decided to follow MPL and other Boost
libraries and generate guards based on physical path to a file
instead of the logical one - namespaces.

So, here is the convention I'm applying:

GGL_<DIR1>_<DIRN>_<FILE>_HPP


Example, header

ggl/geometry/strategies/distance_result.hpp

is gaurded with:

GGL_GEOMETRY_STRATEGIES_DISTANCE_RESULT_HPP

I don't append _INCLUDED suffix, because such guards are long
enough to consider them as (pseudo-)unique.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels
Hi Mateusz,


> As it was discussed and agreed off-list, I've started refactoring
> existing inclusion guards. I decided to follow MPL and other Boost
> libraries and generate guards based on physical path to a file
> instead of the logical one - namespaces.
>
> So, here is the convention I'm applying:
>
> GGL_<DIR1>_<DIRN>_<FILE>_HPP
>
>
> Example, header
>
> ggl/geometry/strategies/distance_result.hpp
>
> is gaurded with:
>
> GGL_GEOMETRY_STRATEGIES_DISTANCE_RESULT_HPP
>
> I don't append _INCLUDED suffix, because such guards are long
> enough to consider them as (pseudo-)unique.
>  
I realize that you already started (I see about 15 updates in
arithmetic, strategies). The GGL_GEOMETRY seems a bit redudant to me.
Don't know how far you are besides the commits. Is it still possible to
change it in GGL_STRATEGIES_DISTANCE_RESULT_HPP? Makes it somewhat
shorter and less redundant.

Regards, Barend



Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
Barend Gehrels wrote:

> Hi Mateusz,
>
>
>> As it was discussed and agreed off-list, I've started refactoring
>> existing inclusion guards. I decided to follow MPL and other Boost
>> libraries and generate guards based on physical path to a file
>> instead of the logical one - namespaces.
>>
>> So, here is the convention I'm applying:
>>
>> GGL_<DIR1>_<DIRN>_<FILE>_HPP
>>
>>
>> Example, header
>>
>> ggl/geometry/strategies/distance_result.hpp
>>
>> is gaurded with:
>>
>> GGL_GEOMETRY_STRATEGIES_DISTANCE_RESULT_HPP
>>
>> I don't append _INCLUDED suffix, because such guards are long
>> enough to consider them as (pseudo-)unique.
>
> I realize that you already started (I see about 15 updates in
> arithmetic, strategies). The GGL_GEOMETRY seems a bit redudant to me.
>  Don't know how far you are besides the commits. Is it still possible
> to change it in GGL_STRATEGIES_DISTANCE_RESULT_HPP? Makes it somewhat
>  shorter and less redundant.

Hi Barend,

I've applied fixes only to a few files so far, so it's not too late.
However, thinking of Boost'ification, I'm not sure, but we may need
to change it again.

Layout of Boost libraries is as follows:
- headers
<root>/boost/<lib>/
- translation units, tests, examples, doc
<root>/libs/<lib>/test
<root>/libs/<lib>/example
<root>/libs/<lib>/doc
<root>/libs/<lib>/...

So, we may need to re-layout GGL as follows:

- headers
ggl/
ggl/core
ggl/strategies
ggl/geometries
...
- no .cpp units, but we have tests, examples, doc
libs/ggl/...

This way it will be very easy to integrated ggl headers and
libs/ggl into Boost tree.

Given that, if we agree to re-structure ggl, then inclusion guards will
look exactly as you are suggesting:

GGL_STRATEGIES_DISTANCE_RESULT_HPP
GGL_GEOMETRIES_...
etc.

Perhaps it seems a cosmetic, but I wouldn't be sure if it is.
Many of Boost conventions have been already applied to GGL,
so why not to apply Boost structure too, especially if GGL
is going to be merged into Boost one day.

Shortly, the point is to not to do the same work twice :-)

What do you think?

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels

>
>
> I've applied fixes only to a few files so far, so it's not too late.
OK, good

> However, thinking of Boost'ification, I'm not sure, but we may need
> to change it again.
>
> Layout of Boost libraries is as follows:
> - headers
> <root>/boost/<lib>/
> - translation units, tests, examples, doc
> <root>/libs/<lib>/test
> <root>/libs/<lib>/example
> <root>/libs/<lib>/doc
> <root>/libs/<lib>/...
Sure, I realized that making the last preview and creating the boost
sandbox. We should change that. This change will need to adapt all
project files and header files and maybe more. But it is necessary.
Let's plan a date for this.
>
> Given that, if we agree to re-structure ggl, then inclusion guards will
> look exactly as you are suggesting:
>
> GGL_STRATEGIES_DISTANCE_RESULT_HPP
> GGL_GEOMETRIES_...
> etc.
OK, so at least that is right then.
>
> Perhaps it seems a cosmetic, but I wouldn't be sure if it is.
> Many of Boost conventions have been already applied to GGL,
> so why not to apply Boost structure too, especially if GGL
> is going to be merged into Boost one day.
Sure. The name GGL is adapted last february, that is why all headerfiles
refer to geometry/ (in fact the name ggl was very old also but then
stood for Geodan Geographic Library)


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090417/9c47e2c6/attachment.html
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
In reply to this post by Mateusz Loskot
Barend Gehrels wrote:

>> However, thinking of Boost'ification, I'm not sure, but we may need
>> to change it again.
>>
>> Layout of Boost libraries is as follows:
>> - headers
>> <root>/boost/<lib>/
>> - translation units, tests, examples, doc
>> <root>/libs/<lib>/test
>> <root>/libs/<lib>/example
>> <root>/libs/<lib>/doc
>> <root>/libs/<lib>/...
>
> Sure, I realized that making the last preview and creating the boost
> sandbox. We should change that. This change will need to adapt all
> project files and header files and maybe more. But it is necessary.

Yes and I'm willing to apply or help in applying relevant changes.

> Let's plan a date for this.

As I imagine this is related to the schedule of issuing previews,
I'd leave the planning to you and Bruno :-)

>> Given that, if we agree to re-structure ggl, then inclusion guards will
>> look exactly as you are suggesting:
>>
>> GGL_STRATEGIES_DISTANCE_RESULT_HPP
>> GGL_GEOMETRIES_...
>> etc.
 >
> OK, so at least that is right then.


That's right.

>> Perhaps it seems a cosmetic, but I wouldn't be sure if it is.
>> Many of Boost conventions have been already applied to GGL,
>> so why not to apply Boost structure too, especially if GGL
>> is going to be merged into Boost one day.
>
> Sure. The name GGL is adapted last february, that is why all headerfiles
> refer to geometry/ (in fact the name ggl was very old also but then
> stood for Geodan Geographic Library)

Got it.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels
In reply to this post by Mateusz Loskot
(sent earlier but meant for the list)

>
>
> I've applied fixes only to a few files so far, so it's not too late.
OK, good

> However, thinking of Boost'ification, I'm not sure, but we may need
> to change it again.
>
> Layout of Boost libraries is as follows:
> - headers
> <root>/boost/<lib>/
> - translation units, tests, examples, doc
> <root>/libs/<lib>/test
> <root>/libs/<lib>/example
> <root>/libs/<lib>/doc
> <root>/libs/<lib>/...
Sure, I realized that making the last preview and creating the boost
sandbox. We should change that. This change will need to adapt all
project files and header files and maybe more. But it is necessary.
Let's plan a date for this.
>
> Given that, if we agree to re-structure ggl, then inclusion guards will
> look exactly as you are suggesting:
>
> GGL_STRATEGIES_DISTANCE_RESULT_HPP
> GGL_GEOMETRIES_...
> etc.
OK, so at least that is right then.
>
> Perhaps it seems a cosmetic, but I wouldn't be sure if it is.
> Many of Boost conventions have been already applied to GGL,
> so why not to apply Boost structure too, especially if GGL
> is going to be merged into Boost one day.
Sure. The name GGL is adapted last february, that is why all headerfiles
refer to geometry/ (in fact the name ggl was very old also but then
stood for Geodan Geographic Library)





Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
In reply to this post by Mateusz Loskot
Mateusz Loskot wrote:

> Barend Gehrels wrote:
>>> However, thinking of Boost'ification, I'm not sure, but we may need
>>> to change it again.
>>>
>>> Layout of Boost libraries is as follows:
>>> - headers
>>> <root>/boost/<lib>/
>>> - translation units, tests, examples, doc
>>> <root>/libs/<lib>/test
>>> <root>/libs/<lib>/example
>>> <root>/libs/<lib>/doc
>>> <root>/libs/<lib>/...
>>
>> Sure, I realized that making the last preview and creating the boost
>> sandbox. We should change that. This change will need to adapt all
>> project files and header files and maybe more. But it is necessary.
>
> Yes and I'm willing to apply or help in applying relevant changes.
>
>> Let's plan a date for this.
>
> As I imagine this is related to the schedule of issuing previews,
> I'd leave the planning to you and Bruno :-)

I've just noticed that if we decide to boostify files structure,
#include directives in most (if not all) headers will need to be
updated, because now all refer to geometry as root of include
directory.

gcc.compile.c++ bin/area.test/gcc-4.3.3/debug/area.o
area.cpp:13:40: error: geometry/algorithms/area.hpp: No such file or
directory
area.cpp:14:45: error: geometry/algorithms/transform.hpp: No such file
or directory

As far as I'm correct, this

#include <geometry/algorithms/area.hpp>

need to be changed to read:

#include <ggl/algorithms/area.hpp>

Then, after GGL will be merged with Boost, it will be simple to
update include paths by search "<ggl" and replace with "<boost/ggl"

Is that OK? Are we ready to plan such update?

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels

> I've just noticed that if we decide to boostify files structure,
> #include directives in most (if not all) headers will need to be
> updated, because now all refer to geometry as root of include
> directory.
>  
Sure, actually that was what I meant in last e-mail by changing all
header files and project files.
> (...)
>
> Then, after GGL will be merged with Boost, it will be simple to
> update include paths by search "<ggl" and replace with "<boost/ggl"
>
> Is that OK? Are we ready to plan such update?
>  
I can do it Friday 25 april, OK for everybody? This means changing to
"<ggl". Some boost sandboxes contain stuff which already starts with
"<boost", there is no guideline about that, it is allowed. However I
prefer to wait until (and if) it is accepted

Barend



Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels

>>
>> Then, after GGL will be merged with Boost, it will be simple to
>> update include paths by search "<ggl" and replace with "<boost/ggl"
>>
>> Is that OK? Are we ready to plan such update?
>>  
One thing more about this: the namespace. Should it be changed as well?
This will have (even) more influence on everything (including on users,
though they can alias).

I think we can leave it just "geometry::", it is not necessary to say
"ggl::", meaning "generic geometry library::", in each phrase. On the
other hand: it is mpl:: and gil::. The BGL (boost graph library) does
not live in a namespace and is in the folder "boost/graph". There is
probably no uniform convention about this. But most namespaces are named
the same as their folders.

What is your opinion? Bruno?

Regards, Barend


Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
In reply to this post by Barend Gehrels
Barend Gehrels wrote:

>> Then, after GGL will be merged with Boost, it will be simple to
>> update include paths by search "<ggl" and replace with "<boost/ggl"
>>
>>
>> Is that OK? Are we ready to plan such update?
>
> I can do it Friday 25 april, OK for everybody? This means changing to
>  "<ggl". Some boost sandboxes contain stuff which already starts with
>  "<boost", there is no guideline about that, it is allowed. However I
>  prefer to wait until (and if) it is accepted

Sure.

The idea behind this proposal is to re-make its structure
assuming as GGL was already a member of Boost libraries tree:

<root>/boost/ggl - for headers
<root>/libs/ggl  - for all the rest

So, if it happens, migration "shock" is less painful to us
and to users :-)

Best regards
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
In reply to this post by Barend Gehrels
Barend Gehrels wrote:
>>> Then, after GGL will be merged with Boost, it will be simple to
>>> update include paths by search "<ggl" and replace with
>>> "<boost/ggl"
>>>
>>> Is that OK? Are we ready to plan such update?
>
> One thing more about this: the namespace. Should it be changed as
> well?

I'd stick to Boost best practice and rename to ggl.
Perhaps there may be another library in the Boost collection
that includes (sub)namespace called geometry then name might be
ambiguous to users.

By the way, Boost Header Policy at
http://www.boost.org/development/header.html says:

The sample header uses the Boost convention of all uppercase letters,
with the header name prefixed by the namespace name, and suffixed with
HPP, separated by underscores.

> I think we can leave it just "geometry::", it is not necessary to say
>  "ggl::", meaning "generic geometry library::", in each phrase. On
> the other hand: it is mpl:: and gil::. The BGL (boost graph library)
> does not live in a namespace and is in the folder "boost/graph".

The name of the library id Graph Library, however after it was submitted
to Boost, it got prefixed with Boost what makes Boost Graph Library.
This naming is mirrored in namespaces layout:

Boost -> boost::

Boost Graph -> boost::graph

Graph is a single word name. GGL, as GIL, is multi-word name.
So, GIL variant seems to fit GGL best and it could be defined as:

- incl. guards
GGL_<FILE>_HPP
GGL_<DIR>_<FILE>_HPP

- namespaces

boost::ggl
boost::ggl::algorithms
boost::ggl::geometries
...

- directories

<boost_root>/boost/ggl
<boost_root>/libs/ggl
<boost_root>/libs/ggl/doc
<boost_root>/libs/ggl/example
<boost_root>/libs/ggl/test

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org

--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels

>
>> One thing more about this: the namespace. Should it be changed as
>> well?
>>    
>
> I'd stick to Boost best practice and rename to ggl. [snipped] GIL variant seems to fit GGL best
OK, I'll think about it.

One thing: if we (ever) merge the geometry cores with e.g. GTL we have
to reconsider this. But that influences probably only the core part.

Barend
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090419/18c8ccd4/attachment.html
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
Barend Gehrels wrote:
>>> One thing more about this: the namespace. Should it be changed as
>>>  well?
>>>
>>
>> I'd stick to Boost best practice and rename to ggl. [snipped] GIL
>> variant seems to fit GGL best
>
> OK, I'll think about it.

You're in charge. I just offer myself to do the dirty work ;-)

> One thing: if we (ever) merge the geometry cores with e.g. GTL we
> have to reconsider this. But that influences probably only the core
> part.

What is GTL?

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Barend Gehrels

> What is GTL?
>
There is more geometry-activity within Boost and the Boost mailing list.
The most serious other one is GTL:
https://svn.boost.org/svn/boost/sandbox/gtl 
<https://svn.boost.org/svn/boost/sandbox/gtl/>

It used to be orientated to integer arithmetic and 45/90 degree polygons
(for VSLI) but the author (Luke, from Intel) is changing this and taking
a more generic approach. He announced a review next month but do not
know the current state.

Barend

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090420/df03a966/attachment.html
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
Barend Gehrels wrote:

>
>> What is GTL?
>>
> There is more geometry-activity within Boost and the Boost mailing
> list. The most serious other one is GTL:
> https://svn.boost.org/svn/boost/sandbox/gtl
>
> It used to be orientated to integer arithmetic and 45/90 degree
> polygons (for VSLI) but the author (Luke, from Intel) is changing
> this and taking a more generic approach. He announced a review next
> month but do not know the current state.

I this one. Hehe, it looks like it's easy to get distracted
with all the new geometry engines :-)

I may take a look at GTL for inspiration, but unless
there is a plan to 'steal' some of its concepts
or merging, I'm not going to invest substantial amount of time in
researching this project.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Bruno Lalande
> I may take a look at GTL for inspiration, but unless
> there is a plan to 'steal' some of its concepts
> or merging, I'm not going to invest substantial amount of time in
> researching this project.

There's actually a project of merging. I have initiated it recently on
the Boost list. The idea would be to take the core concepts of both
libraries and extract one common core from them, and to have the
remaining (algorithms, geometries, concepts models) using this common
core. The remaining would thus be kept separated at first, and the
common core could be proposed to Boost. Then the remaining could be
integrated, as extensions.

This common core would be some sort of "library of concepts", with
mechanisms to easily add new geometry types and integrate them in the
existing ones. For example, it has to be possible to add a "triangle"
type that takes advantage of all the algorithms existing on "polygon"
and specializes some of them. The first decision we'll have to do will
be to choose between tag dispatching and SFINAE for the distribution
of algorithms overloads among geometry types.

Though I didn't have much time to dig this since my proposal, I still
have the intention to do it. People on the list and authors of both
libraries were interested by the idea.

Regards
Bruno
Reply | Threaded
Open this post in threaded view
|

Header inclusion guards

Mateusz Loskot
Administrator
Bruno Lalande wrote:
>> I may take a look at GTL for inspiration, but unless
>> there is a plan to 'steal' some of its concepts
>> or merging, I'm not going to invest substantial amount of time in
>> researching this project.
>
> There's actually a project of merging.

OK, then the situation has changed :-)
If merging is beneficial, as it seems to be, then I'm OK with it.

> I have initiated it recently on
> the Boost list. The idea would be to take the core concepts of both
> libraries and extract one common core from them, and to have the
> remaining (algorithms, geometries, concepts models) using this common
> core. The remaining would thus be kept separated at first, and the
> common core could be proposed to Boost. Then the remaining could be
> integrated, as extensions.

Sounds as a good idea.

> This common core would be some sort of "library of concepts", with
> mechanisms to easily add new geometry types and integrate them in the
> existing ones. For example, it has to be possible to add a "triangle"
> type that takes advantage of all the algorithms existing on "polygon"
> and specializes some of them. The first decision we'll have to do will
> be to choose between tag dispatching and SFINAE for the distribution
> of algorithms overloads among geometry types.

OK, I'll monitor the discussion.

> Though I didn't have much time to dig this since my proposal, I still
> have the intention to do it. People on the list and authors of both
> libraries were interested by the idea.

OK

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net