Re: [clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
Ping.
Deadline to merge the changes to the 3.8 branch is today.
Cheers,
Edward-san
2016-05-24 20:25 GMT+02:00 Edoardo P. :
> Ping,
>
> who's going to merge? I have no commit access.
>
> Cheers,
> Edward-san
>
>
> 2016-05-20 18:34 GMT+02:00 Tom Stellard :
>> 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 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
>>> > 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
>>> > > :
>>> > >> 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 /
>>> > >> ./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::Replacemen
Re: [clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
Ping,
who's going to merge? I have no commit access.
Cheers,
Edward-san
2016-05-20 18:34 GMT+02:00 Tom Stellard :
> 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 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
>> > 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
>> > > :
>> > >> 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 /
>> > >> ./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.getLocWithOffs
Re: [clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
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
:
> 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 /
> ./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> Checks;
>CheckFactories->createChecks(&Context, Checks);
>
> @@ -446,8 +460,24 @@ runClangTidy(std::unique_ptr void handleErrors(const std::vector &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/ClangTid
