This is an archive of the discontinued LLVM Phabricator instance.

[c++20] Add deprecation warnings for the expression forms deprecated by P1120R0.
ClosedPublic

Authored by rsmith on Dec 16 2019, 3:29 PM.

Details

Reviewers
aaron.ballman
rnk
Summary

This covers:

  • usual arithmetic conversions (comparisons, arithmetic, conditionals) between different enumeration types
  • usual arithmetic conversions between enums and floating-point types
  • comparisons between two operands of array type

The deprecation warnings are on-by-default (in C++20 compilations); it
seems likely that these forms will become ill-formed in C++23, so
warning on them now by default seems wise.

For the first two bullets, off-by-default warnings were also added for
all the cases where we didn't already have warnings (covering language
modes prior to C++20). These warnings are in subgroups of the existing
-Wenum-conversion (except that the first case is not warned on if either
enumeration type is anonymous, consistent with our existing
-Wenum-conversion warnings).

Event Timeline

rsmith created this revision.Dec 16 2019, 3:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2019, 3:29 PM
rnk accepted this revision.Dec 16 2019, 3:49 PM

I have no blocking concerns, just some idle thoughts. Up to you if you want Aaron's feedback before landing.

clang/include/clang/Basic/DiagnosticSemaKinds.td
6235

I'm surprised we don't have a TableGen facility to say Warning<warn_foo.Text + " is deprecated">, but I don't see an alternative.

clang/lib/AST/Type.cpp
1865–1866

Is this C++11 comment still relevant? I assume that isComplete handles this case by returning true, and a forward decl can tell us if the enum is scoped.

clang/test/SemaCXX/warn-enum-compare.cpp
79

It seems more technically correct to say that two values are being compared, but I don't see how to keep the diagnostic as well factored as you have it.

This revision is now accepted and ready to land.Dec 16 2019, 3:49 PM
rsmith closed this revision.Dec 16 2019, 5:55 PM
rsmith marked 4 inline comments as done.
clang/lib/AST/Type.cpp
1865–1866

I don't think this is useful -- and probably nor is the isComplete check. I'll look into this in a follow-up commit.

clang/test/SemaCXX/warn-enum-compare.cpp
79

Yeah, I agonized over this a little, but while I agree with you, in the end I think it's not entirely wrong to talk about (eg) a comparing an int with a float, and the ambiguity between types and values there doesn't seem likely to actually be confusing -- I don't think the extra words help comprehension of the diagnostic. I'm happy to change it back, of course.

rsmith marked 3 inline comments as done.Dec 16 2019, 6:34 PM
rsmith added inline comments.
clang/lib/AST/Type.cpp
1865–1866

Fixed in rGeea8ba097c4a86632b88291bea51eb710f8ae4fb. Turns out this was hiding a bug in how we checked static_cast<Enum>(...) for incomplete enum types.