Amend template specialization DXASSERT conditions

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.

Author: Tim Corringham

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -830,11 +830,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(),
@@ -846,11 +847,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);
 
@@ -898,7 +900,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
 
@@ -933,7 +937,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
 
@@ -961,7 +967,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
 

tools/clang/test/DXC/specialization_with_fatal_error.hlsl

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