Skip to content

Commit 5e73f6b

Browse files
committed
hy-check(const_init): reject type-parameter shadows on const conversions, ascriptions, and file constants (fixes #3851)
1 parent 0c1a9cd commit 5e73f6b

2 files changed

Lines changed: 92 additions & 12 deletions

File tree

‎compiler/crates/hy-check/src/const_init.rs‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl<'a> Checker<'a> {
3838
.then(|| self.file_const_decl(name)?.ty.as_ref())
3939
.flatten()
4040
});
41-
let has_non_const_ty = effective_ty.is_some_and(|ty| !self.is_const_allowed_ty(ty));
41+
let has_non_const_ty = effective_ty.is_some_and(|ty| !self.is_const_allowed_ty(ty, st));
4242
if !has_non_const_ty && self.expr_is_constant_inner(value, st, &mut visiting, 0) {
4343
return;
4444
}
@@ -92,7 +92,7 @@ impl<'a> Checker<'a> {
9292
let Some(ty_name) = conversion_callee_name(callee) else {
9393
return false;
9494
};
95-
if st.is_some_and(|s| s.env_lookup(ty_name).is_some()) {
95+
if st.is_some_and(|s| s.env_lookup(ty_name).is_some() || s.type_params.contains(ty_name)) {
9696
return false;
9797
}
9898
if self.fns.contains_key(ty_name)
@@ -133,7 +133,7 @@ impl<'a> Checker<'a> {
133133
return self.expr_is_constant_inner(&args[0].value, st, visiting, ref_depth)
134134
&& self.is_const_string_expr(&args[0].value, st, ref_depth);
135135
}
136-
if !self.is_const_conversion_target(ty_name) {
136+
if !self.is_const_conversion_target(ty_name, st) {
137137
return false;
138138
}
139139
self.expr_is_constant_inner(&args[0].value, st, visiting, ref_depth)
@@ -145,11 +145,15 @@ impl<'a> Checker<'a> {
145145
}
146146
}
147147

148-
fn is_const_conversion_target(&self, name: &str) -> bool {
148+
fn is_const_conversion_target(&self, name: &str, st: Option<&FnState>) -> bool {
149+
if st.is_some_and(|s| s.type_params.contains(name)) {
150+
return false;
151+
}
149152
if let Some(ty) = self.type_aliases.get(name) {
150153
let resolved = self.resolve_type_alias(ty);
151154
if let hy_syntax::TypeExpr::Named { name: target, .. } = resolved {
152-
return !self.structs.contains_key(target.as_str())
155+
return !st.is_some_and(|s| s.type_params.contains(target.as_str()))
156+
&& !self.structs.contains_key(target.as_str())
153157
&& !self.struct_type_params.contains_key(target.as_str())
154158
&& !self.enums.contains_key(target.as_str())
155159
&& is_conversion_type_name(&target);
@@ -160,7 +164,7 @@ impl<'a> Checker<'a> {
160164
}
161165

162166
fn is_const_string_conversion_target(&self, name: &str, st: Option<&FnState>) -> bool {
163-
if st.is_some_and(|s| s.env_lookup(name).is_some())
167+
if st.is_some_and(|s| s.env_lookup(name).is_some() || s.type_params.contains(name))
164168
|| self.fns.contains_key(name)
165169
|| self.structs.contains_key(name)
166170
|| self.struct_type_params.contains_key(name)
@@ -175,6 +179,7 @@ impl<'a> Checker<'a> {
175179
resolved,
176180
hy_syntax::TypeExpr::Named { name: target, .. }
177181
if target == "string"
182+
&& !st.is_some_and(|s| s.type_params.contains(target.as_str()))
178183
&& !self.structs.contains_key("string")
179184
&& !self.struct_type_params.contains_key("string")
180185
&& !self.enums.contains_key("string")
@@ -204,9 +209,12 @@ impl<'a> Checker<'a> {
204209
}
205210
Expr::Ident { name, .. } => {
206211
if let Some(st) = st {
212+
if st.type_params.contains(name.as_str()) {
213+
return false;
214+
}
207215
if let Some(b) = st.env_lookup(name) {
208216
if let Some(ty) = &b.ty {
209-
if !self.is_const_allowed_ty(ty)
217+
if !self.is_const_allowed_ty(ty, Some(st))
210218
|| !matches!(self.shape_of_type(ty), TypeShape::Str)
211219
{
212220
return false;
@@ -217,7 +225,7 @@ impl<'a> Checker<'a> {
217225
}
218226
if let Some(cdecl) = self.file_const_decl(name) {
219227
if let Some(ty) = &cdecl.ty {
220-
if !self.is_const_allowed_ty(ty)
228+
if !self.is_const_allowed_ty(ty, None)
221229
|| !matches!(self.shape_of_type(ty), TypeShape::Str)
222230
{
223231
return false;
@@ -247,8 +255,11 @@ impl<'a> Checker<'a> {
247255
})
248256
}
249257

250-
fn is_const_allowed_ty(&self, ty: &hy_syntax::TypeExpr) -> bool {
258+
fn is_const_allowed_ty(&self, ty: &hy_syntax::TypeExpr, st: Option<&FnState>) -> bool {
251259
if let hy_syntax::TypeExpr::Named { ref name, .. } = ty {
260+
if st.is_some_and(|s| s.type_params.contains(name.as_str())) {
261+
return false;
262+
}
252263
if self.structs.contains_key(name.as_str())
253264
|| self.struct_type_params.contains_key(name.as_str())
254265
|| self.enums.contains_key(name.as_str())
@@ -261,6 +272,9 @@ impl<'a> Checker<'a> {
261272
return false;
262273
}
263274
if let hy_syntax::TypeExpr::Named { ref name, .. } = resolved {
275+
if st.is_some_and(|s| s.type_params.contains(name.as_str())) {
276+
return false;
277+
}
264278
if self.structs.contains_key(name.as_str())
265279
|| self.struct_type_params.contains_key(name.as_str())
266280
|| self.enums.contains_key(name.as_str())
@@ -269,6 +283,11 @@ impl<'a> Checker<'a> {
269283
}
270284
}
271285
if matches!(self.shape_of_type(ty), TypeShape::Primitive | TypeShape::Str) {
286+
if let hy_syntax::TypeExpr::Named { ref name, .. } = ty {
287+
if st.is_some_and(|s| s.type_params.contains(name.as_str())) {
288+
return false;
289+
}
290+
}
272291
return true;
273292
}
274293
matches!(
@@ -288,13 +307,16 @@ impl<'a> Checker<'a> {
288307
if ref_depth > 32 {
289308
return false;
290309
}
291-
// Local bindings and parameters shadow file-level constants.
310+
// Local bindings, parameters, and type parameters shadow file-level constants.
292311
if let Some(st) = st {
312+
if st.type_params.contains(name) {
313+
return false;
314+
}
293315
if let Some(b) = st.env_lookup(name) {
294316
// Only scalar/string/complex `const` bindings are Go compile-time
295317
// constants; composite `const`s lower to Go `var`.
296318
if let Some(ty) = &b.ty {
297-
if !self.is_const_allowed_ty(ty) {
319+
if !self.is_const_allowed_ty(ty, Some(st)) {
298320
return false;
299321
}
300322
}
@@ -313,7 +335,7 @@ impl<'a> Checker<'a> {
313335
return false;
314336
}
315337
if let Some(ty) = &cdecl.ty {
316-
if !self.is_const_allowed_ty(ty) {
338+
if !self.is_const_allowed_ty(ty, None) {
317339
return false;
318340
}
319341
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
2+
use hy_check::check_file;
3+
use hy_syntax::parse_file;
4+
5+
fn get_codes(src: &str) -> Vec<String> {
6+
let f = parse_file("test.hy", src).expect("parse ok");
7+
check_file(&f).diagnostics.into_iter().map(|d| d.code).collect()
8+
}
9+
10+
#[test]
11+
fn test_reproducer_3851() {
12+
let src = r#"
13+
type my_int = int
14+
const k = 42
15+
fn f[my_int, k]() {
16+
const c = my_int(1)
17+
const d my_int = 1
18+
const e = k + 1
19+
_ = c
20+
_ = d
21+
_ = e
22+
}
23+
fn main() {}
24+
"#;
25+
assert_eq!(get_codes(src), vec!["HYB-E6003", "HYB-E6003", "HYB-E6003"]);
26+
}
27+
28+
#[test]
29+
fn test_string_len_type_param_shadow() {
30+
let src_shadowed = r#"
31+
const s = "abc"
32+
fn f[s]() {
33+
const n = len(s)
34+
_ = n
35+
}
36+
fn main() {}
37+
"#;
38+
let codes = get_codes(src_shadowed);
39+
assert!(codes.contains(&"HYB-E6003".to_string()));
40+
41+
let src_unshadowed = r#"
42+
type my_int = int
43+
const k = 42
44+
const s = "abc"
45+
fn g() {
46+
const c = my_int(1)
47+
const d my_int = 1
48+
const e = k + 1
49+
const n = len(s)
50+
_ = c
51+
_ = d
52+
_ = e
53+
_ = n
54+
}
55+
fn main() {}
56+
"#;
57+
assert!(get_codes(src_unshadowed).is_empty());
58+
}

0 commit comments

Comments
 (0)