[PATCH] D52033: [GCOV] Add options to filter files which must be instrumented.
Vedant Kumar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 20 16:34:26 PDT 2018
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
More information about the llvm-commits
mailing list