Skip to content

Commit

Permalink
[move-compiler] Fix panic in coverage for inlined code, add controls …
Browse files Browse the repository at this point in the history
…to display tags and/or color in coverage source listings, don't lean on compiler v1 (#14449)

- Add controls --color and --tag for showing coverage.  Fix panic by dropping inlined code from source coverage listings.
- fix aptos move test --coverage to not necessarily use compiler-v1

Fixes #14377
Fixes #14360
Fixes #14379
Fixes #14454
  • Loading branch information
brmataptos authored Aug 29, 2024
1 parent d231082 commit 5eb0f1d
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 25 deletions.
41 changes: 33 additions & 8 deletions crates/aptos/src/move_tool/coverage.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::common::types::{CliCommand, CliError, CliResult, CliTypedResult, MovePackageDir};
use crate::{
common::types::{CliCommand, CliError, CliResult, CliTypedResult, MovePackageDir},
move_tool::{experiments_from_opt_level, fix_bytecode_version},
};
use aptos_framework::extended_checks;
use async_trait::async_trait;
use clap::{Parser, Subcommand};
use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
use move_coverage::{
coverage_map::CoverageMap, format_csv_summary, format_human_summary,
source_coverage::SourceCoverageBuilder, summary::summarize_inst_cov,
coverage_map::CoverageMap,
format_csv_summary, format_human_summary,
source_coverage::{ColorChoice, SourceCoverageBuilder, TextIndicator},
summary::summarize_inst_cov,
};
use move_disassembler::disassembler::Disassembler;
use move_package::{compilation::compiled_package::CompiledPackage, BuildConfig, CompilerConfig};
Expand Down Expand Up @@ -87,8 +92,18 @@ impl CliCommand<()> for SummaryCoverage {
/// Display coverage information about the module against source code
#[derive(Debug, Parser)]
pub struct SourceCoverage {
/// Show coverage for the given module
#[clap(long = "module")]
pub module_name: String,

/// Colorize output based on coverage
#[clap(long, default_value_t = ColorChoice::Default)]
pub color: ColorChoice,

/// Tag each line with a textual indication of coverage
#[clap(long, default_value_t = TextIndicator::Explicit)]
pub tag: TextIndicator,

#[clap(flatten)]
pub move_options: MovePackageDir,
}
Expand All @@ -110,9 +125,10 @@ impl CliCommand<()> for SourceCoverage {
_ => panic!("Should all be modules"),
};
let source_coverage = SourceCoverageBuilder::new(module, &coverage_map, source_map);
source_coverage
.compute_source_coverage(source_path)
.output_source_coverage(&mut std::io::stdout())
let source_coverage = source_coverage.compute_source_coverage(source_path);
let output_result =
source_coverage.output_source_coverage(&mut std::io::stdout(), self.color, self.tag);
output_result
.map_err(|err| CliError::UnexpectedError(format!("Failed to get coverage {}", err)))
}
}
Expand Down Expand Up @@ -149,14 +165,23 @@ fn compile_coverage(
dev_mode: move_options.dev,
additional_named_addresses: move_options.named_addresses(),
test_mode: false,
full_model_generation: move_options.check_test_code,
install_dir: move_options.output_dir.clone(),
skip_fetch_latest_git_deps: move_options.skip_fetch_latest_git_deps,
compiler_config: CompilerConfig {
known_attributes: extended_checks::get_all_attribute_names().clone(),
skip_attribute_checks: false,
..Default::default()
skip_attribute_checks: move_options.skip_attribute_checks,
bytecode_version: fix_bytecode_version(
move_options.bytecode_version,
move_options.language_version,
),
compiler_version: move_options.compiler_version,
language_version: move_options.language_version,
experiments: experiments_from_opt_level(&move_options.optimize),
},
..Default::default()
};

