[Clang] [NFC] Introduce helpers for defining compatibilty warnings #132129
Conversation
Sirraide
added
the
clang:frontend
|
@llvm/pr-subscribers-clang Author: None (Sirraide) ChangesThis introduces some tablegen helpers for defining compatibility warnings. The main aim of this is to both simplify adding new compatibility warnings as well as to unify the naming of compatibility warnings. I’ve refactored ~half of the compatiblity warnings (that follow the usual scheme) in I haven’t (yet) migrated all warnings even in that one file, and there are some more specialised ones for which the scheme I’ve established here doesn’t work (e.g. because they’re warning+error instead of warning+extwarn; however, warning+extension is supported), but the point of this isn’t to implement all compatibility-related warnings this way, only to make the common case a bit easier to handle. This currently also only handles C++ compatibility warnings, but it should be fairly straight-forward to extend the tablegen code so it can also be used for C compatibility warnings (if this gets merged, I’m planning to do that in a follow-up pr). The vast majority of compatibility warnings are emitted by writing Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::ext_... : diag::warn_...)in accordance with which I’ve chosen the following naming scheme: Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::compat_cxxyz_foo : diag::compat_pre_cxxyz_foo)That is, for a warning about a C++20 feature—i.e. C++≤17 compatibility—we get: Diag(Loc, getLangOpts().CPlusPlus20 ? diag::compat_cxx20_foo : diag::compat_pre_cxx20_foo)While there is an argument to be made against writing ‘ I also experimented with moving all compatibility warnings to a separate file, but 1. I don’t think it’s worth the effort, and 2. I think it hurts compile times a bit because at least in my testing I felt that I had to recompile more code than if we just keep e.g. Sema-specific compat warnings in the Sema diagnostics file. Instead, I’ve opted to put them all in the same place within any one file; currently this is a the very top but I don’t really have strong opinions about this. Patch is 52.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132129.diff 19 Files Affected:
diff --git a/clang/include/clang/Basic/Diagnostic.td b/clang/include/clang/Basic/Diagnostic.td
index 0b8b3af939ba0..85b7276695613 100644
--- a/clang/include/clang/Basic/Diagnostic.td
+++ b/clang/include/clang/Basic/Diagnostic.td
@@ -155,6 +155,49 @@ class DefaultWarnNoWerror {
}
class DefaultRemark { Severity DefaultSeverity = SEV_Remark; }
+// C++ compatibility warnings
+multiclass CXXCompatWarn<
+ string message,
+ bit default_ignore,
+ int std_ver,
+ string std_ver_override = ""#std_ver> {
+ // 'X is a C++YZ extension'.
+ def compat_pre_cxx#std_ver#_#NAME :
+ Diagnostic<!strconcat(message, " a C++", std_ver_override, " extension"),
+ CLASS_EXTENSION,
+ !if(default_ignore, SEV_Ignored, SEV_Warning)>,
+ InGroup<!cast<DiagGroup>("CXX"#std_ver)>;
+
+ // 'X is incompatible with C++98' (if std_ver == 11).
+ // 'X is incompatible with C++ standards before C++YZ' (otherwise).
+ def compat_cxx#std_ver#_#NAME :
+ Warning<!if(!eq(std_ver, 11),
+ !strconcat(message, " incompatible with C++98"),
+ !strconcat(message, " incompatible with C++ standards before C++", std_ver_override))>,
+ InGroup<!cast<DiagGroup>(!if(!eq(std_ver, 11),
+ "CXX98Compat",
+ "CXXPre"#std_ver#"Compat"))>,
+ DefaultIgnore;
+}
+
+multiclass CXX11CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 11>;
+
+multiclass CXX14CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 14>;
+
+multiclass CXX17CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 17>;
+
+multiclass CXX20CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 20>;
+
+multiclass CXX23CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 23>;
+
+multiclass CXX26CompatWarn<string message, bit default_ignore = false>
+ : CXXCompatWarn<message, default_ignore, 26, "2c">;
+
// Definitions for Diagnostics.
include "DiagnosticASTKinds.td"
include "DiagnosticCommentKinds.td"
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1536a3b8c920a..4ef1ab0ca4470 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12,6 +12,61 @@
let Component = "Sema" in {
let CategoryName = "Semantic Issue" in {
+// C++11 compatibility with C++98.
+defm nonclass_type_friend : CXX11CompatWarn<"non-class friend type %0 is">;
+defm static_data_member_in_union : CXX11CompatWarn<"static data member %0 in union is">;
+defm templ_default_in_function_templ : CXX11CompatWarn<
+ "default template arguments for a function template are">;
+defm template_arg_extra_parens : CXX11CompatWarn<
+ "parentheses around address non-type template argument are">;
+defm typename_outside_of_template : CXX11CompatWarn<"'typename' outside of a template is">;
+
+// C++14 compatibility with C++11 and earlier.
+defm constexpr_type_definition : CXX14CompatWarn<
+ "type definition in a constexpr %select{function|constructor}0 is">;
+defm constexpr_local_var : CXX14CompatWarn<
+ "variable declaration in a constexpr %select{function|constructor}0 is">;
+defm constexpr_body_multiple_return : CXX14CompatWarn<
+ "multiple return statements in constexpr function is">;
+defm variable_template : CXX14CompatWarn<"variable templates are">;
+
+// C++17 compatibility with C++14 and earlier.
+defm decomp_decl : CXX17CompatWarn<"decomposition declarations are">;
+defm inline_variable : CXX17CompatWarn<"inline variables are">;
+
+// C++20 compatibility with C++17 and earlier.
+defm decomp_decl_spec : CXX20CompatWarn<
+ "decomposition declaration declared "
+ "%plural{1:'%1'|:with '%1' specifiers}0 is">;
+defm constexpr_local_var_no_init : CXX20CompatWarn<
+ "uninitialized variable in a constexpr %select{function|constructor}0 is">;
+defm constexpr_function_try_block : CXX20CompatWarn<
+ "function try block in constexpr %select{function|constructor}0 is">;
+defm constexpr_union_ctor_no_init : CXX20CompatWarn<
+ "constexpr union constructor that does not initialize any member is">;
+defm constexpr_ctor_missing_init : CXX20CompatWarn<
+ "constexpr constructor that does not initialize all members is">;
+defm adl_only_template_id : CXX20CompatWarn<
+ "use of function template name with no prior declaration in function call "
+ "with explicit template arguments is">;
+
+// C++23 compatibility with C++20 and earlier.
+defm constexpr_static_var : CXX23CompatWarn<
+ "definition of a %select{static|thread_local}1 variable "
+ "in a constexpr %select{function|constructor}0 "
+ "is">;
+
+// C++26 compatibility with C++23 and earlier.
+defm decomp_decl_cond : CXX26CompatWarn<"structured binding declaration in a condition is">;
+
+// Compatibility warnings duplicated across multiple language versions.
+foreach std = [14, 20, 23] in {
+ defm constexpr_body_invalid_stmt : CXXCompatWarn<
+ "use of this statement in a constexpr %select{function|constructor}0 is",
+ /*default_ignore=*/false,
+ std>;
+}
+
def note_previous_decl : Note<"%0 declared here">;
def note_entity_declared_at : Note<"%0 declared here">;
def note_callee_decl : Note<"%0 declared here">;
@@ -523,30 +578,9 @@ def warn_modifying_shadowing_decl :
// C++ decomposition declarations
def err_decomp_decl_context : Error<
"decomposition declaration not permitted in this context">;
-def warn_cxx14_compat_decomp_decl : Warning<
- "decomposition declarations are incompatible with "
- "C++ standards before C++17">, DefaultIgnore, InGroup<CXXPre17Compat>;
-def ext_decomp_decl : ExtWarn<
- "decomposition declarations are a C++17 extension">, InGroup<CXX17>;
-def ext_decomp_decl_cond : ExtWarn<
- "structured binding declaration in a condition is a C++2c extenstion">,
- InGroup<CXX26>;
-def warn_cxx26_decomp_decl_cond : Warning<
- "structured binding declaration in a condition is incompatible with "
- "C++ standards before C++2c">,
- InGroup<CXXPre26Compat>, DefaultIgnore;
def err_decomp_decl_spec : Error<
"decomposition declaration cannot be declared "
"%plural{1:'%1'|:with '%1' specifiers}0">;
-def ext_decomp_decl_spec : ExtWarn<
- "decomposition declaration declared "
- "%plural{1:'%1'|:with '%1' specifiers}0 is a C++20 extension">,
- InGroup<CXX20>;
-def warn_cxx17_compat_decomp_decl_spec : Warning<
- "decomposition declaration declared "
- "%plural{1:'%1'|:with '%1' specifiers}0 "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
def err_decomp_decl_type : Error<
"decomposition declaration cannot be declared with type %0; "
"declared type must be 'auto' or reference to 'auto'">;
@@ -1695,12 +1729,6 @@ def warn_consteval_if_always_true : Warning<
"consteval if is always true in an %select{unevaluated|immediate}0 context">,
InGroup<DiagGroup<"redundant-consteval-if">>;
-def ext_inline_variable : ExtWarn<
- "inline variables are a C++17 extension">, InGroup<CXX17>;
-def warn_cxx14_compat_inline_variable : Warning<
- "inline variables are incompatible with C++ standards before C++17">,
- DefaultIgnore, InGroup<CXXPre17Compat>;
-
def warn_inline_namespace_reopened_noninline : Warning<
"inline namespace reopened as a non-inline namespace">,
InGroup<InlineNamespaceReopenedNoninline>;
@@ -1716,11 +1744,6 @@ def ext_enum_friend : ExtWarn<
InGroup<DiagGroup<"friend-enum">>;
def note_enum_friend : Note<
"remove 'enum%select{| struct| class}0' to befriend an enum">;
-def ext_nonclass_type_friend : ExtWarn<
- "non-class friend type %0 is a C++11 extension">, InGroup<CXX11>;
-def warn_cxx98_compat_nonclass_type_friend : Warning<
- "non-class friend type %0 is incompatible with C++98">,
- InGroup<CXX98Compat>, DefaultIgnore;
def err_friend_is_member : Error<
"friends cannot be members of the declaring class">;
def warn_cxx98_compat_friend_is_member : Warning<
@@ -2152,11 +2175,6 @@ def select_tag_type_kind : TextSubstitution<
def err_static_data_member_not_allowed_in_anon_struct : Error<
"static data member %0 not allowed in anonymous "
"%sub{select_tag_type_kind}1">;
-def ext_static_data_member_in_union : ExtWarn<
- "static data member %0 in union is a C++11 extension">, InGroup<CXX11>;
-def warn_cxx98_compat_static_data_member_in_union : Warning<
- "static data member %0 in union is incompatible with C++98">,
- InGroup<CXX98Compat>, DefaultIgnore;
def ext_union_member_of_reference_type : ExtWarn<
"union member %0 has reference type %1, which is a Microsoft extension">,
InGroup<MicrosoftUnionMemberReference>;
@@ -2904,63 +2922,17 @@ def err_constexpr_non_literal_param : Error<
"not a literal type">;
def err_constexpr_body_invalid_stmt : Error<
"statement not allowed in %select{constexpr|consteval}1 %select{function|constructor}0">;
-def ext_constexpr_body_invalid_stmt : ExtWarn<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is a C++14 extension">, InGroup<CXX14>;
-def warn_cxx11_compat_constexpr_body_invalid_stmt : Warning<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++14">,
- InGroup<CXXPre14Compat>, DefaultIgnore;
-def ext_constexpr_body_invalid_stmt_cxx20 : ExtWarn<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is a C++20 extension">, InGroup<CXX20>;
-def warn_cxx17_compat_constexpr_body_invalid_stmt : Warning<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
-def ext_constexpr_body_invalid_stmt_cxx23 : ExtWarn<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is a C++23 extension">, InGroup<CXX23>;
-def warn_cxx20_compat_constexpr_body_invalid_stmt : Warning<
- "use of this statement in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++23">,
- InGroup<CXXPre23Compat>, DefaultIgnore;
-def ext_constexpr_type_definition : ExtWarn<
- "type definition in a constexpr %select{function|constructor}0 "
- "is a C++14 extension">, InGroup<CXX14>;
-def warn_cxx11_compat_constexpr_type_definition : Warning<
- "type definition in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++14">,
- InGroup<CXXPre14Compat>, DefaultIgnore;
def err_constexpr_vla : Error<
"variably-modified type %0 cannot be used in a constexpr "
"%select{function|constructor}1">;
-def ext_constexpr_local_var : ExtWarn<
- "variable declaration in a constexpr %select{function|constructor}0 "
- "is a C++14 extension">, InGroup<CXX14>;
-def warn_cxx11_compat_constexpr_local_var : Warning<
- "variable declaration in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++14">,
- InGroup<CXXPre14Compat>, DefaultIgnore;
-def ext_constexpr_static_var : ExtWarn<
- "definition of a %select{static|thread_local}1 variable "
- "in a constexpr %select{function|constructor}0 "
- "is a C++23 extension">, InGroup<CXX23>;
def warn_cxx20_compat_constexpr_var : Warning<
- "definition of a %select{static variable|thread_local variable|variable "
- "of non-literal type}1 in a constexpr %select{function|constructor}0 "
+ "definition of a variable of non-literal type in a constexpr "
+ "%select{function|constructor}0 "
"is incompatible with C++ standards before C++23">,
InGroup<CXXPre23Compat>, DefaultIgnore;
def err_constexpr_local_var_non_literal_type : Error<
"variable of non-literal type %1 cannot be defined in a constexpr "
"%select{function|constructor}0 before C++23">;
-def ext_constexpr_local_var_no_init : ExtWarn<
- "uninitialized variable in a constexpr %select{function|constructor}0 "
- "is a C++20 extension">, InGroup<CXX20>;
-def warn_cxx17_compat_constexpr_local_var_no_init : Warning<
- "uninitialized variable in a constexpr %select{function|constructor}0 "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
def ext_constexpr_function_never_constant_expr : ExtWarn<
"%select{constexpr|consteval}1 %select{function|constructor}0 never produces a "
"constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
@@ -2982,41 +2954,11 @@ def err_constexpr_return_missing_expr : Error<
def warn_cxx11_compat_constexpr_body_no_return : Warning<
"constexpr function with no return statements is incompatible with C++ "
"standards before C++14">, InGroup<CXXPre14Compat>, DefaultIgnore;
-def ext_constexpr_body_multiple_return : ExtWarn<
- "multiple return statements in constexpr function is a C++14 extension">,
- InGroup<CXX14>;
-def warn_cxx11_compat_constexpr_body_multiple_return : Warning<
- "multiple return statements in constexpr function "
- "is incompatible with C++ standards before C++14">,
- InGroup<CXXPre14Compat>, DefaultIgnore;
def note_constexpr_body_previous_return : Note<
"previous return statement is here">;
def err_ms_constexpr_cannot_be_applied : Error<
"attribute 'msvc::constexpr' cannot be applied to the %select{constexpr|consteval|virtual}0 function %1">;
-// C++20 function try blocks in constexpr
-def ext_constexpr_function_try_block_cxx20 : ExtWarn<
- "function try block in constexpr %select{function|constructor}0 is "
- "a C++20 extension">, InGroup<CXX20>;
-def warn_cxx17_compat_constexpr_function_try_block : Warning<
- "function try block in constexpr %select{function|constructor}0 is "
- "incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
-
-def ext_constexpr_union_ctor_no_init : ExtWarn<
- "constexpr union constructor that does not initialize any member "
- "is a C++20 extension">, InGroup<CXX20>;
-def warn_cxx17_compat_constexpr_union_ctor_no_init : Warning<
- "constexpr union constructor that does not initialize any member "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
-def ext_constexpr_ctor_missing_init : ExtWarn<
- "constexpr constructor that does not initialize all members "
- "is a C++20 extension">, InGroup<CXX20>;
-def warn_cxx17_compat_constexpr_ctor_missing_init : Warning<
- "constexpr constructor that does not initialize all members "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
def note_constexpr_ctor_missing_init : Note<
"member not initialized by constructor">;
def note_non_literal_no_constexpr_ctors : Note<
@@ -5293,12 +5235,6 @@ def note_template_param_prev_default_arg_in_other_module : Note<
"previous default template argument defined in module %0">;
def err_template_param_default_arg_missing : Error<
"template parameter missing a default argument">;
-def ext_template_parameter_default_in_function_template : ExtWarn<
- "default template arguments for a function template are a C++11 extension">,
- InGroup<CXX11>;
-def warn_cxx98_compat_template_parameter_default_in_function_template : Warning<
- "default template arguments for a function template are incompatible with C++98">,
- InGroup<CXX98Compat>, DefaultIgnore;
def err_template_parameter_default_template_member : Error<
"cannot add a default template argument to the definition of a member of a "
"class template">;
@@ -5307,11 +5243,6 @@ def err_template_parameter_default_friend_template : Error<
def err_template_template_parm_no_parms : Error<
"template template parameter must have its own template parameters">;
-def ext_variable_template : ExtWarn<"variable templates are a C++14 extension">,
- InGroup<CXX14>;
-def warn_cxx11_compat_variable_template : Warning<
- "variable templates are incompatible with C++ standards before C++14">,
- InGroup<CXXPre14Compat>, DefaultIgnore;
def err_template_variable_noparams : Error<
"extraneous 'template<>' in declaration of variable %0">;
def err_template_member : Error<"non-static data member %0 cannot be declared as a template">;
@@ -5321,15 +5252,6 @@ def err_template_member_noparams : Error<
def err_template_tag_noparams : Error<
"extraneous 'template<>' in declaration of %0 %1">;
-def warn_cxx17_compat_adl_only_template_id : Warning<
- "use of function template name with no prior function template "
- "declaration in function call with explicit template arguments "
- "is incompatible with C++ standards before C++20">,
- InGroup<CXXPre20Compat>, DefaultIgnore;
-def ext_adl_only_template_id : ExtWarn<
- "use of function template name with no prior declaration in function call "
- "with explicit template arguments is a C++20 extension">, InGroup<CXX20>;
-
def warn_unqualified_call_to_std_cast_function : Warning<
"unqualified call to '%0'">, InGroup<DiagGroup<"unqualified-std-cast-call">>;
@@ -5468,11 +5390,6 @@ def err_template_arg_not_pointer_to_member_form : Error<
"non-type template argument is not a pointer to member constant">;
def err_template_arg_invalid : Error<
"non-type template argument '%0' is invalid">;
-def ext_template_arg_extra_parens : ExtWarn<
- "address non-type template argument cannot be surrounded by parentheses">;
-def warn_cxx98_compat_template_arg_extra_parens : Warning<
- "redundant parentheses surrounding address non-type template argument are "
- "incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
def err_pointer_to_member_type : Error<
"invalid use of pointer to member type after %select{.*|->*}0">;
def err_pointer_to_member_call_drops_quals : Error<
@@ -5887,11 +5804,6 @@ def err_typename_missing_template
def ext_typename_missing
: ExtWarn<"missing 'typename' prior to dependent type name %0">,
InGroup<DiagGroup<"typename-missing">>;
-def ext_typename_outside_of_template : ExtWarn<
- "'typename' occurs outside of a template">, InGroup<CXX11>;
-def warn_cxx98_compat_typename_outside_of_template : Warning<
- "use of 'typename' outside of a template is incompatible with C++98">,
- InGroup<CXX98Compat>, DefaultIgnore;
def err_typename_refers_to_using_value_decl : Error<
"typename specifier refers to a dependent using declaration for a value "
"%0 in %1">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 61e3cd4066a8d..67d734c8cf8a2 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7648,8 +7648,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// Only C++1y supports variable templates (N3651).
Diag(D.getIdentifierLoc(),
getLangOpts().CPlusPlus14
- ? diag::warn_cxx11_compat_variable_template
- : diag::ext_variable_template);
+ ? diag::compat_cxx14_variable_template
+ : diag::compat_pre_cxx14_variable_template);
}
}
} else {
@@ -7717,8 +7717,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// the program is ill-formed. C++11 drops this restriction.
Diag(D.getIdentifierLoc(),
getLangOpts().CPlusPlus11
- ? diag::warn_cxx98_compat_static_data_member_in_union
- : diag::ext_static_data_member_in_union)
+ ? diag::compat_cxx11_static_data_member_in_union
+ : diag::compat_pre_cxx11_static_data_member_in_union)
<< Name;
}
}
@@ -7821,8 +7821,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
<< FixItHint::CreateRemoval(D.getDeclSpec().getInlineSpecLoc());
} else {
Diag(D.getDeclSpec().getInlineSpecLoc(),
- getLangOpts().CPlusPlus17 ? diag::warn_cxx14_compat_inline_variable
- : diag::ext_inline_variable);
+ getLangOpts().CPlusPlus17 ? diag::compat_cxx17_inline_variable
+ : diag::compat_pre_cxx17_inline_variable);
NewVD->setInlineSpecified();
}
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index bd6321c46a78f..15b49ff2aee99 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDe...
[truncated]
|
There was a problem hiding this comment.
This is awesome, thank you for working on it! One question I have is whether we can also add a helper function for emitting these diagnostics. It's almost always an awkward dance of getLangOpts().CPlusPlusYZ ? diag::ext_... : diag::warn_... and it would be really nice if we could do something like EmitCompatDiags(Loc, diag::decomp_decl_spec) and the helper function handles translating that to the getLangOpts() dance with the correct diagnostic. Do you think something like that would be a plausible next step (along with doing the same for C diagnostics)?
I was thinking about that too. My first idea was that if we can rely on the order of records in tablegen influencing the order of diagnostic IDs, then one idea is to have some CompatDiag(loc, getLangOpts().CPlusPlusXY, diag::compat_cxx_xy_foo)where auto Sema::CompatDiag(SourceLocation Loc, bool Std, unsigned DiagId) {
return Diag(Loc, Std ? DiagId : DiagId + 1);but I’m not sure we can rely on that always doing the right thing.
That would be a bit more involved I think as we’d have to actually modify the diagnostics generator, but I think it should be possible; I haven’t looked into that yet though. |
It would definitely be more ergonomic if we could just write e.g. |
Yeah, I'm not certain that will always work (but it could be fine). I was thinking more along the lines of having tablegen emit something to help with it.
+1 but it could be a follow-up. |
I don't actually spend much time writing compatibility warnings, but think this is an improvement. As a followup perhaps, I would LOVE it if we could modify the DiagnosticsEmitter to make the calls here be WHERE: 2- Where the unsigned it takes is actually an index/etc into a decoder. The decoder is just a function that gets generated, so perhaps something like: BUT As I said, what we have in this patch is at least an improvement, and is perhaps OK because of that. |
That’s an interesting idea: we could use a new set of IDs that get converted to regular diagnostic IDs by the helper and also make it a scoped enum while we’re at it so you can’t confuse the two (because the new IDs would be usable with I’ll look into this next because I feel like this shouldn’t be all that hard to implement |
|
Added a comment that should explain how the multiclasses are used and what they do |
…cs (#132348) This is a follow-up to #132129. Currently, only `Parser` and `SemaBase` get a `DiagCompat()` helper; I’m planning to keep refactoring compatibility warnings and add more helpers to other classes as needed. I also refactored a single parser compat warning just to make sure everything works properly when diagnostics across multiple components (i.e. Sema and Parser in this case) are involved.
…cs (llvm#132348) This is a follow-up to llvm#132129. Currently, only `Parser` and `SemaBase` get a `DiagCompat()` helper; I’m planning to keep refactoring compatibility warnings and add more helpers to other classes as needed. I also refactored a single parser compat warning just to make sure everything works properly when diagnostics across multiple components (i.e. Sema and Parser in this case) are involved.
This introduces some tablegen helpers for defining compatibility warnings. The main aim of this is to both simplify adding new compatibility warnings as well as to unify the naming of compatibility warnings.
I’ve refactored ~half of the compatiblity warnings (that follow the usual scheme) in
DiagnosticSemaKinds.tdfor illustration purposes and also to simplify/unify the wording of some of them (I also corrected a typo in one of them as a drive-by fix).I haven’t (yet) migrated all warnings even in that one file, and there are some more specialised ones for which the scheme I’ve established here doesn’t work (e.g. because they’re warning+error instead of warning+extwarn; however, warning+extension is supported), but the point of this isn’t to implement all compatibility-related warnings this way, only to make the common case a bit easier to handle.
This currently also only handles C++ compatibility warnings, but it should be fairly straight-forward to extend the tablegen code so it can also be used for C compatibility warnings (if this gets merged, I’m planning to do that in a follow-up pr).
The vast majority of compatibility warnings are emitted by writing
Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::ext_... : diag::warn_...)in accordance with which I’ve chosen the following naming scheme:
Diag(Loc, getLangOpts().CPlusPlusYZ ? diag::compat_cxxyz_foo : diag::compat_pre_cxxyz_foo)That is, for a warning about a C++20 feature—i.e. C++≤17 compatibility—we get:
Diag(Loc, getLangOpts().CPlusPlus20 ? diag::compat_cxx20_foo : diag::compat_pre_cxx20_foo)While there is an argument to be made against writing ‘
compat_cxx20’ here since is technically a case of ‘C++17 compatibility’ and not ‘C++20 compatibility’, I at least find this easier to reason about, because I can just write the same number 3 times instead of having to useext_cxx20_foobutwarn_cxx17_foo. Instead, I like to read this as a warning about the ‘compatibility of a C++20 feature’ rather than ‘with C++17’.I also experimented with moving all compatibility warnings to a separate file, but 1. I don’t think it’s worth the effort, and 2. I think it hurts compile times a bit because at least in my testing I felt that I had to recompile more code than if we just keep e.g. Sema-specific compat warnings in the Sema diagnostics file.
Instead, I’ve opted to put them all in the same place within any one file; currently this is a the very top but I don’t really have strong opinions about this.