Skip to content

Commit

Permalink
Update template specialization assert handling
Browse files Browse the repository at this point in the history 
Amend the fix for DXASSERTS in template specialization by incorporating
changes from review comments.
  • Loading branch information
Tim Corringham committed Aug 19, 2024
1 parent 76a8f5f commit 775da60
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 53 deletions.
90 changes: 43 additions & 47 deletions tools/clang/lib/Sema/SemaHLSL.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -830,12 +830,10 @@ GetOrCreateTemplateSpecialization(ASTContext &context, Sema &sema,
if (specializationDecl->getInstantiatedFrom().isNull()) {
// InstantiateClassTemplateSpecialization returns true if it finds an
// error.
bool errorFound = sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation, true);
// Template specialization is suppressed if a fatal error has already been
// registered so don't assert in such cases.
DXVERIFY_NOMSG(sema.Diags.hasFatalErrorOccurred() || !errorFound);
if (sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation, true))
return QualType();
}
return context.getTemplateSpecializationType(
TemplateName(templateDecl), templateArgs.data(), templateArgs.size(),
Expand All @@ -846,19 +844,20 @@ GetOrCreateTemplateSpecialization(ASTContext &context, Sema &sema,
context, TagDecl::TagKind::TTK_Class, currentDeclContext, NoLoc, NoLoc,
templateDecl, templateArgsForDecl.data(), templateArgsForDecl.size(),
nullptr);
// InstantiateClassTemplateSpecialization returns true if it finds an error.
bool errorFound = sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation, true);
// Template specialization is suppressed if a fatal error has already been
// registered so don't assert in such cases.
DXVERIFY_NOMSG(sema.Diags.hasFatalErrorOccurred() || !errorFound);
// template specialization isn't performed if a fatal error has occurred
if (!sema.Diags.hasFatalErrorOccurred()) {
// InstantiateClassTemplateSpecialization returns true if it finds an error.
[[maybe_unused]] bool errorFound =
sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation, true);
assert(!errorFound && "template specialization failed");
}
templateDecl->AddSpecialization(specializationDecl, InsertPos);
specializationDecl->setImplicit(true);

