Re: [PATCH 1/9] ENABLE_CHECKING refactoring

[Jeff Law <law at redhat dot com>]

On 10/18/2015 12:17 AM, Mikhail Maltsev wrote:
> On 10/12/2015 11:57 PM, Jeff Law wrote:
> > > > -#ifdef ENABLE_CHECKING
> > > > +#if CHECKING_P
> > >
> > > I fail to see the point of this change.
> > I'm guessing (and Mikhail, please correct me if I'm wrong), but I think he's
> > trying to get away from ENABLE_CHECKING and instead use a macro which is
> > always defined to a value.
> Yes, exactly. Such macro is better because it can be used both for conditional
> compilation (if needed) and normal if-s (unlike ENABLE_CHECKING).
>
> On 10/14/2015 12:33 AM, Jeff Law wrote:
> > >    gcc/common.opt  | 5 +++++
> > >    gcc/system.h    | 3 +++
> > >    libcpp/system.h | 8 ++++++++
> > >    3 files changed, 16 insertions(+)
> > I committed this prerequisite patch to the trunk.
> >
> > jeff
>
> There is a typo here:
>
> > #ifdef ENABLE_CHECKING
> > #define gcc_checking_assert(EXPR) gcc_assert (EXPR)
> > +#define CHECKING_P 1
> > #else
> > +/* N.B.: in release build EXPR is not evaluated.  */
> > #define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))
> > +#define CHECKING_P 1
> > #endif
>
> In my original patch the second definition actually was
> '+#define CHECKING_P 0'
Not sure how that happened.  I'll take care of it.


I'd actually planned to start extracting the non-controversial stuff out 
of the later patches, but just ran out of time before going onto PTO.

> Also, gcc_checking_assert in libcpp requires gcc_assert to be defined. That was
> missing in my original patch (and was added in patch 2/9), but I think it would
> be better to fix it here, as Bernd noticed (in the last paragraph of [1]).
Sounds wise.  Agreed.

> Besides, I like Richard's proposal [2] about moving CHECKING_P macros into
> configure.ac.
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00550.html
> [2] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00555.html
>
> It required minor tweaking in order to silence autotools' warnings. I attached
> the modified patch.
When I tried that, I couldn't get it to work.  My autotools-fu is quite 
limited.

> OK for trunk (after bootstrap/regtest)?
I'll take a closer look later today, after my morning meetings.

> P.S. I am planning to post at least some of the other updated parts today, and I
> also hope to get in time with the whole series (before stage1 ends).
Excellent.

jeff