Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Align argument types and format strings #524

Merged
merged 10 commits into from Nov 29, 2023
Merged

Align argument types and format strings #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6dfcb92
Test balloon
jennybc Nov 28, 2023
978b264
Something different
jennybc Nov 28, 2023
6439dc6
Apply to all the affected loci
jennybc Nov 28, 2023
d400e77
Take compiler's suggestion
jennybc Nov 28, 2023
2393b89
Cast this
jennybc Nov 28, 2023
c819a93
Undo unrelated whitespaces changes
jennybc Nov 28, 2023
6cb7882
Additional checks suggested by Gabor
jennybc Nov 29, 2023
04d89af
Fix another format specification
jennybc Nov 29, 2023
ecf7d7c
Prune CI a bit
jennybc Nov 29, 2023
957a3b0
NEWS bullet
jennybc Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/R-CMD-check.yaml
View file
Open in desktop
Expand Up @@ -29,6 +29,8 @@ jobs:
- {os: windows-latest, r: '3.6'}
# use 4.1 to check with rtools40's older compiler
- {os: windows-latest, r: '4.1'}
# keep this until the format specifier dust is truly settled (#524)
- {os: windows-latest, r: 'devel'}

- {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
- {os: ubuntu-latest, r: 'release'}
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
@@ -1,5 +1,7 @@
# vroom (development version)

* Internal changes requested by CRAN around format specification (#524).

# vroom 1.6.4

* It is now possible (again?) to read from a list of connections (@bairdj, #514).
Expand Down
8 changes: 8 additions & 0 deletions src/r_utils.h
View file
Open in desktop
Expand Up @@ -9,6 +9,14 @@
#include <string>
#include <vector>

#ifndef R_PRIdXLEN_T
# ifdef LONG_VECTOR_SUPPORT
# define R_PRIdXLEN_T "td"
# else
# define R_PRIdXLEN_T "d"
# endif
#endif

Comment on lines +12 to +19
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspired by https://github.com/wch/r-source/blob/75cd3117e23f2010634200875f2ef50a83461b8f/src/include/Rinternals.h#L71C1-L80

Who/what is going to define LONG_VECTOR_SUPPORT 🤔 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it comes via cpp11/R.hpp, which includes Rinternals.h.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it is coming from the R headers, you are allowed to use it.

namespace vroom {

inline std::string
Expand Down
4 changes: 2 additions & 2 deletions src/unicode_fopen.h
View file
Open in desktop
Expand Up @@ -49,7 +49,7 @@ inline FILE* unicode_fopen(const char* path, const char* mode) {
}
buf = (wchar_t*)R_alloc(len, sizeof(wchar_t));
if (buf == NULL) {
Rf_error("Could not allocate buffer of size: %ll", len);
Rf_error("Could not allocate buffer of size: %zu", len);
}

MultiByteToWideChar(CP_UTF8, 0, path, -1, buf, len);
Expand All @@ -75,7 +75,7 @@ make_mmap_source(const char* file, std::error_code& error) {
}
buf = (wchar_t*)malloc(len * sizeof(wchar_t));
if (buf == NULL) {
Rf_error("Could not allocate buffer of size: %ll", len);
Rf_error("Could not allocate buffer of size: %zu", len);
}

MultiByteToWideChar(CP_UTF8, 0, file, -1, buf, len);
Expand Down
1 change: 1 addition & 0 deletions src/utils.h
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@
#include <cstring>
#include <sstream>
#include <string>
#include <tuple>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say that I completely understand why the need for this was surfaced by the changes in the content and usage of r_utils.h. But I'm also not sure that I really need to get to the bottom of it.


namespace vroom {

Expand Down
3 changes: 2 additions & 1 deletion src/vroom_big_int.h
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@

constexpr long long NA_INTEGER64 = 0x8000000000000000LL;

#include "r_utils.h"
#include "vroom.h"

namespace cpp11 {
Expand Down Expand Up @@ -56,7 +57,7 @@ class vroom_big_int : public vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_big_int (len=%d, materialized=%s)\n",
"vroom_big_int (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_chr.h
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

#include "altrep.h"

#include "r_utils.h"
#include "vroom_vec.h"

cpp11::strings read_chr(vroom_vec_info* info);
Expand Down Expand Up @@ -38,7 +39,7 @@ struct vroom_chr : vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_chr (len=%d, materialized=%s)\n",
"vroom_chr (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_date.h
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

#include <cpp11/doubles.hpp>

#include "r_utils.h"
#include "vroom_dttm.h"

using namespace vroom;
Expand Down Expand Up @@ -47,7 +48,7 @@ class vroom_date : public vroom_dttm {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_date (len=%d, materialized=%s)\n",
"vroom_date (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_dbl.h
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

#include "altrep.h"
#include "parallel.h"
#include "r_utils.h"
#include "vroom_vec.h"

double bsd_strtod(const char* begin, const char* end, const char decimalMark);
Expand Down Expand Up @@ -37,7 +38,7 @@ class vroom_dbl : public vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_dbl (len=%d, materialized=%s)\n",
"vroom_dbl (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_dttm.h
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
#include <cpp11/doubles.hpp>
#include <cpp11/integers.hpp>

#include "r_utils.h"
#include "vroom.h"
#include "vroom_vec.h"

Expand Down Expand Up @@ -65,7 +66,7 @@ class vroom_dttm : public vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_dttm (len=%d, materialized=%s)\n",
"vroom_dttm (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_fct.h
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
#include <cpp11/strings.hpp>

#include "altrep.h"
#include "r_utils.h"
#include "vroom.h"
#include "vroom_vec.h"

Expand Down Expand Up @@ -122,7 +123,7 @@ struct vroom_fct : vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_factor (len=%d, materialized=%s)\n",
"vroom_factor (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_int.h
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

#include "altrep.h"

#include "r_utils.h"
#include "vroom_vec.h"

int strtoi(const char* begin, const char* end);
Expand Down Expand Up @@ -35,7 +36,7 @@ class vroom_int : public vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_int (len=%d, materialized=%s)\n",
"vroom_int (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_num.h
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

#include "altrep.h"

#include "r_utils.h"
#include "vroom_vec.h"

#include "parallel.h"
Expand Down Expand Up @@ -47,7 +48,7 @@ class vroom_num : public vroom_vec {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_num (len=%d, materialized=%s)\n",
"vroom_num (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_rle.h
View file
Open in desktop
@@ -1,6 +1,7 @@
#pragma once

#include "altrep.h"
#include "r_utils.h"

#ifdef HAS_ALTREP

Expand Down Expand Up @@ -40,7 +41,7 @@ class vroom_rle {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_rle (len=%d, materialized=%s)\n",
"vroom_rle (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
3 changes: 2 additions & 1 deletion src/vroom_time.h
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

#include <cpp11/doubles.hpp>

#include "r_utils.h"
#include "vroom.h"
#include "vroom_dttm.h"

Expand Down Expand Up @@ -47,7 +48,7 @@ class vroom_time : public vroom_dttm {
static Rboolean
Inspect(SEXP x, int, int, int, void (*)(SEXP, int, int, int)) {
Rprintf(
"vroom_time (len=%d, materialized=%s)\n",
"vroom_time (len=%" R_PRIdXLEN_T ", materialized=%s)\n",
Length(x),
R_altrep_data2(x) != R_NilValue ? "T" : "F");
return TRUE;
Expand Down
2 changes: 1 addition & 1 deletion src/vroom_write.cc
View file
Open in desktop
Expand Up @@ -270,7 +270,7 @@ void write_buf_con(const std::vector<char>& buf, SEXP con, bool is_stdout) {
if (is_stdout) {
std::string out;
std::copy(buf.begin(), buf.end(), std::back_inserter(out));
Rprintf("%.*s", buf.size(), out.c_str());
Rprintf("%.*s", (int) buf.size(), out.c_str());
} else {
write_buf(buf, con);
}
Expand Down