[base] Improve traits_bag.h error messages with C++20 concepts [chromium/src : main]
0 views
Skip to first unread message
Patrick Monette (Gerrit)
Mar 27, 2026, 2:04:44 PM (6 days ago) Mar 27
to Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Etienne Pierre-Doray
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Mar 27, 2026, 2:18:51 PM (6 days ago) Mar 27
to Patrick Monette, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Patrick Monette
Etienne Pierre-Doray added 3 comments![]( "Open in Gerrit")
File base/traits_bag.h
Line 229, Patchset 4 (Latest):
concept IsConstructible = (std::is_constructible_v<Trait, Args> || ...);Etienne Pierre-Doray . unresolved
Nit: constructible_from for consistency
Line 229, Patchset 4 (Latest):
concept IsConstructible = (std::is_constructible_v<Trait, Args> || ...);Etienne Pierre-Doray . unresolved
Not sure about naming; this makes it sound like Trait is constructible from `Args...`
Perhaps
```
template <typename TraitFilter, typename... Args>
concept HasTraitMatching = ...
```
Though we have HasTrait() below, and the difference is subtle.
Line 143, Patchset 4 (Latest):
(count({std::is_constructible_v<Trait, Args>...}, true) <= 1);Etienne Pierre-Doray . unresolved
Nit: constructible_from for consistency
Related details
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Mar 27, 2026, 2:35:05 PM (6 days ago) Mar 27
to Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Etienne Pierre-Doray
Patrick Monette added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File base/traits_bag.h
Line 229, Patchset 4:
concept IsConstructible = (std::is_constructible_v<Trait, Args> || ...);Etienne Pierre-Doray . unresolved
Not sure about naming; this makes it sound like Trait is constructible from `Args...`
Perhaps
```
template <typename TraitFilter, typename... Args>
concept HasTraitMatching = ...
```Though we have HasTrait() below, and the difference is subtle.
Patrick Monette
I created a HasRequiredEnumTrait concept instead. let me know what you think.
Line 229, Patchset 4:
concept IsConstructible = (std::is_constructible_v<Trait, Args> || ...);Etienne Pierre-Doray . resolved
Patrick MonetteNit: constructible_from for consistency
Done
Line 143, Patchset 4:
(count({std::is_constructible_v<Trait, Args>...}, true) <= 1);Etienne Pierre-Doray . resolved
Patrick MonetteNit: constructible_from for consistency
Done
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Mar 27, 2026, 2:36:37 PM (6 days ago) Mar 27
to Patrick Monette, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Patrick Monette
Etienne Pierre-Doray voted and added 2 comments![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Code-Review | +1 |
2 comments
Patchset-level comments
Etienne Pierre-Doray . resolved
LGTM, thanks!
File base/traits_bag.h
Line 229, Patchset 4:
concept IsConstructible = (std::is_constructible_v<Trait, Args> || ...);Etienne Pierre-Doray . resolved
Patrick MonetteNot sure about naming; this makes it sound like Trait is constructible from `Args...`
Perhaps
```
template <typename TraitFilter, typename... Args>
concept HasTraitMatching = ...
```Though we have HasTrait() below, and the difference is subtle.
I created a HasRequiredEnumTrait concept instead. let me know what you think.
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Mar 27, 2026, 4:23:56 PM (6 days ago) Mar 27
to Gabriel Charette, Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Gabriel Charette
Patrick Monette added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Gabriel Charette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gabriel Charette (Gerrit)
Mar 30, 2026, 9:21:46 PM (3 days ago) Mar 30
to Patrick Monette, Gabriel Charette, Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Patrick Monette
Gabriel Charette voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Mar 31, 2026, 12:20:20 AM (3 days ago) Mar 31
to Gabriel Charette, Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Patrick Monette voted and added 1 comment![]( "Open in Gerrit")
Votes added by Patrick Monette
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Mar 31, 2026, 12:21:10 AM (3 days ago) Mar 31
to Rakina Zata Amni, Gabriel Charette, Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Rakina Zata Amni
Patrick Monette voted and added 1 comment![]( "Open in Gerrit")
Votes added by Patrick Monette
| Auto-Submit | +1 |
1 comment
Patchset-level comments
Patrick Monette . resolved
+Rakina for content/browser PTAL
Related details
Attention is currently required from:
- Rakina Zata Amni
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Mar 31, 2026, 8:46:55 AM (2 days ago) Mar 31
to Patrick Monette, Gabriel Charette, Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, scheduler...@chromium.org
Attention needed from Patrick Monette
Rakina Zata Amni voted![]( "Open in Gerrit")
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Mar 31, 2026, 9:43:16 AM (2 days ago) Mar 31
to Patrick Monette, Rakina Zata Amni, Gabriel Charette, Etienne Pierre-Doray, chromium...@chromium.org, scheduler...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[base] Improve traits_bag.h error messages with C++20 concepts
This change refactors base/traits_bag.h to provide clearer diagnostics
when invalid traits are passed to TaskTraits and other trait-based
constructors (e.g., duplicate traits or missing required traits).
It implements a "fallback pattern" using C++20 concepts:
- Constrained overloads are used to handle valid trait combinations.
- Unconstrained overloads are provided as a fallback to trigger a
static_assert with a descriptive error message.
The fallback overloads include a dummy return value to keep the
constexpr function structurally valid. This prevents Clang from emitting
redundant "must be initialized by a constant expression" errors at the
call site after the static_assert fails.
Additionally:
- HasTrait is converted from a struct to a constexpr function template.
- Updated various .nc (no-compile) tests to reflect the cleaner error
output and the removal of redundant diagnostic noise.
Bug: 441951621
Change-Id: I9777ad642c9e9aea2a6fc95c4143506d393fcc3d
Auto-Submit: Patrick Monette <pmon...@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etie...@chromium.org>
Reviewed-by: Gabriel Charette <g...@chromium.org>
Commit-Queue: Rakina Zata Amni <rak...@chromium.org>
Reviewed-by: Rakina Zata Amni <rak...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1607830}
Files:
- M base/task/task_traits.h
- M base/task/task_traits_nocompile.nc
- M base/traits_bag.h
- M base/traits_bag_nocompile.nc
- M content/browser/browser_task_traits_nocompile.nc
Change size: M
Delta: 5 files changed, 90 insertions(+), 46 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Rakina Zata Amni, +1 by Etienne Pierre-Doray, +1 by Gabriel Charette
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages