Skip to content

feat: Clang --werror fixes & Clang-Format Standardisation#2174

Draft
ninetailedtori wants to merge 5 commits into
ValveSoftware:masterfrom
ninetailedtori:formatting
Draft

feat: Clang --werror fixes & Clang-Format Standardisation#2174
ninetailedtori wants to merge 5 commits into
ValveSoftware:masterfrom
ninetailedtori:formatting

Conversation

@ninetailedtori
Copy link
Copy Markdown

  • .clang-format standardisation
  • add clang-format into workflow to enforce this on repo-side.
  • meson formatting.
  • add clang --werror build fixes.

Signed-off-by: Toria <ninetailedtori@uwu.gal>
- .clang-format standardisation
- clang-format into workflow
- meson cleanup
- add clang `--werror` build fixes.

Signed-off-by: Toria <ninetailedtori@uwu.gal>
Signed-off-by: Toria <ninetailedtori@uwu.gal>
@ninetailedtori ninetailedtori marked this pull request as draft May 12, 2026 18:47
Signed-off-by: Toria <ninetailedtori@uwu.gal>
Signed-off-by: Toria <ninetailedtori@uwu.gal>
@ninetailedtori
Copy link
Copy Markdown
Author

ninetailedtori commented May 13, 2026

I can't compile it rn, but I can't work out where the bug is. Would adore some help with this one, but I think it's my own machine.

Also @misyltoad updated your git submods and wraps to use your new username :3

@pchome
Copy link
Copy Markdown

pchome commented May 13, 2026

"My first 'Fuck you, world!' contribution."

Files changed 116 / +38,124 / -31,825

So, all those 90 open PRs must be redone, most distributions (from the list) must recreate their patches, most forks and maybe even Valve themselves too ...

IDK, LGTM!

@ninetailedtori
Copy link
Copy Markdown
Author

I wouldn't say they're that egregious. This is a wrap fallback adding system, and it manages the submodules in a unified method, that links with meson directly. It can always use system directly first, and most distributions like to package their OWN dependencies as opposed to using wrap, see Arch with arch-meson, designed with --wrapmode=nodownload and wraps disabled outside of forced fallback. I can always drop back the git submodules in, and revert this, but the idea was to clean up the meson build which has stayed rather...stagnant for the state of back-compat, and it's not too violent a change. Besides, we wouldn't be including this in a minor change, this would be in a major version change, as is natural with potentially breaking changes.

@pchome
Copy link
Copy Markdown

pchome commented May 14, 2026

I mean formatting, not meson changes specifically.

But ok, lets pretend it going to be a thing somehow, maybe 4.0 branch or so, here is some notes:

.editorconfig
  1. meson.build common spacing is 2, current spacing is 2, maybe keep it with
    [meson.build]
    indent_size = 2
  2. The reason people have trim_trailing_whitespace = false is to hint editor to not change anything other than
    actual code change. No one like to review PRs where one line was actual change and 100 other edits was just "cleanup".
.clang-format
  1. If formatting going to change every file why not change tabs to spaces everywhere?
    The reason I personally do not like tabs: I use many different tools1 during development,
    not all of them respect the same tab width. So when you staring at the same peace of code
    next time you see it you do not need to reread it, it's like code map near the scroll bar in some editors.
    You understand it is the same code, and then boom ... new indentation. Something like this,
    very annoying if I have to double-check is everything OK. (not to start TAB vs Space war, just personal experience)
  2. Consider increasing column width to at least 100, c++ types sometimes very verbose + 4 spaces indentation ...
    UseTab: Never
    ColumnLimit: 100
  3. Why something( ) [space inside empty brackets]?
  4. Consider using NamespaceIndentation: None, -4 spaces to all code
  5. Consider always using braces after if statements: currently gamescope code is very complex mess, there is
    a chance something in current code looks like
    if (something)
        dosomething();
        something_wrongly_indented();
    something_else();
    I personally once caught myself doing something like this in the gamescope code, so it's a trap for people
    who used to if {} else {}.
  6. My personal preference in modern c++ code: "east const": int const* p; or int const& cref;.
    Some people even use int long const* p;. Could be in consistence with auto and trailing return types.
    This is just a FYI in case maintainers agree with this.
.clang-tidy (going all-in, I mean why not add linter config too)
  1. Minimal configuration could be like this at the begining (to minimize warnings, to avoid "ad blindness")
    Checks: >
      ,bugprone-*
      ,-bugprone-easily-swappable-parameters
      ,concurrency-*
      ,performance-*
      ,
  2. As linter depend on clangd it may require compile_flags.txt tune to act properly, here is one based on my own:
    -std=c++20
    -isystem/usr/include/SDL2
    -I./build/src
    -DHAVE_SDL2
    -DHAVE_LIBCAP
    -DWLR_USE_UNSTABLE
    -UHAVE_PIPEWIRE
    

All this is just my observation and my own experience. Treat as FYI.

EDIT: I also like trailing return types.
Because my includes looks like this:

  auto Init() -> bool override;
  auto PostInit() -> bool override;
  auto GetInstanceExtensions() const -> std::span<char const* const> override;
  auto GetDeviceExtensions(VkPhysicalDevice pVkPhysicalDevice) const -> std::span<char const* const> override;
  auto GetPresentLayout() const -> VkImageLayout override;
  void GetPreferredOutputFormat(uint32_t* pPrimaryPlaneFormat, uint32_t* pOverlayPlaneFormat) const override;
  auto ValidPhysicalDevice(VkPhysicalDevice pVkPhysicalDevice) const -> bool override;

not like this:

		virtual bool Init() override;
		virtual bool PostInit() override;
		virtual std::span<const char *const> GetInstanceExtensions() const override;
		virtual std::span<const char *const> GetDeviceExtensions( VkPhysicalDevice pVkPhysicalDevice ) const override;
		virtual VkImageLayout GetPresentLayout() const override;
		virtual void GetPreferredOutputFormat( uint32_t *pPrimaryPlaneFormat, uint32_t *pOverlayPlaneFormat ) const override;
		virtual bool ValidPhysicalDevice( VkPhysicalDevice pVkPhysicalDevice ) const override;

Just ,modernize-use-trailing-return-type to .clang-tidy config:
https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-trailing-return-type.html

Footnotes

  1. Tools are: git diff, git format-patch, cat patch-name.patch, less patch-name.patch, mcedit patch-name.patch or vim patch-name.patch, reviewing my own changes in editor's diff tool or third-party diff tool, etc...

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.

2 participants