On May 3, 2012, at 11:10 AM, Manuel Klimek <kli...@google.com> wrote:
> On Fri, Apr 27, 2012 at 7:49 PM, Douglas Gregor <dgre...@apple.com> wrote: >> On Apr 19, 2012, at 4:30 AM, Manuel Klimek <kli...@google.com> wrote: >> >>> The attached patch adds a layer on top of the tooling support for >>> cross-TU refactorings. >> >> >> +static const char * const InvalidLocation = "invalid-location"; >> + >> +Replacement::Replacement() >> + : FilePath(InvalidLocation), Offset(0), Length(0) {} >> + >> >> If we have to pick a sentinel, can it at least be an empty FilePath? >> >> +bool saveRewrittenFiles(Rewriter &Rewrite) { >> + for (Rewriter::buffer_iterator I = Rewrite.buffer_begin(), >> + E = Rewrite.buffer_end(); >> + I != E; ++I) { >> + // FIXME: This code is copied from the FixItRewriter.cpp - I think it >> should >> + // go into directly into Rewriter (there we also have the Diagnostics to >> + // handle the error cases better). >> + const FileEntry *Entry = >> + Rewrite.getSourceMgr().getFileEntryForID(I->first); >> + std::string ErrorInfo; >> + llvm::raw_fd_ostream FileStream( >> + Entry->getName(), ErrorInfo, llvm::raw_fd_ostream::F_Binary); >> + if (!ErrorInfo.empty()) >> + return false; >> + I->second.write(FileStream); >> + FileStream.flush(); >> + } >> + return true; >> +} >> >> I agree that this should go into the Rewriter. Go for it :) >> >> +int getRangeSize(SourceManager &Sources, const CharSourceRange &Range) { >> + SourceLocation SpellingBegin = Sources.getSpellingLoc(Range.getBegin()); >> + SourceLocation SpellingEnd = Sources.getSpellingLoc(Range.getEnd()); >> + std::pair<FileID, unsigned> Start = >> Sources.getDecomposedLoc(SpellingBegin); >> + std::pair<FileID, unsigned> End = Sources.getDecomposedLoc(SpellingEnd); >> + if (Start.first != End.first) return -1; >> + if (Range.isTokenRange()) >> + // FIXME: Bring in the correct LangOptions. >> + End.second += Lexer::MeasureTokenLength(SpellingEnd, Sources, >> + LangOptions()); >> + return End.second - Start.second; >> +} >> >> This should probably return an 'unsigned'. It belongs in the Lexer, IMO. > > So return bool and use and unsigned reference to return the value? > > My main concern with moving that right now is that we recently > discovered that this might not be the right solution yet to create a > replacement from an AST node - we'll probably want to drill through > macros and find the right point inside the expansion closest to the > spelling where the full node is expressed. > I've looked at makeFileCharRange in the Lexer, which seemed related at > a first glance, but I have no idea what it really does :) > > My problem is currently that I could need some ideas on what to do > here, as the general case seems really hard to implement and I'm a > little lost. This is the kind of thing Argyrios knows all about :) CC'd. > On the other hand, I think using the spelling location is a good first > approximation (as it actually works most of the time), so I think this > is a good first step, and I don't think keeping the implementation of > how to figure out what range is covered by a node a detail of the > refactoring lib is a big problem right now. > > Thoughts? It's fine to keep it as an implementation detail while working out the details. - Doug _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits