smeenai added a comment.
I put up https://reviews.llvm.org/D41368 to just suppress the warnings instead.
https://reviews.llvm.org/D39149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mclow.lists added a comment.
In https://reviews.llvm.org/D39149#958378, @bcain wrote:
> Let's resurrect these changes since https://reviews.llvm.org/D39462 was not
> the right short-term approach.
Let's not. IMHO, these changes are:
- a bad idea in principle - clang should not be warning
bcain added a comment.
In https://reviews.llvm.org/D39149#912260, @smeenai wrote:
> I confirmed that these warnings go away with https://reviews.llvm.org/D39462
> applied, and they reappear if I manually specify
> `-Wmaybe-tautological-constant-compare`. Thank you!
Let's resurrect these
smeenai abandoned this revision.
smeenai added a comment.
I confirmed that these warnings go away with https://reviews.llvm.org/D39462
applied, and they reappear if I manually specify
`-Wmaybe-tautological-constant-compare`. Thank you!
https://reviews.llvm.org/D39149
lebedev.ri added a comment.
In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:
> I dislike this change fairly strongly.
> I would much rather pursue a clang-based solution (since clang is being
> unhelpful here)
> Don't know if we can get one, though.
@smeenai
smeenai added a comment.
In https://reviews.llvm.org/D39149#911024, @rsmith wrote:
> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing
> > (enabled by default) warning, and have a different warning for
lebedev.ri added a comment.
In https://reviews.llvm.org/D39149#911024, @rsmith wrote:
> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing
> > (enabled by default) warning, and have a different warning for
rsmith added a comment.
In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
> I'm thinking you could account for all possible type sizes in the existing
> (enabled by default) warning, and have a different warning for possibly
> tautological comparisons. E.g. if a `long` is being
bcain added a comment.
In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:
> In https://reviews.llvm.org/D39149#910891, @bcain wrote:
>
> > In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
> >
> > > That is my diagnostic, so i guess this is the time to reply :)
> >
> >
> >
smeenai added a comment.
In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:
> - The fact that under *different* circumstances the comparison is not
> tautological is not a false-positive.
It's not technically a false positive, but my suspicion is that it'll make the
warning a lot
smeenai added a comment.
I'm thinking you could account for all possible type sizes in the existing
(enabled by default) warning, and have a different warning for possibly
tautological comparisons. E.g. if a `long` is being compared against `INT_MAX`,
you know that's only tautological on some
lebedev.ri added a comment.
In https://reviews.llvm.org/D39149#910891, @bcain wrote:
> In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
>
> > That is my diagnostic, so i guess this is the time to reply :)
>
>
> ...
>
> > 3. In `-Wtautological-constant-compare`, ignore any
bcain added a comment.
In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
> That is my diagnostic, so i guess this is the time to reply :)
...
> 3. In `-Wtautological-constant-compare`, ignore any comparisons that compare
> with `std::numeric_limits`. Not a fan of this solution.
smeenai added a comment.
In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:
> I dislike this change fairly strongly.
Agreed. Would you be happier with just disabling the diagnostics around the
problematic parts?
In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
> 1.
smeenai planned changes to this revision.
smeenai added a comment.
In https://reviews.llvm.org/D39149#910858, @mclow.lists wrote:
> If we have to go down this road, I'd prefer the approach used in
>
mclow.lists added a comment.
If we have to go down this road, I'd prefer the approach used in
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?r1=315874=315873=315874
https://reviews.llvm.org/D39149
lebedev.ri added a subscriber: thakis.
lebedev.ri added a comment.
That is my diagnostic, so i guess this is the time to reply :)
In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:
> I dislike this change fairly strongly.
> I would much rather pursue a clang-based solution (since
mclow.lists added a comment.
I dislike this change fairly strongly.
I would much rather pursue a clang-based solution (since clang is being
unhelpful here)
Don't know if we can get one, though.
https://reviews.llvm.org/D39149
___
cfe-commits
smeenai added a comment.
Ping.
Like I said, not a huge fan of this change, but also not a huge fan of running
into clang diagnostics (especially since we build with `-Werror` downstream).
https://reviews.llvm.org/D39149
___
cfe-commits mailing
smeenai created this revision.
Tip-of-tree clang produces `-Wtautological-constant-compare` for
tautological constant comparisons, and these pieces of code will trigger
that diagnostic when `int` and `long` have the same size (for example,
when compiling for a 32-bit target, or for Windows
20 matches
Mail list logo
