Skip to content

Conversation

@StephanTLavavej
Copy link
Member

  • Remove unnecessary comments in ranges tests.
    • We taught clang-format how to sort like this automatically. 😸
  • Fix x86 compiler error in stljobs.h.
    • This never compiled for x86 where size_t and SIZE_T are different types. 🙀
  • Update .clang-format for Clang 10.
    • AllowShortBlocksOnASingleLine changed from false/true to Never/Empty/Always. We still want the default of Never, as Empty appears to misbehave.
    • DeriveLineEnding was added, defaulting to true. We want to disable line ending auto-detection.
    • SortPriority was added. Its documentation is difficult to understand, but it can be used to group includes together while modifying their otherwise-lexicographic ordering. Here, I'm using it to group WinSDK includes together, while still sorting WinIoCtl.h last.
    • IncludeIsMainSourceRegex was added. This doesn't appear to be relevant to us.
    • IndentGotoLabels was added. We've avoided goto.
    • SpaceInEmptyBlock was added. We prefer the default of no space.
    • SpacesInConditionalStatement was added. We definitely prefer the default of no space.
    • SpaceBeforeSquareBrackets was added. Again, we definitely prefer the default of no space.
    • The Standard option was overhauled. Previously, Cpp11 meant "use the latest supported standard". That was confusing, so it has been deprecated in favor of Latest.
    • UseCRLF was added. We currently use CRLF for all files. (Previously, validate.cpp would detect LF files, but clang-format wouldn't fix them.)
  • In .clang-format, delete empty lines and move comment.
    • The empty lines were intended to make it easier to see customized options, but they were actually making it harder to diff the default config against our config.
    • Move the StatementMacros comment next to our customized settings, and rewrap it.
  • Reformat filesystem.cpp.

We taught clang-format how to sort like this automatically.
This never compiled for x86 where size_t and SIZE_T are different types.
AllowShortBlocksOnASingleLine changed from false/true to
Never/Empty/Always. We still want the default of Never, as Empty
appears to misbehave.

DeriveLineEnding was added, defaulting to true. We want to disable line
ending auto-detection.

SortPriority was added. Its documentation is difficult to understand,
but it can be used to group includes together while modifying their
otherwise-lexicographic ordering. Here, I'm using it to group WinSDK
includes together, while still sorting WinIoCtl.h last.

IncludeIsMainSourceRegex was added. This doesn't appear to be relevant
to us.

IndentGotoLabels was added. We've avoided goto.

SpaceInEmptyBlock was added. We prefer the default of no space.

SpacesInConditionalStatement was added. We definitely prefer the
default of no space.

SpaceBeforeSquareBrackets was added. Again, we definitely prefer the
default of no space.

The Standard option was overhauled. Previously, Cpp11 meant "use the
latest supported standard". That was confusing, so it has been
deprecated in favor of Latest.

UseCRLF was added. We currently use CRLF for all files. (Previously,
validate.cpp would detect LF files, but clang-format wouldn't fix
them.)
The empty lines were intended to make it easier to see customized
options, but they were actually making it harder to diff the default
config against our config.

Move the StatementMacros comment next to our customized settings,
and rewrap it.
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jul 22, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner July 22, 2020 02:22
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

  • Fix x86 compiler error in stljobs.h.
    • This never compiled for x86 where size_t and SIZE_T are different types. 🙀

Wait...

This never compiled for x86 where size_t and SIZE_T are different types. 🙀

...What?...

size_t and SIZE_T are different types

NOOOOOOoooooooooo.... I was so much happier not knowing this.

@statementreply
Copy link
Contributor

size_t and SIZE_T are different types

bool/BOOL, byte/BYTE: Join us

@AlexGuteniev
Copy link
Contributor

Guess with vNext size_t in principle can be adjusted to match SIZE_T (on the same priority with extending support for exotic pointers)

@BillyONeal
Copy link
Member

Unlikely because size_t is defined by the CRT, not the STL.

@AlexGuteniev
Copy link
Contributor

AlexGuteniev commented Jul 22, 2020

I guess it is defined in compiler due to having sizeof/alignof, but even this doesn't make the change impossible.

Making bool match BOOL is harder, since you'd want to change the ABI conventions, and ideally also the destination size of setcc...

@BillyONeal
Copy link
Member

I guess it is defined in compiler due to having sizeof/alignof, but even this doesn't make the change impossible.

Impossible? No. Worth it? No.

Making bool match BOOL is harder, since you'd want to change the ABI conventions

Right, that one is out of our hands. Windows is not taking any sort of ABI break.

@CaseyCarter CaseyCarter self-assigned this Jul 27, 2020
@CaseyCarter CaseyCarter merged commit b2f1556 into microsoft:master Jul 27, 2020
@CaseyCarter
Copy link
Contributor

Thank you for keeping clang-format on target so I don't have to think about formatting.

@CaseyCarter CaseyCarter removed their assignment Jul 27, 2020
CaseyCarter pushed a commit to CaseyCarter/STL that referenced this pull request Jul 28, 2020
* Remove unnecessary comments in ranges tests.

We taught clang-format how to sort like this automatically.

* Fix x86 compiler error in stljobs.h.

This never compiled for x86 where size_t and SIZE_T are different types.

* Update .clang-format for Clang 10.

AllowShortBlocksOnASingleLine changed from false/true to
Never/Empty/Always. We still want the default of Never, as Empty
appears to misbehave.

DeriveLineEnding was added, defaulting to true. We want to disable line
ending auto-detection.

SortPriority was added. Its documentation is difficult to understand,
but it can be used to group includes together while modifying their
otherwise-lexicographic ordering. Here, I'm using it to group WinSDK
includes together, while still sorting WinIoCtl.h last.

IncludeIsMainSourceRegex was added. This doesn't appear to be relevant
to us.

IndentGotoLabels was added. We've avoided goto.

SpaceInEmptyBlock was added. We prefer the default of no space.

SpacesInConditionalStatement was added. We definitely prefer the
default of no space.

SpaceBeforeSquareBrackets was added. Again, we definitely prefer the
default of no space.

The Standard option was overhauled. Previously, Cpp11 meant "use the
latest supported standard". That was confusing, so it has been
deprecated in favor of Latest.

UseCRLF was added. We currently use CRLF for all files. (Previously,
validate.cpp would detect LF files, but clang-format wouldn't fix
them.)

* In .clang-format, delete empty lines and move comment.

The empty lines were intended to make it easier to see customized
options, but they were actually making it harder to diff the default
config against our config.

Move the StatementMacros comment next to our customized settings,
and rewrap it.

* Reformat filesystem.cpp.
@StephanTLavavej StephanTLavavej deleted the clang-format branch July 29, 2020 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants