Why does FormatTime() use absl::string_view as "format" string instead of a constexpr?

[pkbehera]

The FormatTime() API is:

std::string FormatTime(absl::string_view format, Time t, TimeZone tz);

The format argument is generally a constexpr, as in StrFormat()

Can anyone explain why FormatTime() accepts a absl::string_view as the format string instead of a constexpr?

[tituswinters]

I suspect it's a blend of a couple things: Work on FormatTime predated work on StrFormat by several years - our tools for constexpr work were more limited at that point. We may just not have invested in it.

TimeZone data is dynamic (loaded at runtime from a per-machine database). It isn't clear to me that doing any TimeZone-related calculation at compile time is correct - eventually that would lead to skew between any TimeZone information used at compilation time vs. what is used at runtime ... and that's gonna be mysterious at best and buggy most of the time.

I suspect the latter is the bigger problem, but if someone has a more nuanced understanding of TimeZone updates and constexpr consistency for rarely-changing data, I could be convinced that there is an opportunity to improve this.

On Wed, Oct 13, 2021 at 3:52 AM Behera @.***> wrote:

The FormatTime() https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h#L1307 API is:

std::string FormatTime(absl::string_view format, Time t, TimeZone tz);

The format argument is generally a constexpr, as in StrFormat() https://github.com/abseil/abseil-cpp/blob/master/absl/strings/str_format.h#L338

Can anyone explain why FormatTime() accepts a absl::string_view as the format string instead of a constexpr?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/abseil/abseil-cpp/issues/1041, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC7CRX7W4A5WOSVKVTEVE7DUGU3CFANCNFSM5F4QMEGQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- If you get an email from me outside of the 9-5 it is not* because I'm always on or expect an immediate response from you; it is because of work flexibility http://www.inc.com/john-boitnott/how-flexible-hours-can-create-a-better-work-life-balance.html . Evening and weekend emails are a sign I allocated some regular working hours for other things (such as family, gym, friends,...). And I encourage you to feel free to do the same.

[pkbehera]

Well FormatTime() internally uses format() from CCTZ, which is:

std::string format(const std::string& format, const time_point<seconds>& tp,
                   const detail::femtoseconds& fs, const time_zone& tz);

And this API actually does a format.c_str() while giving out the formatted string!

So I think this API from CCTZ can easily be changed to accept a constexpr and then FormatTime() can also be changed to accept a constexpr for the format string.

[tituswinters]

There is no such thing as "a constexpr", so you might need to be more specific in what change you're trying to make. For instance, constexpr std::string doesn't work until at least C++23 because of constexpr allocation/container rules.

More importantly, the whole point above about what it would mean to do this computation at compile time is unclear because of the dynamic nature of the timezone data.

I'm fairly sure this is both technically infeasible and not the right design, but if you've got more specific examples you can certainly reopen.

On Thu, Oct 14, 2021 at 12:17 AM Behera @.***> wrote:

Well FormatTime() https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h#L1307 internally uses format() https://github.com/abseil/abseil-cpp/blob/master/absl/time/internal/cctz/src/time_zone_format.cc#L333 from CCTZ, which is:

std::string format(const std::string& format, const time_point& tp, const detail::femtoseconds& fs, const time_zone& tz);

And this API https://github.com/abseil/abseil-cpp/blob/master/absl/time/internal/cctz/src/time_zone_format.cc#L350 actually does a format.c_str() while giving out the formatted string!

So I think this API from CCTZ can easily be changed to accept a constexpr and then FormatTime() can also be changed to accept a constexpr for the format string.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abseil/abseil-cpp/issues/1041#issuecomment-942932084, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC7CRX6DXV6X2FXHHBU36TLUGZKW3ANCNFSM5F4QMEGQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- If you get an email from me outside of the 9-5 it is not* because I'm always on or expect an immediate response from you; it is because of work flexibility http://www.inc.com/john-boitnott/how-flexible-hours-can-create-a-better-work-life-balance.html . Evening and weekend emails are a sign I allocated some regular working hours for other things (such as family, gym, friends,...). And I encourage you to feel free to do the same.

[derekmauro]

I don't think the question is about making FormatTime a constexpr function. I think the question is about things you can do if the format string is a compile-time constant. For example, printf and absl::StrFormat are both capable of warning if the argument types don't match the format string. It can also be a security problem if an attacker manages to take control of a format string, which is why -Wformat-nonliteral exists.

Unlike printf, FormatTime is not a variadic function. It has a constant number arguments, and all the arguments are typed.

Does this answer your question? If not, please explain what problem you are interested in solving.

[pkbehera]

That is exactly why I asked this question, at compile time I am not getting any warning it the format string I pass to FormatString() is not correct (e.g an invalid time-format string), absl::StrFormat(), absl::Substitute etc. check the format string and at compile time and show a warning if the format string is not kosher!

And such compile time checking would be possible for FormatTime() only if it accepts a constexpr.