Re: [PATCH] add simple attribute introspection

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Hi Christophe,

> On Sat, 10 Nov 2018 at 00:43, Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 11/09/2018 12:59 PM, Jeff Law wrote:
>> > On 10/31/18 10:27 AM, Martin Sebor wrote:
>> >> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01473.html
>> >>
>> >> With the C++ bits approved I'm still looking for a review/approval
>> >> of the remaining changes: the C front end and the shared c-family
>> >> handling of the new built-in.
>> > I thought I acked those with just a couple minor doc nits:
>>
>> I don't see a formal approval for the rest in my Inbox or in
>> the archive.
>>
>> >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> >> index 8ffb0cd..dcf4747 100644
>> >> --- a/gcc/doc/extend.texi
>> >> +++ b/gcc/doc/extend.texi
>> >> @@ -2649,8 +2649,9 @@ explicit @code{externally_visible} attributes
>> > are still necessary.
>> >>  @cindex @code{flatten} function attribute
>> >>  Generally, inlining into a function is limited.  For a function
>> > marked with
>> >>  this attribute, every call inside this function is inlined, if possible.
>> >> -Whether the function itself is considered for inlining depends on its
>> > size and
>> >> -the current inlining parameters.
>> >> +Functions declared with attribute @code{noinline} and similar are not
>> >> +inlined.  Whether the function itself is considered for inlining depends
>> >> +on its size and the current inlining parameters.
>> > Guessing this was from another doc patch that I think has already been
>> > approved
>>
>> Yes.  It shouldn't be in the latest patch at the link above.
>>
>> >> @@ -11726,6 +11728,33 @@ check its compatibility with @var{size}.
>> >>
>> >>  @end deftypefn
>> >>
>> >> +@deftypefn {Built-in Function} bool __builtin_has_attribute
>> > (@var{type-or-expression}, @var{attribute})
>> >> +The @code{__builtin_has_attribute} function evaluates to an integer
>> > constant
>> >> +expression equal to @code{true} if the symbol or type referenced by
>> >> +the @var{type-or-expression} argument has been declared with
>> >> +the @var{attribute} referenced by the second argument.  Neither argument
>> >> +is valuated.  The @var{type-or-expression} argument is subject to the
>> > same
>> > s/valuated/evaluated/ ?
>>
>> This should also be fixed in the latest patch at the link above.
>>
>> > Did the implementation change significantly requiring another review
>> > iteration?
>>
>> I don't think it changed too significantly between the last two
>> revisions but I don't have a record of anyone having approved
>> the C FE and the middle-end bits.  (Sorry if I missed it.) Other
>> than this response from you all I see in the archive is this:
>>
>>    https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00606.html
>>
>> Please let me if the last revision is okay to commit.
>>
>
> Hi,
>
> It seems you committed this yesterday as r266335, and I have noticed
> new failures:
> on both aarch64 and arm:
> FAIL: c-c++-common/builtin-has-attribute-3.c  -Wc++-compat  (test for
> excess errors)
>
> gcc.log says:
> Excess errors:
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:11:25: error:
> alignment for 'faligned_1' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:12:25: error:
> alignment for 'faligned_2' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:39:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:40:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:47:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:48:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:50:3: error:
> size of array 'Assert' is negative
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:52:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:52:3: error:
> size of array 'Assert' is negative
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:53:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:56:3: error:
> size of array 'Assert' is negative
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:58:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:59:3: error:
> alignment for '__builtin_has_attribute_tmp.' must be at least 4
> /gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:59:3: error:
> size of array 'Assert' is negative
>
> on arm only:
> gcc.dg/builtin-has-attribute.c (test for excess errors)
> gdb.log says:
> Excess errors:
> /gcc/testsuite/gcc.dg/builtin-has-attribute.c:12:15: error: size of
> array 'Assert' is negative

I'm seeing the same on sparc-sun-solaris2.11, plus

+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++14 (test for excess errors)
+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++17 (test for excess errors)
+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++98 (test for excess errors)

According to gcc-testresults postings, several other targets are also
affected:

	aarch64-unknown-linux-gnu
        armv8l-unknown-linux-gnueabihf
        m68k-unknown-linux-gnu
        mips64el-unknown-linux-gnu
        moxie-unknown-elf
        powerpc64-unknown-linux-gnu
        powerpc64le-unknown-linux-gnu

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University