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

Reply via email to