IDxcRewriter2 interface; change RewriterTest to report errors (#1529)

* IDxcRewriter2 interface; change RewriterTest to report errors

Added IDxcRewriter2 interface with method RewriteWithOptions that allows
setting compiler flags for the rewrite. The current IDxcRewrite methods
use HLSL Version 2015 only. RewriteWithOptions allows changing that
and other compiler options.

Modified RewriterTest to report compilation errors. Changed data of existing
tests that started failing because of this change.

Fixed DeadLoopFail test.

include/dxc/dxctools.h

@@ -64,4 +64,21 @@ EXTERN const CLSID CLSID_DxcRewriter = { /* b489b951-e07f-40b3-968d-93e124734da4
   { 0x96, 0x8d, 0x93, 0xe1, 0x24, 0x73, 0x4d, 0xa4 }
 };
 
+struct __declspec(uuid("261afca1-0609-4ec6-a77f-d98c7035194e"))
+IDxcRewriter2 : public IDxcRewriter {
+
+  virtual HRESULT STDMETHODCALLTYPE RewriteWithOptions(_In_ IDxcBlobEncoding *pSource,
+                                                     // Optional file name for pSource. Used in errors and include handlers.
+                                                     _In_opt_ LPCWSTR pSourceName, 
+                                                     // Compiler arguments
+                                                     _In_count_(argCount) LPCWSTR *pArguments, _In_ UINT32 argCount, 
+                                                     // Defines
+                                                     _In_count_(defineCount) DxcDefine *pDefines, _In_ UINT32 defineCount,
+                                                     // user-provided interface to handle #include directives (optional)
+                                                     _In_opt_ IDxcIncludeHandler *pIncludeHandler,
+                                                     _COM_Outptr_ IDxcOperationResult **ppResult) = 0;
+
+  DECLARE_CROSS_PLATFORM_UUIDOF(IDxcRewriter)
+};
+
 #endif

tools/clang/test/HLSL/rewriter/array-length-rw.hlsl

@@ -1,7 +1,7 @@
 float4 planes[8];
 
 [RootSignature("CBV(b0, space=0, visibility=SHADER_VISIBILITY_ALL)")]
-float main(POSITION) : SV_Target {
+float main() {
     float4 x = float4(1.0, 1.0, 1.0, 1.0);
     for (uint i = 0; i < planes.Length; ++i) {
         x = x + planes[i];

tools/clang/test/HLSL/rewriter/correct_rewrites/array-length-rw_gold.hlsl

@@ -1,7 +1,7 @@
 // Rewrite unchanged result:
 const float4 planes[8];
 [RootSignature(CBV(b0, space=0, visibility=SHADER_VISIBILITY_ALL))]
-float main(int) {
+float main() {
   float4 x = float4(1., 1., 1., 1.);
   for (uint i = 0; i < planes.Length; ++i) {
     x = x + planes[i];

tools/clang/test/HLSL/rewriter/correct_rewrites/cpp-errors_gold.hlsl

@@ -71,7 +71,6 @@ struct my_struct_4 : my_interface {
 };
 struct my_struct_5 : my_class, my_interface {
 };
-struct forward_struct;
 struct my_struct_type_decl {
   int a;
 };

tools/clang/test/HLSL/rewriter/correct_rewrites/effects-syntax_gold.hlsl

@@ -6,14 +6,12 @@ SamplerState samLinear : register(s7);
 sampler S : register(s1);
 SamplerComparisonState SC : register(s3);
 float4 main() : SV_Target {
-  int SB;
   {
     int StateBlock = 1;
   }
   int PixelShadeR = 1;
   Texture2D l_tex;
   int foobar;
-  int foobar2;
   int RenderTargetView = 1;
   int PixelShader = 1;
   int pixelshader = 1;
@@ -22,7 +20,5 @@ float4 main() : SV_Target {
 }
 
 
-const int TechNiQue;
 const int foobar3;
-const int T5;
 const int foobar4;

tools/clang/test/HLSL/rewriter/correct_rewrites/packreg_gold.hlsl

@@ -15,7 +15,6 @@ sampler myVar_2 : register(vs, s8);
 sampler myVar65536 : register(vs, s65536);
 sampler myVar5 : register(vs, s0);
 AppendStructuredBuffer<float4> myVar11 : register(ps, u1);
-Buffer<float4> myVar11_plain : register(ps, u2);
 RWStructuredBuffer<float4> myVar11_rw : register(ps, u0);
 sampler myVar_s : register(ps, s0);
 Texture2D myVar_t : register(ps, t0);

tools/clang/test/HLSL/rewriter/cpp-errors_noerr.hlsl

@@ -328,7 +328,7 @@ struct my_struct_5 : my_class, my_interface { };
 //
 //class my_class_public : public my_class { }; // expected-error {{base type access specifier is unsupported in HLSL}}
 
-struct forward_struct; // this fails in fxc, but we allow it now
+//struct forward_struct; // this fails in fxc, but we allow it now for HLSL version >= 2016
 struct my_struct_type_decl { int a; } my_struct_var_decl;
 //struct my_struct_type_decl_parens { int a; } (my_struct_var_decl_parens); // expected-error {{expected ';' after struct}} expected-error {{HLSL requires a type specifier for all declarations}}
 //struct my_struct_type_const { int a; } const my_struct_type_var; // // expected-error {{expected ';' after struct}} expected-error {{HLSL requires a type specifier for all declarations}}

tools/clang/test/HLSL/rewriter/effects-syntax.hlsl → tools/clang/test/HLSL/rewriter/effects-syntax_noerr.hlsl

@@ -48,7 +48,7 @@ SamplerComparisonState SC : register(s3) = sampler_state {texture=tex;};    /* e
 
 float4 main() : SV_Target
 {
-  StateBlock SB;                                            /* expected-error {{unknown type name 'StateBlock'}} fxc-error {{X3000: unrecognized identifier 'SB'}} fxc-error {{X3000: unrecognized identifier 'StateBlock'}} */
+  //StateBlock SB;                                            /* expected-error {{unknown type name 'StateBlock'}} fxc-error {{X3000: unrecognized identifier 'SB'}} fxc-error {{X3000: unrecognized identifier 'StateBlock'}} */
 
   {
     int StateBlock = 1;
@@ -70,7 +70,7 @@ float4 main() : SV_Target
   // This is allowed (deprecated effect state block warning):
   int foobar {blah=foo;};                                   /* expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-pass {{}} */
   // This is not:
-  int foobar2 {blah=foo;} = 5;                              /* expected-error {{expected ';' at end of declaration}} expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-error {{X3000: syntax error: unexpected token '='}} */
+  //int foobar2 {blah=foo;} = 5;                              /* expected-error {{expected ';' at end of declaration}} expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-error {{X3000: syntax error: unexpected token '='}} */
 
   // But this isn't:
   int RenderTargetView = 1;                                 /* fxc-error {{X3000: syntax error: unexpected token 'RenderTargetView'}} */
@@ -107,12 +107,12 @@ static const PixelShader ps1 { state=foo; };                /* expected-warning
   VarDecl <col:1, col:26> col:26 invalid ps1 'const PixelShader':'const deprecated effect object' static
 */
 // expected-note@? {{'PixelShader' declared here}}
-PixelShadeR ps < int foo=1;>  = ps1;   // Case insensitive! /* expected-error {{unknown type name 'PixelShadeR'; did you mean 'PixelShader'?}} expected-warning {{effect object ignored - effect syntax is deprecated}} expected-warning {{possible effect annotation ignored - effect syntax is deprecated}} expected-error {{use of undeclared identifier 'ps1'}}fxc-pass {{}} */
+//PixelShadeR ps < int foo=1;>  = ps1;   // Case insensitive! /* expected-error {{unknown type name 'PixelShadeR'; did you mean 'PixelShader'?}} expected-warning {{effect object ignored - effect syntax is deprecated}} expected-warning {{possible effect annotation ignored - effect syntax is deprecated}} expected-error {{use of undeclared identifier 'ps1'}}fxc-pass {{}} */
 /*verify-ast
   VarDecl <col:1, col:13> col:13 invalid ps 'PixelShader':'deprecated effect object'
 */
 // expected-note@? {{'VertexShader' declared here}}
-VertexShadeR vs;        // Case insensitive!                /* expected-error {{unknown type name 'VertexShadeR'; did you mean 'VertexShader'?}} expected-warning {{effect object ignored - effect syntax is deprecated}} fxc-pass {{}} */
+//VertexShadeR vs;        // Case insensitive!                /* expected-error {{unknown type name 'VertexShadeR'; did you mean 'VertexShader'?}} expected-warning {{effect object ignored - effect syntax is deprecated}} fxc-pass {{}} */
 
 // Case sensitive
 pixelfragment pfrag;                                        /* expected-warning {{effect object ignored - effect syntax is deprecated}} fxc-pass {{}} */
@@ -155,24 +155,24 @@ technique10                                                 /* expected-warning
 }
 
 // We don't bother handling weird casing, so this will be a syntax error:
-TechNiQue                                                   /* expected-error {{HLSL requires a type specifier for all declarations}} fxc-pass {{}} */
+//TechNiQue                                                   /* expected-error {{HLSL requires a type specifier for all declarations}} fxc-pass {{}} */
 /*verify-ast
   VarDecl <col:1> col:1 invalid TechNiQue 'int'
 */
-{                                                           /* expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-pass {{}} */
-  pass {}
-}                                                           /* expected-error {{expected ';' after top level declarator}} fxc-pass {{}} */
+//{                                                           /* expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-pass {{}} */
+//  pass {}
+//}                                                           /* expected-error {{expected ';' after top level declarator}} fxc-pass {{}} */
 int foobar3;
 /*verify-ast
   VarDecl <col:1, col:5> col:5 foobar3 'int'
 */
-TechNique T5                                                /* expected-error {{unknown type name 'TechNique'}} fxc-pass {{}} */
+//TechNique T5                                                /* expected-error {{unknown type name 'TechNique'}} fxc-pass {{}} */
 /*verify-ast
   VarDecl <col:1, col:11> col:11 invalid T5 'int'
 */
-{                                                           /* expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-pass {{}} */
-  pass {}
-}                                                           /* expected-error {{expected ';' after top level declarator}} fxc-pass {{}} */
+//{                                                           /* expected-warning {{effect state block ignored - effect syntax is deprecated}} fxc-pass {{}} */
+//  pass {}
+//}                                                           /* expected-error {{expected ';' after top level declarator}} fxc-pass {{}} */
 int foobar4;
 /*verify-ast
   VarDecl <col:1, col:5> col:5 foobar4 'int'

tools/clang/test/HLSL/rewriter/packreg_noerr.hlsl

@@ -85,7 +85,7 @@ sampler myVar5 : register(vs, s);
 // fxc error X3000: error X3530: register a not valid
 //sampler myVar10 : register(ps, a); // expected-error {{register type is unsupported - valid values are 'b', 'c', 'i', 's', 't'}} fxc-pass {{}} 
 AppendStructuredBuffer<float4> myVar11 : register(ps, u1);
-Buffer<float4> myVar11_plain : register(ps, u2);
+//Buffer<float4> myVar11_plain : register(ps, u2); 
 RWStructuredBuffer<float4> myVar11_rw : register(ps, u);
 // fxc error X3000: syntax error: unexpected integer constant
 //sampler myVar12 : register(ps, 3); // expected-error {{expected identifier}} fxc-pass {{}} 

tools/clang/tools/dxcompiler/dxcapi.cpp

@@ -38,6 +38,7 @@ DEFINE_CROSS_PLATFORM_UUIDOF(IDxcContainerBuilder)
 DEFINE_CROSS_PLATFORM_UUIDOF(IDxcOptimizerPass)
 DEFINE_CROSS_PLATFORM_UUIDOF(IDxcOptimizer)
 DEFINE_CROSS_PLATFORM_UUIDOF(IDxcRewriter)
+DEFINE_CROSS_PLATFORM_UUIDOF(IDxcRewriter2)
 DEFINE_CROSS_PLATFORM_UUIDOF(IDxcIntelliSense)
 DEFINE_CROSS_PLATFORM_UUIDOF(IDxcLinker)
 

tools/clang/tools/libclang/dxcrewriteunused.cpp

@@ -41,6 +41,7 @@
 #include "dxc/Support/dxcapi.impl.h"
 #include "dxc/Support/DxcLangExtensionsHelper.h"
 #include "dxc/Support/dxcfilesystem.h"
+#include "dxc/Support/HLSLOptions.h"
 
 #define CP_UTF16 1200
 
@@ -109,6 +110,7 @@ void SetupCompilerForRewrite(CompilerInstance &compiler,
                              _In_ LPCSTR pMainFile,
                              _In_ TextDiagnosticPrinter *diagPrinter,
                              _In_opt_ ASTUnit::RemappedFile *rewrite,
+                             _In_ hlsl::options::DxcOpts &opts,
                              _In_opt_ LPCSTR pDefines) {
   // Setup a compiler instance.
   std::shared_ptr<TargetOptions> targetOptions(new TargetOptions);
@@ -121,6 +123,13 @@ void SetupCompilerForRewrite(CompilerInstance &compiler,
   // Not use builtin includes.
   compiler.getHeaderSearchOpts().UseBuiltinIncludes = false;
 
+  // apply compiler options applicable for rewrite
+  if (opts.WarningAsError)
+    compiler.getDiagnostics().setWarningsAsErrors(true);
+  compiler.getDiagnostics().setIgnoreAllWarnings(!opts.OutputWarnings);
+  compiler.getLangOpts().HLSLVersion = (unsigned)opts.HLSLVersion;
+  compiler.getLangOpts().UseMinPrecision = !opts.Enable16BitTypes;
+
   PreprocessorOptions &PPOpts = compiler.getPreprocessorOpts();
   if (rewrite != nullptr) {
     if (llvm::MemoryBuffer *pMemBuf = rewrite->second) {
@@ -321,6 +330,35 @@ void WriteUserMacroDefines(CompilerInstance &compiler, raw_string_ostream &o) {
   WriteMacroDefines(macros, o);
 }
 
+static
+HRESULT ReadOptsAndValidate(LPCWSTR *pArguments, _In_ UINT32 argCount,
+                            hlsl::options::DxcOpts &opts,
+                            _COM_Outptr_ IDxcOperationResult **ppResult) {
+  hlsl::options::MainArgs mainArgs(argCount, pArguments, 0);
+  const llvm::opt::OptTable *table = ::options::getHlslOptTable();
+
+  CComPtr<AbstractMemoryStream> pOutputStream;
+  IFT(CreateMemoryStream(GetGlobalHeapMalloc(), &pOutputStream));
+  raw_stream_ostream outStream(pOutputStream);
+
+  if (0 != hlsl::options::ReadDxcOpts(table, hlsl::options::CompilerFlags,
+                                      mainArgs, opts, outStream)) {
+    CComPtr<IDxcBlob> pErrorBlob;
+    IFT(pOutputStream->QueryInterface(&pErrorBlob));
+    CComPtr<IDxcBlobEncoding> pErrorBlobWithEncoding;
+    outStream.flush();
+    IFT(DxcCreateBlobWithEncodingSet(pErrorBlob.p, CP_UTF8,
+                                     &pErrorBlobWithEncoding));
+    IFT(DxcOperationResult::CreateFromResultErrorStatus(
+        nullptr, pErrorBlobWithEncoding.p, E_INVALIDARG, ppResult));
+    return E_INVALIDARG;
+  }
+  DXASSERT(opts.HLSLVersion > 2015,
+           "else ReadDxcOpts didn't fail for non-isense");
+  return S_OK;
+}
+
+
 static
 HRESULT DoRewriteUnused(_In_ DxcLangExtensionsHelper *pHelper,
                      _In_ LPCSTR pFileName,
@@ -340,7 +378,11 @@ HRESULT DoRewriteUnused(_In_ DxcLangExtensionsHelper *pHelper,
   CompilerInstance compiler;
   std::unique_ptr<TextDiagnosticPrinter> diagPrinter =
       llvm::make_unique<TextDiagnosticPrinter>(w, &compiler.getDiagnosticOpts());  
-  SetupCompilerForRewrite(compiler, pHelper, pFileName, diagPrinter.get(), pRemap, pDefines);
+
+  hlsl::options::DxcOpts opts;
+  opts.HLSLVersion = 2015;
+
+  SetupCompilerForRewrite(compiler, pHelper, pFileName, diagPrinter.get(), pRemap, opts, pDefines);
 
   // Parse the source file.
   compiler.getDiagnosticClient().BeginSourceFile(compiler.getLangOpts(), &compiler.getPreprocessor());
@@ -481,6 +523,7 @@ static
 HRESULT DoSimpleReWrite(_In_ DxcLangExtensionsHelper *pHelper,
                _In_ LPCSTR pFileName,
                _In_ ASTUnit::RemappedFile *pRemap,
+               _In_ hlsl::options::DxcOpts &opts,
                _In_ LPCSTR pDefines,
                _In_ UINT32 rewriteOption,
                _Outptr_result_z_ LPSTR *pWarnings,
@@ -501,7 +544,7 @@ HRESULT DoSimpleReWrite(_In_ DxcLangExtensionsHelper *pHelper,
   CompilerInstance compiler;
   std::unique_ptr<TextDiagnosticPrinter> diagPrinter =
       llvm::make_unique<TextDiagnosticPrinter>(w, &compiler.getDiagnosticOpts());    
-  SetupCompilerForRewrite(compiler, pHelper, pFileName, diagPrinter.get(), pRemap, pDefines);
+  SetupCompilerForRewrite(compiler, pHelper, pFileName, diagPrinter.get(), pRemap, opts, pDefines);
 
   // Parse the source file.
   compiler.getDiagnosticClient().BeginSourceFile(compiler.getLangOpts(), &compiler.getPreprocessor());
@@ -538,7 +581,7 @@ HRESULT DoSimpleReWrite(_In_ DxcLangExtensionsHelper *pHelper,
   return S_OK;
 }
 
-class DxcRewriter : public IDxcRewriter, public IDxcLangExtensions {
+class DxcRewriter : public IDxcRewriter2, public IDxcLangExtensions {
 private:
   DXC_MICROCOM_TM_REF_FIELDS()
   DxcLangExtensionsHelper m_langExtensionsHelper;
@@ -548,7 +591,7 @@ public:
   DXC_LANGEXTENSIONS_HELPER_IMPL(m_langExtensionsHelper)
 
   HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override {
-    return DoBasicQueryInterface<IDxcRewriter, IDxcLangExtensions>(this, iid, ppvObject);
+    return DoBasicQueryInterface<IDxcRewriter2, IDxcRewriter, IDxcLangExtensions>(this, iid, ppvObject);
   }
 
   HRESULT STDMETHODCALLTYPE RemoveUnusedGlobals(_In_ IDxcBlobEncoding *pSource,
@@ -625,10 +668,13 @@ public:
 
       std::string definesStr = DefinesToString(pDefines, defineCount);
 
+      hlsl::options::DxcOpts opts;
+      opts.HLSLVersion = 2015;
+
       LPSTR errors = nullptr;
       LPSTR rewrite = nullptr;
       HRESULT status =
-          DoSimpleReWrite(&m_langExtensionsHelper, fakeName, pRemap.get(),
+          DoSimpleReWrite(&m_langExtensionsHelper, fakeName, pRemap.get(), opts,
                           defineCount > 0 ? definesStr.c_str() : nullptr,
                           RewriterOptionMask::Default, &errors, &rewrite);
 
@@ -673,10 +719,13 @@ public:
 
       std::string definesStr = DefinesToString(pDefines, defineCount);
 
+      hlsl::options::DxcOpts opts;
+      opts.HLSLVersion = 2015;
+
       LPSTR errors = nullptr;
       LPSTR rewrite = nullptr;
       HRESULT status =
-          DoSimpleReWrite(&m_langExtensionsHelper, fName, pRemap.get(),
+          DoSimpleReWrite(&m_langExtensionsHelper, fName, pRemap.get(), opts,
                           defineCount > 0 ? definesStr.c_str() : nullptr,
                           rewriteOption, &errors, &rewrite);
 
@@ -687,6 +736,65 @@ public:
 
   }
 
+    HRESULT STDMETHODCALLTYPE RewriteWithOptions(
+        _In_ IDxcBlobEncoding *pSource,
+        // Optional file name for pSource. Used in errors and include handlers.
+        _In_opt_ LPCWSTR pSourceName, 
+        // Compiler arguments
+        _In_count_(argCount) LPCWSTR *pArguments, _In_ UINT32 argCount, 
+        // Defines
+        _In_count_(defineCount) DxcDefine *pDefines, _In_ UINT32 defineCount,
+        // user-provided interface to handle #include directives (optional)
+        _In_opt_ IDxcIncludeHandler *pIncludeHandler,
+        _COM_Outptr_ IDxcOperationResult **ppResult) override {
+
+    if (pSource == nullptr || ppResult == nullptr ||
+        (argCount > 0 && pArguments == nullptr) ||
+        (defineCount > 0 && pDefines == nullptr))
+      return E_POINTER;
+
+    *ppResult = nullptr;
+
+    DxcThreadMalloc TM(m_pMalloc);
+
+    CComPtr<IDxcBlobEncoding> utf8Source;
+    IFR(hlsl::DxcGetBlobAsUtf8(pSource, &utf8Source));
+
+    CW2A utf8SourceName(pSourceName, CP_UTF8);
+    LPCSTR fName = utf8SourceName.m_psz;
+
+    try {
+      dxcutil::DxcArgsFileSystem *msfPtr = dxcutil::CreateDxcArgsFileSystem(
+          utf8Source, pSourceName, pIncludeHandler);
+      std::unique_ptr<::llvm::sys::fs::MSFileSystem> msf(msfPtr);
+      ::llvm::sys::fs::AutoPerThreadSystem pts(msf.get());
+      IFTLLVM(pts.error_code());
+
+      StringRef Data((LPCSTR)utf8Source->GetBufferPointer(),
+                     utf8Source->GetBufferSize());
+      std::unique_ptr<llvm::MemoryBuffer> pBuffer(
+          llvm::MemoryBuffer::getMemBufferCopy(Data, fName));
+      std::unique_ptr<ASTUnit::RemappedFile> pRemap(
+          new ASTUnit::RemappedFile(fName, pBuffer.release()));
+
+      std::string definesStr = DefinesToString(pDefines, defineCount);
+
+      hlsl::options::DxcOpts opts;
+      IFR(ReadOptsAndValidate(pArguments, argCount, opts, ppResult));
+
+      LPSTR errors = nullptr;
+      LPSTR rewrite = nullptr;
+      HRESULT status =
+          DoSimpleReWrite(&m_langExtensionsHelper, fName, pRemap.get(), opts,
+                          defineCount > 0 ? definesStr.c_str() : nullptr,
+                          Default, &errors, &rewrite);
+
+      return DxcOperationResult::CreateFromUtf8Strings(errors, rewrite, status,
+                                                       ppResult);
+    }
+    CATCH_CPP_RETURN_HRESULT();
+  }
+
   std::string DefinesToString(_In_count_(defineCount) DxcDefine *pDefines, _In_ UINT32 defineCount) {
     std::string defineStr;
     for (UINT32 i = 0; i < defineCount; i++) {

tools/clang/unittests/HLSL/RewriterTest.cpp

@@ -52,8 +52,7 @@ public:
     TEST_METHOD_PROPERTY(L"Priority", L"0")
   END_TEST_CLASS()
 
-  //TODO: Enable this tests with -HV 2016 once we have a way to change HLSL version
-  //TEST_METHOD(RunArrayLength);
+  TEST_METHOD(RunArrayLength);
   TEST_METHOD(RunAttributes);
   TEST_METHOD(RunAnonymousStruct);
   TEST_METHOD(RunCppErrors);
@@ -86,6 +85,7 @@ public:
   TEST_METHOD(RunNoFunctionBodyInclude);
   TEST_METHOD(RunNoStatic);
   TEST_METHOD(RunKeepUserMacro);
+  TEST_METHOD(RunRewriterFails)
 
   dxc::DxcDllSupport m_dllSupport;
   CComPtr<IDxcIncludeHandler> m_pIncludeHandler;
@@ -227,6 +227,20 @@ public:
     VERIFY_SUCCEEDED(rewriter->RewriteUnchanged(source.BlobEncoding, myDefines,
                                                 myDefinesCount, ppResult));
 
+    // check for compilation errors
+    HRESULT hrStatus;
+    VERIFY_SUCCEEDED((*ppResult)->GetStatus(&hrStatus));
+
+    if (!(SUCCEEDED(hrStatus))) {
+        ::WEX::Logging::Log::Error(L"\nCompilation failed.\n");
+        CComPtr<IDxcBlobEncoding> pErrorBuffer;
+        IFT((*ppResult)->GetErrorBuffer(&pErrorBuffer));
+        std::wstring errorStr = BlobToUtf16(pErrorBuffer);
+        ::WEX::Logging::Log::Error(errorStr.data());
+        VERIFY_SUCCEEDED(hrStatus);
+        return;
+    }
+
     CComPtr<IDxcBlob> pRewriteResult;
     IFT((*ppResult)->GetResult(&pRewriteResult));
     std::string firstPass = BlobToUtf8(pRewriteResult);
@@ -290,10 +304,9 @@ public:
   }
 };
 
-//TODO: Enable this tests with -HV 2016 once we have a way to change HLSL version
-// TEST_F(RewriterTest, RunArrayLength) {
-//  CheckVerifiesHLSL(L"rewriter\\array-length-rw.hlsl", L"rewriter\\correct_rewrites\\array-length-rw_gold.hlsl");
-//}
+TEST_F(RewriterTest, RunArrayLength) {
+  CheckVerifiesHLSL(L"rewriter\\array-length-rw.hlsl", L"rewriter\\correct_rewrites\\array-length-rw_gold.hlsl");
+}
 
 TEST_F(RewriterTest, RunAttributes) {
     CheckVerifiesHLSL(L"rewriter\\attributes_noerr.hlsl", L"rewriter\\correct_rewrites\\attributes_gold.hlsl");
@@ -478,7 +491,7 @@ TEST_F(RewriterTest, RunNonUnicode) {
 }
 
 TEST_F(RewriterTest, RunEffect) {
-  CheckVerifiesHLSL(L"rewriter\\effects-syntax.hlsl", L"rewriter\\correct_rewrites\\effects-syntax_gold.hlsl");
+  CheckVerifiesHLSL(L"rewriter\\effects-syntax_noerr.hlsl", L"rewriter\\correct_rewrites\\effects-syntax_gold.hlsl");
 }
 
 TEST_F(RewriterTest, RunSemanticDefines) {
@@ -620,3 +633,36 @@ float test(float a, float b) {\n\
 #define Y(A, B)  ( ( A ) + ( B ) )\n\
 ") == 0);
 }
+
+TEST_F(RewriterTest, RunRewriterFails) {
+  CComPtr<IDxcRewriter> pRewriter;
+  CComPtr<IDxcRewriter2> pRewriter2;
+  VERIFY_SUCCEEDED(CreateRewriter(&pRewriter));
+  VERIFY_SUCCEEDED(pRewriter->QueryInterface(&pRewriter2));
+
+  // Get the source text from a file
+  std::wstring sourceName = GetPathToHlslDataFile(L"rewriter\\array-length-rw.hlsl");
+  FileWithBlob source(m_dllSupport, sourceName.c_str());
+
+  // Compilation should fail with these options
+  CComPtr<IDxcOperationResult> pRewriteResult;
+  LPCWSTR compileOptions[] = {L"-HV", L"2018"};
+  
+  // Run rewrite on the source code
+  VERIFY_SUCCEEDED(pRewriter2->RewriteWithOptions(source.BlobEncoding, sourceName.c_str(), compileOptions, 2, 
+                                                  nullptr, 0, nullptr, &pRewriteResult));
+
+  // Verify it failed
+  HRESULT hrStatus;
+  VERIFY_SUCCEEDED(pRewriteResult->GetStatus(&hrStatus));
+  VERIFY_FAILED(hrStatus);
+
+  ::WEX::Logging::Log::Comment(L"\nCompilation failed as expected.\n");
+  CComPtr<IDxcBlobEncoding> pErrorBuffer;
+  IFT(pRewriteResult->GetErrorBuffer(&pErrorBuffer));
+  std::wstring errorStr = BlobToUtf16(pErrorBuffer);
+  
+  ::WEX::Logging::Log::Comment(errorStr.data());
+
+  VERIFY_IS_TRUE(errorStr.find(L"Length is only allowed for HLSL 2016 and lower.") >= 0);
+}

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -682,9 +682,9 @@ TEST_F(ValidationTest, DeadLoopFail) {
   RewriteAssemblyCheckMsg(
       L"..\\CodeGenHLSL\\loop1.hlsl", "ps_6_0",
       {"br i1 %exitcond, label %for.end.loopexit, label %for.body, !llvm.loop !([0-9]+)",
-       "%add.lcssa = phi float \\[ %add, %for.body \\]",
+       "?%add(\\.lcssa)? = phi float \\[ %add, %for.body \\]",
        "!dx.entryPoints = !\\{!([0-9]+)\\}",
-       "\\[ %add.lcssa, %for.end.loopexit \\]"
+       "\\[ %add(\\.lcssa)?, %for.end.loopexit \\]"
       },
       {"br label %for.body",
        "",