Author: Hamish Knight Date: 2023-06-30T16:02:12-07:00 New Revision: a57bdc8fe68753c341cac0fcecabcd4d35e45466
URL: https://github.com/llvm/llvm-project/commit/a57bdc8fe68753c341cac0fcecabcd4d35e45466 DIFF: https://github.com/llvm/llvm-project/commit/a57bdc8fe68753c341cac0fcecabcd4d35e45466.diff LOG: [clang] Fix leak in LoadFromCommandLineWorkingDirectory unit test Change `ASTUnit::LoadFromCommandLine` to return a `std::unique_ptr` instead of a +1 pointer, fixing a leak in the unit test `LoadFromCommandLineWorkingDirectory`. Reviewed By: bnbarham, benlangmuir Differential Revision: https://reviews.llvm.org/D154257 Added: Modified: clang/include/clang/Frontend/ASTUnit.h clang/lib/CrossTU/CrossTranslationUnit.cpp clang/lib/Frontend/ASTUnit.cpp clang/tools/libclang/CIndex.cpp clang/unittests/Frontend/ASTUnitTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index db8b598866c251..b762be1c9b1d6d 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -826,7 +826,7 @@ class ASTUnit { /// // FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, we // shouldn't need to specify them at construction time. - static ASTUnit *LoadFromCommandLine( + static std::unique_ptr<ASTUnit> LoadFromCommandLine( const char **ArgBegin, const char **ArgEnd, std::shared_ptr<PCHContainerOperations> PCHContainerOps, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath, diff --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp index ad4657ddcedea0..1ead01e49ec121 100644 --- a/clang/lib/CrossTU/CrossTranslationUnit.cpp +++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp @@ -609,10 +609,10 @@ CrossTranslationUnitContext::ASTLoader::loadFromSource( IntrusiveRefCntPtr<DiagnosticsEngine> Diags( new DiagnosticsEngine{DiagID, &*DiagOpts, DiagClient}); - return std::unique_ptr<ASTUnit>(ASTUnit::LoadFromCommandLine( - CommandLineArgs.begin(), (CommandLineArgs.end()), - CI.getPCHContainerOperations(), Diags, - CI.getHeaderSearchOpts().ResourceDir)); + return ASTUnit::LoadFromCommandLine(CommandLineArgs.begin(), + (CommandLineArgs.end()), + CI.getPCHContainerOperations(), Diags, + CI.getHeaderSearchOpts().ResourceDir); } llvm::Expected<InvocationListTy> diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 30ddfb2e84cf93..c13cec2dfa5813 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1738,7 +1738,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation( return AST; } -ASTUnit *ASTUnit::LoadFromCommandLine( +std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine( const char **ArgBegin, const char **ArgEnd, std::shared_ptr<PCHContainerOperations> PCHContainerOps, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath, @@ -1841,7 +1841,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine( return nullptr; } - return AST.release(); + return AST; } bool ASTUnit::Reparse(std::shared_ptr<PCHContainerOperations> PCHContainerOps, diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index fb6cc96cac55d7..ec309fa2caaabf 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -3962,7 +3962,7 @@ clang_parseTranslationUnit_Impl(CXIndex CIdx, const char *source_filename, *CXXIdx, LibclangInvocationReporter::OperationKind::ParseOperation, options, llvm::ArrayRef(*Args), /*InvocationArgs=*/std::nullopt, unsaved_files); - std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCommandLine( + std::unique_ptr<ASTUnit> Unit = ASTUnit::LoadFromCommandLine( Args->data(), Args->data() + Args->size(), CXXIdx->getPCHContainerOperations(), Diags, CXXIdx->getClangResourcesPath(), CXXIdx->getStorePreamblesInMemory(), @@ -3973,7 +3973,7 @@ clang_parseTranslationUnit_Impl(CXIndex CIdx, const char *source_filename, /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse, /*UserFilesAreVolatile=*/true, ForSerialization, RetainExcludedCB, CXXIdx->getPCHContainerOperations()->getRawReader().getFormats().front(), - &ErrUnit)); + &ErrUnit); // Early failures in LoadFromCommandLine may return with ErrUnit unset. if (!Unit && !ErrUnit) diff --git a/clang/unittests/Frontend/ASTUnitTest.cpp b/clang/unittests/Frontend/ASTUnitTest.cpp index 852cfc7118b23a..64fc240852edec 100644 --- a/clang/unittests/Frontend/ASTUnitTest.cpp +++ b/clang/unittests/Frontend/ASTUnitTest.cpp @@ -167,7 +167,7 @@ TEST_F(ASTUnitTest, LoadFromCommandLineEarlyError) { auto PCHContainerOps = std::make_shared<PCHContainerOperations>(); std::unique_ptr<clang::ASTUnit> ErrUnit; - ASTUnit *AST = ASTUnit::LoadFromCommandLine( + std::unique_ptr<ASTUnit> AST = ASTUnit::LoadFromCommandLine( &Args[0], &Args[4], PCHContainerOps, Diags, "", false, "", false, CaptureDiagsKind::All, std::nullopt, true, 0, TU_Complete, false, false, false, SkipFunctionBodiesScope::None, false, true, false, false, @@ -194,7 +194,7 @@ TEST_F(ASTUnitTest, LoadFromCommandLineWorkingDirectory) { auto PCHContainerOps = std::make_shared<PCHContainerOperations>(); std::unique_ptr<clang::ASTUnit> ErrUnit; - auto *AST = ASTUnit::LoadFromCommandLine( + std::unique_ptr<ASTUnit> AST = ASTUnit::LoadFromCommandLine( &Args[0], &Args[4], PCHContainerOps, Diags, "", false, "", false, CaptureDiagsKind::All, std::nullopt, true, 0, TU_Complete, false, false, false, SkipFunctionBodiesScope::None, false, true, false, false, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits