feat: Overload client functions - #129
Conversation
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <115723925+NeaguGeorgiana23@users.noreply.github.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
…pdate_global_api
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
📝 WalkthroughWalkthrough
ChangesEvaluationOptions evaluation support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Features
participant ClientAPI
participant EvaluateFlag
participant ProviderCallback
Features->>ClientAPI: request flag value with EvaluationOptions
ClientAPI->>EvaluateFlag: pass context and options
EvaluateFlag->>ProviderCallback: dispatch typed evaluation with options
ProviderCallback-->>ClientAPI: return evaluation result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
…verload_client_functions
Signed-off-by: NeaguGeorgiana23 <115723925+NeaguGeorgiana23@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openfeature/client_api.h`:
- Around line 102-109: Remove one duplicate declaration set for AddHooks,
AddHook, and GetHooks in ClientAPI, retaining exactly one declaration of each
method with its existing signatures and override specifiers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72258ec3-7646-44db-9ace-43f58c46c062
📒 Files selected for processing (5)
openfeature/BUILDopenfeature/client_api.cppopenfeature/client_api.hopenfeature/features.htest/client_api_test.cpp
| // Adds one or more hooks to the client-level hook repository. | ||
| void AddHooks(std::vector<std::shared_ptr<GeneralHook>> hooks) override; | ||
|
|
||
| // Adds a single hook to the client-level hook repository. | ||
| void AddHook(std::shared_ptr<GeneralHook> hook) override; | ||
|
|
||
| // Retrieves all configured client-level hooks. | ||
| std::vector<std::shared_ptr<GeneralHook>> GetHooks() const override; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline openfeature/client_api.h --match ClientAPI --view expanded
sed -n '95,120p' openfeature/client_api.hRepository: open-feature/cpp-sdk
Length of output: 1413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,185p' openfeature/client_api.h
printf '\nDuplicate declarations:\n'
python3 - <<'PY'
from pathlib import Path
import re
text = Path("openfeature/client_api.h").read_text()
patterns = [
r'void AddHooks\(std::vector<std::shared_ptr<GeneralHook>> hooks\) override;',
r'void AddHook\(std::shared_ptr<GeneralHook> hook\) override;',
r'std::vector<std::shared_ptr<GeneralHook>> GetHooks\(\) const override;',
]
for pattern in patterns:
matches = list(re.finditer(pattern, text))
print(pattern, "count=", len(matches),
"lines=", [text.count("\n", 0, m.start()) + 1 for m in matches])
PY
printf '\nAvailable C++ compiler:\n'
command -v c++ || true
command -v g++ || trueRepository: open-feature/cpp-sdk
Length of output: 8684
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'struct Base { virtual void f() = 0; };' \
'struct Derived : Base { void f() override; void f() override; };' \
'int main() { return 0; }' |
c++ -std=c++17 -fsyntax-only -x c++ -
printf 'standalone duplicate-member declaration accepted\n'Repository: open-feature/cpp-sdk
Length of output: 260
🌐 Web query:
C++ standard duplicate identical member function declarations in the same class allowed or ill-formed
💡 Result:
In C++, declaring the exact same member function twice within the same class definition is generally ill-formed, as the C++ standard explicitly states that a member shall not be declared twice in the member-specification of a class [1][2]. However, this rule is subject to specific nuances and exceptions: 1. Overloading vs. Redeclaration: If you declare two member functions with the same name, they are not necessarily "duplicates." If their parameter-type-lists or cv-qualifiers differ in ways that allow for function overloading, the compiler treats them as separate overloads rather than a forbidden duplicate declaration [3][4]. The program only becomes ill-formed if the signatures are identical (or equivalent) in a way that does not constitute valid overloading [5][3]. 2. General Rule for Entities: Under the general rules for conflicting declarations in C++, two declarations in the same scope cannot reintroduce the same entity unless they are compatible (e.g., in the case of overloading) [5][6]. If two declarations for the same member function appear in the same class scope and have identical signatures, it is considered a redeclaration of the same entity and is ill-formed [5][3]. 3. Exceptions: The standard provides limited exceptions where a member may be redeclared. For example, nested classes or member class templates can be declared and then later defined [1][4]. Additionally, an enumeration can be introduced with an opaque-enum-declaration and later redeclared with an enum-specifier [2][4]. These exceptions do not apply to standard member functions. In summary, attempting to provide an identical declaration for a member function in the same class will cause a compiler error because it violates the restriction against declaring the same member multiple times [1][2]. If you intended to overload the function, you must ensure the signatures differ sufficiently according to C++ overload resolution rules [3][4].
Citations:
- 1: https://eel.is/c++draft/class.mem.general
- 2: https://timsong-cpp.github.io/cppwp/std23/class.mem.general
- 3: https://timsong-cpp.github.io/cppwp/std20/over
- 4: https://cplusplus.github.io/CWG/issues/2979.html
- 5: https://cppreference.net/cpp/language/conflicting_declarations.html
- 6: https://cppref.node4good.org/cpp/language/conflicting_declarations.html
Remove the duplicate hook declarations.
ClientAPI declares AddHooks, AddHook, and GetHooks twice with identical signatures. This makes the class definition ill-formed and blocks compilation. Remove either declaration set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openfeature/client_api.h` around lines 102 - 109, Remove one duplicate
declaration set for AddHooks, AddHook, and GetHooks in ClientAPI, retaining
exactly one declaration of each method with its existing signatures and override
specifiers.
This PR
EvaluationOptionsoverloads for all typed flag evaluation methods (GetBooleanValue,GetStringValue,GetIntegerValue,GetDoubleValue,GetObjectValue) in theFeaturesinterface andClientAPIimplementation per OpenFeature Specification Requirements 1.3.1.1 and 1.5.1.(flag_key, default_value, options)and(flag_key, default_value, context, options)call signatures.ClientAPI::EvaluateFlagand private typed evaluation helpers to accept and propagatestd::optional<EvaluationOptions>.:evaluation_optionsdependency to the:featureslibrary target inopenfeature/BUILD.test/client_api_test.cppverifying each typed evaluation method withEvaluationOptionsand combinedEvaluationContext+EvaluationOptions.Related Issues
Fixes #128
Notes
options.hooks) and metadata hints (options.hook_hints) to individual evaluation calls.Follow-up Tasks
ClientAPI::EvaluateFlagto execute invocation-level hooks and propagateHookHints.Get*Details) returning `Flag