xazax.hun added a comment.
In http://reviews.llvm.org/D15411#310539, @alexfh wrote:
> Thank you for the new awesome check!
Thank you for the review. I committed the new matcher to clang as well. I went
with isAnyCharacter name for now, but we can rename it anytime.
Repository:
rL LLVM
htt
This revision was automatically updated to reflect the committed changes.
Closed by commit rL255630: [clang-tidy] Check for suspicious string
assignments. (authored by xazax).
Changed prior to commit:
http://reviews.llvm.org/D15411?vs=42710&id=42824#toc
Repository:
rL LLVM
http://reviews.ll
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a few nits.
Thank you for the new awesome check!
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:74
@@ +73,3 @@
+return;
+ } else if (IsLite
LegalizeAdulthood added inline comments.
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:33
@@ +32,3 @@
+ s = 'c';
+ s = (char)6;
+
What happens if the test code had used `static_cast`?
http://reviews.llvm.org/D15411
_
LegalizeAdulthood added inline comments.
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:46
@@ +45,3 @@
+ ws += L'c';
+ ws += (wchar_t)6;
+
Use `static_cast<>` instead of C-style cast?
http://reviews.llvm.org/D15411
__
xazax.hun added inline comments.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+
aaron.ballman wrote:
> alexfh wrote:
> > I prefer `isAnyCha
xazax.hun updated this revision to Diff 42710.
xazax.hun marked 4 inline comments as done.
xazax.hun added a comment.
- Reimplemented fixits.
- Addressed the review comments.
http://reviews.llvm.org/D15411
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy
aaron.ballman added a reviewer: klimek.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+
alexfh wrote:
> I prefer `isAnyCharacter` for consis
alexfh added inline comments.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+
I prefer `isAnyCharacter` for consistency with `isInteger`. I'
xazax.hun updated this revision to Diff 42698.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Addressed review comments.
- Removed fixit suggestions.
http://reviews.llvm.org/D15411
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tid
xazax.hun marked 7 inline comments as done.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:19
@@ +18,3 @@
+namespace {
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
aaron.ballman wrote:
> I think this w
LegalizeAdulthood added a subscriber: LegalizeAdulthood.
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:25
@@ +24,3 @@
+int main() {
+ std::string s("foobar");
+
Use {} initialization perhaps?
http://reviews.llvm.org/D15411
__
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:19
@@ +18,3 @@
+namespace {
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
I think this would be reasonable to add to A
alexfh added a comment.
In http://reviews.llvm.org/D15411#307274, @aaron.ballman wrote:
> In http://reviews.llvm.org/D15411#307030, @alexfh wrote:
>
> > BTW, Richard, is the possibility of assignment of an integer to a
> > std::string a bug in the standard?
>
>
> This is the result of:
>
> bas
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.
In http://reviews.llvm.org/D15411#307030, @alexfh wrote:
> BTW, Richard, is the possibility of assignment of an integer to a std::string
> a bug in the standard?
This is the result of:
basic_string& operator=(ch
alexfh added inline comments.
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:45
@@ +44,3 @@
+
+ auto Diag = diag(Loc, "probably missing to_string operation");
+ if (!Loc.isMacroID() && getLangOpts().CPlusPlus11) {
I'd expand the message with an exp
xazax.hun added a comment.
Thank you for the review. I have seen a few false positives in the llvm
codebase. I agree that these kind of conversions are unfortunate but I am sure
fixing this would cause a huge backward compatibility problem. Nontheless I
also think that it might be something tha
xazax.hun updated this revision to Diff 42427.
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.
- Modified the patch according to the review.
http://reviews.llvm.org/D15411
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/Strin
alexfh added a comment.
BTW, Richard, is the possibility of assignment of an integer to a std::string a
bug in the standard?
http://reviews.llvm.org/D15411
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
alexfh added a comment.
That's quite a surprising behavior. Looks like a bug in the library
implementation (or in the standard) to me.
But the check is awesome. Thank you!
Comment at: clang-tidy/misc/StringAssignmentCheck.cpp:21
@@ +20,3 @@
+void StringAssignmentCheck::registe
xazax.hun updated this revision to Diff 42406.
xazax.hun added a comment.
Minor update to docs.
http://reviews.llvm.org/D15411
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/StringAssignmentCheck.cpp
clang-tidy/misc/StringAssignmentCheck.h
doc
xazax.hun created this revision.
xazax.hun added a reviewer: alexfh.
xazax.hun added subscribers: dkrupp, cfe-commits.
It is possible to assign arbitrary integer types to strings. Sometimes it is
the result of missing to_string call or apostrophes.
This check aims to find such errors.
http://r
22 matches
Mail list logo
