Skip to content

FluidDataSet: point args in add and update are now const#311

Merged
tremblap merged 3 commits into
flucoma:mainfrom
tremblap:fix/const-fix-ds-add-and-update
Jun 25, 2025
Merged

FluidDataSet: point args in add and update are now const#311
tremblap merged 3 commits into
flucoma:mainfrom
tremblap:fix/const-fix-ds-add-and-update

Conversation

@tremblap
Copy link
Copy Markdown
Member

@tremblap tremblap commented Jun 20, 2025

road tested on all UTs, fixes #80

@tremblap tremblap requested a review from weefuzzy June 20, 2025 13:10
weefuzzy
weefuzzy requested changes Jun 25, 2025
View reviewed changes
Comment thread include/flucoma/data/FluidDataSet.hpp Outdated
Comment thread include/flucoma/data/FluidDataSet.hpp Outdated
tremblap and others added 2 commits June 25, 2025 19:08
@tremblap @weefuzzy
Update include/flucoma/data/FluidDataSet.hpp
5eb1fcc 
Co-authored-by: Owen Green <gungwho@gmail.com>
@tremblap @weefuzzy
Update include/flucoma/data/FluidDataSet.hpp
02e2f33 
Co-authored-by: Owen Green <gungwho@gmail.com>
@tremblap
Copy link
Copy Markdown
Member Author

tremblap commented Jun 25, 2025

I've committed both of your suggestions - I don't completely understand the subtleties but I trust you. I'll run the UTs on them all the same before pinging back here

@tremblap
Copy link
Copy Markdown
Member Author

tremblap commented Jun 25, 2025

ok it all passes uts so i'd merge

@weefuzzy
Copy link
Copy Markdown
Member

weefuzzy commented Jun 25, 2025

The subtleties are just to do with const semantics and pointers, and the difference between pointing to const and the pointer itself being const (https://isocpp.org/wiki/faq/const-correctness#const-ptr-vs-ptr-const).

So here, View<const T> points to const (i.e. the data can't be changed), but const View<T> just means that the reference address can't be changed but the data can. We want the former, i.e. these functions shouldn't be changing the data that's passed in.

@tremblap tremblap requested a review from weefuzzy June 25, 2025 18:17
@tremblap
Copy link
Copy Markdown
Member Author

tremblap commented Jun 25, 2025

thanks for the explanation. The templating understanding of yours truly is still fragile...

@tremblap tremblap merged commit 04bbf3d into flucoma:main Jun 25, 2025
2 of 3 checks passed
@tremblap tremblap deleted the fix/const-fix-ds-add-and-update branch June 25, 2025 18:19
@tremblap tremblap mentioned this pull request Jun 25, 2025
Closed
weefuzzy added a commit to weefuzzy/flucoma-core that referenced this pull request Nov 25, 2025
@tremblap @weefuzzy
FluidDataSet: point args in add and update are now const (fluco…
94cd6bb 
…ma#311)

* FluidDataSet: `point` args in `add` and `update` are now const

* Update include/flucoma/data/FluidDataSet.hpp

Co-authored-by: Owen Green <gungwho@gmail.com>

* Update include/flucoma/data/FluidDataSet.hpp

Co-authored-by: Owen Green <gungwho@gmail.com>

---------

Co-authored-by: Owen Green <gungwho@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weefuzzy weefuzzy Awaiting requested review from weefuzzy

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

FluidDataSet: can point args in add and update be made const?

2 participants

@tremblap @weefuzzy