Logging fmt syntax by tarek-y-ismail · Pull Request #4682 · canonical/mir

tarek-y-ismail · 2026-02-13T16:22:49Z

Closes #???

What's new?

Converts mir::log_* and Logger to use brace format syntax.

How to test

Read the code

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 107 out of 107 changed files in this pull request and generated 13 comments.

src/wayland/protocol_error.cpp

src/miral/window_management_trace.cpp

src/server/frontend_xwayland/xwayland_clipboard_source.cpp

src/platform/renderers/gl/renderer.cpp

include/core/mir/logging/logger.h

src/platforms/eglstream-kms/server/platform_symbols.cpp

src/platforms/eglstream-kms/server/platform.cpp

src/platform/renderers/gl/renderer.cpp

src/miral/static_display_config.cpp

include/core/mir/logging/logger.h

include/core/mir/log.h

src/miral/input_configuration.cpp

src/platforms/eglstream-kms/server/platform_symbols.cpp

tarek-y-ismail · 2026-02-16T11:15:06Z

src/platforms/gbm-kms/server/kms/quirks.cpp

+        auto const devnode = mgc::value_or(device.devnode(), "");
+        mir::log_debug("Quirks(egl-destroy-surface): checking device with devnode: {}, driver {}", devnode, driver);


Passing nulls as format arguments crashes.

src/platforms/x11/x11_resources.cpp

src/server/frontend_xwayland/xwayland_clipboard_source.cpp

Copilot · 2026-02-16T11:28:15Z

@tarek-y-ismail I've opened a new pull request, #4687, to work on those changes. Once the pull request is ready, I'll request review from you.

After investigation, keeping both the variadic C-style overload and the template std::format-based overload causes unavoidable ambiguity according to ISO C++. The standard treats variadic functions specially in overload resolution, making both overloads match even when the template is technically a better match. Since PR #4682 already converts all usage to the new std::format-based API, we accept this as a breaking change and remove the deprecated variadic overload. Migration: Change printf-style format specifiers (%) to std::format style ({}). Co-authored-by: tarek-y-ismail <[email protected]>

…mat` based ones

tarek-y-ismail · 2026-02-16T13:55:03Z

Having both va list and brace style logging functions (mir::logging::log and Logger::log) with the same name is ambiguous. The compiler isn't smart enough to distinguish between them through the format string (would be neato!). I see two options:

We remove the old functions, breaking ABI and API ❌
We add the new ones with a different name or under a different namespace, log_fmt or mir::log2::log for example.

tarek-y-ismail · 2026-02-16T17:01:48Z

Gradual transition options I can think of at the moment include:

Introducing a new logging namespace, users/us can the transition to use this on a call-by-call basis
Introducing a preprocessor macro to switch between the two implementations on a file-by-file basis.

AlanGriffiths · 2026-02-17T12:55:05Z

Gradual transition options I can think of at the moment include:

1. Introducing a new logging namespace, users/us can the transition to use this on a call-by-call basis

2. Introducing a preprocessor macro to switch between the two implementations on a file-by-file basis.

Provided both APIs are available I don't see the need for either of these options. Each call site requires updating the format string, changing the function called can happen at the same time.

tarek-y-ismail · 2026-02-17T13:02:36Z

Provided both APIs are available

That's the catch. see #4682 (comment)

Closes #4682 ## What's new? They require a working graphics setup, which we can't reliably expect in CI. ## How to test TICS passes. ## Checklist - [x] Tests added and pass - [ ] Adequate documentation added - [ ] (optional) Added Screenshots or videos

tarek-y-ismail requested a review from Copilot February 13, 2026 16:22

tarek-y-ismail self-assigned this Feb 13, 2026

Copilot started reviewing on behalf of tarek-y-ismail February 13, 2026 16:23 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

tarek-y-ismail force-pushed the logging-fmt-syntax branch from 0e58d23 to 966c6f9 Compare February 16, 2026 10:51

tarek-y-ismail requested a review from Copilot February 16, 2026 10:51

Copilot started reviewing on behalf of tarek-y-ismail February 16, 2026 10:51 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

tarek-y-ismail commented Feb 16, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 16, 2026

Accept breaking change: Remove variadic Logger::log to eliminate ISO C++ ambiguity #4687

Closed

3 tasks

tarek-y-ismail added 5 commits February 16, 2026 15:01

Convert log_debug to use std::format sytnax

6d62bc2

Convert log_info to use std::format sytnax

c5e5e92

Convert log_error to use std::format sytnax

9ffdd0b

Convert log_warning to use std::format sytnax

8c60260

Convert log_critical to use std::format sytnax

1a224e7

tarek-y-ismail force-pushed the logging-fmt-syntax branch from 966c6f9 to ffc982b Compare February 16, 2026 13:12

tarek-y-ismail added 2 commits February 16, 2026 15:35

Convert Logger::log to use std::format syntax

31463a7

Convert DisplayReport to use brace format syntax

7f8e1de

tarek-y-ismail force-pushed the logging-fmt-syntax branch from ffc982b to 062c628 Compare February 16, 2026 13:39

Remove variable argument list logging functions in favor of `std::for…

cb4a25c

…mat` based ones

Saviq mentioned this pull request Feb 16, 2026

tics: skip performance testing #4686

Merged

3 tasks

Reroute mir::log* to use the new fmt mir::log

9539df3

tarek-y-ismail force-pushed the logging-fmt-syntax branch from 062c628 to 9539df3 Compare February 16, 2026 14:16

AlanGriffiths closed this in #4686 Feb 17, 2026

Saviq reopened this Feb 17, 2026

		auto const devnode = mgc::value_or(device.devnode(), "");
		mir::log_debug("Quirks(egl-destroy-surface): checking device with devnode: {}, driver {}", devnode, driver);

Conversation

tarek-y-ismail commented Feb 13, 2026

What's new?

How to test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarek-y-ismail Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Feb 16, 2026

Uh oh!

tarek-y-ismail commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tarek-y-ismail commented Feb 16, 2026

Uh oh!

AlanGriffiths commented Feb 17, 2026

Uh oh!

tarek-y-ismail commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tarek-y-ismail commented Feb 16, 2026 •

edited

Loading