[llvm] [TableGen] Fix MSVC C4125 error after 5d13ff1e940b (PR #207855)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-tablegen

Author: Jinsong Ji (jsji)

<details>
<summary>Changes</summary>

Upstream 5d13ff1e940b ("[MC] Generate FeatureKV with compact string table")
introduced StringToOffsetTable::EmitString which generates octal escape
sequences like \000. When followed by a digit and split across 70-character
lines, this creates patterns like "\0002" where MSVC warns "C4125: decimal
digit terminates octal escape sequence."

This can be triggered by numeric feature names (e.g., "100", "200", etc.)
which create sequences like \000200 in the string table. When line breaks
split this as "\0002" + "00...", MSVC sees an octal escape immediately
followed by more digits and warns about potential ambiguity.

Most upstream targets don't typically hit this because:
1. Their feature names rarely start with pure digits
2. When they do, line breaks happen to fall at different positions

Fix by inserting a string literal boundary after any octal escape followed
by a digit: "\000" "200" instead of "\000200". This prevents line breaks
from ever splitting the sequence ambiguously, and makes it clear that the
digit is not part of the escape sequence.

Added unit tests to verify the fix handles octal escapes followed by digits
correctly while not affecting other cases.

Co-Authored-By: Claude Sonnet 4.5 <noreply@<!-- -->anthropic.com>


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


3 Files Affected:

- (modified) llvm/lib/TableGen/StringToOffsetTable.cpp (+6) 
- (modified) llvm/unittests/TableGen/CMakeLists.txt (+1) 
- (added) llvm/unittests/TableGen/StringToOffsetTableTest.cpp (+115) 


``````````diff
diff --git a/llvm/lib/TableGen/StringToOffsetTable.cpp b/llvm/lib/TableGen/StringToOffsetTable.cpp
index 06d8240486245..2c8f45f362612 100644
--- a/llvm/lib/TableGen/StringToOffsetTable.cpp
+++ b/llvm/lib/TableGen/StringToOffsetTable.cpp
@@ -118,6 +118,12 @@ void StringToOffsetTable::EmitString(raw_ostream &O) const {
       O << EscapedStr[++i];
       O << EscapedStr[++i];
       CharsPrinted += 3;
+      // MSVC C4125: If next char is a digit, close string and start new one
+      // to avoid octal escape at line-end being followed by digit at line-start
+      if (i + 1 < e && isDigit(EscapedStr[i + 1])) {
+        O << "\" \"";
+        CharsPrinted = 2; // Reset but account for the opening quote chars
+      }
     } else {
       O << EscapedStr[++i];
       ++CharsPrinted;
diff --git a/llvm/unittests/TableGen/CMakeLists.txt b/llvm/unittests/TableGen/CMakeLists.txt
index 854f6c0f9b162..830eb293c3a8e 100644
--- a/llvm/unittests/TableGen/CMakeLists.txt
+++ b/llvm/unittests/TableGen/CMakeLists.txt
@@ -13,6 +13,7 @@ add_llvm_unittest(TableGenTests
   AutomataTest.cpp
   CodeExpanderTest.cpp
   ParserEntryPointTest.cpp
+  StringToOffsetTableTest.cpp
 
   DISABLE_LLVM_LINK_LLVM_DYLIB
   )
diff --git a/llvm/unittests/TableGen/StringToOffsetTableTest.cpp b/llvm/unittests/TableGen/StringToOffsetTableTest.cpp
new file mode 100644
index 0000000000000..2be9494bb1865
--- /dev/null
+++ b/llvm/unittests/TableGen/StringToOffsetTableTest.cpp
@@ -0,0 +1,115 @@
+//===- StringToOffsetTableTest.cpp - StringToOffsetTable tests -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/TableGen/StringToOffsetTable.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+TEST(StringToOffsetTableTest, OctalEscapeFollowedByDigit) {
+  // Test that when an octal escape sequence (\000) is followed by a digit,
+  // the output inserts a string literal boundary to prevent MSVC C4125 warning.
+  // This can happen when numeric feature names create sequences like "\000200".
+
+  StringToOffsetTable Table;
+
+  // Add strings that create octal escape followed by digit pattern.
+  // '\0' will be emitted as \000, and "200" starts with a digit.
+  std::string TestStr;
+  TestStr += '\0';  // Will become \000
+  TestStr += "200"; // Starts with digit
+
+  Table.GetOrAddStringOffset(TestStr);
+
+  std::string Output;
+  raw_string_ostream OS(Output);
+  Table.EmitString(OS);
+  OS.flush();
+
+  // The output should contain a string boundary after the octal escape:
+  // Should be: "    "\000" "200""
+  // Not:       "    "\000200""
+  // This prevents line breaks from creating "\0002" + "00..." which triggers
+  // MSVC warning C4125: decimal digit terminates octal escape sequence.
+
+  EXPECT_TRUE(Output.find("\\000\" \"2") != std::string::npos)
+      << "Expected string boundary after octal escape before digit, got: "
+      << Output;
+}
+
+TEST(StringToOffsetTableTest, OctalEscapeNotFollowedByDigit) {
+  // Test that octal escape NOT followed by digit doesn't insert boundary.
+
+  StringToOffsetTable Table;
+
+  std::string TestStr;
+  TestStr += '\0';  // Will become \000
+  TestStr += "abc"; // Starts with non-digit
+
+  Table.GetOrAddStringOffset(TestStr);
+
+  std::string Output;
+  raw_string_ostream OS(Output);
+  Table.EmitString(OS);
+  OS.flush();
+
+  // Should NOT insert a boundary when not followed by digit:
+  // Should be: "    "\000abc""
+
+  EXPECT_TRUE(Output.find("\\000abc") != std::string::npos)
+      << "Expected no string boundary after octal escape when not followed by "
+         "digit, got: "
+      << Output;
+  EXPECT_TRUE(Output.find("\\000\" \"a") == std::string::npos)
+      << "Unexpected string boundary found: " << Output;
+}
+
+TEST(StringToOffsetTableTest, MultipleOctalDigitSequences) {
+  // Test multiple sequences of octal escapes followed by digits.
+
+  StringToOffsetTable Table;
+
+  std::string TestStr;
+  TestStr += '\0';
+  TestStr += "100";
+  TestStr += '\0';
+  TestStr += "200";
+  TestStr += '\0';
+  TestStr += "300";
+
+  Table.GetOrAddStringOffset(TestStr);
+
+  std::string Output;
+  raw_string_ostream OS(Output);
+  Table.EmitString(OS);
+  OS.flush();
+
+  // Each octal escape followed by a digit should have a boundary.
+  EXPECT_TRUE(Output.find("\\000\" \"1") != std::string::npos)
+      << "Expected first boundary, got: " << Output;
+  EXPECT_TRUE(Output.find("\\000\" \"2") != std::string::npos)
+      << "Expected second boundary, got: " << Output;
+  EXPECT_TRUE(Output.find("\\000\" \"3") != std::string::npos)
+      << "Expected third boundary, got: " << Output;
+}
+
+TEST(StringToOffsetTableTest, PlainString) {
+  // Test that plain strings without null characters work normally.
+
+  StringToOffsetTable Table;
+  Table.GetOrAddStringOffset("hello");
+
+  std::string Output;
+  raw_string_ostream OS(Output);
+  Table.EmitString(OS);
+  OS.flush();
+
+  EXPECT_TRUE(Output.find("hello") != std::string::npos)
+      << "Expected plain string, got: " << Output;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/207855