[Demangle] Add support for multiple identifiers in D qualified names

[dblaikie]

Looks good, thanks!

llvm/lib/Demangle/DLangDemangle.cpp

200–210

Might be more legible to write this as:

bool First = true;
do {
  if (First)
    *Demangled << '.';
  First = false;
  Mangled = parseIdentifier...

Rather than incrementing N which is rather non-descript.

[ljmf00]

llvm/lib/Demangle/DLangDemangle.cpp

200–210

Yeah, that's is better and more performant, although I think you meant the other way around although. The intention here is to add dot on every iteration other than the first.

[dblaikie]

llvm/lib/Demangle/DLangDemangle.cpp

200–210

Right right!

[ljmf00]

Done @dblaikie, can you review?

[dblaikie]

In D114305#3154699, @ljmf00 wrote:

Done @dblaikie, can you review?

Oh, generally if something's "Approved with suggestions" it's fine to submit it if you think you've applied the suggestions. So, this is fine, but I'll mark it as approved again.

llvm/lib/Demangle/DLangDemangle.cpp

201–205

I'd probably make this "First" rather than "NotFirst" - might be a bit clearer/more direct.

[ljmf00]

In D114305#3154851, @dblaikie wrote:

In D114305#3154699, @ljmf00 wrote:

Done @dblaikie, can you review?

Oh, generally if something's "Approved with suggestions" it's fine to submit it if you think you've applied the suggestions. So, this is fine, but I'll mark it as approved again.

Yeah, I don't have permission to land it, but thanks for letting me know that. A bit offtopic, but what are the rules to apply for commit access? I believe it is early to request that, although.

EDIT: It is not landable yet, as it has dependencies not yet approved

[dblaikie]

In D114305#3155925, @ljmf00 wrote:

In D114305#3154851, @dblaikie wrote:

In D114305#3154699, @ljmf00 wrote:

Done @dblaikie, can you review?

Oh, generally if something's "Approved with suggestions" it's fine to submit it if you think you've applied the suggestions. So, this is fine, but I'll mark it as approved again.

Yeah, I don't have permission to land it, but thanks for letting me know that. A bit offtopic, but what are the rules to apply for commit access? I believe it is early to request that, although.

Generally once you've had a few patches approved, you can request pre-review commit access ( https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access - though the documentation might be a bit out of date, I'd probably still email Chris and he can bounce it to whomever & maybe update the docs) & then you sort of play it by ear - anything that you have a pretty good estimate will be trivially approved, you can eventually commit without pre-commit review (& be responsive to post-commit review).

EDIT: It is not landable yet, as it has dependencies not yet approved

Ah, cool - just ping here if you need me to commit it when it comes time.

[mehdi_amini]

llvm/lib/Demangle/DLangDemangle.cpp

149

This breaks the windows bots: C:\buildbot\mlir-x64-windows-ninja\llvm-project\llvm\lib\Demangle\DLangDemangle.cpp(125): error C2039: 'isdigit': is not a member of 'std'

[ljmf00]

llvm/lib/Demangle/DLangDemangle.cpp

149

This got fixed on https://github.com/llvm/llvm-project/commit/bd4c6a476fd037fb07a1c484f75d93ee40713d3d

llvm/lib/Demangle/DLangDemangle.cpp

private:
   ///
   /// \return The remaining string on success or nullptr on failure.
   ///
   /// \note A result larger than UINT_MAX is considered a failure.
   ///
   /// \see https://dlang.org/spec/abi.html#Number .
   const char *decodeNumber(const char *Mangled, unsigned long *Ret);
 
+  /// Check whether it is the beginning of a symbol name.
+  ///
+  /// \param Mangled string to extract the symbol name.
+  ///
+  /// \return true on success, false otherwise.
+  ///
+  /// \see https://dlang.org/spec/abi.html#SymbolName .
+  bool isSymbolName(const char *Mangled);
+
   /// Extract and demangle an identifier from a given mangled symbol append it
   /// to the output string.
   ///
   /// \param Demangled Output buffer to write the demangled name.
   /// \param Mangled Mangled symbol to be demangled.
   ///
   /// \return The remaining string on success or nullptr on failure.
   ///
const char *Demangler::decodeNumber(const char *Mangled, unsigned long *Ret) {
 
   if (*Mangled == '\0')
     return nullptr;
 
   *Ret = Val;
   return Mangled;
 }
 
+bool Demangler::isSymbolName(const char *Mangled) {
+  if (std::isdigit(*Mangled))
+    return true;
+
+  // TODO: Handle symbol back references and template instances.
+  return false;
+}
+
 const char *Demangler::parseMangle(OutputBuffer *Demangled,
                                    const char *Mangled) {
   // A D mangled symbol is comprised of both scope and type information.
   //    MangleName:
   //        _D QualifiedName Type
   //        _D QualifiedName Z
   //        ^
   // The caller should have guaranteed that the start pointer is at the
const char *Demangler::parseQualified(OutputBuffer *Demangled,
   //        ^
   //    SymbolFunctionName:
   //        SymbolName
   //        SymbolName TypeFunctionNoReturn
   //        SymbolName M TypeFunctionNoReturn
   //        SymbolName M TypeModifiers TypeFunctionNoReturn
   // The start pointer should be at the above location.
 
-  // TODO: Parse multiple identifiers
+  // Whether it has more than one symbol
+  size_t NotFirst = false;
+  do {
+    if (NotFirst)
+      *Demangled << '.';
+    NotFirst = true;
+
+    Mangled = parseIdentifier(Demangled, Mangled);
+
+  } while (Mangled && isSymbolName(Mangled));
 
-  return parseIdentifier(Demangled, Mangled);
+  return Mangled;
 }
 
 const char *Demangler::parseIdentifier(OutputBuffer *Demangled,
                                        const char *Mangled) {
   unsigned long Len;
 
   if (Mangled == nullptr || *Mangled == '\0')
     return nullptr;

llvm/unittests/Demangle/DLangDemangleTest.cpp

struct DLangDemangleTestFixture
 
   void TearDown() override { std::free(Demangled); }
 };
 
 TEST_P(DLangDemangleTestFixture, DLangDemangleTest) {
   EXPECT_STREQ(Demangled, GetParam().second);
 }
 
-INSTANTIATE_TEST_SUITE_P(DLangDemangleTest, DLangDemangleTestFixture,
-                         testing::Values(std::make_pair("_Dmain", "D main"),
-                                         std::make_pair(nullptr, nullptr),
-                                         std::make_pair("_Z", nullptr),
-                                         std::make_pair("_DDD", nullptr),
-                                         std::make_pair("_D88", nullptr),
-                                         std::make_pair("_D8demangleZ",
-                                                        "demangle")));
+INSTANTIATE_TEST_SUITE_P(
+    DLangDemangleTest, DLangDemangleTestFixture,
+    testing::Values(
+        std::make_pair("_Dmain", "D main"), std::make_pair(nullptr, nullptr),
+        std::make_pair("_Z", nullptr), std::make_pair("_DDD", nullptr),
+        std::make_pair("_D8demangleZ", "demangle"),
+        std::make_pair("_D8demangle4testZ", "demangle.test"),
+        std::make_pair("_D8demangle4test5test2Z", "demangle.test.test2")));