[TargetLoweringObjectFileImpl] Use llvm::transform

[orivej]

Summary

Fixes a build issue with libc++ configured with _LIBCPP_RAW_ITERATORS (ADL not effective)

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1602:3: error: no matching function for call to 'transform'
  transform(HexString.begin(), HexString.end(), HexString.begin(), tolower);
  ^~~~~~~~~

[MaskRay]

Which compiler does you use? The file misses <algorithm>

You can also use llvm::transform

[orivej]

I have reproduced the error with Clang 7 and 10. I am compiling with a slightly customized version of a fresh libcxx.

The interesting question is why you or CI can build llvm without this error. What brings std::transform into scope?

[MaskRay]

In D80475#2052294, @orivej wrote:

I have reproduced the error with Clang 7 and 10. I am compiling with a slightly customized version of a fresh libcxx.

The interesting question is why you or CI can build llvm without this error. What brings std::transform into scope?

I cannot reproduce with clang 7. My command line:

/home/ray/ccls/build/clang+llvm-7.0.0-x86_64-linux-gnu-ubuntu-16.04/bin/clang -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/CodeGen -I/home/ray/llvm/llvm/lib/CodeGen -I/usr/include/libxml2 -Iinclude -I/home/ray/llvm/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -g -fPIC -fno-exceptions -fno-rtti -gsplit-dwarf -std=c++14 -c /home/ray/llvm/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp -fsyntax-only

You can delete TargetLoweringObjectFileImpl.cpp.o and run ninja -v llc to get the clang command line. By checking -H, I find that <algorithm> is included via llvm/ADT/Hashing.h

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1749

Use llvm::transform to avoid future ADL gotcha.

[orivej]

My C++ library does not allow even this:

#include <algorithm>
#include <string>

int main() {
    std::string s("hello");
    transform(s.begin(), s.end(), s.begin(), [](char c) -> char { return c; });
}

main.cpp:7:5: error: use of undeclared identifier 'transform'; did you mean 'std::transform'?
    transform(s.begin(), s.end(), s.begin(), [](char c) -> char { return c; });
    ^~~~~~~~~
    std::transform

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1749

Are you sure? Changing the signature like this makes the name resolution argument independent, since llvm::transform is in scope due to using namespace llvm;. Nothing else in this file qualifies llvm::.

[MaskRay]

In D80475#2052429, @orivej wrote:

My C++ library does not allow even this:

Can you double check?? The code below even works with clang 3.4 and GCC 4.7, which are both very old (lower than LLVM's build requirement).
Probably not libstdc++ or libc++.. Sigh

#include <algorithm>
#include <string>

int main() {
    std::string s("hello");
    transform(s.begin(), s.end(), s.begin(), [](char c) -> char { return c; });
}

main.cpp:7:5: error: use of undeclared identifier 'transform'; did you mean 'std::transform'?
    transform(s.begin(), s.end(), s.begin(), [](char c) -> char { return c; });
    ^~~~~~~~~
    std::transform

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1749

We prefer a qualified name. This is similar to llvm::sort. Unqualified name lookups can cause ADL. STLExtras.h defines names the same as in std::. A misuse may easily pick up an unintended overload in std::

[orivej]

I suppose that the newer versions are generally more restrictive than the older ones. Old libstdc++ used to bring std names into the global scope.

I have found what causes the build failure on my part: it is that string.begin() and end() return a char *, not an iterator class in the std namespace: https://github.com/catboost/catboost/blob/08a65a2e/contrib/libs/cxxsupp/libcxx/include/string#L697

Could you please commit on my behalf?

[orivej]

The char * iterator feature could have been enabled in libcxx via _LIBCPP_RAW_ITERATORS config definition until https://reviews.llvm.org/D79323.

[MaskRay]

Will do!

[MaskRay]

In D80475#2052869, @orivej wrote:

I suppose that the newer versions are generally more restrictive than the older ones. Old libstdc++ used to bring std names into the global scope.

If string::begin returns a std::string::iterator (everywhere else in the world...), names in std:: are looked up ("The innermost enclosing namespaces in the classes added to the set") when you call an unqualified transform.

If string::begin returns const char *, ADL does not fire and std:: is not looked up.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

const MCExpr *TargetLoweringObjectFileCOFF::lowerRelativeReference(
   return MCSymbolRefExpr::create(TM.getSymbol(LHS),
                                  MCSymbolRefExpr::VK_COFF_IMGREL32,
                                  getContext());
 }
 
 static std::string APIntToHexString(const APInt &AI) {
   unsigned Width = (AI.getBitWidth() / 8) * 2;
   std::string HexString = AI.toString(16, /*Signed=*/false);
-  transform(HexString.begin(), HexString.end(), HexString.begin(), tolower);
+  std::transform(HexString.begin(), HexString.end(), HexString.begin(), tolower);
   unsigned Size = HexString.size();
   assert(Width >= Size && "hex string is too large!");
   HexString.insert(HexString.begin(), Width - Size, '0');
 
   return HexString;
 }
 
 static std::string scalarConstantToHexString(const Constant *C) {