[NFC][HLSL] Move Sema work from ParseMicrosoftRootSignatureAttributeArgs

[inbelic]

This separates semantic analysis from parsing by moving RootSignatureDecl creation, scope storage, and lookup logic into SemaHLSL.

For more context see: #142834.

• Define ActOnStartRootSignatureDecl and ActOnFinishRootSignatureDecl on SemaHLSL
• NFC so no test changes.

Resolves: #142834

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes

This separates semantic analysis from parsing by moving RootSignatureDecl creation, scope storage, and lookup logic into Sema.

For more context see: #142834.

• Define ActOnStartRootSignatureDecl and ActOnFinishRootSignatureDecl on SemaDecl
• NFC so no test changes.

Resolves: #142834

---

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

3 Files Affected:

• (modified) clang/include/clang/Sema/Sema.h (+13)
• (modified) clang/lib/Parse/ParseDeclCXX.cpp (+9-17)
• (modified) clang/lib/Sema/SemaDecl.cpp (+24)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f9a086b6966d9..0eed7b922e32e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3619,6 +3619,19 @@ class Sema final : public SemaBase {
                                       SourceLocation NameLoc,
                                       bool IsTemplateTypeArg);
 
+  /// Computes the unique Root Signature identifier from the given signature,
+  /// then lookup if there is a previousy created Root Signature decl.
+  ///
+  /// Returns the identifier and if it was found
+  std::pair<IdentifierInfo *, bool>
+  ActOnStartRootSignatureDecl(StringRef Signature);
+
+  /// Creates the Root Signature decl of the parsed Root Signature elements
+  /// onto the AST and push it onto current Scope
+  void ActOnFinishRootSignatureDecl(
+      SourceLocation Loc, IdentifierInfo *DeclIdent,
+      SmallVector<llvm::hlsl::rootsig::RootElement> &Elements);
+
   class NameClassification {
     NameClassificationKind Kind;
     union {
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 2cf33a856c4f4..5c878ed22d47d 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4942,18 +4942,13 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) {
 
   // Construct our identifier
   StringRef Signature = StrLiteral.value()->getString();
-  auto Hash = llvm::hash_value(Signature);
-  std::string IdStr = "__hlsl_rootsig_decl_" + std::to_string(Hash);
-  IdentifierInfo *DeclIdent = &(Actions.getASTContext().Idents.get(IdStr));
-
-  LookupResult R(Actions, DeclIdent, SourceLocation(),
-                 Sema::LookupOrdinaryName);
-  // Check if we have already found a decl of the same name, if we haven't
-  // then parse the root signature string and construct the in-memory elements
-  if (!Actions.LookupQualifiedName(R, Actions.CurContext)) {
+  auto [DeclIdent, Found] = Actions.ActOnStartRootSignatureDecl(Signature);
+  // If we haven't found an already defined DeclIdent then parse the root
+  // signature string and construct the in-memory elements
+  if (!Found) {
+    // Offset location 1 to account for '"'
     SourceLocation SignatureLoc =
-        StrLiteral.value()->getExprLoc().getLocWithOffset(
-            1); // offset 1 for '"'
+        StrLiteral.value()->getExprLoc().getLocWithOffset(1);
     // Invoke the root signature parser to construct the in-memory constructs
     hlsl::RootSignatureLexer Lexer(Signature, SignatureLoc);
     SmallVector<llvm::hlsl::rootsig::RootElement> RootElements;
@@ -4963,12 +4958,9 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) {
       return;
     }
 
-    // Create the Root Signature
-    auto *SignatureDecl = HLSLRootSignatureDecl::Create(
-        Actions.getASTContext(), /*DeclContext=*/Actions.CurContext,
-        RootSignatureLoc, DeclIdent, RootElements);
-    SignatureDecl->setImplicit();
-    Actions.PushOnScopeChains(SignatureDecl, getCurScope());
+    // Perform constructin of declaration
+    Actions.ActOnFinishRootSignatureDecl(RootSignatureLoc, DeclIdent,
+                                         RootElements);
   }
 
   // Create the arg for the ParsedAttr
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 60e911b9fecc0..ec602f954dcfe 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -62,6 +62,7 @@
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/TargetParser/Triple.h"
 #include <algorithm>
@@ -653,6 +654,29 @@ ParsedType Sema::ActOnMSVCUnknownTypeName(const IdentifierInfo &II,
   return CreateParsedType(T, Builder.getTypeSourceInfo(Context, T));
 }
 
+std::pair<IdentifierInfo *, bool>
+Sema::ActOnStartRootSignatureDecl(StringRef Signature) {
+  auto Hash = llvm::hash_value(Signature);
+  std::string IdStr = "__hlsl_rootsig_decl_" + std::to_string(Hash);
+  IdentifierInfo *DeclIdent = &(getASTContext().Idents.get(IdStr));
+
+  // Check if we have already found a decl of the same name
+  LookupResult R(*this, DeclIdent, SourceLocation(), Sema::LookupOrdinaryName);
+  bool Found = LookupQualifiedName(R, this->CurContext);
+  return {DeclIdent, Found};
+}
+
+void Sema::ActOnFinishRootSignatureDecl(
+    SourceLocation Loc, IdentifierInfo *DeclIdent,
+    SmallVector<llvm::hlsl::rootsig::RootElement> &Elements) {
+  // Create the Root Signature
+  auto *SignatureDecl = HLSLRootSignatureDecl::Create(
+      getASTContext(), /*DeclContext=*/CurContext, Loc, DeclIdent, Elements);
+
+  SignatureDecl->setImplicit();
+  PushOnScopeChains(SignatureDecl, getCurScope());
+}
+
 DeclSpec::TST Sema::isTagName(IdentifierInfo &II, Scope *S) {
   // Do a tag name lookup in this scope.
   LookupResult R(*this, &II, SourceLocation(), LookupTagName);

[hekota]

Nit: consider using std::optional<IdentifierInfo*>.

[inbelic]

If it is not found we still want to return it so that we don't need to re-compute it in ActOnFinish... when constructing the decl

[AaronBallman]

Thank you for working on this! I think it's heading in the right direction.