public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: GNU C Library <libc-alpha@sourceware.org>,
	DJ Delorie <dj@redhat.com>, Arjun Shankar <arjun@redhat.com>
Subject: Re: Failing misc/check-installed-headers-c with new kernel headers.
Date: Thu, 17 Jan 2019 02:49:00 -0000	[thread overview]
Message-ID: <8cbaf667-6a32-822f-fe3c-ef2d8b6f89ca@redhat.com> (raw)
In-Reply-To: <87zhs0ti16.fsf@oldenburg2.str.redhat.com>

On 1/16/19 4:21 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> Should we expand the regexp in some way to ignore text within single-line
>> comments that we can easily detect?
> 
> I think we should use #pragma GCC poison in $cih_test_c, like this:
> 
> diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
> index 8e7beffd82..f9e39eb995 100644
> --- a/scripts/check-installed-headers.sh
> +++ b/scripts/check-installed-headers.sh
> @@ -144,6 +144,12 @@ EOF
>     inappropriate for this test.  */
>  #undef _LIBC
>  #undef _GNU_SOURCE
> +
> +/* Obsolete type names.  */
> +#pragma GCC poison ushort
> +#pragma GCC poison ulong
> +#pragma GCC poison uint
> +
>  /* The library mode is selected here rather than on the command line to
>     ensure that this selection wins. */
>  $expanded_lib_mode
> 
> And then get rid of the grep.

I like this solution, but it drastically complicates the test.

It's certainly more robust than the regexp.

The problem is that you have to split the tail part of the
test into 2 compilations.

One to test for obsolete types, and another to test for the
correct compilation under the std/macro test matrix.

You can no longer keep the two together because the poison
always triggers on the old types in the base headers.

And when you're looking for poisoning you now have to go
through all the raised compiler errors just like you would
for headers, and exclude any from headers that are allowed
to have the obsolete types.

I guess all-in-all it's more robust, but more work than
I was expecting :/

-- 
Cheers,
Carlos.

  reply	other threads:[~2019-01-17  2:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-16 21:11 Carlos O'Donell
2019-01-16 21:22 ` Florian Weimer
2019-01-17  2:49   ` Carlos O'Donell [this message]
2019-01-18 10:16     ` Florian Weimer
2019-01-16 21:23 ` DJ Delorie
2019-01-16 23:06   ` [RFC] Ignore source code comments in scripts/check-installed-headers.sh Tulio Magno Quites Machado Filho
2019-01-16 23:15     ` DJ Delorie
2019-01-16 23:22       ` Tulio Magno Quites Machado Filho
2019-01-16 23:29         ` DJ Delorie
2019-01-17  0:14           ` Tulio Magno Quites Machado Filho
2019-01-17 10:06             ` Szabolcs Nagy
2019-01-17 10:10               ` Szabolcs Nagy
2019-01-19 16:44 ` Failing misc/check-installed-headers-c with new kernel headers Zack Weinberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8cbaf667-6a32-822f-fe3c-ef2d8b6f89ca@redhat.com \
    --to=carlos@redhat.com \
    --cc=arjun@redhat.com \
    --cc=dj@redhat.com \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).