[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL347570: [clang-tidy] Improving narrowing conversions (authored by gchatelet, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. In D53488#1307834 , @aaron.ballman wrote: > LGTM! Thx ! \O/ Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/D53488 _

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/D53488 ___ cfe-commits mailing list cfe-commi

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175231. gchatelet marked 4 inline comments as done. gchatelet added a comment. - Removed redundant options in regression tests Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I think we're almost there -- I had a few outstanding questions about the config options in the tests and making sure we cover all the cases. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions-castingliterals-option.cpp:3 +// RU

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175204. gchatelet added a comment. - Fixing documentation. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsC

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. Thank you for bearing with me @aaron.ballman. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:259-263 +void NarrowingConversionsCheck::handleIntegralToBoolean( +const ASTContext &Context, SourceLocation SourceLoc, const Exp

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-26 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175203. gchatelet marked 16 inline comments as done. gchatelet added a comment. - Addressing comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/D53488 Files: clang-ti

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:31 + Options.get("WarnOnFloatingPointNarrowingConversion", 1)), + WarnOnCastingLiterals(Options.get("WarnOnCastingLiterals", 1)) {} + I think

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-23 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet marked an inline comment as done. gchatelet added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:42-44 i += 2.0; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-n

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-23 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175139. gchatelet added a comment. - Added a new option warn about wrong type literals Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53488/new/ https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguide

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:42-44 i += 2.0; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions] + // CHECK-MESSAG

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-22 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175033. gchatelet marked an inline comment as done. gchatelet added a comment. - Reverting the WarnOnFloatingPointNarrowingConversion option to be on by default Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cpp

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-22 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet marked 2 inline comments as done. gchatelet added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:42-44 i += 2.0; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-n

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-22 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 175030. gchatelet marked an inline comment as done. gchatelet added a comment. - Refactored the code to make it simpler, added more tests Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/Narrowing

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions-long-is-32bits.cpp:4-10 + int i;// i.e. int32_t + long l; // i.e. int32_t + long long ll; // i.e. int64_t + + unsigned int ui;// i.e. uint32_t + un

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-16 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 174374. gchatelet marked 13 inline comments as done. gchatelet added a comment. - Addressed comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp clang-tidy/cpp

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-16 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:79-81 + // TODO: Provide an automatic fix if the number is exactly representable in the destination type. + f += 2.0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowi

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:79-81 + // TODO: Provide an automatic fix if the number is exactly representable in the destination type. + f += 2.0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: nar

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-14 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. Thx for the review. I have two suggestions in the comments let me know what you think. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:79-81 + // TODO: Provide an automatic fix if the number is exactly representable in the d

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-14 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 174044. gchatelet marked 7 inline comments as done. gchatelet added a comment. - State char signess clearly Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp clang-t

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:79-81 + // TODO: Provide an automatic fix if the number is exactly representable in the destination type. + f += 2.0; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: nar

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-13 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment. In https://reviews.llvm.org/D53488#1296940, @JonasToth wrote: > So, finally LGTM :) > Please give @aaron.ballman a chance to comment, as he reviewed too. > > Thanks for your patch! Thank you for bearing with me. Waiting for input from @aaron.ballman Repository: r

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. So, finally LGTM :) Please give @aaron.ballman a chance to comment, as he reviewed too. Thanks for your patch! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 _

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-13 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 173844. gchatelet added a comment. - Remove debugging leftover Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp clang-tidy/cppcoreguidelines/NarrowingConversionsChe

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-13 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:299 + Rhs->isCXX11ConstantExpr(Context, &Constant)) { +if (Constant.isFloat()) + diagIfNarrowFloatingPointConstant(Context, SourceLoc, Lhs, Rhs, Constant); -

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-13 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 173836. gchatelet marked 6 inline comments as done. gchatelet added a comment. - Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp clang-tidy/cppcor

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-12 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Only a few nits from me. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:30 + WarnOnFloatingPointNarrowingConversion( + Options.get("WarnOnFloatingPointNarrowingConversion", 0)) {} + I would make t

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-12 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; +// Conversions to unsigned integer are well defined and follow modulo 2 +// arithmetic. JonasToth wrote: > gchatelet wrote: > > Jon

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-12 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 173633. gchatelet marked 2 inline comments as done. gchatelet added a comment. - Address comments + fix hex values display Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversionsChec

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:108-113 + if (T.isSignedInteger()) +return {llvm::APSInt::getMinValue(Context.getTypeSize(&T), false), +llvm::APSInt::getMaxValue(Context.getTypeSize(&T),

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-09 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 173343. gchatelet marked 13 inline comments as done. gchatelet added a comment. - Addressing comments, adding Option to disable FP to FP narrowing warnings, handling FP literals. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; +// Conversions to unsigned integer are well defined and follow modulo 2 +// arithmetic. gchatelet wrote: > JonasToth wrote: > > I a

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:17 +#include "llvm/ADT/SmallVector.h" +#include "llvm/Support/Mutex.h" + Is this include needed? Comment at: clang-tidy/cppcoreguideline