A deferred region should end before the start of a label, and should not
extend to the start of the label sub-statement.
Fixes llvm.org/PR35867.
llvm-svn: 333715
Discard the last uncompleted deferred region in a decl, if one exists.
This prevents lines at the end of a function containing only whitespace
or closing braces from being marked as uncovered, if they follow a
region terminator (return/break/etc).
The previous behavior was to heuristically complete deferred regions at
the end of a decl. In practice this ended up being too brittle for too
little gain. Users would complain that there was no way to reach full
code coverage because whitespace at the end of a function would be
marked uncovered.
rdar://40238228
Differential Revision: https://reviews.llvm.org/D46918
llvm-svn: 333609
This is similar to the LLVM change https://reviews.llvm.org/D46290.
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\@brief'); do perl -pi -e 's/\@brief //g' $i & done
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
Differential Revision: https://reviews.llvm.org/D46320
llvm-svn: 331834
When a '>>' token is split into two '>' tokens (in C++11 onwards), or (as an
extension) when we do the same for other tokens starting with a '>', we can't
just use a location pointing to the first '>' as the location of the split
token, because that would result in our miscomputing the length and spelling
for the token. As a consequence, for example, a refactoring replacing 'A<X>'
with something else would sometimes replace one character too many, and
similarly diagnostics highlighting a template-id source range would highlight
one character too many.
Fix this by creating an expansion range covering the first character of the
'>>' token, whose spelling is '>'. For this to work, we generalize the
expansion range of a macro FileID to be either a token range (the common case)
or a character range (used in this new case).
llvm-svn: 331155
Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:
archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
methode
nd
optin
ot
pres
statics
te
thru
Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few
files that have dubious fixes reverted.)
Differential revision: https://reviews.llvm.org/D44188
llvm-svn: 329399
When parsing C++ type construction expressions with list initialization,
forward the locations of the braces to Sema.
Without these locations, the code coverage pass crashes on the given test
case, because the pass relies on getLocEnd() returning a valid location.
Here is what this patch does in more detail:
- Forwards init-list brace locations to Sema (ParseExprCXX),
- Builds an InitializationKind with these locations (SemaExprCXX), and
- Uses these locations for constructor initialization (SemaInit).
The remaining changes fall out of introducing a new overload for
creating direct-list InitializationKinds.
Testing: check-clang, and a stage2 coverage-enabled build of clang with
asserts enabled.
Differential Revision: https://reviews.llvm.org/D41921
llvm-svn: 322729
Emit a gap area starting after the r-paren location and ending at the
start of the body for the braces-optional statements (for, for-each,
while, etc). The count for the gap area equal to the body's count. This
extends the fix in r317758.
Fixes PR35387, rdar://35570345
Testing: stage2 coverage-enabled build of clang, check-clang
llvm-svn: 319373
There are some limitations with emitting regions in macro expansions
because we don't gather file IDs within the expansions. Fix the check
that prevents us from emitting deferred regions in expansions to make an
exception for headers, which is something we can handle.
rdar://35373009
llvm-svn: 317760
The area immediately after a terminated region in the function top-level
should have the same count as the label it precedes.
This solves another problem with wrapped segments. Consider:
1| a:
2| return 0;
3| b:
4| return 1;
Without a gap area starting after the first return, the wrapped segment
from line 2 would make it look like line 3 is executed, when it's not.
rdar://35373009
llvm-svn: 317759
The area immediately after the closing right-paren of an if condition
should have a count equal to the 'then' block's count. Use a gap region
to set this count, so that region highlighting for the 'then' block
remains precise.
This solves a problem we have with wrapped segments. Consider:
1| if (false)
2| foo();
Without a gap area starting after the condition, the wrapped segment
from line 1 would make it look like line 2 is executed, when it's not.
rdar://35373009
llvm-svn: 317758
A trailing deferred region isn't necessary in a function that ends with
this pattern:
...
else {
...
return;
}
Special-case this pattern so that the closing curly brace of the
function isn't marked as uncovered. This issue came up in PR34962.
llvm-svn: 315982
This makes it possible to view sub-line region counts for the l.h.s of
&& and || expressions in coverage reports.
It also fixes PR33465, which shows an example of incorrect coverage
output for an assignment statement containing '||'.
llvm-svn: 315979
As a special case, throw away deferred regions for trailing returns.
This allows the closing curly brace to have a count, and is less
distracting.
llvm-svn: 313603
This patch teaches the preprocessor to report more precise source ranges for
code that is skipped due to conditional directives.
The new behavior includes the '#' from the opening directive and the full text
of the line containing the closing directive in the skipped area. This matches
up clang's behavior (we don't IRGen the code between the closing "endif" and
the end of a line).
This also affects the code coverage implementation. See llvm.org/PR34166 (this
also happens to be rdar://problem/23224058).
The old behavior (report the end of the skipped range as the end
location of the 'endif' token) is preserved for indexing clients.
Differential Revision: https://reviews.llvm.org/D36642
llvm-svn: 312947
The current coverage implementation doesn't handle region termination
very precisely. Take for example an `if' statement with a `return':
void f() {
if (true) {
return; // The `if' body's region is terminated here.
}
// This line gets the same coverage as the `if' condition.
}
If the function `f' is called, the line containing the comment will be
marked as having executed once, which is not correct.
The solution here is to create a deferred region after terminating a
region. The deferred region is completed once the start location of the
next statement is known, and is then pushed onto the region stack.
In the cases where it's not possible to complete a deferred region, it
can safely be dropped.
Testing: lit test updates, a stage2 coverage-enabled build of clang
This is a reapplication but there are no changes from the original commit.
With D36813, the segment builder in llvm will be able to handle deferred
regions correctly.
llvm-svn: 312818
The code after a noreturn call doesn't execute.
The pattern in the testcase is pretty common in LLVM (a switch with
a default case that calls llvm_unreachable).
The original version of this patch was reverted in r309995 due to a
crash. This version includes a fix for that crash (testcase in
test/CoverageMapping/md.cpp).
Differential Revision: https://reviews.llvm.org/D36250
llvm-svn: 310406
This reverts commit r310010. I don't think there's anything wrong with
this commit, but it's causing clang to generate output that llvm-cov
doesn't do a good job with and the fix isn't immediately clear.
See Eli's comment in D36250 for more context.
I'm reverting the clang change so the coverage bot can revert back to
producing sensible output, and to give myself some time to investigate
what went wrong in llvm.
llvm-svn: 310154
The current coverage implementation doesn't handle region termination
very precisely. Take for example an `if' statement with a `return':
void f() {
if (true) {
return; // The `if' body's region is terminated here.
}
// This line gets the same coverage as the `if' condition.
}
If the function `f' is called, the line containing the comment will be
marked as having executed once, which is not correct.
The solution here is to create a deferred region after terminating a
region. The deferred region is completed once the start location of the
next statement is known, and is then pushed onto the region stack.
In the cases where it's not possible to complete a deferred region, it
can safely be dropped.
Testing: lit test updates, a stage2 coverage-enabled build of clang
llvm-svn: 310010
The code after a noreturn call doesn't execute.
The pattern in the testcase is pretty common in LLVM (a switch with
a default case that calls llvm_unreachable).
Differential Revision: https://reviews.llvm.org/D36250
llvm-svn: 309995
We never overwrite the end location of a region, so we would end up with
an overly large region when we reused the switch's region.
It's possible this code will be substantially rewritten in the near
future to deal with fallthrough more accurately, but this seems like
an improvement on its own for now.
Differential Revision: https://reviews.llvm.org/D34801
llvm-svn: 309901
The fixed code is basically identical to the same loop below, which
might indicate an opportunity for refactoring. I just wanted to fix
the use-of-temporary issue.
Caught by adding a similar check to StringRef as r283798 did for
ArrayRef. I'll be upstreaming that soon.
Reviewed by Vedant Kumar as https://reviews.llvm.org/D26317.
llvm-svn: 286122
This patch fixes a regression introduced in r262697 that changed the way the
coverage regions for switches are constructed. The PGO instrumentation counter
for a switch statement refers to the counter at the exit of the switch.
Therefore, the coverage region for the switch statement should cover the code
that comes after the switch, and not the switch statement itself.
rdar://28480997
Differential Revision: https://reviews.llvm.org/D24981
llvm-svn: 282554
In most cases these code regions are just redundant, but sometimes they
could be assigned to the counter of the parent code region instead of
the counter of the nested block.
Differential Revision: https://reviews.llvm.org/D23987
llvm-svn: 280199
If there were several nested statements arranged in a way that all of them
end up with the same macro, then the expansion of this macro was assigned
with all the corresponding counters of these statements.
As a result, the wrong counter value was shown for the macro in llvm-cov.
This patch fixes the issue by preventing adding a counter for an expanded
source range if it already has an assigned counter, which is expected
to come from the most specific statement.
Differential Revision: https://reviews.llvm.org/D23160
llvm-svn: 279962
After r275121, we stopped mapping regions from system headers. Lambdas
declared in regions belonging to system headers started producing empty
coverage mappings, since the files corresponding to their spelling locs
were being ignored.
The coverage reader doesn't know what to do with these empty mappings.
This commit makes sure that we don't produce them and adds a test. I'll
make the reader stricter in a follow-up commit.
llvm-svn: 276716
This fixes the issue of having duplicate entries for the same file in a
coverage report s.t none of the entries actually displayed the correct
coverage information.
llvm-svn: 275913
Do not assign source regions located within system headers file ID's,
and do not construct counter mapping regions out of them.
This makes coverage reports less cluttered and less mysterious. E.g
using the "assert" macro doesn't cause assert.h to appear in reports,
and it no longer shows the "assertion failed" branch as an uncovered
region.
It also makes coverage mapping sections a bit smaller (e.g a 1%
reduction in a stage2 build of bin/llvm-as).
llvm-svn: 275121
This reverts commit 161ff9db3a3d0d62880d1cb18d58182cd3034912 (r273056).
This is breaking stage2 instrumented builds with "malformed coverage
data" errors.
llvm-svn: 274104
Push a new region for the try block and propagate execution counts
through it. This ensures that catch statements get a region counter
distinct from the try block's counter.
llvm-svn: 273463
We have an assertion failure if, for example, the definition of an unused
inline function starts in one macro and ends in another. This patch fixes
the issue by finding the common ancestor of the start and end locations
of that function's body and changing the locations accordingly.
Thanks to NAKAMURA Takumi for helping with fixing the test failure on Windows.
Differential Revision: http://reviews.llvm.org/D20997
llvm-svn: 271995
We have an assertion failure if, for example, the definition of an unused
inline function starts in one macro and ends in another. This patch fixes
the issue by finding the common ancestor of the start and end locations
of that function's body and changing the locations accordingly.
Differential Revision: http://reviews.llvm.org/D20997
llvm-svn: 271969
I added this call in r271308. It's redundant because it's dominated by a
call to extendRegion().
Thanks to Justin Bogner for pointing this out!
llvm-svn: 271331
The issue happened when a macro contained a full for or
while statement, which body ended at the end of the macro.
Differential Revision: http://reviews.llvm.org/D19725
llvm-svn: 268511
While pushing switch statements onto the region stack we neglected to
specify their start/end locations. This results in a crash (PR26825) if
we end up in nested macro expansions without enough information to
handle the relevant file exits.
I added a test in switchmacro.c and fixed up a bunch of incorrect CHECK
lines that specify strange end locations for switches.
llvm-svn: 262697
When handling 'if' statements, we crash if the condition and the consequent
branch are spanned by a single macro expansion.
The crash occurs because of a sanity 'reset' in popRegions(): if an expansion
exactly spans an entire region, we set MostRecentLocation to the start of the
expansion (its 'include location'). This ensures we don't handleFileExit()
ourselves out of the expansion before we're done processing all of the regions
within it. This is tested in test/CoverageMapping/macro-expressions.c.
This causes a problem when an expansion spans both the condition and the
consequent branch of an 'if' statement. MostRecentLocation is updated to the
start of the 'if' statement in popRegions(), so the file for the expansion
isn't exited by the time we're done handling the statement. We then crash with
'fatal: File exit not handled before popRegions'.
The fix for this is to detect these kinds of expansions, and conservatively
update MostRecentLocation to the end of expansion region containing the
conditional. I've added tests to make sure we don't have the same problem with
other kinds of statements.
rdar://problem/23630316
Differential Revision: http://reviews.llvm.org/D16934
llvm-svn: 260129
Replace a string append operation in addFunctionMappingRecord with a
vector append. The existing behavior is quadratic in the worst case:
this patch makes it linear.
Differential Revision: http://reviews.llvm.org/D16395
llvm-svn: 258424
Coverage mapping data may reference names of functions
that are skipped by FE (e.g, unused inline functions). Since
those functions are skipped, normal instr-prof function lowering
pass won't put those names in the right section, so special
handling is needed to walk through coverage mapping structure
and recollect the references.
With this patch, only names that are skipped are processed. This
simplifies the lowering code and it no longer needs to make
assumptions coverage mapping data layout. It should also be
more efficient.
llvm-svn: 257092
This is one last remaining instrumentatation related structure
that needs to be migrate to use the centralized template
definition. With this change, instrumentation code
related to coverage module header will be kept in sync
with the coverage mapping reader. The remaining code
which makes implicit assumption about covmap control
structure layout in the the lowering pass will cleaned
up in a different patch. This patch is not intended to
have no functional change.
llvm-svn: 256714
In llvm commit r243581, a reverse range adapter was added which allows
us to change code such as
for (auto I = Fields.rbegin(), E = Fields.rend(); I != E; ++I) {
in to
for (const FieldDecl *I : llvm::reverse(Fields))
This commit changes a few of the places in clang which are eligible to use
this new adapter.
llvm-svn: 243663
The catch keyword isn't really part of a region, so it's fairly
meaningless to extend into it. This was usually harmless, but it could
crash when catch blocks involved macros in strange ways.
llvm-svn: 243066
If this assert does fire, the no-asserts behaviour is an infinite
loop. It's better to crash in this case so we get a crash report and
stop wasting the user's cpu cycles.
llvm-svn: 242591
The patch is generated using this command:
$ tools/extra/clang-tidy/tool/run-clang-tidy.py -fix \
-checks=-*,llvm-namespace-comment -header-filter='llvm/.*|clang/.*' \
work/llvm/tools/clang
To reduce churn, not touching namespaces spanning less than 10 lines.
llvm-svn: 240270
We were propagating the coverage map into the body of an if statement,
but not into the condition thereafter. This is fine as long as the two
locations are in the same virtual file, but they won't be when the
"if" part of the statement is from a macro and the condition is not.
llvm-svn: 239803
The issue I was trying to solve in r236547 was about built-in macros,
but I disabled coverage in all system macros. This is actually a bit
of overkill, and makes the display of coverage around system macros
degrade unnecessarily. Instead, limit this to builtins specifically.
llvm-svn: 237397
It doesn't make much sense to try to show coverage inside system
macros, and source locations in builtins confuses the coverage
mapping. Just avoid doing this.
Fixes an assert that fired when a __block storage specifier starts a
region.
llvm-svn: 236547
This fixes a crash when we're emitting coverage and a macro appears
between two binary conditional operators, ie, "foo ?: MACRO ?: bar",
and fixes the interaction of macros and conditional operators in
general.
llvm-svn: 235793
When we try to find the end loc for a token, we have to re-lex the
token. This was running into a problem when we'd store the end loc of
a macro's coverage region, since we wouldn't actually be at the
beginning of a token when we tried to re-lex it, leading us to do
silly things (and eventually assert) when whitespace or comments
followed.
This pushes our use of getPreciseTokenLocEnd earlier, so that we won't
call it when it doesn't make sense to. It also removes an unnecessary
adjustment by 1 that was working around this problem in some cases.
llvm-svn: 233169
When generating coverage maps, we were traversing the body as if it
were part of the parent function, but this doesn't make sense since
we're currently counting lambdas as separate functions.
llvm-svn: 230304
When tools like llvm-cov show regions, it's much easier to understand
what's happening if the condition of an if shows a counter as well as
the body.
llvm-svn: 229813
The coverage mapping generation code previously generated a large
number of redundant coverage regions and then tried to merge similar
ones back together. This then relied on some awkward heuristics to
prevent combining of regions that were importantly different but
happened to have the same count. The end result was inefficient and
hard to follow.
Now, we more carefully create the regions we actually want. This makes
it much easier to create regions at precise locations as well as
making the basic approach quite a bit easier to follow. There's still
a fair bit of complexity here dealing with included code and macro
expansions, but that's pretty hard to avoid without significantly
reducing the quality of data we provide.
I had to modify quite a few tests where the source ranges became more
precise or the old ranges seemed to be wrong anyways, and I've added
quite a few new tests since a large number of constructs didn't seem
to be tested before.
llvm-svn: 229748
Richard Smith