beanz added inline comments.
================ Comment at: clang/include/clang/Parse/Parser.h:2820 SourceLocation *EndLoc = nullptr); + Decl *ParseCTBuffer(SourceLocation &DeclEnd, + SourceLocation InlineLoc = SourceLocation()); ---------------- nit: maybe `ParseHLSLBuffer`, I don't think `ParseCTBuffer` makes it apparent what this is doing, also everything else is `HLSLBuffer` ================ Comment at: clang/test/SemaHLSL/cbuffer_tbuffer.hlsl:1 +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s + ---------------- This looks like it should be an AST test, not SEMA. We should also have parser tests for malformed parse cases: ``` cbuffer { ... }; cbuffer missing_definition; int cbuffer; // using a keyword as a variable name cbuffer; // lots wrong here... cbuffer missing_semicolon { int woo; } ``` It looks like this patch doesn't handle making the buffer variables constant. Having not looked at that I don't know how big of a change that is, so it might be okay as a subsequent patch, but we'll need that support to handle cases like: ``` cbuffer cb{ int x; }; [numthreads(1,1,1)] void main(int GI : SV_GROUPINDEX) { x = GI; // error: buffer content is const } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129883/new/ https://reviews.llvm.org/D129883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits