[PATCH] D52033: [GCOV] Add options to filter files which must be instrumented.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: lib/Transforms/Instrumentation/GCOVProfiling.cpp:487
+          !isFilenameMatchingRegex(RealFilename, ExcludeRe);
+  }
+  InstrumentedFiles[Filename] = Ret;
----------------
calixte wrote:
> vsk wrote:
> > Does this do the right thing if both sets of filters are empty?
> > 
> > Stepping back a bit, what is the motivation for having both inclusion & exclusion filters? Why not just have one?
> > 
> > Is it so that you can, say, exclude "/usr/*", but also include "/usr/local/*"? In that case the user needs to know the order in which you apply filters, because they're not commutative.
> If both are empty then we returned at the beginning of the function.
> 
> Having only one is quite difficult to use.
> The 2 common cases I thought about:
>  - exclude /usr/include/ stuff
>  - include only foo.cpp, bar.cpp (to make code coverage on modified files in a patch)
> Having only one will induce that it will be more difficult to write regex.
> 
> When we've both then a file is included when it matches one of the inc regex and when it doesn't match all the exc regex.
> So exc /usr/* and inc /usr/local/* means that all files in /usr/ are rejected, if you want only the /usr/local/ just don't put exc stuff.
> And exc /usr/local/* and inc /usr/* means that we take all files in /usr except the ones in /usr/local.
> But definitely, I must make a clearer doc.
I see, what I was looking for is the rule you specified for when inclusion + exclusion lists exist. Sounds good.


================
Comment at: lib/Transforms/Instrumentation/GCOVProfiling.cpp:474
+  const StringRef RealFilename =
+      sys::fs::real_path(Filename, RealPath) ? Filename : StringRef(RealPath);
+
----------------
If it's safe to use `Filename` when there's an error getting the real_path, I'm not sure why it's necessary to use the real_path at all. Why not fall back to the default behavior when there's an error?


Repository:
  rL LLVM

https://reviews.llvm.org/D52033