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