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

Code cleanups #827

Merged
merged 11 commits into from
May 14, 2020
Merged

Code cleanups #827

merged 11 commits into from
May 14, 2020

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented May 13, 2020

  • Use empty braces to construct objects.
    • Note that all of these are our types, not user-defined types.
    • <experimental/filesystem>
      • Use file_status{} and {} for path.
      • Drop unnecessary comments for begin() and end().
      • Return {} for directory_iterator and recursive_directory_iterator end(). (This is what Standard <filesystem> does.)
    • <filesystem>
      • Use {} for path.
    • <memory>
      • Use weak_ptr{}.
    • <random>
      • Use true_type{}, is_arithmetic<_Gen>{}, and is_signed<_Ty>{}.
  • Add _STD qualification.
    • We conventionally qualify all non-_Ugly non-member function calls, even those with zero arguments (and therefore zero risk of ADL hijacking). However, many calls to future_category(), generic_category(), iostream_category(), and system_category() were missing _STD.
  • Call _Code.clear().
    • This has the same effect as _Code = error_code();, but with better throughput (avoiding operator= SFINAE) and increased consistency with Standard <filesystem>.
  • Change if (_Code != error_code()) to if (_Code).
    • This is more efficient (because it avoids calling system_category()) and consistent with Standard <filesystem>. There's a slight semantic difference, which I believe is either irrelevant here or is an improvement: error_code equality considers both value() and category(), so an error_code with value() == 0 but generic_category() would compare non-equal with error_code(), yet it would test as false. I believe that this is irrelevant because the <experimental/filesystem> machinery will never construct such an error_code.
  • Change another test of _Code != error_code() to _Code.
  • Change a test of _Code == error_code() to !_Code.
  • Remove unnecessary empty lines.
    • The empty lines at the beginning of pair's verbose assignment operators were debatable, but I decided to remove them.
  • Remove using-directive in <regex>.
    • This was the only occurrence of a "convenient" using-directive in the entire STL's product code. (The others are doing necessary work, or working around compiler bugs.)
    • While this was function-local and didn't create correctness issues, I felt that it was inconsistent with our usual conventions, and wasn't even saving significant verbosity. (Long ago, this was an enormously nested conditional expression, where the using-directive indeed helped. That was changed by Microsoft-internal MSVC-PR-182277, to the current chain of if-statements.)
  • Adjust braces and comments.

Note that all of these are our types, not user-defined types.

experimental/filesystem
Use `path{}` and `file_status{}`.

Drop unnecessary comments for `begin()` and `end()`.

Return `{}` for `directory_iterator` and `recursive_directory_iterator`
`end()`. (This is what Standard `<filesystem>` does.)

filesystem
Use `path{}`.

memory
Use `weak_ptr{}`.

random
Use `true_type{}`, `is_arithmetic<_Gen>{}`, and `is_signed<_Ty>{}`.
We conventionally qualify all non-`_Ugly` non-member function calls,
even those with zero arguments (and therefore zero risk of ADL
hijacking). However, many calls to `future_category()`,
`generic_category()`, `iostream_category()`, and `system_category()`
were missing `_STD`.
This has the same effect as `_Code = error_code();`, but with better
throughput (avoiding `operator=` SFINAE) and increased consistency with
Standard `<filesystem>`.
This is more efficient (because it avoids calling `system_category()`)
and consistent with Standard `<filesystem>`. There's a slight semantic
difference, which I believe is either irrelevant here or is an
improvement: `error_code` equality considers both `value()` and
`category()`, so an `error_code` with `value() == 0` but
`generic_category()` would compare non-equal with `error_code()`, yet
it would test as `false`. I believe that this is irrelevant because the
`<experimental/filesystem>` machinery will never construct such an
`error_code`.
The empty lines at the beginning of pair's verbose assignment operators
were debatable, but I decided to remove them.
This was the only occurrence of a "convenient" using-directive in the
entire STL's product code. (The others are doing necessary work,
or working around compiler bugs.)

While this was function-local and didn't create correctness issues,
I felt that it was inconsistent with our usual conventions, and wasn't
even saving significant verbosity. (Long ago, this was an enormously
nested conditional expression, where the using-directive indeed helped.
That was changed by Microsoft-internal MSVC-PR-182277, to the current
chain of if-statements.)
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label May 13, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner May 13, 2020 01:25
stl/inc/system_error Outdated Show resolved Hide resolved
stl/inc/system_error Outdated Show resolved Hide resolved
stl/inc/filesystem Outdated Show resolved Hide resolved
stl/inc/execution Outdated Show resolved Hide resolved
stl/inc/functional Outdated Show resolved Hide resolved
stl/inc/functional Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/utility Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej merged commit e76f500 into microsoft:master May 14, 2020
@StephanTLavavej StephanTLavavej deleted the braces branch May 14, 2020 22:59
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.

3 participants