ast: Remove Node::set_source()

All nodes that don't have a Source constructor will need to have one added.
We can then find all missing Source mappings with a search for `Source{}`

Bug: tint:396
Bug: tint:390
Change-Id: I06f9689d4da0f3fd1bd757c7358dcc65f15dc752
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35018
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
Ben Clayton 2020-12-12 01:24:53 +00:00 committed by Commit Bot service account
parent 4226b6a1d8
commit 604bc72dd9
5 changed files with 21 additions and 61 deletions

View File

@ -47,9 +47,6 @@ class Node : public Castable<Node> {
/// @returns the node source data /// @returns the node source data
const Source& source() const { return source_; } const Source& source() const { return source_; }
/// Sets the source data
/// @param source the source data
void set_source(const Source& source) { source_ = source; }
/// @returns true if the node is valid /// @returns true if the node is valid
virtual bool IsValid() const = 0; virtual bool IsValid() const = 0;

View File

@ -16,6 +16,7 @@
#define SRC_AST_TEST_HELPER_H_ #define SRC_AST_TEST_HELPER_H_
#include <memory> #include <memory>
#include <string>
#include <utility> #include <utility>
#include "gtest/gtest.h" #include "gtest/gtest.h"

View File

@ -608,12 +608,6 @@ class StructuredTraverser {
std::unordered_set<uint32_t> visited_; std::unordered_set<uint32_t> visited_;
}; };
/// @param src a source record
/// @returns true if `src` is a non-default Source
bool HasSource(const Source& src) {
return src.range.begin.line > 0 || src.range.begin.column != 0;
}
} // namespace } // namespace
BlockInfo::BlockInfo(const spvtools::opt::BasicBlock& bb) BlockInfo::BlockInfo(const spvtools::opt::BasicBlock& bb)
@ -722,14 +716,6 @@ ast::Statement* FunctionEmitter::AddStatement(ast::Statement* statement) {
return result; return result;
} }
ast::Statement* FunctionEmitter::AddStatementForInstruction(
ast::Statement* statement,
const spvtools::opt::Instruction& inst) {
auto* node = AddStatement(statement);
ApplySourceForInstruction(node, inst);
return node;
}
ast::Statement* FunctionEmitter::LastStatement() { ast::Statement* FunctionEmitter::LastStatement() {
assert(!statements_stack_.empty()); assert(!statements_stack_.empty());
auto* statement_list = statements_stack_.back().statements_; auto* statement_list = statements_stack_.back().statements_;
@ -1880,7 +1866,7 @@ bool FunctionEmitter::EmitFunctionVariables() {
inst.result_id(), ast::StorageClass::kFunction, var_store_type, false, inst.result_id(), ast::StorageClass::kFunction, var_store_type, false,
constructor, ast::VariableDecorationList{}); constructor, ast::VariableDecorationList{});
auto* var_decl_stmt = create<ast::VariableDeclStatement>(var); auto* var_decl_stmt = create<ast::VariableDeclStatement>(var);
AddStatementForInstruction(var_decl_stmt, inst); AddStatement(var_decl_stmt);
// Save this as an already-named value. // Save this as an already-named value.
identifier_values_.insert(inst.result_id()); identifier_values_.insert(inst.result_id());
} }
@ -2762,8 +2748,7 @@ bool FunctionEmitter::EmitConstDefinition(
if (!ast_const) { if (!ast_const) {
return false; return false;
} }
AddStatementForInstruction(create<ast::VariableDeclStatement>(ast_const), AddStatement(create<ast::VariableDeclStatement>(ast_const));
inst);
// Save this as an already-named value. // Save this as an already-named value.
identifier_values_.insert(inst.result_id()); identifier_values_.insert(inst.result_id());
return success(); return success();
@ -2777,11 +2762,10 @@ bool FunctionEmitter::EmitConstDefOrWriteToHoistedVar(
if (def_info && def_info->requires_hoisted_def) { if (def_info && def_info->requires_hoisted_def) {
auto name = namer_.Name(result_id); auto name = namer_.Name(result_id);
// Emit an assignment of the expression to the hoisted variable. // Emit an assignment of the expression to the hoisted variable.
AddStatementForInstruction(create<ast::AssignmentStatement>( AddStatement(create<ast::AssignmentStatement>(
create<ast::IdentifierExpression>( create<ast::IdentifierExpression>(ast_module_.RegisterSymbol(name),
ast_module_.RegisterSymbol(name), name), namer_.Name(result_id)),
ast_expr.expr), ast_expr.expr));
inst);
return true; return true;
} }
return EmitConstDefinition(inst, ast_expr); return EmitConstDefinition(inst, ast_expr);
@ -2848,8 +2832,7 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
// TODO(dneto): Order of evaluation? // TODO(dneto): Order of evaluation?
auto lhs = MakeExpression(ptr_id); auto lhs = MakeExpression(ptr_id);
auto rhs = MakeExpression(value_id); auto rhs = MakeExpression(value_id);
AddStatementForInstruction( AddStatement(create<ast::AssignmentStatement>(lhs.expr, rhs.expr));
create<ast::AssignmentStatement>(lhs.expr, rhs.expr), inst);
return success(); return success();
} }
case SpvOpLoad: { case SpvOpLoad: {
@ -3751,8 +3734,7 @@ bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) {
} }
if (result_type->Is<ast::type::Void>()) { if (result_type->Is<ast::type::Void>()) {
return nullptr != AddStatementForInstruction( return nullptr != AddStatement(create<ast::CallStatement>(call_expr));
create<ast::CallStatement>(call_expr), inst);
} }
return EmitConstDefOrWriteToHoistedVar(inst, {result_type, call_expr}); return EmitConstDefOrWriteToHoistedVar(inst, {result_type, call_expr});
@ -3816,16 +3798,9 @@ TypedExpression FunctionEmitter::MakeSimpleSelect(
return {}; return {};
} }
void FunctionEmitter::ApplySourceForInstruction( Source FunctionEmitter::GetSourceForInst(
ast::Node* node, const spvtools::opt::Instruction& inst) const {
const spvtools::opt::Instruction& inst) { return parser_impl_.GetSourceForInst(&inst);
if (!node) {
return;
}
const Source& existing = node->source();
if (!HasSource(existing)) {
node->set_source(parser_impl_.GetSourceForInst(&inst));
}
} }
bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
@ -4028,7 +4003,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
} else { } else {
// It's an image write. No value is returned, so make a statement out // It's an image write. No value is returned, so make a statement out
// of the call. // of the call.
AddStatementForInstruction(create<ast::CallStatement>(call_expr), inst); AddStatement(create<ast::CallStatement>(call_expr));
} }
return success(); return success();
} }

