[Symbol] Change ClangASTContext::GetCXXClassName return type

[xiaobai]

Summary

Instead of filling out a std::string and returning a bool to indicate
success, returning a std::string directly and testing to see if it's
empty seems like a cleaner solution overall.

[teemperor]

Could this return an Optional<std::string> or Expected<std::string>? It's not clear from a std::string return value that the method can fail and it will return an empty string on error.

[xiaobai]

In D69641#1727787, @teemperor wrote:

Could this return an Optional<std::string> or Expected<std::string>? It's not clear from a std::string return value that the method can fail and it will return an empty string on error.

That sounds like a better idea to me. Will do that instead.

[xiaobai]

Use llvm::Optional

[teemperor]

LGTM, thanks!

[labath]

awesome

lldb/include/lldb/Symbol/ClangASTContext.h

public:
   bool IsTypedefType(lldb::opaque_compiler_type_t type) override;
 
   bool IsVoidType(lldb::opaque_compiler_type_t type) override;
 
   bool CanPassInRegisters(const CompilerType &type) override;
 
   bool SupportsLanguage(lldb::LanguageType language) override;
 
-  static bool GetCXXClassName(const CompilerType &type,
-                              std::string &class_name);
+  static llvm::Optional<std::string> GetCXXClassName(const CompilerType &type);
 
   // Type Completion
 
   bool GetCompleteType(lldb::opaque_compiler_type_t type) override;
 
   // Accessors
 
   ConstString GetTypeName(lldb::opaque_compiler_type_t type) override;

lldb/source/Core/ValueObject.cpp

bool ValueObject::HasSyntheticValue() {
   return m_synthetic_value != nullptr;
 }
 
 bool ValueObject::GetBaseClassPath(Stream &s) {
   if (IsBaseClass()) {
     bool parent_had_base_class =
         GetParent() && GetParent()->GetBaseClassPath(s);
     CompilerType compiler_type = GetCompilerType();
-    std::string cxx_class_name;
-    bool this_had_base_class =
-        ClangASTContext::GetCXXClassName(compiler_type, cxx_class_name);
-    if (this_had_base_class) {
+    llvm::Optional<std::string> cxx_class_name =
+        ClangASTContext::GetCXXClassName(compiler_type);
+    if (cxx_class_name) {
       if (parent_had_base_class)
         s.PutCString("::");
-      s.PutCString(cxx_class_name);
+      s.PutCString(cxx_class_name.getValue());
     }
-    return parent_had_base_class || this_had_base_class;
+    return parent_had_base_class || cxx_class_name;
   }
   return false;
 }
 
 ValueObject *ValueObject::GetNonBaseClassParent() {
   if (GetParent()) {
     if (GetParent()->IsBaseClass())
       return GetParent()->GetNonBaseClassParent();

lldb/source/Symbol/ClangASTContext.cpp

bool ClangASTContext::CanPassInRegisters(const CompilerType &type) {
   }
   return false;
 }
 
 bool ClangASTContext::SupportsLanguage(lldb::LanguageType language) {
   return ClangASTContextSupportsLanguage(language);
 }
 
-bool ClangASTContext::GetCXXClassName(const CompilerType &type,
-                                      std::string &class_name) {
-  if (type) {
-    clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
-    if (!qual_type.isNull()) {
-      clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-      if (cxx_record_decl) {
-        class_name.assign(cxx_record_decl->getIdentifier()->getNameStart());
-        return true;
-      }
-    }
-  }
-  class_name.clear();
-  return false;
+Optional<std::string>
+ClangASTContext::GetCXXClassName(const CompilerType &type) {
+  if (!type)
+    return llvm::None;
+
+  if (qual_type.isNull())
+    return llvm::None;
+
+  if (!cxx_record_decl)
+    return llvm::None;
+
+  return std::string(cxx_record_decl->getIdentifier()->getNameStart());
 }
 
 bool ClangASTContext::IsCXXClassType(const CompilerType &type) {
   if (!type)
     return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
   return !qual_type.isNull() && qual_type->getAsCXXRecordDecl() != nullptr;