This is an archive of the discontinued LLVM Phabricator instance.

Move from llvm::makeArrayRef to ArrayRef deduction guides - llvm/ part
ClosedPublic

Authored by serge-sans-paille on Jan 3 2023, 11:39 PM.

Details

Summary

Use deduction guides instead of helper functions.

The only non-automatic changes have been:

  1. ArrayRef(some_uint8_pointer, 0) needs to be changed into ArrayRef(some_uint8_pointer, (size_t)0) to avoid an ambiguous call with ArrayRef((uint8_t*), (uint8_t*))
  2. CVSymbol sym(makeArrayRef(symStorage)); needed to be rewritten as CVSymbol sym{ArrayRef(symStorage)}; otherwise the compiler is confused and thinks we have a (bad) function prototype. There was a few similar situation across the codebase.
  3. ADL doesn't seem to work the same for deduction-guides and functions, so at some point the llvm namespace must be explicitly stated.
  4. The "reference mode" of makeArrayRef(ArrayRef<T> &) that acts as no-op is not supported (a constructor cannot achieve that).

This is a follow-up to https://reviews.llvm.org/D140896 that introduced the deduction guides.

Diff Detail

Event Timeline

serge-sans-paille requested review of this revision.Jan 3 2023, 11:39 PM
mehdi_amini added inline comments.Jan 4 2023, 12:49 PM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
347

It seems that in such cases we can remove the explicit ArrayRef(...) around Table here right ?

(I wonder if a clang-tidy check could detect this maybe?)

dblaikie added inline comments.Jan 4 2023, 2:36 PM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
347

Perhaps these would be considered pre-existing issues, since they could've been written the way you are suggesting even without CTAD for ArrayRef - but, yeah, if they can be simply cleaned up while doing this transformation, it'd be a perk/nice to have.

Removed some former call to makeArrayRef when they were no longer needed.

mehdi_amini added inline comments.Jan 5 2023, 3:01 AM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
347

You're right, I thought that these simplification were enabled by CTAD but it seems the makeArrayRef were always spurious!

mehdi_amini accepted this revision.Jan 5 2023, 3:01 AM
This revision is now accepted and ready to land.Jan 5 2023, 3:01 AM
This revision was landed with ongoing or failed builds.Jan 5 2023, 5:13 AM
This revision was automatically updated to reflect the committed changes.