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.

Sign up for GitHub

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

Jump to bottom

ICU-22740 Add missing template arguments in MF2 parser #2971

Merged
merged 2 commits into from
May 14, 2024
Merged

ICU-22740 Add missing template arguments in MF2 parser #2971

merged 2 commits into from
May 14, 2024

Conversation

catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Apr 18, 2024 •
edited by roubert

This fixes a clang-17 -Wctad-maybe-unsupported warning.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22740
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

ALLOW_MANY_COMMITS=true

@catamorphism
ICU-22740 Add missing template arguments in MF2 parser
3075448 
This fixes a clang-17 `-Wctad-maybe-unsupported` warning.
@catamorphism catamorphism reopened this Apr 18, 2024
@catamorphism catamorphism changed the base branch from main to maint/maint-75 April 18, 2024 19:16
@catamorphism
Copy link
Contributor Author

cc @roubert @echeran

@roubert roubert changed the base branch from maint/maint-75 to main April 19, 2024 15:39
@roubert roubert self-requested a review April 19, 2024 15:51
@roubert roubert self-assigned this Apr 19, 2024
roubert
roubert previously approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@roubert roubert left a comment

Choose a reason for hiding this comment

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

LGTM

roubert
roubert reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@roubert roubert left a comment

Choose a reason for hiding this comment

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

Hm, I didn't realize before that there were configurations that used -Werror without also using -Wall -Wextra, maybe this'll be a bit easier to maintain if only those get the new warning added.

@catamorphism
Copy link
Contributor Author

Hm, I didn't realize before that there were configurations that used -Werror without also using -Wall -Wextra, maybe this'll be a bit easier to maintain if only those get the new warning added.

Fixed in b849e63

@roubert
Copy link
Member

roubert commented Apr 22, 2024

Fixed in b849e63

Now you got it backwards, -Wctad-maybe-unsupported should go together with -Werror -Wall -Wextra.

@catamorphism
Copy link
Contributor Author

Now you got it backwards, -Wctad-maybe-unsupported should go together with -Werror -Wall -Wextra.

Oops. Should be fixed now.

@catamorphism @roubert
ICU-22740 Add -Wctad-maybe-unsupported to CI jobs that use -Wall -Wex…
561de1d 
…tra.

This modifies the following jobs:

C: Linux Clang-16 WarningsAsErrors (Ubuntu 20.04)
C: macOS(Latest) Clang WarningsAsErrors
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

roubert
roubert approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@roubert roubert left a comment

Choose a reason for hiding this comment

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

LGTM

@echeran echeran merged commit 084af5c into unicode-org:main May 14, 2024
97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roubert roubert roubert approved these changes

Assignees

@roubert roubert

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@catamorphism @roubert @echeran