r-lib / rlang

Low-level API for programming with R
https://rlang.r-lib.org
Other
511 stars 138 forks source link

Handle assignment functions in backtrace? #1369

Closed hadley closed 2 years ago

hadley commented 2 years ago

This would be a bit easier to read:

#>     ▆
#>  1. └─global shift(range(1, 10), 10)
#>  2.   ├─base::`@<-`(`*tmp*`, "start", value = x@start + shift)
#>  3.   └─R7::`@<-.R7_object`(`*tmp*`, "start", value = x@start + shift)
#>  4.     └─R7::`prop<-`(`*tmp*`, nme, value = `<promise>`) at OOP-WG/R/property.R:239:2
#>  5.       └─R7::validate(object, properties = FALSE) at OOP-WG/R/property.R:203:6

If it was:

#>     ▆
#>  1. └─global shift(range(1, 10), 10)
#>  2.   ├─`*tmp*`@start <- x@start + shift
#>  3.   └─R7::`@<-.R7_object`(`*tmp*`, "start", value = x@start + shift)
#>  4.     └─R7::prop(`*tmp*`, nme) <- `<promise>` at OOP-WG/R/property.R:239:2
#>  5.       └─R7::validate(object, properties = FALSE) at OOP-WG/R/property.R:203:6

But it's not that big of a difference because there's no nice way to display R7::`@<-.R7_object`

lionel- commented 2 years ago

Will it be confusing that this call in user code

foo(x) <- 2

becomes this in the backtrace?

foo(`*tmp*`) <- `<dbl>`

I guess that's still an improvement.

lionel- commented 2 years ago

Maybe we could do better on the R side by propagating the relevant srcref when calling the replacement function? We don't generally use srcrefs for displaying calls in the backtrace though.

hadley commented 2 years ago

Yeah, this no longer seems like a big win to me.