=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>, =?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/84...@github.com>
steakhal wrote: > I feel that I'm getting bogged down in this area with changes that are more > general than what's strictly necessary and only tangentially related to my > real goals... > > I'm not familiar with the unittest framework and if unittests were necessary > for this change, then I'd sooner abandon it, because it's not worth the > effort. > > (Disclaimer: this comment reflects my current annoyance, and is not > necessarily aligned with my long-term opinion.) No worries! Note the `IsCLibraryFunctionTest.AcceptsStdNamespace` test case which would fail on llvm/main, but would pass with the PR. Here is what I had in mind: (I could not push to your branch, neither upload the patch file, so here is the verbatim content instead: ``` commit 1cfbeec724d758b136d36966696df29cf850ca5b Author: Balazs Benics <benicsbal...@gmail.com> Date: Fri Mar 8 17:26:49 2024 +0100 Add unittests FYI IsCLibraryFunctionTest.AcceptsStdNamespace would have been FALSE prior this PR. diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt index 775f0f8486b8..db56e77331b8 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_unittest(StaticAnalysisTests CallEventTest.cpp ConflictingEvalCallsTest.cpp FalsePositiveRefutationBRVisitorTest.cpp + IsCLibraryFunctionTest.cpp NoStateChangeFuncVisitorTest.cpp ParamRegionTest.cpp RangeSetTest.cpp diff --git a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp new file mode 100644 index 000000000000..e1dea3458bc1 --- /dev/null +++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp @@ -0,0 +1,89 @@ +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Frontend/ASTUnit.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +#include <memory> + +using namespace clang; +using namespace ento; +using namespace ast_matchers; + +testing::AssertionResult extractFunctionDecl(StringRef Code, + const FunctionDecl *&Result) { + auto ASTUnit = tooling::buildASTFromCode(Code); + if (!ASTUnit) + return testing::AssertionFailure() << "AST construction failed"; + + ASTContext &Context = ASTUnit->getASTContext(); + if (Context.getDiagnostics().hasErrorOccurred()) + return testing::AssertionFailure() << "Compilation error"; + + auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context); + if (Matches.empty()) + return testing::AssertionFailure() << "No function declaration found"; + + if (Matches.size() > 1) + return testing::AssertionFailure() + << "Multiple function declarations found"; + + Result = Matches[0].getNodeAs<FunctionDecl>("fn"); + return testing::AssertionSuccess(); +} + +TEST(IsCLibraryFunctionTest, AcceptsGlobal) { + const FunctionDecl *Result; + ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result)); + EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) { + const FunctionDecl *Result; + ASSERT_TRUE( + extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result)); + EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, AcceptsStaticGlobal) { + const FunctionDecl *Result; + ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result)); + EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result)); + // FIXME: Shouldn't this be TRUE? +} + +TEST(IsCLibraryFunctionTest, RejectsAnonymousNamespace) { + const FunctionDecl *Result; + ASSERT_TRUE( + extractFunctionDecl(R"cpp(namespace { void fun(); })cpp", Result)); + EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, AcceptsStdNamespace) { + const FunctionDecl *Result; + ASSERT_TRUE( + extractFunctionDecl(R"cpp(namespace std { void fun(); })cpp", Result)); + EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, RejectsOtherNamespaces) { + const FunctionDecl *Result; + ASSERT_TRUE( + extractFunctionDecl(R"cpp(namespace stdx { void fun(); })cpp", Result)); + EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, RejectsClassStatic) { + const FunctionDecl *Result; + ASSERT_TRUE( + extractFunctionDecl(R"cpp(class A { static void fun(); };)cpp", Result)); + EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result)); +} + +TEST(IsCLibraryFunctionTest, RejectsClassMember) { + const FunctionDecl *Result; + ASSERT_TRUE(extractFunctionDecl(R"cpp(class A { void fun(); };)cpp", Result)); + EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result)); +} diff --git a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn index 01c2b6ced336..9c240cff1816 100644 --- a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn @@ -19,6 +19,7 @@ unittest("StaticAnalysisTests") { "CallEventTest.cpp", "ConflictingEvalCallsTest.cpp", "FalsePositiveRefutationBRVisitorTest.cpp", + "IsCLibraryFunctionTest.cpp", "NoStateChangeFuncVisitorTest.cpp", "ParamRegionTest.cpp", "RangeSetTest.cpp", ``` https://github.com/llvm/llvm-project/pull/84469 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits