Fix tui compilation with Solaris libcurses (PR tui/21482)

  Hi Pedro,

>>> Ok for mainline and 8.0 branch?
>>
>> The cast bits are OK.  I'd like to hear your opinion on
>> moving the NOMACROS define to gdb_curses.h, before including
>> <curses.h>.
>
> The move makes sense to me: I just wasn't aware of that file.  I'll
> prepare a separate patch.

I've checked in the cast part now.  Here's the NOMACROS part for
gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
amd64-pc-solaris2.12 (ncurses).  Ok too?

Interestingly, with that patch the previous link failure on Solaris
11/12 (missing wattr_on/wattr_off) is gone, too.  This seems to happen
because <ncurses/ncurses.h> no longer defines

/usr/include/ncurses/ncurses.h:#define wattron(win,at)          wattr_on(win, NCURSES_CAST(attr_t, at), NULL)
/usr/include/ncurses/ncurses.h:#define attr_on(a,o)             wattr_on(stdscr,a,o)

and the references to wattron can be satisfied from libcurses just as
from libncurses.

Thanks.
        Rainer
  

On 05/19/2017 01:50 PM, Rainer Orth wrote:
> Hi Pedro,
> 
>>>> Ok for mainline and 8.0 branch?
>>>
>>> The cast bits are OK.  I'd like to hear your opinion on
>>> moving the NOMACROS define to gdb_curses.h, before including
>>> <curses.h>.
>>
>> The move makes sense to me: I just wasn't aware of that file.  I'll
>> prepare a separate patch.
> 
> I've checked in the cast part now.  Here's the NOMACROS part for
> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
> amd64-pc-solaris2.12 (ncurses).  Ok too?

OK.

> 
> Interestingly, with that patch the previous link failure on Solaris
> 11/12 (missing wattr_on/wattr_off) is gone, too.  This seems to happen
> because <ncurses/ncurses.h> no longer defines
> 
> /usr/include/ncurses/ncurses.h:#define wattron(win,at)          wattr_on(win, NCURSES_CAST(attr_t, at), NULL)
> /usr/include/ncurses/ncurses.h:#define attr_on(a,o)             wattr_on(stdscr,a,o)
> 
> and the references to wattron can be satisfied from libcurses just as
> from libncurses.

Eh, great.

Thanks,
Pedro Alves
  

> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
> Cc: gdb-patches@sourceware.org
> Date: Fri, 19 May 2017 14:50:06 +0200
> 
> I've checked in the cast part now.  Here's the NOMACROS part for
> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
> amd64-pc-solaris2.12 (ncurses).  Ok too?

I think this should be guarded by some OS-specific macro, so as not to
affect other platforms, where the original problem doesn't exist.  (I
see 6 instances of these macros being tested in my ncurses headers,
and I'm not on Solaris.)  Who knows what new problems this could cause?

Thanks.
  

Hi Eli,

>> I've checked in the cast part now.  Here's the NOMACROS part for
>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>> amd64-pc-solaris2.12 (ncurses).  Ok too?
>
> I think this should be guarded by some OS-specific macro, so as not to
> affect other platforms, where the original problem doesn't exist.  (I
> see 6 instances of these macros being tested in my ncurses headers,
> and I'm not on Solaris.)  Who knows what new problems this could cause?

that's what I had done initially (via configure.ac for solaris2.* only),
but Pedro suggested to do it unconditionally since some other targets
(AIX notably) seem to be having the same problem.

	Rainer
  

On 05/19/2017 02:20 PM, Eli Zaretskii wrote:
>> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
>> Cc: gdb-patches@sourceware.org
>> Date: Fri, 19 May 2017 14:50:06 +0200
>>
>> I've checked in the cast part now.  Here's the NOMACROS part for
>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>> amd64-pc-solaris2.12 (ncurses).  Ok too?
> 
> I think this should be guarded by some OS-specific macro, so as not to
> affect other platforms, where the original problem doesn't exist. 

The problem exists, we just hadn't tripped on it yet.  
Googling around we see people running into curses defining "move" as
macro for instance, which conflicts with std::move.  

So I'd rather take the opposite approach.  GDB must be able to compile
with  NOMACROS defined on some hosts, so defining it everywhere makes
gdb behave the same everywhere and reduces the testing axis.  A patch
introducing a problem for Solaris or any host using the same curses
will be quickly exposed on any host.

> (I
> see 6 instances of these macros being tested in my ncurses headers,
> and I'm not on Solaris.)  Who knows what new problems this could cause?

I expect it'll fix more problems than create them.  Defining macros without
some sort of namespace prefix in C++ is a sure recipe for problems.
NOMACROS was surely invented to avoid these problems.

Thanks,
Pedro Alves
  

On 05/19/2017 02:26 PM, Rainer Orth wrote:
> Hi Eli,
> 
>>> I've checked in the cast part now.  Here's the NOMACROS part for
>>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>>> amd64-pc-solaris2.12 (ncurses).  Ok too?
>>
>> I think this should be guarded by some OS-specific macro, so as not to
>> affect other platforms, where the original problem doesn't exist.  (I
>> see 6 instances of these macros being tested in my ncurses headers,
>> and I'm not on Solaris.)  Who knows what new problems this could cause?
> 
> that's what I had done initially (via configure.ac for solaris2.* only),
> but Pedro suggested to do it unconditionally since some other targets
> (AIX notably) seem to be having the same problem.

Yes, and it's not host specific, but really curses-implementation
specific.  On the same host you may compile against different versions
of curses (BSD curses, ncurses, pdcurses, etc.).  I don't see any
benefit to complicate things when we have no evidence that telling
curses to avoid defining its symbols as macros causes problems.

Thanks,
Pedro Alves