[tint][wgsl] Improve 'cannot take address of' diagnostic
Also rename:
`value expression of type 'X' `
to
`value of type 'X' `
As 'expression' is superfluous.
Fixed: tint:2159
Change-Id: Ic32b3af171a1b637f9e16b6f484accdf6fc48b92
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/172863
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
diff --git a/src/tint/lang/wgsl/resolver/assignment_validation_test.cc b/src/tint/lang/wgsl/resolver/assignment_validation_test.cc
index aeb4d61..826fbd0 100644
--- a/src/tint/lang/wgsl/resolver/assignment_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/assignment_validation_test.cc
@@ -222,7 +222,7 @@
Assign(Expr(Source{{12, 34}}, 1_i), "my_var"));
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value expression of type 'i32')");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value of type 'i32')");
}
TEST_F(ResolverAssignmentValidationTest, AssignToOverride_Fail) {
@@ -292,7 +292,7 @@
Assign(MemberAccessor(Source{{12, 34}}, Expr(Source{{56, 78}}, "a"), "i"), 2_i));
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value expression of type 'i32'
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value of type 'i32'
56:78 note: 'let' variables are immutable
98:76 note: let 'a' declared here)");
}
diff --git a/src/tint/lang/wgsl/resolver/call_validation_test.cc b/src/tint/lang/wgsl/resolver/call_validation_test.cc
index 9a0d7c1..d7bdc4e 100644
--- a/src/tint/lang/wgsl/resolver/call_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/call_validation_test.cc
@@ -143,7 +143,7 @@
});
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression");
+ EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of let 'z'");
}
TEST_F(ResolverCallValidationTest,
@@ -214,7 +214,7 @@
});
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression");
+ EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of value of type 'i32'");
}
TEST_F(ResolverCallValidationTest, PointerArgument_FunctionParam) {
diff --git a/src/tint/lang/wgsl/resolver/compound_assignment_validation_test.cc b/src/tint/lang/wgsl/resolver/compound_assignment_validation_test.cc
index ada0fb4..578cd22 100644
--- a/src/tint/lang/wgsl/resolver/compound_assignment_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/compound_assignment_validation_test.cc
@@ -276,7 +276,7 @@
WrapInFunction(CompoundAssign(Expr(Source{{56, 78}}, 1_i), 1_i, core::BinaryOp::kAdd));
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to value expression of type 'i32')");
+ EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to value of type 'i32')");
}
TEST_F(ResolverCompoundAssignmentValidationTest, LhsAtomic) {
diff --git a/src/tint/lang/wgsl/resolver/ptr_ref_validation_test.cc b/src/tint/lang/wgsl/resolver/ptr_ref_validation_test.cc
index cc61eb3..488b5c6 100644
--- a/src/tint/lang/wgsl/resolver/ptr_ref_validation_test.cc
+++ b/src/tint/lang/wgsl/resolver/ptr_ref_validation_test.cc
@@ -49,7 +49,7 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of value of type 'i32')");
}
TEST_F(ResolverPtrRefValidationTest, AddressOfLet) {
@@ -62,7 +62,45 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of let 'l')");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfConst) {
+ // const c : i32 = 1;
+ // &c
+ auto* l = Const("c", ty.i32(), Expr(1_i));
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "c"));
+
+ WrapInFunction(l, expr);
+
+ EXPECT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of const 'c')");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfOverride) {
+ // override c : i32;
+ // &o
+ Override("o", ty.i32());
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "o"));
+
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of override 'o')");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfParameter) {
+ // fn F(p : i32) { _ = &p }
+ // &F
+ Func("F", Vector{Param("p", ty.i32())}, ty.void_(),
+ Vector{
+ Assign(Phony(), AddressOf(Expr(Source{{12, 34}}, "p"))),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of parameter 'p')");
}
TEST_F(ResolverPtrRefValidationTest, AddressOfHandle) {
@@ -75,8 +113,111 @@
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
- "12:34 error: cannot take the address of expression in handle "
- "address space");
+ R"(12:34 error: cannot take the address of var 't' in handle address space)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfFunction) {
+ // fn F() {}
+ // &F
+ Func("F", Empty, ty.void_(), Empty);
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "F"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(12:34 error: cannot use function 'F' as value
+note: function 'F' declared here
+12:34 note: are you missing '()'?)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfBuiltinFunction) {
+ // &max
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "max"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(12:34 error: cannot use builtin function 'max' as value
+12:34 note: are you missing '()'?)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfType) {
+ // &i32
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "i32"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(12:34 error: cannot use type 'i32' as value
+12:34 note: are you missing '()'?)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfTypeAlias) {
+ // alias T = i32
+ // &T
+ Alias("T", ty.i32());
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "T"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(12:34 error: cannot use type 'i32' as value
+12:34 note: are you missing '()'?)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfAccess) {
+ // &read_write
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "read_write"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot use access 'read_write' as value)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfAddressSpace) {
+ // &handle
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "uniform"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot use address space 'uniform' as value)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfBuiltinValue) {
+ // &position
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "position"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot use builtin value 'position' as value)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfInterpolationSampling) {
+ // ¢roid
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "centroid"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(12:34 error: cannot use interpolation sampling 'centroid' as value)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfInterpolationType) {
+ // &perspective
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "perspective"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot use interpolation type 'perspective' as value)");
+}
+
+TEST_F(ResolverPtrRefValidationTest, AddressOfTexelFormat) {
+ // &rgba8snorm
+ auto* expr = AddressOf(Expr(Source{{12, 34}}, "rgba8snorm"));
+ WrapInFunction(expr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot use texel format 'rgba8snorm' as value)");
}
TEST_F(ResolverPtrRefValidationTest, AddressOfVectorComponent_MemberAccessor) {
@@ -89,7 +230,7 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of a vector component");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of a vector component)");
}
TEST_F(ResolverPtrRefValidationTest, AddressOfVectorComponent_IndexAccessor) {
@@ -102,7 +243,7 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of a vector component");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot take the address of a vector component)");
}
TEST_F(ResolverPtrRefValidationTest, IndirectOfAddressOfHandle) {
@@ -115,8 +256,7 @@
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
- "12:34 error: cannot take the address of expression in handle "
- "address space");
+ R"(12:34 error: cannot take the address of var 't' in handle address space)");
}
TEST_F(ResolverPtrRefValidationTest, DerefOfLiteral) {
@@ -128,7 +268,7 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot dereference expression of type 'i32'");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot dereference expression of type 'i32')");
}
TEST_F(ResolverPtrRefValidationTest, DerefOfVar) {
@@ -141,7 +281,7 @@
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: cannot dereference expression of type 'i32'");
+ EXPECT_EQ(r()->error(), R"(12:34 error: cannot dereference expression of type 'i32')");
}
TEST_F(ResolverPtrRefValidationTest, InferredPtrAccessMismatch) {
diff --git a/src/tint/lang/wgsl/resolver/resolver.cc b/src/tint/lang/wgsl/resolver/resolver.cc
index 16c25ae..3e717d4 100644
--- a/src/tint/lang/wgsl/resolver/resolver.cc
+++ b/src/tint/lang/wgsl/resolver/resolver.cc
@@ -3694,7 +3694,8 @@
case core::UnaryOp::kAddressOf:
if (auto* ref = expr_ty->As<core::type::Reference>()) {
if (ref->StoreType()->UnwrapRef()->is_handle()) {
- AddError("cannot take the address of expression in handle address space",
+ AddError("cannot take the address of " + sem_.Describe(expr) +
+ " in handle address space",
unary->expr->source);
return nullptr;
}
@@ -3713,7 +3714,7 @@
root_ident = expr->RootIdentifier();
} else {
- AddError("cannot take the address of expression", unary->expr->source);
+ AddError("cannot take the address of " + sem_.Describe(expr), unary->expr->source);
return nullptr;
}
break;
diff --git a/src/tint/lang/wgsl/resolver/resolver_test.cc b/src/tint/lang/wgsl/resolver/resolver_test.cc
index 8eb0457..fe4027e 100644
--- a/src/tint/lang/wgsl/resolver/resolver_test.cc
+++ b/src/tint/lang/wgsl/resolver/resolver_test.cc
@@ -138,7 +138,7 @@
WrapInFunction(cond_var, Switch("i", Case(CaseSelector(AddressOf(1_a)), Block())));
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "error: cannot take the address of expression");
+ EXPECT_EQ(r()->error(), "error: cannot take the address of value of type 'abstract-int'");
}
TEST_F(ResolverTest, Stmt_Block) {
@@ -2411,7 +2411,7 @@
});
ASSERT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "error: cannot take the address of expression in handle address space");
+ EXPECT_EQ(r()->error(), "error: cannot take the address of var 's' in handle address space");
}
TEST_F(ResolverTest, ModuleDependencyOrderedDeclarations) {
diff --git a/src/tint/lang/wgsl/resolver/sem_helper.cc b/src/tint/lang/wgsl/resolver/sem_helper.cc
index a68734a..d6e8b89 100644
--- a/src/tint/lang/wgsl/resolver/sem_helper.cc
+++ b/src/tint/lang/wgsl/resolver/sem_helper.cc
@@ -94,7 +94,7 @@
},
[&](const sem::ValueExpression* val_expr) {
auto type = val_expr->Type()->FriendlyName();
- return "value expression of type '" + type + "'";
+ return "value of type '" + type + "'";
},
[&](const sem::TypeExpression* ty_expr) {
auto name = ty_expr->Type()->FriendlyName();
