Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default index 0 for ByteAddressBuffer::Load #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

python3kgae
Copy link
Collaborator

No description provided.

@llvm-beanz
Copy link
Collaborator

I think we need to work through and flesh out some wording here. I think I understand the use case and I like it.

Instead of adding a default parameter, I'd suggest adding a new API that takes no parameters and call the old API with 0 as the index. The reason I think a new API makes more sense is because API additions and removals are simpler to express in language version changes than a change like adding a default parameter.

@python3kgae
Copy link
Collaborator Author

I think we need to work through and flesh out some wording here. I think I understand the use case and I like it.

Instead of adding a default parameter, I'd suggest adding a new API that takes no parameters and call the old API with 0 as the index. The reason I think a new API makes more sense is because API additions and removals are simpler to express in language version changes than a change like adding a default parameter.

Will this work?

template<typename T>
T ByteAddressBuffer::getAs() {
  return Load<T>(0);
}

@llvm-beanz
Copy link
Collaborator

Will this work?

template<typename T>
T ByteAddressBuffer::getAs() {
  return Load<T>(0);
}

I feel like getAs might not be the right naming. Mixing getAs and Load calls on the same object seems confusing, and you’re not really getting the buffer as an object, you’re pulling an object out of the buffer.

@python3kgae
Copy link
Collaborator Author

Will this work?

template<typename T>
T ByteAddressBuffer::getAs() {
  return Load<T>(0);
}

I feel like getAs might not be the right naming. Mixing getAs and Load calls on the same object seems confusing, and you’re not really getting the buffer as an object, you’re pulling an object out of the buffer.

How about loadAs?

@llvm-beanz
Copy link
Collaborator

How about loadAs?

Why not just Load? Is the As not implied by the template argument as with other methods?

template<typename T>
T ByteAddressBuffer::Load() {
  return Load<T>(0);
}

Seems to be consistent with other builtin APIs and provide the desired behavior?

@python3kgae
Copy link
Collaborator Author

How about loadAs?

Why not just Load? Is the As not implied by the template argument as with other methods?

template<typename T>
T ByteAddressBuffer::Load() {
  return Load<T>(0);
}

Seems to be consistent with other builtin APIs and provide the desired behavior?

If the name is still Load, I would like to make it a default parameter 0 instead of adding a new overload when implementing.
The result will be the same. We can add the overload version in the documentation only.

@llvm-beanz llvm-beanz added the Design Meeting Agenda item for the design meeting label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Meeting Agenda item for the design meeting
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants