Hi Martin,
> On 8 Jan 2019, at 00:25, Martin Sebor <mse...@gmail.com> wrote:
>
> On 1/7/19 9:55 AM, Iain Sandoe wrote:
>> Hi Martin,
>> A)
>> Some of the builtin-has-attribute tests fail because a sub-set of them need
>> symbol alias support.
>> Darwin has only support for weak aliases and therefore we need to skip these.
>> However, the sub-set is small, and I am reluctant to throw out the entire
>> set for the sake of a small number, so I propose to wrap that small number
>> in #ifndef that can be enabled by targets without the necessary support
>> (Darwin is not the only one, just the most frequently tested and therefore
>> often “guilty” of finding the problem ;) )
>> It’s a tricky trade-off between having too many test-cases and having test
>> cases that try to cover too many circumstances...
>> B) [builtin-has-attribute-4.c]
>> I am concerned by the diagnostics for the lack of support for the
>> “protected” mode (Darwin doesn’t have this, at least at present).
>> B.1 the reported diagnostic appears on the closing brace of the function,
>> rather than on the statement that triggers it (line 233).
>> B.2 I think you’re perhaps missing a %< %> pair - there’s no ‘’ around
>> “protected".
>> B.3. there are a bunch of other lines with the “protected” visibility
>> marking, but no diagnostic (perhaps that’s intended, I am not sure).
>> Addressing B is a separate issue from making the current tests pass, it
>> might not be appropriate at this stage .. it’s more of a “head’s up”.
>> as for the test fixes, OK for trunk?
>
> Jeff already approved the patch so let me just say thanks for
> the cleanup and the heads up.
I will apply the patch as is (probably my tomorrow AM) for now - and then we
can adjust if there’s any change to the diagnostics.
>
> Using an #ifdef like your did makes sense to me for now. If other
> targets need something similar it might be worth considering whether
> to add an effective-target for them instead of enumerating them in
> the test, and also change the test to verify that the appropriate
> warning is issued. I'll try to remember to look into it.
>
> I suspect the missing quotes from around "protected" in the warning
> comed from gcc/config/darwin.c:
>
> warning (OPT_Wattributes, "protected visibility attribute "
> "not supported in this configuration; ignored”);
Ah.. and that’s a “warning” rather than a “warning_at” which explains the
placement on the final brace. Without looking at the context, not sure what
location info we might have there. Note to self: better audit other stuff in
Darwin’s port-specific attrs.
> Let me add them along with a test for them.
There is a test in builtin-has-attribute-4.c, by default and ….
>> +} /* { dg-warning "protected visibility attribute not supported" "" {
>> target { *-*-darwin* } } } */
> I'll see if I can
> quickly tell also why the warning isn't issued consistently.
… this might produce a bunch more warnings.
thanks
Iain
