Ping, who's going to merge? I have no commit access.
Cheers, Edward-san 2016-05-20 18:34 GMT+02:00 Tom Stellard <t...@stellard.net>: > Hi, > > This looks fine to me, go ahead and merge. > > -Tom > > On Thu, May 19, 2016 at 08:29:14PM +0200, Alexander Kornienko wrote: >> On Thu, May 19, 2016 at 4:45 PM, Hans Wennborg <h...@chromium.org> wrote: >> >> > +Tom who manages 3.8.1 >> > +Alex who's owner of clang-tidy: is this ok for 3.8.1? >> > >> >> Yes, would be nice to have this in 3.8.1. This fixes a rather annoying >> problem. >> >> >> > >> > 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 >> > -- 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