Skip to content

Commit 31f70f1

Browse files
AndrewNolteAndrewNolte
committed
Structured Assigns: better diags
- No more generic 'missing fields' - elaborate which fields - For arrays, specify the type of the array so the user can see which indexes to add, or which type to add - Highlight only first part of struct assign so that the diag doesn't obscure more important diags like syntax errors. stack-info: PR: #1927, branch: AndrewNolte/stack/27
1 parent fa4a4f2 commit 31f70f1

3 files changed

Lines changed: 41 additions & 25 deletions

File tree

‎scripts/diagnostics.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,8 @@ error AssignmentPatternKeyDupDefault "assignment pattern has multiple default ke
642642
error AssignmentPatternKeyDupValue "assignment pattern has multiple keys for index {}"
643643
error AssignmentPatternKeyDupName "assignment pattern has multiple keys for member '{}'"
644644
error AssignmentPatternNoMember "member '{}' is not covered by any assignment pattern key"
645-
error AssignmentPatternMissingElements "not all elements of array are covered by an assignment pattern key"
645+
error AssignmentPatternMissingElements "assignment pattern does not cover all elements of type {}"
646+
error AssignmentPatternMissingDynamicElements "assignment pattern does not cover all elements in an array of length {} of type {}"
646647
error AssignmentPatternDynamicType "assignment patterns for dynamic arrays, associative arrays, and queues cannot have type keys"
647648
error AssignmentPatternAssociativeType "assignment pattern for associative array must specify key:value pairs"
648649
error EmptyArgNotAllowed "empty argument not allowed"

