hfinkel added a comment. In http://reviews.llvm.org/D19678#415902, @rsmith wrote:
> You give this example: > > > 343 | Loc = ConvertBackendLocation(D, > > Context->getSourceManager()); > > > I | ^ > > > I | ^ > > > How does this look for a case like `p->Foo()->Bar()` (where one or both of > the calls are inlined)? Can we get the source location to point at the > function name instead of the start of the expression to reduce the scope for > ambiguity? That does not currently work very well (I assume this needs a backend fix, but I'll check). $ cat /tmp/i.cpp void ext(); struct Bar { void bar() { ext(); } }; struct Foo { Bar *b; Bar *foo() { return b; } }; void test(Foo *f) { f->foo()->bar(); } And we get: 14 I | f->foo()->bar(); because both inlining remarks come from the backend with the same column number: $ clang -O3 -c -o /tmp/i.o /tmp/i.cpp -flisting -Rpass=inline /tmp/i.cpp:14:3: remark: _ZN3Foo3fooEv inlined into _Z4testP3Foo [-Rpass=inline] f->foo()->bar(); ^ /tmp/i.cpp:14:3: remark: _ZN3Bar3barEv inlined into _Z4testP3Foo [-Rpass=inline] ================ Comment at: lib/CodeGen/CodeGenAction.cpp:665-761 @@ +664,99 @@ + +void BackendConsumer::GenerateListingFile() { + if (CodeGenOpts.ListingFile.empty()) + return; + + std::error_code EC; + llvm::raw_fd_ostream OS(CodeGenOpts.ListingFile, EC, + llvm::sys::fs::F_Text); + if (EC) { + Diags.Report(diag::err_fe_error_opening) << CodeGenOpts.ListingFile << + EC.message(); + return; + } + + SourceManager &SourceMgr = Context->getSourceManager(); + std::set<FileID> FileIDs; + for (auto &I : ListingInfo) + FileIDs.insert(SourceMgr.getFileID(I.first)); + + for (auto &FID : FileIDs) { + SourceLocation FirstLoc = SourceMgr.getLocForStartOfFile(FID); + OS << "< " << SourceMgr.getFilename(FirstLoc) << "\n"; + + auto I = ListingInfo.lower_bound(FirstLoc); + StringRef MB = SourceMgr.getBufferData(FID); + const SrcMgr::ContentCache * + Content = SourceMgr.getSLocEntry(FID).getFile().getContentCache(); + unsigned LNDigits = llvm::utostr(Content->NumLines).size(); + for (unsigned L = 0; L < Content->NumLines - 1; ++L) { + unsigned LStartOff = Content->SourceLineCache[L]; + unsigned LEndOff = (L == Content->NumLines) ? + Content->getSize() : + Content->SourceLineCache[L + 1]; + + std::map<unsigned, ListingLineInfo> ColsInfo; + unsigned InlinedCols = 0, UnrolledCols = 0, VectorizedCols = 0; + + ListingLineInfo LLI; + if (I != ListingInfo.end()) { + auto DI = SourceMgr.getDecomposedLoc(I->first); + while (I != ListingInfo.end() && DI.first == FID && + DI.second < LStartOff) { + ++I; + if (I != ListingInfo.end()) + DI = SourceMgr.getDecomposedLoc(I->first); + } + + while (I != ListingInfo.end() && DI.first == FID && + DI.second >= LStartOff && DI.second < LEndOff) { + unsigned Col = SourceMgr.getColumnNumber(FID, DI.second); + ColsInfo[Col] = I->second; + InlinedCols += I->second.Inlined.Analyzed; + UnrolledCols += I->second.Unrolled.Analyzed; + VectorizedCols += I->second.Vectorized.Analyzed; + LLI |= I->second; + + ++I; + if (I != ListingInfo.end()) + DI = SourceMgr.getDecomposedLoc(I->first); + } + } + + // We try to keep the output as concise as possible. If only one thing on + // a given line could have been inlined, vectorized, etc. then we can put + // the marker on the source line itself. If there are multiple options + // then we want to distinguish them by placing the marker for each + // transformation on a separate line following the source line. When we + // do this, we use a '^' character to point to the appropriate column in + // the source line. + + OS << llvm::format_decimal(L + 1, LNDigits) << " "; + OS << (LLI.Inlined.Transformed && InlinedCols < 2 ? "I" : " "); + OS << (LLI.Unrolled.Transformed && UnrolledCols < 2 ? "U" : " "); + OS << (LLI.Vectorized.Transformed && VectorizedCols < 2 ? "V" : " "); + + OS << " | " << MB.slice(LStartOff, LEndOff); + + for (auto &J : ColsInfo) { + if ((J.second.Inlined.Transformed && InlinedCols > 1) || + (J.second.Unrolled.Transformed && UnrolledCols > 1) || + (J.second.Vectorized.Transformed && VectorizedCols > 1)) { + OS << std::string(LNDigits + 1, ' '); + OS << (J.second.Inlined.Transformed && + InlinedCols > 1 ? "I" : " "); + OS << (J.second.Unrolled.Transformed && + UnrolledCols > 1 ? "U" : " "); + OS << (J.second.Vectorized.Transformed && + VectorizedCols > 1 ? "V" : " "); + + OS << " | " << std::string(J.first - 1, ' ') << "^\n"; + } + } + + if (LEndOff == Content->getSize()) + OS << "\n"; + } + } +} + ---------------- rsmith wrote: > I'd like this to be factored out and moved somewhere more appropriate (such > as Frontend). It seems appropriate for CodeGen to generate the data structure > here, but it should not be deciding how to format the report nor doing file > IO to put it somewhere. > > I would hope that we can combine this report information with the static > analyzer's existing support for generating syntax-highlighted, annotated > source code as HTML as a future extension. > I'd like this to be factored out and moved somewhere more appropriate (such > as Frontend). It seems appropriate for CodeGen to generate the data structure > here, but it should not be deciding how to format the report nor doing file > IO to put it somewhere. Makes sense. > I would hope that we can combine this report information with the static > analyzer's existing support for generating syntax-highlighted, annotated > source code as HTML as a future extension. I like this idea. http://reviews.llvm.org/D19678 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits