[libcxx-commits] [libcxx] [libc++] Improve diagnostic when failing to parse the tzdb (PR #122125)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/122125

>From 5781d7d1ae64c34f2f3f2388a9aba83192fc3a62 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 8 Jan 2025 10:21:19 -0500
Subject: [PATCH 1/2] [libc++] Improve diagnostic when failing to parse the
 tzdb

Providing the character that we failed on is helpful for figuring
out what's going wrong in the tztb.
---
 libcxx/src/experimental/tzdb.cpp              | 20 ++++++++++++++-----
 .../time.zone/time.zone.db/rules.pass.cpp     | 15 +++++++++-----
 .../time.zone/time.zone.db/version.pass.cpp   |  5 +++--
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/libcxx/src/experimental/tzdb.cpp b/libcxx/src/experimental/tzdb.cpp
index 638d45f69e033e..10f7001d74806b 100644
--- a/libcxx/src/experimental/tzdb.cpp
+++ b/libcxx/src/experimental/tzdb.cpp
@@ -8,6 +8,7 @@
 
 // For information see https://libcxx.llvm.org/DesignDocs/TimeZone.html
 
+#include <__assert>
 #include <algorithm>
 #include <cctype>
 #include <chrono>
@@ -97,14 +98,23 @@ static void __skip(istream& __input, string_view __suffix) {
 }
 
 static void __matches(istream& __input, char __expected) {
-  if (std::tolower(__input.get()) != __expected)
-    std::__throw_runtime_error((string("corrupt tzdb: expected character '") + __expected + '\'').c_str());
+  _LIBCPP_ASSERT_INTERNAL(std::islower(__expected), "lowercase characters only here!");
+  char __c = __input.get();
+  if (std::tolower(__c) != __expected)
+    std::__throw_runtime_error(
+        (string("corrupt tzdb: expected character '") + __expected + "', got '" + __c + "' instead").c_str());
 }
 
 static void __matches(istream& __input, string_view __expected) {
-  for (auto __c : __expected)
-    if (std::tolower(__input.get()) != __c)
-      std::__throw_runtime_error((string("corrupt tzdb: expected string '") + string(__expected) + '\'').c_str());
+  for (auto __c : __expected) {
+    _LIBCPP_ASSERT_INTERNAL(std::islower(__c), "lowercase strings only here!");
+    char __actual = __input.get();
+    if (std::tolower(__actual) != __c)
+      std::__throw_runtime_error(
+          (string("corrupt tzdb: expected character '") + __c + "' from string '" + string(__expected) + "', got '" +
+           __actual + "' instead")
+              .c_str());
+  }
 }
 
 [[nodiscard]] static string __parse_string(istream& __input) {
diff --git a/libcxx/test/libcxx/time/time.zone/time.zone.db/rules.pass.cpp b/libcxx/test/libcxx/time/time.zone/time.zone.db/rules.pass.cpp
index 7d9759320c535b..237a206b3a95bd 100644
--- a/libcxx/test/libcxx/time/time.zone/time.zone.db/rules.pass.cpp
+++ b/libcxx/test/libcxx/time/time.zone/time.zone.db/rules.pass.cpp
@@ -20,9 +20,10 @@
 // ADDITIONAL_COMPILE_FLAGS: -I %{libcxx-dir}/src/experimental/include
 
 #include <chrono>
+#include <cstdio>
 #include <fstream>
-#include <string>
 #include <string_view>
+#include <string>
 #include <variant>
 
 #include "assert_macros.h"
@@ -96,7 +97,7 @@ static void test_invalid() {
   test_exception("R r 0 mix", "corrupt tzdb: expected whitespace");
   test_exception("R r 0 1", "corrupt tzdb: expected whitespace");
 
-  test_exception("R r 0 1 X", "corrupt tzdb: expected character '-'");
+  test_exception("R r 0 1 X", "corrupt tzdb: expected character '-', got 'X' instead");
 
   test_exception("R r 0 1 -", "corrupt tzdb: expected whitespace");
 
@@ -106,13 +107,17 @@ static void test_invalid() {
 
   test_exception("R r 0 1 - Ja +", "corrupt tzdb weekday: invalid name");
   test_exception("R r 0 1 - Ja 32", "corrupt tzdb day: value too large");
-  test_exception("R r 0 1 - Ja l", "corrupt tzdb: expected string 'last'");
+  test_exception(
+      "R r 0 1 - Ja l",
+      std::string{"corrupt tzdb: expected character 'a' from string 'last', got '"} + (char)EOF + "' instead");
   test_exception("R r 0 1 - Ja last", "corrupt tzdb weekday: invalid name");
   test_exception("R r 0 1 - Ja lastS", "corrupt tzdb weekday: invalid name");
   test_exception("R r 0 1 - Ja S", "corrupt tzdb weekday: invalid name");
   test_exception("R r 0 1 - Ja Su", "corrupt tzdb on: expected '>=' or '<='");
-  test_exception("R r 0 1 - Ja Su>", "corrupt tzdb: expected character '='");
-  test_exception("R r 0 1 - Ja Su<", "corrupt tzdb: expected character '='");
+  test_exception(
+      "R r 0 1 - Ja Su>", std::string{"corrupt tzdb: expected character '=', got '"} + (char)EOF + "' instead");
+  test_exception(
+      "R r 0 1 - Ja Su<", std::string{"corrupt tzdb: expected character '=', got '"} + (char)EOF + "' instead");
   test_exception("R r 0 1 - Ja Su>=+", "corrupt tzdb: expected a non-zero digit");
   test_exception("R r 0 1 - Ja Su>=0", "corrupt tzdb: expected a non-zero digit");
   test_exception("R r 0 1 - Ja Su>=32", "corrupt tzdb day: value too large");
diff --git a/libcxx/test/libcxx/time/time.zone/time.zone.db/version.pass.cpp b/libcxx/test/libcxx/time/time.zone/time.zone.db/version.pass.cpp
index b4f32a1b6fd785..ca3a890f1fa548 100644
--- a/libcxx/test/libcxx/time/time.zone/time.zone.db/version.pass.cpp
+++ b/libcxx/test/libcxx/time/time.zone/time.zone.db/version.pass.cpp
@@ -18,9 +18,10 @@
 // This is not part of the public tzdb interface.
 
 #include <chrono>
+#include <cstdio>
 #include <fstream>
-#include <string>
 #include <string_view>
+#include <string>
 
 #include "assert_macros.h"
 #include "concat_macros.h"
@@ -60,7 +61,7 @@ static void test_exception(std::string_view input, [[maybe_unused]] std::string_
 }
 
 int main(int, const char**) {
-  test_exception("", "corrupt tzdb: expected character '#'");
+  test_exception("", std::string{"corrupt tzdb: expected character '#', got '"} + (char)EOF + "' instead");
   test_exception("#version", "corrupt tzdb: expected whitespace");
   test("#version     \t                      ABCD", "ABCD");
   test("#Version     \t                      ABCD", "ABCD");

>From 4e1c48eb0e0f9136f9489458a71e41c2892ba14f Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 9 Jan 2025 16:09:55 -0500
Subject: [PATCH 2/2] Handle non alpha characters

---
 libcxx/src/experimental/tzdb.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/src/experimental/tzdb.cpp b/libcxx/src/experimental/tzdb.cpp
index 10f7001d74806b..83c4fe51a586a0 100644
--- a/libcxx/src/experimental/tzdb.cpp
+++ b/libcxx/src/experimental/tzdb.cpp
@@ -98,7 +98,7 @@ static void __skip(istream& __input, string_view __suffix) {
 }
 
 static void __matches(istream& __input, char __expected) {
-  _LIBCPP_ASSERT_INTERNAL(std::islower(__expected), "lowercase characters only here!");
+  _LIBCPP_ASSERT_INTERNAL(!std::isalpha(__expected) || std::islower(__expected), "lowercase characters only here!");
   char __c = __input.get();
   if (std::tolower(__c) != __expected)
     std::__throw_runtime_error(
@@ -107,7 +107,7 @@ static void __matches(istream& __input, char __expected) {
 
 static void __matches(istream& __input, string_view __expected) {
   for (auto __c : __expected) {
-    _LIBCPP_ASSERT_INTERNAL(std::islower(__c), "lowercase strings only here!");
+    _LIBCPP_ASSERT_INTERNAL(!std::isalpha(__expected) || std::islower(__c), "lowercase strings only here!");
     char __actual = __input.get();
     if (std::tolower(__actual) != __c)
       std::__throw_runtime_error(