‎source/ast/expressions/AssignmentExpressions.cpp‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,7 +1170,7 @@ ConstantValue SimpleAssignmentPatternExpression::applyConversions(EvalContext& c
11701170

11711171
static const Expression* matchElementValue(
11721172
const ASTContext& context, const Type& elementType, const FieldSymbol* targetField,
1173-
SourceRange sourceRange,
1173+
const Type& assignmentTargetType, const StructuredAssignmentPatternSyntax& syntax,
11741174
std::span<const StructuredAssignmentPatternExpression::TypeSetter> typeSetters,
11751175
const Expression* defaultSetter) {
11761176

@@ -1227,8 +1227,8 @@ static const Expression* matchElementValue(
12271227
if (type.isError() || field.name.empty())
12281228
return nullptr;
12291229

1230-
auto elemExpr = matchElementValue(context, type, &field, sourceRange, typeSetters,
1231-
defaultSetter);
1230+
auto elemExpr = matchElementValue(context, type, &field, assignmentTargetType, syntax,
1231+
typeSetters, defaultSetter);
12321232
if (!elemExpr)
12331233
return nullptr;
12341234

@@ -1237,15 +1237,16 @@ static const Expression* matchElementValue(
12371237

12381238
auto& comp = context.getCompilation();
12391239
return comp.emplace<SimpleAssignmentPatternExpression>(elementType, /* isLValue */ false,
1240-
elements.copy(comp), sourceRange);
1240+
elements.copy(comp),
1241+
syntax.sourceRange());
12411242
}
12421243

12431244
if (elementType.isArray() && elementType.hasFixedRange()) {
12441245
auto nestedElemType = elementType.getArrayElementType();
12451246
SLANG_ASSERT(nestedElemType);
12461247

1247-
auto elemExpr = matchElementValue(context, *nestedElemType, nullptr, sourceRange,
1248-
typeSetters, defaultSetter);
1248+
auto elemExpr = matchElementValue(context, *nestedElemType, targetField,
1249+
assignmentTargetType, syntax, typeSetters, defaultSetter);
12491250
if (!elemExpr)
12501251
return nullptr;
12511252

@@ -1256,7 +1257,8 @@ static const Expression* matchElementValue(
12561257

12571258
auto& comp = context.getCompilation();
12581259
return comp.emplace<SimpleAssignmentPatternExpression>(elementType, /* isLValue */ false,
1259-
elements.copy(comp), sourceRange);
1260+
elements.copy(comp),
1261+
syntax.sourceRange());
12601262
}
12611263

12621264
// Finally, if we have a default then it must now be assignment compatible.
@@ -1265,12 +1267,15 @@ static const Expression* matchElementValue(
12651267

12661268
// Otherwise there's no setter for this element, which is an error.
12671269
if (targetField) {
1268-
auto& diag = context.addDiag(diag::AssignmentPatternNoMember, sourceRange);
1270+
auto& diag = context.addDiag(diag::AssignmentPatternNoMember,
1271+
syntax.getFirstToken().range());
12691272
diag << targetField->name;
12701273
diag.addNote(diag::NoteDeclarationHere, targetField->location);
12711274
}
12721275
else {
1273-
context.addDiag(diag::AssignmentPatternMissingElements, sourceRange);
1276+
SLANG_ASSERT(assignmentTargetType.hasFixedRange());
1277+
context.addDiag(diag::AssignmentPatternMissingElements, syntax.getFirstToken().range())
1278+
<< assignmentTargetType;
12741279
}
12751280

12761281
return nullptr;
@@ -1390,7 +1395,7 @@ Expression& StructuredAssignmentPatternExpression::forStruct(
13901395
continue;
13911396
}
13921397

1393-
auto expr = matchElementValue(context, fieldType, &field, sourceRange, typeSetters,
1398+
auto expr = matchElementValue(context, fieldType, &field, type, syntax, typeSetters,
13941399
defaultSetter);
13951400
if (!expr) {
13961401
bad = true;
@@ -1504,8 +1509,8 @@ Expression& StructuredAssignmentPatternExpression::forFixedArray(
15041509
}
15051510

15061511
if (!cachedVal) {
1507-
cachedVal = matchElementValue(context, elementType, nullptr, syntax.sourceRange(),
1508-
typeSetters, defaultSetter);
1512+
cachedVal = matchElementValue(context, elementType, nullptr, type, syntax, typeSetters,
1513+
defaultSetter);
15091514
if (!cachedVal.value()) {
15101515
bad = true;
15111516
break;
@@ -1567,7 +1572,7 @@ Expression& StructuredAssignmentPatternExpression::forDynamicArray(
15671572
// If there is a default setter expression, translate it to the target type
15681573
// of the array, and store that in case we need it to do constant evaluation.
15691574
if (defaultSetter) {
1570-
auto matched = matchElementValue(context, elementType, nullptr, syntax.sourceRange(), {},
1575+
auto matched = matchElementValue(context, elementType, nullptr, type, syntax, {},
15711576
defaultSetter);
15721577
if (!matched)
15731578
bad = true;
@@ -1578,7 +1583,9 @@ Expression& StructuredAssignmentPatternExpression::forDynamicArray(
15781583
SmallVector<const Expression*> elements;
15791584
if (indexMap.size() != maxIndex + 1 && !defaultSetter) {
15801585
if (!bad) {
1581-
context.addDiag(diag::AssignmentPatternMissingElements, sourceRange);
1586+
context.addDiag(diag::AssignmentPatternMissingDynamicElements,
1587+
syntax.getFirstToken().range())
1588+
<< maxIndex + 1 << type;
15821589
bad = true;
15831590
}
15841591
}

‎tests/regression/ast/assignment-pattern-diags.sv‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ module m;
99
int scalar;
1010
// ^ NoteDeclarationHere declared here - for AssignmentPatternNoMember(scalar)
1111
int array[2];
12+
// ^ NoteDeclarationHere declared here - for AssignmentPatternNoMember(array)
1213
} st;
1314

1415
st value = '{real: 1.0};
15-
// ^^^^^^^^^^^^ AssignmentPatternMissingElements not all elements of array are covered by an assignment pattern key
16-
// ^^^^^^^^^^^^ AssignmentPatternNoMember member 'scalar' is not covered by any assignment pattern key
16+
// ^^ AssignmentPatternNoMember member 'scalar' is not covered by any assignment pattern key
17+
// ^^ AssignmentPatternNoMember member 'array' is not covered by any assignment pattern key
1718

1819
st too_many = '{
1920
// ^^ WrongNumberAssignmentPatterns assignment pattern for 'st' requires 2 elements but 3 were provided
@@ -64,7 +65,13 @@ module assignment_pattern_errors;
6465
// ^^^^^ AssignmentPatternKeyExpr expression is not a valid assignment pattern member name or type
6566
// ^ IndexValueInvalid cannot refer to element 9 of 'int$[1:2]'
6667
int g[] = '{1:1};
67-
// ^^^^^^ AssignmentPatternMissingElements not all elements of array are covered by an assignment pattern key
68+
// ^^ AssignmentPatternMissingDynamicElements assignment pattern does not cover all elements in an array of length 2 of type 'int$[]'
69+
int sparse[] = '{2:1, 5:1, 1000:1};
70+
// ^^ AssignmentPatternMissingDynamicElements assignment pattern does not cover all elements in an array of length 1001 of type 'int$[]'
71+
int very_sparse[] = '{2147483647:1};
72+
// ^^ AssignmentPatternMissingDynamicElements assignment pattern does not cover all elements in an array of length 2147483648 of type 'int$[]'
73+
int fixed_gaps[5:1] = '{5:1, 2:1};
74+
// ^^ AssignmentPatternMissingElements assignment pattern does not cover all elements of type 'int$[5:1]'
6875

6976
st h = '{-1{0}};
7077
// ^^ ValueMustBePositive value must be positive
@@ -82,9 +89,9 @@ module assignment_pattern_errors;
8289
// ^^^ AssignmentPatternDynamicType assignment patterns for dynamic arrays, associative arrays, and queues cannot have type keys
8390

8491
int m[2][2] = '{real:3.14};
85-
// ^^^^^^^^^^^^ AssignmentPatternMissingElements not all elements of array are covered by an assignment pattern key
92+
// ^^ AssignmentPatternMissingElements assignment pattern does not cover all elements of type 'int$[2][2]'
8693
struct { int i; real r; } n[2] = '{real:3.14};
87-
// ^^^^^^^^^^^^ AssignmentPatternNoMember member 'i' is not covered by any assignment pattern key
94+
// ^^ AssignmentPatternNoMember member 'i' is not covered by any assignment pattern key
8895
// ^ NoteDeclarationHere declared here - for AssignmentPatternNoMember(i)
8996
endmodule
9097

@@ -93,12 +100,13 @@ module assignment_pattern_statement;
93100
int scalar;
94101
// ^ NoteDeclarationHere declared here - for AssignmentPatternNoMember(scalar)
95102
int array[2];
103+
// ^ NoteDeclarationHere declared here - for AssignmentPatternNoMember(array)
96104
} st;
97105

98106
st other;
99107
initial other = '{real: 1.0};
100-
// ^^^^^^^^^^^^ AssignmentPatternMissingElements not all elements of array are covered by an assignment pattern key
101-
// ^^^^^^^^^^^^ AssignmentPatternNoMember member 'scalar' is not covered by any assignment pattern key
108+
// ^^ AssignmentPatternNoMember member 'scalar' is not covered by any assignment pattern key
109+
// ^^ AssignmentPatternNoMember member 'array' is not covered by any assignment pattern key
102110
endmodule
103111

104112
module nested_assignment_pattern_statement;
@@ -113,7 +121,7 @@ module nested_assignment_pattern_statement;
113121

114122
outer_t value;
115123
initial value = '{inner: '{present: 1}};
116-
// ^^^^^^^^^^^^^ AssignmentPatternNoMember member 'missing' is not covered by any assignment pattern key
124+
// ^^ AssignmentPatternNoMember member 'missing' is not covered by any assignment pattern key
117125
endmodule
118126

119127
module nested_and_outer_missing_assignment_pattern_statement;
@@ -130,6 +138,6 @@ module nested_and_outer_missing_assignment_pattern_statement;
130138

131139
outer_t value;
132140
initial value = '{inner: '{present: 1}};
133-
// ^^^^^^^^^^^^^^^^^^^^^^^ AssignmentPatternNoMember member 'outer_missing' is not covered by any assignment pattern key
134-
// ^^^^^^^^^^^^^ AssignmentPatternNoMember member 'missing' is not covered by any assignment pattern key
141+
// ^^ AssignmentPatternNoMember member 'outer_missing' is not covered by any assignment pattern key
142+
// ^^ AssignmentPatternNoMember member 'missing' is not covered by any assignment pattern key
135143
endmodule

0 commit comments

Comments
 (0)