This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR 40989: Fix Wimplicit


On Thu, May 6, 2010 at 5:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 6, 2010 at 3:25 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 3 May 2010 18:11, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>>> On Sat, May 1, 2010 at 3:14 PM, Manuel López-Ibáñez
>>> <lopezibanez@gmail.com> wrote:
>>>> On 1 May 2010 17:05, Gabriel Dos Reis <dosreis@gmail.com> wrote:
>>>>> On Sat, May 1, 2010 at 7:47 AM, Manuel López-Ibáñez
>>>>> <lopezibanez@gmail.com> wrote:
>>>>>> The problem here is that group options (options that enable other
>>>>>> options) are not handled correctly when enabled by -Werror=. The
>>>>>> following patch fixes this issue for Wimplicit but also adds the
>>>>>> infrastructure to fix it for all other options. When
>>>>>> enabling/disabling an option, we should always use the new
>>>>>> process_option function.
>>>>>>
>>>>>> In the future, we should be able to generate the calls to
>>>>>> process_option automatically from the .opt files.
>>>>>>
>>>>>> Bootstrapped and regression tested on x86_64-linux-gnu. OK?
>>>>>
>>>>> -Wimplicit actually does not make sense for C++, so C++ should not
>>>>> be named here (and I guess ObjC++ by extension.)
>>>>
>>>> Sure.
>>>>
>>>>> It seems to me that this is warning is valid only for the C-family language.
>>>>> Rename 'lang_mask' to 'c_family_mask' or 'c_family_lang_mask'.
>>>>
>>>> Ok.
>>>>
>>>>> the addition of the name 'process_option' to 'handle_option' is a bit confusing.
>>>>> I would suggest to rename process_option to broadcast_user_option.
>>>>
>>>> broadcast sounds a bit strange. I think process_option should be named
>>>> handle_option because it is the parent of all the language-specific
>>>> and target-specific handle_option functions.
>>>>
>>>> The current handle_option reads an option from the command line (from
>>>> argv), so I would rename it to read_cmdline_option. This change is
>>>> very minor, because this function is only used in opts.c and called
>>>> only once.
>>>>
>>>> In summary, the new function for handling an option in general would
>>>> be handle_option, whereas the function for reading an option from the
>>>> command line would be read_cmdline_option. What do you think about
>>>> this?
>>>
>>> agreed.
>>
>> Committed as revision 159102.
>>
>> The bug is only fixed for Wimplicit, all other group options need to
>> use handle_option to enable sub-options. It would be nice to do this
>> automatically from the *.opt files. But I am not working on it.
>>
>
> I think it breaks bootstrap with LTO:
>
> ../../src-trunk/gcc/lto/lto-lang.c:1170:32: error: initialization from
> incompatible pointer type [-Werror]
>

I checked in this patch to fix it.



-- 
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 159104)
+++ ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2010-05-06  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* lto-lang.c (lto_handle_option): Add argument kind.
+
 2010-05-05  Jan Hubicka  <jh@suse.cz>

 	* lto.c (lto_promote_cross_file_statics): Compute boundary based on refs.
Index: lto-lang.c
===================================================================
--- lto-lang.c	(revision 159104)
+++ lto-lang.c	(working copy)
@@ -616,7 +616,8 @@ lto_init_options (unsigned int argc ATTR

 const char *resolution_file_name;
 static int
-lto_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
+lto_handle_option (size_t scode, const char *arg,
+		   int value ATTRIBUTE_UNUSED, int kind ATTRIBUTE_UNUSED)
 {
   enum opt_code code = (enum opt_code) scode;
   int result = 1;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]