Skip to content

refactor: drop const-stripping casts in eval_ctx setter calls#4

Merged
kjdev merged 1 commit into
mainfrom
refactor/drop-eval-ctx-const-casts
Jun 1, 2026
Merged

refactor: drop const-stripping casts in eval_ctx setter calls#4
kjdev merged 1 commit into
mainfrom
refactor/drop-eval-ctx-const-casts

Conversation

@kjdev

@kjdev kjdev commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Refactor
    • Streamlined internal evaluation context setup by removing unnecessary type conversions.

The upstream const-correctness fix landed via PR #2 (merge 65d68c9):
php_cedar_eval_ctx_set_principal/action/resource now accept
const php_cedar_str_t *. Remove the now-unnecessary (php_cedar_str_t *)
casts that previously stripped const from the request arguments.
@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 864c69e7-adba-404f-aec5-2bcfb19ba58b

📥 Commits

Reviewing files that changed from the base of the PR and between 02035e8 and d9dd204.

📒 Files selected for processing (1)
  • cedar.c

📝 Walkthrough

Walkthrough

The pull request removes explicit (php_cedar_str_t *) pointer casts from three function calls in cedar_evaluate_request(). The arguments are already the correct type, so the casts were redundant and are now passed directly to php_cedar_eval_ctx_set_principal(), php_cedar_eval_ctx_set_action(), and php_cedar_eval_ctx_set_resource().

Changes

Eval Context Initialization Cleanup

Layer / File(s) Summary
Remove redundant pointer casts from eval context calls
cedar.c
Explicit (php_cedar_str_t *) casts are removed from three consecutive function calls in cedar_evaluate_request() (lines 1010–1012), simplifying the code while preserving type safety.

Estimated Code Review Effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A cast removed here, a cast removed there,
Now arguments pass without needless care,
The types align true, no magic required,
Just cleaner code, as elegance inspired!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring change: removing const-stripping casts from eval_ctx setter function calls, which aligns with the upstream const-correctness fixes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/drop-eval-ctx-const-casts

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
cedar.c

cedar.c:15:10: fatal error: 'php.h' file not found
15 | #include "php.h"
| ^~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/d9dd2047c5a1cbe3c6fbba368d274adff8ab944c-7b81cec47cd86491/tmp/clang_command_.tmp.5e6920.txt
++Contents of '/tmp/coderabbit-infer/d9dd2047c5a1cbe3c6fbba368d274adff8ab944c-7b81cec47cd86491/tmp/clang_command_.tmp.5e6920.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-triple"
"x86_64-unknown-linux-gnu" "-emit-obj" "-mrelax-all" "-disable-free"
"-clear-ast-before-backend" "-disabl

... [truncated 638 characters] ...

stem"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/lib/clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-ferror-limit" "19" "-fgnuc-version=4.2.1" "-fskip-odr-check-in-gmf"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/7b81cec47cd86491/file.o" "-x" "c" "cedar.c" "-O0"
"-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"


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.

❤️ Share

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

@kjdev

kjdev commented Jun 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@kjdev kjdev merged commit e8f7dc7 into main Jun 1, 2026
7 checks passed
@kjdev kjdev deleted the refactor/drop-eval-ctx-const-casts branch June 1, 2026 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant