Fix DataExtractor symbol conflict

[emrekultursay]

Summary

There are two DataExtractors in scope: one from the llvm namespace
and one from the lldb_private namespace. Some Microsoft Visual C++
compilers (I tested with MSVC 14.23 specifically) cannot handle this
situation, and generate ambiguous symbol errors. This change fixes
this compile error.

[shafik]

LGTM

[emrekultursay]

Hi shafik, can you also submit it? I don't have submit permission. Thanks.

[JDevlieghere]

I think the better solution here is to get rid of the using namespace llvm; in the implementation file instead.

[shafik]

In D120718#3352450, @JDevlieghere wrote:

I think the better solution here is to get rid of the using namespace llvm; in the implementation file instead.

That is a good point. I did a quick look at the file DataFileCache.h and none of the files it includes directly use using namespace llvm;.

It looks like across llvm-project we have 54 headers that use using namespace llvm; I thought it was a lot more.

I personally dislike it even in .cpp files using llvm:: or lldb_private:: does not feel like an undue burden. Although the count for .cpp files is 4000+ 😱

[labath]

In D120718#3352635, @shafik wrote:

In D120718#3352450, @JDevlieghere wrote:

I think the better solution here is to get rid of the using namespace llvm; in the implementation file instead.

That is a good point. I did a quick look at the file DataFileCache.h and none of the files it includes directly use using namespace llvm;.

It looks like across llvm-project we have 54 headers that use using namespace llvm; I thought it was a lot more.

It looks like at least some of those are inside functions, which is kind of OK. But what's worse, some of those are not inside functions, which is 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱. using directives in headers completely mangle the symbol lookups in any file which includes them.

[emrekultursay]

Removed "using namespace llvm" from the implementation file
and qualified all associated references with "llvm::".

[emrekultursay]

In D120718#3352450, @JDevlieghere wrote:

I think the better solution here is to get rid of the using namespace llvm; in the implementation file instead.

Done. PTAL.

[JDevlieghere]

Thanks for cleaning up the implementation file. LGMT module removing the added lldb_private:: which I believe we no longer need.

lldb/include/lldb/Core/DataFileCache.h

164–165

I assume the header was never a problem and you did this to match the signature in the implementation file (which no longer needs the namespace qualifier without the using namespace llvm), unless one of the headers included here (transitively) includes a header that has the "😱 😱 😱" top level using namespace llvm;?

[emrekultursay]

Reverted the changes to the header file.

[emrekultursay]

Thanks. PTAL.

[JDevlieghere]

Thank you! LGTM

lldb/include/lldb/Core/DataFileCache.h

struct CacheSignature {
   ///   The decoder object that references the serialized data.
   ///
   /// \param offset_ptr
   ///   A pointer that contains the offset from which the data will be decoded
   ///   from that gets updated as data gets decoded.
   ///
   /// \return
   ///   True if the signature was successfully decoded, false otherwise.
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+              lldb::offset_t *offset_ptr);
 };
 
 /// Many cache files require string tables to store data efficiently. This
 /// class helps create string tables.
 class ConstStringTable {
 public:
   ConstStringTable() = default;
   /// Add a string into the string table.

 /// class helps give out strings from a string table that was read from a
 /// cache file.
 class StringTableReader {
 public:
   StringTableReader() = default;
 
   llvm::StringRef Get(uint32_t offset) const;
 
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+              lldb::offset_t *offset_ptr);
 
 protected:
   /// All of the strings in the string table are contained in m_data.
   llvm::StringRef m_data;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_CORE_DATAFILECACHE_H

lldb/source/Core/DataFileCache.cpp

bool CacheSignature::Encode(DataEncoder &encoder) {
   if (m_obj_mod_time.hasValue()) {
     encoder.AppendU8(eSignatureObjectModTime);
     encoder.AppendU32(*m_obj_mod_time);
   }
   encoder.AppendU8(eSignatureEnd);
   return true;
 }
 
-bool CacheSignature::Decode(const DataExtractor &data,
+bool CacheSignature::Decode(const lldb_private::DataExtractor &data,
                             lldb::offset_t *offset_ptr) {
   Clear();
   while (uint8_t sig_encoding = data.GetU8(offset_ptr)) {
     switch (sig_encoding) {
     case eSignatureUUID: {
       const uint8_t length = data.GetU8(offset_ptr);
       const uint8_t *bytes = (const uint8_t *)data.GetData(offset_ptr, length);
       if (bytes != nullptr && length > 0)
for (auto s: m_strings) {
     // Append the C string into the encoder
     encoder.AppendCString(s.GetStringRef());
   }
   // Fixup the string table length.
   encoder.PutU32(length_offset, encoder.GetByteSize() - strtab_offset);
   return true;
 }
 
-bool StringTableReader::Decode(const DataExtractor &data,
+bool StringTableReader::Decode(const lldb_private::DataExtractor &data,
                                lldb::offset_t *offset_ptr) {
   llvm::StringRef identifier((const char *)data.GetData(offset_ptr, 4), 4);
   if (identifier != kStringTableIdentifier)
     return false;
   const uint32_t length = data.GetU32(offset_ptr);
   // We always have at least one byte for the empty string at offset zero.
   if (length == 0)
     return false;