fix: warn unused type variables

This commit is contained in:
Shunsuke Shibayama 2023-08-02 15:07:31 +09:00
parent 9734028b21
commit f298db96d1
13 changed files with 307 additions and 51 deletions

View file

@ -1663,7 +1663,7 @@ impl Context {
} else {
let tv = Type::FreeVar(new_fv);
let varname = VarName::from_str(name.clone());
tv_cache.push_or_init_tyvar(&varname, &tv, self);
tv_cache.dummy_push_or_init_tyvar(&varname, &tv, self);
tv
}
}
@ -1689,7 +1689,7 @@ impl Context {
} else {
let tp = TyParam::FreeVar(new_fv);
let varname = VarName::from_str(name.clone());
tv_cache.push_or_init_typaram(&varname, &tp, self);
tv_cache.dummy_push_or_init_typaram(&varname, &tp, self);
tp
}
}

View file

@ -101,6 +101,11 @@ impl Context {
}
None
})
.or_else(|| {
self.tv_cache
.as_ref()
.and_then(|tv_cache| tv_cache.var_infos.get(name))
})
}
pub(crate) fn get_mut_current_scope_var(&mut self, name: &VarName) -> Option<&mut VarInfo> {
@ -3045,20 +3050,23 @@ impl Context {
}
}
pub(crate) fn get_tp_from_tv_cache(
&self,
pub(crate) fn get_tp_from_tv_cache<'v>(
&'v self,
name: &str,
tmp_tv_cache: &TyVarCache,
) -> Option<TyParam> {
tmp_tv_cache: &'v TyVarCache,
) -> Option<(TyParam, &'v VarInfo)> {
if let Some(tp) = tmp_tv_cache.get_typaram(name) {
Some(tp.clone())
Some((tp.clone(), &tmp_tv_cache.var_infos[name]))
} else if let Some(t) = tmp_tv_cache.get_tyvar(name) {
Some(TyParam::t(t.clone()))
Some((TyParam::t(t.clone()), &tmp_tv_cache.var_infos[name]))
} else if let Some(tv_ctx) = &self.tv_cache {
if let Some(t) = tv_ctx.get_tyvar(name) {
Some(TyParam::t(t.clone()))
Some((TyParam::t(t.clone()), &tv_ctx.var_infos[name]))
} else {
tv_ctx.get_typaram(name).cloned()
tv_ctx
.get_typaram(name)
.cloned()
.map(|tp| (tp, &tv_ctx.var_infos[name]))
}
} else {
None

View file

@ -19,7 +19,7 @@ use crate::ty::{HasType, Predicate, Type};
use crate::{type_feature_error, unreachable_error};
use Type::*;
use crate::context::Context;
use crate::context::{Context, VarInfo};
use crate::error::{TyCheckError, TyCheckErrors, TyCheckResult};
use crate::hir;
@ -36,6 +36,7 @@ pub struct TyVarCache {
pub(crate) already_appeared: Set<Str>,
pub(crate) tyvar_instances: Dict<VarName, Type>,
pub(crate) typaram_instances: Dict<VarName, TyParam>,
pub(crate) var_infos: Dict<VarName, VarInfo>,
pub(crate) structural_inner: bool,
}
@ -43,8 +44,8 @@ impl fmt::Display for TyVarCache {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"TyVarInstContext {{ tyvar_instances: {}, typaram_instances: {} }}",
self.tyvar_instances, self.typaram_instances,
"TyVarContext {{ tyvar_instances: {}, typaram_instances: {}, var_infos: {} }}",
self.tyvar_instances, self.typaram_instances, self.var_infos
)
}
}
@ -56,6 +57,7 @@ impl TyVarCache {
already_appeared: Set::new(),
tyvar_instances: Dict::new(),
typaram_instances: Dict::new(),
var_infos: Dict::new(),
structural_inner: false,
}
}
@ -90,6 +92,25 @@ impl TyVarCache {
}
}
/// Warn when a type does not need to be a type variable, such as `|T| T -> Int` (it should be `Obj -> Int`).
///
/// TODO: This warning is currently disabled because it raises a false warning in cases like `|T|(x: T) -> (y: T) -> (x, y)`.
pub fn warn_isolated_vars(&self, ctx: &Context) {
for (name, vi) in self.var_infos.iter() {
let refs = &ctx.index().get_refs(&vi.def_loc).unwrap().referrers;
if refs.len() == 1 {
let warn = TyCheckError::unnecessary_tyvar_warning(
ctx.cfg.input.clone(),
line!() as usize,
vi.def_loc.loc,
name.inspect(),
ctx.caused_by(),
);
ctx.shared().warns.push(warn);
}
}
}
fn instantiate_constraint(
&mut self,
constr: Constraint,
@ -134,6 +155,25 @@ impl TyVarCache {
}
pub(crate) fn push_or_init_tyvar(&mut self, name: &VarName, tv: &Type, ctx: &Context) {
if let Some(inst) = self.tyvar_instances.get(name) {
self.update_tyvar(inst, tv, ctx);
} else if let Some(inst) = self.typaram_instances.get(name) {
if let Ok(inst) = <&Type>::try_from(inst) {
self.update_tyvar(inst, tv, ctx);
} else if let TyParam::FreeVar(_fv) = inst {
inst.link(&TyParam::t(tv.clone()));
} else {
unreachable!()
}
} else {
self.tyvar_instances.insert(name.clone(), tv.clone());
let vi = VarInfo::type_var(Type::Type, ctx.absolutize(name.loc()), ctx.name.clone());
ctx.index().register(name.inspect().clone(), &vi);
self.var_infos.insert(name.clone(), vi);
}
}
pub(crate) fn dummy_push_or_init_tyvar(&mut self, name: &VarName, tv: &Type, ctx: &Context) {
if let Some(inst) = self.tyvar_instances.get(name) {
self.update_tyvar(inst, tv, ctx);
} else if let Some(inst) = self.typaram_instances.get(name) {
@ -176,6 +216,30 @@ impl TyVarCache {
}
pub(crate) fn push_or_init_typaram(&mut self, name: &VarName, tp: &TyParam, ctx: &Context) {
// FIXME:
if let Some(inst) = self.typaram_instances.get(name) {
self.update_typaram(inst, tp, ctx);
} else if let Some(inst) = self.tyvar_instances.get(name) {
if let Ok(tv) = <&Type>::try_from(tp) {
self.update_tyvar(inst, tv, ctx);
} else {
unreachable!()
}
} else {
let t = ctx.get_tp_t(tp).unwrap_or(Type::Obj);
let vi = VarInfo::type_var(t, ctx.absolutize(name.loc()), ctx.name.clone());
ctx.index().register(name.inspect().clone(), &vi);
self.var_infos.insert(name.clone(), vi);
self.typaram_instances.insert(name.clone(), tp.clone());
}
}
pub(crate) fn dummy_push_or_init_typaram(
&mut self,
name: &VarName,
tp: &TyParam,
ctx: &Context,
) {
// FIXME:
if let Some(inst) = self.typaram_instances.get(name) {
self.update_typaram(inst, tp, ctx);
@ -270,7 +334,7 @@ impl Context {
if tmp_tv_cache.appeared(&name) {
let tp =
TyParam::named_free_var(name.clone(), self.level, Constraint::Uninited);
tmp_tv_cache.push_or_init_typaram(&varname, &tp, self);
tmp_tv_cache.dummy_push_or_init_typaram(&varname, &tp, self);
return Ok(tp);
}
if let Some(tv_cache) = &self.tv_cache {
@ -283,7 +347,7 @@ impl Context {
tmp_tv_cache.push_appeared(name.clone());
let constr = tmp_tv_cache.instantiate_constraint(constr, self, loc)?;
let tp = TyParam::named_free_var(name.clone(), self.level, constr);
tmp_tv_cache.push_or_init_typaram(&varname, &tp, self);
tmp_tv_cache.dummy_push_or_init_typaram(&varname, &tp, self);
Ok(tp)
}
}
@ -426,7 +490,7 @@ impl Context {
let varname = VarName::from_str(name.clone());
if tmp_tv_cache.appeared(&name) {
let tyvar = named_free_var(name.clone(), self.level, Constraint::Uninited);
tmp_tv_cache.push_or_init_tyvar(&varname, &tyvar, self);
tmp_tv_cache.dummy_push_or_init_tyvar(&varname, &tyvar, self);
return Ok(tyvar);
}
if let Some(tv_ctx) = &self.tv_cache {
@ -447,7 +511,7 @@ impl Context {
tmp_tv_cache.push_appeared(name.clone());
let constr = tmp_tv_cache.instantiate_constraint(constr, self, loc)?;
let tyvar = named_free_var(name.clone(), self.level, constr);
tmp_tv_cache.push_or_init_tyvar(&varname, &tyvar, self);
tmp_tv_cache.dummy_push_or_init_tyvar(&varname, &tyvar, self);
Ok(tyvar)
}
}

View file

@ -328,6 +328,7 @@ impl Context {
};
free_var(level, Constraint::new_type_of(Type))
};
// tmp_tv_cache.warn_isolated_vars(self);
let typ = if sig.ident.is_procedural() {
proc(non_defaults, var_args, defaults, spec_return_t)
} else {
@ -415,7 +416,7 @@ impl Context {
.and_then(|name| self.get_const_local(name.token(), &self.name).ok())
{
return Ok(ParamTy::Pos(v_enum(set! { value })));
} else if let Some(tp) = sig
} else if let Some((tp, _vi)) = sig
.name()
.and_then(|name| self.get_tp_from_tv_cache(name.inspect(), tmp_tv_cache))
{
@ -453,7 +454,7 @@ impl Context {
tmp_tv_cache: &mut TyVarCache,
not_found_is_qvar: bool,
) -> TyCheckResult<Type> {
self.inc_ref_predecl_typespec(predecl, self);
self.inc_ref_predecl_typespec(predecl, self, tmp_tv_cache);
match predecl {
ast::PreDeclTypeSpec::Mono(simple) => {
self.instantiate_mono_t(simple, opt_decl_t, tmp_tv_cache, not_found_is_qvar)
@ -537,7 +538,9 @@ impl Context {
ident.inspect(),
))),
other => {
if let Some(TyParam::Type(t)) = self.get_tp_from_tv_cache(other, tmp_tv_cache) {
if let Some((TyParam::Type(t), vi)) = self.get_tp_from_tv_cache(other, tmp_tv_cache)
{
self.inc_ref(ident.inspect(), vi, ident, self);
return Ok(*t);
}
if let Some(outer) = &self.outer {
@ -744,7 +747,7 @@ impl Context {
tmp_tv_cache: &mut TyVarCache,
not_found_is_qvar: bool,
) -> TyCheckResult<TyParam> {
self.inc_ref_acc(&acc.clone().downgrade(), self);
self.inc_ref_acc(&acc.clone().downgrade(), self, tmp_tv_cache);
match acc {
ast::ConstAccessor::Attr(attr) => {
let obj = self.instantiate_const_expr(
@ -794,7 +797,7 @@ impl Context {
};
return Ok(TyParam::erased(t));
}
if let Some(tp) = self.get_tp_from_tv_cache(name.inspect(), tmp_tv_cache) {
if let Some((tp, _vi)) = self.get_tp_from_tv_cache(name.inspect(), tmp_tv_cache) {
return Ok(tp);
}
if let Some(value) = self.rec_get_const_obj(name.inspect()) {
@ -1201,13 +1204,26 @@ impl Context {
default_t: Option<&TypeSpec>,
tmp_tv_cache: &mut TyVarCache,
mode: RegistrationMode,
not_found_is_qvar: bool,
) -> TyCheckResult<ParamTy> {
let t = self.instantiate_typespec_full(&p.ty, opt_decl_t, tmp_tv_cache, mode, false)?;
let t = self.instantiate_typespec_full(
&p.ty,
opt_decl_t,
tmp_tv_cache,
mode,
not_found_is_qvar,
)?;
if let Some(default_t) = default_t {
Ok(ParamTy::kw_default(
p.name.as_ref().unwrap().inspect().to_owned(),
t,
self.instantiate_typespec_full(default_t, opt_decl_t, tmp_tv_cache, mode, false)?,
self.instantiate_typespec_full(
default_t,
opt_decl_t,
tmp_tv_cache,
mode,
not_found_is_qvar,
)?,
))
} else {
Ok(ParamTy::pos_or_kw(
@ -1482,13 +1498,27 @@ impl Context {
tmp_tv_cache
};
let non_defaults = try_map_mut(subr.non_defaults.iter(), |p| {
self.instantiate_func_param_spec(p, opt_decl_t, None, tmp_tv_ctx, mode)
self.instantiate_func_param_spec(
p,
opt_decl_t,
None,
tmp_tv_ctx,
mode,
not_found_is_qvar,
)
})?;
let var_params = subr
.var_params
.as_ref()
.map(|p| {
self.instantiate_func_param_spec(p, opt_decl_t, None, tmp_tv_ctx, mode)
self.instantiate_func_param_spec(
p,
opt_decl_t,
None,
tmp_tv_ctx,
mode,
not_found_is_qvar,
)
})
.transpose()?;
let defaults = try_map_mut(subr.defaults.iter(), |p| {
@ -1498,6 +1528,7 @@ impl Context {
Some(&p.default),
tmp_tv_ctx,
mode,
not_found_is_qvar,
)
})?
.into_iter()

View file

@ -2292,21 +2292,29 @@ impl Context {
}
}
pub(crate) fn inc_ref_acc(&self, acc: &ast::Accessor, namespace: &Context) -> bool {
pub(crate) fn inc_ref_acc(
&self,
acc: &ast::Accessor,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
match acc {
ast::Accessor::Ident(ident) => self.inc_ref_local(ident, namespace),
ast::Accessor::Ident(ident) => self.inc_ref_local(ident, namespace, tmp_tv_cache),
ast::Accessor::Attr(attr) => {
self.inc_ref_expr(&attr.obj, namespace);
self.inc_ref_expr(&attr.obj, namespace, tmp_tv_cache);
if let Ok(ctxs) = self.get_singular_ctxs(&attr.obj, self) {
for ctx in ctxs {
if ctx.inc_ref_local(&attr.ident, namespace) {
if ctx.inc_ref_local(&attr.ident, namespace, tmp_tv_cache) {
return true;
}
}
}
false
}
_ => false,
other => {
log!(err "inc_ref_acc: {other}");
false
}
}
}
@ -2314,15 +2322,20 @@ impl Context {
&self,
predecl: &PreDeclTypeSpec,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
match predecl {
PreDeclTypeSpec::Mono(mono) => self.inc_ref_mono_typespec(mono, namespace),
PreDeclTypeSpec::Poly(poly) => self.inc_ref_poly_typespec(poly, namespace),
PreDeclTypeSpec::Mono(mono) => {
self.inc_ref_mono_typespec(mono, namespace, tmp_tv_cache)
}
PreDeclTypeSpec::Poly(poly) => {
self.inc_ref_poly_typespec(poly, namespace, tmp_tv_cache)
}
PreDeclTypeSpec::Attr { namespace: obj, t } => {
self.inc_ref_expr(obj, namespace);
self.inc_ref_expr(obj, namespace, tmp_tv_cache);
if let Ok(ctxs) = self.get_singular_ctxs(obj, self) {
for ctx in ctxs {
if ctx.inc_ref_mono_typespec(t, namespace) {
if ctx.inc_ref_mono_typespec(t, namespace, tmp_tv_cache) {
return true;
}
}
@ -2334,7 +2347,12 @@ impl Context {
}
}
fn inc_ref_mono_typespec(&self, ident: &Identifier, namespace: &Context) -> bool {
fn inc_ref_mono_typespec(
&self,
ident: &Identifier,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
if let Triple::Ok(vi) = self.rec_get_var_info(
ident,
crate::compile::AccessKind::Name,
@ -2343,17 +2361,39 @@ impl Context {
) {
self.inc_ref(ident.inspect(), &vi, &ident.name, namespace);
true
} else if let Some(vi) = tmp_tv_cache.var_infos.get(&ident.name) {
self.inc_ref(ident.inspect(), vi, &ident.name, namespace);
true
} else {
false
}
}
/// TODO: params
fn inc_ref_poly_typespec(&self, poly: &PolyTypeSpec, namespace: &Context) -> bool {
self.inc_ref_acc(&poly.acc.clone().downgrade(), namespace)
fn inc_ref_poly_typespec(
&self,
poly: &PolyTypeSpec,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
for arg in poly.args.pos_args() {
log!(err "{}", arg.expr);
self.inc_ref_expr(&arg.expr.clone().downgrade(), namespace, tmp_tv_cache);
}
if let Some(arg) = poly.args.var_args.as_ref() {
self.inc_ref_expr(&arg.expr.clone().downgrade(), namespace, tmp_tv_cache);
}
for arg in poly.args.kw_args() {
self.inc_ref_expr(&arg.expr.clone().downgrade(), namespace, tmp_tv_cache);
}
self.inc_ref_acc(&poly.acc.clone().downgrade(), namespace, tmp_tv_cache)
}
fn inc_ref_local(&self, local: &ConstIdentifier, namespace: &Context) -> bool {
fn inc_ref_local(
&self,
local: &ConstIdentifier,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
if let Triple::Ok(vi) = self.rec_get_var_info(
local,
crate::compile::AccessKind::Name,
@ -2362,17 +2402,51 @@ impl Context {
) {
self.inc_ref(local.inspect(), &vi, &local.name, namespace);
true
} else if let Some(vi) = tmp_tv_cache.var_infos.get(&local.name) {
self.inc_ref(local.inspect(), vi, &local.name, namespace);
true
} else {
&local.inspect()[..] == "module" || &local.inspect()[..] == "global"
}
}
fn inc_ref_expr(&self, expr: &ast::Expr, namespace: &Context) -> bool {
fn inc_ref_block(
&self,
block: &ast::Block,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
let mut res = false;
for expr in block.iter() {
if self.inc_ref_expr(expr, namespace, tmp_tv_cache) {
res = true;
}
}
res
}
fn inc_ref_expr(
&self,
expr: &ast::Expr,
namespace: &Context,
tmp_tv_cache: &TyVarCache,
) -> bool {
#[allow(clippy::single_match)]
match expr {
ast::Expr::Accessor(acc) => self.inc_ref_acc(acc, namespace),
// TODO:
_ => false,
ast::Expr::Accessor(acc) => self.inc_ref_acc(acc, namespace, tmp_tv_cache),
ast::Expr::Record(ast::Record::Normal(rec)) => {
let mut res = false;
for val in rec.attrs.iter() {
if self.inc_ref_block(&val.body.block, namespace, tmp_tv_cache) {
res = true;
}
}
res
}
other => {
log!(err "inc_ref_expr: {other}");
false
}
}
}
}

View file

@ -799,6 +799,8 @@ impl ASTLowerer {
}
}
}
HIR::new(ast.name, module)
let hir = HIR::new(ast.name, module);
self.lint(&hir, "declare");
hir
}
}

View file

@ -11,7 +11,9 @@ use crate::error::*;
use crate::ty::{ParamTy, Predicate, TyParam, Type};
pub type TyCheckError = CompileError;
pub type TyCheckWarning = CompileWarning;
pub type TyCheckErrors = CompileErrors;
pub type TyCheckWarnings = CompileWarnings;
pub type TyCheckResult<T> = CompileResult<T>;
pub type SingleTyCheckResult<T> = SingleCompileResult<T>;
@ -1346,3 +1348,30 @@ passed keyword args: {kw_args_len}"
)
}
}
impl TyCheckWarning {
pub fn unnecessary_tyvar_warning(
input: Input,
errno: usize,
loc: Location,
name: &str,
caused_by: String,
) -> Self {
Self::new(
ErrorCore::new(
vec![SubMessage::only_loc(loc)],
switch_lang!(
"japanese" => format!("`{name}`は1回しか使われておらず、型変数として宣言する必要がありません"),
"simplified_chinese" => format!("`{name}`只被使用了一次,不需要声明为类型变量"),
"traditional_chinese" => format!("`{name}`只被使用了一次,不需要聲明為類型變量"),
"english" => format!("`{name}` is used only once, so it is not necessary to declare it as a type variable"),
),
errno,
TypeWarning,
loc,
),
input,
caused_by,
)
}
}

View file

@ -2487,6 +2487,13 @@ impl ASTLowerer {
)
}
pub(crate) fn lint(&mut self, hir: &HIR, mode: &str) {
self.warn_implicit_union(hir);
self.warn_unused_expr(&hir.module, mode);
self.check_doc_comments(hir);
self.warn_unused_local_vars(mode);
}
pub fn lower(&mut self, ast: AST, mode: &str) -> Result<CompleteArtifact, IncompleteArtifact> {
log!(info "the AST lowering process has started.");
log!(info "the type-checking process has started.");
@ -2543,10 +2550,7 @@ impl ASTLowerer {
return Err(self.return_incomplete_artifact(hir));
}
};
self.warn_implicit_union(&hir);
self.warn_unused_expr(&hir.module, mode);
self.check_doc_comments(&hir);
self.warn_unused_local_vars(mode);
self.lint(&hir, mode);
if &self.module.context.name[..] == "<module>" || ELS {
if ELS {
self.module.context.shared().promises.join_children();

View file

@ -1,7 +1,7 @@
use erg_common::shared::Shared;
use erg_common::traits::Stream;
use crate::error::CompileErrors;
use crate::error::{CompileError, CompileErrors};
#[derive(Debug, Clone, Default)]
pub struct SharedCompileErrors(Shared<CompileErrors>);
@ -11,6 +11,10 @@ impl SharedCompileErrors {
Self(Shared::new(CompileErrors::empty()))
}
pub fn push(&self, error: CompileError) {
self.0.borrow_mut().push(error);
}
pub fn extend(&self, errors: CompileErrors) {
self.0.borrow_mut().extend(errors);
}

View file

@ -86,6 +86,9 @@ impl ModuleIndex {
}
pub fn register(&mut self, name: Str, vi: &VarInfo) {
if self.members.contains_key(&vi.def_loc) {
return;
}
let referee = vi.def_loc.clone();
let value = ModuleIndexValue::new(name, vi.clone(), set! {});
self.members.insert(referee, value);

View file

@ -348,6 +348,19 @@ impl VarInfo {
)
}
pub fn type_var(t: Type, def_loc: AbsLocation, namespace: Str) -> Self {
Self::new(
t,
Const,
Visibility::private(namespace),
VarKind::Declared,
None,
None,
None,
def_loc,
)
}
pub fn is_untyped_parameter(&self) -> bool {
self.kind.is_parameter() && self.t.is_unbound_var()
}