feat: refine project constexpr generation#96
Merged
Conversation
Reviewer's GuideRefines project-level constexpr generation by making the constexpr namespace configurable, adding explicit {expr}/{string} forms, preserving include and constexpr formatting consistently in generated C++, stabilizing auto-generated module instance IDs in the YAML config, improving readability of generated module instantiations, and bumping the package version and docs to match the new behavior. Flow diagram for configurable constexpr header generationflowchart TD
Config["Config dict with constexpr_namespace, constexpr_includes, constexprs"]
GenHdr["_generate_constexpr_header(config)"]
GetNS["_get_constexpr_namespace(config)"]
ValId["_validate_cpp_identifier(name, field_name='constexpr namespace')"]
LoopInc["Loop over constexpr_includes"]
FmtInc["_format_include_directive(header)"]
Dedup["Deduplicate directives via seen_includes"]
LoopConst["Loop over constexprs items (name, spec)"]
ValRef["_validate_constexpr_ref(name)"]
FmtType["Validate non-empty type"]
FmtVal["_format_cpp_value(cpp_value, constexpr_namespace)"]
IsDict{{"value is dict?"}}
IsConstRef{{"_is_constexpr_ref(value)?"}}
IsExpr{{"_is_expr_value(value)?"}}
IsString{{"_is_string_value(value)?"}}
DictAgg["Aggregate-init nested mapping via recursive _format_cpp_value"]
RawExpr["_format_raw_expr(value['expr'])"]
RawString["_format_string_literal(value['string'])"]
IsList{{"value is list?"}}
ListAgg["Aggregate-init list via recursive _format_cpp_value"]
IsBool{{"value is bool?"}}
BoolOut["Emit true/false"]
IsStr{{"value is str?"}}
StrInstance{{"value startswith @?"}}
StrScope{{"Scoped name or enum?"}}
StrNumber{{"Numeric string?"}}
StrLit["Emit string literal via _format_string_literal"]
StrAsIs["Emit value[1:], raw scoped name or number"]
Other["Fallback str(value)"]
Config --> GenHdr
GenHdr --> GetNS --> ValId
GetNS --> GenHdr
GenHdr --> LoopInc --> FmtInc --> Dedup --> LoopInc
GenHdr --> LoopConst --> ValRef --> FmtType --> FmtVal
FmtVal --> IsDict
IsDict -- yes --> IsConstRef
IsConstRef -- yes --> RawConstRef["Emit constexpr_namespace::Name"] --> FmtVal
IsConstRef -- no --> IsExpr
IsExpr -- yes --> RawExpr --> FmtVal
IsExpr -- no --> IsString
IsString -- yes --> RawString --> FmtVal
IsString -- no --> DictAgg --> FmtVal
IsDict -- no --> IsList
IsList -- yes --> ListAgg --> FmtVal
IsList -- no --> IsBool
IsBool -- yes --> BoolOut --> FmtVal
IsBool -- no --> IsStr
IsStr -- yes --> StrInstance
StrInstance -- yes --> StrAsIs --> FmtVal
StrInstance -- no --> StrScope
StrScope -- yes --> StrAsIs --> FmtVal
StrScope -- no --> StrNumber
StrNumber -- yes --> StrAsIs --> FmtVal
StrNumber -- no --> StrLit --> FmtVal
IsStr -- no --> Other --> FmtVal
LoopConst --> GenHdr
Flow diagram for main code generation with stable instance ids and multiline instantiationflowchart TD
AConf["Config dict with global_settings, modules, constexpr_namespace"]
AMods["List of discovered module names"]
Extract["extract_constructor_args(module_dir, modules, config)"]
AutoIdxInit["auto_inst_index = {}"]
ForMod["For each mod in modules"]
BuildArgs["Build args_ordered and tmpl_ordered"]
GenId["idx = auto_inst_index.get(mod, 0); id = f'{mod}_{idx}'"]
BumpIdx["auto_inst_index[mod] = idx + 1"]
MakeEntry["Create OrderedDict with id, name, constructor_args"]
AppendEntry["Append module entry to config['modules']"]
GenMain["generate_xrobot_main_code(hw_var, modules, config)"]
GetNS["constexpr_namespace = _get_constexpr_namespace(config)"]
Headers["Emit app_framework.hpp, libxr.hpp, module headers"]
MaybeIncConst["If constexprs exists, include xrobot_constexpr.hpp"]
BodyStart["Emit XRobotMain signature, using namespace, appmgr"]
ForEntry["For each module entry in config['modules']"]
MatchName{{"Entry name in modules list?"}}
GetInstanceName["instance_name = entry['id'] (e.g. BlinkLED_0)"]
GetCtorArgs["_require_mapping(constructor_args)"]
FmtCtorArgs["_format_cpp_value(v, key, constexpr_namespace) for each"]
GetTmplArgs["_require_mapping(template_args)"]
FmtTmplArgs["_format_template_arg(v, constexpr_namespace) for each"]
BuildStmt["_format_module_instance_statement(mod, tmpl_params, instance_name, hw_var, args_list)"]
OneLine{{"Single-line length <= 100?"}}
OneLineOut["Emit: static Mod<...> id(hw, appmgr, ...);"]
NeedsWrap["Format multiline type and/or arg list"]
LoopBody["Append statement to body"]
MainLoop["Emit while(true) with appmgr.MonitorAll() and Thread::Sleep(monitor_sleep_ms)"]
AConf --> Extract
AMods --> Extract
Extract --> AutoIdxInit --> ForMod
ForMod --> BuildArgs --> GenId --> BumpIdx --> MakeEntry --> AppendEntry --> ForMod
AConf --> GenMain
AMods --> GenMain
GenMain --> GetNS --> Headers --> MaybeIncConst --> BodyStart --> ForEntry
ForEntry --> MatchName
MatchName -- no --> ForEntry
MatchName -- yes --> GetInstanceName --> GetCtorArgs --> FmtCtorArgs --> GetTmplArgs --> FmtTmplArgs --> BuildStmt
BuildStmt --> OneLine
OneLine -- yes --> OneLineOut --> LoopBody --> ForEntry
OneLine -- no --> NeedsWrap --> LoopBody --> ForEntry
ForEntry --> MainLoop
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
_get_constexpr_namespacevalidation currently restrictsconstexpr_namespaceto a single identifier (^[A-Za-z_]\w*$), which prevents using more idiomatic C++ nested namespaces likemy_project::constexprs; consider relaxing this to allow namespace-qualified identifiers (e.g., permitting::and possibly inline/anonymous namespace patterns) so users can organize constexprs in richer namespaces.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_get_constexpr_namespace` validation currently restricts `constexpr_namespace` to a single identifier (`^[A-Za-z_]\w*$`), which prevents using more idiomatic C++ nested namespaces like `my_project::constexprs`; consider relaxing this to allow namespace-qualified identifiers (e.g., permitting `::` and possibly inline/anonymous namespace patterns) so users can organize constexprs in richer namespaces.
## Individual Comments
### Comment 1
<location path="README.md" line_range="346" />
<code_context>
+- `constexpr_namespace` 可选，用于指定生成的项目级常量命名空间；默认值为 `ProjectConstexpr`。
+ `constexpr_namespace` is optional and controls the generated project-level constexpr namespace; the default is `ProjectConstexpr`.
+- `{constexpr: Name}` 会展开为 `ProjectConstexpr::Name` 或你配置的 `constexpr_namespace::Name`；若顶层存在 `constexprs`，则会额外生成 `xrobot_constexpr.hpp`。
+ `{constexpr: Name}` expands to `ProjectConstexpr::Name` or your configured `constexpr_namespace::Name`; when top-level `constexprs` exists, `xrobot_constexpr.hpp` is generated alongside the main file.
+- `{expr: Code}` 会原样输出为 C++ 表达式，`{string: Text}` 会强制输出为字符串字面量。
+ `{expr: Code}` is emitted as a raw C++ expression, while `{string: Text}` forces a string literal.
</code_context>
<issue_to_address>
**nitpick (typo):** Use plural verb form with plural subject in this sentence.
Change “when top-level `constexprs` exists” to “when top-level `constexprs` exist” to agree with the plural subject.
```suggestion
`{constexpr: Name}` expands to `ProjectConstexpr::Name` or your configured `constexpr_namespace::Name`; when top-level `constexprs` exist, `xrobot_constexpr.hpp` is generated alongside the main file.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - `constexpr_namespace` 可选，用于指定生成的项目级常量命名空间；默认值为 `ProjectConstexpr`。 | ||
| `constexpr_namespace` is optional and controls the generated project-level constexpr namespace; the default is `ProjectConstexpr`. | ||
| - `{constexpr: Name}` 会展开为 `ProjectConstexpr::Name` 或你配置的 `constexpr_namespace::Name`；若顶层存在 `constexprs`，则会额外生成 `xrobot_constexpr.hpp`。 | ||
| `{constexpr: Name}` expands to `ProjectConstexpr::Name` or your configured `constexpr_namespace::Name`; when top-level `constexprs` exists, `xrobot_constexpr.hpp` is generated alongside the main file. |
There was a problem hiding this comment.
nitpick (typo): Use plural verb form with plural subject in this sentence.
Change “when top-level constexprs exists” to “when top-level constexprs exist” to agree with the plural subject.
Suggested change
| `{constexpr: Name}` expands to `ProjectConstexpr::Name` or your configured `constexpr_namespace::Name`; when top-level `constexprs` exists, `xrobot_constexpr.hpp` is generated alongside the main file. | |
| `{constexpr: Name}` expands to `ProjectConstexpr::Name` or your configured `constexpr_namespace::Name`; when top-level `constexprs` exist, `xrobot_constexpr.hpp` is generated alongside the main file. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refines the recently added project-level constexpr generation path and folds in one missing config-generation fix that is still absent from
XRobot2.0.The first problem is that auto-generated
User/xrobot.yamlentries still omit stable instance ids. That breaks module-to-module references on the autogen path because downstream configs and examples expect names likeBlinkLED_0, but the generated config only keptname. This PR preserves auto-generated ids in the generated config so the generated C++ instance names and config references stay aligned.The second problem is that the current project-level constexpr path is still too rigid and slightly misleading for real use. The namespace is hard-coded, include rendering forces quoted includes, string-vs-expression intent still relies on inference, and long generated instantiations become hard to read. In addition, the generated
xrobot_constexpr.hppdoes not need to auto-inject module headers when it is already consumed as part of the generated entry chain.This PR makes the constexpr generation path more explicit and easier to use:
ProjectConstexpras the default namespace but adds top-levelconstexpr_namespaceoverride support{expr: ...}and{string: ...}forms so YAML can describe intent without relying only on parser heuristics<...>and"..."forms inconstexpr_includesxrobot_main.hppand limitsxrobot_constexpr.hppto explicit includes only0.2.9to0.2.10I also updated the README so the documented config syntax matches the current generator behavior and examples.
Validation used for this branch:
python3 -m compileall src/xrobot/GenerateMain.py/tmp/xrobot_constexpr_selfcontained_local_20260419/home/xiao/runs/xrobot_cfg_refine_20260419T2358Zconstexpr_namespaceplus{expr}/{string}:/home/xiao/runs/xrobot_cfg_probe_20260420T0002ZSummary by Sourcery
Refine project-level constexpr code generation and auto-generated module configs to make generated C++ instances, namespaces, and includes more flexible and aligned with configuration intent.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation: