Skip to content

Commit 957ee28

Browse files
authored
Store expr not quo for last_dplyr_warnings() (#7750)
1 parent e4ad5d7 commit 957ee28

File tree

3 files changed

+19
-16
lines changed

3 files changed

+19
-16
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# dplyr (development version)
22

3+
* `last_dplyr_warnings()` no longer prevents objects from being garbage collected (#7649).
4+
35
* Progress towards making dplyr conformant with the public C API of R (#7741).
46

57
* `case_when()` now throws correctly indexed errors when `NULL`s are supplied in `...` (#7739).

R/conditions.R

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ cnd_bullet_rowwise_unlist <- function() {
4242
if (peek_mask()$is_rowwise()) {
4343
glue_data(
4444
peek_error_context(),
45-
"Did you mean: `{error_name} = list({quo_as_label(error_quo)})` ?"
45+
"Did you mean: `{error_name} = list({expr_as_label(error_expr)})` ?"
4646
)
4747
}
4848
}
@@ -66,8 +66,7 @@ is_data_pronoun <- function(x) {
6666
}
6767

6868
# Because as_label() strips off .data$<> and .data[[<>]]
69-
quo_as_label <- function(quo) {
70-
expr <- quo_get_expr(quo)
69+
expr_as_label <- function(expr) {
7170
if (is_data_pronoun(expr)) {
7271
deparse(expr)[[1]]
7372
} else {
@@ -87,15 +86,17 @@ new_error_context <- function(dots, i, mask) {
8786
if (!length(dots) || i == 0L) {
8887
env(
8988
error_name = "",
90-
error_quo = NULL,
89+
error_expr = NULL,
9190
mask = mask
9291
)
9392
} else {
94-
# Saving the quosure rather than the result of `quo_as_label()` to avoid
95-
# slow label creation unless required
93+
# Saving the expression rather than the result of `expr_as_label()` to avoid
94+
# slow label creation unless required. Not saving the quosure itself because
95+
# carrying around its environment past the scope of a dplyr verb's lifetime
96+
# can be very expensive (#7649)!
9697
env(
9798
error_name = names(dots)[[i]],
98-
error_quo = dots[[i]],
99+
error_expr = quo_get_expr(dots[[i]]),
99100
mask = mask
100101
)
101102
}
@@ -120,24 +121,24 @@ mask_type <- function(mask = peek_mask()) {
120121
}
121122

122123
ctxt_error_label <- function(ctxt = peek_error_context()) {
123-
error_label(ctxt$error_name, ctxt$error_quo)
124+
error_label(ctxt$error_name, ctxt$error_expr)
124125
}
125-
error_label <- function(name, quo) {
126+
error_label <- function(name, expr) {
126127
if (is_null(name) || !nzchar(name)) {
127-
quo_as_label(quo)
128+
expr_as_label(expr)
128129
} else {
129130
name
130131
}
131132
}
132133

133134
ctxt_error_label_named <- function(ctxt = peek_error_context()) {
134-
error_label_named(ctxt$error_name, ctxt$error_quo)
135+
error_label_named(ctxt$error_name, ctxt$error_expr)
135136
}
136-
error_label_named <- function(name, quo) {
137+
error_label_named <- function(name, expr) {
137138
if (is_null(name) || !nzchar(name)) {
138-
quo_as_label(quo)
139+
expr_as_label(expr)
139140
} else {
140-
paste0(name, " = ", quo_as_label(quo))
141+
paste0(name, " = ", expr_as_label(expr))
141142
}
142143
}
143144

@@ -379,7 +380,7 @@ new_dplyr_warning <- function(data) {
379380
group_label <- ""
380381
}
381382

382-
label <- error_label_named(data$name, data$quo)
383+
label <- error_label_named(data$name, data$expr)
383384

384385
msg <- c(
385386
"i" = glue::glue("In argument: `{label}`."),

R/context.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ cnd_data <- function(cnd, ctxt, mask, call) {
120120
list(
121121
cnd = cnd,
122122
name = ctxt$error_name,
123-
quo = ctxt$error_quo,
123+
expr = ctxt$error_expr,
124124
type = mask_type,
125125
has_group_data = has_group_data,
126126
group_data = group_data,

0 commit comments

Comments
 (0)