public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
To: Bernd Schmidt <bschmidt@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: Error out on -fvtable-verify without --enable-vtable-verify
Date: Mon, 09 May 2016 11:28:00 -0000	[thread overview]
Message-ID: <yddlh3j79oa.fsf@CeBiTec.Uni-Bielefeld.DE> (raw)
In-Reply-To: <57305717.5090508@redhat.com> (Bernd Schmidt's message of "Mon, 9	May 2016 11:23:35 +0200")

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

Hi Bernd,

> On 05/08/2016 12:44 PM, Rainer Orth wrote:
>> With the recent change not to install libvtv without
>> --enable-vtable-verify, I noticed that gcc/g++ would still accept
>> -fvtable-verify without errors, only to emit obscure link-time errors
>> about missing vtv_*.o (which hadn't been installed in that situation
>> before) and libvtv.
>>
>> It seems to me a much better user experience to emit a clear error
>> message in this case, which is what this patch does.
>
> Generally ok, but...
>
>> +AC_ARG_ENABLE(vtable-verify,
>> +[AS_HELP_STRING([--enable-vtable-verify],
>> +		[enable vtable verification feature])],
>> +[case "$enableval" in
>> + yes) enable_vtable_verify=yes ;;
>> + no)  enable_vtable_verify=no ;;
>> + *)   enable_vtable_verify=no;;
>> + esac],
>> +[enable_vtable_verify=no])
>> +vtable_verify=`if test $enable_vtable_verify != no; then echo 1; else
>> echo 0; fi`
>> +AC_DEFINE_UNQUOTED(ENABLE_VTABLE_VERIFY, $vtable_verify,
>> +[Define 0/1 if vtable verification feature is enabled.])
>
> That looks a little overly complicated. Don't you get the enable_ variables
> set by autoconf? And if you do need the case statement, you might as well
> set things to 0/1 directly and skip the enable_vtable_verify variable
> entirely.

that's what you get for blindly copying configure.ac fragments.  The
following works instead:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: c.a.patch --]
[-- Type: text/x-patch, Size: 648 bytes --]

diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -865,6 +865,14 @@ Valid choices are 'yes' and 'no'.]) ;;
   esac
 ], [enable_tls=''])
 
+AC_ARG_ENABLE(vtable-verify,
+[AS_HELP_STRING([--enable-vtable-verify],
+		[enable vtable verification feature])],,
+[enable_vtable_verify=no])
+vtable_verify=`if test x$enable_vtable_verify = xyes; then echo 1; else echo 0; fi`
+AC_DEFINE_UNQUOTED(ENABLE_VTABLE_VERIFY, $vtable_verify,
+[Define 0/1 if vtable verification feature is enabled.])
+
 AC_ARG_ENABLE(objc-gc,
 [AS_HELP_STRING([--enable-objc-gc],
 		[enable the use of Boehm's garbage collector with

[-- Attachment #3: Type: text/plain, Size: 447 bytes --]


I had to tighten the $enable_vtable_verify test to guard against a
non-no argument to --enable-vtable-verify being interpreted as yes.

Tested by just running gcc/configure without and with
--enable-vtable-verify, and with --enable-vtable-verify=nonstd (handled
as no).

Ok now?

Thanks.
        Rainer

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

  reply	other threads:[~2016-05-09 11:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-08 10:44 Rainer Orth
2016-05-09  9:23 ` Bernd Schmidt
2016-05-09 11:28   ` Rainer Orth [this message]
2016-05-09 11:29     ` Bernd Schmidt

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=yddlh3j79oa.fsf@CeBiTec.Uni-Bielefeld.DE \
    --to=ro@cebitec.uni-bielefeld.de \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.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).