[PATCH] D146492: Add new printNumber() for size_t

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

My understanding is that `size_t` isn't a fundamental type, and it shouldn't need to have its own overload. @junhee-yoo, to help us understand why the ambiguity occurs, I think we need to know what type your system's `size_t` actually is. For example, if I go to the definition of `size_t` on my Windows host, I find that it is defined as: `typedef unsigned __int64 size_t;`, which is synonymous with `unsigned long long`, the same type as what `uint64_t` is a typedef of. Before blindly adding a `size_t` overload, I think we need to find out what your `size_t` is actually defined as on your system. You'll need to dig through your system headers to find where the size_t type is defined (there may be a more effective way of doing this through an IDE, but I'm not familiar with Mac systems, so can't advise you on this).

As you have observed, your change actually causes build failures on the pre-merge builds. If you take a look at the Windows report (https://buildkite.com/llvm-project/premerge-checks/builds/142201#01870203-5f63-451f-9b53-30b0614ebda9), you will see that there's a compilation failure as follows:

  C:\ws\w5\llvm-project\premerge-checks\llvm\include\llvm/Support/ScopedPrinter.h(205): error C2535: 'void llvm::ScopedPrinter::printNumber(llvm::StringRef,size_t)': member function already defined or declared
  C:\ws\w5\llvm-project\premerge-checks\llvm\include\llvm/Support/ScopedPrinter.h(201): note: see declaration of 'llvm::ScopedPrinter::printNumber'

You'll observe that the two lines referenced refer to the `size_t` and `uint64_t` overloads. This is because the two have the same fundamental type and therefore it's equivalent to you writing 'void llvm::ScopedPrinter::printNumber(llvm::StringRef,unsigned long long)' twice. The Linux pre-merge check produces essentially the same error for the same reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146492/new/

https://reviews.llvm.org/D146492