Skip to content

STYLE: Remove std:: prefix from types that work without it#3250

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
Leengit:remove_std_from_itk_types
Mar 6, 2022
Merged

STYLE: Remove std:: prefix from types that work without it#3250
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
Leengit:remove_std_from_itk_types

Conversation

@Leengit
Copy link
Copy Markdown
Contributor

@Leengit Leengit commented Mar 4, 2022

This is accomplished with

cd git/ITK
find * -type f |
  egrep '\.(c|cc|cxx|cpp|h|hh|hxx|hpp)$' |
  fgrep -v ThirdParty |
  fgrep -v itkIntTypes.h |
  xargs sed -i -r -e \
    's/std::((size|ptrdiff|max_align|u?int(_fast|_least|max|ptr)?[0-9]*)_t)/\1/g'

Closes #3248.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

@github-actions github-actions bot added area:Core Issues affecting the Core module area:Examples Demonstration of the use of classes area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Mar 4, 2022
This is accomplished with
cd git/ITK
find * -type f |
  egrep '\.(c|cc|cxx|cpp|h|hh|hxx|hpp)$' |
  fgrep -v ThirdParty |
  fgrep -v itkIntTypes.h |
  xargs sed -i -r -e \
    's/std::((size|ptrdiff|max_align|u?int(_fast|_least|max|ptr)?[0-9]*)_t)/\1/g'
@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Mar 4, 2022

@Leengit C++ doesn't specify whether or not the std integer types from <cstdint> are also defined in the global namespace. Is the idea for ITK code to always use the aliases from the itk namespace, defined by "itkIntTypes.h"?

@Leengit
Copy link
Copy Markdown
Contributor Author

Leengit commented Mar 4, 2022

Is the idea for ITK code to always use the aliases from the itk namespace, defined by "itkIntTypes.h"?

Wherever they come from doesn't matter to me. Folks have been using the short form predominantly, it works, and I am bringing the minority of cases into the fold.

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Mar 4, 2022

Wherever they come from doesn't matter to me. Folks have been using the short form predominantly, it works, and I am bringing the minority of cases into the fold.

The majority isn't always right! It does help if we could specify why and when the std:: may be dropped. Because we won't remove all std:: in the entire code base, right?

There is no "max_align..." alias in "itkIntTypes.h", should it still be added?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Mar 4, 2022

integer types from <cstdint> are also defined in the global namespace

I understood this is true from https://en.cppreference.com/w/cpp/types/integer.

@Leengit
Copy link
Copy Markdown
Contributor Author

Leengit commented Mar 4, 2022

Good point about max_align_t, @N-Dekker. I'd say defer. This pull request conversation is hereby amended with the following disclaimer.

Some of these types are in the global namespace and some are in the itk namespace due to itkIntTypes.h, or are in both. Some of the types listed in the sed code snippet above might be in neither, but they are not causing problems because they are not currently used in our code base. If the short form for one of these is used later and it fails then it may make sense to add it to itkIntTypes.h.

@Leengit
Copy link
Copy Markdown
Contributor Author

Leengit commented Mar 4, 2022

The majority isn't always right!

What do you propose?

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Mar 4, 2022

The majority isn't always right!
What do you propose?

In general: if there is a significant minority of the code doing it otherwise, double-check if the majority of the code really does the right thing, before adjusting the minority to the majority.

Specifically in this case, it appears reasonable to remove std:: within the ITK code base, for those types that have an alias in "itkIntTypes.h". 👍

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Mar 4, 2022

@dzenanz

integer types from <cstdint> are also defined in the global namespace

I understood this is true from https://en.cppreference.com/w/cpp/types/integer.

The latest draft of the C++ Standard section [headers] says:

Except as noted in Clause 16 through Clause 32 and Annex D, the contents of each header cname is the
same as that of the corresponding header name.h as specified in the C standard library (Clause 2). In the
C++ standard library, however, the declarations (except for names which are defined as macros in C) are
within namespace scope (6.4.5) of the namespace std. It is unspecified whether these names (including any
overloads added in Clause 17 through Clause 32 and Annex D) are first declared within the global namespace
scope and are then injected into namespace std by explicit using-declarations (9.9).

HTH, Niels

@hjmjohnson hjmjohnson merged commit c173dfd into InsightSoftwareConsortium:master Mar 6, 2022
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Apr 8, 2022
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Apr 8, 2022
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Apr 11, 2022
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Apr 13, 2022
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Nov 11, 2022
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Jan 10, 2023
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
N-Dekker added a commit to N-Dekker/ITKSoftwareGuide that referenced this pull request Feb 28, 2023
Added coding style guidelines for specifying built-in integer types and
integer types from the C library.

Following recent ITK commits, authored by Lee Newberg:

pull request InsightSoftwareConsortium/ITK#3365
commit InsightSoftwareConsortium/ITK@8c2c654
commit InsightSoftwareConsortium/ITK@e03a941
"STYLE: Drop `signed` and `int` from some types"

pull request InsightSoftwareConsortium/ITK#3139
commit InsightSoftwareConsortium/ITK@a812aa3
"STYLE: Change unsigned to unsigned int."

pull request InsightSoftwareConsortium/ITK#3250
commit InsightSoftwareConsortium/ITK@c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
@Leengit Leengit deleted the remove_std_from_itk_types branch May 26, 2023 13:52
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jan 9, 2024
Follow-up to pull request InsightSoftwareConsortium#3250
commit c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
Lee Newberg, March 6, 2022.
dzenanz pushed a commit that referenced this pull request Jan 10, 2024
Follow-up to pull request #3250
commit c173dfd
"STYLE: Remove `std::` prefix from types that work without it"
Lee Newberg, March 6, 2022.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Examples Demonstration of the use of classes area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

STYLE: size_t vs. std::size_t

4 participants