[lldb] Fix matchers for char array formatters

[labath]

Summary

They were being applied too narrowly (they didn't cover signed char *,
for instance), and too broadly (they covered SomeTemplate<char[6]>) at
the same time.

Depends on D112708.

[shafik]

Nice catch!

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

21

It feels like writing out the decls by hand would have been quicker but perhaps less satisfying...

[labath]

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

21

:D

[labath]

ping

[labath]

@jingham, @shafik, do you see any issues with this patch?

[jingham]

The substance is fine, you kept the common cases fast and then added fallback for the others with the regex.

That test seems like an object lesson in what not to do in an inline test, but that wasn't your invention, so it's hard to complain too much.

[shafik]

Pavel, I apologize no one let you know earlier but it looks like this change broke TestFormattersBoolRefPtr.py test.

see: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/39449/

[shafik]

Because BOOL is actually a signed char

[shafik]

In D112709#3223098, @shafik wrote:

Because BOOL is actually a signed char

@aprantl corrected me, it can vary on some platforms it is signed char but on others it is bool.

[labath]

Thanks for the heads up. I've reverted the problematic parts in 31c7165a2b. I'll come with a proper fix soon-ish.

[aprantl]

it can vary on some platforms it is signed char but on others it is bool.

For those interested, Objective-C BOOL is a signed char on macOS and 32-bit iOS and bool on 64-bit iOS and derived platforms (watchOS & tvOS).

lldb/source/DataFormatters/FormatManager.cpp

string_array_flags.SetCascades(true)
       .SetHideItemNames(false);
 
   lldb::TypeSummaryImplSP string_format(
       new StringSummaryFormat(string_flags, "${var%s}"));
 
   lldb::TypeSummaryImplSP string_array_format(
       new StringSummaryFormat(string_array_flags, "${var%char[]}"));
 
-  RegularExpression any_size_char_arr(llvm::StringRef("char ?\\[[0-9]+\\]"));
+  RegularExpression any_size_char_arr(R"(^((un)?signed )?char ?\[[0-9]+\]$)");
 
   TypeCategoryImpl::SharedPointer sys_category_sp =
       GetCategory(m_system_category_name);
 
   sys_category_sp->GetTypeSummariesContainer()->Add(ConstString("char *"),
                                                     string_format);
   sys_category_sp->GetTypeSummariesContainer()->Add(
       ConstString("unsigned char *"), string_format);
+  sys_category_sp->GetTypeSummariesContainer()->Add(
+      ConstString("signed char *"), string_format);
+
   sys_category_sp->GetRegexTypeSummariesContainer()->Add(
       std::move(any_size_char_arr), string_array_format);
 
   lldb::TypeSummaryImplSP ostype_summary(
       new StringSummaryFormat(TypeSummaryImpl::Flags()
                                   .SetCascades(false)
                                   .SetSkipPointers(true)
                                   .SetSkipReferences(true)

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

 #include <string>
 #include <cstring>
 
 struct A {
   char data[4];
   char overflow[4];
 };
 
+#define MAKE_VARS3(c, v, s)                                                    \
+  c v s char c##v##s##chararray[] = #c #v #s "char";                           \
+  c v s char *c##v##s##charstar = c##v##s##chararray
+#define MAKE_VARS2(c, v)                                                       \
+  MAKE_VARS3(c, v, );                                                          \
+  MAKE_VARS3(c, v, signed);                                                    \
+  MAKE_VARS3(c, v, unsigned)
+#define MAKE_VARS(c)                                                           \
+  MAKE_VARS2(c, );                                                             \
+  MAKE_VARS2(c, volatile)
+
+MAKE_VARS();
+MAKE_VARS(const);
+
+template<typename T>
+struct S {
+  int x = 0;
+};
+S<char[5]> Schar5;
+S<char *> Scharstar;
+
 int main (int argc, char const *argv[])
 {
     A a, b, c;
     // Deliberately write past the end of data to test that the formatter stops
     // at the end of array.
     memcpy(a.data, "FOOBAR", 7);
     memcpy(b.data, "FO\0BAR", 7);
     memcpy(c.data, "F\0O\0AR", 7);
     std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); //%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables true"))
-    const char* constcharstar = stdstring.c_str();
+    const char *charwithtabs = stdstring.c_str();
     std::string longstring(
 "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
 " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
 " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
 " for science, or something"
       "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
       " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
       " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
       " for science, or something"
             "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
             " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
             " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
             " for science, or something"
       );
     const char* longconstcharstar = longstring.c_str();
     return 0;     //% if self.TraceOn(): self.runCmd('frame variable')
+    //%
     //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
-    //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+    //% self.expect_var_path('charwithtabs', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
     //% self.expect_var_path("a.data", summary='"FOOB"')
     //% self.expect_var_path("b.data", summary=r'"FO\0B"')
     //% self.expect_var_path("c.data", summary=r'"F\0O"')
     //%
+    //% for c in ["", "const"]:
+    //%   for v in ["", "volatile"]:
+    //%     for s in ["", "signed", "unsigned"]:
+    //%       summary = '"'+c+v+s+'char"'
+    //%       self.expect_var_path(c+v+s+"chararray", summary=summary)
+    //% # These should be printed normally
+    //%       self.expect_var_path(c+v+s+"charstar", summary=summary)
+    //% Schar5 = self.expect_var_path("Schar5",
+    //%     children=[ValueCheck(name="x", value="0")])
+    //% self.assertIsNone(Schar5.GetSummary())
+    //% Scharstar = self.expect_var_path("Scharstar",
+    //%     children=[ValueCheck(name="x", value="0")])
+    //% self.assertIsNone(Scharstar.GetSummary())
+    //%
     //% self.runCmd("setting set escape-non-printables false")
     //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
-    //% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
+    //% self.expect_var_path('charwithtabs', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
     //% self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
     //% self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
     // FIXME: make "b.data" and "c.data" work sanely
 }