aaron.ballman added a comment.
Mostly looks good; the only thing left is a test to ensure the fixits are
placing the parens in the expected location.
Comment at: lib/Sema/SemaChecking.cpp:6725
@@ -6706,1 +6724,3 @@
static void AnalyzeComparison(Sema , BinaryOperator *E) {
+
mgrabovsky added a comment.
Any other remarks?
http://reviews.llvm.org/D13643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mgrabovsky updated this revision to Diff 37985.
mgrabovsky added a comment.
Fix comparisons with implicit casts and mismatched types
http://reviews.llvm.org/D13643
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/bool-compare.c
Index:
mgrabovsky added a comment.
In http://reviews.llvm.org/D13643#270813, @danielmarjamaki wrote:
> If you're interested.. it does not currently warn about "1.0 < 2.0 < 3.0" as
> far as I see.
I have now fixed this for C, but the issue remains for C++ and I have
absolutely no idea what's going
mgrabovsky added a comment.
In http://reviews.llvm.org/D13643#272046, @danielmarjamaki wrote:
> In http://reviews.llvm.org/D13643#271885, @danielmarjamaki wrote:
>
> > I tested this on 2199 debian projects.. and got 16 warnings.
> >
> >
mgrabovsky updated this revision to Diff 37987.
mgrabovsky added a comment.
Correction for C++ tests
http://reviews.llvm.org/D13643
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/bool-compare.c
test/SemaCXX/bool-compare.cpp
Index:
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13643#271885, @danielmarjamaki wrote:
> I tested this on 2199 debian projects.. and got 16 warnings.
>
> ftp://ftp.se.debian.org/debian/pool/main/t/tf5/tf5_5.0beta8.orig.tar.gz
> expr.c:738:22: warning: comparisons such as 'a < b
danielmarjamaki added a subscriber: danielmarjamaki.
danielmarjamaki added a comment.
Interesting checker.
I'll test it on some debian projects.
If you're interested.. it does not currently warn about "1.0 < 2.0 < 3.0" as
far as I see.
http://reviews.llvm.org/D13643
danielmarjamaki added a comment.
I tested this on 2199 debian projects.. and got 16 warnings.
ftp://ftp.se.debian.org/debian/pool/main/t/tf5/tf5_5.0beta8.orig.tar.gz
expr.c:738:22: warning: comparisons such as 'a < b != c' do not have their
mathematical meaning [-Wparentheses]
if
mgrabovsky updated this revision to Diff 37720.
mgrabovsky added a comment.
Change message wording
http://reviews.llvm.org/D13643
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/bool-compare.c
Index: test/Sema/bool-compare.c
mgrabovsky added a comment.
In http://reviews.llvm.org/D13643#266926, @aaron.ballman wrote:
> I would spend some time digging into how GCC handles those cases, and use
> that as a baseline that we can then improve upon. I like the fact that GCC
> basically says "use parens to clarify your
aaron.ballman added a comment.
In http://reviews.llvm.org/D13643#266649, @mgrabovsky wrote:
> In http://reviews.llvm.org/D13643#265976, @aaron.ballman wrote:
>
> > Should there be an exception to this diagnostic for code involving Boolean
> > values? e.g.,
> >
> > void f(bool a, bool b, bool c)
mgrabovsky added a comment.
In http://reviews.llvm.org/D13643#265976, @aaron.ballman wrote:
> Should there be an exception to this diagnostic for code involving Boolean
> values? e.g.,
>
> void f(bool a, bool b, bool c) {
>
> if (a == b == c)
> ;
>
> }
>
> At the very least, it seems like
aaron.ballman added a comment.
Should there be an exception to this diagnostic for code involving Boolean
values? e.g.,
void f(bool a, bool b, bool c) {
if (a == b == c)
;
}
At the very least, it seems like this should also follow GCC's behavior and
suggest parenthesis directly.
mgrabovsky created this revision.
mgrabovsky added a subscriber: cfe-commits.
This change adds a Sema diagnostic for comparisons like `a < b < c`,
which usually do not behave as intended by the author.
Fixes bug 20082.
http://reviews.llvm.org/D13643
Files:
15 matches
Mail list logo
