This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Ensure that dump calls are guarded with dump_enabled_p


On Sun, Nov 11, 2018 at 04:01:42PM -0500, David Malcolm wrote:
> On Sun, 2018-11-11 at 12:10 -0700, Martin Sebor wrote:
> > > +#define VERIFY_DUMP_ENABLED_P \
> > > +  do {					\
> > > +    gcc_assert (dump_enabled_p ());	\
> > > +    if (!dump_enabled_p ())		\
> > > +      return;				\
> > > +  } while (0)
> > 
> > Since it behaves more like a function call (compound statement
> > to be exact) I would suggest to make VERIFY_DUMP_ENABLED_P
> > a function-like macro rather than object-like one.
> 
> FWIW it's not quite a function call: the "return" statement within it
> returns from the function it's been put inside.  Maybe that needs
> expressing in the name of the macro?  (all this depends on whether we
> want to return or abort, or both.  If we just want to return, I'd write
> that directly, without a macro)

Why would we want to return?  Is the assert testing something that is not
required?  Then why is it an assert?  If on the other hand it is vital,
just returning is terrible (and won't work like this for non-void
functions of course).

It should be either

#define VERIFY_DUMP_ENABLED_P gcc_assert (dump_enabled_p ())

or

#define VERIFY_DUMP_ENABLED_P do { if (!dump_enabled_p ()) abort (); } while (0)

(depending on if you want to abort only with ENABLE_ASSERT_CHECKING or
always).


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]