let path = move_options.get_package_path()?;
let coverage_map =
CoverageMap::from_binary_file(path.join(".coverage_map.mvcov")).map_err(|err| {
Expand Down
4 changes: 2 additions & 2 deletions crates/aptos/src/move_tool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ pub struct TestPackage {
pub dump_state: bool,
}

fn fix_bytecode_version(
pub(crate) fn fix_bytecode_version(
bytecode_version_in: Option<u32>,
language_version: Option<LanguageVersion>,
) -> Option<u32> {
Expand Down Expand Up @@ -814,7 +814,7 @@ impl FromStr for IncludedArtifacts {
}
}

fn experiments_from_opt_level(optlevel: &Option<OptimizationLevel>) -> Vec<String> {
pub(crate) fn experiments_from_opt_level(optlevel: &Option<OptimizationLevel>) -> Vec<String> {
match optlevel {
None | Some(OptimizationLevel::Default) => {
vec![format!("{}=on", Experiment::OPTIMIZE.to_string())]
Expand Down
21 changes: 16 additions & 5 deletions third_party/move/tools/move-cli/src/base/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ use super::reroot_path;
use clap::*;
use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
use move_coverage::{
coverage_map::CoverageMap, format_csv_summary, format_human_summary,
source_coverage::SourceCoverageBuilder, summary::summarize_inst_cov,
coverage_map::CoverageMap,
format_csv_summary, format_human_summary,
source_coverage::{ColorChoice, SourceCoverageBuilder, TextIndicator},
summary::summarize_inst_cov,
};
use move_disassembler::disassembler::Disassembler;
use move_package::BuildConfig;
Expand Down Expand Up @@ -45,6 +47,14 @@ pub enum CoverageSummaryOptions {
pub struct Coverage {
#[clap(subcommand)]
pub options: CoverageSummaryOptions,

/// Colorize output based on coverage
#[clap(long, default_value_t = ColorChoice::Default)]
pub color: ColorChoice,

/// Tag each line with a textual indication of coverage
#[clap(long, default_value_t = TextIndicator::Explicit)]
pub tag: TextIndicator,
}

impl Coverage {
Expand All @@ -69,10 +79,11 @@ impl Coverage {
}) => (module, source_map),
_ => panic!("Should all be modules"),
};
let source_coverage = SourceCoverageBuilder::new(module, &coverage_map, source_map);
let source_coverage_builder =
SourceCoverageBuilder::new(module, &coverage_map, source_map);
let source_coverage = source_coverage_builder.compute_source_coverage(source_path);
source_coverage
.compute_source_coverage(source_path)
.output_source_coverage(&mut std::io::stdout())
.output_source_coverage(&mut std::io::stdout(), self.color, self.tag)
.unwrap();
},
CoverageSummaryOptions::Summary {
Expand Down
17 changes: 13 additions & 4 deletions third_party/move/tools/move-coverage/src/bin/source-coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use clap::Parser;
use move_binary_format::file_format::CompiledModule;
use move_bytecode_source_map::utils::source_map_from_file;
use move_command_line_common::files::SOURCE_MAP_EXTENSION;
use move_coverage::{coverage_map::CoverageMap, source_coverage::SourceCoverageBuilder};
use move_coverage::{
coverage_map::CoverageMap,
source_coverage::{ColorChoice, SourceCoverageBuilder, TextIndicator},
};
use std::{
fs,
fs::File,
Expand Down Expand Up @@ -39,6 +42,12 @@ struct Args {
/// Optional path to save coverage. Printed to stdout if not present.
#[clap(long = "coverage-path", short = 'o')]
pub coverage_path: Option<String>,
/// Colorize output based on coverage
#[clap(long, default_value_t = ColorChoice::Default)]
pub color: ColorChoice,
/// Tag each line with a textual indication of coverage
#[clap(long, default_value_t = TextIndicator::Explicit)]
pub tag: TextIndicator,
}

fn main() {
Expand Down Expand Up @@ -69,9 +78,9 @@ fn main() {
None => Box::new(io::stdout()),
};

source_cov
.compute_source_coverage(source_path)
.output_source_coverage(&mut coverage_writer)
let source_coverage = source_cov.compute_source_coverage(source_path);
source_coverage
.output_source_coverage(&mut coverage_writer, args.color, args.tag)
.unwrap();
}

Expand Down
Loading

0 comments on commit 5eb0f1d

Please sign in to comment.