<div><br></div><div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 6:51 AM Aleksandr Urakov via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">aleksandr.urakov added a comment.<br>
<br>
This change looks reasonable to me for solving the problem with the current `LF_NESTTYPE` approach. But I'm not sure... Now we all the same need to analyze mangled names and make a decision based on this. Does the current `LF_NESTTYPE` approach still has advantages over the "parse-scope" approach? I'm afraid that we will have to fully analyze a mangled name in the future for scoped types, so if the `LF_NESTTYPE` approach doesn't have a significant advantages over the "parse-scope" approach, do we really need to support them both?</blockquote><div dir="auto">I think we need to support both.  Certain pieces of information are not represented in the mangling at all, so if we rely purely on the mangling we will never be able to perfectly reconstruct the DeclContext hierarchy.  So I think each approach by itself would be imperfect, but combined it will be very good.</div><div dir="auto"><br></div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
<br>
<br>
================<br>
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:560-576<br>
+  // If it's an inner definition, then treat whatever name we have here as a<br>
+  // single component of a mangled name.  So we can inject it into the parent's<br>
+  // mangled name to see if it matches.<br>
+  CVTagRecord child = CVTagRecord::create(tpi.getType(Record.Type));<br>
+  std::string qname = parent.asTag().getUniqueName();<br>
+  if (qname.size() < 4 || child.asTag().getUniqueName().size() < 4)<br>
+    return llvm::None;<br>
----------------<br>
May be it would be a good idea to make the demangler (or to implement a mangler) more flexible to hide these details there? I do not mean to do it exactly in this commit, but what do you think about this at all?<br>
<br>
<br>
================<br>
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:881-887<br>
+    // If there is no parent in the debug info, but some of the scopes have<br>
+    // template params, then this is a case of bad debug info.  See, for<br>
+    // example, <a href="http://llvm.org/pr39607" rel="noreferrer" target="_blank">llvm.org/pr39607</a>.  We don't want to create an ambiguity between<br>
+    // a NamespaceDecl and a CXXRecordDecl, so instead we create a class at<br>
+    // global scope with the fully qualified name.<br>
+    if (AnyScopesHaveTemplateParams(scopes))<br>
+      return {context, record.Name};<br>
----------------<br>
Is this a clang only bug, or MSVC also does so? I do not have anything against this solution for now, I just mean, may be we'll replace it with an `assert` when the bug will be fixed?</blockquote><div dir="auto"><br></div><div dir="auto">Clang only.  But I don’t think we can ever replace it with an assert, debug info is basically user input, so we have to be able to handle every manner of malformed input.  In fact, when this bug is fixed in clang, i will probably try to keep a test case around that manually generates the bad debug info using llvm-mc or something, just to make sure it will not break.  Because, for example, someone could be debugging a program that was built with the buggy compiler version.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div>