[GRASS-dev] Compiling r.viewshed and r.terraflow with C++11

GRASS GIS GRASS GIS developer
1

Hi,

the r.viewshed and r.terraflow modules needs fix to compile with C++11. The errors are:

error: reference to ‘is_empty’ is ambiguous
error: reference to ‘is_void’ is ambiguous

The cause are the functions in modules which are named in the same way as functions newly added in C++11. The problem is actually caused by the fact that modules are using

using namespace std;

As explained by Glynn in [1] rewriting modules in the way that they don’t use std namespace in this way could be difficult.

So my question is, was this fixed in the time of [1]? It is not clear from the discussion. If not, what is the right fix?

I suggest to put these functions into some namespace (I was not thinking about name yet) and using these functions with a namespace. There is only limited number of calls of these functions. Do you think it is a good idea?

Apparently, this issue needs to be fixed because on Mac OS X Mavericks it is already an issue (C++11 is the default as predicted by Glynn in [1]). Compile error should be reproducible with older GCC/clang by adding -std=c++11 option.

Vaclav

[1] Grass SVN in Android, display issue, http://lists.osgeo.org/pipermail/grass-dev/2012-September/060088.html, http://osgeo-org.1560.x6.nabble.com/Grass-SVN-in-Android-display-issue-td5002221.html

2

Vaclav Petras wrote:

So my question is, was this fixed in the time of [1]? It is not clear from
the discussion.

No.

If not, what is the right fix?

The right fix is to remove all occurrences of "using namespace std;",
then fix the (probably hundreds) of errors which arise, either by
using qualified names (e.g. std::vector) or by adding "using"
statements for individual names (e.g. "using std::vector;").

[Note that iostream uses templates, so it might matter which approach
is taken. A qualified name forces the use of a specific function (or
type, etc), whereas a "using" statement adds the name as a fallback
but argument-dependent lookup takes priority.]

A more comprehensive fix would be to simply not accept code written in
C++. It's a deceptively complex language, and writing portable C++
code is significantly harder than writing "works-on-my-system" code.
Consequently, a very large proportion of C++ code falls into the
latter category.

That isn't a problem if you're writing something for your personal use
and you're willing to abandon it once it no longer compiles on current
systems. OTOH, it is a problem for a project like GRASS, which aims to
support multiple platforms over the long term.

--
Glynn Clements <glynn@gclements.plus.com>

3

On Tue, Dec 10, 2013 at 5:41 AM, Glynn Clements <glynn@gclements.plus.com>wrote:

Vaclav Petras wrote:

> So my question is, was this fixed in the time of [1]? It is not clear
from
> the discussion.

No.

> If not, what is the right fix?

The right fix is to remove all occurrences of "using namespace std;",
then fix the (probably hundreds) of errors which arise, either by
using qualified names (e.g. std::vector) or by adding "using"
statements for individual names (e.g. "using std::vector;").

I tried to delete all "using namespace std;" but the error "

reference...ambiguous" was still there in slightly different form
(unfortunately I lost them, so I cannot cite them). I decided that I will
rename the functions in r58434. It seemed better than putting them into
namespace because the namespace would be just for those particular
functions or it would cause a lot of changes or "using yyy::xxx;" if it
would be applied to more functions/classes.

[r58434] https://trac.osgeo.org/grass/changeset/58434

[Note that iostream uses templates, so it might matter which approach

is taken. A qualified name forces the use of a specific function (or
type, etc), whereas a "using" statement adds the name as a fallback
but argument-dependent lookup takes priority.]

I was afraid that removing of overloading will cause some problems but it

compiles, runs and results seems ok (but I actually don't know what to test
except for the example in manual).

A more comprehensive fix would be to simply not accept code written in
C++. It's a deceptively complex language, and writing portable C++
code is significantly harder than writing "works-on-my-system" code.
Consequently, a very large proportion of C++ code falls into the
latter category.

That isn't a problem if you're writing something for your personal use
and you're willing to abandon it once it no longer compiles on current
systems. OTOH, it is a problem for a project like GRASS, which aims to
support multiple platforms over the long term.

If you think that this is an issue it should be discussed and resulting

policy should go to submitting rules.

--

Glynn Clements <glynn@gclements.plus.com>

4

Vaclav Petras wrote:

> The right fix is to remove all occurrences of "using namespace std;",
> then fix the (probably hundreds) of errors which arise, either by
> using qualified names (e.g. std::vector) or by adding "using"
> statements for individual names (e.g. "using std::vector;").
>
> I tried to delete all "using namespace std;" but the error "
reference...ambiguous" was still there in slightly different form

The issue was also present in the libiostream headers used by
r.terraflow and r.viewshed.

Try r58455.

--
Glynn Clements <glynn@gclements.plus.com>

5

On Fri, Dec 13, 2013 at 7:11 AM, Glynn Clements <glynn@gclements.plus.com>wrote:

Vaclav Petras wrote:

> > The right fix is to remove all occurrences of "using namespace std;",
> > then fix the (probably hundreds) of errors which arise, either by
> > using qualified names (e.g. std::vector) or by adding "using"
> > statements for individual names (e.g. "using std::vector;").
> >
> > I tried to delete all "using namespace std;" but the error "
> reference...ambiguous" was still there in slightly different form

The issue was also present in the libiostream headers used by
r.terraflow and r.viewshed.

Try r58455.

Thanks Glynn, I haven't noticed GRASS libiostream. Now it compiles (make
distclean && make). Tested with clang on Ubuntu and Mac.

Vaclav

For the record: r58434 is reverted in r58455 because r58455 makes r58434
unnecessary.

[r58434] https://trac.osgeo.org/grass/changeset/58434
[r58455] https://trac.osgeo.org/grass/changeset/58455

By the way, grepping "using namespace" now returns:

./imagery/i.atcorr/common.h:14:using namespace std;
./lib/iostream/mm_utils.cpp:41:using namespace std;
./lib/iostream/mm.cpp:44:using namespace std;

--
Glynn Clements <glynn@gclements.plus.com>

6

Vaclav Petras wrote:

By the way, grepping "using namespace" now returns:

./imagery/i.atcorr/common.h:14:using namespace std;

./lib/iostream/mm_utils.cpp:41:using namespace std;
./lib/iostream/mm.cpp:44:using namespace std;

Not sure how I missed the last two. Those are fixed by r58457.

i.atcorr to follow.

--
Glynn Clements <glynn@gclements.plus.com>

7

Glynn Clements wrote:

i.atcorr to follow.

Done in r58458.

--
Glynn Clements <glynn@gclements.plus.com>