Skip to content

Commit f101fd8

Browse files
committed
Fixed cross-crate generic call test to compile lib and bin separately
The original test produced a single crate with two mods, which was not the goal of the test.
1 parent b0c140a commit f101fd8

File tree

34 files changed

+1390
-650
lines changed

34 files changed

+1390
-650
lines changed

Diff for: src/test/run-make-fulldeps/coverage-reports/Makefile

+27-4
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ DEBUG_FLAG=--debug
6767
endif
6868

6969
ifeq ($(LLVM_VERSION_11_PLUS),true)
70-
all: $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs))
70+
all: $(patsubst $(SOURCEDIR)/lib/%.rs,%,$(wildcard $(SOURCEDIR)/lib/*.rs)) $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs))
7171
else
7272
$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.)
7373
all:
@@ -84,12 +84,22 @@ endif
8484

8585
-include clear_expected_if_blessed
8686

87+
%: $(SOURCEDIR)/lib/%.rs
88+
# Compile the test library with coverage instrumentation
89+
$(RUSTC) $(SOURCEDIR)/lib/$@.rs \
90+
$$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/lib/$@.rs && \
91+
echo "--edition=2018" \
92+
) \
93+
--crate-type rlib \
94+
-Zinstrument-coverage
95+
8796
%: $(SOURCEDIR)/%.rs
88-
# Compile the test program with coverage instrumentation and generate relevant MIR.
97+
# Compile the test program with coverage instrumentation
8998
$(RUSTC) $(SOURCEDIR)/$@.rs \
9099
$$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && \
91100
echo "--edition=2018" \
92101
) \
102+
-L "$(TMPDIR)" \
93103
-Zinstrument-coverage
94104

95105
# Run it in order to generate some profiling data,
@@ -142,8 +152,17 @@ else
142152
# Compare the show coverage output (`--bless` refreshes `typical` files)
143153
# Note `llvm-cov show` output for some programs can vary, but can be ignored
144154
# by inserting `// ignore-llvm-cov-show-diffs` at the top of the source file.
145-
146-
$(DIFF) [email protected] "$(TMPDIR)"/[email protected] || \
155+
#
156+
# FIXME(richkadel): It looks like most past variations seem to have been mitigated. None of the
157+
# Rust test source samples have the `// ignore-llvm-cov-show-diffs` anymore. The main variation
158+
# I had seen (and is still present in the new `coverage/lib/used_crate.rs`) is the `llvm-cov show`
159+
# reporting of multiple instantiations of a generic function with different type substitutions.
160+
# For some reason, `llvm-cov show` can report these in a non-deterministic order, breaking the
161+
# `diff` comparision. I was able to work around the problem with `diff --ignore-matching-lines=RE`
162+
# to ignore each line prefixing each generic instantiation coverage code region.
163+
164+
$(DIFF) --ignore-matching-lines='::<.*>.*:$$' \
165+
147166
( grep -q '^\/\/ ignore-llvm-cov-show-diffs' $(SOURCEDIR)/[email protected] && \
148167
>&2 echo 'diff failed, but suppressed with `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/[email protected]' \
149168
) || \
@@ -177,6 +196,10 @@ endif
177196
$(call BIN,"$(TMPDIR)"/$@) \
178197
| "$(PYTHON)" $(BASEDIR)/prettify_json.py \
179198
> "$(TMPDIR)"/[email protected]
199+
# FIXME(richkadel): With the addition of `--ignore-matching-lines=RE` to ignore the
200+
# non-deterministically-ordered coverage results for multiple instantiations of generics with
201+
# differing type substitutions, I probably don't need the `.json` files anymore (and may not
202+
# need `prettify_json.py` either).
180203

181204
ifdef RUSTC_BLESS_TEST
182205

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json

+27-27
Original file line numberDiff line numberDiff line change
@@ -3,53 +3,53 @@
33
{
44
"files": [
55
{
6-
"filename": "../coverage/used_crate/mod.rs",
6+
"filename": "../coverage/lib/used_crate.rs",
77
"summary": {
88
"functions": {
9-
"count": 3,
10-
"covered": 3,
11-
"percent": 100
9+
"count": 6,
10+
"covered": 5,
11+
"percent": 83.33333333333334
1212
},
1313
"instantiations": {
14-
"count": 4,
15-
"covered": 4,
16-
"percent": 100
14+
"count": 10,
15+
"covered": 8,
16+
"percent": 80
1717
},
1818
"lines": {
19-
"count": 31,
20-
"covered": 14,
21-
"percent": 45.16129032258064
19+
"count": 46,
20+
"covered": 26,
21+
"percent": 56.52173913043478
2222
},
2323
"regions": {
24-
"count": 16,
25-
"covered": 6,
26-
"notcovered": 10,
27-
"percent": 37.5
24+
"count": 19,
25+
"covered": 8,
26+
"notcovered": 11,
27+
"percent": 42.10526315789473
2828
}
2929
}
3030
}
3131
],
3232
"totals": {
3333
"functions": {
34-
"count": 3,
35-
"covered": 3,
36-
"percent": 100
34+
"count": 6,
35+
"covered": 5,
36+
"percent": 83.33333333333334
3737
},
3838
"instantiations": {
39-
"count": 4,
40-
"covered": 4,
41-
"percent": 100
39+
"count": 10,
40+
"covered": 8,
41+
"percent": 80
4242
},
4343
"lines": {
44-
"count": 31,
45-
"covered": 14,
46-
"percent": 45.16129032258064
44+
"count": 46,
45+
"covered": 26,
46+
"percent": 56.52173913043478
4747
},
4848
"regions": {
49-
"count": 16,
50-
"covered": 6,
51-
"notcovered": 10,
52-
"percent": 37.5
49+
"count": 19,
50+
"covered": 8,
51+
"notcovered": 11,
52+
"percent": 42.10526315789473
5353
}
5454
}
5555
}

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments)]
1+
1| |#![allow(unused_assignments, dead_code)]
22
2| |
33
3| |// require-rust-edition-2018
44
4| |

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.if_else.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments)]
1+
1| |#![allow(unused_assignments, unused_variables)]
22
2| |
33
3| 1|fn main() {
44
4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments, unused_variables)]
1+
1| |#![allow(unused_assignments, unused_variables, dead_code)]
22
2| |
33
3| 1|fn main() {
44
4| | // Initialize test constants in a way that cannot be determined at compile time, to ensure

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loop_break_value.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments)]
1+
1| |#![allow(unused_assignments, unused_variables)]
22
2| |
33
3| 1|fn main() {
44
4| 1| let result

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments)]
1+
1| |#![allow(unused_assignments, unused_variables, while_true)]
22
2| |
33
3| |// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the
44
4| |// structure of this `fmt` function.

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.simple_match.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1| |#![allow(unused_assignments)]
1+
1| |#![allow(unused_assignments, unused_variables)]
22
2| |
33
3| 1|fn main() {
44
4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt

+108-22
Original file line numberDiff line numberDiff line change
@@ -12,45 +12,131 @@
1212
12| 1| countdown = 10;
1313
13| 1| }
1414
^0
15-
14| 1| used_twice_generic_function("some str");
15+
14| 1| use_this_lib_crate();
1616
15| 1|}
1717
16| |
18-
17| 1|pub fn used_generic_function<T: Debug>(arg: T) {
19-
18| 1| println!("used_generic_function with {:?}", arg);
20-
19| 1|}
18+
17| 2|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
19+
18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
20+
19| 2|}
21+
------------------
22+
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
23+
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
24+
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
25+
| 19| 1|}
26+
------------------
27+
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
28+
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
29+
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
30+
| 19| 1|}
31+
------------------
2132
20| |
22-
21| 2|pub fn used_twice_generic_function<T: Debug>(arg: T) {
23-
22| 2| println!("used_twice_generic_function with {:?}", arg);
33+
21| 2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
34+
22| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
2435
23| 2|}
2536
------------------
26-
| uses_crate::used_crate::used_twice_generic_function::<alloc::vec::Vec<i32>>:
27-
| 21| 1|pub fn used_twice_generic_function<T: Debug>(arg: T) {
28-
| 22| 1| println!("used_twice_generic_function with {:?}", arg);
37+
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
38+
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
39+
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
2940
| 23| 1|}
3041
------------------
31-
| uses_crate::used_crate::used_twice_generic_function::<&str>:
32-
| 21| 1|pub fn used_twice_generic_function<T: Debug>(arg: T) {
33-
| 22| 1| println!("used_twice_generic_function with {:?}", arg);
42+
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
43+
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
44+
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
3445
| 23| 1|}
3546
------------------
3647
24| |
37-
25| 0|pub fn unused_generic_function<T: Debug>(arg: T) {
38-
26| 0| println!("unused_generic_function with {:?}", arg);
39-
27| 0|}
48+
25| 2|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
49+
26| 2| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
50+
27| 2|}
51+
------------------
52+
| used_crate::used_from_bin_crate_and_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
53+
| 25| 1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
54+
| 26| 1| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
55+
| 27| 1|}
56+
------------------
57+
| used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>:
58+
| 25| 1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
59+
| 26| 1| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
60+
| 27| 1|}
61+
------------------
4062
28| |
41-
29| 0|pub fn unused_function() {
42-
30| 0| let is_true = std::env::args().len() == 1;
43-
31| 0| let mut countdown = 2;
44-
32| 0| if !is_true {
45-
33| 0| countdown = 20;
46-
34| 0| }
63+
29| 2|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
64+
30| 2| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
65+
31| 2|}
66+
32| |
67+
33| 0|pub fn unused_generic_function<T: Debug>(arg: T) {
68+
34| 0| println!("unused_generic_function with {:?}", arg);
4769
35| 0|}
4870
36| |
49-
37| 0|fn unused_private_function() {
71+
37| 0|pub fn unused_function() {
5072
38| 0| let is_true = std::env::args().len() == 1;
5173
39| 0| let mut countdown = 2;
5274
40| 0| if !is_true {
5375
41| 0| countdown = 20;
5476
42| 0| }
5577
43| 0|}
78+
44| |
79+
45| 0|fn unused_private_function() {
80+
46| 0| let is_true = std::env::args().len() == 1;
81+
47| 0| let mut countdown = 2;
82+
48| 0| if !is_true {
83+
49| 0| countdown = 20;
84+
50| 0| }
85+
51| 0|}
86+
52| |
87+
53| 1|fn use_this_lib_crate() {
88+
54| 1| used_from_bin_crate_and_lib_crate_generic_function("used from library used_crate.rs");
89+
55| 1| used_with_same_type_from_bin_crate_and_lib_crate_generic_function(
90+
56| 1| "used from library used_crate.rs",
91+
57| 1| );
92+
58| 1| let some_vec = vec![5, 6, 7, 8];
93+
59| 1| used_only_from_this_lib_crate_generic_function(some_vec);
94+
60| 1| used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs");
95+
61| 1|}
96+
------------------
97+
| Unexecuted instantiation: used_crate::use_this_lib_crate
98+
------------------
99+
62| |
100+
63| |// FIXME(#79651): `used_from_bin_crate_and_lib_crate_generic_function()` is covered and executed
101+
64| |// `2` times, but the coverage output also shows (at the bottom of the coverage report):
102+
65| |// ------------------
103+
66| |// | Unexecuted instantiation: <some function name here>
104+
67| |// ------------------
105+
68| |//
106+
69| |// Note, the function name shown in the error seems to change depending on the structure of the
107+
70| |// code, for some reason, including:
108+
71| |//
109+
72| |// * used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>
110+
73| |// * used_crate::use_this_lib_crate
111+
74| |//
112+
75| |// The `Unexecuted instantiation` error may be related to more than one generic function. Since the
113+
76| |// reporting is not consistent, it may not be obvious if there are multiple problems here; however,
114+
77| |// `used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>` (which I have seen
115+
78| |// with this error) is the only generic function missing instantiaion coverage counts.
116+
79| |//
117+
80| |// The `&str` variant was called from within this `lib` crate, and the `bin` crate also calls this
118+
81| |// function, but with `T` type `&Vec<i32>`.
119+
82| |//
120+
83| |// I believe the reason is that one or both crates are generating `Zero` counters for what it
121+
84| |// believes are "Unreachable" instantiations, but those instantiations are counted from the
122+
85| |// coverage map in the other crate.
123+
86| |//
124+
87| |// See `add_unreachable_coverage()` in `mapgen.rs` for more on how these `Zero` counters are added
125+
88| |// for what the funciton believes are `DefId`s that did not get codegenned. I suspect the issue
126+
89| |// may be related to this process, but this needs to be confirmed. It may not be possible to know
127+
90| |// for sure if a function is truly unused and should be reported with `Zero` coverage if it may
128+
91| |// still get used from an external crate. (Something to look at: If the `DefId` in MIR corresponds
129+
92| |// _only_ to the generic function without type parameters, is the `DefId` in the codegenned set,
130+
93| |// instantiated with one of the type parameters (in either or both crates) a *different* `DefId`?
131+
94| |// If so, `add_unreachable_coverage()` would assume the MIR `DefId` was uncovered, and would add
132+
95| |// unreachable coverage.
133+
96| |//
134+
97| |// I didn't think they could be different, but if they can, we would need to find the `DefId` for
135+
98| |// the generic function MIR and include it in the set of "codegenned" DefIds if any instantiation
136+
99| |// of that generic function does exist.
137+
100| |//
138+
101| |// Note, however, for `used_with_same_type_from_bin_crate_and_lib_crate_generic_function()` both
139+
102| |// crates use this function with the same type variant. The function does not have multiple
140+
103| |// instantiations, so the coverage analysis is not confused. No "Unexecuted instantiations" errors
141+
104| |// are reported.
56142

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.async.txt

+6-6
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,18 @@ Counter in file 0 79:14 -> 79:16, 0
2828
Counter in file 0 81:1 -> 81:2, 0
2929
Counter in file 0 91:25 -> 91:34, 0
3030
Counter in file 0 5:1 -> 5:25, #1
31-
Counter in file 0 5:25 -> 6:14, #1
32-
Counter in file 0 7:9 -> 7:10, #2
33-
Counter in file 0 9:9 -> 9:10, (#1 - #2)
34-
Counter in file 0 11:1 -> 11:2, (#2 + (#1 - #2))
3531
Counter in file 0 21:1 -> 21:23, #1
32+
Counter in file 0 17:20 -> 17:21, #1
3633
Counter in file 0 67:5 -> 67:23, #1
3734
Counter in file 0 38:1 -> 38:19, #1
35+
Counter in file 0 13:20 -> 13:21, #1
3836
Counter in file 0 29:1 -> 29:22, #1
3937
Counter in file 0 93:1 -> 101:2, #1
4038
Counter in file 0 91:1 -> 91:25, #1
39+
Counter in file 0 5:25 -> 6:14, #1
40+
Counter in file 0 7:9 -> 7:10, #2
41+
Counter in file 0 9:9 -> 9:10, (#1 - #2)
42+
Counter in file 0 11:1 -> 11:2, (#2 + (#1 - #2))
4143
Counter in file 0 38:19 -> 42:12, #1
4244
Counter in file 0 43:9 -> 43:10, #3
4345
Counter in file 0 43:14 -> 43:18, (#1 + 0)
@@ -53,7 +55,6 @@ Counter in file 0 51:5 -> 52:18, #1
5355
Counter in file 0 53:13 -> 53:14, #2
5456
Counter in file 0 63:13 -> 63:14, (#1 - #2)
5557
Counter in file 0 65:5 -> 65:6, (#2 + (#1 - #2))
56-
Counter in file 0 17:20 -> 17:21, #1
5758
Counter in file 0 49:1 -> 68:12, #1
5859
Counter in file 0 69:9 -> 69:10, #2
5960
Counter in file 0 69:14 -> 69:27, (#1 + 0)
@@ -69,7 +70,6 @@ Counter in file 0 86:14 -> 86:16, #2
6970
Counter in file 0 87:14 -> 87:16, #3
7071
Counter in file 0 89:1 -> 89:2, (#3 + (#2 + (#1 - (#3 + #2))))
7172
Counter in file 0 17:1 -> 17:20, #1
72-
Counter in file 0 13:20 -> 13:21, #1
7373
Counter in file 0 66:5 -> 66:23, #1
7474
Counter in file 0 17:9 -> 17:10, #1
7575
Counter in file 0 17:9 -> 17:10, #1

0 commit comments

Comments
 (0)