Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make code gen support EmptyGram #1121

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Make code gen support EmptyGram #1121

merged 1 commit into from
Nov 28, 2023

Conversation

tuxji
Copy link
Contributor

@tuxji tuxji commented Nov 20, 2023

Fix the Daffodil C code generator to stop crashing on someone's private DFDL schema. Their schema is private and too large to include, but somehow it creates an EmptyGram object. Make sure the code gen recognizes the EmptyGram object instead of crashing when seeing it.

c/DaffodilCCodeGenerator.scala: Add clause to support EmptyGram.

DAFFODIL-2863

@tuxji tuxji self-assigned this Nov 20, 2023
@@ -275,6 +276,7 @@ object DaffodilCCodeGenerator
case g: ElementParseAndUnspecifiedLength =>
elementParseAndUnspecifiedLengthGenerateCode(g, cgState)
case g: ElementUnused => noop(g)
case g: Gram if g eq EmptyGram => noop(g)
Copy link
Member

Choose a reason for hiding this comment

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

It might better to add this as the first case instead:

case g: Gram if g.isEmpty => noop(g)

This will still noop EmptyGram's like you have, but also noop non-EmptyGram grammars that determine that they are empty. I'm not sure if it's possible for codgen-c to walk into those or if you'll only ever see empty grammars that are EmptyGram, but it wouldn't hurt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I have improved the fix as suggested and added a new test case with a test schema containing an empty gram. Ready for review again.

Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

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

+1 👍

@tuxji tuxji requested review from mbeckerle, mike-mcgann, stricklandrbls, shanedell and olabusayoT and removed request for mbeckerle November 27, 2023 14:21
Fix the Daffodil C code generator to stop crashing when a schema
contains an empty grammar object.  Make sure the code gen recognizes
empty grams instead of crashing when seeing them.

c/DaffodilCCodeGenerator.scala: Add clause to support empty grams.

c/TestDaffodilCExamplesGenerator.scala: Add test which fails or passes
without or with above fix to verify that fix actually works.

DAFFODIL-2863
@tuxji tuxji force-pushed the ji/emptygram/2863 branch from 8886d64 to ba9bfd9 Compare November 28, 2023 14:40
@tuxji tuxji merged commit 8a3d607 into apache:main Nov 28, 2023
9 checks passed
@tuxji tuxji deleted the ji/emptygram/2863 branch November 28, 2023 14:44
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.

3 participants