On 7/17/19 1:14 PM, Alexandre Oliva wrote:
On Jul 17, 2019, Martin Sebor <mse...@gmail.com> wrote:

Isn't this test sufficient to avoid the problems?

              if (!k && kmax > 1)
                continue;

It is, unless someone (i) doesn't realize attributes that are present in
the type can't be present in the decl, (ii) misreads the '!k' as just
'k', and (iii) uses the wrong toolchain to confirm the consequences of
the misreading ;-/  doh!

Can you put together a test case that does do the wrong thing?

I'm afraid not, all of the additional errors I observed were correctly
covered by the test I misunderstood once I grasped the actual logic.
Sorry about the, erhm, false positive ;-)

No worries.  The purpose of the test above is far from intuitive
(even to me now).

The change looks cleaner than the cumbersome code that's there
now so I have no problem with it but I'm not sure it does more
than the test above

Would you like me to put it in regardless?

Sure, if it's worthwhile to you I think it's an improvement even
if it doesn't fix a bug.  (In full disclosure I'm not empowered
to formally approve bigger patches but I think cleanups like this
can safely be committed as obvious.)

Does it make sense to put the testcase in anyway?

If it isn't already covered by one of the existing tests I'd say
definitely.  I also tried the following while playing with it so
if this variation isn't being exercised either it might be worth
adding to the new test as well:

  template <class T>
  f_type
  missing_nothing2;

  template <>
  void *
  ATTR ((alloc_size (1)))
  missing_nothing2<char>(int);

It's your call.

Thanks!
Martin

Reply via email to