address Tex, and make a flawed attempt at writing a unit test

Author: bob80905

include/dxc/Support/dxcapi.extval.h

@@ -1,20 +1,34 @@
 #include "dxc/Support/dxcapi.use.h"
-#include "dxc/dxcapi.h"
+#include <string>
 
 class DxcDllExtValidationSupport : public dxc::DxcDllSupport {
   // DxcDllExtValidationSupport manages the
   // lifetime of dxcompiler.dll, while the member, m_DxilSupport,
   // manages the lifetime of dxil.dll
+  protected:
   dxc::DxcDllSupport DxilSupport;
 
   std::string DxilDllPath;
+
+  HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName) override;
+
+  public:
   // override DxcDllSupport's implementation of InitializeInternal,
   // adding the environment variable value check for a path to a dxil.dll
 
   std::string GetDxilDllPath() { return DxilDllPath; }
   bool DxilDllFailedToLoad() {
     return !DxilDllPath.empty() && !DxilSupport.IsEnabled();
   }
-
-  HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName);
+  
+  void Cleanup() override {
+    DxilSupport.Cleanup();
+    DxcDllSupport::Cleanup();
+  }
+  
+  HMODULE Detach() override {
+    // Can't Detach and return a handle for DxilSupport. Cleanup() instead.
+    DxilSupport.Cleanup();
+    return DxcDllSupport::Detach();
+  }
 };

include/dxc/Support/dxcapi.use.h

@@ -13,8 +13,6 @@
 #define __DXCAPI_USE_H__
 
 #include "dxc/dxcapi.h"
