mavgen_cpp11: fix IndexError when an enum entry is a prefix of the enum name - #1277
Open
azrabano23 wants to merge 1 commit into
Open
mavgen_cpp11: fix IndexError when an enum entry is a prefix of the enum name#1277azrabano23 wants to merge 1 commit into
azrabano23 wants to merge 1 commit into
Conversation
…um name
The C++11 generator strips the enum name from each entry to build the
scoped `enum class` names. enum_remove_prefix() pops one leading
component of the entry for every matching component of the enum name,
but never checks that something is left. For an entry whose name is a
prefix of the enum name, e.g.
<enum name="SOME_ENUM_NAME">
<entry value="0" name="SOME_ENUM"/>
</enum>
the list of components runs empty and the next comparison raises:
File "generator/mavgen_cpp11.py", line 304, in enum_remove_prefix
if pl[i] == sl[0]:
IndexError: list index out of range
Stop stripping once a single component is left, so the entry keeps its
last component (SOME_ENUM -> ENUM). This only changes behaviour for
inputs that previously crashed: any entry that used to strip down to
exactly one component is unaffected, and regenerating C++11 headers
for message_definitions/v1.0/all.xml produces byte-identical output.
Add tests for enum_remove_prefix() and for end-to-end C++11 generation
from an XML file containing such an enum.
Fixes ArduPilot#887
Signed-off-by: Azra Bano <azrabano.work@gmail.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
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.
What was wrong
mavgen.py --lang=C++11crashes when an enum contains an entry whose name is a prefix of the enum name (issue #887):The C generator handles the same file fine; only the C++11 output is affected.
Root cause
enum_remove_prefix()builds the scopedenum classentry names by popping one leading component of the entry name for each matching component of the enum name. It never checks that at least one component is left, so forSOME_ENUMinSOME_ENUM_NAMEthe list runs empty after two pops and the third comparison indexes into an empty list. (Even if the loop ended there, the followingsl[0][0].isdigit()would fail the same way.)The fix
Stop stripping once a single component remains, so the entry keeps its last component:
SOME_ENUMbecomesSOME_ENUM_NAME::ENUM. This is the guard @shancock884 suggested on the issue.It only changes behaviour for inputs that previously crashed: an entry that used to strip down to exactly one component takes the same path as before. To confirm, I regenerated C++11 headers for
message_definitions/v1.0/all.xmlbefore and after the change; all 389.hppfiles are byte-identical (diff -r).Verification
New
tests/test_mavgen_cpp11.py:test_enum_remove_prefix: existing cases (MAV_CMD_NAV_WAYPOINT->NAV_WAYPOINT, the digit caseMAV_SYS_STATUS_SENSOR_3D_GYRO->SENSOR_3D_GYRO) plus the prefix cases from the issue.test_cpp11_generator_enum_entry_is_prefix_of_enum_name: runsmavgen.mavgen(..., language="C++11")on a temp XML with the enum above and checks the generatedtest.hppcontainsenum class SOME_ENUM_NAMEwithENUM=0andVALUE=1.Both fail on master with the
IndexErrorabove and pass with this change.Tested with Python 3.12 on macOS.
Fixes #887
🤖 Generated with Claude Code