[PATCH] D14889: sancov -not-covered-functions.

Mike Aizatsky via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 24 11:05:45 PST 2015


aizatsky marked 3 inline comments as done.
aizatsky added a comment.

ptal


================
Comment at: lib/Object/ObjectFile.cpp:118
@@ +117,3 @@
+
+Triple ObjectFile::getTriple() const {
+  llvm::Triple TheTriple("unknown-unknown-unknown");
----------------
aizatsky wrote:
> pcc wrote:
> > samsonov wrote:
> > > pcc wrote:
> > > > I'd probably prefer if this function returned a `Target *` to reflect the fact that it isn't really supposed to (nor can it) return an accurate triple but rather something that can be used to summon appropriate bits of MC etc.
> > > Agree: you can't rely deduce triple from object file. At the very least, you should reflect that you're guessing the triple in method name.
> > Actually it seems that since the MC constructors use the target triple string we should return a triple string here at least for now.
> Sorry, can't do. Triple name is used to obtain various pieces of MC.
> Plus target lookup changes Triple object (!). That's why I can't simply return std::string instead of Triple here. I've added the clarifying comment to the .h files about the function.
I don't feel attached to it. If you'd rather see me copy this code - I'm fine.

================
Comment at: tools/sancov/sancov.cc:411
@@ -145,6 +410,3 @@
 
-    auto AddrsVector = llvm::make_unique<std::vector<uint64_t>>(
-        Addrs.begin(), Addrs.end());
-    return std::unique_ptr<CoverageData>(
-        new CoverageData(std::move(AddrsVector)));
+    return std::unique_ptr<CoverageData>(new CoverageData(std::move(Addrs)));
   }
----------------
samsonov wrote:
> `return llvm::make_unique<CoverageData>(std::move(Addrs));`
can't do because CoverageData constructor is private.

================
Comment at: tools/sancov/sancov.cc:493
@@ -266,2 +492,2 @@
   }
 }
----------------
samsonov wrote:
> Add llvm_unreachable() at the end?
again, why?


http://reviews.llvm.org/D14889





More information about the llvm-commits mailing list