Skip to content

Commit

Permalink
[libc] Update libc namespace clang-tidy checks
Browse files Browse the repository at this point in the history
Namespace macro that should be used to declare a new namespace is
updated from LIBC_NAMESPACE to LIBC_NAMESPACE_DECL which by default has
hidden visibility (#97109). This commit updates the clang-tidy checks to
match the new policy.
  • Loading branch information
Prabhuk authored and ilovepi committed Jul 10, 2024
1 parent 6789e1b commit 2270c6a
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
// __llvm_libc, we're good.
const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
if (NS && Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) &&
NS->getName().starts_with(RequiredNamespaceStart))
NS->getName().starts_with(RequiredNamespaceRefStart))
return;

const DeclarationName &Name = FuncDecl->getDeclName();
Expand All @@ -62,7 +62,7 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
diag(UsageSiteExpr->getBeginLoc(),
"%0 must resolve to a function declared "
"within the namespace defined by the '%1' macro")
<< FuncDecl << RequiredNamespaceMacroName;
<< FuncDecl << RequiredNamespaceRefMacroName;

diag(FuncDecl->getLocation(), "resolves to this declaration",
clang::DiagnosticIDs::Note);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs<Decl>("child_of_translation_unit");
const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl);

// LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
diag(MatchedDecl->getLocation(),
"declaration must be enclosed within the '%0' namespace")
<< RequiredNamespaceMacroName;
<< RequiredNamespaceDeclMacroName;
return;
}

// Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
<< RequiredNamespaceMacroName;
<< RequiredNamespaceDeclMacroName;
return;
}
if (NS->getName().starts_with(RequiredNamespaceStart) == false) {

// We want the macro to have [[gnu::visibility("hidden")]] as a prefix, but
// visibility is just an attribute in the AST construct, so we check that
// instead.
if (NS->getVisibility() != Visibility::HiddenVisibility) {
diag(NS->getLocation(), "the '%0' macro should start with '%1'")
<< RequiredNamespaceMacroName << RequiredNamespaceStart;
<< RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
return;
}

// Lastly, make sure the namespace name actually has the __llvm_libc prefix
if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
diag(NS->getLocation(), "the '%0' macro expansion should start with '%1'")
<< RequiredNamespaceDeclMacroName << RequiredNamespaceRefStart;
return;
}
}
Expand Down
8 changes: 6 additions & 2 deletions clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

namespace clang::tidy::llvm_libc {

const static llvm::StringRef RequiredNamespaceStart = "__llvm_libc";
const static llvm::StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
const static llvm::StringRef RequiredNamespaceRefStart = "__llvm_libc";
const static llvm::StringRef RequiredNamespaceRefMacroName = "LIBC_NAMESPACE";
const static llvm::StringRef RequiredNamespaceDeclStart =
"[[gnu::visibility(\"hidden\")]] __llvm_libc";
const static llvm::StringRef RequiredNamespaceDeclMacroName =
"LIBC_NAMESPACE_DECL";

} // namespace clang::tidy::llvm_libc
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,30 @@ correct namespace.

.. code-block:: c++

// Implementation inside the LIBC_NAMESPACE namespace.
// Implementation inside the LIBC_NAMESPACE_DECL namespace.
// Correct if:
// - LIBC_NAMESPACE is a macro
// - LIBC_NAMESPACE expansion starts with `__llvm_libc`
namespace LIBC_NAMESPACE {
// - LIBC_NAMESPACE_DECL is a macro
// - LIBC_NAMESPACE_DECL expansion starts with `[[gnu::visibility("hidden")]] __llvm_libc`
namespace LIBC_NAMESPACE_DECL {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
// Namespaces within LIBC_NAMESPACE namespace are allowed.
// Namespaces within LIBC_NAMESPACE_DECL namespace are allowed.
namespace inner {
int localVar = 0;
}
// Functions with C linkage are allowed.
extern "C" void str_fuzz() {}
}
// Incorrect: implementation not in the LIBC_NAMESPACE namespace.
// Incorrect: implementation not in the LIBC_NAMESPACE_DECL namespace.
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
// Incorrect: outer most namespace is not the LIBC_NAMESPACE macro.
// Incorrect: outer most namespace is not the LIBC_NAMESPACE_DECL macro.
namespace something_else {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
}
// Incorrect: outer most namespace expansion does not start with `__llvm_libc`.
#define LIBC_NAMESPACE custom_namespace
namespace LIBC_NAMESPACE {
// Incorrect: outer most namespace expansion does not start with `[[gnu::visibility("hidden")]] __llvm_libc`.
#define LIBC_NAMESPACE_DECL custom_namespace
namespace LIBC_NAMESPACE_DECL {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,60 +3,61 @@
#define MACRO_A "defining macros outside namespace is valid"

class ClassB;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace
struct StructC {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
char *VarD = MACRO_A;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace
const char *VarD = MACRO_A;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace
typedef int typeE;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace
void funcF() {}
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace

namespace outer_most {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro
class A {};
}

// Wrapped in anonymous namespace.
namespace {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration must be enclosed within the 'LIBC_NAMESPACE' namespace
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration must be enclosed within the 'LIBC_NAMESPACE_DECL' namespace
class A {};
}

namespace namespaceG {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro
namespace __llvm_libc {
namespace namespaceH {
class ClassB;
} // namespace namespaceH
struct StructC {};
} // namespace __llvm_libc
char *VarD = MACRO_A;
const char *VarD = MACRO_A;
typedef int typeE;
void funcF() {}
} // namespace namespaceG

// Wrapped in macro namespace but with an incorrect name
#define LIBC_NAMESPACE custom_namespace
namespace LIBC_NAMESPACE {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'LIBC_NAMESPACE' macro should start with '__llvm_libc'
#define LIBC_NAMESPACE_DECL [[gnu::visibility("hidden")]] custom_namespace
namespace LIBC_NAMESPACE_DECL {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'LIBC_NAMESPACE_DECL' macro expansion should start with '__llvm_libc'

namespace namespaceH {
class ClassB;
} // namespace namespaceH
} // namespace LIBC_NAMESPACE
} // namespace LIBC_NAMESPACE_DECL


// Wrapped in macro namespace with a valid name, LIBC_NAMESPACE starts with '__llvm_libc'
#undef LIBC_NAMESPACE
#define LIBC_NAMESPACE __llvm_libc_xyz
namespace LIBC_NAMESPACE {
// Wrapped in macro namespace with a valid name, LIBC_NAMESPACE_DECL starts with '__llvm_libc'
#undef LIBC_NAMESPACE_DECL
#define LIBC_NAMESPACE_DECL [[gnu::visibility("hidden")]] __llvm_libc_xyz
namespace LIBC_NAMESPACE_DECL {
namespace namespaceI {
class ClassB;
} // namespace namespaceI
struct StructC {};
char *VarD = MACRO_A;
const char *VarD = MACRO_A;
typedef int typeE;
void funcF() {}
extern "C" void extern_funcJ() {}
} // namespace LIBC_NAMESPACE
} // namespace LIBC_NAMESPACE_DECL

0 comments on commit 2270c6a

Please sign in to comment.