Page MenuHomePhabricator

[clang-tidy] Add check llvmlibc-implementation-in-namespace.
Needs ReviewPublic

Authored by PaulkaToast on Wed, Mar 25, 6:25 PM.

Details

Summary

This check makes sure all llvm-libc implementations falls within the __llvm_libc namespace.

The check is quite general, lots of projects do this by conversion to avoid pollution. Not sure if there is a desire to put this in a different module?

Diff Detail

Event Timeline

PaulkaToast created this revision.Wed, Mar 25, 6:25 PM

Please also add isLanguageVersionSupported(), because namespaces make sense only in C++.

sivachandra added inline comments.Wed, Mar 25, 10:21 PM
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
31

For completeness, can you include a nested namespace and a function declaration here?

If you want to make it a general check, you should consider making the default module options set the correct namespace
RequiredNamespace

ClangTidyOptions getModuleOptions() override {
  ClangTidyOptions Options;
  Options.CheckOptions["llvmlibc-implementation-in-namespace.RequiredNamespace"] = "__llvm_libc";
  return Options;
}
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
7

Small nit : Not a fan of the diagnostic saying CXX Record. Maybe a pain but getDeclKindName isn't the best to expose to users.

Updated to handle nested namespaces, exclude C linkages functions, and made check language specific. (:

PaulkaToast marked 3 inline comments as done.Fri, Mar 27, 4:48 PM

If you want to make it a general check, you should consider making the default module options set the correct namespace
RequiredNamespace

Changed the check a bit so I'm not sure if pulling it out is still something desired, if so what module do you recommend this should live under?

clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
7

Removed getDeclKindName as recommended.

PaulkaToast marked an inline comment as done.Tue, Mar 31, 3:35 AM

Friendly Ping @njames93 (:
Any other changes needed?

sivachandra added inline comments.Tue, Mar 31, 10:14 PM
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
16

Can you add __llvm_libc as a nested namespace here.