[clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
Edoardo P. via cfe-commits
cfe-commits at lists.llvm.org
Tue May 24 11:25:12 PDT 2016
Ping,
who's going to merge? I have no commit access.
Cheers,
Edward-san
2016-05-20 18:34 GMT+02:00 Tom Stellard <tom at 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 <hans at 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 at 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 at 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 at 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 at 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)
More information about the cfe-commits
mailing list