Page MenuHomePhabricator

Replace "nullptr-terminated" C-arrays of OptionValueEnumeration with safer llvm::ArrayRef
ClosedPublic

Authored by tatyana-krasnukha on Jul 6 2018, 3:40 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

This comment was removed by tatyana-krasnukha.

My only concern with this is are we adding a bunch of C++ global constructors now in each global "static PropertyDefinition ...". Before each "static PropertyDefinition" would just be in the const data section.

Thank you for pointing this problem. Though ArrayRef constructor with std::array parameter is declared as constexpr, it is not a constant expression actually because std::array::data() member function is not constexpr until C++17.
So, I reverted using std::array and added constexpr keyword to each static PropertyDefinition and OptionDefinition to ensure that constant initialization requirements remain satisfied.

Could you look at it again, please?

Looks fine to me. Would like one more OK before we proceed. Jim or Zach?

Looks fine to me, constexpr on the declaration guarantees it will be linker
initialized

This revision is now accepted and ready to land.Sep 25 2018, 9:45 AM
jingham accepted this revision.Sep 25 2018, 9:52 AM

Sure, this seems like a nice cleanup.

tatyana-krasnukha retitled this revision from Replace "nullptr-terminated" C-array with safer llvm::ArrayRef for PropertyDefinition and OptionDefinition. to Replace "nullptr-terminated" C-arrays of OptionValueEnumeration with safer llvm::ArrayRef.Sep 26 2018, 11:50 AM
This revision was automatically updated to reflect the committed changes.

This broke the apt.llvm.org CI on Debian Jessie and Ubuntu trusty (at least).
I reported https://bugs.llvm.org/show_bug.cgi?id=39131