external/: ADD C++17 support

[abhinav-s235]

This commit add the support of C++17 in TizenRT

[seokhun-eom24]

Automated nightly Codex PR review
PR: #7282 — external/: ADD C++17 support
PR URL: #7282
GitHub updatedAt: 2026-04-17T07:13:43Z
Refreshed (UTC): 2026-04-17 17:46:01 UTC
Refreshed (KST): 2026-04-18 02:46:01 KST
Comment model: single evolving Codex-authored PR comment

PR #7282 — external/: ADD C++17 support

This review done by codex. AI reviews can be inaccurate.
UTC: 2026-04-17 17:44 UTC
KST (UTC+9): 2026-04-18 02:44 KST

---

Repository: Samsung/TizenRT

Base → Head: master (2810e6d2ce6d245303287bb91170981211def169) → pr/7282

HEAD Commit: 16c9d4cf4ea14195f6c701b73c5f829d38fcf8dc

Scope: Adds libc++ variant plus the utility / any in-place tag updates needed to expose the new header in TizenRT's C++17 library surface.

---

🔎 Review Summary

Category	Status
Build / Compile Status	⚠️
Functional Correctness	❌
Validation Coverage	⚠️

Final Verdict: ❗ Request Changes

---

🚨 Must-Fix Issues

1. std::variant::emplace is exposed with the wrong C++17 return type

Item	Details
Location	external/include/libcxx/variant:1153
Severity	High
Type	Functional correctness

Problem
All four public std::variant::emplace overloads are declared and implemented as void. In C++17 these overloads must return a reference to the newly active alternative. As written, any conforming C++17 code such as auto& x = v.emplace<int>(42); fails to compile against this library, so the patch does not actually provide a standards-compatible std::variant surface.

Impact

• Breaks valid C++17 call sites that depend on the standard emplace contract.
• Leaves TizenRT's libc++ surface inconsistent with the rest of the C++17 API work already present in-tree.
• Undercuts the PR's stated goal of adding C++17 support because a core library type is still API-incompatible.

Evidence

• external/include/libcxx/variant:1153 — index-based emplace(_Args&&...) returns void.
• external/include/libcxx/variant:1166 — index-based initializer-list overload also returns void.
• external/include/libcxx/variant:1177 — type-based emplace(_Args&&...) returns void.
• external/include/libcxx/variant:1190 — type-based initializer-list overload also returns void.
• external/include/libcxx/optional:752-763 — the existing C++17 optional::emplace implementation in this tree already switches to returning value_type&, which is the same standards pattern variant should follow.

Required Fix
Files to edit:

• external/include/libcxx/variant — the four public emplace overloads

Change outline:

• Change the two index-based overloads to return variant_alternative_t<_Ip, variant<_Types...>>&.
• Change the two type-based overloads to return _Tp&.
• After construction, return a reference to the active alternative, for example via _VSTD::get<_Ip>(*this), so callers receive the inserted object as required by the C++17 API.

Example patch:

diff --git a/external/include/libcxx/variant b/external/include/libcxx/variant
@@
-  void emplace(_Args&&... __args) {
+  variant_alternative_t<_Ip, variant<_Types...>>& emplace(_Args&&... __args) {
     __impl.template __emplace<_Ip>(_VSTD::forward<_Args>(__args)...);
+    return _VSTD::get<_Ip>(*this);
   }
@@
-  void emplace(_Args&&... __args) {
+  _Tp& emplace(_Args&&... __args) {
     __impl.template __emplace<_Ip>(_VSTD::forward<_Args>(__args)...);
+    return _VSTD::get<_Ip>(*this);
   }

Inference:

• The exact helper used for the returned reference is inferred from static review. Returning _VSTD::get<_Ip>(*this) is the mechanically simplest fix that matches the current implementation style.

Validation Method

• Add a libcxx-test or smoke test that does std::variant<int, long> v; auto& ref = v.emplace<int>(7); static_assert(std::is_same_v<decltype(ref), int&>, "");.
• Rebuild the libc++ target and confirm the test compiles and runs for at least one CONFIG_HAVE_CXX17 defconfig.

---

🟡 Nice-to-Have Improvements

1. Add focused libcxx-test coverage for variant construction and valueless paths

Item	Details
Location	external/libcxx-test/support/variant_test_helpers.hpp:40
Severity	Medium
Type	Validation

Problem
The tree already carries variant-specific test helpers, including makeEmpty() for valueless_by_exception() scenarios, but this PR adds no matching libcxx-test coverage for the newly exposed header and API surface.

Impact

• Leaves the new variant implementation largely unproven in the TizenRT integration layer.
• The emplace signature regression above would have been caught immediately by even a small compile-time test.

Recommended Action

• Add a small external/libcxx-test/std/utilities/variant/ test set and wire it into the existing libcxx-test build.
• Cover at minimum: construction, get / get_if, visit, emplace return types, and valueless_by_exception() using the existing helper in variant_test_helpers.hpp.
• Run that test set on one C++17-enabled defconfig before merging.

Expected Output

• A libcxx-test target that compiles and passes the added variant cases.
• A review note or CI artifact showing the test run for a CONFIG_HAVE_CXX17 configuration.

---

✅ Notable Improvements

Notable Improvements

✔ The PR updates the shared in-place utility plumbing instead of hard-coding variant-only tags

• external/include/libcxx/utility:931-990 adds the in_place, in_place_type, and in_place_index objects plus the matching trait plumbing that both any and variant need.
• That is the right integration point for this tree because it keeps the C++17 construction tags centralized in one libc++ utility header.

✔ any was updated alongside utility so the tag model stays internally consistent

• external/include/libcxx/any:216-221 and external/include/libcxx/any:578-587 switch any and make_any over to the new in-place trait/object model.
• Updating both headers together avoids introducing two incompatible ways to spell in-place construction across the libc++ surface.

✔ The out-of-line bad_variant_access symbol is added to the archive build

• external/libcxx/Makefile:67 adds variant.cxx, and external/libcxx/variant.cxx:10-16 supplies the bad_variant_access::what() definition.
• That keeps the new exception type linkable instead of leaving the implementation header-only by accident.

---

🧾 Final Assessment

Must-Fix Summary

• std::variant::emplace is currently exported with void return types, which is not C++17-conforming and will reject valid downstream code.

Nice-to-Have Summary

• Add focused libcxx-test coverage for variant so API and exception-path regressions are caught automatically.

Residual Risk / Test Gap

• This review was grounded in static inspection of the checked-out PR branch and surrounding libc++ code.
• I did not run a full TizenRT board build or libcxx-test execution in this automation pass, so runtime coverage remains unverified.

---

📌 Final Verdict

❗ Request Changes

The integration approach is mostly sensible, but the public std::variant::emplace API is still incompatible with C++17. Fix that contract mismatch first, then add at least minimal variant-focused validation before merge.