Skip to content

Commit

Permalink
Auto merge of #119069 - matthiaskrgr:rollup-xxk4m30, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 5 pull requests

Successful merges:

 - #118852 (coverage: Skip instrumenting a function if no spans were extracted from MIR)
 - #118905 ([AIX] Fix XCOFF metadata)
 - #118967 (Add better ICE messages for some undescriptive panics)
 - #119051 (Replace `FileAllocationInfo` with `FileEndOfFileInfo`)
 - #119059 (Deny `~const` trait bounds in inherent impl headers)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Dec 18, 2023
2 parents cda4736 + c088f6a commit e004adb
Show file tree
Hide file tree
Showing 18 changed files with 162 additions and 47 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_ast::{self as ast, *};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, PartialRes, Res};
use rustc_hir::GenericArg;
use rustc_middle::span_bug;
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{BytePos, Span, DUMMY_SP};

Expand Down Expand Up @@ -285,7 +286,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let (start, end) = match self.resolver.get_lifetime_res(segment_id) {
Some(LifetimeRes::ElidedAnchor { start, end }) => (start, end),
None => return,
Some(_) => panic!(),
Some(res) => {
span_bug!(path_span, "expected an elided lifetime to insert. found {res:?}")
}
};
let expected_lifetimes = end.as_usize() - start.as_usize();
debug!(expected_lifetimes);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ ast_passes_tilde_const_disallowed = `~const` is not allowed here
.closure = closures cannot have `~const` trait bounds
.function = this function is not `const`, so it cannot have `~const` trait bounds
.trait = this trait is not a `#[const_trait]`, so it cannot have `~const` trait bounds
.impl = this impl is not `const`, so it cannot have `~const` trait bounds
.trait_impl = this impl is not `const`, so it cannot have `~const` trait bounds
.impl = inherent impls cannot have `~const` trait bounds
.object = trait objects cannot have `~const` trait bounds
.item = this item cannot have `~const` trait bounds
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ enum DisallowTildeConstContext<'a> {
TraitObject,
Fn(FnKind<'a>),
Trait(Span),
TraitImpl(Span),
Impl(Span),
Item,
}
Expand Down Expand Up @@ -836,7 +837,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
this.visit_vis(&item.vis);
this.visit_ident(item.ident);
let disallowed = matches!(constness, Const::No)
.then(|| DisallowTildeConstContext::Impl(item.span));
.then(|| DisallowTildeConstContext::TraitImpl(item.span));
this.with_tilde_const(disallowed, |this| this.visit_generics(generics));
this.visit_trait_ref(t);
this.visit_ty(self_ty);
Expand Down Expand Up @@ -889,7 +890,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

self.visit_vis(&item.vis);
self.visit_ident(item.ident);
self.with_tilde_const(None, |this| this.visit_generics(generics));
self.with_tilde_const(Some(DisallowTildeConstContext::Impl(item.span)), |this| {
this.visit_generics(generics)
});
self.visit_ty(self_ty);
walk_list!(self, visit_assoc_item, items, AssocCtxt::Impl);
walk_list!(self, visit_attribute, &item.attrs);
Expand Down Expand Up @@ -1215,7 +1218,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
&DisallowTildeConstContext::Trait(span) => {
errors::TildeConstReason::Trait { span }
}
&DisallowTildeConstContext::TraitImpl(span) => {
errors::TildeConstReason::TraitImpl { span }
}
&DisallowTildeConstContext::Impl(span) => {
// FIXME(effects): Consider providing a help message or even a structured
// suggestion for moving such bounds to the assoc const fns if available.
errors::TildeConstReason::Impl { span }
}
DisallowTildeConstContext::TraitObject => {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,11 @@ pub enum TildeConstReason {
#[primary_span]
span: Span,
},
#[note(ast_passes_trait_impl)]
TraitImpl {
#[primary_span]
span: Span,
},
#[note(ast_passes_impl)]
Impl {
#[primary_span]
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,9 @@ impl<'a> State<'a> {
}
match bound {
ast::GenericBound::Outlives(lt) => self.print_lifetime(*lt),
_ => panic!(),
_ => {
panic!("expected a lifetime bound, found a trait bound")
}
}
}
}
Expand Down
14 changes: 11 additions & 3 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {

let bx = self;

match coverage.kind {
// Marker statements have no effect during codegen,
// so return early and don't create `func_coverage`.
CoverageKind::SpanMarker => return,
// Match exhaustively to ensure that newly-added kinds are classified correctly.
CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. } => {}
}

