Skip to content

refactor: make StringTable::lookup const#209

Merged
KowalskiThomas merged 1 commit intoP403n1x87:mainfrom
KowalskiThomas:kowalski/refactor-make-stringtable-lookup-const
Dec 8, 2025
Merged

refactor: make StringTable::lookup const#209
KowalskiThomas merged 1 commit intoP403n1x87:mainfrom
KowalskiThomas:kowalski/refactor-make-stringtable-lookup-const

Conversation

@KowalskiThomas
Copy link
Collaborator

@KowalskiThomas KowalskiThomas commented Dec 5, 2025

What does this PR do?

This PR updates the StringTable::lookup method to be const. This required making the std::mutex we have mutable, which is OK – mutable is typically used for that.

Additionally, it also makes the return type std::reference_wrapper<const std::string> as I don't think we should allow callers of lookup to change the underlying data, which the previous signature would allow.

[cppreference] mutable is used to specify that the member does not affect the externally visible state of the class (as often used for mutexes, memo caches, lazy evaluation, and access instrumentation).

dd-trace-py PR: DataDog/dd-trace-py#15547

KowalskiThomas added a commit to DataDog/dd-trace-py that referenced this pull request Dec 8, 2025
## What does this PR do?

This PR updates the `StringTable::lookup` method to be `const`. This
required making the `std::mutex` we have `mutable`, which is OK –
`mutable` is typically used for that.

Additionally, it also makes the return type
`std::reference_wrapper<const std::string>` as I don't think we should
allow callers of `lookup` to change the underlying data, which the
previous signature would allow.

>
[[cppreference]](https://en.cppreference.com/w/cpp/language/cv.html#mutable)
mutable is used to specify that the member does not affect the
externally visible state of the class (as often used for mutexes, memo
caches, lazy evaluation, and access instrumentation).

Echion PR: P403n1x87/echion#209
@KowalskiThomas KowalskiThomas merged commit aca4064 into P403n1x87:main Dec 8, 2025
116 of 126 checks passed
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.

1 participant