This revision was automatically updated to reflect the committed changes.
Closed by commit rL297761: Warn on enum assignment to bitfields that can't fit
all values (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D30923?vs=91749&id=91752#toc
Repository:
rL LLVM
https://r
rnk marked an inline comment as done.
rnk added a comment.
Cool, thanks for the reviews, this is definitely in better shape now. This is
off by default, so I'm going to commit and experiment with it on a few
codebases.
I'd still like to hear if either of the Richards have diagnostic wording
su
hfinkel added a comment.
In https://reviews.llvm.org/D30923#700780, @rnk wrote:
> In https://reviews.llvm.org/D30923#700708, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30923#700696, @rnk wrote:
> >
> > > Do you think it's worth indicating that the error can be suppressed with
> > > an ex
rnk added a comment.
In https://reviews.llvm.org/D30923#700708, @hfinkel wrote:
> In https://reviews.llvm.org/D30923#700696, @rnk wrote:
>
> > Do you think it's worth indicating that the error can be suppressed with an
> > explicit cast, or is that wasted space?
>
>
> What might this look like?
rnk updated this revision to Diff 91749.
rnk added a comment.
- Test that explicit casts suppress the warning
https://reviews.llvm.org/D30923
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/SemaCXX/warn-bitfield-en
hfinkel added a comment.
In https://reviews.llvm.org/D30923#700696, @rnk wrote:
> In https://reviews.llvm.org/D30923#700626, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30923#700620, @thakis wrote:
> >
> > > Maybe it should have some "to suppress the warning, do X" fixit?
> >
> >
> > I thi
rnk updated this revision to Diff 91741.
rnk added a comment.
- Beef up test as Nico suggests
- Add two notes, one to change the bitfield signedness, one to indicate the
required bitwidth
https://reviews.llvm.org/D30923
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/Dia
rnk added a comment.
In https://reviews.llvm.org/D30923#700626, @hfinkel wrote:
> In https://reviews.llvm.org/D30923#700620, @thakis wrote:
>
> > Maybe it should have some "to suppress the warning, do X" fixit?
>
>
> I think we should at least include how many bits the field should have to fix
>
hfinkel added a comment.
In https://reviews.llvm.org/D30923#700620, @thakis wrote:
> Maybe it should have some "to suppress the warning, do X" fixit?
I think we should at least include how many bits the field should have to fix
the problem (pointing to the relevant field definition certainly s
thakis added a comment.
Maybe it should have some "to suppress the warning, do X" fixit?
https://reviews.llvm.org/D30923
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.
Looks like a cool warning. Two suggestions for more exhaustive testing, but I
think this looks good.
Comment at: test/SemaCXX/warn-bitfield-enum-conversion.cpp:1
+// RUN: %c
rnk updated this revision to Diff 91653.
rnk marked an inline comment as done.
rnk added a comment.
- Actually make this warning off by default
https://reviews.llvm.org/D30923
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.c
rnk marked an inline comment as done.
rnk added inline comments.
Comment at: test/Sema/warn-bitfield-enum-conversion.c:3
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
thakis wrote:
> can you add an enum with an e
rnk updated this revision to Diff 91652.
rnk added a comment.
- Make test C++, add fixed type enum
https://reviews.llvm.org/D30923
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/SemaCXX/warn-bitfield-enum-conversi
thakis added inline comments.
Comment at: test/Sema/warn-bitfield-enum-conversion.c:3
+
+enum TwoBits { Hi1 = 3 } two_bits;
+enum TwoBitsSigned { Lo2 = -2, Hi2 = 1 } two_bits_signed;
can you add an enum with an explicit underlying type? will this warning always
rnk created this revision.
This adds -Wbitfield-enum-conversion, which warns on implicit
conversions that happen on bitfield assignment that change the value of
some enumerators.
Values of enum type typically take on a very small range of values, so
they are frequently stored in bitfields. Unfort
16 matches
Mail list logo