alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
In http://reviews.llvm.org/D18821#403117, @Prazek wrote:
> In http://reviews.llvm.org/D18821#402686, @Prazek wrote:
>
> > In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
> >
Prazek added a comment.
In http://reviews.llvm.org/D18821#402686, @Prazek wrote:
> In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
>
> > BTW, why is the check in the 'modernize' module? It doesn't seem to make
> > anything more modern. I would guess, the pattern it detects is most
Prazek added a comment.
In http://reviews.llvm.org/D18821#403103, @Quuxplusone wrote:
> I would like to see a new version of http://reviews.llvm.org/D19105 with all
> the "1-bit-bitfield" diffs removed.
> Right now, it's hard to see that there's *anything* in
> http://reviews.llvm.org/D19105
Quuxplusone added a subscriber: Quuxplusone.
Quuxplusone added a comment.
I would like to see a new version of http://reviews.llvm.org/D19105 with all
the "1-bit-bitfield" diffs removed.
Right now, it's hard to see that there's *anything* in
http://reviews.llvm.org/D19105 that's not a
Prazek added a comment.
In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
> BTW, why is the check in the 'modernize' module? It doesn't seem to make
> anything more modern. I would guess, the pattern it detects is most likely to
> result from a programming error. Also, the fix, though
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 53909.
Prazek marked an inline comment as done.
Prazek added a comment.
I will think name for new module that would have all the checks like this.
I added ingnoring of bitfields of size 1
Prazek added a comment.
http://reviews.llvm.org/D19105
Here is a diff containing fixes for clang. What I see is that it would be nice
to detect bitfields of 1 bit and treet it as bool, so it won't warn it such
cases.
http://reviews.llvm.org/D18821
Prazek added a comment.
In http://reviews.llvm.org/D18821#399079, @alexfh wrote:
> In http://reviews.llvm.org/D18821#399064, @Prazek wrote:
>
> > In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
> >
> > > BTW, why is the check in the 'modernize' module? It doesn't seem to make
> > >
alexfh added a comment.
In http://reviews.llvm.org/D18821#399064, @Prazek wrote:
> In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
>
> > BTW, why is the check in the 'modernize' module? It doesn't seem to make
> > anything more modern. I would guess, the pattern it detects is most
Prazek added a comment.
In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
> BTW, why is the check in the 'modernize' module? It doesn't seem to make
> anything more modern. I would guess, the pattern it detects is most likely to
> result from a programming error. Also, the fix, though
alexfh added a comment.
BTW, why is the check in the 'modernize' module? It doesn't seem to make
anything more modern. I would guess, the pattern it detects is most likely to
result from a programming error. Also, the fix, though it retains the behavior,
has a high chance to be incorrect. Can
Prazek updated this revision to Diff 53114.
Prazek marked 2 inline comments as done.
Prazek added a comment.
Used isMacroID to determinate if it's macro
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
Prazek updated this revision to Diff 53113.
Prazek marked 2 inline comments as done.
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
On Thu, 7 Apr 2016 15:48:56 +, Alexander Kornienko via cfe-commits said:
>Actually, did you think about adding this as a clang diagnostic?
>
>Richard, what do you think about complaining in Clang about `int i =
>true;` kind of code?
If you don't mind a lurker interjecting... :) I think
alexfh added inline comments.
Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:47
@@ +46,3 @@
+ const auto Type = Cast->getType().getLocalUnqualifiedType();
+ if (isPreprocessorIndependent(BoolLiteral, Result)) {
+diag(BoolLiteral->getLocation(),
Prazek added a comment.
In http://reviews.llvm.org/D18821#394486, @alexfh wrote:
> Actually, did you think about adding this as a clang diagnostic?
>
> Richard, what do you think about complaining in Clang about `int i = true;`
> kind of code?
Glad to hear that :)
alexfh added a comment.
Actually, did you think about adding this as a clang diagnostic?
Richard, what do you think about complaining in Clang about `int i = true;`
kind of code?
Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:49
@@ +48,3 @@
+
Prazek updated this revision to Diff 52855.
Prazek added a comment.
Added new test cases
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
Prazek updated this revision to Diff 52851.
Prazek marked an inline comment as done.
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
mnbvmar added a comment.
This check throws a warning also on the conversion to floats (probably very
rare ones):
double number = true;
Even though this behavior is correct, the code warns about the implicit
conversion to **integers**.
Comment at: docs/ReleaseNotes.rst:119
Prazek marked an inline comment as done.
Prazek added a comment.
In http://reviews.llvm.org/D18821#393556, @mnbvmar wrote:
> This check throws a warning also on the conversion to floats (probably very
> rare ones):
>
> double number = true;
>
>
> Even though this behavior is correct, the code
krystyna added inline comments.
Comment at: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst:9
@@ +8,3 @@
+.. code-block:: C++
+ int a = false
+ vector v(true); // Makes vector of one element
int a = false;
http://reviews.llvm.org/D18821
Prazek added a comment.
Maybe we should merge it with http://reviews.llvm.org/D18745 and name it
'modernize-wrong-literal-cast'. The other question is, will it be better to
move it to readability?
http://reviews.llvm.org/D18821
___
cfe-commits
Prazek created this revision.
Prazek added reviewers: alexfh, staronj.
Prazek added a subscriber: cfe-commits.
Herald added a subscriber: joker.eph.
Tested on llvm codebase.
It have found many places like:
- returning true/false in function returning int,
- assigning true/false to integer inside
24 matches
Mail list logo
