Skip to content

Fix string alias codegen#676

Merged
bocchino merged 4 commits intomainfrom
tumbar-fix-string-aliases
Apr 11, 2025
Merged

Fix string alias codegen#676
bocchino merged 4 commits intomainfrom
tumbar-fix-string-aliases

Conversation

@Kronos3
Copy link
Copy Markdown
Collaborator

@Kronos3 Kronos3 commented Apr 10, 2025

#645 did not get the string alias codegen quite right. We need to keep the same underlying C++ type in all locations that utilize FPP strings (and their aliases).

This PR will only utilize the aliased name if the codegen expects Fw::String. We decided that the original (pre-alias) behavior should be maintained for string aliases. A C++ using AliasName = Fw::String is still generated

There was a missing testcase that caused issues while working on #659 (see updated .fpp test).

Comment thread compiler/tools/fpp-to-cpp/test/struct/alias_type.fpp Outdated
Comment thread compiler/lib/src/main/scala/codegen/CppWriter/TypeCppWriter.scala Outdated
Copy link
Copy Markdown
Collaborator

@bocchino bocchino left a comment

Choose a reason for hiding this comment

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

Good catch! This PR looks good to me. Please look at the comments I made.

@bocchino
Copy link
Copy Markdown
Collaborator

Looks good! I'm just running check-cpp now.

Comment thread compiler/lib/src/main/scala/codegen/CppWriter/TypeCppWriter.scala
@bocchino bocchino self-requested a review April 10, 2025 18:00
Copy link
Copy Markdown
Collaborator

@bocchino bocchino left a comment

Choose a reason for hiding this comment

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

Looks good! check-cpp passed.

@bocchino bocchino merged commit 4733196 into main Apr 11, 2025
11 checks passed
@bocchino bocchino deleted the tumbar-fix-string-aliases branch April 11, 2025 00: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.

2 participants