`as_sexp_strings()` could use a single `unwind_protect()` around the loop

Abierto
#460 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
48/100
Tipo de issue
Refactorización
Claridad
Bastante claro
Estado de actividad
Estancado
Stack tecnológico
cpp, r
Área
tooling

Línea de trabajo

Start in inst/include/cpp11/as.hpp at as_sexp_strings(), then read the unwind_protect example in vignettes/FAQ.Rmd. Compare the loop and the documented manual-call pitfalls; done means the conversion uses one protection around the loop without changing its behavior or safety.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

To convert from std::vector<std::string> to cpp11::strings you might do cpp11::strings(as_sexp(x)). That goes through as_sexp_strings() which looks like it could be a little more efficient. Rather than calling safe[Rf_mkCharCE] on each iteration, I think we could wrap the whole loop in a single unwind_protect().

https://github.com/r-lib/cpp11/blob/05c888b0c6f49e7b252b79b028e4719e6d5b299d/inst/include/cpp11/as.hpp#L287-L306

Something like
https://github.com/r-lib/cpp11/blob/05c888b0c6f49e7b252b79b028e4719e6d5b299d/vignettes/FAQ.Rmd#L499-L503

Be careful to avoid all the pitfalls of calling unwind_protect() manually that are mentioned in that vignette

Lenguaje dominante
C++
Estrellas
224
Forks
52
Métricas de merge de PR
Sin PR fusionados en 30 d

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de r-lib/cpp11

Todos los issues de r-lib/cpp11

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.