This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h
- From: Pedro Alves <palves at redhat dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>, John Marshall <John dot W dot Marshall at glasgow dot ac dot uk>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 4 Jun 2018 14:35:50 +0100
- Subject: Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h
- References: <20180521121557.16535-1-hjl.tools@gmail.com> <B781F464-751E-4D45-8881-36F6003759BE@glasgow.ac.uk> <CAMe9rOoRV_SYna2Sp4oQvMHbqBuhw6bOw66gtrcSYfhcxFPtOw@mail.gmail.com>
On 06/04/2018 02:12 PM, H.J. Lu wrote:
> On Mon, Jun 4, 2018 at 3:51 AM, John Marshall
> <John.W.Marshall@glasgow.ac.uk> wrote:
>> On 21 May 2018, at 13:15, H dot J dot Lu <hjl dot tools at gmail dot com> wrote:
>>> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
>>> be used in binutils.
>>
>> This patch broke building gdb on MacOS with clang (i.e., after ./configure with no options):
>>
>> CXX gdb.o
>> In file included from ../../../binutils-gdb/gdb/gdb.c:19:
>> In file included from ../../../binutils-gdb/gdb/defs.h:531:
>> In file included from ../../../binutils-gdb/gdb/gdbarch.h:39:
>> In file included from ../../../binutils-gdb/gdb/frame.h:72:
>> In file included from ../../../binutils-gdb/gdb/language.h:26:
>> ../../../binutils-gdb/gdb/symtab.h:1361:1: error: _Pragma takes a parenthesized string literal
>> DEF_VEC_P (symtab_ptr);
I think clang is printing a bogus location here. symtab.h includes
vec.h, which includes diagnostics.h and does:
/* clang has a bug that makes it warn (-Wunused-function) about unused functions
that are the result of the DEF_VEC_* macro expansion. See:
https://bugs.llvm.org/show_bug.cgi?id=22712
We specifically ignore this warning for the vec functions when the compiler
is clang. */
#ifdef __clang__
# define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION \
DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
#else
# define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION
#endif
and that's most certainly what is tripping on the _Pragma+STRINGIFY.
>>
>>> --- a/gdb/common/diagnostics.h
>>> +++ b/include/diagnostics.h
>>> [snip]
>>> @@ -15,10 +13,8 @@
>>> You should have received a copy of the GNU General Public License
>>> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>>>
>>> -#ifndef COMMON_DIAGNOSTICS_H
>>> -#define COMMON_DIAGNOSTICS_H
>>> -
>>> -#include "common/preprocessor.h"
>>
>> Putting this #include back fixes the build. Apparently in this configuration, include/diagnostics.h doesn't otherwise have a definition of STRINGIFY whereas on Linux or other platforms it does, via some coincidence of different host-related includes or something.
>
> Please add
>
> #include "common/preprocessor.h"
>
> to gdb.c.
>
I don't think so, see above.
Where does binutils get the definition of STRINGIFY from?
Your other patch does:
> --- a/include/diagnostics.h
> +++ b/include/diagnostics.h
> @@ -19,8 +19,13 @@
> #ifdef __GNUC__
> # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
> # define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
> +
> +/* Stringification. */
> +# define DIAGNOSTIC_STRINGIFY_1(x) #x
> +# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
> +
> # define DIAGNOSTIC_IGNORE(option) \
> - _Pragma (STRINGIFY (GCC diagnostic ignored option))
> + _Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
> #else
So I'm surprised by your suggestion. That DIAGNOSTIC_STRINGIFY
bit should be split out of that other patch and pushed in
separately, IMO. Alternatively, preprocessor.h should be shared too.
Thanks,
Pedro Alves