beanz added inline comments.

================
Comment at: clang/test/SemaHLSL/pch.hlsl:5
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl \
+// RUN:  -finclude-default-header -include-pch %t -ast-dump-all  /dev/null \
+// RUN: | FileCheck  %s
----------------
I want to make sure I'm following this test.

You compile this file to a PCH (%t), then compile /dev/null using this %t as 
the PCH.

IIUC, that doesn't actually verify that PCH and the HLSL sema work together. 
There is no test here that pulls from the PCH and HLSL Sema in the same 
translation unit.

My suggestion would be to:
(1) move this test under AST/HLSL (since it is an AST test, not a Sema test)
(2) put a header under AST/HLSL/Inputs that (like this file) contains a 
function that uses HLSL types
(3) make the test file separate from the header, and have it use both the 
function in the header and some completely different HLSL-provided type (like 
declare a RWBuffer).
(4) your test can then generate the PCH, and use the PCH to compile the source 
verifying that both AST sources work


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132421/new/

https://reviews.llvm.org/D132421

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to