rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land.
I have no blocking concerns, just some idle thoughts. Up to you if you want Aaron's feedback before landing. ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6235 + "%plural{2:with|4:from|:and}0 " + "%select{enumeration|floating-point}1 type %3 is deprecated">, + InGroup<DeprecatedEnumFloatConversion>; ---------------- 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. ================ Comment at: clang/lib/AST/Type.cpp:1865-1866 // enumeration type in the sense required here. // C++0x: However, if the underlying type of the enum is fixed, it is // considered complete. if (const auto *ET = dyn_cast<EnumType>(CanonicalType)) ---------------- 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. ================ Comment at: clang/test/SemaCXX/warn-enum-compare.cpp:79 - while (B1 == B2); // expected-warning {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}} - while (name1::B2 == name2::B3); // expected-warning {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}} ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71576/new/ https://reviews.llvm.org/D71576 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits