Hi,

I have updated the check to warn on a++,a=1 and a+func() as the comments 
suggested.

/Anders

> I think we should actually be warning, since only one side of the branch will 
> ever be executing. Maybe we can include a flag here that says whether the 
> expression walker should step into calls? What do you think?

This sounds reasonable. We could warn about other "non-const" expressions too, 
I believe:

   a ? b++ : b++;
    a ? b=2 : b=2;

Best regards,
Daniel Marjamäki

..................................................................................................................
Daniel Marjamäki Senior Engineer
Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden

Mobile:                 +46 (0)709 12 42 62
E-mail:                 Daniel.Marjamaki at evidente.se

www.evidente.se

________________________________________
Från: Jordan Rose [jordan_rose at apple.com]
Skickat: den 20 november 2013 18:22
Till: Per Viberg
Cc: cfe-commits at cs.uiuc.edu; Daniel Marjamäki
Ämne: Re: [PATCH] new check checking use of identical expressions inside a 
conditional expression, i.e. '?'.

Hm, interesting. In this case:

+void test_expr_negative_func() {
+  unsigned a = 0;
+  unsigned b = 1;
+  a = a > 5 ? a+func() : a+func(); // no warning
+}

I think we should actually be warning, since only one side of the branch will 
ever be executing. Maybe we can include a flag here that says whether the 
expression walker should step into calls? What do you think?

Other than that, this looks good, except that I would add the true-expr and 
false-expr as ranges to highlight in addition to putting the warning itself on 
the colon.

Jordan

Attachment: identicalexpr.diff
Description: identicalexpr.diff

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to