fix: GCC compilation issue caused by template specialization by Perdixky · Pull Request #287 · clice-io/clice

Perdixky · 2025-10-25T06:58:51Z

Summary by CodeRabbit

Refactor
- Internal rewrite of async task result handling to unify void vs. value cases and add a compiler-compatibility workaround.
- Adjusted reflection member-count computation to use an internal query path rather than direct base-class counts.
Bug Fixes
- Fixed a serialization template parameter mismatch to ensure consistent (de)serialization behavior for small-string types.

coderabbitai · 2025-10-25T06:59:00Z

Walkthrough

Replaces a full void specialization in include/Async/Task.h with a templated std::same_as<void> specialization and adds a return_void() helper; updates a template parameter type in include/Support/JSON.h; and switches member_count aggregation to use impl::member_count<T>() in include/Support/Struct.h.

Changes

Cohort / File(s)	Summary
Void-specialization change `include/Async/Task.h`	Replaces the explicit `template <> struct promise_result<void>` with a templated specialization `template<std::same_as<void> V> struct promise_result<V>`, adds `return_void() noexcept`, and inserts a workaround comment referencing GCC bug 85282. Non-void `promise_result` remains unchanged.
SmallString Serde template param `include/Support/JSON.h`	Changes the `Serde` specialization template parameter from `template <std::size_t N>` to `template <unsigned N>` for `llvm::SmallString<N>`.
Member-count indirection `include/Support/Struct.h`	Updates `Struct<Inheritance<Ts...>>` to compute `member_count` using `(impl::member_count<Ts>() + ...)` instead of `(Struct<Ts>::member_count + ...)` in both occurrences. Other collection/name logic unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Verify the templated std::same_as<void> specialization compiles and behaves identically across supported compilers (GCC, Clang, MSVC).
Confirm return_void() signature and noexcept match prior semantics and that no caller relies on a different overload or ADL.
Check serialization behavior of Serde<llvm::SmallString<N>> after switching N from std::size_t to unsigned for potential template-matching or overflow edge cases.
Ensure impl::member_count<T>() yields the same values as Struct<T>::member_count in all reflectable/boundary cases and that no ODR/visibility issues appear.

Poem

🐇 I hopped through templates, nibbling a bit,
Turned a void-special into a clever fit,
SmallStrings and structs got tidied too,
Return_void hums softly — the promise stays true,
My floppy ears applaud this tidy commit.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "[Fix] GCC compilation issue caused by template specialization" directly aligns with the primary change in the pull request. The main modification in include/Async/Task.h involves replacing a full specialization of promise_result with a templated specialization to work around GCC bug 85282, which is precisely what the title describes. The title is clear, specific, and accurately captures the central objective of the changeset. While the PR includes additional modifications to JSON.h and Struct.h, these appear to be complementary changes, and the title appropriately emphasizes the most significant issue being addressed.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

include/Async/Task.h (2)
197-222: Disallow reference T to avoid optional ill-formed instantiations.

optional cannot hold reference types; guard early to improve diagnostics.
 struct promise_type : promise_base, promise_result {
+        static_assert(!std::is_reference_v<T>,
+                      "Task<T> does not support reference types; use std::reference_wrapper<T> if needed.");
         promise_type(std::source_location location = std::source_location::current()) {
338-339: Minor: make return_void constexpr.

Tiny polish; communicates intent and enables constant-eval friendliness.
-    void return_void() noexcept {}
+    constexpr void return_void() noexcept {}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a10908d and b3d3090.

📒 Files selected for processing (1)

include/Async/Task.h (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

GitHub Check: windows (windows-2025)
GitHub Check: linux (ubuntu-24.04, release)
GitHub Check: macos (macos-15, release)
GitHub Check: macos (macos-15, debug)
GitHub Check: linux (ubuntu-24.04, debug)
GitHub Check: build (ubuntu-24.04)
GitHub Check: build (windows-2025)
GitHub Check: build (macos-15)

🔇 Additional comments (2)

include/Async/Task.h (2)

7-11: Includes look right for the new implementation.

Adding , <type_traits>, and is necessary for the value storage and forwarding. LGTM.

191-196: Structural verification confirms pattern is sound; manual compile check required on target compilers.

The bool-gated template specialization pattern is correctly implemented:

Primary template (line 192) declares promise_result_impl<V, bool = std::is_void_v<V>>

Alias (line 195) correctly uses promise_result_impl<T>

Specialization for false (line 325): non-void case with std::optional<V> storage

Specialization for true (line 337): void case with no storage

This pattern correctly avoids GCC specialization pitfalls by using explicit bool NTTP gating rather than partial specialization on void/non-void.

However, the sandbox lacks C++ compilers for the requested cross-compiler verification (GCC, Clang, MSVC with C++23). Please manually verify compilation on your target compilers before merging.

…lization is not allowed outsideof namespace scope

…owed by GCC)

Perdixky · 2025-10-29T01:29:35Z

Now it can be built with gcc!

16bit-ykiko · 2025-10-29T01:45:36Z

Thanks.

coderabbitai Bot reviewed Oct 25, 2025

View reviewed changes

Comment thread include/Async/Task.h Outdated

16bit-ykiko reviewed Oct 27, 2025

View reviewed changes

Comment thread include/Async/Task.h

Use a workaround to circumvent the GCC bug where full template specia…

16a1ec3

…lization is not allowed outsideof namespace scope

Perdixky force-pushed the main branch from 7b99afc to 16a1ec3 Compare October 29, 2025 01:19

Perdixky added 2 commits October 29, 2025 09:26

Fix implicit type conversion for non-type template parameters (disall…

72dc241

…owed by GCC)

Fix class accessed before instantiation issue

0b63ae9

16bit-ykiko changed the title ~~Fix GCC compilation issue caused by promise specialization~~ [Fix] GCC compilation issue caused by promise specialization Oct 29, 2025

16bit-ykiko changed the title ~~[Fix] GCC compilation issue caused by promise specialization~~ [Fix] GCC compilation issue caused by template specialization Oct 29, 2025

16bit-ykiko merged commit 7d71c0f into clice-io:main Oct 29, 2025
10 checks passed

16bit-ykiko changed the title ~~[Fix] GCC compilation issue caused by template specialization~~ fix: GCC compilation issue caused by template specialization Apr 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: GCC compilation issue caused by template specialization#287

fix: GCC compilation issue caused by template specialization#287
16bit-ykiko merged 3 commits into
clice-io:mainfrom
Perdixky:main

Perdixky commented Oct 25, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 25, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Perdixky commented Oct 29, 2025

Uh oh!

16bit-ykiko commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Perdixky commented Oct 25, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Perdixky commented Oct 29, 2025

Uh oh!

16bit-ykiko commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Perdixky commented Oct 25, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 25, 2025 •

edited

Loading