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

StringBuf niceties #3204

Merged
merged 1 commit into from
Jan 4, 2025
Merged

StringBuf niceties #3204

merged 1 commit into from
Jan 4, 2025

Conversation

stellar-aria
Copy link
Collaborator

Pared down from #3196 after having it pointed out to me by @sapphire-arches that alloca would not do what I wanted the way I was using it...

Copy link
Contributor

github-actions bot commented Jan 4, 2025

Test Results

106 tests  ±0   106 ✅ ±0   0s ⏱️ ±0s
 16 suites ±0     0 💤 ±0 
 16 files   ±0     0 ❌ ±0 

Results for commit 1c47d5a. ± Comparison against base commit 598cd8e.

[[nodiscard]] std::size_t size() const { return ::strlen(buf_); }
[[nodiscard]] size_t capacity() const { return capacity_; }
[[nodiscard]] size_t size() const { return std::strlen(buf_); }
[[nodiscard]] size_t length() const { return std::strlen(buf_); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why both? All the std stuff tends to .size() and the deluge specials tend to .getLength() or .getNumWhatevs(), so .lenght() seems a bit inconsistent?

Not asking changes, just wondering.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's conformance to the string interface, which has length() because of strlen and size() to act as a standard container

@nikodemus
Copy link
Collaborator

Pared down from #3196 after having it pointed out to me by @sapphire-arches that alloca would not do what I wanted the way I was using it...

I wonder, if we had something like:

class StackStringBase {
    virtual char* buf() = 0;
    virtual size_t capacitu() = 0;
    ...
}:

template <int SZ>
class StackString final : public StackStringBase {
protected:
    inline char* buf() final { return buf_; }
    inline size_t capacity() final { return SZ; }
private:
    char buf_[SZ];
};

How much extra crap per allocated size would we get? Could we force the size to a power of 2 to keep tha down to something sensible?

@stellar-aria
Copy link
Collaborator Author

in-line definitions are always inline linkage, but we may get some benefit from making them constexpr if possible. I don't think there are constexpr definitions of the c-string stuff, but there are for std::string_view. there may be some benefit to rewriting these as constexpr since then the compiler could reason about and catch out-of-bounds errors at compile time

@stellar-aria stellar-aria added this pull request to the merge queue Jan 4, 2025
Merged via the queue into community with commit e936bb8 Jan 4, 2025
6 checks passed
@stellar-aria stellar-aria deleted the stringbuf2 branch January 4, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants