Replace include guards in all *.h-files by #pragma once

[SunBlack]

Fixes #429

There are still more include guards in hpp files, but I reverted my changes to these files because I need to cleanup cpp files to can remove include guards there (a lot of cpp files are including hpp files instead of h files). And I believe this MR is big enough for now ;)

[taketwo]

Thanks. Could you also update the style guide? I think we should keep only the first sentence that macros/defines have to be uppercase and drop the rest.

[SunBlack]

Done. Hope it is okay so.

[jasjuang]

I think this is not a good idea, please see https://stackoverflow.com/a/34884735/3667089

[taketwo]

Thanks for the link. I've read the answer and the ensuing comment thread and I'm not convinced. The guy points at rather obscure use-cases which I believe are not relevant to us.

[SunBlack]

Include guards may be more robust in case of bad code - thats true. But PCL is using just one namespace and naming of file should match containing class, so there should be never equal file names. And really: I don't know any reason why I should duplicate a source file. In the referenced link I see only two reasons why include guards are better than #pragma once: #pragma once is not part of C++ standard (thats why I can understand they are not using it in Linux kernel) and you wan't to allow to write ugly code.

#pragma once may be less robust, but is less prone to making mistakes. Example of PCL:

• Not all defines are matching style guide (Exaple: missing _ at end of PCL_IO_BUFFERS_H, missing define string in comment after #endif)
• HPP files should never be included in any other file except in corresponding header. If you are using only #pragma once in header files, instead of spamming include guards, this is not possible anymore. In PCl we have instead currently following situation (thats why I have reverted my changed to hpp):
  1. example.hpp is included in example.h (ok)
  2. example.h is included in example.hpp (not required)
  3. another.cpp includes example.hpp (only possible because of 2., but this is really ugly because you should only include example.h and never example.hpp)

[taketwo]

HPP files should never be included in any other file except in corresponding header

This is not correct. There are two ways to build PCL: with and without precompilation. First, let's consider the latter case. When you include a PCL header in your project, it automatically pulls in corresponding hpp file with template definitions and everything is fine. In the former case, however, header files do not automatically include corresponding hpps and thus you don't get template definitions. In most cases that's fine; the templates were already preinstantiated during library build with various point types, so you are covered. But sometimes you are out of luck and the combination of class and point type that you are interested in was not preinstantiated. This is where you have no other choice but to include hpp files. And this is precisely the reason why PCL installs hpps at all.

Edit: this is just a clarification, it does not contradict your argument.

[SunBlack]

I don't understand your point. I didn't say we should stop install hpp files. I just wrote we should always include *.h files and only this *.h files should include matching .hpp file. On this way we don't need include guards in hpp files and we don't lose any functionality. So we are still installing hpp files, we just don't reference them from any file anymore except corresponding header file.

[taketwo]

That sentence about installation was just a side note.

I just wrote we should always include *.h files and only this *.h files should include matching .hpp file.

This I understand. However I explained a situation when the user has to include hpp file explicitly.

[jasjuang]

Not all defines are matching style guide (Exaple: missing _ at end of PCL_IO_BUFFERS_H, missing define string in comment after #endif)

This can be easily enforced by incorporating cpplint.

What is the benefit of #pragma once besides there is less code to type? Less code to type doesn't seem like a strong benefit to me. To my understanding include guards are more robust (mostly to corner cases, I know), and faster compilation (less difference in modern compilers, I know). In my opinion there is zero benefit of doing so and there is an added chance (low, I know) of something wrong coming to bite us the in future.

[SunBlack]

This can be easily enforced by incorporating cpplint.

True, but why force a naming schema if there is a way without any naming schema required?

What is the benefit of #pragma once besides there is less code to type? Less code to type doesn't seem like a strong benefit to me. To my understanding include guards are more robust (mostly to corner cases, I know), and faster compilation (less difference in modern compilers, I know). In my opinion there is zero benefit of doing so and there is an added chance (low, I know) of something wrong coming to bite us the in future.

Best summary I found to include guards vs pragma is from Microsoft. And because they are building a compiler I trust them more than any Stack Overflow, Reedit or and other community entry ;-).

[SergioRAgostinho]

I don't have a strong opinion on this topic. It feels like one of those topics I would put the community to vote.

And because they are building a compiler I trust them more than any Stack Overflow, Reedit or and other community entry ;-).

https://developercommunity.visualstudio.com/content/problem/120156/-cplusplus-macro-still-defined-as-pre-c11-value.html

The same guys. Unconditional trust is a dangerous thing :)

[SunBlack]

Any decision here, if it will be merged or not?

[SergioRAgostinho]

I'm ok in adopting this. If we hit some weird corner case, we can worry about it then.

I have two additional requests:

• rebasing to the tip of master. master is now running CI on all three major platforms.
• this PR only needs two separate commits, one for the actual header guards, one with the changes to the code style text. Can you do the appropriate squashing?

We're about to release a patch version 1.9.1 and this will only be merged after that.

[taketwo]

I'm also okay with adopting this. @jspricke do you want to have a last word here?

[jspricke]

I'm ok with merging this for 1.10.

[SergioRAgostinho]

Just a minor change.