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

Format: Update clang format #9519

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Conversation

JohnHolmesII
Copy link
Contributor

@JohnHolmesII JohnHolmesII commented Dec 29, 2020

Old clang format file was extremely broken, causing issues when viewing
code on github
, and also featuring stale values.

  • Fix indentation information
  • Force wrapping on lambdas

The main reason for this is the GitHub thing. Github uses a repos clang format file to format code views (i.e. indent size), and since the old file is broken, the diffs are very hard to read, and nested code is completely unreadable. I don't know if the updates here will allow us to enable clang-format for everyone, especially since the lambda formatter requires clang-format-11, but this should at least alleviate some issues. There seems to still be some strange behavior with lambdas when embedding in a block. For example,

suspend_work work{Prio, &op, &result, [](void* func, void* res_buf)
{
    *static_cast<std::invoke_result_t<F>*>(res_buf) = std::invoke(*static_cast<F*>(func));
}};

becomes

suspend_work work{Prio, &op, &result, [](void* func, void* res_buf)
    {
        *static_cast<std::invoke_result_t<F>*>(res_buf) = std::invoke(*static_cast<F*>(func));
    }};

So I'm not sure if its really a fix.
Also, this is a good opportunity to reconsider what styles we want to actually enforce moving forward.

If it doesn't seem worth it yet to address, might be easiest to just close the PR.

@Megamouse
Copy link
Contributor

I don't like the lambda thing. that's why I didn't add it yet.

But I haven't really noticed any problem with github 🤔

@JohnHolmesII
Copy link
Contributor Author

Wow, it was a stylesheet. Nvm about the github thing. Ironically, the issue is still there and almost identical in IDEs like CLion, where the code is still unreadable. So I would definitely like to keep the changes for at least that reason.

Even without the lambda setting, I think it is work at least setting the rest to valid values, as I have done. Also, the lambda situation could be handled manually, since normal lambdas are formatted correctly:

const auto&& bob = [](void* func, void* res_buf)
{
    *static_cast<std::invoke_result_t<F>*>(res_buf) = std::invoke(*static_cast<F*>(func));
};

suspend_work work{Prio, &op, &result, bob};

A little extra legwork, but something like that looks nice at least.

@Megamouse
Copy link
Contributor

I still don't understand how a clang-format file influences your IDE's visualization.
I never had any issues. (Maybe because I never configured it that way)

@JohnHolmesII
Copy link
Contributor Author

The first few lines where the Tab-Width etc is set to 1 makes it use tabs 1-column wide. Using clang format for visualization is on by default in Visual Studio, CLion, and VS Code. Probably more. I know that most older rpcs3 devs have clang-format disabled in visual studio, but that seems to be motivated by the fact that it is broken and reformats code wrong...

Which is partially why this is open.

@Megamouse
Copy link
Contributor

BeforeLambdaBody and BeforeWhile are not supported on the current version of VS2019.
So the formatting fails with an error message.

After removing those lines, everything seems to be fine (based on 2-3 files i formatted)

@JohnHolmesII
Copy link
Contributor Author

That is unfortunate. I unrolled the braces list precisely because BeforeLambdaBody is one I know was a real holdup (I recall discussions from discord long past...). I can just revert to the override list I suppose.

Old clang format file was extremely broken, causing issues when viewing
code on github, and also featuring stale values.

- Fix indentation information
- Force wrapping on lambdas
@Nekotekina Nekotekina merged commit e68027b into RPCS3:master Dec 30, 2020
@JohnHolmesII JohnHolmesII deleted the clang_format branch December 30, 2020 17:21
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.

None yet

3 participants