fix: Handle global array/struct variable paths in debug.c generation#25
Conversation
Fix incorrect C path generation for global variables with array or struct access patterns. Previously, paths like CONFIG0.GLOBALVAR.value.table[0] were incorrectly converted to GLOBALVAR__value.table[0], causing compilation errors because the variable is actually declared as CONFIG0__GLOBALVAR. The fix now correctly handles: - Global ARRAY variables (e.g., ARRAY [1..10] OF DINT) - Global Function Block instances (e.g., TON, PID) - Global ARRAY of Function Blocks (e.g., ARRAY [1..2] OF TON) For paths starting with CONFIG and containing .value. access patterns, the path is now correctly constructed as CONFIG0__VARNAME.value.xxx Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect C path generation for global variables that use array indexing or struct field access. The issue occurred when global variables declared at the CONFIG level (like CONFIG0.GLOBALVAR.value.table[0]) were incorrectly converted to GLOBALVAR__value.table[0], causing compilation errors since the actual declaration is CONFIG0__GLOBALVAR.
Changes:
- Added detection logic for CONFIG-level global variables with
.value.access patterns - Modified C path generation to preserve CONFIG prefix for these cases
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # These should become: CONFIG0__VARNAME.value.table[x] or CONFIG0__VARNAME.value.fieldname | ||
| if parts[0].startswith("CONFIG") and parts[2].startswith("value"): | ||
| # Global array or struct access - keep CONFIG prefix | ||
| attrs["C_path"] = parts[0] + "__" + parts[1] + "." + parts[2] |
There was a problem hiding this comment.
This implementation only handles paths with exactly 3 parts (CONFIG0.VARNAME.value), but the PR description mentions supporting patterns like CONFIG0.GLOBALVAR.value.table[0] which would have 4 or more parts. The remaining parts (beyond parts[2]) are discarded. Consider joining all parts from index 2 onwards: attrs['C_path'] = parts[0] + '__' + parts[1] + '.' + '.'.join(parts[2:])
| # Check if this is a global array/struct variable at CONFIG level | ||
| # Pattern: CONFIG0.VARNAME.value.table[x] or CONFIG0.VARNAME.value.fieldname | ||
| # These should become: CONFIG0__VARNAME.value.table[x] or CONFIG0__VARNAME.value.fieldname | ||
| if parts[0].startswith("CONFIG") and parts[2].startswith("value"): |
There was a problem hiding this comment.
This condition assumes parts has at least 3 elements, but there's no length check before accessing parts[2]. This will raise an IndexError if the path has fewer than 3 parts. Add a length check: if len(parts) >= 3 and parts[0].startswith('CONFIG') and parts[2].startswith('value'):
Fix incorrect C path generation for global variables with array or struct access patterns. Previously, paths like CONFIG0.GLOBALVAR.value.table[0] were incorrectly converted to GLOBALVAR__value.table[0], causing compilation errors because the variable is actually declared as CONFIG0__GLOBALVAR.
The fix now correctly handles:
For paths starting with CONFIG and containing .value. access patterns, the path is now correctly constructed as CONFIG0__VARNAME.value.xxx