RFC pragma once or ifndef

[ddn0]

Context: #199

Vote here and optionally add your reasoning as a comment.

:+1: pragma once :-1: ifndef :confused: neither, this doesn't need to be consistent, I like the status quo, etc.

[ddn0]

My reasoning for :-1:

We are probably going to bring in more contributors pretty soon and my preference is for us to follow some established style guide like https://google.github.io/styleguide/cppguide.html rather than having an in-house style. I don't think that guide is amazing but having a comprehensive set of rules is better than coming up with one ourselves, piecemeal. Having a bunch of house rules slows down onboarding and creates unnecessary friction during review.

On the particular subject of #pragma once or not. Yes it is supported by any compiler that we could use, and yes it is less error-prone than #ifndef but the typical convention in the projects I have seen is to not use it (https://github.com/llvm/llvm-project, https://github.com/chromium/chromium) and frankly it is non-standard; even in the major refactoring era of C++14, C++17, C++20 there is no proposal to have something like #pragma once (because its edge-case semantics are subtle).

[l-hoang]

pragma once

I actually prefer the ifndef just because I'm more used to it + convention, but pragma once (when it works/has support) is much cleaner. ifndef is kind of annoying since you have to come up with a keyword.

I got ninja'd by Donald's comment just now, but what he says also makes sense.

[ddn0]

I got ninja'd by Donald's comment just now, but what he says also makes sense.

Join us.... :D

[insertinterestingnamehere]

The #pragma once vs #ifndef distinction doesn't matter much to me. Both work.

WRT style guides, standardizing on some style guide is a good idea, though I'm not aware of any style guide that suits our needs well. The Google style guide has a lot of good ideas in it, but it also specifies far more than we'd realistically want to enforce. That makes some sense in a large organization like Google where code uniformity is critical, but forcing our code to fit their style guide will be a useless hassle. A few examples: they standardize on .cc instead of .cpp, they completely outlaw implicit conversions since they are often unintentional, they forbid doing work during construction of objects, and they attach additional semantic meaning to struct vs class that isn't actually present in the C++ standard. I remember in some of my prior work by far the best solution to a problem we had was to combine ADL with implicit conversions. The Google style guide would have forbidden that. The LLVM style seems a lot more sane, but they still outlaw exceptions and RTTI which we currently have to use in our graph converter tools. Using a standard style guide makes some sense, but I'm opposed to adopting the Google style guide. It's very restrictive and we'd gain nothing from the additional restrictions.

My approach thus far has been to focus on using clang-format to standardize things in an automated way. We can extend this approach to clang-tidy as well and use these tools to improve standardization. Turning warnings into errors also fits within this approach. This way the onboarding process basically just consists of telling people to run the automated cleanup tools. Style guides beyond that have value in teaching people how to write better quality C++, but if something can't be automated it's difficult to enforce.

[roshandathathri]

Agree with Ian.

I'm ambivalent towards a style guide that can't be enforced easily.

My vote is for pragma once, although I don't mind the other approach. I think not being part of the standard is not that much of an issue. I don't see compilers dropping support for that - it would break a lot of code, which they would be hesitant to do.

[ddn0]

My approach thus far has been to focus on using clang-format to standardize things in an automated way.

Bam!

https://clang.llvm.org/extra/clang-tidy/checks/llvm-header-guard.html

Style guides beyond that have value in teaching people how to write better quality C++, but if something can't be automated it's difficult to enforce.

I have seen things go too far the other way; in the sense that people get obsessed with writing automated linters/analyzers to enforce a particular idiosyncratic style (e.g., special formatting for this DSL-like library). I think automation should be a tool to achieve some result not an end itself.

[l-hoang]

If we can automate it, I'm for whatever the automation does. :P

[insertinterestingnamehere]

Yes. It's absolutely possible to over-automate as well. My main points are just that we should try to keep the style guidelines somewhat more modest (not like Google's style guide) and focus on things that can be easily automated with existing tools.

[amberhassaan]

I'm in agreement with what Ian said above. LLVM style guide makes more sense and it seems that's what Andrew followed more or less when he wrote the first C++ version.

More details: This issue came up in our very small team at my work, mainly to have something for future developers and interns to refer to, So we looked around and initially picked Google style guide. I didn't care until I read the Google style guide and certain part annoyed me, e.g. _ in names, and functions starting with caps. Looking around, Google's style guide is the most detailed perhaps, but like Ian said, parts of it are just strange. If what I recall is still true that our code is more like LLVM style, then converting it to Google's style will be a huge effort. This is something we realized at work as well, that we aren't used to Google's style and are not following it completely. Google has cpplint to check parts of the GSG, but it's not precise in practice so that's another factor to move away from it. I am liking clang-tidy more but I haven't tried it seriously yet.

[ddn0]

On the topic of #pragma once vs ifndef, if there is no strong preference one way or another, the choice is now whether we want to be consistent or not. I think we should at least be internally consistent because this concern is minor enough that exceptions hinder understanding more than they improve it.

ddn@spork:katana$ find . -name '*.h' | grep -v external | grep -v experimental | xargs grep -l 'pragma.*once' | wc -l
68
ddn@spork:katana$ find . -name '*.h' | grep -v external | grep -v experimental | wc -l
304

Now, we could additionally restrict consistency on a per-project basis. I would suggest against that because it is a distinction that is hard to maintain. But if one were to go that way, there are still outliers like #pragma once in libgalois.

On the topic of style guides, I will say two things and leave the discussion: (1) typically everyone has something they object to in any guide. In some sense, this is their purpose: to make a bunch of trivial but emotional choices upfront, so people can focus on work that is actually interesting. (2) I doubt that the goals and methods of this project are so sui generis that they are not shared by an existing project.

[insertinterestingnamehere]

WRT style guides, I'm primarily (and strongly) opposed to adopting the Google style guide. The LLVM style guide is more pragmatic and mostly matches how our code is already written. We can just list off the few areas where we need something a bit different. For example, we prefer standard library functions over LLVM ones and need exceptions/RTTI in some places.

For code quality guidelines I particularly like the C++ core guidelines (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md).

It doesn't seem realistic to try to whip the codebase into full compliance with the LLVM style guide or the C++ core guidelines for this release, but we could cite those as recommended guidelines to keep in mind for new code.