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

Remove trailing whitespaces. #1

Closed
wants to merge 1 commit into from
Closed

Conversation

eyalz800
Copy link

@eyalz800 eyalz800 commented Sep 17, 2022

Hi,

Great work, hope you don't mind to remove trailing whitespaces as it can be distracting in some IDEs.

By the way - it could be nice to sometime convert the cppfront code itself to cpp2, and see how it goes.

@hsutter
Copy link
Owner

hsutter commented Sep 18, 2022

@eyalz800 Thanks! I noticed that this affects all the files, so I just went through and re-saved them all myself in VS Code with the trim-trailing-whitespace option set. I'll check in that commit instead of this PR. Thanks for pointing this out!

Yes, I intend to start writing parts of cppfront itself in Cpp2 syntax in the medium term.

@hsutter hsutter closed this in 58bb182 Sep 18, 2022
Azmah-Bad pushed a commit to Azmah-Bad/cppfront that referenced this pull request Feb 24, 2023
hsutter added a commit that referenced this pull request Apr 1, 2023
Note this change adds `{` `}` around member assignment statements in assignment operators.

That change now triggers Clang's `-Wbraced-scalar-init` warning, but this warning appears to be spurious. See the open Clang issue here llvm/llvm-project#54702 which mentions the same justification I had in mind (disallow implicit narrowing, that's the #1 reason to recommend using `{` `}` init where possible). Unfortunately that Clang issue still open without comments after a year and so it's hard to know the status.

(BTW, what a coincidence -- I actually hit this on the exact first anniversary of that issue, which also happens to be April 1 but I swear this is not a joke and I think the LLVM issue wasn't either...)
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
Note this change adds `{` `}` around member assignment statements in assignment operators.

That change now triggers Clang's `-Wbraced-scalar-init` warning, but this warning appears to be spurious. See the open Clang issue here llvm/llvm-project#54702 which mentions the same justification I had in mind (disallow implicit narrowing, that's the hsutter#1 reason to recommend using `{` `}` init where possible). Unfortunately that Clang issue still open without comments after a year and so it's hard to know the status.

(BTW, what a coincidence -- I actually hit this on the exact first anniversary of that issue, which also happens to be April 1 but I swear this is not a joke and I think the LLVM issue wasn't either...)
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