Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HLSL] Add helpers to simplify HLSL resource type declarations. NFC #73967

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Nov 30, 2023

A few changes to HLSLExternalSemaSource and its BuiltinTypeDeclBuilder
to make defining buffer types less verbose. This will make it a lot
easier to see what the differences between the various buffer types
are once we start introducing more of them.

Created using spr 1.3.5
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Nov 30, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 30, 2023

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Justin Bogner (bogner)

Changes

A few changes to HLSLExternalSemaSource and its BuiltinTypeDeclBuilder
to make defining buffer types less verbose. This will make it a lot
easier to see what the differences between the various buffer types
are once we start introducing more of them.


Full diff: https://github.com/llvm/llvm-project/pull/73967.diff

2 Files Affected:

  • (modified) clang/include/clang/Sema/HLSLExternalSemaSource.h (+2-2)
  • (modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+34-20)
diff --git a/clang/include/clang/Sema/HLSLExternalSemaSource.h b/clang/include/clang/Sema/HLSLExternalSemaSource.h
index 4b6bc96f72e225c..c0bfff327139f8c 100644
--- a/clang/include/clang/Sema/HLSLExternalSemaSource.h
+++ b/clang/include/clang/Sema/HLSLExternalSemaSource.h
@@ -30,9 +30,9 @@ class HLSLExternalSemaSource : public ExternalSemaSource {
 
   void defineHLSLVectorAlias();
   void defineTrivialHLSLTypes();
-  void forwardDeclareHLSLTypes();
+  void defineHLSLTypesWithForwardDeclarations();
 
-  void completeBufferType(CXXRecordDecl *Record);
+  void onCompletion(CXXRecordDecl *Record, CompletionFunction Fn);
 
 public:
   ~HLSLExternalSemaSource() override;
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 5057bc6629f046a..8ed6480a9f5c9c8 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -306,6 +306,7 @@ struct BuiltinTypeDeclBuilder {
   }
 
   TemplateParameterListBuilder addTemplateArgumentList();
+  BuiltinTypeDeclBuilder &addSimpleTemplateParams(ArrayRef<StringRef> Names);
 };
 
 struct TemplateParameterListBuilder {
@@ -360,11 +361,19 @@ struct TemplateParameterListBuilder {
     return Builder;
   }
 };
+} // namespace
 
 TemplateParameterListBuilder BuiltinTypeDeclBuilder::addTemplateArgumentList() {
   return TemplateParameterListBuilder(*this);
 }
-} // namespace
+
+BuiltinTypeDeclBuilder &
+BuiltinTypeDeclBuilder::addSimpleTemplateParams(ArrayRef<StringRef> Names) {
+  TemplateParameterListBuilder Builder = this->addTemplateArgumentList();
+  for (StringRef Name : Names)
+    Builder.addTypeParameter(Name);
+  return Builder.finalizeTemplateArgs();
+}
 
 HLSLExternalSemaSource::~HLSLExternalSemaSource() {}
 
@@ -390,7 +399,7 @@ void HLSLExternalSemaSource::InitializeSema(Sema &S) {
   // Force external decls in the HLSL namespace to load from the PCH.
   (void)HLSLNamespace->getCanonicalDecl()->decls_begin();
   defineTrivialHLSLTypes();
-  forwardDeclareHLSLTypes();
+  defineHLSLTypesWithForwardDeclarations();
 
   // This adds a `using namespace hlsl` directive. In DXC, we don't put HLSL's
   // built in types inside a namespace, but we are planning to change that in
@@ -467,18 +476,32 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
                      .Record;
 }
 
-void HLSLExternalSemaSource::forwardDeclareHLSLTypes() {
+/// Set up common members and attributes for buffer types
+static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
+                                              ResourceClass RC,
+                                              ResourceKind RK) {
+  return BuiltinTypeDeclBuilder(Decl)
+      .addHandleMember()
+      .addDefaultHandleConstructor(S, RC)
+      .annotateResourceClass(RC, RK);
+}
+
+void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
   CXXRecordDecl *Decl;
   Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
-             .addTemplateArgumentList()
-             .addTypeParameter("element_type")
-             .finalizeTemplateArgs()
+             .addSimpleTemplateParams({"element_type"})
              .Record;
-  if (!Decl->isCompleteDefinition())
-    Completions.insert(
-        std::make_pair(Decl->getCanonicalDecl(),
-                       std::bind(&HLSLExternalSemaSource::completeBufferType,
-                                 this, std::placeholders::_1)));
+  onCompletion(Decl, [this](CXXRecordDecl *Decl) {
+    setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
+                    ResourceKind::TypedBuffer)
+        .addArraySubscriptOperators()
+        .completeDefinition();
+  });
+}
+
+void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,
+                                          CompletionFunction Fn) {
+  Completions.insert(std::make_pair(Record->getCanonicalDecl(), Fn));
 }
 
 void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) {
@@ -496,12 +519,3 @@ void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) {
     return;
   It->second(Record);
 }
-
-void HLSLExternalSemaSource::completeBufferType(CXXRecordDecl *Record) {
-  BuiltinTypeDeclBuilder(Record)
-      .addHandleMember()
-      .addDefaultHandleConstructor(*SemaPtr, ResourceClass::UAV)
-      .addArraySubscriptOperators()
-      .annotateResourceClass(ResourceClass::UAV, ResourceKind::TypedBuffer)
-      .completeDefinition();
-}

@bogner bogner self-assigned this Nov 30, 2023
@bogner bogner linked an issue Nov 30, 2023 that may be closed by this pull request
Created using spr 1.3.5
@bogner bogner merged commit 84cdead into users/bogner/spr/main.hlsl-add-helpers-to-simplify-hlsl-resource-type-declarations-nfc Dec 8, 2023
7 checks passed
@bogner bogner deleted the users/bogner/spr/hlsl-add-helpers-to-simplify-hlsl-resource-type-declarations-nfc branch December 8, 2023 01:28
bogner added a commit that referenced this pull request Dec 8, 2023
A few changes to HLSLExternalSemaSource and its BuiltinTypeDeclBuilder
to make defining buffer types less verbose. This will make it a lot
easier to see what the differences between the various buffer types
are once we start introducing more of them.

Pull Request: #73967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor HLSLExternalSemaSource's builder API
5 participants