* Fixed many tests that now failed validation. Most of the time,
functions declared that they returned a type, but with no return
statement.
* ProgramBuilder::WrapInFunction now returns the function is creates,
and std::moves its StatementList.
* ProgramBuilder: Added Return function to create ast::ReturnStatements
more easily.
Bug: tint:642
Change-Id: I3011314e66e264ebd7b89bf9271392391be6a0e5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45382
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
The readers must not produce invalid ASTs.
If readers cannot produce a valid AST, then they should error instead.
If a reader does produce an invalid AST, this change catches this bad behavior early, significantly helping identify the root of the broken logic.
IsValid() made a bit more sense in the days where the AST was mutable, and was constructed by calling setters on the nodes to build up the tree.
In order to detect bad ASTs, IsValid() would have to perform an entire AST traversal and give a yes / no answer for the entire tree. Not only was this slow, an answer of 'no' didn't tell you *where* the AST was invalid, resulting in a lot of manual debugging.
Now that the AST is fully immutable, all child nodes need to be built before their parents. The AST node constructors now become a perfect place to perform pointer sanity checking.
The argument for attempting to catch and handle invalid ASTs is not a compelling one.
Invalid ASTs are invalid compiler behavior, not something that should ever happen with a correctly functioning compiler.
If this were to happen in production, the user would be utterly clueless to _why_ the program is invalid, or _how_ to fix it.
Attempting to handle invalid ASTs is just masking a much larger problem.
Let's just let the fuzzers do their job to catch any of these cases early.
Fixed: chromium:1185569
Change-Id: I6496426a3a9da9d42627d2c1ca23917bfd04cc5c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44048
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
All includes from .cc to .h are preserved, even when transitively included.
It's clear that there are far too many includes in header files, and we should be more aggressive with forward declarations. tint:532 will continue to track this work.
There are, however, plenty of includes that have accumulated over time which are no longer required directly or transitively, so this change starts with a clean slate of *required* includes.
Bug: tint:532
Change-Id: Ie1718dad565f8309fa180ef91bcf3920e76dba18
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44042
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Move out of the src root and into its own subdirectory
Rename methods to remove the 'Determine' prefix.
Fixed: tint:529
Change-Id: Idf89d647780f8a2e7495c1c9e6c402e00ad45b7c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44041
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
When copying or moving a ProgramBuilder, we always want the ProgramBuilder::ty to point back to the owner.
We were previously std::move()'ing the ty field, which is not correct - this will result in the TypesBuilder pointing to the wrong ProgramBuilder.
I'm not sure why we've not seen any issues with this using clang, but running under MSVC immediately highlighted this brokenness.
Change-Id: I4293bb00ac4fbdfa66d12b1504a7bd060e014cd6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41861
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Useful if you have a symbol already
Change-Id: Ib9e15ea761f58ee67dc3cc722d9129cd5369d92b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41481
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Adds a vector<CastableBase*> to ast::Module which stores the list of
global variables, functions, and types, in the order that they were
declared.
This will be used to fix validation and backend issues around name
uniqueness.
Change-Id: I14491f6ebc0fc7341bd3fb3b3f408faa234a91f7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41301
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Make private all TypeDeterminer::DetermineXXX() methods, forcing all tests to use the root-level TypeDeterminer::Determine() method.
Remove TypeDeterminer::RegisterVariableForTesting().
The main use for calling the TypeDeterminer::DetermineXXX() methods was to perform type determination on a partial AST.
This was messy and often resulting in multiple calls into TypeDeterminer. Most tests already perform a full TypeDeterminer::Determine() call when the program is built, so many of these were redundant.
The exposure of these internal methods for testing also makes refactoring the TypeDeterminer extremely difficult.
Add a number of ProgramBuilder helper methods for attaching the partial AST in these tests to the root of the AST, greatly simplifying the use of the TypeDeterminer:
* ProgramBuilder::Global() and ProgramBuilder::GlobalConst() are helpers that register the variable returned by ProgramBuilder::Var() and ProgramBuilder::Const(), respectively.
* ProgramBuilder::WrapInFunction() is a variadic function that accepts variables, expressions and statements, attaching these to the root of the AST via a dummy function.
Most test classes now no longer use their own TypeDeterminer, and instead properly depend on the automatic type determination performed at Program build time.
Bug: tint:390
Change-Id: Ie901890420c5de170cdf2a7aaef9b96fc3bebd60
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40062
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Remove the mutable `result_type` from the ast::Expression.
Replace this with the use of semantic::Expression.
Bug: tint:390
Change-Id: I1f0eaf0dce8fde46fefe50bf2c5fe5b2e4d2d2df
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39007
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Follows the basic shallow-clone pattern of type::Manager::Wrap(),
which this new method uses.
Use ProgramBuilder::Wrap() in the writers, where we were previously
using type::Manager::Wrap(). This simplifies some of the logic
(especially around AppendVector()).
This is now required as semantic information is generated inside the
writers for newly constructed AST nodes, and we need to register
this semantic info so it can be fetched later.
Change-Id: Ia362204a1f13054e50545baeb7d269f210a36826
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39004
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Add helpers on Program and ProgramBuilder that significantly simplify
usage.
Also demangle - this also reduces a bunch of copy-pasta code.
Change-Id: I6215c346e7f6e49c20aced058a6150603253ed93
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39342
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Will hold the mutable fields that currently reside in the otherwise immutable-AST.
Change the AST string methods to accept a `const semantic::Info&`. This is required as some nodes include type-resolved information in their output strings.
Bug: tint:390
Change-Id: Iba494a9c5645ce2096da0a8cfe63a4309a9d9c3c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39003
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
By putting diagnostics into the program, we can hold all the diagnostic messages for parsing and type determination in one place.
This also means that we can simplify the public WGSL and SPIR-V Parser interfaces to a single function.
Change-Id: Ib6ab5fa180addd45c4aaf0c6b192d47182ffb50a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38920
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Program is now immutable*, and remains part of the public Tint
interface.
ProgramBuilder is the mutable builder for Programs, and is not part of
the public Tint interface. ast::Builder has been folded into
ProgramBuilder.
Immutable Programs can be cloned into a mutable ProgramBuilder with
Program::CloneAsBuilder().
Mutable ProgramBuilders can be moved into immutable Programs.
* - mostly immutable. It still has a move constructor and move
assignment operator - required for practical usage - and the
semantic information on AST nodes is still mutable.
Bug: tint:390
Change-Id: Ia856c50b1880c2f95c91467a9eef5024cbc380c6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38240
Reviewed-by: dan sinclair <dsinclair@chromium.org>