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: [ping] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3


On Tue, 23 Jul 2013, Bernd Edlinger wrote:

> H-P: I hope you can approve my little patch for trunk now,
> although it turned out to be less trivial than I'd have expected.

Sorry, I'm not an approver.  (People who are not approvers are
welcome to review any gcc patch where they might say something
useful, whether unique or according to the gcc line, in order to
ease the burden of those with actual approval burdens^wrights.
In this case I *did* ask for the warning, so all the reason for
me to do a little review.)

Thanks!  Looks reasonable (to my tree-ignorant eyes) modulo a
few nits:

Please put the "as it would" parts of the changelog entries as
comments in the code instead.  (ChangeLog says "what", not "why".)

I'd also tweak the head comment of warn_portable_volatility_p
(like the documentation change) to not refer to
-fstrict-volatile-bitfields as the sole intended cause of
concern; it should instead say something like "at present this
function only covers -fstrict-volatile-bitfields" in order to
open up for future amendments.

Please also change the name to check_portable_volatility instead
of warn_portable_volatility_p; the "_p" suffix is canonically
used for boolean predicates.  (You might have copied the wrong
use of _p from somewhere else in the gcc code, but that's also
in error.)

> Of course it is dependent on Sandra's patch part 1 and part 2,
> which must be applied first.

brgds, H-P


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