[lldb] Change ObjectValueDictionary to use a StringMap

[bulbazord]

Summary

llvm has a structure for maps where the key's type is a string. Using
that also means that the keys for OptionValueDictionary don't stick
around forever in ConstString's StringPool (even after they are gone).

The only thing we lose here is ordering: iterating over the map where the keys
are ConstStrings guarantees that we iterate in alphabetical order.
StringMap makes no guarantees about the ordering when you iterate over
the entire map.

[jingham]

No reason for these strings to be in the ConstString pool, so that part is fine.

But if we're going to use this to store things like the env-vars, having no ordering guarantees when we dump them seems likely to be a bit annoying. If the order of element output in settings show env-vars dump changes around every time I add an element to it, that would be make the results hard to follow. Can we make the dumper for this option value pull the keys out, alphabetize the keys, then look the values up in that order? I don't think printing these objects is ever going to be performance critical.

lldb/source/Core/Disassembler.cpp

845

That's kind of answering a question with a question: Why isn't there an eTypeUInt32?

lldb/source/Interpreter/OptionValueDictionary.cpp

38–39

Does the llvm::StringMap dingus not support for (auto value : m_values) { ?

[bulbazord]

lldb/source/Core/Disassembler.cpp

845

I'm not sure, we'd have to ask Caroline who added this back in 2011. I added a comment to help me remember, but I suppose this comment may add more confusion than remove. I'll remove the note.

lldb/source/Interpreter/OptionValueDictionary.cpp

38–39

Oh, it probably does. Didn't even think about it, good catch.

[JDevlieghere]

+1 on making the output deterministic with a sort

[jingham]

I think something went wrong here. This was a patch for OptionValueDictionary, but it seems to have become a patch for OpenInExternalEditor?

[JDevlieghere]

That's my bad: I put the wrong differential URL in my commit (13dbc16b4d82b9adc98c0919873b2b59ccc46afd). The correct differential revision for this was https://reviews.llvm.org/D149472. Sorry Alex!

[bulbazord]

Sort output for dumping to be deterministic (based on alphabetical order like previous implementation)

[JDevlieghere]

LGTM

lldb/source/Core/Disassembler.cpp

804–805

I don't think you need this: key is a std::string that can be used directly.

[bulbazord]

Remove unneeded StringRef

lldb/include/lldb/Interpreter/OptionValueDictionary.h

 //===-- OptionValueDictionary.h ---------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
 #define LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
 
-#include <map>
-
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/lldb-private-types.h"
 
+#include "llvm/ADT/StringMap.h"
+
 namespace lldb_private {
 
 class OptionValueDictionary
     : public Cloneable<OptionValueDictionary, OptionValue> {
 public:
   OptionValueDictionary(uint32_t type_mask = UINT32_MAX,
                         OptionEnumValues enum_values = OptionEnumValues(),
                         bool raw_value_dump = true)
public:
   bool IsHomogenous() const {
     return ConvertTypeMaskToType(m_type_mask) != eTypeInvalid;
   }
 
   // Subclass specific functions
 
   size_t GetNumValues() const { return m_values.size(); }
 
-  lldb::OptionValueSP GetValueForKey(ConstString key) const;
+  lldb::OptionValueSP GetValueForKey(llvm::StringRef key) const;
 
   lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
                                   llvm::StringRef name, bool will_modify,
                                   Status &error) const override;
 
   Status SetSubValue(const ExecutionContext *exe_ctx, VarSetOperationType op,
                      llvm::StringRef name, llvm::StringRef value) override;
 
-  bool SetValueForKey(ConstString key,
-                      const lldb::OptionValueSP &value_sp,
+  bool SetValueForKey(llvm::StringRef key, const lldb::OptionValueSP &value_sp,
                       bool can_replace = true);
 
-  bool DeleteValueForKey(ConstString key);
+  bool DeleteValueForKey(llvm::StringRef key);
 
   size_t GetArgs(Args &args) const;
 
   Status SetArgs(const Args &args, VarSetOperationType op);
 
 protected:
-  typedef std::map<ConstString, lldb::OptionValueSP> collection;
   uint32_t m_type_mask;
   OptionEnumValues m_enum_values;
-  collection m_values;
+  llvm::StringMap<lldb::OptionValueSP> m_values;
   bool m_raw_value_dump;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H

lldb/source/Core/Disassembler.cpp

OptionValueSP Instruction::ReadArray(FILE *in_file, Stream *out_stream,
   return option_value_sp;
 }
 
 OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
   bool done = false;
   char buffer[1024];
 
   auto option_value_sp = std::make_shared<OptionValueDictionary>();
-  static ConstString encoding_key("data_encoding");
+  static constexpr llvm::StringLiteral encoding_key("data_encoding");
   OptionValue::Type data_type = OptionValue::eTypeInvalid;
 
   while (!done) {
     // Read the next line in the file
     if (!fgets(buffer, 1023, in_file)) {
       out_stream->Printf(
           "Instruction::ReadDictionary: Error reading file (fgets).\n");
       option_value_sp.reset();
if (!line.empty()) {
         key = matches[1].str();
         value = matches[2].str();
       } else {
         out_stream->Printf("Instruction::ReadDictionary: Failure executing "
                            "regular expression.\n");
         option_value_sp.reset();
         return option_value_sp;
       }
 
-      ConstString const_key(key.c_str());
       // Check value to see if it's the start of an array or dictionary.
 
       lldb::OptionValueSP value_sp;
       assert(value.empty() == false);
       assert(key.empty() == false);
 
       if (value[0] == '{') {
         assert(value.size() == 1);
         // value is a dictionary
if (!line.empty()) {
         value_sp->SetValueFromString(value);
       } else {
         size_t len = value.size();
         if ((value[0] == '"') && (value[len - 1] == '"'))
           value = value.substr(1, len - 2);
         value_sp = std::make_shared<OptionValueString>(value.c_str(), "");
       }
 
-      if (const_key == encoding_key) {
+      if (key == encoding_key) {
         // A 'data_encoding=..." is NOT a normal key-value pair; it is meta-data
-        // indicating the
-        // data type of an upcoming array (usually the next bit of data to be
-        // read in).
-        if (strcmp(value.c_str(), "uint32_t") == 0)
+        // indicating the data type of an upcoming array (usually the next bit
+        // of data to be read in).
+        if (llvm::StringRef(value) == "uint32_t")
           data_type = OptionValue::eTypeUInt64;
       } else
-        option_value_sp->GetAsDictionary()->SetValueForKey(const_key, value_sp,
+        option_value_sp->GetAsDictionary()->SetValueForKey(key, value_sp,
                                                            false);
     }
   }
 
   return option_value_sp;
 }
 
 bool Instruction::TestEmulation(Stream *out_stream, const char *file_name) {

lldb/source/Interpreter/OptionValueDictionary.cpp

if (dump_mask & eDumpOptionType) {
     else
       strm.Printf("(%s)", GetTypeAsCString());
   }
   if (dump_mask & eDumpOptionValue) {
     const bool one_line = dump_mask & eDumpOptionCommand;
     if (dump_mask & eDumpOptionType)
       strm.PutCString(" =");
 
-    collection::iterator pos, end = m_values.end();
-
     if (!one_line)
       strm.IndentMore();
 
-    for (pos = m_values.begin(); pos != end; ++pos) {
-      OptionValue *option_value = pos->second.get();
+    // m_values is not guaranteed to be sorted alphabetically, so for
+    // consistentcy we will sort them here before dumping
+    std::map<llvm::StringRef, OptionValue *> sorted_values;
+    for (const auto &value : m_values) {
+      sorted_values[value.first()] = value.second.get();
+    }
+    for (const auto &value : sorted_values) {
+      OptionValue *option_value = value.second;
 
       if (one_line)
         strm << ' ';
       else
         strm.EOL();
 
-      strm.Indent(pos->first.GetStringRef());
+      strm.Indent(value.first);
 
       const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
       switch (dict_type) {
       default:
       case eTypeArray:
       case eTypeDictionary:
       case eTypeProperties:
       case eTypeFileSpecList:
if (!one_line)
       strm.IndentLess();
   }
 }
 
 llvm::json::Value
 OptionValueDictionary::ToJSON(const ExecutionContext *exe_ctx) {
   llvm::json::Object dict;
   for (const auto &value : m_values) {
-    dict.try_emplace(value.first.GetCString(), value.second->ToJSON(exe_ctx));
+    dict.try_emplace(value.first(), value.second->ToJSON(exe_ctx));
   }
   return dict;
 }
 
 size_t OptionValueDictionary::GetArgs(Args &args) const {
   args.Clear();
-  collection::const_iterator pos, end = m_values.end();
-  for (pos = m_values.begin(); pos != end; ++pos) {
+  for (const auto &value : m_values) {
     StreamString strm;
-    strm.Printf("%s=", pos->first.GetCString());
-    pos->second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
+    strm.Printf("%s=", value.first().data());
+    value.second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
     args.AppendArgument(strm.GetString());
   }
   return args.GetArgumentCount();
 }
 
 Status OptionValueDictionary::SetArgs(const Args &args,
                                       VarSetOperationType op) {
   Status error;
for (const auto &entry : args) {
 
       if (m_type_mask == 1u << eTypeEnum) {
         auto enum_value =
             std::make_shared<OptionValueEnumeration>(m_enum_values, 0);
         error = enum_value->SetValueFromString(value);
         if (error.Fail())
           return error;
         m_value_was_set = true;
-        SetValueForKey(ConstString(key), enum_value, true);
+        SetValueForKey(key, enum_value, true);
       } else {
         lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(
             value.str().c_str(), m_type_mask, error));
         if (value_sp) {
           if (error.Fail())
             return error;
           m_value_was_set = true;
-          SetValueForKey(ConstString(key), value_sp, true);
+          SetValueForKey(key, value_sp, true);
         } else {
           error.SetErrorString("dictionaries that can contain multiple types "
                                "must subclass OptionValueArray");
         }
       }
     }
     break;
 
   case eVarSetOperationRemove:
     if (argc > 0) {
       for (size_t i = 0; i < argc; ++i) {
-        ConstString key(args.GetArgumentAtIndex(i));
+        llvm::StringRef key(args.GetArgumentAtIndex(i));
         if (!DeleteValueForKey(key)) {
           error.SetErrorStringWithFormat(
               "no value found named '%s', aborting remove operation",
-              key.GetCString());
+              key.data());
           break;
         }
       }
     } else {
       error.SetErrorString("remove operation takes one or more key arguments");
     }
     break;
 
OptionValueDictionary::GetSubValue(const ExecutionContext *exe_ctx,
   if (!key.consume_back(quote_char) || key.empty()) {
     error.SetErrorStringWithFormat("invalid value path '%s', "
       "key names must be formatted as ['<key>'] where <key> "
       "is a string that doesn't contain quotes and the quote"
       " char is optional", name.str().c_str());
     return nullptr;
   }
 
-  value_sp = GetValueForKey(ConstString(key));
+  value_sp = GetValueForKey(key);
   if (!value_sp) {
     error.SetErrorStringWithFormat(
       "dictionary does not contain a value for the key name '%s'",
       key.str().c_str());
     return nullptr;
   }
 
   if (sub_name.empty())
Status OptionValueDictionary::SetSubValue(const ExecutionContext *exe_ctx,
   else {
     if (error.AsCString() == nullptr)
       error.SetErrorStringWithFormat("invalid value path '%s'", name.str().c_str());
   }
   return error;
 }
 
 lldb::OptionValueSP
-OptionValueDictionary::GetValueForKey(ConstString key) const {
+OptionValueDictionary::GetValueForKey(llvm::StringRef key) const {
   lldb::OptionValueSP value_sp;
-  collection::const_iterator pos = m_values.find(key);
+  auto pos = m_values.find(key);
   if (pos != m_values.end())
     value_sp = pos->second;
   return value_sp;
 }
 
-bool OptionValueDictionary::SetValueForKey(ConstString key,
+bool OptionValueDictionary::SetValueForKey(llvm::StringRef key,
                                            const lldb::OptionValueSP &value_sp,
                                            bool can_replace) {
   // Make sure the value_sp object is allowed to contain values of the type
   // passed in...
   if (value_sp && (m_type_mask & value_sp->GetTypeAsMask())) {
     if (!can_replace) {
-      collection::const_iterator pos = m_values.find(key);
+      auto pos = m_values.find(key);
       if (pos != m_values.end())
         return false;
     }
     m_values[key] = value_sp;
     return true;
   }
   return false;
 }
 
-bool OptionValueDictionary::DeleteValueForKey(ConstString key) {
-  collection::iterator pos = m_values.find(key);
+bool OptionValueDictionary::DeleteValueForKey(llvm::StringRef key) {
+  auto pos = m_values.find(key);
   if (pos != m_values.end()) {
     m_values.erase(pos);
     return true;
   }
   return false;
 }
 
 OptionValueSP

lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h

 //===-- EmulateInstructionARM.h ---------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_INSTRUCTION_ARM_EMULATEINSTRUCTIONARM_H
 #define LLDB_SOURCE_PLUGINS_INSTRUCTION_ARM_EMULATEINSTRUCTIONARM_H
 
 #include "Plugins/Process/Utility/ARMDefines.h"
 #include "lldb/Core/EmulateInstruction.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 #include <optional>
 
 namespace lldb_private {
 
 // ITSession - Keep track of the IT Block progression.
 class ITSession {
 public:

lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp

 #include "EmulationStateARM.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Interpreter/OptionValueArray.h"
 #include "lldb/Interpreter/OptionValueDictionary.h"
 #include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Stream.h"
 
 #include "Plugins/Process/Utility/ARMDefines.h"
 #include "Plugins/Process/Utility/ARMUtils.h"
 #include "Utility/ARM_DWARF_Registers.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MathExtras.h"

 
 bool EmulateInstructionARM::TestEmulation(Stream *out_stream, ArchSpec &arch,
                                           OptionValueDictionary *test_data) {
   if (!test_data) {
     out_stream->Printf("TestEmulation: Missing test data.\n");
     return false;
   }
 
-  static ConstString opcode_key("opcode");
-  static ConstString before_key("before_state");
-  static ConstString after_key("after_state");
+  static constexpr llvm::StringLiteral opcode_key("opcode");
+  static constexpr llvm::StringLiteral before_key("before_state");
+  static constexpr llvm::StringLiteral after_key("after_state");
 
   OptionValueSP value_sp = test_data->GetValueForKey(opcode_key);
 
   uint32_t test_opcode;
   if ((value_sp.get() == nullptr) ||
       (value_sp->GetType() != OptionValue::eTypeUInt64)) {
     out_stream->Printf("TestEmulation: Error reading opcode from test file.\n");
     return false;

lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp

 }
 
 bool EmulationStateARM::LoadRegistersStateFromDictionary(
     OptionValueDictionary *reg_dict, char kind, int first_reg, int num) {
   StreamString sstr;
   for (int i = 0; i < num; ++i) {
     sstr.Clear();
     sstr.Printf("%c%d", kind, i);
-    OptionValueSP value_sp =
-        reg_dict->GetValueForKey(ConstString(sstr.GetString()));
+    OptionValueSP value_sp = reg_dict->GetValueForKey(sstr.GetString());
     if (value_sp.get() == nullptr)
       return false;
     uint64_t reg_value = value_sp->GetUInt64Value();
     StorePseudoRegisterValue(first_reg + i, reg_value);
   }
 
   return true;
 }
 
 bool EmulationStateARM::LoadStateFromDictionary(
     OptionValueDictionary *test_data) {
-  static ConstString memory_key("memory");
-  static ConstString registers_key("registers");
+  static constexpr llvm::StringLiteral memory_key("memory");
+  static constexpr llvm::StringLiteral registers_key("registers");
 
   if (!test_data)
     return false;
 
   OptionValueSP value_sp = test_data->GetValueForKey(memory_key);
 
   // Load memory, if present.
 
   if (value_sp.get() != nullptr) {
-    static ConstString address_key("address");
-    static ConstString data_key("data");
+    static constexpr llvm::StringLiteral address_key("address");
+    static constexpr llvm::StringLiteral data_key("data");
     uint64_t start_address = 0;
 
     OptionValueDictionary *mem_dict = value_sp->GetAsDictionary();
     value_sp = mem_dict->GetValueForKey(address_key);
     if (value_sp.get() == nullptr)
       return false;
     else
       start_address = value_sp->GetUInt64Value();
if (value_sp.get() == nullptr)
     return false;
 
   // Load General Registers
 
   OptionValueDictionary *reg_dict = value_sp->GetAsDictionary();
   if (!LoadRegistersStateFromDictionary(reg_dict, 'r', dwarf_r0, 16))
     return false;
 
-  static ConstString cpsr_name("cpsr");
+  static constexpr llvm::StringLiteral cpsr_name("cpsr");
   value_sp = reg_dict->GetValueForKey(cpsr_name);
   if (value_sp.get() == nullptr)
     return false;
   StorePseudoRegisterValue(dwarf_cpsr, value_sp->GetUInt64Value());
 
   // Load s/d Registers
   // To prevent you giving both types in a state and overwriting
   // one or the other, we'll expect to get either all S registers,
   // or all D registers. Not a mix of the two.
   bool found_s_registers =
       LoadRegistersStateFromDictionary(reg_dict, 's', dwarf_s0, 32);
   bool found_d_registers =
       LoadRegistersStateFromDictionary(reg_dict, 'd', dwarf_d0, 32);
 
   return found_s_registers != found_d_registers;
 }

lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp

 //===-- EmulateInstructionARM64.cpp ---------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "EmulateInstructionARM64.h"
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/Support/CheckedArithmetic.h"
 
 #include "Plugins/Process/Utility/ARMDefines.h"
 #include "Plugins/Process/Utility/ARMUtils.h"
 #include "Plugins/Process/Utility/lldb-arm64-register-enums.h"

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

size_t ObjectFilePECOFF::GetModuleSpecifications(
   const auto *map = GetGlobalPluginProperties().ModuleABIMap();
   if (map->GetNumValues() > 0) {
     // Step 1: Try with the exact file name.
     auto name = file.GetLastPathComponent();
     module_env_option = map->GetValueForKey(name);
     if (!module_env_option) {
       // Step 2: Try with the file name in lowercase.
       auto name_lower = name.GetStringRef().lower();
-      module_env_option =
-          map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
+      module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
     }
     if (!module_env_option) {
       // Step 3: Try with the file name with ".debug" suffix stripped.
       auto name_stripped = name.GetStringRef();
       if (name_stripped.consume_back_insensitive(".debug")) {
-        module_env_option = map->GetValueForKey(ConstString(name_stripped));
+        module_env_option = map->GetValueForKey(name_stripped);
         if (!module_env_option) {
           // Step 4: Try with the file name in lowercase with ".debug" suffix
           // stripped.
           auto name_lower = name_stripped.lower();
-          module_env_option =
-              map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
+          module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
         }
       }
     }
   }
   llvm::Triple::EnvironmentType env;
   if (module_env_option)
     env =
         (llvm::Triple::EnvironmentType)module_env_option->GetEnumerationValue();

lldb/test/API/commands/settings/TestSettings.py

def test_set_interpreter_repeat_prev_command(self):
         res = lldb.SBCommandReturnObject()
         ci.HandleCommand('breakpoint set -n main', res)
         self.assertTrue(res.Succeeded(), "Command failed.")
         ci.HandleCommand('', res)
         self.assertTrue(res.Succeeded(), "Empty command failed.")
         self.assertEqual(self.dbg.GetSelectedTarget().GetNumBreakpoints(), 1)
 
     def test_append_target_env_vars(self):
-        """Test that 'append target.run-args' works."""
+        """Test that 'append target.env-vars' works."""
         # Append the env-vars.
         self.runCmd('settings append target.env-vars MY_ENV_VAR=YES')
         # And add hooks to restore the settings during tearDown().
         self.addTearDownHook(
             lambda: self.runCmd("settings clear target.env-vars"))
 
         # Check it immediately!
         self.expect('settings show target.env-vars',
def do_test_run_args_and_env_vars(self, use_launchsimple):
         target = self.dbg.GetTargetAtIndex(0)
         launch_info = target.GetLaunchInfo()
         found_env_var = False
         for i in range(0, launch_info.GetNumEnvironmentEntries()):
             if launch_info.GetEnvironmentEntryAtIndex(i) == "MY_ENV_VAR=YES":
                 found_env_var = True
                 break
         self.assertTrue(found_env_var,
-                        "MY_ENV_VAR was not set in LunchInfo object")
+                        "MY_ENV_VAR was not set in LaunchInfo object")
 
         self.assertEqual(launch_info.GetNumArguments(), 3)
         self.assertEqual(launch_info.GetArgumentAtIndex(0), "A")
         self.assertEqual(launch_info.GetArgumentAtIndex(1), "B")
         self.assertEqual(launch_info.GetArgumentAtIndex(2), "C")
 
         self.expect(
             'target show-launch-environment',

lldb/unittests/Interpreter/TestOptionValue.cpp

TEST(TestProperties, DeepCopy) {
 
   // Test the first child.
   auto dict_copy_ptr = props_copy_ptr->GetDictionary();
   ASSERT_TRUE(dict_copy_ptr);
   ASSERT_EQ(dict_copy_ptr->GetParent(), copy_sp);
   ASSERT_TRUE(dict_copy_ptr->OptionWasSet());
   ASSERT_EQ(dict_copy_ptr->GetNumValues(), 2U);
 
-  auto value_ptr = dict_copy_ptr->GetValueForKey(ConstString("A"));
+  auto value_ptr = dict_copy_ptr->GetValueForKey("A");
   ASSERT_TRUE(value_ptr);
   ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
   ASSERT_EQ(value_ptr->GetUInt64Value(), 1U);
 
-  value_ptr = dict_copy_ptr->GetValueForKey(ConstString("B"));
+  value_ptr = dict_copy_ptr->GetValueForKey("B");
   ASSERT_TRUE(value_ptr);
   ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
   ASSERT_EQ(value_ptr->GetUInt64Value(), 2U);
 
   // Test the second child.
   auto file_list_copy_ptr = props_copy_ptr->GetFileList();
   ASSERT_TRUE(file_list_copy_ptr);
   ASSERT_EQ(file_list_copy_ptr->GetParent(), copy_sp);