Skip to content

[clang] Add scoped enum support to StreamingDiagnostic #138089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2025
Merged

[clang] Add scoped enum support to StreamingDiagnostic #138089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions clang/include/clang/Basic/Diagnostic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,22 @@ operator<<(const StreamingDiagnostic &DB, T *DC) {
return DB;
}

// Convert scope enums to their underlying type, so that we don't have
// clutter the emitting code with `llvm::to_underlying()`.
// We also need to disable implicit conversion for the first argument,
// because classes that derive from StreamingDiagnostic define their own
// templated operator<< that accept a wide variety of types, leading
// to ambiguity.
template <typename T, typename U>
inline std::enable_if_t<
std::is_same_v<std::remove_const_t<T>, StreamingDiagnostic> &&
llvm::is_scoped_enum_v<std::remove_reference_t<U>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this to be std::remove_cvref_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that const qualifier doesn't get in the way, and that's what we do in other similar cases. Happy to be proven wrong by buildbots.

const StreamingDiagnostic &>
operator<<(const T &DB, U &&SE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why operator<<(const StreamingDiagnostic &DB, U &&SE) would not work - if there are ambiguities in the derived classes, can we fix those instead? you have examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do what you suggest, this pre-existing operator<<

llvm-project/clang/include/clang/Basic/Diagnostic.h

Lines 1297 to 1302 in 1121a49

template <typename T> const DiagnosticBuilder &operator<<(const T &V) const {
assert(isActive() && "Clients must not add to cleared diagnostic!");
const StreamingDiagnostic &DB = *this;
DB << V;
return *this;
}

causes the following ambiguity:

/home/user/endill/llvm-project/clang/lib/Parse/Parser.cpp:229:9: error: use of overloaded operator '<<' is ambiguous (with operand types 'DiagnosticBuilder' and 'ExtraSemiKind')
  228 |     Diag(StartLoc, diag::ext_extra_semi)
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  229 |         << Kind
      |         ^  ~~~~
/home/user/endill/llvm-project/clang/include/clang/Basic/Diagnostic.h:1297:50: note: candidate function [with T = clang::ExtraSemiKind]
 1297 |   template <typename T> const DiagnosticBuilder &operator<<(const T &V) const {
      |                                                  ^
/home/user/endill/llvm-project/clang/include/clang/Basic/Diagnostic.h:1443:1: note: candidate function [with U = clang::ExtraSemiKind &]
 1443 | operator<<(const StreamingDiagnostic &DB, U &&SE) {
      | ^

This patch is far from my first attempt to implement this, but the first one that didn't run into one of those ambiguity errors. Overload set of operator<< for diagnostics is somewhat convoluted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given only two classes seem to inherit from StreamingDiagnostic, maybe we can add an overload to each of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that. First you add an overload, then you need to modify the existing overload to not be a viable candidate when a scoped enum is passed. The new overload also have to copy any checks or asserts that the old one does, otherwise they will be skipped. I wasn't able to make it work, and it would be more complicated than the approach here anyway.

DB << llvm::to_underlying(SE);
return DB;
}

inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
SourceLocation L) {
DB.AddSourceRange(CharSourceRange::getTokenRange(L));
Expand Down
11 changes: 0 additions & 11 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,6 @@ enum class AssignmentAction {
Casting,
Passing_CFAudited
};
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
const AssignmentAction &AA) {
DB << llvm::to_underlying(AA);
return DB;
}

namespace threadSafety {
class BeforeSet;
Expand Down Expand Up @@ -15471,12 +15466,6 @@ void Sema::PragmaStack<Sema::AlignPackInfo>::Act(SourceLocation PragmaLocation,
llvm::StringRef StackSlotLabel,
AlignPackInfo Value);

inline const StreamingDiagnostic &
operator<<(const StreamingDiagnostic &DB, Sema::StringEvaluationContext Ctx) {
DB << llvm::to_underlying(Ctx);
return DB;
}

} // end namespace clang

#endif
6 changes: 2 additions & 4 deletions clang/lib/AST/ODRDiagsEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,8 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
}
if (FirstMethod->getImplementationControl() !=
SecondMethod->getImplementationControl()) {
DiagError(ControlLevel)
<< llvm::to_underlying(FirstMethod->getImplementationControl());
DiagNote(ControlLevel) << llvm::to_underlying(
SecondMethod->getImplementationControl());
DiagError(ControlLevel) << FirstMethod->getImplementationControl();
DiagNote(ControlLevel) << SecondMethod->getImplementationControl();
return true;
}
if (FirstMethod->isThisDeclarationADesignatedInitializer() !=
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2578,7 +2578,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
if (TemplateInfo.Kind != ParsedTemplateKind::NonTemplate &&
D.isFirstDeclarator()) {
Diag(CommaLoc, diag::err_multiple_template_declarators)
<< llvm::to_underlying(TemplateInfo.Kind);
<< TemplateInfo.Kind;
}

// Parse the next declarator.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3472,7 +3472,7 @@ Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclaration(
if (TemplateInfo.Kind != ParsedTemplateKind::NonTemplate &&
DeclaratorInfo.isFirstDeclarator()) {
Diag(CommaLoc, diag::err_multiple_template_declarators)
<< llvm::to_underlying(TemplateInfo.Kind);
<< TemplateInfo.Kind;
}

// Parse the next declarator.
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Parse/ParsePragma.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2341,7 +2341,8 @@ void PragmaClangSectionHandler::HandlePragma(Preprocessor &PP,
SourceLocation PragmaLocation = Tok.getLocation();
PP.Lex(Tok); // eat ['bss'|'data'|'rodata'|'text']
if (Tok.isNot(tok::equal)) {
PP.Diag(Tok.getLocation(), diag::err_pragma_clang_section_expected_equal) << llvm::to_underlying(SecKind);
PP.Diag(Tok.getLocation(), diag::err_pragma_clang_section_expected_equal)
<< SecKind;
return;
}

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/Parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, DeclSpec::TST TST) {

if (Kind != ExtraSemiKind::AfterMemberFunctionDefinition || HadMultipleSemis)
Diag(StartLoc, diag::ext_extra_semi)
<< llvm::to_underlying(Kind)
<< Kind
<< DeclSpec::getSpecifierName(
TST, Actions.getASTContext().getPrintingPolicy())
<< FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
Expand Down
9 changes: 3 additions & 6 deletions clang/lib/Sema/SemaAccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,24 +1670,21 @@ Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
case InitializedEntity::EK_Base:
PD = PDiag(diag::err_access_base_ctor);
PD << Entity.isInheritedVirtualBase()
<< Entity.getBaseSpecifier()->getType()
<< llvm::to_underlying(getSpecialMember(Constructor));
<< Entity.getBaseSpecifier()->getType() << getSpecialMember(Constructor);
break;

case InitializedEntity::EK_Member:
case InitializedEntity::EK_ParenAggInitMember: {
const FieldDecl *Field = cast<FieldDecl>(Entity.getDecl());
PD = PDiag(diag::err_access_field_ctor);
PD << Field->getType()
<< llvm::to_underlying(getSpecialMember(Constructor));
PD << Field->getType() << getSpecialMember(Constructor);
break;
}

case InitializedEntity::EK_LambdaCapture: {
StringRef VarName = Entity.getCapturedVarName();
PD = PDiag(diag::err_access_lambda_capture);
PD << VarName << Entity.getType()
<< llvm::to_underlying(getSpecialMember(Constructor));
PD << VarName << Entity.getType() << getSpecialMember(Constructor);
break;
}

Expand Down
17 changes: 7 additions & 10 deletions clang/lib/Sema/SemaCUDA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,7 @@ bool SemaCUDA::inferTargetForImplicitSpecialMember(CXXRecordDecl *ClassDecl,
if (Diagnose) {
Diag(ClassDecl->getLocation(),
diag::note_implicit_member_target_infer_collision)
<< (unsigned)CSM << llvm::to_underlying(*InferredTarget)
<< llvm::to_underlying(BaseMethodTarget);
<< (unsigned)CSM << *InferredTarget << BaseMethodTarget;
}
MemberDecl->addAttr(
CUDAInvalidTargetAttr::CreateImplicit(getASTContext()));
Expand Down Expand Up @@ -496,8 +495,7 @@ bool SemaCUDA::inferTargetForImplicitSpecialMember(CXXRecordDecl *ClassDecl,
if (Diagnose) {
Diag(ClassDecl->getLocation(),
diag::note_implicit_member_target_infer_collision)
<< (unsigned)CSM << llvm::to_underlying(*InferredTarget)
<< llvm::to_underlying(FieldMethodTarget);
<< (unsigned)CSM << *InferredTarget << FieldMethodTarget;
}
MemberDecl->addAttr(
CUDAInvalidTargetAttr::CreateImplicit(getASTContext()));
Expand Down Expand Up @@ -713,7 +711,7 @@ void SemaCUDA::checkAllowedInitializer(VarDecl *VD) {
if (InitFnTarget != CUDAFunctionTarget::Host &&
InitFnTarget != CUDAFunctionTarget::HostDevice) {
Diag(VD->getLocation(), diag::err_ref_bad_target_global_initializer)
<< llvm::to_underlying(InitFnTarget) << InitFn;
<< InitFnTarget << InitFn;
Diag(InitFn->getLocation(), diag::note_previous_decl) << InitFn;
VD->setInvalidDecl();
}
Expand Down Expand Up @@ -952,8 +950,8 @@ bool SemaCUDA::CheckCall(SourceLocation Loc, FunctionDecl *Callee) {

SemaDiagnosticBuilder(DiagKind, Loc, diag::err_ref_bad_target, Caller,
SemaRef)
<< llvm::to_underlying(IdentifyTarget(Callee)) << /*function*/ 0 << Callee
<< llvm::to_underlying(IdentifyTarget(Caller));
<< IdentifyTarget(Callee) << /*function*/ 0 << Callee
<< IdentifyTarget(Caller);
if (!Callee->getBuiltinID())
SemaDiagnosticBuilder(DiagKind, Callee->getLocation(),
diag::note_previous_decl, Caller, SemaRef)
Expand Down Expand Up @@ -1049,8 +1047,7 @@ void SemaCUDA::checkTargetOverload(FunctionDecl *NewFD,
(NewTarget == CUDAFunctionTarget::Global) ||
(OldTarget == CUDAFunctionTarget::Global)) {
Diag(NewFD->getLocation(), diag::err_cuda_ovl_target)
<< llvm::to_underlying(NewTarget) << NewFD->getDeclName()
<< llvm::to_underlying(OldTarget) << OldFD;
<< NewTarget << NewFD->getDeclName() << OldTarget << OldFD;
Diag(OldFD->getLocation(), diag::note_previous_declaration);
NewFD->setInvalidDecl();
break;
Expand All @@ -1060,7 +1057,7 @@ void SemaCUDA::checkTargetOverload(FunctionDecl *NewFD,
(NewTarget == CUDAFunctionTarget::Device &&
OldTarget == CUDAFunctionTarget::Host)) {
Diag(NewFD->getLocation(), diag::warn_offload_incompatible_redeclare)
<< llvm::to_underlying(NewTarget) << llvm::to_underlying(OldTarget);
<< NewTarget << OldTarget;
Diag(OldFD->getLocation(), diag::note_previous_declaration);
}
}
Expand Down
8 changes: 3 additions & 5 deletions clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8339,8 +8339,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
} else {
EmitFormatDiagnostic(
S.PDiag(diag::warn_non_pod_vararg_with_format_string)
<< S.getLangOpts().CPlusPlus11 << ExprTy
<< llvm::to_underlying(CallType)
<< S.getLangOpts().CPlusPlus11 << ExprTy << CallType
<< AT.getRepresentativeTypeName(S.Context) << CSR
<< E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation*/ false, CSR);
Expand All @@ -8354,16 +8353,15 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
else if (ExprTy->isObjCObjectType())
EmitFormatDiagnostic(
S.PDiag(diag::err_cannot_pass_objc_interface_to_vararg_format)
<< S.getLangOpts().CPlusPlus11 << ExprTy
<< llvm::to_underlying(CallType)
<< S.getLangOpts().CPlusPlus11 << ExprTy << CallType
<< AT.getRepresentativeTypeName(S.Context) << CSR
<< E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation*/ false, CSR);
else
// FIXME: If this is an initializer list, suggest removing the braces
// or inserting a cast to the target type.
S.Diag(E->getBeginLoc(), diag::err_cannot_pass_to_vararg_format)
<< isa<InitListExpr>(E) << ExprTy << llvm::to_underlying(CallType)
<< isa<InitListExpr>(E) << ExprTy << CallType
<< AT.getRepresentativeTypeName(S.Context) << E->getSourceRange();
break;
}
Expand Down
27 changes: 12 additions & 15 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4034,13 +4034,13 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
} else {
Diag(NewMethod->getLocation(),
diag::err_definition_of_implicitly_declared_member)
<< New << llvm::to_underlying(getSpecialMember(OldMethod));
<< New << getSpecialMember(OldMethod);
return true;
}
} else if (OldMethod->getFirstDecl()->isExplicitlyDefaulted() && !isFriend) {
Diag(NewMethod->getLocation(),
diag::err_definition_of_explicitly_defaulted_member)
<< llvm::to_underlying(getSpecialMember(OldMethod));
<< getSpecialMember(OldMethod);
return true;
}
}
Expand Down Expand Up @@ -5246,7 +5246,7 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
if (DS.isModulePrivateSpecified() &&
Tag && Tag->getDeclContext()->isFunctionOrMethod())
Diag(DS.getModulePrivateSpecLoc(), diag::err_module_private_local_class)
<< llvm::to_underlying(Tag->getTagKind())
<< Tag->getTagKind()
<< FixItHint::CreateRemoval(DS.getModulePrivateSpecLoc());

ActOnDocumentableDecl(TagD);
Expand Down Expand Up @@ -7719,15 +7719,14 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// data members.
Diag(D.getIdentifierLoc(),
diag::err_static_data_member_not_allowed_in_local_class)
<< Name << RD->getDeclName()
<< llvm::to_underlying(RD->getTagKind());
<< Name << RD->getDeclName() << RD->getTagKind();
} else if (AnonStruct) {
// C++ [class.static.data]p4: Unnamed classes and classes contained
// directly or indirectly within unnamed classes shall not contain
// static data members.
Diag(D.getIdentifierLoc(),
diag::err_static_data_member_not_allowed_in_anon_struct)
<< Name << llvm::to_underlying(AnonStruct->getTagKind());
<< Name << AnonStruct->getTagKind();
Invalid = true;
} else if (RD->isUnion()) {
// C++98 [class.union]p1: If a union contains a static data member,
Expand Down Expand Up @@ -17658,7 +17657,7 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,

// A tag 'foo::bar' must already exist.
Diag(NameLoc, diag::err_not_tag_in_scope)
<< llvm::to_underlying(Kind) << Name << DC << SS.getRange();
<< Kind << Name << DC << SS.getRange();
Name = nullptr;
Invalid = true;
goto CreateNewDecl;
Expand Down Expand Up @@ -18116,7 +18115,7 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
!Previous.isForRedeclaration()) {
NonTagKind NTK = getNonTagTypeDeclKind(PrevDecl, Kind);
Diag(NameLoc, diag::err_tag_reference_non_tag)
<< PrevDecl << NTK << llvm::to_underlying(Kind);
<< PrevDecl << NTK << Kind;
Diag(PrevDecl->getLocation(), diag::note_declared_at);
Invalid = true;

Expand Down Expand Up @@ -19015,8 +19014,7 @@ bool Sema::CheckNontrivialField(FieldDecl *FD) {
getLangOpts().CPlusPlus11
? diag::warn_cxx98_compat_nontrivial_union_or_anon_struct_member
: diag::err_illegal_union_or_anon_struct_member)
<< FD->getParent()->isUnion() << FD->getDeclName()
<< llvm::to_underlying(member);
<< FD->getParent()->isUnion() << FD->getDeclName() << member;
DiagnoseNontrivial(RDecl, member);
return !getLangOpts().CPlusPlus11;
}
Expand Down Expand Up @@ -19356,8 +19354,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
unsigned DiagID = 0;
if (!Record->isUnion() && !IsLastField) {
Diag(FD->getLocation(), diag::err_flexible_array_not_at_end)
<< FD->getDeclName() << FD->getType()
<< llvm::to_underlying(Record->getTagKind());
<< FD->getDeclName() << FD->getType() << Record->getTagKind();
Diag((*(i + 1))->getLocation(), diag::note_next_field_declaration);
FD->setInvalidDecl();
EnclosingDecl->setInvalidDecl();
Expand All @@ -19373,18 +19370,18 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,

if (DiagID)
Diag(FD->getLocation(), DiagID)
<< FD->getDeclName() << llvm::to_underlying(Record->getTagKind());
<< FD->getDeclName() << Record->getTagKind();
// While the layout of types that contain virtual bases is not specified
// by the C++ standard, both the Itanium and Microsoft C++ ABIs place
// virtual bases after the derived members. This would make a flexible
// array member declared at the end of an object not adjacent to the end
// of the type.
if (CXXRecord && CXXRecord->getNumVBases() != 0)
Diag(FD->getLocation(), diag::err_flexible_array_virtual_base)
<< FD->getDeclName() << llvm::to_underlying(Record->getTagKind());
<< FD->getDeclName() << Record->getTagKind();
if (!getLangOpts().C99)
Diag(FD->getLocation(), diag::ext_c99_flexible_array_member)
<< FD->getDeclName() << llvm::to_underlying(Record->getTagKind());
<< FD->getDeclName() << Record->getTagKind();

// If the element type has a non-trivial destructor, we would not
// implicitly destroy the elements, so disallow it for now.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5058,7 +5058,7 @@ static void handleSharedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
}
if (S.getLangOpts().CUDA && VD->hasLocalStorage() &&
S.CUDA().DiagIfHostCode(AL.getLoc(), diag::err_cuda_host_shared)
<< llvm::to_underlying(S.CUDA().CurrentTarget()))
<< S.CUDA().CurrentTarget())
return;
D->addAttr(::new (S.Context) CUDASharedAttr(S.Context, AL));
}
Expand Down
Loading