Hi,
A new iteration on the checker. I have moved it to be a warning in semaexpr now.
//Anders
.......................................................................................................................
Anders Rönnholm Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)70 912 42 54
E-mail: [email protected]
www.evidente.se
________________________________________
Från: Jordan Rose [[email protected]]
Skickat: den 2 oktober 2013 18:12
Till: Daniel Marjamäki
Cc: Richard Smith; Anders Rönnholm; cfe commits
Ämne: Re: [PATCH] [StaticAnalyzer] New checker StringPlusChar
On Oct 1, 2013, at 22:16 , Daniel Marjamäki <[email protected]>
wrote:
>
> Hello!
>
>> This should be a warning, not a static analyser check, shouldn't it?
>
> Is anybody against this?
>
> Personally I feel very confident about this check regarding signal/noise.
> However putting it in the static analyser to start with felt like a safe
> choice.
I'm fine with this. I didn't realize at first it was just for character
literals; with that restriction this does seem like false positives would be
unlikely. Thanks, Richard. (Sorry to force another iteration, Anders. The
existing warning is in SemaExpr.cpp: diagnoseStringPlusInt.)
Daniel, delaying += and char plus string seem fine to me, though they might
just be handled if you hook into diagnoseStringPlusInt. And thanks for the
reminder about things like s8, u8, etc.
Jordan
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 191941)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -6913,6 +6913,47 @@
Self.Diag(OpLoc, diag::note_string_plus_int_silence);
}
+/// \brief Emit a warning when adding a char literal to a string.
+static void diagnoseStringPlusChar(Sema &Self, SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ const DeclRefExpr *StringRefExpr =
+ dyn_cast<DeclRefExpr>(LHSExpr->IgnoreImpCasts());
+ const CharacterLiteral *CharExpr =
+ dyn_cast<CharacterLiteral>(RHSExpr->IgnoreImpCasts());
+
+ if (!StringRefExpr) {
+ StringRefExpr = dyn_cast<DeclRefExpr>(RHSExpr->IgnoreImpCasts());
+ CharExpr = dyn_cast<CharacterLiteral>(LHSExpr->IgnoreImpCasts());
+ }
+
+ if (!CharExpr || !StringRefExpr)
+ return;
+
+ const QualType StringType = StringRefExpr->getType();
+
+ // Return if not a PointerType.
+ if (!StringType->isAnyPointerType())
+ return;
+
+ // Return if not a CharacterType.
+ if (!StringType->getPointeeType()->isAnyCharacterType())
+ return;
+
+ SourceRange DiagRange(LHSExpr->getLocStart(), RHSExpr->getLocEnd());
+ Self.Diag(OpLoc, diag::warn_string_plus_char) << DiagRange;
+
+ // Only print a fixit for str + char, not for char + str.
+ if (isa<CharacterLiteral>(RHSExpr->IgnoreImpCasts())) {
+ SourceLocation EndLoc = Self.PP.getLocForEndOfToken(RHSExpr->getLocEnd());
+ Self.Diag(OpLoc, diag::note_string_plus_char_silence)
+ << FixItHint::CreateInsertion(LHSExpr->getLocStart(), "&")
+ << FixItHint::CreateReplacement(SourceRange(OpLoc), "[")
+ << FixItHint::CreateInsertion(EndLoc, "]");
+ } else {
+ Self.Diag(OpLoc, diag::note_string_plus_char_silence);
+ }
+}
+
/// \brief Emit error when two pointers are incompatible.
static void diagnosePointerIncompatibility(Sema &S, SourceLocation Loc,
Expr *LHSExpr, Expr *RHSExpr) {
@@ -6939,9 +6980,11 @@
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
- // Diagnose "string literal" '+' int.
- if (Opc == BO_Add)
+ // Diagnose "string literal" '+' int and string '+' "char literal".
+ if (Opc == BO_Add) {
diagnoseStringPlusInt(*this, Loc, LHS.get(), RHS.get());
+ diagnoseStringPlusChar(*this, Loc, LHS.get(), RHS.get());
+ }
// handle the common case first (both operands are arithmetic).
if (!compType.isNull() && compType->isArithmeticType()) {
Index: test/SemaCXX/string-plus-char.cpp
===================================================================
--- test/SemaCXX/string-plus-char.cpp (revision 0)
+++ test/SemaCXX/string-plus-char.cpp (working copy)
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+ A(): str() { }
+ A(const char *p) { }
+ A(char *p) : str(p + 'a') { } // expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+ A& operator+(const char *p) { return *this; }
+ A& operator+(char ch) { return *this; }
+ char * str;
+};
+
+void f(const char *s) {
+ A a = s + 'a'; // // expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+ a = a + s + 'b'; // no-warning
+
+ char *str = 0;
+ char *str2 = str + 'c'; // expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+ const char *constStr = s + 'c'; // expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+ str = 'c' + str;// expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+ wchar_t *wstr;
+ wstr = wstr + L'c'; // expected-warning {{adding 'char' to a string pointer does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+ // no-warning
+ char c = 'c';
+ str = str + c;
+ str = c + str;
+}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 191941)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -4271,6 +4271,12 @@
def note_string_plus_int_silence : Note<
"use array indexing to silence this warning">;
+def warn_string_plus_char : Warning<
+ "adding 'char' to a string pointer does not append to the string">,
+ InGroup<StringPlusChar>;
+def note_string_plus_char_silence : Note<
+ "use array indexing to silence this warning">;
+
def warn_sizeof_array_param : Warning<
"sizeof on array function parameter will return size of %0 instead of %1">,
InGroup<SizeofArrayArgument>;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 191941)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -267,6 +267,7 @@
def StaticFloatInit : DiagGroup<"static-float-init", [GNUStaticFloatInit]>;
def GNUStatementExpression : DiagGroup<"gnu-statement-expression">;
def StringPlusInt : DiagGroup<"string-plus-int">;
+def StringPlusChar : DiagGroup<"string-plus-char">;
def StrncatSize : DiagGroup<"strncat-size">;
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
def TautologicalCompare : DiagGroup<"tautological-compare",
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
