Skip to content

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Sep 16, 2025

We can construct the return values directly and simplify the interface.

Copy link
Contributor Author

ilovepi commented Sep 16, 2025

@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2025

@llvm/pr-subscribers-llvm-support

Author: Paul Kirth (ilovepi)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Support/Mustache.cpp (+13-13)
diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index e2de7645e8dfb..f948344883452 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -11,6 +11,7 @@
 #include "llvm/Support/raw_ostream.h"
 
 #include <cctype>
+#include <optional>
 #include <sstream>
 
 #define DEBUG_TYPE "mustache"
@@ -325,9 +326,8 @@ struct Tag {
   size_t StartPosition = StringRef::npos;
 };
 
-static Tag findNextTag(StringRef Template, size_t StartPos,
-                       const SmallString<8> &Open,
-                       const SmallString<8> &Close) {
+static Tag findNextTag(StringRef Template, size_t StartPos, StringRef Open,
+                       StringRef Close) {
   const StringLiteral TripleOpen("{{{");
   const StringLiteral TripleClose("}}}");
 
@@ -368,14 +368,14 @@ static Tag findNextTag(StringRef Template, size_t StartPos,
   return Result;
 }
 
-static void processTag(const Tag &T, SmallVectorImpl<Token> &Tokens,
-                       SmallString<8> &Open, SmallString<8> &Close) {
+static std::optional<std::pair<StringRef, StringRef>>
+processTag(const Tag &T, SmallVectorImpl<Token> &Tokens) {
   LLVM_DEBUG(dbgs() << "  Found tag: \"" << T.FullMatch << "\", Content: \""
                     << T.Content << "\"\n");
   if (T.TagKind == Tag::Kind::Triple) {
     Tokens.emplace_back(T.FullMatch.str(), "&" + T.Content.str(), '&');
     LLVM_DEBUG(dbgs() << "  Created UnescapeVariable token.\n");
-    return;
+    return std::nullopt;
   }
   StringRef Interpolated = T.Content;
   std::string RawBody = T.FullMatch.str();
@@ -383,7 +383,7 @@ static void processTag(const Tag &T, SmallVectorImpl<Token> &Tokens,
     char Front = Interpolated.empty() ? ' ' : Interpolated.trim().front();
     Tokens.emplace_back(RawBody, Interpolated.str(), Front);
     LLVM_DEBUG(dbgs() << "  Created tag token of type '" << Front << "'\n");
-    return;
+    return std::nullopt;
   }
   Tokens.emplace_back(RawBody, Interpolated.str(), '=');
   StringRef DelimSpec = Interpolated.trim();
@@ -392,11 +392,9 @@ static void processTag(const Tag &T, SmallVectorImpl<Token> &Tokens,
   DelimSpec = DelimSpec.trim();
 
   auto [NewOpen, NewClose] = DelimSpec.split(' ');
-  Open = NewOpen;
-  Close = NewClose;
-
-  LLVM_DEBUG(dbgs() << "  Found Set Delimiter tag. NewOpen='" << Open
-                    << "', NewClose='" << Close << "'\n");
+  LLVM_DEBUG(dbgs() << "  Found Set Delimiter tag. NewOpen='" << NewOpen
+                    << "', NewClose='" << NewClose << "'\n");
+  return std::make_pair(NewOpen, NewClose);
 }
 
 // Simple tokenizer that splits the template into tokens.
@@ -430,7 +428,9 @@ static SmallVector<Token> tokenize(StringRef Template) {
       LLVM_DEBUG(dbgs() << "  Created Text token: \"" << Text << "\"\n");
     }
 
-    processTag(T, Tokens, Open, Close);
+    if (auto NewDelims = processTag(T, Tokens)) {
+      std::tie(Open, Close) = *NewDelims;
+    }
 
     // Move past the tag.
     Start = T.StartPosition + T.FullMatch.size();

@ilovepi ilovepi force-pushed the users/ilovepi/mustache-strinref branch from 0b15e73 to 452b7c4 Compare September 25, 2025 22:12
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-renderer branch 2 times, most recently from 3d00de1 to f18412d Compare September 26, 2025 01:55
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-strinref branch from 452b7c4 to ea687e4 Compare September 26, 2025 01:55
Copy link
Member

@evelez7 evelez7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mega-nit but shouldn't this patch be called "Use StringRef return values"? The return value is what's being changed to a pair of StringRefs, not the parameters.

Copy link
Contributor Author

ilovepi commented Sep 29, 2025

Ah, I guess the commit title is stale. thanks. I can update it.

@ilovepi ilovepi force-pushed the users/ilovepi/mustache-strinref branch from ea687e4 to 2ddc19d Compare September 29, 2025 17:39
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-renderer branch 2 times, most recently from 7ca57bb to af8b255 Compare September 29, 2025 22:28
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-strinref branch from 2ddc19d to bfd4244 Compare September 29, 2025 22:28
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-renderer branch 2 times, most recently from 26f0197 to 16f12ac Compare September 30, 2025 01:22
Base automatically changed from users/ilovepi/mustache-renderer to main September 30, 2025 01:48
Just return an optional pair of StringRefs instead.
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-strinref branch 2 times, most recently from 7be7b16 to cb3731b Compare September 30, 2025 01:54
@ilovepi ilovepi changed the title [llvm][mustache] Use StringRef parameters [llvm][mustache] Remove out parameters from processTags() Sep 30, 2025
@ilovepi ilovepi enabled auto-merge (squash) September 30, 2025 01:58
@ilovepi ilovepi merged commit 978644c into main Sep 30, 2025
9 checks passed
@ilovepi ilovepi deleted the users/ilovepi/mustache-strinref branch September 30, 2025 02:37
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
We can construct the return values directly and simplify the interface.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants