Skip to content

Add GCC and MSVC settings to ensure C++ standard conformance#130

Merged
doomlaur merged 9 commits into
masterfrom
improve_conformance
Jan 12, 2026
Merged

Add GCC and MSVC settings to ensure C++ standard conformance#130
doomlaur merged 9 commits into
masterfrom
improve_conformance

Conversation

@doomlaur
Copy link
Copy Markdown
Contributor

@doomlaur doomlaur commented Jan 9, 2026

To ensure C++ standard conformance in the future, this PR:

  • adds -Wpedantic for GCC, and -pedantic-errors if XLNT_ALL_WARNINGS_AS_ERRORS is set
  • adds /permissive- for MSVC 2017 and newer
  • note: for Clang, we already used -Weverything, which already sets -Wpedantic

Depends on #131 to fix compilation under MSVC 2017 (see discussion below).

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 9, 2026

Pull Request Test Coverage Report for Build ffc61b29-c678-4be2-9cce-415c1e565ae2

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 82.718%

Totals Coverage Status
Change from base Build fcbaa237-6534-4795-9135-85f999fd1fde: 0.0%
Covered Lines: 11837
Relevant Lines: 14310

💛 - Coveralls

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 9, 2026

Coverage report is available at: full | review summary

@doomlaur
Copy link
Copy Markdown
Contributor Author

I think the compilation error with MSVC 2017 is due to a bug in MSVC 2017 triggered by fmt. Before writing more information, I'll update fmt very quickly to test my theory, then remove the update from this PR and add its own PR for it.

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 10, 2026

Coverage report is available at: full | review summary

@doomlaur
Copy link
Copy Markdown
Contributor Author

Alright, it seems that MSVC 2017 with /permissive- breaks due to issue fmtlib/fmt#4412 and has been fixed by PR fmtlib/fmt#4413, which is available since fmt 11.2.0. We were previously using a commit from the master branch from fmt between 11.1.2 and 11.1.3. I will send another PR for updating fmt.

While I would have liked to update our dependencies shortly before releasing XLNT 1.7.0, I will make an exception here (and we can also update fmt twice too, if we think it's worth it). I will update the other dependencies shortly before XLNT 1.7.0.

@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 10, 2026

Coverage report is available at: full | review summary

@doomlaur doomlaur mentioned this pull request Jan 10, 2026
Copy link
Copy Markdown
Member

@m7913d m7913d left a comment

Choose a reason for hiding this comment

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

I'm already approving, so you can merge the PR once the failed test is fixed (by the other PR).

doomlaur added a commit that referenced this pull request Jan 12, 2026
Updates `fmt` to 12.1.0.

This also fixes an issue when using MSVC 2017 with `/permissive-`, which
breaks due to issue fmtlib/fmt#4412 and has
been fixed by PR fmtlib/fmt#4413, which is
available since `fmt` 11.2.0. We were previously using a commit from the
`master` branch from `fmt` between 11.1.2 and 11.1.3.

For more details, see discussion on PR
#130
@m7913d
Copy link
Copy Markdown
Member

m7913d commented Jan 12, 2026

Coverage report is available at: full | review summary

@doomlaur doomlaur merged commit 9cd7274 into master Jan 12, 2026
37 checks passed
@doomlaur doomlaur deleted the improve_conformance branch January 12, 2026 05:28
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