Fix compilation for folly::StringPiece args (regression from v10)

[mhx]

When building fbthrift on Fedora Rawhide (which bundles fmt v11), the following assertion fails:

/usr/include/fmt/base.h:1612:17: error: static assertion failed: Mixing character types is disallowed.
 1612 |   static_assert(formattable_char, "Mixing character types is disallowed.");
      |                 ^~~~~~~~~~~~~~~~
/usr/include/fmt/base.h:1612:17: note: 'formattable_char' evaluates to false

This happens as it's trying to format a folly::StringPiece argument:

/usr/include/fmt/base.h: In instantiation of 'constexpr fmt::v11::detail::value<Context> fmt::v11::detail::make_arg(T&) [with bool PACKED = true; Context = fmt::v11::context; T = folly::Range<const char*>; typename std::enable_if<PACKED, int>::type <anonymous> = 0]':
/usr/include/fmt/base.h:2003:74:   required from 'constexpr fmt::v11::detail::format_arg_store<Context, NUM_ARGS, 0, DESC> fmt::v11::make_format_args(T& ...) [with Context = context; T = {folly::Range<const char*>, folly::Range<const char*>}; long unsigned int NUM_ARGS = 2; long unsigned int NUM_NAMED_ARGS = 0; long long unsigned int DESC = 255; typename std::enable_if<(NUM_NAMED_ARGS == 0), int>::type <anonymous> = 0]'
 2003 |   return {{detail::make_arg<NUM_ARGS <= detail::max_packed_args, Context>(
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
 2004 |       args)...}};
      |       ~~~~~

This is ultimately caused by folly::Range<const char*> defining value_type as const char, which does not compare equal to char.

This change is a potential fix that uses remove_const_t to remove the const-ness of value_type for string-like types.

[vitaut]

Thanks for the PR but I think it should be fixed in Folly by making the definition of value_type compatible to the one in string_view.

[mhx]

Thanks for the PR but I think it should be fixed in Folly by making the definition of value_type compatible to the one in string_view.

I agree to some extent, but:

• It's still a regression :-)
• I don't know about all the implications of changing the constness of folly::Range::value_type; I'll see if I can come up with a change in the hope that it won't break anything that relies on the constness of value_type.

[mhx]

If facebook/folly#2265 is a viable solution, this PR can be closed.

[yfeldblum]

StringPiece is an instance of Range, which is more of a span type than a string-view type.

Changing Range in this way is questionable, or at least a larger question. It may be the case that all containers having a member type alias value_type ought to ensure that alias is to a true unqualified value type, and that we need to have the const only on reference_type or such.

[mhx]

StringPiece is an instance of Range, which is more of a span type than a string-view type.

See my comment on facebook/folly#2265, which argues that std::span also has an unqualified value_type.

Changing Range in this way is questionable, or at least a larger question.

I think it'd be the right change, but I don't know if there are any downstream dependencies that already rely on the qualiefied-ness of value_type.

[vitaut]

Closing in favor of the folly fix (facebook/folly@21e8dcd). Thanks for reporting (and the fix attempt)!