QualType canonType = context.getTypeDeclType(specializationDecl);
DXASSERT(isa<RecordType>(canonType),
"type of non-dependent specialization is not a RecordType");
assert(isa<RecordType>(canonType) &&
"type of non-dependent specialization is not a RecordType");
TemplateArgumentListInfo templateArgumentList(NoLoc, NoLoc);
TemplateArgumentLocInfo NoTemplateArgumentLocInfo;
for (unsigned i = 0; i < templateArgs.size(); i++) {
Expand Down Expand Up @@ -893,18 +892,17 @@ static QualType GetOrCreateMatrixSpecialization(
context, *sema, matrixTemplateDecl,
ArrayRef<TemplateArgument>(templateArgs));

#ifndef NDEBUG
// Verify that we can read the field member from the template record.
DXASSERT(matrixSpecializationType->getAsCXXRecordDecl(),
if (!matrixSpecializationType.isNull() &&
!sema->Diags.hasFatalErrorOccurred()) {
assert(matrixSpecializationType->getAsCXXRecordDecl() &&
"type of non-dependent specialization is not a RecordType");
DeclContext::lookup_result lookupResult =
matrixSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
// Template specialization is suppressed if a fatal error has been registered
// so only assert if lookup failed for some other reason.
DXASSERT(sema->Diags.hasFatalErrorOccurred() || !lookupResult.empty(),
// Verify that we can read the field member from the template record.
[[maybe_unused]] DeclContext::lookup_result lookupResult =
matrixSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
assert(!lookupResult.empty() &&
"otherwise matrix handle cannot be looked up");
#endif
}

return matrixSpecializationType;
}
Expand All @@ -930,18 +928,17 @@ GetOrCreateVectorSpecialization(ASTContext &context, Sema *sema,
context, *sema, vectorTemplateDecl,
ArrayRef<TemplateArgument>(templateArgs));

#ifndef NDEBUG
// Verify that we can read the field member from the template record.
DXASSERT(vectorSpecializationType->getAsCXXRecordDecl(),
if (!vectorSpecializationType.isNull() &&
!sema->Diags.hasFatalErrorOccurred()) {
assert(vectorSpecializationType->getAsCXXRecordDecl() &&
"type of non-dependent specialization is not a RecordType");
DeclContext::lookup_result lookupResult =
vectorSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
// Template specialization is suppressed if a fatal error has been registered
// so only assert if lookup failed for some other reason.
DXASSERT(sema->Diags.hasFatalErrorOccurred() || !lookupResult.empty(),
// Verify that we can read the field member from the template record.
[[maybe_unused]] DeclContext::lookup_result lookupResult =
vectorSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
assert(!lookupResult.empty() &&
"otherwise vector handle cannot be looked up");
#endif
}

return vectorSpecializationType;
}
Expand All @@ -960,18 +957,16 @@ GetOrCreateNodeOutputRecordSpecialization(ASTContext &context, Sema *sema,
QualType specializationType = GetOrCreateTemplateSpecialization(
context, *sema, templateDecl, ArrayRef<TemplateArgument>(templateArgs));

#ifdef DBG
// Verify that we can read the field member from the template record.
DXASSERT(specializationType->getAsCXXRecordDecl(),
if (!specializationType.isNull() && !sema->Diags.hasFatalErrorOccurred()) {
assert(specializationType->getAsCXXRecordDecl() &&
"type of non-dependent specialization is not a RecordType");
DeclContext::lookup_result lookupResult =
specializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
// Template specialization is suppressed if a fatal error has been registered
// so only assert if lookup failed for some other reason.
DXASSERT(sema->Diags.hasFatalErrorOccurred() || !lookupResult.empty(),
// Verify that we can read the field member from the template record.
[[maybe_unused]] DeclContext::lookup_result lookupResult =
specializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
assert(!lookupResult.empty() &&
"otherwise *NodeOutputRecords handle cannot be looked up");
#endif
}

return specializationType;
}
Expand Down Expand Up @@ -5105,12 +5100,13 @@ class HLSLExternalSource : public ExternalSemaSource {
// This is a bit of a special case we need to handle. Because the
// buffer types don't use their template parameter in a way that would
// force instantiation, we need to force specialization here.
GetOrCreateTemplateSpecialization(
QualType Ty = GetOrCreateTemplateSpecialization(
*m_context, *m_sema,
cast<ClassTemplateDecl>(
TST->getTemplateName().getAsTemplateDecl()),
llvm::ArrayRef<TemplateArgument>(TST->getArgs(),
TST->getNumArgs()));
return Ty.isNull();
}
if (const RecordType *recordType = argType->getAs<RecordType>()) {
if (!recordType->getDecl()->isCompleteDefinition()) {
Expand Down
12 changes: 6 additions & 6 deletions tools/clang/test/DXC/specialization_with_fatal_error.hlsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

// Clang suppresses template specialization if a fatal error has been
// registered (this reduces the risk of a cascade of secondary errors).
// However, DXC DXASSERTs if a template specialization fails - which
// prevents the error diagnostic being generated.
// We check here that a DXASSERT is no longer raised if a fatal error
// However, DXC asserted if a template specialization failed - which
// prevented the error diagnostic being generated.
// We check here that an assert is no longer raised if a fatal error
// has been registered, and that the error diagnostic is generated.

float a;
Expand All @@ -16,7 +16,7 @@ float a;
void b() {};

int3 c(int X) {
// DXASSERT was triggered if include file a.h doesn't exist, and the error
// diagnostic was not produced.
return X.xxx;
// an assert was triggered for the expression below when include file a.h
// doesn't exist, and the error diagnostic expected above was not produced.
return X.xxx;
}

0 comments on commit 775da60

Please sign in to comment.