Skip to content

Conversation

hekota
Copy link
Member

@hekota hekota commented Sep 23, 2025

Replace early exit from builder methods with assert - they should be not called on a completed record.
Also removes commented-out code.

Replace early exit from builder methods with assert - they should be not called on a completed record.
Also removed commented-out code.
@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 Sep 23, 2025
@hekota hekota changed the title [HLSL][NFC] Cleanup builtin type declaration builder [HLSL][NFC] Builtin type declaration builder cleanup Sep 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Helena Kotas (hekota)

Changes

Replace early exit from builder methods with assert - they should be not called on a completed record.
Also removes commented-out code.


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

1 Files Affected:

  • (modified) clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp (+6-18)
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
index 5eafd03d89efe..97a6a7f1439db 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
@@ -748,8 +748,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleMember(
 // Adds default constructor to the resource class:
 // Resource::Resource()
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addDefaultHandleConstructor() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   using PH = BuiltinTypeMethodBuilder::PlaceHolder;
   QualType HandleType = getResourceHandleField()->getType();
@@ -773,8 +772,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addDefaultHandleConstructor() {
 //   return tmp;
 // }
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromBinding() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   using PH = BuiltinTypeMethodBuilder::PlaceHolder;
   ASTContext &AST = SemaRef.getASTContext();
@@ -811,8 +809,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromBinding() {
 //   return tmp;
 // }
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromImplicitBinding() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   using PH = BuiltinTypeMethodBuilder::PlaceHolder;
   ASTContext &AST = SemaRef.getASTContext();
@@ -838,8 +835,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromImplicitBinding() {
 }
 
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCopyConstructor() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   ASTContext &AST = SemaRef.getASTContext();
   QualType RecordType = AST.getCanonicalTagType(Record);
@@ -857,8 +853,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCopyConstructor() {
 }
 
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCopyAssignmentOperator() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   ASTContext &AST = SemaRef.getASTContext();
   QualType RecordType = AST.getCanonicalTagType(Record);
@@ -889,8 +884,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addArraySubscriptOperators() {
 }
 
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addLoadMethods() {
-  if (Record->isCompleteDefinition())
-    return *this;
+  assert(!Record->isCompleteDefinition() && "record is already complete");
 
   ASTContext &AST = Record->getASTContext();
   IdentifierInfo &II = AST.Idents.get("Load", tok::TokenKind::identifier);
@@ -931,12 +925,6 @@ BuiltinTypeDeclBuilder::getResourceAttrs() const {
   return cast<HLSLAttributedResourceType>(HandleType)->getAttrs();
 }
 
-// BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::startDefinition() {
-//   assert(!Record->isCompleteDefinition() && "record is already complete");
-//   Record->startDefinition();
-//   return *this;
-// }
-
 BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::completeDefinition() {
   assert(!Record->isCompleteDefinition() && "record is already complete");
   assert(Record->isBeingDefined() &&

@hekota hekota merged commit e5acda7 into llvm:main Sep 23, 2025
14 checks passed
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
None yet
Development

Successfully merging this pull request may close these issues.

4 participants