Skip to content

Remove implicit conversion operators on sexp - #390

Open
DavisVaughan wants to merge 2 commits into
r-lib:mainfrom
DavisVaughan:feature/remove-sexp-conversion-operators
Open

DavisVaughan wants to merge 2 commits into
r-lib:mainfrom
DavisVaughan:feature/remove-sexp-conversion-operators

Conversation

@DavisVaughan

@DavisVaughan DavisVaughan commented Aug 22, 2024

Copy link
Copy Markdown
Member

cpp11 currently allows some somewhat scary implicit conversions between sexp and the 3 types bool, size_t, and double. These are unchecked conversions, and we'd like to remove them. However, doing so breaks readxl, vroom, and supercells, so we sent them PRs for this:

We can merge this once those packages have merged and updated on CRAN

@DavisVaughan DavisVaughan added the breaking change ☠️ API change likely to affect existing code label Aug 23, 2024
krlmlr added a commit to krlmlr/cpp11 that referenced this pull request Sep 12, 2026
The merge is textually clean,
but its result is not:
the three `[[deprecated]]` declarations run to about 135 characters,
and `.github/workflows/format.yaml` fails a tree
that `clang-format` would reformat.
The branch already failed that check before the merge,
so this is the merge that fixes it rather than one that broke it.

`clang-format` splits each attribute onto its own line
and puts the body in braces.
Whitespace only:
the attributes, their messages and the conversions are unchanged.

The feature itself still stands.
Upstream's own r-lib#390 would remove these three conversion
operators outright rather than deprecate them,
which would retire this patch --
but it is blocked on readxl, vroom and supercells,
has not moved since August 2024,
and no longer merges cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K8MneV8KqHYUuC8fWV3X5Q
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change ☠️ API change likely to affect existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant