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.


http://reviews.llvm.org/D17484



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to