[cfe-dev] Warnings not raised from a macro: -Wundefined-bool-conversion/-Wtautological-undefined-compare

[Alexander Kornienko]

On Thu, Mar 5, 2015 at 2:51 PM, Marek Kurdej <curdeius at gmail.com> wrote:

> Hi,
> As Alex has stated already, the problem lies in the fact that the code
> doesn't handle macros.
> That wasn't an attempt to avoid breaking code, but rather a question of
> simplicity and lack of knowledge about clang-tidy's way of handling macros.
> I'll take a look at it in my spare time this weekend and give you more
> feedback about what can be done.
> I've just seen that twice in the code, the check returns immediately if it
> encounters a macro.
>

It's the easiest way to avoid messing up code in macros (it was probably me
who suggested inserting this check), but there may be a more accurate way
to do this that won't lead to completely failing to insert braces when a
statement is a single macro expansion.

As a possible way to solve this, I'd suggest trying to check if both
locations where the braces are going to be inserted are on the same level
of macro expansion in the same file. Lexer::makeFileCharRange is a rather
advanced method that could help here.

In any case, it makes sense to continue the discussion (if any) on
llvm.org/PR22785.

-- Alex


>
> Best regards,
> Marek
>
> Le mer. 4 mars 2015 à 12:31, Alexander Kornienko <alexfh at google.com> a
> écrit :
>
> +curdeius at gmail.com
>>
>> I found this thread while searching for something else in the list
>> archive.
>>
>> This report would definitely be more visible on llvm.org/bugs, I've
>> filed it as llvm.org/PR22785.
>>
>> The issue has nothing to do with the Clang's treatment of macros
>> in -Wundefined-bool-conversion/-Wtautological-undefined-compare
>> diagnostics. It's just an over-eager attempt to avoid breaking the code by
>> messing with macros.
>>
>> Marek (cc-ed here) is the author of the corresponding check. Maybe he'll
>> be interested in fixing the issue.
>>
>> -- Alex
>>
>> On Mon, Nov 10, 2014 at 1:59 PM, miroslav.fontan <
>> miroslav.fontan at wincor-nixdorf.cz> wrote:
>>
>>> Hi,
>>>
>>> Thanks for the -save-temps hint, it helps with this particular issue,
>>> unfortunatelly it does not solve the cause of the problem.
>>>
>>> I guess that Clang-based tools inherit the same macro treatment from
>>> clang
>>> parser and give an inaccurate results.
>>>
>>> For example this code fragment is handled incorrectly by command:
>>> clang-tidy
>>> -check=-*,readability-braces-around-statements -fix
>>>
>>> #define MACRO_1 i++
>>> #define MACRO_2
>>>
>>> void test(int i)
>>> {
>>>         if( i % 3) i--; // OK
>>>         else if( i % 2) MACRO_1; // KO, no braces
>>>         else MACRO_2; // OK
>>> }
>>>
>>> Maybe I am stubborn, I still think that preprocess step should be made
>>> completely before the next steps.
>>>
>>> Best regards
>>>
>>> Mira
>>>
>>>
>>> > -----Original Message-----
>>> > From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu]
>>> > On Behalf Of Joerg Sonnenberger
>>> > Sent: Wednesday, November 05, 2014 12:20 PM
>>> > To: cfe-dev at cs.uiuc.edu
>>> > Subject: Re: [cfe-dev] Warnings not raised from a macro: -Wundefined-
>>> > bool-conversion/-Wtautological-undefined-compare
>>> >
>>> > On Wed, Nov 05, 2014 at 11:58:55AM +0100, miroslav.fontan wrote:
>>> > > I think Martin is right. Preprocessor is only textual replacement,
>>> > > compiler should not know about this step and compiles code resulting
>>> > > from preprocess step.
>>> >
>>> > A lot of people disagree. If you want to force it to give all warnings,
>>> > use -save-temps.
>>> >
>>> > Joerg
>>> > _______________________________________________
>>> > cfe-dev mailing list
>>> > cfe-dev at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150305/c9b31d8b/attachment.html>