[clang] [HLSL] Add implicit resource element type concepts to AST (PR #116413)

Chris B via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 14:17:34 PST 2024


https://github.com/llvm-beanz commented:

I think the way that you're staging these changes is awkward and results in more review time than we strictly need. We're also going to need to re-review a bunch of this code in a wider context in the near future, so I'm not sure I'm comfortable with this approach.

In a future change, you're going to introduce a new builtin (`__builtin_hlsl_is_typed_resource_element_compatible`), and a bunch of the code here will go away, and a bunch of the tests will be extended and updated to use that builtin. At that time we'll need to re-review this work.

Why not introduce the new builtin first so that this code can be merged in its completed state rather than in an intermediate state?

https://github.com/llvm/llvm-project/pull/116413


More information about the cfe-commits mailing list