From: Nick Lewycky Date: Thu, 15 Apr 2010 06:46:58 +0000 (+0000) Subject: Teach -fixit to modify all of its inputs instead of just the main file, unless X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d4a97a18ea3cda3ba095e7c0c6708e7a39cf31db;p=clang Teach -fixit to modify all of its inputs instead of just the main file, unless -fixit-at specified a particular fixit to fix, or the -o flag was used. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101359 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Frontend/FixItRewriter.h b/include/clang/Frontend/FixItRewriter.h index fac87afade..95fd3da3c6 100644 --- a/include/clang/Frontend/FixItRewriter.h +++ b/include/clang/Frontend/FixItRewriter.h @@ -16,16 +16,20 @@ #define LLVM_CLANG_FRONTEND_FIX_IT_REWRITER_H #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Rewrite/Rewriter.h" #include "llvm/ADT/SmallVector.h" +namespace llvm { class raw_ostream; } + namespace clang { class SourceManager; class FileEntry; /// \brief Stores a source location in the form that it shows up on -/// the Clang command line, e.g., file:line:column. +/// the Clang command line, e.g., file:line:column. A line and column of zero +/// indicates the whole file. /// /// FIXME: Would prefer to use real SourceLocations, but I don't see a /// good way to resolve them during parsing. @@ -56,6 +60,8 @@ class FixItRewriter : public DiagnosticClient { llvm::SmallVector FixItLocations; public: + typedef Rewriter::buffer_iterator iterator; + /// \brief Initialize a new fix-it rewriter. FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr, const LangOptions &LangOpts); @@ -69,16 +75,29 @@ public: FixItLocations.push_back(Loc); } - /// \brief Write the modified source file. + /// \brief Check whether there are modifications for a given file. + bool IsModified(FileID ID) const { + return Rewrite.getRewriteBufferFor(ID) != NULL; + } + + // Iteration over files with changes. + iterator buffer_begin() { return Rewrite.buffer_begin(); } + iterator buffer_end() { return Rewrite.buffer_end(); } + + /// \brief Write a single modified source file. + /// + /// \returns true if there was an error, false otherwise. + bool WriteFixedFile(FileID ID, llvm::raw_ostream &OS); + + /// \brief Write the modified source files. /// /// \returns true if there was an error, false otherwise. - bool WriteFixedFile(const std::string &InFileName, - const std::string &OutFileName = std::string()); + bool WriteFixedFiles(); /// IncludeInDiagnosticCounts - This method (whose default implementation - /// returns true) indicates whether the diagnostics handled by this - /// DiagnosticClient should be included in the number of diagnostics - /// reported by Diagnostic. + /// returns true) indicates whether the diagnostics handled by this + /// DiagnosticClient should be included in the number of diagnostics + /// reported by Diagnostic. virtual bool IncludeInDiagnosticCounts() const; /// HandleDiagnostic - Handle this diagnostic, reporting it to the user or diff --git a/include/clang/Rewrite/Rewriter.h b/include/clang/Rewrite/Rewriter.h index 1692180a6d..d99ca8bb1f 100644 --- a/include/clang/Rewrite/Rewriter.h +++ b/include/clang/Rewrite/Rewriter.h @@ -25,9 +25,9 @@ #include "llvm/ADT/StringRef.h" namespace clang { - class SourceManager; class LangOptions; class Rewriter; + class SourceManager; class Stmt; /// RewriteBuffer - As code is rewritten, SourceBuffer's from the original @@ -125,6 +125,8 @@ class Rewriter { const LangOptions *LangOpts; std::map RewriteBuffers; public: + typedef std::map::iterator buffer_iterator; + explicit Rewriter(SourceManager &SM, const LangOptions &LO) : SourceMgr(&SM), LangOpts(&LO) {} explicit Rewriter() : SourceMgr(0), LangOpts(0) {} @@ -192,6 +194,12 @@ public: /// could not be rewritten, or false if successful. bool ReplaceStmt(Stmt *From, Stmt *To); + /// getEditBuffer - This is like getRewriteBufferFor, but always returns a + /// buffer, and allows you to write on it directly. This is useful if you + /// want efficient low-level access to apis for scribbling on one specific + /// FileID's buffer. + RewriteBuffer &getEditBuffer(FileID FID); + /// getRewriteBufferFor - Return the rewrite buffer for the specified FileID. /// If no modification has been made to it, return null. const RewriteBuffer *getRewriteBufferFor(FileID FID) const { @@ -200,11 +208,9 @@ public: return I == RewriteBuffers.end() ? 0 : &I->second; } - /// getEditBuffer - This is like getRewriteBufferFor, but always returns a - /// buffer, and allows you to write on it directly. This is useful if you - /// want efficient low-level access to apis for scribbling on one specific - /// FileID's buffer. - RewriteBuffer &getEditBuffer(FileID FID); + // Iterators over rewrite buffers. + buffer_iterator buffer_begin() { return RewriteBuffers.begin(); } + buffer_iterator buffer_end() { return RewriteBuffers.end(); } private: unsigned getLocationOffsetAndFileID(SourceLocation Loc, FileID &FID) const; diff --git a/lib/Frontend/FixItRewriter.cpp b/lib/Frontend/FixItRewriter.cpp index 20d452e76a..b26401212f 100644 --- a/lib/Frontend/FixItRewriter.cpp +++ b/lib/Frontend/FixItRewriter.cpp @@ -14,6 +14,8 @@ //===----------------------------------------------------------------------===// #include "clang/Frontend/FixItRewriter.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/FrontendDiagnostic.h" #include "llvm/Support/raw_ostream.h" @@ -34,47 +36,43 @@ FixItRewriter::~FixItRewriter() { Diags.setClient(Client); } -bool FixItRewriter::WriteFixedFile(const std::string &InFileName, - const std::string &OutFileName) { +bool FixItRewriter::WriteFixedFile(FileID ID, llvm::raw_ostream &OS) { + const RewriteBuffer *RewriteBuf = Rewrite.getRewriteBufferFor(ID); + if (!RewriteBuf) return true; + OS << std::string(RewriteBuf->begin(), RewriteBuf->end()); + OS.flush(); + return false; +} + +bool FixItRewriter::WriteFixedFiles() { if (NumFailures > 0) { Diag(FullSourceLoc(), diag::warn_fixit_no_changes); return true; } - llvm::OwningPtr OwnedStream; - llvm::raw_ostream *OutFile; - if (!OutFileName.empty()) { - std::string Err; - OutFile = new llvm::raw_fd_ostream(OutFileName.c_str(), Err, - llvm::raw_fd_ostream::F_Binary); - OwnedStream.reset(OutFile); - } else if (InFileName == "-") { - OutFile = &llvm::outs(); - } else { - llvm::sys::Path Path(InFileName); + for (iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { + const FileEntry *Entry = Rewrite.getSourceMgr().getFileEntryForID(I->first); + llvm::sys::Path Path(Entry->getName()); std::string Suffix = Path.getSuffix(); Path.eraseSuffix(); Path.appendSuffix("fixit." + Suffix); std::string Err; - OutFile = new llvm::raw_fd_ostream(Path.c_str(), Err, - llvm::raw_fd_ostream::F_Binary); - OwnedStream.reset(OutFile); - } - - FileID MainFileID = Rewrite.getSourceMgr().getMainFileID(); - if (const RewriteBuffer *RewriteBuf = - Rewrite.getRewriteBufferFor(MainFileID)) { - *OutFile << std::string(RewriteBuf->begin(), RewriteBuf->end()); - } else { - Diag(FullSourceLoc(), diag::note_fixit_main_file_unchanged); + llvm::raw_fd_ostream OS(Path.c_str(), Err, llvm::raw_fd_ostream::F_Binary); + if (!Err.empty()) { + Diags.Report(clang::diag::err_fe_unable_to_open_output) + << Path.c_str() << Err; + continue; + } + RewriteBuffer &RewriteBuf = I->second; + OS << std::string(RewriteBuf.begin(), RewriteBuf.end()); + OS.flush(); } - OutFile->flush(); return false; } bool FixItRewriter::IncludeInDiagnosticCounts() const { - return Client? Client->IncludeInDiagnosticCounts() : true; + return Client ? Client->IncludeInDiagnosticCounts() : true; } void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel, @@ -85,6 +83,7 @@ void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel, if (DiagLevel == Diagnostic::Ignored) return; + const SourceManager &SM = Rewrite.getSourceMgr(); if (!FixItLocations.empty()) { // The user has specified the locations where we should perform // the various fix-it modifications. @@ -99,15 +98,17 @@ void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel, // See if the location of the error is one that matches what the // user requested. bool AcceptableLocation = false; - const FileEntry *File - = Rewrite.getSourceMgr().getFileEntryForID( - Info.getLocation().getFileID()); + const FileEntry *File = SM.getFileEntryForID( + Info.getLocation().getFileID()); unsigned Line = Info.getLocation().getSpellingLineNumber(); unsigned Column = Info.getLocation().getSpellingColumnNumber(); for (llvm::SmallVector::iterator Loc = FixItLocations.begin(), LocEnd = FixItLocations.end(); Loc != LocEnd; ++Loc) { - if (Loc->File == File && Loc->Line == Line && Loc->Column == Column) { + if (Loc->File == File && + ((Loc->Line == 0 && Loc->Column == 0 && + DiagLevel > Diagnostic::Note) || + (Loc->Line == Line && Loc->Column == Column))) { AcceptableLocation = true; break; } diff --git a/lib/Frontend/FrontendActions.cpp b/lib/Frontend/FrontendActions.cpp index 6d7c6a8460..ce3e841b90 100644 --- a/lib/Frontend/FrontendActions.cpp +++ b/lib/Frontend/FrontendActions.cpp @@ -134,6 +134,23 @@ static bool AddFixItLocations(CompilerInstance &CI, FixItRewrite.addFixItLocation(Requested); } + const std::string &OutputFile = CI.getFrontendOpts().OutputFile; + if (Locs.empty() && !OutputFile.empty()) { + // FIXME: we will issue "FIX-IT applied suggested code changes" for every + // input, but only the main file will actually be rewritten. + const std::vector > &Inputs = + CI.getFrontendOpts().Inputs; + for (unsigned i = 0, e = Inputs.size(); i != e; ++i) { + const FileEntry *File = CI.getFileManager().getFile(Inputs[i].second); + assert(File && "Input file not found in FileManager"); + RequestedSourceLocation Requested; + Requested.File = File; + Requested.Line = 0; + Requested.Column = 0; + FixItRewrite.addFixItLocation(Requested); + } + } + return true; } @@ -149,7 +166,34 @@ bool FixItAction::BeginSourceFileAction(CompilerInstance &CI, void FixItAction::EndSourceFileAction() { const FrontendOptions &FEOpts = getCompilerInstance().getFrontendOpts(); - Rewriter->WriteFixedFile(getCurrentFile(), FEOpts.OutputFile); + if (!FEOpts.OutputFile.empty()) { + // When called with 'clang -fixit -o filename' output only the main file. + + const SourceManager &SM = getCompilerInstance().getSourceManager(); + FileID MainFileID = SM.getMainFileID(); + if (!Rewriter->IsModified(MainFileID)) { + getCompilerInstance().getDiagnostics().Report( + diag::note_fixit_main_file_unchanged); + return; + } + + llvm::OwningPtr OwnedStream; + llvm::raw_ostream *OutFile; + if (FEOpts.OutputFile == "-") { + OutFile = &llvm::outs(); + } else { + std::string Err; + OutFile = new llvm::raw_fd_ostream(FEOpts.OutputFile.c_str(), Err, + llvm::raw_fd_ostream::F_Binary); + OwnedStream.reset(OutFile); + } + + Rewriter->WriteFixedFile(MainFileID, *OutFile); + return; + } + + // Otherwise rewrite all files. + Rewriter->WriteFixedFiles(); } ASTConsumer *RewriteObjCAction::CreateASTConsumer(CompilerInstance &CI, diff --git a/test/FixIt/fixit-errors.c b/test/FixIt/fixit-errors.c index 7bf9a58430..0a60e5b117 100644 --- a/test/FixIt/fixit-errors.c +++ b/test/FixIt/fixit-errors.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -pedantic -fixit %s -o - | %clang_cc1 -pedantic -Werror -x c - +// XFAIL: * /* This is a test of the various code modification hints that are provided as part of warning or extension diagnostics. All of the diff --git a/test/FixIt/fixit-pmem.cpp b/test/FixIt/fixit-pmem.cpp index 0926309a9a..8dfab8f658 100644 --- a/test/FixIt/fixit-pmem.cpp +++ b/test/FixIt/fixit-pmem.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -pedantic -fixit %s -o - | %clang_cc1 -fsyntax-only -pedantic -Werror -x c++ - +// XFAIL: * /* This is a test of the various code modification hints that are provided as part of warning or extension diagnostics. All of the diff --git a/test/FixIt/typo.m b/test/FixIt/typo.m index ccd94b78c5..fa0918a1e6 100644 --- a/test/FixIt/typo.m +++ b/test/FixIt/typo.m @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s // FIXME: the test below isn't testing quite what we want... // RUN: %clang_cc1 -fsyntax-only -fixit -o - %s | %clang_cc1 -fsyntax-only -pedantic -Werror -x objective-c - +// XFAIL: * @interface NSString // expected-note{{'NSString' declared here}} + (int)method:(int)x;