let Some(function_coverage_info) =
bx.tcx.instance_mir(instance.def).function_coverage_info.as_deref()
else {
Expand All @@ -100,9 +108,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {

let Coverage { kind } = coverage;
match *kind {
// Span markers are only meaningful during MIR instrumentation,
// and have no effect during codegen.
CoverageKind::SpanMarker => {}
CoverageKind::SpanMarker => unreachable!(
"unexpected marker statement {kind:?} should have caused an early return"
),
CoverageKind::CounterIncrement { id } => {
func_coverage.mark_counter_id_seen(id);
// We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,
Expand Down
24 changes: 14 additions & 10 deletions compiler/rustc_codegen_ssa/src/back/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,13 @@ pub(super) fn get_metadata_xcoff<'a>(path: &Path, data: &'a [u8]) -> Result<&'a
file.symbols().find(|sym| sym.name() == Ok(AIX_METADATA_SYMBOL_NAME))
{
let offset = metadata_symbol.address() as usize;
if offset < 8 {
// The offset specifies the location of rustc metadata in the .info section of XCOFF.
// Each string stored in .info section of XCOFF is preceded by a 4-byte length field.
if offset < 4 {
return Err(format!("Invalid metadata symbol offset: {offset}"));
}
// The offset specifies the location of rustc metadata in the comment section.
// The metadata is preceded by a 8-byte length field.
let len = u64::from_le_bytes(info_data[(offset - 8)..offset].try_into().unwrap()) as usize;
// XCOFF format uses big-endian byte order.
let len = u32::from_be_bytes(info_data[(offset - 4)..offset].try_into().unwrap()) as usize;
if offset + len > (info_data.len() as usize) {
return Err(format!(
"Metadata at offset {offset} with size {len} is beyond .info section"
Expand Down Expand Up @@ -478,9 +479,12 @@ pub fn create_wrapper_file(
file.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text);
file.section_mut(section).flags =
SectionFlags::Xcoff { s_flags: xcoff::STYP_INFO as u32 };

let len = data.len() as u64;
let offset = file.append_section_data(section, &len.to_le_bytes(), 1);
// Encode string stored in .info section of XCOFF.
// FIXME: The length of data here is not guaranteed to fit in a u32.
// We may have to split the data into multiple pieces in order to
// store in .info section.
let len: u32 = data.len().try_into().unwrap();
let offset = file.append_section_data(section, &len.to_be_bytes(), 1);
// Add a symbol referring to the data in .info section.
file.add_symbol(Symbol {
name: AIX_METADATA_SYMBOL_NAME.into(),
Expand Down Expand Up @@ -599,12 +603,12 @@ pub fn create_compressed_metadata_file_for_xcoff(
section: SymbolSection::Section(data_section),
flags: SymbolFlags::None,
});
let len = data.len() as u64;
let offset = file.append_section_data(section, &len.to_le_bytes(), 1);
let len: u32 = data.len().try_into().unwrap();
let offset = file.append_section_data(section, &len.to_be_bytes(), 1);
// Add a symbol referring to the rustc metadata.
file.add_symbol(Symbol {
name: AIX_METADATA_SYMBOL_NAME.into(),
value: offset + 8, // The metadata is preceded by a 8-byte length field.
value: offset + 4, // The metadata is preceded by a 4-byte length field.
size: 0,
kind: SymbolKind::Unknown,
scope: SymbolScope::Dynamic,
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
fn inject_counters(&'a mut self) {
////////////////////////////////////////////////////
// Compute coverage spans from the `CoverageGraph`.
let coverage_spans = CoverageSpans::generate_coverage_spans(
let Some(coverage_spans) = CoverageSpans::generate_coverage_spans(
self.mir_body,
self.fn_sig_span,
self.body_span,
&self.basic_coverage_blocks,
);
) else {
// No relevant spans were found in MIR, so skip instrumenting this function.
return;
};

////////////////////////////////////////////////////
// Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,34 @@ pub(super) struct CoverageSpans {
}

impl CoverageSpans {
/// Extracts coverage-relevant spans from MIR, and associates them with
/// their corresponding BCBs.
///
/// Returns `None` if no coverage-relevant spans could be extracted.
pub(super) fn generate_coverage_spans(
mir_body: &mir::Body<'_>,
fn_sig_span: Span,
body_span: Span,
basic_coverage_blocks: &CoverageGraph,
) -> Self {
) -> Option<Self> {
let coverage_spans = CoverageSpansGenerator::generate_coverage_spans(
mir_body,
fn_sig_span,
body_span,
basic_coverage_blocks,
);

if coverage_spans.is_empty() {
return None;
}

// Group the coverage spans by BCB, with the BCBs in sorted order.
let mut bcb_to_spans = IndexVec::from_elem_n(Vec::new(), basic_coverage_blocks.num_nodes());
for CoverageSpan { bcb, span, .. } in coverage_spans {
bcb_to_spans[bcb].push(span);
}

Self { bcb_to_spans }
Some(Self { bcb_to_spans })
}

pub(super) fn bcb_has_coverage_spans(&self, bcb: BasicCoverageBlock) -> bool {
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_span/src/caching_source_map_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'sm> CachingSourceMapView<'sm> {
self.time_stamp += 1;

// Check if lo and hi are in the cached lines.
let lo_cache_idx = self.cache_entry_index(span_data.lo);
let lo_cache_idx: isize = self.cache_entry_index(span_data.lo);
let hi_cache_idx = self.cache_entry_index(span_data.hi);

if lo_cache_idx != -1 && hi_cache_idx != -1 {
Expand Down Expand Up @@ -205,7 +205,9 @@ impl<'sm> CachingSourceMapView<'sm> {
(lo_cache_idx as usize, oldest)
}
_ => {
panic!();
panic!(
"the case of neither value being equal to -1 was handled above and the function returns."
);
}
};

Expand Down
14 changes: 8 additions & 6 deletions library/std/src/sys/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,16 @@ impl File {
&& unsafe { c::GetLastError() } == c::ERROR_ALREADY_EXISTS
{
unsafe {
// Setting the allocation size to zero also sets the
// EOF position to zero.
let alloc = c::FILE_ALLOCATION_INFO { AllocationSize: 0 };
// This originally used `FileAllocationInfo` instead of
// `FileEndOfFileInfo` but that wasn't supported by WINE.
// It's arguable which fits the semantics of `OpenOptions`
// better so let's just use the more widely supported method.
let eof = c::FILE_END_OF_FILE_INFO { EndOfFile: 0 };
let result = c::SetFileInformationByHandle(
handle.as_raw_handle(),
c::FileAllocationInfo,
ptr::addr_of!(alloc).cast::<c_void>(),
mem::size_of::<c::FILE_ALLOCATION_INFO>() as u32,
c::FileEndOfFileInfo,
ptr::addr_of!(eof).cast::<c_void>(),
mem::size_of::<c::FILE_END_OF_FILE_INFO>() as u32,
);
if result == 0 {
return Err(io::Error::last_os_error());
Expand Down
8 changes: 8 additions & 0 deletions tests/coverage/no_spans_if_not.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Function name: no_spans_if_not::main
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 01, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 11, 1) to (start + 2, 2)

30 changes: 30 additions & 0 deletions tests/coverage/no_spans_if_not.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
LL| |// edition: 2021
LL| |
LL| |// If the span extractor can't find any relevant spans for a function,
LL| |// but the function contains coverage span-marker statements (e.g. inserted
LL| |// for `if !`), coverage codegen may think that it is instrumented and
LL| |// consequently complain that it has no spans.
LL| |//
LL| |// Regression test for <https://github.com/rust-lang/rust/issues/118850>,
LL| |// "A used function should have had coverage mapping data but did not".
LL| |
LL| 1|fn main() {
LL| 1| affected_function();
LL| 1|}
LL| |
LL| |macro_rules! macro_that_defines_a_function {
LL| | (fn $name:ident () $body:tt) => {
LL| | fn $name () $body
LL| | }
LL| |}
LL| |
LL| |macro_that_defines_a_function! {
LL| | fn affected_function() {
LL| | if !false {
LL| | ()
LL| | } else {
LL| | ()
LL| | }
LL| | }
LL| |}

29 changes: 29 additions & 0 deletions tests/coverage/no_spans_if_not.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// edition: 2021

// If the span extractor can't find any relevant spans for a function,
// but the function contains coverage span-marker statements (e.g. inserted
// for `if !`), coverage codegen may think that it is instrumented and
// consequently complain that it has no spans.
//
// Regression test for <https://github.com/rust-lang/rust/issues/118850>,
// "A used function should have had coverage mapping data but did not".

fn main() {
affected_function();
}

macro_rules! macro_that_defines_a_function {
(fn $name:ident () $body:tt) => {
fn $name () $body
}
}

macro_that_defines_a_function! {
fn affected_function() {
if !false {
()
} else {
()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,7 @@ trait Child1 where Self: ~const Trait {} //~ ERROR `~const` is not allowed
// non-const impl
impl<T: ~const Trait> Trait for T {} //~ ERROR `~const` is not allowed

// inherent impl (regression test for issue #117004)
impl<T: ~const Trait> Struct<T> {} //~ ERROR `~const` is not allowed

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,18 @@ note: this impl is not `const`, so it cannot have `~const` trait bounds
LL | impl<T: ~const Trait> Trait for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: `~const` is not allowed here
--> $DIR/tilde-const-invalid-places.rs:56:9
|
LL | impl<T: ~const Trait> Struct<T> {}
| ^^^^^^
|
note: inherent impls cannot have `~const` trait bounds
--> $DIR/tilde-const-invalid-places.rs:56:1
|
LL | impl<T: ~const Trait> Struct<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0658]: generic const items are experimental
--> $DIR/tilde-const-invalid-places.rs:19:15
|
Expand Down Expand Up @@ -239,6 +251,6 @@ LL | type Type<T: ~const Trait> = ();
= note: see issue #8995 <https://github.com/rust-lang/rust/issues/8995> for more information
= help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable

error: aborting due to 26 previous errors
error: aborting due to 27 previous errors

For more information about this error, try `rustc --explain E0658`.
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// known-bug: #110395
// FIXME check-pass
// check-pass
#![feature(const_trait_impl, effects)]

#[const_trait]
Expand All @@ -9,8 +8,8 @@ trait Foo {

struct Bar<T>(T);

impl<T: ~const Foo> Bar<T> {
const fn foo(&self) {
impl<T> Bar<T> {
const fn foo(&self) where T: ~const Foo {
self.0.foo()
}
}
Expand Down
Loading

0 comments on commit e004adb

Please sign in to comment.