[PATCH] D17484: [clang-tidy] introduce modernize-deprecated-headers check
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 22 10:15:27 PST 2016
omtcyf0 added a comment.
Thank you for a review, Alex!
I'll clean up the parts you mentioned soon!
In http://reviews.llvm.org/D17484#358617, @alexfh wrote:
> The most important question to this check is that the standard doesn't guarantee that the C++ headers declare all the same functions **in the global namespace** (at least, this is how I understand the section of the standard you quoted). This means that the check in its current form can break the code that uses library symbols from the global namespace. The check could add `std::` wherever it's needed (though it may be not completely trivial) to mitigate the risk. It's not what needs to be addressed in the first iteration, but it's definitely worth a look at in order to make the check actually useful. It also could be a separate check or a separate mode (configurable via parameters) of this check, while someone could have reasons to do the migration in two stages (add `std::` qualifiers and then switch to new headers).
Yes, you are correct: the check might cause some troubles in codebases, because it is not guaranteed that the functions from the C library headers will occur in the global namespace.
I was initially aware of that, but I thought it'd be better to add the initial check first and enhance it later. Appropriate FIXME seems great.
In http://reviews.llvm.org/D17484#358757, @LegalizeAdulthood wrote:
> It would be reasonable for this check to insert a `using namespace std;` at the top of the translation unit after all the `#include` lines as a means of preserving meaning without trying to identify every symbol that needs `std` on it.
I'm not sure that's a good option. I would certainly argue about putting using namespaces like std anywhere during the vanilla check run. Simply because of the collisions caused by some libraries with the functions having analogies in STL.
However, I can see it as an option for the further variations of this check.
Say, let it have three options:
- Don't bother about namespaces: in case user already has `using namespace std` or doesn't care about implementation-dependend things. Seems like a reasonable scenario to me.
- Add `std::` prefix to all functions from these headers in the process of migration: generally it seems a bit better to me.
- Put `using namespace std;` somewhere (maybe even specify where: i.e. if user wants to put it right to the top of TU or in each source file where a C library is used. That's considerably good, too.
//just note for the future//
I also think there might be a good place in clang-tidy for an independent check responsible for namespace migrations. Say, user has been using only one library and now when he wants to add another one it would be good to put the appropriate namespace prefixes where appropriate.
Seems quite sophisticated ATM though.
More information about the cfe-commits