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

Unify the coding style of using size_t or std::size_t #114

Open
xlinsist opened this issue Feb 9, 2023 · 3 comments
Open

Unify the coding style of using size_t or std::size_t #114

xlinsist opened this issue Feb 9, 2023 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@xlinsist
Copy link
Collaborator

xlinsist commented Feb 9, 2023

size_t and std::size_t are both aliases of unsigned long in normal cases but defined in different standards. Though differences between them are subtle, unifications of usage is good for code style and code readability. Should we need clarification about it?

Currently buddy-mlir is using size_t and std::size_t combinely (e.g. in Container.cpp), which is somethat misleading and one is better to be replaced with the other. @meshtag and I have discussed about it and summarized that std::size_t is more friendly and simple for the compiler while size_t is neater for developers to use. Considerations remain open.

@Origami404
Copy link
Contributor

I made a quick and little stat:

llvm-project$ rg 'std::size_t' --glob '*.cpp'  --no-heading | wc -l                                                   
3921
llvm-project$ rg '[^:]size_t' --glob '*.cpp'  --no-heading | wc -l                                                            
18011
llvm-project$ git rev-parse HEAD                                                                                            
3d7b853233580527245eba885000dbf468983eb4

In this case, --no-heading to make ripgrep giving grep-like output (one match for a line), --glob '*.cpp' to only search in C++ source file.

According to the result, both std::size_t and size_t are widely used in LLVM, but size_t is more popular. I'm going to use git blame to get when they were introduced, and see if recent LLVM committers have a particular tendency.

FYI, in my personal flavour, I prefer std::size_t because it looks more "C++-ish" by using the std namespace.

@meshtag meshtag added enhancement New feature or request help wanted Extra attention is needed labels Feb 9, 2023
@zhanghb97
Copy link
Member

@xlinsist @Origami404 Thanks for the topic and discussion!

I used a casual style for this point, and I think we do need a unified convention.
From my point of view, the using directive way and the explicit way (e.g. std::) have trade-offs between conciseness and clarity. We should choose a specific way according to the context of the code we are writing.

The general convention in my mind are:

  • Use using directive: We may use using directive when we need to access many symbols from a namespace frequently within a single code block or file. It can make our code more concise and easier to read. We may NOT use the using directive in header files, as it can cause name collisions if multiple headers use the same namespace.

  • Use explicit way: We may use the explicit way when we only need to use a few symbols from the namespace, or when we are working in a header file.

In general, the key is to make our code clear and maintainable, while avoiding naming conflicts and other issues that can arise from overusing the using directive.

@xlinsist
Copy link
Collaborator Author

According to the result, both std::size_t and size_t are widely used in LLVM, but size_t is more popular. I'm going to use git blame to get when they were introduced, and see if recent LLVM committers have a particular tendency.

That's really cool! I did do a tally on llvm project but not as decent as it is.

The general convention in my mind are:

  • Use using directive: We may use using directive when we need to access many symbols from a namespace frequently within a single code block or file. It can make our code more concise and easier to read. We may NOT use the using directive in header files, as it can cause name collisions if multiple headers use the same namespace.
  • Use explicit way: We may use the explicit way when we only need to use a few symbols from the namespace, or when we are working in a header file.

In general, the key is to make our code clear and maintainable, while avoiding naming conflicts and other issues that can arise from overusing the using directive.

It sums it up. The essence of the convention is to distinguish the so-called using directive way and the explicit way in various circumstances. As for this "size_t" situation, we should be very careful for potential consequences of programs when we want to omit std:: namespace for convenience, but we also no bother to hold std::size_t all the time when the context is clear enough for using size_t. Now I think we can reach consensus here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants