[cfe-commits] [PATCH] Remove -Wdisabled-macro-expansion

[Abramo Bagnara]

Il 30/10/2012 23:18, Eli Friedman ha scritto:
> On Tue, Oct 30, 2012 at 2:52 PM, Abramo Bagnara
> <abramo.bagnara at bugseng.com> wrote:
>> Il 30/10/2012 21:58, Eli Friedman ha scritto:
>>> On Tue, Oct 30, 2012 at 12:27 AM, Abramo Bagnara
>>> <abramo.bagnara at bugseng.com> wrote:
>>>> Il 30/10/2012 05:07, Ted Kremenek ha scritto:
>>>>> On Oct 29, 2012, at 8:38 PM, Eli Friedman <eli.friedman at gmail.com
>>>>> <mailto:eli.friedman at gmail.com>> wrote:
>>>>>
>>>>>> I think there's a reasonable workflow for small projects which
>>>>>> involves using -Weverything and subtracting out the warnings which
>>>>>> don't make sense.
>>>>>
>>>>> Right.  I commented about this in my other email before I saw your response.
>>>>>
>>>>>>  I agree that there are limits to what we can/should
>>>>>> do with -Weverything; I'm not trying to argue for getting rid of
>>>>>> warnings because they make -Weverything too noisy.  On the other hand,
>>>>>> I think we should have a high standard for the usefulness of
>>>>>> off-by-default warnings.  And I think that there's future
>>>>>> infrastructure work we can do both to help people write higher quality
>>>>>> code.
>>>>>
>>>>> Agreed.  FWIW, the developers we have spoken to about -Weverything
>>>>> really like how it has been very helpful in discovering warning flags
>>>>> that are very good for their codebase.
>>>>>
>>>>>>
>>>>>>>> it isn't acceptable for it to trigger warnings for usage
>>>>>>>> of headers included with the compiler, and as far as I can tell, there
>>>>>>>> isn't any way to fix the header. (See patch for a testcase that
>>>>>>>> checks we don't trigger any warnings from stdbool.h.)
>>>>>>>>
>>>>>>>> I'm planning to commit this unless someone has an alternative
>>>>>>>> suggestion.
>>>>>>>
>>>>>>> Could you suppress the warning if the spelling location for the token
>>>>>>> which would have been expanded is in a system header?
>>>>>>
>>>>>> I think that would end up being more confusing than helpful because it
>>>>>> suppresses some, but not all, loops involving system macros.
>>>>>>
>>>>>> I would put more effort into this if I thought it was generally
>>>>>> useful, but the fact that it isn't on by default, and that the headers
>>>>>> included with clang manage to trigger it, and there isn't any specific
>>>>>> class of users this is useful for, all indicate it isn't worth the
>>>>>> effort.
>>>>>
>>>>> If others agree with this argument, I can see a strong argument here to
>>>>> remove the warning entirely.
>>>>
>>>> This thread clarifies why the warning is very important for a class of
>>>> users and why to reach the same aim has no pratical alternatives
>>>>
>>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20111226/050687.html
>>>>
>>>> Please do not remove it.
>>>
>>> Unless I'm missing something, this warning doesn't address any of the
>>> pieces of "Specifically, the use of token pasting (cf. MISRA-C:2004
>>> Rules 19.12 and 19.13), variable argument lists (ellipses) (cf.
>>> MISRA-C:2004 Rule 16.1), and recursive macro calls are excluded by
>>> this rule. All macros are required to expand into complete syntactic
>>> units (cf. MISRA-C:2004 Rule 19.4)."  Actually, as far as I can tell,
>>> "#define true true" doesn't violate any guidelines at all in the
>>> document you cite, ignoring the vague restriction that macros should
>>> be "simple".
>>
>> It is a recursive macro, just as
> 
> How is it recursive?  The whole point of the C rules here is that it
> *does't* recurse.

I meant that it is a possible attempt to make a recursive macro
expansion, while in C/C++ this cannot happen. The whole point of this
coding standard rule is exactly that: to show to the developer that the
macro definitions it has done would lead to a recursion, while this is
disabled by language rules.

Note that using complex macros structure the recursion attempt might be
rather hidden and hard to spot without automatic checking.


-- 
Abramo Bagnara

BUGSENG srl - http://bugseng.com
mailto:abramo.bagnara at bugseng.com