Skip to content

Revise enum code gen#1011

Open
bocchino wants to merge 13 commits into
mainfrom
issue-1001-invalid-enum
Open

Revise enum code gen#1011
bocchino wants to merge 13 commits into
mainfrom
issue-1001-invalid-enum

Conversation

@bocchino

@bocchino bocchino commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

This PR revises the isValid function for enums so the implementation does not rely on undefined behavior in C++. It also incidentally modernizes typedef to using in the generated code.

Closes #1001.

Tandem with this branch: https://github.com/bocchino/fprime/tree/fpp-issue-1001-invalid-enum.

@bocchino bocchino added the fprime-fpp tandem F Prime and FPP tandem development label Jun 4, 2026

@Kronos3 Kronos3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some minor nits.

Comment thread compiler/tools/fpp-to-cpp/test/enum/AliasSerialTypeEnumAc.ref.cpp
{
SerialType es;
Fw::SerializeStatus status = buffer.deserializeTo(es, mode);
if ((status == Fw::FW_SERIALIZE_OK) && !isValid(es)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this->isValid()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an instance member, I would agree. However, isValid is a static member. Are we sure we want to use this-> here? It seems misleading to me, because in fact this has no part in the dispatch of that function.

@LeStarch Do you have an opinion here? I guess the F Prime style guide does say to use this-> for all class members. https://github.com/nasa/fprime/wiki/F´-Style-Guidelines#class-self-references-this-

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right I didnt realize this was a static function. Though that does bring up an interesting point, maybe ClassName::isValid() is best since it's more clear what the function is.

|| ((e >= B) && (e <= B))
|| ((e >= C) && (e <= C))
|| ((e >= D) && (e <= D));
return isValid(this->e);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this->isValid()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fprime-fpp tandem F Prime and FPP tandem development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fpp Enum Autocode Uses Undefined Behavior

2 participants