Skip to content

renamed parser functions #7248

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

renamed parser functions #7248

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf9681e
renamed parser functions
badasahog Aug 11, 2025
ec0aa0b
make parser function table const
badasahog Aug 11, 2025
98e2196
also renamed the parser array itself. 'fun' is not very descriptive
badasahog Aug 11, 2025
01fb33c
Merge branch 'master' into parserFunctionRename
badasahog Aug 13, 2025
572d67f
Merge branch 'master' into parserFunctionRename
badasahog Aug 28, 2025
edbe3f5
Merge branch 'master' into parserFunctionRename
badasahog Aug 29, 2025
3278bd9
Merge branch 'master' into parserFunctionRename
badasahog Sep 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/fread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,12 +636,12 @@ static void str_to_i32_core(const char **pch, int32_t *target, bool parse_date)
}
}

static void StrtoI32(FieldParseContext *ctx)
static void parse_i32(FieldParseContext *ctx)
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't the consistent naming here be parse_int32_str (parse_${DEST_TYPE}_${SOURCE_DESCRIPTION})?

{
str_to_i32_core(ctx->ch, (int32_t*) ctx->targets[sizeof(int32_t)], false);
}

static void StrtoI64(FieldParseContext *ctx)
static void parse_i64(FieldParseContext *ctx)
{
const char *ch = *ctx->ch;
int64_t *target = ctx->targets[sizeof(int64_t)];
Expand Down Expand Up @@ -1124,7 +1124,7 @@ static void parse_empty(FieldParseContext *ctx)
}

/* Parse numbers 0 | 1 as boolean and ,, as NA (fwrite's default) */
static void parse_bool_numeric(FieldParseContext *ctx)
static void parse_bool_10(FieldParseContext *ctx)
Copy link
Member

Choose a reason for hiding this comment

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

I prefer the old name, right now it's only 1/0 but I'm not sure we would add another parser if we wanted to, say, include 00,01. numeric->bool seems the best description here.

{
const char *ch = *ctx->ch;
int8_t *target = ctx->targets[sizeof(int8_t)];
Expand Down Expand Up @@ -1190,7 +1190,7 @@ static void parse_bool_lowercase(FieldParseContext *ctx)
}

/* Parse Y | y | N | n as boolean */
static void parse_bool_yesno(FieldParseContext *ctx)
static void parse_bool_yn(FieldParseContext *ctx)
Copy link
Member

Choose a reason for hiding this comment

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

same here, "yes/no" is the most general description for what this parser could do, anchoring on only y/n in a sense exposes the implementation detail that this currently only applies to those two characters -- in principle eventually y e s | n o could be parsed here.

{
const char *ch = *ctx->ch;
int8_t *target = ctx->targets[sizeof(int8_t)];
Expand All @@ -1207,22 +1207,22 @@ static void parse_bool_yesno(FieldParseContext *ctx)

/* How to register a new parser
* (1) Write the parser
* (2) Add it to fun array here
* (2) Add it to parser_funs array here
* (3) Extend disabled_parsers, typeName, and typeSize here as appropriate
* (4) Extend colType typdef in fread.h as appropriate
* (5) Extend typeSxp, typeRName, typeEnum in freadR.c as appropriate
*/
typedef void (*reader_fun_t)(FieldParseContext *ctx);
static reader_fun_t fun[NUMTYPE] = {
static const reader_fun_t parser_funs[NUMTYPE] = {
&Field, // CT_DROP
&parse_empty, // CT_EMPTY
&parse_bool_numeric,
&parse_bool_10,
&parse_bool_uppercase,
&parse_bool_titlecase,
&parse_bool_lowercase,
&parse_bool_yesno,
&StrtoI32,
&StrtoI64,
&parse_bool_yn,
&parse_i32,
&parse_i64,
&parse_double_regular,
&parse_double_extended,
&parse_double_hexadecimal,
Expand Down Expand Up @@ -1261,11 +1261,11 @@ static int detect_types(const char **pch, int ncol, bool *bumped)
dec = '.';
}

fun[tmpType[field]](&fctx);
parser_funs[tmpType[field]](&fctx);
if (end_of_field(ch)) break;
skip_white(&ch);
if (end_of_field(ch)) break;
ch = end_NA_string(fieldStart); // fieldStart here is correct (already after skip_white above); fun[]() may have part processed field so not ch
ch = end_NA_string(fieldStart); // fieldStart here is correct (already after skip_white above); parser_funs[]() may have part processed field so not ch
skip_white(&ch);
if (end_of_field(ch)) break;
ch = fieldStart;
Expand All @@ -1276,7 +1276,7 @@ static int detect_types(const char **pch, int ncol, bool *bumped)
}
if (*ch == quote && quote) { // && quote to exclude quote='\0' (user passed quote="")
ch++;
fun[tmpType[field]](&fctx);
parser_funs[tmpType[field]](&fctx);
if (*ch == quote) {
ch++;
skip_white(&ch);
Expand Down Expand Up @@ -2460,7 +2460,7 @@ int freadMain(freadMainArgs _args)
// DTPRINT(_("Field %d: '%.10s' as type %d (tch=%p)\n"), j+1, tch, type[j], tch);
fieldStart = tch;
int8_t thisType = type[j]; // fetch shared type once. Cannot read half-written byte is one reason type's type is single byte to avoid atomic read here.
fun[IGNORE_BUMP(thisType)](&fctx);
parser_funs[IGNORE_BUMP(thisType)](&fctx);
if (*tch != sep) break;
int8_t thisSize = size[j];
if (thisSize) ((char**) targets)[thisSize] += thisSize; // 'if' for when rereading to avoid undefined NULL+0
Expand Down Expand Up @@ -2524,7 +2524,7 @@ int freadMain(freadMainArgs _args)
if (!end_of_field(tch)) tch = afterSpace; // else it is the field_end, we're on closing sep|eol and we'll let processor write appropriate NA as if field was empty
if (*tch == quote && quote) { quoted = true; tch++; }
} // else Field() handles NA inside it unlike other processors e.g. ,, is interpreted as "" or NA depending on option read inside Field()
fun[IGNORE_BUMP(thisType)](&fctx);
parser_funs[IGNORE_BUMP(thisType)](&fctx);

bool typeBump = false;
if (quoted) { // quoted was only set to true with '&& quote' above (=> quote!='\0' now)
Expand Down
Loading