0

How to let know developers automatically that this "bits/shared_ptr.h" is internal to standard library (gcc and clang).

#include <bits/shared_ptr.h>

// some code using std::shared_ptr

The best would be to also inform <memory> should be used instead.

This <bits/shared_ptr.h> is just an example - I mean - how to warn about any implementation header being included.

By "automatically" I mean - compiler warning or static analysis like clang-tidy.

I have tried "-Wall -Wextra -pedantic" and all clang-tidy checks, without llvm* checks - these llvm* warns for almost every header and it is just for llvm developers, not for us, regular developers.

Any advice?

I prefer existing solution, I know I can write script for that.


Ok, I found one check in clang-tidy that I can use.

It is portability-restrict-system-includes

Just need to specify in config that "bits" things are not allowed:

-config="CheckOptions: {portability-restrict-system-includes.Includes: '*,-bits/*,bitset'}"

See demo.

But, well, it is not perfect solution - one would need to maintain list of "not allowed" headers.

PiotrNycz
  • 23,099
  • 7
  • 66
  • 112
  • Write white-list and/or black-list (possibly with pattern)? I don't think there is a (public) C++ standard header of the form`<**/*.h>`. – Jarod42 Jun 02 '23 at 10:46
  • If you use a version control system you can make it a check on check-in. – Pepijn Kramer Jun 02 '23 at 10:59
  • No headers currently in the C++ standard library have a `/` in the name of headers. It wouldn't be difficult to write a short code fragment that parses C++ source or header files in a project, and checks if any contain `#include` directives that have a `/`. It also wouldn't be difficult to check include directives against some "allowed" list, although you would need to create that list. Bear in mind, however, that it is not uncommon for system-specific headers (e.g. under windows or unix) or headers for third-party libraries to be in a folder/path structure. – Peter Jun 02 '23 at 11:16
  • 4
    Isn't this the sort of thing that code reviews are for? – Paul Sanders Jun 02 '23 at 11:34
  • @PaulSanders IMHO - definitely not. I do not want to waste reviewers time on checking things that tools should do. Reviewers shall review readability, maintainability and things that are really hard to check automatically. – PiotrNycz Jun 02 '23 at 11:45
  • I would create another app that looks for such includes in the source files and show warnings. – Michael Chourdakis Jun 02 '23 at 11:46
  • `or static analysis` just `find . | xargs -r grep '^#include * – KamilCuk Jun 02 '23 at 13:18
  • @KamilCuk - but I cannot be sure only bits directory is for internal files. I would like to have solution that is not needed to be maintained, "upgraded" when we get new compiler – PiotrNycz Jun 02 '23 at 13:32
  • You have to check during code reviews anyway, because developers are too creative. What if I add a symlink and then include ``? Give the coders a pre-review check list with the rules. Has to be filled in before the review starts. – BoP Jun 02 '23 at 13:40
  • @Bop - there is information inside this internal header - that it is internal header - in its `@file` section. I think this can be used in the scripts. Besides - I do not think this is a game - I never met in my 30+ years of writing programs that someone is doing bad thing not by mistake - but simply because of having evil soul ;) – PiotrNycz Jun 02 '23 at 14:32
  • @PiotrNycz - I have met a couple of guys who wrote a "comment generator". Their manager set up a rule that 30% of the source files must be comments, so they wrote a checkin preprocessor to add extra comments as needed. :-) – BoP Jun 02 '23 at 14:47
  • @BoP I believe this manager's decision to have so many comments was simple mistake due to some lack of knowledge. He/she did not do this evil decision on evil intention ;) – PiotrNycz Jun 02 '23 at 15:35
  • 1
    Your clang-tidy check restricts `#include ` btw – Artyer Jun 02 '23 at 17:05
  • @Artyer - clang-tidy config is fixed now – PiotrNycz Jun 05 '23 at 09:32

1 Answers1

2

It seems like include-what-you-use does what you want.

It has a mapping of what names are supposed to come from what header, and it seems to know which headers are internal.

For example, when including <bits/shared_ptr.h> https://godbolt.org/z/cvq7354K6:

#include <bits/shared_ptr.h>

std::shared_ptr<int> x;

It says to remove <bits/shared_ptr.h> and add <memory>.

Artyer
  • 31,034
  • 3
  • 47
  • 75
  • why it wants ``? "#include // for declval". I don't declval in this short code – PiotrNycz Jun 03 '23 at 20:07
  • 1
    @PiotrNycz I think that's for something in ``. But it tells you to remove it if you actually do add it: https://godbolt.org/z/onf1GMdnc so it works in the end – Artyer Jun 03 '23 at 20:08