From a9febfbc4ff5d4076665f5da6d71626872cabd33 Mon Sep 17 00:00:00 2001 From: Calvin Date: Sat, 16 Nov 2024 20:29:14 +0800 Subject: [PATCH] beautify the error message --- rap/Cargo.lock | 77 ++++++++++++------- rap/Cargo.toml | 1 + rap/src/analysis/core/dataflow.rs | 2 +- rap/src/analysis/core/dataflow/graph.rs | 4 +- .../analysis/opt/checking/bounds_checking.rs | 48 +++++++----- rap/src/utils/log.rs | 47 ++++++----- 6 files changed, 110 insertions(+), 69 deletions(-) diff --git a/rap/Cargo.lock b/rap/Cargo.lock index ab0aaa6..183b21b 100644 --- a/rap/Cargo.lock +++ b/rap/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -41,6 +41,22 @@ dependencies = [ "libc", ] +[[package]] +name = "annotate-snippets" +version = "0.11.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24e35ed54e5ea7997c14ed4c70ba043478db1112e98263b3b035907aa197d991" +dependencies = [ + "anstyle", + "unicode-width", +] + +[[package]] +name = "anstyle" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55cc3b69f167a1ef2e161439aa98aed94e6028e5f9a59be9a6ffb47aef1651f9" + [[package]] name = "autocfg" version = "1.1.0" @@ -404,6 +420,7 @@ dependencies = [ name = "rap" version = "1.0.0" dependencies = [ + "annotate-snippets", "cargo_metadata", "chrono", "colorful", @@ -593,6 +610,12 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" +[[package]] +name = "unicode-width" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" + [[package]] name = "wait-timeout" version = "0.2.0" @@ -712,7 +735,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -732,18 +755,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f0713a46559409d202e70e28227288446bf7841d3211583a4b53e3f6d96e7eb" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.5", - "windows_aarch64_msvc 0.52.5", - "windows_i686_gnu 0.52.5", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", "windows_i686_gnullvm", - "windows_i686_msvc 0.52.5", - "windows_x86_64_gnu 0.52.5", - "windows_x86_64_gnullvm 0.52.5", - "windows_x86_64_msvc 0.52.5", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -754,9 +777,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7088eed71e8b8dda258ecc8bac5fb1153c5cffaf2578fc8ff5d61e23578d3263" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" @@ -766,9 +789,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9985fd1504e250c615ca5f281c3f7a6da76213ebd5ccc9561496568a2752afb6" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" @@ -778,15 +801,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88ba073cf16d5372720ec942a8ccbf61626074c6d4dd2e745299726ce8b89670" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" [[package]] name = "windows_i686_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f4261229030a858f36b459e748ae97545d6f1ec60e5e0d6a3d32e0dc232ee9" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" @@ -796,9 +819,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db3c2bf3d13d5b658be73463284eaf12830ac9a26a90c717b7f771dfe97487bf" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" @@ -808,9 +831,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e4246f76bdeff09eb48875a0fd3e2af6aada79d409d33011886d3e1581517d9" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" @@ -820,9 +843,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "852298e482cd67c356ddd9570386e2862b5673c85bd5f88df9ab6802b334c596" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" @@ -832,9 +855,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "z3" diff --git a/rap/Cargo.toml b/rap/Cargo.toml index b1c0981..12a8945 100644 --- a/rap/Cargo.toml +++ b/rap/Cargo.toml @@ -31,6 +31,7 @@ regex = "1.11.1" once_cell = "1.20.1" walkdir = "2" cargo_metadata = "0.18" +annotate-snippets = "0.11.4" [features] backtraces = ["snafu/backtraces", "snafu/backtraces-impl-backtrace-crate"] diff --git a/rap/src/analysis/core/dataflow.rs b/rap/src/analysis/core/dataflow.rs index 1556a58..cce2788 100644 --- a/rap/src/analysis/core/dataflow.rs +++ b/rap/src/analysis/core/dataflow.rs @@ -47,7 +47,7 @@ impl<'tcx> DataFlow<'tcx> { fn build_graph(&self, def_id: DefId) -> Graph { let body: &Body = self.tcx.optimized_mir(def_id); - let mut graph = Graph::new(def_id, body.arg_count, body.local_decls.len()); + let mut graph = Graph::new(def_id, body.span, body.arg_count, body.local_decls.len()); let basic_blocks = &body.basic_blocks; for basic_block_data in basic_blocks.iter() { for statement in basic_block_data.statements.iter() { diff --git a/rap/src/analysis/core/dataflow/graph.rs b/rap/src/analysis/core/dataflow/graph.rs index b96ad31..b81be7c 100644 --- a/rap/src/analysis/core/dataflow/graph.rs +++ b/rap/src/analysis/core/dataflow/graph.rs @@ -81,6 +81,7 @@ pub type GraphNodes = IndexVec; pub type GraphEdges = IndexVec; pub struct Graph { pub def_id: DefId, + pub span: Span, pub argc: usize, pub nodes: GraphNodes, //constsis of locals in mir and newly created markers pub edges: GraphEdges, @@ -88,9 +89,10 @@ pub struct Graph { } impl Graph { - pub fn new(def_id: DefId, argc: usize, n_locals: usize) -> Self { + pub fn new(def_id: DefId, span: Span, argc: usize, n_locals: usize) -> Self { Self { def_id, + span, argc, nodes: GraphNodes::from_elem_n(GraphNode::new(), n_locals), edges: GraphEdges::new(), diff --git a/rap/src/analysis/opt/checking/bounds_checking.rs b/rap/src/analysis/opt/checking/bounds_checking.rs index 35bec4b..0981f10 100644 --- a/rap/src/analysis/opt/checking/bounds_checking.rs +++ b/rap/src/analysis/opt/checking/bounds_checking.rs @@ -7,8 +7,10 @@ use crate::analysis::core::dataflow::graph::{ AggKind, DFSStatus, Direction, Graph, GraphEdge, GraphNode, NodeOp, }; use crate::analysis::utils::def_path::DefPath; -use crate::rap_warn; -use crate::utils::log::underline_span_in_the_line; +use crate::utils::log::{ + relative_pos_range, span_to_filename, span_to_line_number, span_to_source_code, +}; +use annotate_snippets::{Level, Renderer, Snippet}; static DEFPATHS: OnceCell = OnceCell::new(); @@ -117,21 +119,29 @@ pub fn check(graph: &Graph, tcx: &TyCtxt) { } fn report_bug(graph: &Graph, upperbound_node_idx: Local, index_record: &Vec) { - rap_warn!( - "Unnecessary bounds checkings detected in function {:?}. -Index is upperbounded at: -{} -Checked at: -{} -", - graph.def_id, - underline_span_in_the_line(graph.nodes[upperbound_node_idx].span), - index_record - .iter() - .map(|node_idx| { - underline_span_in_the_line(graph.nodes[*node_idx].span) // buggy, what about multiple index in the same line? - }) - .collect::>() - .join("\n") - ); + let upperbound_span = graph.nodes[upperbound_node_idx].span; + let code_source = span_to_source_code(graph.span); + let filename = span_to_filename(upperbound_span); + let mut snippet = Snippet::source(&code_source) + .line_start(span_to_line_number(graph.span)) + .origin(&filename) + .fold(true) + .annotation( + Level::Info + .span(relative_pos_range(graph.span, upperbound_span)) + .label("Index is upperbounded."), + ); + for node_idx in index_record { + let index_span = graph.nodes[*node_idx].span; + snippet = snippet.annotation( + Level::Error + .span(relative_pos_range(graph.span, index_span)) + .label("Checked here."), + ); + } + let message = Level::Warning + .title("Unnecessary bounds checkings detected") + .snippet(snippet); + let renderer = Renderer::styled(); + println!("{}", renderer.render(message)); } diff --git a/rap/src/utils/log.rs b/rap/src/utils/log.rs index 1aedc63..032fa58 100644 --- a/rap/src/utils/log.rs +++ b/rap/src/utils/log.rs @@ -3,7 +3,8 @@ use fern::colors::{Color, ColoredLevelConfig}; use fern::{self, Dispatch}; use log::LevelFilter; use rustc_span::source_map::get_source_map; -use rustc_span::{Pos, Span}; +use rustc_span::{FileNameDisplayPreference, Pos, Span}; +use std::ops::Range; fn log_level() -> LevelFilter { if let Ok(s) = std::env::var("RAP_LOG") { @@ -86,25 +87,29 @@ pub fn rap_error_and_exit(msg: impl AsRef) -> ! { std::process::exit(1) } -pub fn underline_span_in_the_line(span: Span) -> String { - fn compose_underline(line_span: Span, span: Span) -> String { - let line_len = (line_span.hi() - line_span.lo()).to_u32(); - let line_start_pos = line_span.lo(); - let lo = (span.lo() - line_start_pos).to_u32(); - let hi = (span.hi() - line_start_pos).to_u32(); - (0..line_len) - .map(|i| if i >= lo && i < hi { '^' } else { ' ' }) - .collect() - } +#[inline] +pub fn span_to_source_code(span: Span) -> String { + get_source_map().unwrap().span_to_snippet(span).unwrap() +} + +#[inline] +pub fn span_to_filename(span: Span) -> String { + get_source_map() + .unwrap() + .span_to_filename(span) + .display(FileNameDisplayPreference::Local) + .to_string() +} + +#[inline] +pub fn span_to_line_number(span: Span) -> usize { + get_source_map().unwrap().lookup_char_pos(span.lo()).line +} - let source_map = get_source_map().unwrap(); - let line_span = source_map.span_extend_to_line(span); - let line = source_map.span_to_snippet(line_span).unwrap(); - let underline = compose_underline(line_span, span); - format!( - "{}\n{}\n{}", - source_map.span_to_diagnostic_string(span), - line, - underline - ) +#[inline] +pub fn relative_pos_range(span: Span, sub_span: Span) -> Range { + let start_pos = span.lo(); + let lo = (sub_span.lo() - start_pos).to_usize(); + let hi = (sub_span.hi() - start_pos).to_usize(); + lo..hi }