This is the mail archive of the 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] accept all C integer types in function parameters referenced by alloc_align (PR 88363)

On 12/11/18 11:15 AM, Marek Polacek wrote:
On Tue, Dec 11, 2018 at 09:59:27AM -0700, Martin Sebor wrote:
[*] The change in the patch is obvious enough to me.  All it
does is accept more of the things that are accepted by GCC 8
(enums, bools, wchar_t, etc.) and that inadvertently started
to be rejected as a result of my prior change.   That the rules
can be made more restrictive is something different.
Obvious changes should be obvious to anyone, not just the committer, IMHO. I don't think we should make the rules more restrictive;

I was referring to the rules of what expression the compiler accepts
as positional parameters.

what we have in place
seems to have worked fine and I would have thought it's clear that changing
what the compiler accepts will never be an obvious change, unlike, say, fixing
a test that fails with -m32 because it uses 'unsigned long' instead of size_t.

If that's the intent it should be stated in the policy then.  Seems
simple enough:

  Obvious fixes <ins>that have no impact on constructs accepted
  or rejected by the compiler</ins> can be committed without
  prior approval.

I have no problem with this clarification.  I just wouldn't have
thought it to apply to restoring previous behavior that I myself
had inadvertently removed (i.e., fixing my own regression).

I recently brought up the question of the write w/o approval
policy on the gcc list:

looking for clarification.  Except for Jeff's comment (which
I'm afraid didn't really clarify things), didn't get any.

You (the maintainers) have put it in place.  If you don't intend
for the rest of us to make use of it, or if it's not meant to be
interpreted to give us the freedom to decide what is or isn't
obvious, then change it. But it's disingenuous to claim that "We
don't want to get overly restrictive about checkin policies" and
then chastise people each time they say they might check something
in on their own.

...or fixing typos in comments and formatting fixes should be obvious, adding
new tests for fixed bugs likely as well, but outlining semantics in a comment
doesn't strike me as obvious at all.  "When in doubt, ask for a review."

I was not in doubt, but I posted the patch for review just
the same, didn't I?

If by "outlining semantics in a comment" you are referring to
the patch mentioned in the other thread then if such changes are
also not meant to fall under obvious then again, update the policy:

  Obvious fixes <ins>that have no impact on constructs accepted
  or rejected by the compiler and that do not outline the semantics
  of GCC internals in comments</ins> can be committed without prior

In both of these cases I posted the patch and invited feedback
to make sure that was seemed clearly correct to me wouldn't be
viewed differently by others.  That still seems perfectly
appropriate to me.  But if even that isn't acceptable then update
the policy to make it clear that posting a patch and saying that
you'll commit it if no one objects is also against the rules.


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