On the hasAttribute and attributeNames usage: If we added Rf_existsAttribute and R_getAttribNames one issue is how to handle the implicit names attribute for pairlist and language objects. Adding the implicit attribute to the names and returning TRUE for exists would be most consistent with the R level and with Rf_getAttrib, but would affect code now looking only at ATTRIB. Any thoughts from your end?

On the hasAttribute and attributeNames usage: If we added Rf_existsAttribute and R_getAttribNames

This would be very convenient for us.

one issue is how to handle the implicit names attribute for pairlist and language objects. Adding the implicit attribute to the names and returning TRUE for exists would be most consistent with the R level and with Rf_getAttrib, but would affect code now looking only at ATTRIB. Any thoughts from your end?

Since ATTRIB is not part of the API from now on, this is not a problem.

Oh, you meant that, from now on, pairlists and language objects would return a new attribute names compared to previous versions of Rcpp, right? It's highly unlikely this breaks any existing code, because I don't think many people use Rcpp to manipulate pairlists and language objects.

All good. I mostly wanted to see if I could get away from ATTRIB now as we have the next upload coming up. We can likely simplify further. Too bad I didn't 'click' on the already possible simpler nrow() (esp as I looked at do_shortRowNames() just below it !!). Also, getting Rf_existsAttribute and R_getAttribNames sounds promising and would have simplified this now. We will have to check if something is affected by returning an attribute names, that is likely an empirical question.

The thought of going the column length for a simpler nrow occurred to me too (though I didn't "finish" the thought with a four-line implementation as @ltierney suggested). Somehow going with R_RowNamesSymbol) feels cleaner. Using it with Shield<> makes it a two-liner

Shield<SEXP> rn = Rf_getAttrib(Parent::get__(), R_RowNamesSymbol);
return Rf_length(rn);

which stays closer to R itself at the cost of the one allocation in Shield. I believe we can use Rf_xlength() here to and return the R_xlen_t. We 'upgraded' from int before without issues. Of course, needs a reverse dependency check too.

On the other hand rewriting what @ltierney suggested as follows (with a second XLENGTH, and a missing )) is pretty tempting too

SEXP x = Parent::get__();
// We can simplify: zero row DFs have no names, else length of all vector same
// by design constraint so can use vector length for desired row count
return (XLENGTH(x) == 0) ? 0 :  XLENGTH(VECTOR_ELT(x, 0))

All those variants pass our 1600+ tests (of which several dozen hit data.frame objects).

On the other hand rewriting what @ltierney suggested as follows (with a second XLENGTH, and a missing )) is pretty tempting too

SEXP x = Parent::get__();
// We can simplify: zero row DFs have no names, else length of all vector same
// by design constraint so can use vector length for desired row count
return (XLENGTH(x) == 0) ? 0 :  XLENGTH(VECTOR_ELT(x, 0))

Unfortunately my comment is wrong: you can have a data frame with zero columns and non-zero rows:

data.frame(row.names = 1:3)

So for now going with R_RowNamesSymbol is probably best until we figure out whether Rf_nrows can be modified to handle this or maybe add another function that does. Definitely seems like something that should be in the API.

Thanks so much for checking. The other two-liner relying on Shield<SEXP> for the one allocation here is much safer, and closer to what R does.

I am about to commit the re-done attribute proxy using two lambda visitors too. I still think copilot is wrong and that row.names always the last element meaning we need all of them anyway but heck, on the chance it gains at least sometimes it is worth it.

Those were really helpful by all -- big thanks to you all: @ltierney @Enchufa2 @JanMarvin !