[DebugInfo] Fix build when std::vector::iterator is a pointer

[orivej]

Summary

std::vector::iterator type may be a pointer, then iterator::value_type fails to compile since iterator is not a class, namespace, or enumeration.

[orivej]

Without this, the error (abbreviated) is:

lib/CodeGen/AsmPrinter/DwarfDebug.cpp: error: 'llvm::MapVector<>::const_iterator' (aka 'const std::pair<> *') is not a class, namespace, or enumeration

@hans, it would be useful to merge this into LLVM 7.

[dblaikie]

Looks good!

[dblaikie]

Looks good!

[orivej]

OK, please commit this into trunk first on my behalf!

[kristina]

Hi, I know it's a minor change but it should be easy to add a test case for it (a unit test would be appropriate here), especially considering this isn't currently causing issues in any of the current builds as far as I can tell. Also while I'm not insisting, a more-descriptive explanation of the change be nice as it currently, just a sentence or two, so it's more clear on the mailing lists. Thank you.

[orivej]

I have slightly extended the description.

I don't think this is possible to test with a unit test, as this requires an integration test with a c++ library those std::vector::iterator is a pointer. libc++ becomes such a library with this diff applied to include/vector:

     typedef typename __base::pointer                 pointer;
     typedef typename __base::const_pointer           const_pointer;
+    typedef typename __base::pointer                 iterator;
+    typedef typename __base::const_pointer           const_iterator;
-    typedef __wrap_iter<pointer>                     iterator;
-    typedef __wrap_iter<const_pointer>               const_iterator;

[kristina]

Alright, I'll just run this on a single buildbot and then land it. (I think it's possible to mock it, I can make a differential for a testcase later myself)

[hans]

In D52142#1236117, @orivej wrote:

Without this, the error (abbreviated) is:

lib/CodeGen/AsmPrinter/DwarfDebug.cpp: error: 'llvm::MapVector<>::const_iterator' (aka 'const std::pair<> *') is not a class, namespace, or enumeration

Do you have an example of a c++ std library where this happens?

@hans, it would be useful to merge this into LLVM 7.

I've put it on my list, but we're currently not planning another release candidate before final, so it may have to wait for 7.0.1.

[orivej]

Do you have an example of a c++ std library where this happens?

I'm using libc++ with the patch given above. I don't know about other libraries where iterators are pointers, only that this is allowed by the C++ standard.

we're currently not planning another release candidate before final, so it may have to wait for 7.0.1.

That's probably OK if all popular C++ libraries have iterator::value_type, although I'd be surprised if that's the case because its existence seems to be a non standardized convention.

[orivej]

Do you have an example of a c++ std library where this happens?

I have noticed that EASTL also has a pointer iterator: https://github.com/electronicarts/EASTL/blob/3.12.04/include/EASTL/vector.h#L20

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 /// Emit address ranges into the .debug_ranges section or into the DWARF v5
 /// .debug_rnglists section.
 void DwarfDebug::emitDebugRanges() {
   if (CUMap.empty())
     return;
 
   auto NoRangesPresent = [this]() {
     return llvm::all_of(
-        CUMap, [](const decltype(CUMap)::const_iterator::value_type &Pair) {
+        CUMap, [](const decltype(CUMap)::value_type &Pair) {
           return Pair.second->getRangeLists().empty();
         });
   };
 
   if (llvm::all_of(CUMap, [](const decltype(CUMap)::value_type &Pair) {
         return Pair.second->getCUNode()->isDebugDirectivesOnly();
       })) {
     assert(NoRangesPresent() && "No debug ranges expected.");