* [PATCH] Do not expand macros to 'defined'
@ 2019-01-16 6:34 Павел Крюков
2019-01-16 20:02 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Павел Крюков @ 2019-01-16 6:34 UTC (permalink / raw)
To: gdb-patches, Simon Marchi
Expanding a macro which contains 'defined' PP keyword is UB.
sim/common/Changelog:
2019-01-16 Pavel I. Kryukov <kryukov@frtk.ru>
* sim-arange.c: eliminate DEFINE_NON_INLINE_P
---
sim/common/sim-arange.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/sim/common/sim-arange.c b/sim/common/sim-arange.c
index 6373b742ce8..b3488ab564a 100644
--- a/sim/common/sim-arange.c
+++ b/sim/common/sim-arange.c
@@ -32,10 +32,7 @@ along with this program. If not, see
<http://www.gnu.org/licenses/>. */
#include <string.h>
#endif
-#define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
-#define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
-
-#if DEFINE_NON_INLINE_P
+#ifdef SIM_ARANGE_C_INCLUDED
/* Insert a range. */
@@ -280,9 +277,7 @@ sim_addr_range_delete (ADDR_RANGE *ar,
address_word start, address_word end)
build_search_tree (ar);
}
-#endif /* DEFINE_NON_INLINE_P */
-
-#if DEFINE_INLINE_P
+#else /* SIM_ARANGE_C_INCLUDED */
SIM_ARANGE_INLINE int
sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
@@ -301,4 +296,4 @@ sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
return 0;
}
-#endif /* DEFINE_INLINE_P */
+#endif /* SIM_ARANGE_C_INCLUDED */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Do not expand macros to 'defined'
2019-01-16 6:34 [PATCH] Do not expand macros to 'defined' Павел Крюков
@ 2019-01-16 20:02 ` Simon Marchi
2019-01-16 20:38 ` Pavel Kryukov
2019-01-16 20:49 ` Andreas Schwab
0 siblings, 2 replies; 6+ messages in thread
From: Simon Marchi @ 2019-01-16 20:02 UTC (permalink / raw)
To: Павел
Крюков
Cc: gdb-patches
On 2019-01-16 01:34, Ðавел ÐÑÑков wrote:
> Expanding a macro which contains 'defined' PP keyword is UB.
>
> sim/common/Changelog:
> 2019-01-16 Pavel I. Kryukov <kryukov@frtk.ru>
>
> * sim-arange.c: eliminate DEFINE_NON_INLINE_P
> ---
> sim/common/sim-arange.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/sim/common/sim-arange.c b/sim/common/sim-arange.c
> index 6373b742ce8..b3488ab564a 100644
> --- a/sim/common/sim-arange.c
> +++ b/sim/common/sim-arange.c
> @@ -32,10 +32,7 @@ along with this program. If not, see
> <http://www.gnu.org/licenses/>. */
> #include <string.h>
> #endif
>
> -#define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
> -#define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
> -
> -#if DEFINE_NON_INLINE_P
> +#ifdef SIM_ARANGE_C_INCLUDED
>
> /* Insert a range. */
>
> @@ -280,9 +277,7 @@ sim_addr_range_delete (ADDR_RANGE *ar,
> address_word start, address_word end)
> build_search_tree (ar);
> }
>
> -#endif /* DEFINE_NON_INLINE_P */
> -
> -#if DEFINE_INLINE_P
> +#else /* SIM_ARANGE_C_INCLUDED */
>
> SIM_ARANGE_INLINE int
> sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
> @@ -301,4 +296,4 @@ sim_addr_range_hit_p (ADDR_RANGE *ar, address_word
> addr)
> return 0;
> }
>
> -#endif /* DEFINE_INLINE_P */
> +#endif /* SIM_ARANGE_C_INCLUDED */
The patch LGTM, I agree these macros are not really needed. But I am
confused, does this actually fix your c++ build? You mentioned you were
using g++ (and therefore cpp), which supposedly can handle this fine:
https://gcc.gnu.org/onlinedocs/cpp/Defined.html
If so, what did the error look like?
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Do not expand macros to 'defined'
2019-01-16 20:02 ` Simon Marchi
@ 2019-01-16 20:38 ` Pavel Kryukov
2019-01-16 21:11 ` Simon Marchi
2019-01-16 20:49 ` Andreas Schwab
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Kryukov @ 2019-01-16 20:38 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> which supposedly can handle this fine
Right, but not with '-Wall -Wextra -Werror' flags.
These pedantic options help us (a little) keeping C++ project portable
between compilers, although we do not intent to have this property on GDB integration.
--
Pavel
> 16 янв. 2019 г., в 23:02, Simon Marchi <simon.marchi@polymtl.ca> написал(а):
>
>> On 2019-01-16 01:34, Павел Крюков wrote:
>> Expanding a macro which contains 'defined' PP keyword is UB.
>> sim/common/Changelog:
>> 2019-01-16 Pavel I. Kryukov <kryukov@frtk.ru>
>> * sim-arange.c: eliminate DEFINE_NON_INLINE_P
>> ---
>> sim/common/sim-arange.c | 11 +++--------
>> 1 file changed, 3 insertions(+), 8 deletions(-)
>> diff --git a/sim/common/sim-arange.c b/sim/common/sim-arange.c
>> index 6373b742ce8..b3488ab564a 100644
>> --- a/sim/common/sim-arange.c
>> +++ b/sim/common/sim-arange.c
>> @@ -32,10 +32,7 @@ along with this program. If not, see
>> <http://www.gnu.org/licenses/>. */
>> #include <string.h>
>> #endif
>> -#define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
>> -#define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
>> -
>> -#if DEFINE_NON_INLINE_P
>> +#ifdef SIM_ARANGE_C_INCLUDED
>> /* Insert a range. */
>> @@ -280,9 +277,7 @@ sim_addr_range_delete (ADDR_RANGE *ar,
>> address_word start, address_word end)
>> build_search_tree (ar);
>> }
>> -#endif /* DEFINE_NON_INLINE_P */
>> -
>> -#if DEFINE_INLINE_P
>> +#else /* SIM_ARANGE_C_INCLUDED */
>> SIM_ARANGE_INLINE int
>> sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
>> @@ -301,4 +296,4 @@ sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
>> return 0;
>> }
>> -#endif /* DEFINE_INLINE_P */
>> +#endif /* SIM_ARANGE_C_INCLUDED */
>
> The patch LGTM, I agree these macros are not really needed. But I am confused, does this actually fix your c++ build? You mentioned you were using g++ (and therefore cpp), which supposedly can handle this fine:
>
> https://gcc.gnu.org/onlinedocs/cpp/Defined.html
>
> If so, what did the error look like?
>
> Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Do not expand macros to 'defined'
2019-01-16 20:02 ` Simon Marchi
2019-01-16 20:38 ` Pavel Kryukov
@ 2019-01-16 20:49 ` Andreas Schwab
2019-01-16 20:59 ` Simon Marchi
1 sibling, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2019-01-16 20:49 UTC (permalink / raw)
To: Simon Marchi
Cc: Павел
Крюков,
gdb-patches
On Jan 16 2019, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> The patch LGTM, I agree these macros are not really needed. But I am
> confused, does this actually fix your c++ build? You mentioned you were
> using g++ (and therefore cpp), which supposedly can handle this fine:
>
> https://gcc.gnu.org/onlinedocs/cpp/Defined.html
>
> If so, what did the error look like?
This has nothing to do with C vs. C++, both have the same wording:
If the token defined is generated as a result of this replacement
process or use of the defined unary operator does not match one of
the two specified forms prior to macro replacement, the behavior is
undefined.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Do not expand macros to 'defined'
2019-01-16 20:49 ` Andreas Schwab
@ 2019-01-16 20:59 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2019-01-16 20:59 UTC (permalink / raw)
To: Andreas Schwab
Cc: Павел
Крюков,
gdb-patches
On 2019-01-16 15:48, Andreas Schwab wrote:
> On Jan 16 2019, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
>> The patch LGTM, I agree these macros are not really needed. But I am
>> confused, does this actually fix your c++ build? You mentioned you
>> were
>> using g++ (and therefore cpp), which supposedly can handle this fine:
>>
>> https://gcc.gnu.org/onlinedocs/cpp/Defined.html
>>
>> If so, what did the error look like?
>
> This has nothing to do with C vs. C++, both have the same wording:
>
> If the token defined is generated as a result of this replacement
> process or use of the defined unary operator does not match one of
> the two specified forms prior to macro replacement, the behavior is
> undefined.
>
> Andreas.
That's why I was confused, the purpose of Pavel's patch that was
replaced by this one is C++-oriented:
https://sourceware.org/ml/gdb-patches/2019-01/msg00241.html
So in the end it looks like the issue is indeed not about C++, but about
his usage of -Wextra (-Wexpansion-to-defined).
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Do not expand macros to 'defined'
2019-01-16 20:38 ` Pavel Kryukov
@ 2019-01-16 21:11 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2019-01-16 21:11 UTC (permalink / raw)
To: Pavel Kryukov; +Cc: gdb-patches
On 2019-01-16 15:38, Pavel Kryukov wrote:
>> which supposedly can handle this fine
>
> Right, but not with '-Wall -Wextra -Werror' flags.
> These pedantic options help us (a little) keeping C++ project portable
> between compilers, although we do not intent to have this property on
> GDB integration.
Thanks, that clears it up. I pushed the patch.
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-16 21:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16 6:34 [PATCH] Do not expand macros to 'defined' Павел Крюков
2019-01-16 20:02 ` Simon Marchi
2019-01-16 20:38 ` Pavel Kryukov
2019-01-16 21:11 ` Simon Marchi
2019-01-16 20:49 ` Andreas Schwab
2019-01-16 20:59 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).