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

Allow concatenation of string literals at compile time #1299

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

vlstill
Copy link
Contributor

@vlstill vlstill commented Aug 7, 2024

This gives wording to #1297, the compile-time concatenation of strings. The P4C part of this is p4lang/p4c#4856. The changes are relatively small, I hope I have not missed any more parts that mention that there are no string operations. Note that as stated here, it also allows string concatenation in @name, @deprecated and @noWarn pragmas. The last one has probably no use for it, but I think it is better to be consistent there.

Resolves #1297.

@vlstill vlstill changed the title Add support for string concatenation Allow concatenation of string literals at compile time Aug 7, 2024
@vlstill
Copy link
Contributor Author

vlstill commented Aug 14, 2024

Resulting PDF spec P4-16-spec.pdf.

@vlstill vlstill marked this pull request as ready for review August 14, 2024 16:05
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
@vlstill
Copy link
Contributor Author

vlstill commented Sep 15, 2024

@jafingerhut, @jonathan-dilorenzo, @rcgoodfellow, any comments? I'd love for this one to get in before the next spec revision goes out.

Copy link
Collaborator

@rcgoodfellow rcgoodfellow left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Just a few nits.

p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
Signed-off-by: Vladimir Still <[email protected]>
@vlstill
Copy link
Contributor Author

vlstill commented Sep 22, 2024

Thanks for the comments and sorry for the delayed response. I think I've fixed all the issues now.

@jafingerhut
Copy link
Collaborator

jafingerhut commented Nov 14, 2024

@vlstill Sorry for the inconvenience, but we have, after several months of planning, transitioned the P4 language specification source from Madoko to AsciiDoc. Thus most or all of your PRs will need to be updated so that they apply to that version. Hopefully this should be fairly straightforward, e.g. by looking at examples of how things like section headings, bullet items, etc. are marked up in the latest version of the file P4-16-spec.adoc

If you have questions on how to change things for AsciiDoc, feel free to ask. Note that creating a new PR with similar changes as this PR might be easier than updating this PR.

@vlstill
Copy link
Contributor Author

vlstill commented Nov 28, 2024

@jafingerhut, no problem, I've refreshed the PR for the new asciidoc documentation.

@vlstill
Copy link
Contributor Author

vlstill commented Dec 10, 2024

@jonathan-dilorenzo, @rcgoodfellow this was already discussed, now there is just updated wording in the new template, can we get this merged now?

@vlstill vlstill added the ldwg-discussion Plan to discuss at next LDWG label Jan 6, 2025
@vlstill vlstill force-pushed the string-concat branch 2 times, most recently from eb9119f to 2651459 Compare February 3, 2025 13:33
Signed-off-by: Vladimír Štill <[email protected]>
@vlstill vlstill force-pushed the string-concat branch 3 times, most recently from 949d2ee to 958c576 Compare February 3, 2025 13:38
@vlstill vlstill requested a review from jaehyun1ee February 3, 2025 13:44
Copy link
Contributor

@jaehyun1ee jaehyun1ee left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonathan-dilorenzo
Copy link
Collaborator

@jonathan-dilorenzo TAL at this and give comments or merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ldwg-discussion Plan to discuss at next LDWG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow compile-time concatenation of strings
5 participants