View File

@ -803,22 +803,10 @@ class FunctionEmitter {
/// @returns a pointer to the statement. /// @returns a pointer to the statement.
ast::Statement* AddStatement(ast::Statement* statement); ast::Statement* AddStatement(ast::Statement* statement);
/// Appends a new statement to the top of the statement stack, and attaches /// Returns the source record for the given instruction.
/// source location information from the given instruction. Does nothing if
/// the statement is null.
/// @param statement the new statement
/// @returns a pointer to the statement.
ast::Statement* AddStatementForInstruction(
ast::Statement* statement,
const spvtools::opt::Instruction& inst);
/// Sets the source information for the given instruction to the given
/// node, if the node doesn't already have a source record. Does nothing
/// if `node` is null.
/// @param node the AST node
/// @param inst the SPIR-V instruction /// @param inst the SPIR-V instruction
void ApplySourceForInstruction(ast::Node* node, /// @return the Source record, or a default one
const spvtools::opt::Instruction& inst); Source GetSourceForInst(const spvtools::opt::Instruction& inst) const;
/// @returns the last statetment in the top of the statement stack. /// @returns the last statetment in the top of the statement stack.
ast::Statement* LastStatement(); ast::Statement* LastStatement();

View File

@ -73,6 +73,7 @@ namespace {
class FakeStmt : public ast::Statement { class FakeStmt : public ast::Statement {
public: public:
explicit FakeStmt(Source source) : ast::Statement(source) {}
FakeStmt* Clone(ast::CloneContext*) const override { return nullptr; } FakeStmt* Clone(ast::CloneContext*) const override { return nullptr; }
bool IsValid() const override { return true; } bool IsValid() const override { return true; }
void to_str(std::ostream& out, size_t) const override { out << "Fake"; } void to_str(std::ostream& out, size_t) const override { out << "Fake"; }
@ -80,6 +81,7 @@ class FakeStmt : public ast::Statement {
class FakeExpr : public ast::Expression { class FakeExpr : public ast::Expression {
public: public:
explicit FakeExpr(Source source) : ast::Expression(source) {}
FakeExpr* Clone(ast::CloneContext*) const override { return nullptr; } FakeExpr* Clone(ast::CloneContext*) const override { return nullptr; }
bool IsValid() const override { return true; } bool IsValid() const override { return true; }
void to_str(std::ostream&, size_t) const override {} void to_str(std::ostream&, size_t) const override {}
@ -106,8 +108,7 @@ class TypeDeterminerTestWithParam : public TypeDeterminerHelper,
public testing::TestWithParam<T> {}; public testing::TestWithParam<T> {};
TEST_F(TypeDeterminerTest, Error_WithEmptySource) { TEST_F(TypeDeterminerTest, Error_WithEmptySource) {
FakeStmt s; FakeStmt s(Source{});
s.set_source(Source{});
EXPECT_FALSE(td()->DetermineResultType(&s)); EXPECT_FALSE(td()->DetermineResultType(&s));
EXPECT_EQ(td()->error(), EXPECT_EQ(td()->error(),
@ -115,8 +116,7 @@ TEST_F(TypeDeterminerTest, Error_WithEmptySource) {
} }
TEST_F(TypeDeterminerTest, Stmt_Error_Unknown) { TEST_F(TypeDeterminerTest, Stmt_Error_Unknown) {
FakeStmt s; FakeStmt s(Source{Source::Location{2, 30}});
s.set_source(Source{Source::Location{2, 30}});
EXPECT_FALSE(td()->DetermineResultType(&s)); EXPECT_FALSE(td()->DetermineResultType(&s));
EXPECT_EQ(td()->error(), EXPECT_EQ(td()->error(),
@ -432,8 +432,7 @@ TEST_F(TypeDeterminerTest, Stmt_VariableDecl_ModuleScope) {
} }
TEST_F(TypeDeterminerTest, Expr_Error_Unknown) { TEST_F(TypeDeterminerTest, Expr_Error_Unknown) {
FakeExpr e; FakeExpr e(Source{Source::Location{2, 30}});
e.set_source(Source{Source::Location{2, 30}});
EXPECT_FALSE(td()->DetermineResultType(&e)); EXPECT_FALSE(td()->DetermineResultType(&e));
EXPECT_EQ(td()->error(), "2:30: unknown expression for type determination"); EXPECT_EQ(td()->error(), "2:30: unknown expression for type determination");