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

ttcn: add "template" ["(omit)" | "(value)" | "(present)"] #3456

Merged
merged 1 commit into from
Aug 22, 2022
Merged

ttcn: add "template" ["(omit)" | "(value)" | "(present)"] #3456

merged 1 commit into from
Aug 22, 2022

Conversation

neeels
Copy link
Contributor

@neeels neeels commented Aug 10, 2022

Modern TTCN3 has TemplateRestriction keywords, which improve detection
of invalid template use at compile time. They simply add one of
"(omit)", "(value)", "(present)" after the "template" keyword.

Example:

template (value) ts_FOO_Request(integer bar := 0) :=
{ member := bar };

template (present) tr_FOO_Request(template integer bar := *) :=
{ member := bar };

Before this patch, u-ctags would not tag these two definitons above,
making my work on the Osmocom test suite hard. (See
https://gitea.osmocom.org/ttcn3/osmo-ttcn3-hacks)

At first I tried to exactly match the three keywords within the braces,
but the token parsing does not support unrolling more than one token. So
if an unmatching token follows after the opening brace, I cannot un-get
the opening brace. The easiest solution, and one that also is tolerant
to keyword typos, is to just accept any braces after "template".

Related: ETSI ES 201 873-1 V4.14.1 § A.1.6.1.3

@masatake
Copy link
Member

Thank you. Could you add a test case?
See https://docs.ctags.io/en/latest/testing-parser.html about how to write a test case.

@masatake
Copy link
Member

@akrzyz, could you review this?

@akrzyz
Copy link
Contributor

akrzyz commented Aug 12, 2022

Looks fine but please add test case for TemplateRestriction and test there all three possible restrictions.

@codecov
Copy link

codecov bot commented Aug 14, 2022

Codecov Report

Merging #3456 (d92a92f) into master (40551e2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3456   +/-   ##
=======================================
  Coverage   83.32%   83.32%           
=======================================
  Files         219      219           
  Lines       52750    52752    +2     
=======================================
+ Hits        43956    43958    +2     
  Misses       8794     8794           
Impacted Files Coverage Δ
parsers/ttcn.c 93.28% <100.00%> (+0.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@neeels
Copy link
Contributor Author

neeels commented Aug 16, 2022

Thanks for your review!
I added Units/parser-ttcn.r/ttcn-template-restriction.d/
Hope that's all correct.

@masatake
Copy link
Member

See #3466.
Could you rebase on the latest master?

Modern TTCN3 has TemplateRestriction keywords, which improve detection
of invalid template use at compile time. They simply add one of
"(omit)", "(value)", "(present)" after the "template" keyword.

Example:

 template (value) ts_FOO_Request(integer bar := 0) :=
 	{ member := bar };

 template (present) tr_FOO_Request(template integer bar := *) :=
 	{ member := bar };

Before this patch, u-ctags would not tag these two definitons above,
making my work on the Osmocom test suite hard. (See
https://gitea.osmocom.org/ttcn3/osmo-ttcn3-hacks)

At first I tried to exactly match the three keywords within the braces,
but the token parsing does not support unrolling more than one token. So
if an unmatching token follows after the opening brace, I cannot un-get
the opening brace. The easiest solution, and one that also is tolerant
to keyword typos, is to just accept any braces after "template".

Related: ETSI ES 201 873-1 V4.14.1 § A.1.6.1.3
@neeels
Copy link
Contributor Author

neeels commented Aug 21, 2022

done

@masatake masatake merged commit dfffd3d into universal-ctags:master Aug 22, 2022
@masatake
Copy link
Member

@neeels thank you for the contribution.
@akrzyz thank you for the review.

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.

None yet

3 participants