-#include <dxc/Support/Global.h> // for hresult handling with DXC_FAILED
-#include <filesystem>           // C++17 and later
 
 namespace dxc {
 
@@ -28,7 +26,7 @@ class DxcDllSupport {
   DxcCreateInstanceProc m_createFn;
   DxcCreateInstance2Proc m_createFn2;
 
-  HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName) {
+  virtual HRESULT InitializeInternal(LPCSTR dllName, LPCSTR fnName) {
     if (m_dll != nullptr)
       return S_OK;
 
@@ -89,11 +87,11 @@ class DxcDllSupport {
 
   virtual ~DxcDllSupport() { Cleanup(); }
 
-  HRESULT virtual Initialize() {
+  HRESULT Initialize() {
     return InitializeInternal(kDxCompilerLib, "DxcCreateInstance");
   }
 
-  HRESULT virtual InitializeForDll(LPCSTR dll, LPCSTR entryPoint) {
+  HRESULT InitializeForDll(LPCSTR dll, LPCSTR entryPoint) {
     return InitializeInternal(dll, entryPoint);
   }
 
@@ -102,7 +100,7 @@ class DxcDllSupport {
     return CreateInstance(clsid, __uuidof(TInterface), (IUnknown **)pResult);
   }
 
-  HRESULT virtual CreateInstance(REFCLSID clsid, REFIID riid,
+  virtual HRESULT CreateInstance(REFCLSID clsid, REFIID riid,
                                  IUnknown **pResult) {
     if (pResult == nullptr)
       return E_POINTER;
@@ -119,7 +117,7 @@ class DxcDllSupport {
                            (IUnknown **)pResult);
   }
 
-  HRESULT virtual CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
+  virtual HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
                                   IUnknown **pResult) {
     if (pResult == nullptr)
       return E_POINTER;
@@ -144,7 +142,7 @@ class DxcDllSupport {
     return true;
   }
 
-  void virtual Cleanup() {
+  virtual void Cleanup() {
     if (m_dll != nullptr) {
       m_createFn = nullptr;
       m_createFn2 = nullptr;
@@ -157,7 +155,7 @@ class DxcDllSupport {
     }
   }
 
-  HMODULE virtual Detach() {
+  virtual HMODULE Detach() {
     HMODULE hModule = m_dll;
     m_dll = nullptr;
     return hModule;

lib/DxcSupport/dxcapi.extval.cpp

@@ -1,11 +1,14 @@
 #include "dxc/Support/WinIncludes.h"
+#include <dxc/Support/Global.h> // for hresult handling with DXC_FAILED
+#include <filesystem>           // C++17 and later
 // WinIncludes must come before dxcapi.extval.h
 #include "dxc/Support/dxcapi.extval.h"
 
+
 HRESULT DxcDllExtValidationSupport::InitializeInternal(LPCSTR dllName,
                                                        LPCSTR fnName) {
   // Load dxcompiler.dll
-  HRESULT result = InitializeForDll(dllName, fnName);
+  HRESULT result = DxcDllSupport::InitializeInternal(dllName, fnName);
   // if dxcompiler.dll fails to load, return the failed HRESULT
   if (DXC_FAILED(result)) {
     return result;

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -32,7 +32,8 @@
 #include "dxc/Support/Global.h"
 
 #include "dxc/DXIL/DxilShaderModel.h"
-#include "dxc/Test/DxcTestUtils.h"
+#include "dxc/Test/DxcTestUtils.h" // includes dxcapi.use.h
+#include "dxc/Support/dxcapi.extval.h"
 #include "dxc/Test/HlslTestUtils.h"
 
 using namespace std;
@@ -299,6 +300,7 @@ class ValidationTest : public ::testing::Test {
   TEST_METHOD(ValidatePrintfNotAllowed)
 
   TEST_METHOD(ValidateWithHash)
+  TEST_METHOD(ValidateWithExternalValidator)
   TEST_METHOD(ValidateVersionNotAllowed)
   TEST_METHOD(ValidatePreviewBypassHash)
   TEST_METHOD(ValidateProgramVersionAgainstDxilModule)
@@ -328,6 +330,7 @@ class ValidationTest : public ::testing::Test {
   TEST_METHOD(WrongPSVVersion)
 
   dxc::DxcDllSupport m_dllSupport;
+  DxcDllExtValidationSupport m_dllExtSupport;
   VersionSupportInfo m_ver;
 
   void TestCheck(LPCWSTR name) {
@@ -4207,6 +4210,33 @@ TEST_F(ValidationTest, ValidateWithHash) {
   VERIFY_ARE_EQUAL(memcmp(Result, pHeader->Hash.Digest, sizeof(Result)), 0);
 }
 
+TEST_F(ValidationTest, ValidateWithExternalValidator) {
+  if (!m_dllExtSupport.IsEnabled()) {
+    VERIFY_SUCCEEDED(m_dllExtSupport.Initialize());
+  }
+
+  if (m_ver.SkipDxilVersion(1, ShaderModel::kHighestReleasedMinor))
+    return;
+  CComPtr<IDxcBlob> pProgram;
+  CompileSource("float4 main(float a:A, float b:B) : SV_Target { return 1; }",
+                "ps_6_0", &pProgram);
+
+  CComPtr<IDxcValidator> pValidator;
+  CComPtr<IDxcOperationResult> pResult;
+  unsigned Flags = 0;
+  VERIFY_SUCCEEDED(
+      m_dllExtSupport.CreateInstance(CLSID_DxcValidator, &pValidator));
+  // With hash.
+  VERIFY_SUCCEEDED(pValidator->Validate(pProgram, Flags, &pResult));
+  // Make sure the validation was successful.
+  HRESULT status;
+  VERIFY_IS_NOT_NULL(pResult);
+  CComPtr<IDxcBlob> pValidationOutput;
+  pResult->GetStatus(&status);
+  VERIFY_SUCCEEDED(status);
+  pResult->GetResult(&pValidationOutput);  
+}
+
 TEST_F(ValidationTest, ValidatePreviewBypassHash) {
   if (m_ver.SkipDxilVersion(1, ShaderModel::kHighestMinor))
     return;