sync declaration & implementation signatures #1827
Conversation
src/utils.h
Outdated
| @@ -148,7 +148,7 @@ SEXP s3_find_method2(const char* generic, | |||
| SEXP x, | |||
| SEXP table, | |||
| SEXP* method_sym_out); | |||
| SEXP s3_paste_method_sym(const char* generic, const char* cls); | |||
| SEXP s3_paste_method_sym(const char* generic, const char* class); | |||
There was a problem hiding this comment.
Should probably be the other way around since class is reserved in C++. I think we changed it because some IDE (RStudio?) got confused.
There was a problem hiding this comment.
Makes sense... now using cls everywhere in utils.{c,h}.
I noticed class being used elsewhere in src/, would it make sense to tackle the issue generally with this PR?
There was a problem hiding this comment.
How bigger does the diff become? It's ended up surprisingly quite big already so if it's only a few more lines let's do it, otherwise we can leave it.
There was a problem hiding this comment.
Here's the rest, roughly:
grep -Enr "^[^/\"]*\bclass\b" src --exclude=utils.{c,h}
src/rlang/attrib.c:129: * With push_ prefix, assumes there is no `class` attribute in the
src/decl/utils-dispatch-decl.h:4:enum vctrs_class_type class_type_impl(r_obj* class);
src/proxy-restore.c:83: r_obj* class = r_null;
src/proxy-restore.c:97: class = r_node_car(node);
src/proxy-restore.c:147: if (class != r_null) {
src/proxy-restore.c:148: r_attrib_poke(x, r_syms.class_, class);
src/ptype2.c:154: * common class fallback is enabled we return the `vec_ptype2()` of
src/utils-dispatch.c:15: r_obj* class = KEEP(r_class(x));
src/utils-dispatch.c:18: if (class == r_null) {
src/utils-dispatch.c:23: enum vctrs_class_type type = class_type_impl(class);
src/utils-dispatch.c:30:enum vctrs_class_type class_type_impl(r_obj* class) {
src/utils-dispatch.c:31: int n = r_length(class);
src/utils-dispatch.c:32: r_obj* const* p = r_chr_cbegin(class);
src/c.c:205: r_obj* class = r_attrib_get(ptype, syms_fallback_class);
src/c.c:206: class = r_chr_get(class, r_length(class) - 1);
src/c.c:208: return class != strings_vctrs_vctr;
src/c.c:275: r_obj* class = KEEP(r_attrib_get(ptype, syms_fallback_class));
src/c.c:276: bool implements_c = class_implements_base_c(class);At a glance that's 6 distinct scopes. 18 hits here vs. 34 from utils.{c,h}
There was a problem hiding this comment.
Went ahead and made the change. We can
(1) submit as is
(2) revert this latest commit 1b5577e
(3) minify 3f0d9b1 by only addressing places directly related to the declaration-definition mismatch originally targeted by this PR (i.e. ignore any variables class with scope internal to a function)
Happy to do any!
There was a problem hiding this comment.
LGTM if it looks good to @lionel-, I tweaked a few local variable names to remain consistent
Co-authored-by: Davis Vaughan <davis@posit.co>
Co-authored-by: Davis Vaughan <davis@posit.co>
Co-authored-by: Davis Vaughan <davis@posit.co>
Co-authored-by: Davis Vaughan <davis@posit.co>
|
Thank you |
No description provided.