ChuanqiXu created this revision.
ChuanqiXu added reviewers: urnathan, erichkeane, aaron.ballman, rjmccall,
rsmith.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.
This patch tries to refactor ODR checking process for default template argument
in ASTReader. We could save about 100 lines of code after the refactoring. The
patch is intended to be NFC.
Test Plan:
check-all
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118437
Files:
clang/include/clang/Basic/DiagnosticSerializationKinds.td
clang/lib/Serialization/ASTReader.cpp
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -10134,13 +10134,6 @@
assert(!FirstTemplate == !SecondTemplate &&
"Both pointers should be null or non-null");
- enum ODRTemplateDifference {
- ParamEmptyName,
- ParamName,
- ParamSingleDefaultArgument,
- ParamDifferentDefaultArgument,
- };
-
if (FirstTemplate && SecondTemplate) {
DeclHashes FirstTemplateHashes;
DeclHashes SecondTemplateHashes;
@@ -10166,155 +10159,60 @@
if (FirstIt->second == SecondIt->second)
continue;
- auto ODRDiagTemplateError = [FirstRecord, &FirstModule, this](
- SourceLocation Loc, SourceRange Range,
- ODRTemplateDifference DiffType) {
- return Diag(Loc, diag::err_module_odr_violation_template_parameter)
- << FirstRecord << FirstModule.empty() << FirstModule << Range
- << DiffType;
- };
- auto ODRDiagTemplateNote = [&SecondModule, this](
- SourceLocation Loc, SourceRange Range,
- ODRTemplateDifference DiffType) {
- return Diag(Loc, diag::note_module_odr_violation_template_parameter)
- << SecondModule << Range << DiffType;
- };
-
const NamedDecl* FirstDecl = cast<NamedDecl>(FirstIt->first);
const NamedDecl* SecondDecl = cast<NamedDecl>(SecondIt->first);
assert(FirstDecl->getKind() == SecondDecl->getKind() &&
"Parameter Decl's should be the same kind.");
+ enum ODRTemplateDifference {
+ ParamEmptyName,
+ ParamName,
+ ParamSingleDefaultArgument,
+ ParamDifferentDefaultArgument,
+ };
+
+ auto hasDefaultArg = [](const NamedDecl *D) {
+ if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(D))
+ return TTP->hasDefaultArgument() &&
+ !TTP->defaultArgumentWasInherited();
+ if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(D))
+ return NTTP->hasDefaultArgument() &&
+ !NTTP->defaultArgumentWasInherited();
+ auto *TTP = cast<TemplateTemplateParmDecl>(D);
+ return TTP->hasDefaultArgument() &&
+ !TTP->defaultArgumentWasInherited();
+ };
+ bool hasFirstArg = hasDefaultArg(FirstDecl);
+ bool hasSecondArg = hasDefaultArg(SecondDecl);
+
+ ODRTemplateDifference ErrDiffType;
+ ODRTemplateDifference NoteDiffType;
+
DeclarationName FirstName = FirstDecl->getDeclName();
DeclarationName SecondName = SecondDecl->getDeclName();
if (FirstName != SecondName) {
- const bool FirstNameEmpty =
+ bool FirstNameEmpty =
FirstName.isIdentifier() && !FirstName.getAsIdentifierInfo();
- const bool SecondNameEmpty =
- SecondName.isIdentifier() && !SecondName.getAsIdentifierInfo();
- assert((!FirstNameEmpty || !SecondNameEmpty) &&
- "Both template parameters cannot be unnamed.");
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- FirstNameEmpty ? ParamEmptyName : ParamName)
- << FirstName;
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- SecondNameEmpty ? ParamEmptyName : ParamName)
- << SecondName;
- break;
- }
-
- switch (FirstDecl->getKind()) {
- default:
- llvm_unreachable("Invalid template parameter type.");
- case Decl::TemplateTypeParm: {
- const auto *FirstParam = cast<TemplateTypeParmDecl>(FirstDecl);
- const auto *SecondParam = cast<TemplateTypeParmDecl>(SecondDecl);
- const bool HasFirstDefaultArgument =
- FirstParam->hasDefaultArgument() &&
- !FirstParam->defaultArgumentWasInherited();
- const bool HasSecondDefaultArgument =
- SecondParam->hasDefaultArgument() &&
- !SecondParam->defaultArgumentWasInherited();
-
- if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasFirstDefaultArgument;
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasSecondDefaultArgument;
- break;
- }
-
- assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
- "Expecting default arguments.");
-
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
-
- break;
- }
- case Decl::NonTypeTemplateParm: {
- const auto *FirstParam = cast<NonTypeTemplateParmDecl>(FirstDecl);
- const auto *SecondParam = cast<NonTypeTemplateParmDecl>(SecondDecl);
- const bool HasFirstDefaultArgument =
- FirstParam->hasDefaultArgument() &&
- !FirstParam->defaultArgumentWasInherited();
- const bool HasSecondDefaultArgument =
- SecondParam->hasDefaultArgument() &&
- !SecondParam->defaultArgumentWasInherited();
-
- if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasFirstDefaultArgument;
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasSecondDefaultArgument;
- break;
- }
-
- assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
- "Expecting default arguments.");
-
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
-
- break;
- }
- case Decl::TemplateTemplateParm: {
- const auto *FirstParam = cast<TemplateTemplateParmDecl>(FirstDecl);
- const auto *SecondParam =
- cast<TemplateTemplateParmDecl>(SecondDecl);
- const bool HasFirstDefaultArgument =
- FirstParam->hasDefaultArgument() &&
- !FirstParam->defaultArgumentWasInherited();
- const bool HasSecondDefaultArgument =
- SecondParam->hasDefaultArgument() &&
- !SecondParam->defaultArgumentWasInherited();
-
- if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasFirstDefaultArgument;
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamSingleDefaultArgument)
- << HasSecondDefaultArgument;
- break;
- }
-
- assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
- "Expecting default arguments.");
-
- ODRDiagTemplateError(FirstDecl->getLocation(),
- FirstDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
- ODRDiagTemplateNote(SecondDecl->getLocation(),
- SecondDecl->getSourceRange(),
- ParamDifferentDefaultArgument);
-
- break;
- }
- }
-
+ bool SecondNameEmpty = SecondName.isIdentifier() &&
+ !SecondName.getAsIdentifierInfo();
+ ErrDiffType = FirstNameEmpty ? ParamEmptyName : ParamName;
+ NoteDiffType = SecondNameEmpty ? ParamEmptyName : ParamName;
+ } else if (hasFirstArg == hasSecondArg)
+ ErrDiffType = NoteDiffType = ParamDifferentDefaultArgument;
+ else
+ ErrDiffType = NoteDiffType = ParamSingleDefaultArgument;
+
+ Diag(FirstDecl->getLocation(),
+ diag::err_module_odr_violation_template_parameter)
+ << FirstRecord << FirstModule.empty() << FirstModule
+ << FirstDecl->getSourceRange() << ErrDiffType << hasFirstArg
+ << FirstName;
+ Diag(SecondDecl->getLocation(),
+ diag::note_module_odr_violation_template_parameter)
+ << SecondModule << SecondDecl->getSourceRange() << NoteDiffType
+ << hasSecondArg << SecondName;
break;
}
Index: clang/include/clang/Basic/DiagnosticSerializationKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -154,16 +154,15 @@
"%select{definition in module '%2'|defined here}1 found "
"%select{"
"unnamed template parameter|"
- "template parameter %4|"
+ "template parameter %5|"
"template parameter with %select{no |}4default argument|"
"template parameter with default argument}3">;
-
def note_module_odr_violation_template_parameter : Note <
"but in '%0' found "
"%select{"
"unnamed template parameter %2|"
- "template parameter %2|"
+ "template parameter %3|"
"template parameter with %select{no |}2default argument|"
"template parameter with different default argument}1">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
