+Tom who manages 3.8.1 +Alex who's owner of clang-tidy: is this ok for 3.8.1?
On Thu, May 19, 2016 at 1:56 AM, Edoardo P. via cfe-commits <cfe-commits@lists.llvm.org> wrote: > Is it possible to port this commit to 3.8.1? > > Cheers, > Edward-san > > 2016-02-26 10:19 GMT+01:00 Haojian Wu via cfe-commits > <cfe-commits@lists.llvm.org>: >> Author: hokein >> Date: Fri Feb 26 03:19:33 2016 >> New Revision: 261991 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=261991&view=rev >> Log: >> [clang-tidy] Fix a crash issue when clang-tidy runs with compilation >> database. >> >> Summary: >> The clang-tidy will trigger an assertion if it's not in the building >> directory. >> >> TEST: >> cd <llvm-repo>/ >> ./build/bin/clang-tidy --checks=-*,modernize-use-nullptr -p build >> tools/clang/tools/extra/clang-tidy/ClangTidy.cpp >> >> The crash issue is gone after applying this patch. >> >> Fixes PR24834, PR26241 >> >> Reviewers: bkramer, alexfh >> >> Subscribers: rizsotto.mailinglist, cfe-commits >> >> Differential Revision: http://reviews.llvm.org/D17335 >> >> Added: >> clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/ >> >> clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json >> clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp >> Modified: >> clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp >> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp >> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h >> >> Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=261991&r1=261990&r2=261991&view=diff >> ============================================================================== >> --- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original) >> +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Fri Feb 26 03:19:33 2016 >> @@ -107,6 +107,10 @@ public: >> DiagPrinter->BeginSourceFile(LangOpts); >> } >> >> + SourceManager& getSourceManager() { >> + return SourceMgr; >> + } >> + >> void reportDiagnostic(const ClangTidyError &Error) { >> const ClangTidyMessage &Message = Error.Message; >> SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset); >> @@ -124,7 +128,10 @@ public: >> auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]")) >> << Message.Message << Name; >> for (const tooling::Replacement &Fix : Error.Fix) { >> - SourceLocation FixLoc = getLocation(Fix.getFilePath(), >> Fix.getOffset()); >> + SmallString<128> FixAbsoluteFilePath = Fix.getFilePath(); >> + Files.makeAbsolutePath(FixAbsoluteFilePath); >> + SourceLocation FixLoc = >> + getLocation(FixAbsoluteFilePath, Fix.getOffset()); >> SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength()); >> Diag << FixItHint::CreateReplacement(SourceRange(FixLoc, FixEndLoc), >> Fix.getReplacementText()); >> @@ -232,6 +239,13 @@ ClangTidyASTConsumerFactory::CreateASTCo >> Context.setCurrentFile(File); >> Context.setASTContext(&Compiler.getASTContext()); >> >> + auto WorkingDir = Compiler.getSourceManager() >> + .getFileManager() >> + .getVirtualFileSystem() >> + ->getCurrentWorkingDirectory(); >> + if (WorkingDir) >> + Context.setCurrentBuildDirectory(WorkingDir.get()); >> + >> std::vector<std::unique_ptr<ClangTidyCheck>> Checks; >> CheckFactories->createChecks(&Context, Checks); >> >> @@ -446,8 +460,24 @@ runClangTidy(std::unique_ptr<ClangTidyOp >> void handleErrors(const std::vector<ClangTidyError> &Errors, bool Fix, >> unsigned &WarningsAsErrorsCount) { >> ErrorReporter Reporter(Fix); >> - for (const ClangTidyError &Error : Errors) >> + vfs::FileSystem &FileSystem = >> + *Reporter.getSourceManager().getFileManager().getVirtualFileSystem(); >> + auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory(); >> + if (!InitialWorkingDir) >> + llvm::report_fatal_error("Cannot get current working path."); >> + >> + for (const ClangTidyError &Error : Errors) { >> + if (!Error.BuildDirectory.empty()) { >> + // By default, the working directory of file system is the current >> + // clang-tidy running directory. >> + // >> + // Change the directory to the one used during the analysis. >> + FileSystem.setCurrentWorkingDirectory(Error.BuildDirectory); >> + } >> Reporter.reportDiagnostic(Error); >> + // Return to the initial directory to correctly resolve next Error. >> + FileSystem.setCurrentWorkingDirectory(InitialWorkingDir.get()); >> + } >> Reporter.Finish(); >> WarningsAsErrorsCount += Reporter.getWarningsAsErrorsCount(); >> } >> >> Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=261991&r1=261990&r2=261991&view=diff >> ============================================================================== >> --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp >> (original) >> +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Fri >> Feb 26 03:19:33 2016 >> @@ -116,8 +116,9 @@ ClangTidyMessage::ClangTidyMessage(Strin >> >> ClangTidyError::ClangTidyError(StringRef CheckName, >> ClangTidyError::Level DiagLevel, >> - bool IsWarningAsError) >> - : CheckName(CheckName), DiagLevel(DiagLevel), >> + bool IsWarningAsError, >> + StringRef BuildDirectory) >> + : CheckName(CheckName), BuildDirectory(BuildDirectory), >> DiagLevel(DiagLevel), >> IsWarningAsError(IsWarningAsError) {} >> >> // Returns true if GlobList starts with the negative indicator ('-'), >> removes it >> @@ -335,7 +336,8 @@ void ClangTidyDiagnosticConsumer::Handle >> bool IsWarningAsError = >> DiagLevel == DiagnosticsEngine::Warning && >> Context.getWarningAsErrorFilter().contains(CheckName); >> - Errors.push_back(ClangTidyError(CheckName, Level, IsWarningAsError)); >> + Errors.push_back(ClangTidyError(CheckName, Level, IsWarningAsError, >> + Context.getCurrentBuildDirectory())); >> } >> >> ClangTidyDiagnosticRenderer Converter( >> >> Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h >> URL: >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=261991&r1=261990&r2=261991&view=diff >> ============================================================================== >> --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h >> (original) >> +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Fri Feb >> 26 03:19:33 2016 >> @@ -57,13 +57,23 @@ struct ClangTidyError { >> Error = DiagnosticsEngine::Error >> }; >> >> - ClangTidyError(StringRef CheckName, Level DiagLevel, bool >> IsWarningAsError); >> + ClangTidyError(StringRef CheckName, Level DiagLevel, bool >> IsWarningAsError, >> + StringRef BuildDirectory); >> >> std::string CheckName; >> ClangTidyMessage Message; >> tooling::Replacements Fix; >> SmallVector<ClangTidyMessage, 1> Notes; >> >> + // A build directory of the diagnostic source file. >> + // >> + // It's an absolute path which is `directory` field of the source file in >> + // compilation database. If users don't specify the compilation database >> + // directory, it is the current directory where clang-tidy runs. >> + // >> + // Note: it is empty in unittest. >> + std::string BuildDirectory; >> + >> Level DiagLevel; >> bool IsWarningAsError; >> }; >> @@ -198,6 +208,16 @@ public: >> void setCheckProfileData(ProfileData *Profile); >> ProfileData *getCheckProfileData() const { return Profile; } >> >> + /// \brief Should be called when starting to process new translation unit. >> + void setCurrentBuildDirectory(StringRef BuildDirectory) { >> + CurrentBuildDirectory = BuildDirectory; >> + } >> + >> + /// \brief Returns build directory of the current translation unit. >> + const std::string &getCurrentBuildDirectory() { >> + return CurrentBuildDirectory; >> + } >> + >> private: >> // Calls setDiagnosticsEngine() and storeError(). >> friend class ClangTidyDiagnosticConsumer; >> @@ -222,6 +242,8 @@ private: >> >> ClangTidyStats Stats; >> >> + std::string CurrentBuildDirectory; >> + >> llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID; >> >> ProfileData *Profile; >> >> Added: >> clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json >> URL: >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json?rev=261991&view=auto >> ============================================================================== >> --- >> clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json >> (added) >> +++ >> clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json >> Fri Feb 26 03:19:33 2016 >> @@ -0,0 +1,32 @@ >> +[ >> +{ >> + "directory": "test_dir/a", >> + "command": "clang++ -o test.o test_dir/a/a.cpp", >> + "file": "test_dir/a/a.cpp" >> +}, >> +{ >> + "directory": "test_dir/a", >> + "command": "clang++ -o test.o test_dir/a/b.cpp", >> + "file": "test_dir/a/b.cpp" >> +}, >> +{ >> + "directory": "test_dir/", >> + "command": "clang++ -o test.o test_dir/b/b.cpp", >> + "file": "test_dir/b/b.cpp" >> +}, >> +{ >> + "directory": "test_dir/b", >> + "command": "clang++ -o test.o ../b/c.cpp", >> + "file": "test_dir/b/c.cpp" >> +}, >> +{ >> + "directory": "test_dir/b", >> + "command": "clang++ -I../include -o test.o ../b/d.cpp", >> + "file": "test_dir/b/d.cpp" >> +}, >> +{ >> + "directory": "test_dir/", >> + "command": "clang++ -o test.o test_dir/b/not-exist.cpp", >> + "file": "test_dir/b/not-exist.cpp" >> +} >> +] >> >> Added: >> clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp?rev=261991&view=auto >> ============================================================================== >> --- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp >> (added) >> +++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp >> Fri Feb 26 03:19:33 2016 >> @@ -0,0 +1,24 @@ >> +// REQUIRES: shell >> +// RUN: mkdir -p %T/compilation-database-test/include >> +// RUN: mkdir -p %T/compilation-database-test/a >> +// RUN: mkdir -p %T/compilation-database-test/b >> +// RUN: echo 'int *AA = 0;' > %T/compilation-database-test/a/a.cpp >> +// RUN: echo 'int *AB = 0;' > %T/compilation-database-test/a/b.cpp >> +// RUN: echo 'int *BB = 0;' > %T/compilation-database-test/b/b.cpp >> +// RUN: echo 'int *BC = 0;' > %T/compilation-database-test/b/c.cpp >> +// RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h >> +// RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp >> +// RUN: sed 's|test_dir|%T/compilation-database-test|g' >> %S/Inputs/compilation-database/template.json > %T/compile_commands.json >> +// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T >> %T/compilation-database-test/b/not-exist -header-filter=.* >> +// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T >> %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp >> %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp >> %T/compilation-database-test/b/d.cpp -header-filter=.* -fix >> +// RUN: FileCheck -input-file=%T/compilation-database-test/a/a.cpp %s >> -check-prefix=CHECK-FIX1 >> +// RUN: FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s >> -check-prefix=CHECK-FIX2 >> +// RUN: FileCheck -input-file=%T/compilation-database-test/b/b.cpp %s >> -check-prefix=CHECK-FIX3 >> +// RUN: FileCheck -input-file=%T/compilation-database-test/b/c.cpp %s >> -check-prefix=CHECK-FIX4 >> +// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h >> %s -check-prefix=CHECK-FIX5 >> + >> +// CHECK-FIX1: int *AA = nullptr; >> +// CHECK-FIX2: int *AB = nullptr; >> +// CHECK-FIX3: int *BB = nullptr; >> +// CHECK-FIX4: int *BC = nullptr; >> +// CHECK-FIX5: int *HP = nullptr; >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > -- > Mathematics is the language with which God has written the universe. (Galilei) > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits