Generate more diagnostics from IRGenerator, when given bad code.

Two minor changes:
- Converting a Block with bad statements will now generate a partial
  block instead of nullptr.

The change mirrors how DSL behaves; functions containing invalid
statements will now be created and added to the program. Previously, we
would discard a function definition with any invalid statements inside;
this prevented duplicate-function-definition errors from appearing.

- Converting a return with a bad expression will now generate a
  poisoned return instead of nullptr.

This change improves diagnostics for functions with invalid return
statements. If we eliminate the return statements (by returning null),
we report bad return statements as "function can exit without returning
a value" (which is confusing).

Change-Id: I6d998d5c50585f8d96bb7e3cb7f59b63125d6a62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446325
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-09-07 15:22:02 -04:00 committed by SkCQ
parent 887230a36a
commit ff6c5bf5ed
6 changed files with 18 additions and 16 deletions

View File

@ -47,6 +47,7 @@
#include "src/sksl/ir/SkSLInterfaceBlock.h" #include "src/sksl/ir/SkSLInterfaceBlock.h"
#include "src/sksl/ir/SkSLMethodReference.h" #include "src/sksl/ir/SkSLMethodReference.h"
#include "src/sksl/ir/SkSLNop.h" #include "src/sksl/ir/SkSLNop.h"
#include "src/sksl/ir/SkSLPoison.h"
#include "src/sksl/ir/SkSLPostfixExpression.h" #include "src/sksl/ir/SkSLPostfixExpression.h"
#include "src/sksl/ir/SkSLPrefixExpression.h" #include "src/sksl/ir/SkSLPrefixExpression.h"
#include "src/sksl/ir/SkSLReturnStatement.h" #include "src/sksl/ir/SkSLReturnStatement.h"
@ -141,10 +142,9 @@ std::unique_ptr<Block> IRGenerator::convertBlock(const ASTNode& block) {
StatementArray statements; StatementArray statements;
for (const auto& child : block) { for (const auto& child : block) {
std::unique_ptr<Statement> statement = this->convertStatement(child); std::unique_ptr<Statement> statement = this->convertStatement(child);
if (!statement) { if (statement) {
return nullptr; statements.push_back(std::move(statement));
} }
statements.push_back(std::move(statement));
} }
return Block::Make(block.fOffset, std::move(statements), fSymbolTable); return Block::Make(block.fOffset, std::move(statements), fSymbolTable);
} }
@ -518,14 +518,14 @@ std::unique_ptr<Statement> IRGenerator::convertReturn(int offset,
std::unique_ptr<Statement> IRGenerator::convertReturn(const ASTNode& r) { std::unique_ptr<Statement> IRGenerator::convertReturn(const ASTNode& r) {
SkASSERT(r.fKind == ASTNode::Kind::kReturn); SkASSERT(r.fKind == ASTNode::Kind::kReturn);
if (r.begin() != r.end()) { if (r.begin() != r.end()) {
std::unique_ptr<Expression> value = this->convertExpression(*r.begin()); if (std::unique_ptr<Expression> value = this->convertExpression(*r.begin())) {
if (!value) { return this->convertReturn(r.fOffset, std::move(value));
return nullptr; } else {
return this->convertReturn(r.fOffset, Poison::Make(r.fOffset, fContext));
} }
return this->convertReturn(r.fOffset, std::move(value));
} else {
return this->convertReturn(r.fOffset, /*result=*/nullptr);
} }
return this->convertReturn(r.fOffset, /*result=*/nullptr);
} }
std::unique_ptr<Statement> IRGenerator::convertBreak(const ASTNode& b) { std::unique_ptr<Statement> IRGenerator::convertBreak(const ASTNode& b) {

View File

@ -5,7 +5,9 @@ error: 29: type mismatch: '=' cannot operate on 'uint', 'float'
error: 30: type mismatch: '=' cannot operate on 'int', 'uint' error: 30: type mismatch: '=' cannot operate on 'int', 'uint'
error: 31: type mismatch: '=' cannot operate on 'int', 'float' error: 31: type mismatch: '=' cannot operate on 'int', 'float'
error: 32: type mismatch: '=' cannot operate on 'float', 'int' error: 32: type mismatch: '=' cannot operate on 'float', 'int'
error: 32: function 'f_eq_i_disallowed' can exit without returning a value
error: 33: type mismatch: '=' cannot operate on 'float', 'uint' error: 33: type mismatch: '=' cannot operate on 'float', 'uint'
error: 33: function 'f_eq_u_disallowed' can exit without returning a value
error: 34: type mismatch: '=' cannot operate on 'uint', 'int' error: 34: type mismatch: '=' cannot operate on 'uint', 'int'
error: 35: type mismatch: '=' cannot operate on 'uint', 'float' error: 35: type mismatch: '=' cannot operate on 'uint', 'float'
error: 36: type mismatch: '+' cannot operate on 'int', 'float' error: 36: type mismatch: '+' cannot operate on 'int', 'float'
@ -48,4 +50,4 @@ error: 72: type mismatch: '+' cannot operate on 'uint', 'int'
error: 73: type mismatch: '-' cannot operate on 'uint', 'int' error: 73: type mismatch: '-' cannot operate on 'uint', 'int'
error: 74: type mismatch: '*' cannot operate on 'uint', 'int' error: 74: type mismatch: '*' cannot operate on 'uint', 'int'
error: 75: type mismatch: '/' cannot operate on 'uint', 'int' error: 75: type mismatch: '/' cannot operate on 'uint', 'int'
48 errors 50 errors

View File

@ -1,4 +1,5 @@
### Compilation failed: ### Compilation failed:
error: 1: expected 'int', but found 'float' error: 1: expected 'int', but found 'float'
1 error error: 1: unknown identifier 'i'
2 errors

View File

@ -1,5 +1,4 @@
### Compilation failed: ### Compilation failed:
error: 2: invalid arguments to 'float[1]' constructor (expected 1 elements, but found 0) error: 2: invalid arguments to 'float[1]' constructor (expected 1 elements, but found 0)
error: 3: function 'void wna()' is not defined 1 error
2 errors

View File

@ -1,5 +1,4 @@
### Compilation failed: ### Compilation failed:
error: 2: invalid arguments to 'float[1]' constructor (expected 1 elements, but found 0) error: 2: invalid arguments to 'float[1]' constructor (expected 1 elements, but found 0)
error: 3: function 'void wna()' is not defined 1 error
2 errors

View File

@ -4,4 +4,5 @@ error: 3: discard statement is only permitted in fragment shaders
error: 5: do-while loops are not supported error: 5: do-while loops are not supported
error: 7: while loops are not supported error: 7: while loops are not supported
error: 9: switch statements are not supported error: 9: switch statements are not supported
4 errors error: 9: function 'switch_stmt' can exit without returning a value
5 errors