[clang-doc][nfc] Avoid constructing SmallString in ToString method

[ilovepi]

This patch updates the return type of HTMLTag::toString from
SmallString<16> to StringRef, since this API only returns string
literals. As a result, there is no need to construct a full heap
allocated or owned string.

Additionally, this patch renames ToString to toString to match the LLVM
style guide.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

Author: Paul Kirth (ilovepi)

Changes

This patch updates the return type HTMLTag::toString from
SmallString<16> to const char*, since this API only has a single use to
emit the string into an output stream. As a result, there is no need to
construct a full heap allocated or owned string. Arguably we should
instead provide an overload to the output stream operator that performs
this action directly. However, this change is minimal, and functionally
equivalent for now.

Additionally, this patch renames ToString in the LLVM style as toString.

---

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

1 Files Affected:

• (modified) clang-tools-extra/clang-doc/HTMLGenerator.cpp (+21-21)

diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
index c0faf5f7e8fd9..cc5b48077faea 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -56,7 +56,7 @@ class HTMLTag {
   operator bool() = delete;
 
   bool IsSelfClosing() const;
-  llvm::SmallString<16> ToString() const;
+  const char* toString() const;
 
 private:
   TagType Value;
@@ -137,42 +137,42 @@ bool HTMLTag::IsSelfClosing() const {
   llvm_unreachable("Unhandled HTMLTag::TagType");
 }
 
-llvm::SmallString<16> HTMLTag::ToString() const {
+const char* HTMLTag::toString() const {
   switch (Value) {
   case HTMLTag::TAG_A:
-    return llvm::SmallString<16>("a");
+    return "a";
   case HTMLTag::TAG_DIV:
-    return llvm::SmallString<16>("div");
+    return "div";
   case HTMLTag::TAG_FOOTER:
-    return llvm::SmallString<16>("footer");
+    return "footer";
   case HTMLTag::TAG_H1:
-    return llvm::SmallString<16>("h1");
+    return "h1";
   case HTMLTag::TAG_H2:
-    return llvm::SmallString<16>("h2");
+    return "h2";
   case HTMLTag::TAG_H3:
-    return llvm::SmallString<16>("h3");
+    return "h3";
   case HTMLTag::TAG_HEADER:
-    return llvm::SmallString<16>("header");
+    return "header";
   case HTMLTag::TAG_LI:
-    return llvm::SmallString<16>("li");
+    return "li";
   case HTMLTag::TAG_LINK:
-    return llvm::SmallString<16>("link");
+    return "link";
   case HTMLTag::TAG_MAIN:
-    return llvm::SmallString<16>("main");
+    return "main";
   case HTMLTag::TAG_META:
-    return llvm::SmallString<16>("meta");
+    return "meta";
   case HTMLTag::TAG_OL:
-    return llvm::SmallString<16>("ol");
+    return "ol";
   case HTMLTag::TAG_P:
-    return llvm::SmallString<16>("p");
+    return "p";
   case HTMLTag::TAG_SCRIPT:
-    return llvm::SmallString<16>("script");
+    return "script";
   case HTMLTag::TAG_SPAN:
-    return llvm::SmallString<16>("span");
+    return "span";
   case HTMLTag::TAG_TITLE:
-    return llvm::SmallString<16>("title");
+    return "title";
   case HTMLTag::TAG_UL:
-    return llvm::SmallString<16>("ul");
+    return "ul";
   }
   llvm_unreachable("Unhandled HTMLTag::TagType");
 }
@@ -191,7 +191,7 @@ void TagNode::Render(llvm::raw_ostream &OS, int IndentationLevel) {
       break;
     }
   OS.indent(IndentationLevel * 2);
-  OS << "<" << Tag.ToString();
+  OS << "<" << Tag.toString();
   for (const auto &A : Attributes)
     OS << " " << A.first << "=\"" << A.second << "\"";
   if (Tag.IsSelfClosing()) {
@@ -216,7 +216,7 @@ void TagNode::Render(llvm::raw_ostream &OS, int IndentationLevel) {
   }
   if (!InlineChildren)
     OS.indent(IndentationLevel * 2);
-  OS << "</" << Tag.ToString() << ">";
+  OS << "</" << Tag.toString() << ">";
 }
 
 template <typename Derived, typename Base,

[ilovepi]

@PeterChou1 please take a look and provide some feedback.

[ilovepi]

Maybe this should be renamed to c_str() or str()?

[PeterChou1]

I think it would be more appropriate to rename it c_str since in my mind c_str equals char* and str means std::string

[ilovepi]

done

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[petrhosek]

I'd use std::string_view as a return value instead of const char * as a more idiomatic C++ in which case you don't need to rename the method to c_str().

[ilovepi]

I'd use std::string_view as a return value instead of const char * as a more idiomatic C++ in which case you don't need to rename the method to c_str().

I opted for StringRef, since that seems more in line w/ the conventions here, but I'm happy to switch to std::string_view if you'd prefer.

[petrhosek]

I'd use std::string_view as a return value instead of const char * as a more idiomatic C++ in which case you don't need to rename the method to c_str().

I opted for StringRef, since that seems more in line w/ the conventions here, but I'm happy to switch to std::string_view if you'd prefer.

AFAIK the long-term plan is to replace StringRef with std::string_view but I'm fine using StringRef for consistency with the rest of the codebase.