Skip to content

Commit

Permalink
Amend template specialization DXASSERT conditions
Browse files Browse the repository at this point in the history
Clang suppresses template specialization if a fatal error has been
reported in order to reduce the risk of a cascade of secondary error
diagnostics.

However, DXC DXASSERTs if template specialization fails - even if that
is due to an unrelated fatal error - which has the unintended result of
hiding the fatal error and hence providing no indication of what the
problem is.

The DXASSERT conditions have been amended so they are no longer raised
if a fatal error has been registered.
  • Loading branch information
Tim Corringham committed May 13, 2024
1 parent 01007bc commit 7f66c33
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 13 deletions.
34 changes: 21 additions & 13 deletions tools/clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -869,11 +869,12 @@ GetOrCreateTemplateSpecialization(ASTContext &context, Sema &sema,
if (specializationDecl->getInstantiatedFrom().isNull()) {
// InstantiateClassTemplateSpecialization returns true if it finds an
// error.
DXVERIFY_NOMSG(false ==
sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation,
true));
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);
}
return context.getTemplateSpecializationType(
TemplateName(templateDecl), templateArgs.data(), templateArgs.size(),
Expand All @@ -885,11 +886,12 @@ GetOrCreateTemplateSpecialization(ASTContext &context, Sema &sema,
templateDecl, templateArgsForDecl.data(), templateArgsForDecl.size(),
nullptr);
// InstantiateClassTemplateSpecialization returns true if it finds an error.
DXVERIFY_NOMSG(false ==
sema.InstantiateClassTemplateSpecialization(
NoLoc, specializationDecl,
TemplateSpecializationKind::TSK_ImplicitInstantiation,
true));
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);
templateDecl->AddSpecialization(specializationDecl, InsertPos);
specializationDecl->setImplicit(true);

Expand Down Expand Up @@ -937,7 +939,9 @@ static QualType GetOrCreateMatrixSpecialization(
DeclContext::lookup_result lookupResult =
matrixSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
DXASSERT(!lookupResult.empty(),
// 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(),
"otherwise matrix handle cannot be looked up");
#endif

Expand Down Expand Up @@ -972,7 +976,9 @@ GetOrCreateVectorSpecialization(ASTContext &context, Sema *sema,
DeclContext::lookup_result lookupResult =
vectorSpecializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
DXASSERT(!lookupResult.empty(),
// 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(),
"otherwise vector handle cannot be looked up");
#endif

Expand Down Expand Up @@ -1028,7 +1034,9 @@ GetOrCreateNodeOutputRecordSpecialization(ASTContext &context, Sema *sema,
DeclContext::lookup_result lookupResult =
specializationType->getAsCXXRecordDecl()->lookup(
DeclarationName(&context.Idents.get(StringRef("h"))));
DXASSERT(!lookupResult.empty(),
// 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(),
"otherwise *NodeOutputRecords handle cannot be looked up");
#endif

Expand Down
22 changes: 22 additions & 0 deletions tools/clang/test/DXC/specialization_with_fatal_error.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %dxc -T lib_6_8 -verify %s

// 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
// has been registered, and that the error diagnostic is generated.

float a;

// the include file doesn't exist - this should produce a fatal error diagnostic
// expected-error@+1 {{'a.h' file not found}}
#include "a.h"

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;
}

0 comments on commit 7f66c33

Please sign in to comment.