optimize implemention of Index template to improve build speed. - #326
Conversation
2805400 to
85a297a
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a737c8f324
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static_assert((std::is_same_v<T, Types> || ...), "Type not found in tuple"); | ||
| static constexpr std::size_t kValue = [] { | ||
| std::size_t i = 0; | ||
| ((std::is_same_v<T, Types> ? false : (++i, true)) && ...); |
There was a problem hiding this comment.
Index does not trip clang-diagnostic-unused-value
.github/workflows/test.yml runs make clang-tidy, and .clang-tidy promotes every warning to an error (WarningsAsErrors: '*'). This fold expression now emits clang-diagnostic-unused-value for each Index<...> instantiation because the (++i, true) branch's value is discarded; I reproduced it with clang-tidy /tmp/index_test.cc -- -std=c++17 -I/workspace/envpool. Since Index is the helper behind Take() in envpool/core/dict.h, ordinary dict lookups will start failing the lint job.
Useful? React with 👍 / 👎.
3 participants
Description
Optimize implemention of
Indextemplate to improve build speed.Motivation and Context
close #327
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Implemented Tasks
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!
make format(required)make lint(required)make bazel-testpass. (required)