set call setalloccol for non-selfrefok tables when removing columns

[ben-schwen]

Closes #7501
Follows #7492

[MichaelChirico]

I think #7492 omitted NEWS because it was fixing an issue in dev; I guess this needs NEWS because 1.18.0 will already be on CRAN? I guess this will need to simmer a bit to answer that with certainty.

[ben-schwen]

I think #7492 omitted NEWS because it was fixing an issue in dev; I guess this needs NEWS because 1.18.0 will already be on CRAN? I guess this will need to simmer a bit to answer that with certainty.

True, but I guess that depends on whether we were able to submit 1.18.0. With all the revdeps failing, I'm not so sure about it...

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (dc1dbe9) to head (156f4ff).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7500   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files          87       87           
  Lines       16694    16699    +5     
=======================================
+ Hits        16534    16539    +5     
  Misses        160      160           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

No obvious timing issues in HEAD=set_delete

Generated via commit 156f4ff

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 59 seconds
Installing different package versions	22 seconds
Running and plotting the test cases	5 minutes and 5 seconds

[TysonStanley]

I think #7492 omitted NEWS because it was fixing an issue in dev; I guess this needs NEWS because 1.18.0 will already be on CRAN? I guess this will need to simmer a bit to answer that with certainty.

True, but I guess that depends on whether we were able to submit 1.18.0. With all the revdeps failing, I'm not so sure about it...

Not sure either, I've asked Kurt about it. Will let you know when he tells me. Asked what the priority is (getting a fix on CRAN with needing a patch in the next month or waiting to get it on CRAN until after the holidays).

[aitap]

Unfortunately, this only fixes x in the environment of set. Now set needs to reassign x in the caller's environment. Would this be enough, or do we need the deeper magic from [.data.table?

xsub <- substitute(x)
# setalloccol(...)
eval(
  substitute(
    x <- xnew,
    list(x = xsub, xnew = x)
  ),
  parent.frame()
)

[HughParsonage]

    if ((is.null(value) || (is.list(value) && any(vapply_1b(value, is.null)))) && selfrefok(x, verbose=FALSE) < 1L) {
      name = substitute(x)
      setalloccol(x, verbose=FALSE)
      if (is.name(name)) {
        assign(as.character(name), x, parent.frame(), inherits=TRUE)
      }
    }

[HughParsonage]

?

[aitap]

Are we expecting people to call set() on sub-expressions (e.g. set(foo[[bar]]$baz, i, j, value))? If not, this should definitely work, thank you!

[ben-schwen]

well if someone is putting set on sub-expressions, he should probably self assign the results anyway