qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: qemu-devel@nongnu.org, Richard Henderson <rth@twiddle.net>,
	pbonzini@redhat.com, peter.maydell@linaro.org
Subject: Re: [Qemu-devel] [PATCH] configure: Force the C standard to gnu11
Date: Wed, 9 Jan 2019 13:27:29 +0000	[thread overview]
Message-ID: <20190109132729.GN3998@redhat.com> (raw)
In-Reply-To: <860317b6-6489-9c18-bdf8-95a70fbec9b3@redhat.com>

On Wed, Jan 09, 2019 at 02:20:02PM +0100, Thomas Huth wrote:
> On 2019-01-09 13:58, Daniel P. Berrangé wrote:
> > On Wed, Jan 09, 2019 at 01:52:02PM +0100, Thomas Huth wrote:
> >> On 2019-01-09 12:44, Daniel P. Berrangé wrote:
> >>> On Wed, Jan 09, 2019 at 12:25:43PM +0100, Thomas Huth wrote:
> >>>> On 2019-01-09 11:58, Daniel P. Berrangé wrote:
> >>>>> On Mon, Jan 07, 2019 at 11:45:26AM +0100, Thomas Huth wrote:
> >>>>>> Different versions of GCC and Clang use different versions of the C standard.
> >>>>>> This repeatedly caused problems already, e.g. with duplicated typedefs:
> >>>>>>
> >>>>>>  https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
> >>>>>>
> >>>>>> or with for-loop variable initializers:
> >>>>>>
> >>>>>>  https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
> >>>>>>
> >>>>>> To avoid these problems, we should enforce the C language version to the
> >>>>>> same level for all compilers. Since our minimum compiler versions are
> >>>>>> GCC v4.8 and Clang v3.4 now, and both basically support "gnu11" already,
> >>>>>> this seems to be a good choice.
> >>>>>
> >>>>> In 4.x   gnu11 is marked as experimental. I'm not really comfortable
> >>>>> using experimental features - even if its warning free there's a risk
> >>>>> it would silently mis-compile something.
> >>>>>
> >>>>> gnu99 is ok with 4.x - it is merely "incomplete".
> >>>>
> >>>> gnu11 has the big advantage that it also fixes the problem with
> >>>> duplicated typedefs that are reported by older versions of Clang.
> >>>>
> >>>> Are you sure about the experimental character in 4.x? I just looked at
> >>>> https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html and it says:
> >>>>
> >>>> "A fourth version of the C standard, known as C11, was published in 2011
> >>>> as ISO/IEC 9899:2011. GCC has limited incomplete support for parts of
> >>>> this standard, enabled with -std=c11 or -std=iso9899:2011."
> >>>>
> >>>> It does not say anything about "experimental" there. The word
> >>>> "experimental" is only used for the C++ support, but we hardly have C++
> >>>> code in QEMU -- if you worry about that, I could simply drop the
> >>>> "-std=gnu++11" part from my patch?
> >>>
> >>> I was looking at the "info gcc" docs on RHEL7, gcc-4.8.5-16.el7_4.1.x86_64:
> >>>
> >>>   "3.4 Options Controlling C Dialect
> >>>
> >>>        ....snip...
> >>>
> >>>      'gnu11'
> >>>      'gnu1x'
> >>>           GNU dialect of ISO C11.  Support is incomplete and
> >>>           experimental.  The name 'gnu1x' is deprecated."
> >>
> >> Ok. Looks like the "Support is incomplete and experimental" sentence has
> >> been removed with GCC 4.9.0 here. So GCC 4.8 is likely pretty close
> >> already. IMHO we could give it a try and enable gnu11 for QEMU with GCC
> >> v4.8, too. If we later find problems, we could still switch back to
> >> gnu99 instead. Other opinions?
> > 
> > Our code is already cleanly compiling with gnu99 standard - the problem
> > is merely that we sometimes introduce regressions due to not enforcing
> > that standard level. I don't think the features in gnu11 are compelling
> > enough to justify using something that's declared experimental.
> What about the duplicated typedef problem? See:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
> 
> That one occured with Clang, but I think we've had plenty of these in
> the past with GCC, too...

IIUC, That's only a problem because we don't pass any  -std  flag, and
so get the compilers default, which may be gnu11.  If we explicitly
set -std=gnu99, that problem will be reported by patchew, travis and
maintainers own build tests, and thus won't get anywhere near git master.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  reply	other threads:[~2019-01-09 13:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-07 10:45 [Qemu-devel] [PATCH] configure: Force the C standard to gnu11 Thomas Huth
2019-01-09 10:58 ` Daniel P. Berrangé
2019-01-09 11:25   ` Thomas Huth
2019-01-09 11:44     ` Daniel P. Berrangé
2019-01-09 12:52       ` Thomas Huth
2019-01-09 12:58         ` Daniel P. Berrangé
2019-01-09 13:20           ` Thomas Huth
2019-01-09 13:27             ` Daniel P. Berrangé [this message]
2019-01-09 13:54               ` Thomas Huth
2019-01-09 13:10         ` Markus Armbruster
2019-01-09 13:17           ` Thomas Huth
2019-01-09 14:20             ` Markus Armbruster
2019-01-09 14:27               ` Thomas Huth

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=20190109132729.GN3998@redhat.com \
    --to=berrange@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=thuth@redhat.com \
    /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 NNTP newsgroup(s).