Skip to content

Commit 3f3636a

Browse files
DavisVaughanDavisVaughan
authored
Generate warnings when formatting naive-times with %Z or %z (#211)
* Generalize `parse_failures` class to `failures` * Generate warnings when formatting naive-times with `%Z` or `%z`
1 parent 0b246ba commit 3f3636a

File tree

12 files changed

+231
-94
lines changed

12 files changed

+231
-94
lines changed

‎NEWS.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838

3939
* Errors resulting from invalid dates or nonexistent/ambiguous times are now
4040
a little nicer to read through the usage of an info bullet (#200).
41+
42+
* Formatting a naive-time with `%Z` or `%z` now warns that there were
43+
format failures (#204).
4144

4245
* Fixed a Solaris ambiguous behavior issue from calling `pow(int, int)`.
4346

‎R/utils.R‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ warn_clock <- function(message, class = character()) {
208208
# Thrown from C++
209209
warn_clock_parse_failures <- function(n, first) {
210210
if (n == 0) {
211-
abort("Internal error: warning thrown with zero parse failures.")
211+
abort("Internal error: warning thrown with zero failures.")
212212
} else if (n == 1) {
213213
message <- paste0(
214214
"Failed to parse 1 string at location ", first, ". ",
@@ -224,6 +224,25 @@ warn_clock_parse_failures <- function(n, first) {
224224
warn_clock(message, "clock_warning_parse_failures")
225225
}
226226

227+
# Thrown from C++
228+
warn_clock_format_failures <- function(n, first) {
229+
if (n == 0) {
230+
abort("Internal error: warning thrown with zero failures.")
231+
} else if (n == 1) {
232+
message <- paste0(
233+
"Failed to format 1 string at location ", first, ". ",
234+
"Returning `NA` at that location."
235+
)
236+
} else {
237+
message <- paste0(
238+
"Failed to format ", n, " strings, beginning at location ", first, ". ",
239+
"Returning `NA` at the locations where there were format failures."
240+
)
241+
}
242+
243+
warn_clock(message, "clock_warning_format_failures")
244+
}
245+
227246
# ------------------------------------------------------------------------------
228247

229248
max_collect <- function(max) {

‎src/failure.h‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#ifndef CLOCK_FAILURE_H
2+
#define CLOCK_FAILURE_H
3+
4+
#include "clock.h"
5+
6+
// -----------------------------------------------------------------------------
7+
8+
namespace rclock {
9+
10+
class failures {
11+
private:
12+
r_ssize n_;
13+
r_ssize first_;
14+
15+
public:
16+
CONSTCD11 failures() NOEXCEPT;
17+
18+
void write(r_ssize i);
19+
CONSTCD11 bool any_failures() const NOEXCEPT;
20+
21+
void warn_parse() const;
22+
void warn_format() const;
23+
};
24+
25+
CONSTCD11
26+
inline
27+
failures::failures() NOEXCEPT
28+
: n_(0),
29+
first_(0)
30+
{}
31+
32+
inline
33+
void
34+
failures::write(r_ssize i) {
35+
if (n_ == 0) {
36+
first_ = i;
37+
}
38+
++n_;
39+
}
40+
41+
CONSTCD11
42+
inline
43+
bool
44+
failures::any_failures() const NOEXCEPT {
45+
return n_ > 0;
46+
}
47+
48+
inline
49+
void
50+
failures::warn_parse() const {
51+
cpp11::writable::integers n(1);
52+
cpp11::writable::integers first(1);
53+
n[0] = (int) n_;
54+
first[0] = (int) first_ + 1;
55+
auto r_warn = cpp11::package("clock")["warn_clock_parse_failures"];
56+
r_warn(n, first);
57+
}
58+
59+
inline
60+
void
61+
failures::warn_format() const {
62+
cpp11::writable::integers n(1);
63+
cpp11::writable::integers first(1);
64+
n[0] = (int) n_;
65+
first[0] = (int) first_ + 1;
66+
auto r_warn = cpp11::package("clock")["warn_clock_format_failures"];
67+
r_warn(n, first);
68+
}
69+
70+
} // namespace rclock
71+
72+
// -----------------------------------------------------------------------------
73+
#endif // CLOCK_FAILURE_H

‎src/format.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "check.h"
77
#include "zone.h"
88
#include "locale.h"
9+
#include "failure.h"
910
#include <sstream>
1011
#include <locale>
1112

@@ -1059,6 +1060,8 @@ cpp11::writable::strings format_time_point_impl(const ClockDuration& cd,
10591060
std::string decimal_mark_string = decimal_mark[0];
10601061
const char* decimal_mark_char = decimal_mark_string.c_str();
10611062

1063+
rclock::failures fail{};
1064+
10621065
for (r_ssize i = 0; i < size; ++i) {
10631066
if (cd.is_na(i)) {
10641067
SET_STRING_ELT(out, i, r_chr_na);
@@ -1082,6 +1085,7 @@ cpp11::writable::strings format_time_point_impl(const ClockDuration& cd,
10821085
);
10831086

10841087
if (stream.fail()) {
1088+
fail.write(i);
10851089
SET_STRING_ELT(out, i, r_chr_na);
10861090
continue;
10871091
}
@@ -1090,6 +1094,10 @@ cpp11::writable::strings format_time_point_impl(const ClockDuration& cd,
10901094
SET_STRING_ELT(out, i, Rf_mkCharLenCE(str.c_str(), str.size(), CE_UTF8));
10911095
}
10921096

1097+
if (fail.any_failures()) {
1098+
fail.warn_format();
1099+
}
1100+
10931101
return out;
10941102
}
10951103

@@ -1192,6 +1200,8 @@ cpp11::writable::strings format_zoned_time_impl(const ClockDuration& cd,
11921200
const std::string decimal_mark_string = decimal_mark[0];
11931201
const char* decimal_mark_char = decimal_mark_string.c_str();
11941202

1203+
rclock::failures fail{};
1204+
11951205
for (r_ssize i = 0; i < size; ++i) {
11961206
if (cd.is_na(i)) {
11971207
SET_STRING_ELT(out, i, r_chr_na);
@@ -1227,6 +1237,7 @@ cpp11::writable::strings format_zoned_time_impl(const ClockDuration& cd,
12271237
);
12281238

12291239
if (stream.fail()) {
1240+
fail.write(i);
12301241
SET_STRING_ELT(out, i, r_chr_na);
12311242
continue;
12321243
}
@@ -1235,6 +1246,10 @@ cpp11::writable::strings format_zoned_time_impl(const ClockDuration& cd,
12351246
SET_STRING_ELT(out, i, Rf_mkCharLenCE(str.c_str(), str.size(), CE_UTF8));
12361247
}
12371248

1249+
if (fail.any_failures()) {
1250+
fail.warn_format();
1251+
}
1252+
12381253
return out;
12391254
}
12401255

‎src/gregorian-year-month-day.cpp‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "enums.h"
66
#include "get.h"
77
#include "parse.h"
8+
#include "failure.h"
89
#include "locale.h"
910
#include "rcrd.h"
1011

@@ -829,7 +830,7 @@ year_month_day_from_stream(std::istringstream& stream,
829830
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
830831
const char& decimal_mark,
831832
const r_ssize& i,
832-
rclock::parse_failures& failures,
833+
rclock::failures& fail,
833834
Calendar& out) {
834835
using Duration = typename Calendar::duration;
835836
const r_ssize size = fmts.size();
@@ -863,7 +864,7 @@ year_month_day_from_stream(std::istringstream& stream,
863864
}
864865
}
865866

866-
failures.write(i);
867+
fail.write(i);
867868
out.assign_na(i);
868869
}
869870

@@ -877,7 +878,7 @@ year_month_day_from_stream(std::istringstream& stream,
877878
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
878879
const char& decimal_mark,
879880
const r_ssize& i,
880-
rclock::parse_failures& failures,
881+
rclock::failures& fail,
881882
rclock::gregorian::y& out) {
882883
const r_ssize size = fmts.size();
883884

@@ -904,7 +905,7 @@ year_month_day_from_stream(std::istringstream& stream,
904905
}
905906
}
906907

907-
failures.write(i);
908+
fail.write(i);
908909
out.assign_na(i);
909910
}
910911

@@ -918,7 +919,7 @@ year_month_day_from_stream(std::istringstream& stream,
918919
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
919920
const char& decimal_mark,
920921
const r_ssize& i,
921-
rclock::parse_failures& failures,
922+
rclock::failures& fail,
922923
rclock::gregorian::ym& out) {
923924
const r_ssize size = fmts.size();
924925

@@ -945,7 +946,7 @@ year_month_day_from_stream(std::istringstream& stream,
945946
}
946947
}
947948

948-
failures.write(i);
949+
fail.write(i);
949950
out.assign_na(i);
950951
}
951952

@@ -959,7 +960,7 @@ year_month_day_from_stream(std::istringstream& stream,
959960
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
960961
const char& decimal_mark,
961962
const r_ssize& i,
962-
rclock::parse_failures& failures,
963+
rclock::failures& fail,
963964
rclock::gregorian::ymd& out) {
964965
const r_ssize size = fmts.size();
965966

@@ -986,7 +987,7 @@ year_month_day_from_stream(std::istringstream& stream,
986987
}
987988
}
988989

989-
failures.write(i);
990+
fail.write(i);
990991
out.assign_na(i);
991992
}
992993

@@ -1000,7 +1001,7 @@ year_month_day_from_stream(std::istringstream& stream,
10001001
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
10011002
const char& decimal_mark,
10021003
const r_ssize& i,
1003-
rclock::parse_failures& failures,
1004+
rclock::failures& fail,
10041005
rclock::gregorian::ymdh& out) {
10051006
const r_ssize size = fmts.size();
10061007

@@ -1030,7 +1031,7 @@ year_month_day_from_stream(std::istringstream& stream,
10301031
}
10311032
}
10321033

1033-
failures.write(i);
1034+
fail.write(i);
10341035
out.assign_na(i);
10351036
}
10361037

@@ -1044,7 +1045,7 @@ year_month_day_from_stream(std::istringstream& stream,
10441045
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
10451046
const char& decimal_mark,
10461047
const r_ssize& i,
1047-
rclock::parse_failures& failures,
1048+
rclock::failures& fail,
10481049
rclock::gregorian::ymdhm& out) {
10491050
const r_ssize size = fmts.size();
10501051

@@ -1075,7 +1076,7 @@ year_month_day_from_stream(std::istringstream& stream,
10751076
}
10761077
}
10771078

1078-
failures.write(i);
1079+
fail.write(i);
10791080
out.assign_na(i);
10801081
}
10811082

@@ -1089,7 +1090,7 @@ year_month_day_from_stream(std::istringstream& stream,
10891090
const std::pair<const std::string*, const std::string*>& ampm_names_pair,
10901091
const char& decimal_mark,
10911092
const r_ssize& i,
1092-
rclock::parse_failures& failures,
1093+
rclock::failures& fail,
10931094
rclock::gregorian::ymdhms& out) {
10941095
const r_ssize size = fmts.size();
10951096

@@ -1121,7 +1122,7 @@ year_month_day_from_stream(std::istringstream& stream,
11211122
}
11221123
}
11231124

1124-
failures.write(i);
1125+
fail.write(i);
11251126
out.assign_na(i);
11261127
}
11271128

@@ -1169,7 +1170,7 @@ year_month_day_parse_impl(const cpp11::strings& x,
11691170
ampm_names
11701171
);
11711172

1172-
rclock::parse_failures failures{};
1173+
rclock::failures fail{};
11731174

11741175
std::istringstream stream;
11751176

@@ -1195,15 +1196,15 @@ year_month_day_parse_impl(const cpp11::strings& x,
11951196
ampm_names_pair,
11961197
dmark,
11971198
i,
1198-
failures,
1199+
fail,
11991200
out
12001201
);
12011202
}
12021203

12031204
vmaxset(vmax);
12041205

1205-
if (failures.any_failures()) {
1206-
failures.warn();
1206+
if (fail.any_failures()) {
1207+
fail.warn_parse();
12071208
}
12081209

12091210
return out.to_list();

0 commit comments

Comments
 (0)