Handle access control on var declarations instead of via [[access]]
decorations. This change does the minimal work to migrate the WGSL
parser over to the new syntax. Additional changes will be needed
to correctly generate defaulted access qualifiers, as well as
validating access usage.
The [[access]] decorations are still supported by the WGSL parser,
with new deprecated warnings, but not for aliases. Example:
var x : [[access(x)]] alias_to_struct;
Making this work is far more effort than I want to dedicate to backwards
compatibility, and I do not beleive any real-world usage will be doing
this.
Still TODO:
* Adding access control as the optional, third parameter to ptr<>.
* Calculating default accesses for the various storage types.
* Validating usage of variables against the different accesses.
Bug: tint:846
Change-Id: If8ca82e5d16ec319ecd01f9a2cafffd930963bde
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53088
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Change the type of the values in an ast::WorkgroupDecoration to be
ast::Expression nodes, so that they can represent both
ast::ScalarExpression (literal) and ast::IdentifierExpression
(module-scope constant).
The Resolver processes these nodes to produce a uint32_t for the
default value on each dimension, and captures a reference to the
module-scope constant if it is overridable (which will soon be used by
the inspector and backends).
The WGSL parser now uses `primary_expression` to parse arguments to
workgroup_size.
Also added some WorkgroupSize() helpers to ProgramBuilder.
Bug: tint:713
Change-Id: I44b7b0021b925c84f25f65e26dc7da6b19ede508
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51262
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
The workgroup size should not be a property of the function in the
AST, and this lays the groundwork for allowing both literals and
module-scope constants to be used for this attribute.
Bug: tint:713
Change-Id: I014be879e2adb81cfc5b0ea0e221035fae626223
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51261
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Remove all sem::Type references from the AST.
ConstructedTypes are now all AST types.
The parsers will still create semantic types, but these are now disjoint
and ignored.
The parsers will be updated with future changes to stop creating these
semantic types.
Resolver creates semantic types from the AST types. Most downstream
logic continues to use the semantic types, however transforms will now
need to rebuild AST type information instead of reassigning semantic
information, as semantic nodes are fully rebuilt by the Resolver.
Bug: tint:724
Change-Id: I4ce03a075f13c77648cda5c3691bae202752ecc5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49747
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
A general code cleanup
Change-Id: Ib251ca2d4b71f75736bdba8b4255965593a76c31
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48680
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Assert in each AST constructor that symbols belong to the program of the parent.
Bug: tint:709
Change-Id: I82ae9b23c88e89714a44e057a0272f0293385aaf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47624
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Assert in each AST constructor that child nodes belong to the program of the parent.
Bug: tint:709
Change-Id: Icc89b69691d099e358ff632a0ca6fd7943cb0193
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47623
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Fix some SPIR-V tests that were wrongly expecting parameters to be
loaded from memory as a result of them not being const.
Change-Id: Ieab6f1edd4a4ba6efd226fe190e7a49b309048c5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47281
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Remove the decoration groupings (Array, Function, Struct,
StructMember, Type, Variable), such that all *Decoration classes now
subclass ast::Decoration directly. This allows for decorations to be
used in multiple places; for example, builtin decorations are now
valid for both variables and struct members.
Checking that decoration lists only contain decorations that are valid
for the node that they are attached to is now done inside the
validator.
Change-Id: Ie8c0e53e5730a7dedea50a1dec8f26f9e7b00e8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44320
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
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 the storage parameter after type.
Const() has no use for storage classes, so this parameter will be removed in the next change.
This reordering keeps Var() and Const() parameter types identical for the first two non-optional fields
Change-Id: I66669d19fa2175c4f10f615941e69efcab4c23e1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41540
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Pull the mutable semantic fields from ast::Function and into a new semantic::Function node.
Have the TypeDeterminer create these semantic::Function nodes.
Bug: tint:390
Change-Id: I237b1bed8709dd9a3cfa24d85d48fc77b7e532da
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39902
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
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>
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>
This builder will be merged into ProgramBuilder, where these will become methods.
To breakup this change, perform the refactoring as a separate change.
Bug: tint:390
Change-Id: I2c9151cd9f198e99d88eaf296dd994293df6c425
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38720
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This is required in order to support move operators for TypesBuilder.
Bug: tint:390
Change-Id: I9667bda5f5be267df092f5cd94dc40db053ae6e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38555
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Module will be split into Module (immutable) and ModuleBuilder (mutable).
By moving these methods to the FunctionList, we can deduplicate a bunch of common logic.
Bug: tint:390
Change-Id: I3fd85200aae4e8dc3d5afce8c9aaa6512809a3a0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38363
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This CL removes the function name accessor and changes all usages to use
the symbol.
Change-Id: I19b92bf1bc557ba14e68ef8cb381487a4ad1f7ee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36821
Reviewed-by: Ben Clayton <bclayton@google.com>
This CL goes through and converts things which call RegisterSymbol to
use the helper builders. Several variables are also updated to use the
helper as it will need RegisterSymbol in the near future.
Change-Id: Ib5a8e8be54c1eaad123384fab09f6625421d9fcd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35880
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Remove Source{} with ast::Builder::create<>
Use Builder helpers where possible
Change-Id: I7b3188fa8239bf11e88ff6b396a57c23d76a3152
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35660
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This consistency can be utilized by the ast::Builder to inject the source parameter if it isn't provided.
Bug: tint:396
Bug: tint:390
Change-Id: I2f19002131e79daae799b8cbe918eb192d6bfc75
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35503
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This CL adds a simple demangler to convert the `tint_symbol_YYY` back to
the original symbol name.
Change-Id: I532ed13dc4c52e0f0e3568b8b7d8d0a5c67d8472
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35440
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
This Cl adds a Symbol representing the function name to the function
AST. The symbol is added alongside the name for now. When all usages of
the function name are removed then the string version will be removed
from the constructor.
Change-Id: Ib2450e5fe531e988b25bb7d2937acc6af2187871
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35220
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Remove all Variable setters (with exception to set_storage_class() which is called by the TypeDeterminer)
Bug: tint:390
Change-Id: I172667e21e2b02e85dcea6703aa1e608ec718250
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35015
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
set_source() will be removed, so sources will only be specifiable at construction time.
Bug: tint:390
Change-Id: I5c79efd3fa501ebd9308f7f93cfb77bc12198047
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35009
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Move them to the constructor
Bug: tint:390
Change-Id: I30bb6a1de060b790bf5202194d020d4e3889a307
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35008
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
They already exist in a `ast::type` namespace, so `ast::type::BlahType` is just stuttering.
This is more important now that Is<> and As<> use the full type name.
Change-Id: I7c661fe58cdc33ba7e9a95c82c996a799786661f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34321
Reviewed-by: dan sinclair <dsinclair@chromium.org>
No need to prefix with `ast::` when you're in the ast namespace already.
Change-Id: Iac6cd3a215c05a80ee2035d582500f1d6c882a06
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34320
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Remove all redundant std::move()s. I've also removed calls to
std::move() in tests, even if they act as an optimization. This is for
two reasons:
(a) Performance is not important for testing, and this helps with
readability.
(b) A whole bunch tests were relying on std::move() clearing vectors so
they can be repopulated and used again. This is undefined behavior:
> Objects of types defined in the C++ standard library may be moved from
> (12.8). Move operations may be explicitly specified or implicitly
> generated. Unless otherwise specified, such moved-from objects shall
> be placed in a valid but unspecified state.
All of these UB cases have been fixed.
Removed all duplicate variables left over from:
`auto* foo_ptr = foo.get()`
which became:
`auto* foo_ptr = foo`
Bug: tint:322
Change-Id: Ibd08a2379671382320fd4d8da296ccc6a378b8af
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32900
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This is a minimal effort to fix up the code. There's substantial code
cleanup which can now be done, which is done in the next change.
Bug: tint:322
Change-Id: Iafcf5e814837d9534889e8c21333de4931a19cfa
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32864
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
In a near-future change, AST nodes, such as ast::BlockStatement will no longer
be std::unique_ptrs, and will have to be constructed and owned by an external
class. This means AST nodes can no longer allocate default child nodes.
Bug: tint:322
Change-Id: Iddb5605b9bc0de80ad2710ced0e429f89410af2f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32675
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
create() is currently just a simple forwarder to std::make_unique<>, but
will be later replaced with a function that returns a raw pointer,
and owned by the context.
Bug: tint:322
Change-Id: I0e68992963f52e432d4d485feae1123f35732552
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32664
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
It doesn't actually do anything (yet), but will hold the context and
a helper for constructing AST nodes.
Bug: tint:322
Change-Id: Ic7ba92bf39abf64ff2ac51d81c8a6338f5eff608
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32663
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Once a `Decoration` has been parsed, it'll be placed into a `DecorationList` and validated later in the parse. In order to create error diagnostics that refer back to the decoration, we need to know its source.
Bug: tint:282
Bug: tint:291
Change-Id: I38de708adbd041601b61d7e0a4d0402e9a2fe526
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31722
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This was a temporary overload to break up the changes into smaller chunks.
Change all call sites to use one of the other constructors.
Bug: tint:282
Change-Id: I500fe9700d22f72312827808caa22f7feef7b294
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31440
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This is the first step in improving the error messages produced while parsing.
The `line` and `column` information of `Source` has been moved to `Source::Location`.
`Source::Range` has been added that contains a `Location` interval - allowing error messages to highlight the full region of the error.
The `File` information provides an optional file path, and pre-splits the content into lines. These lines can be used to print the full line containing an error.
This CL contains a few temporary changes that help split up this work, and to ease integration with Tint.
Bug: tint:282
Change-Id: I7aa501b0a9631f286e8e93fd7396bdbe38175727
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31420
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
This CL adds the workgroup_size decoration to functions and emits as
needed from the various backends.
Change-Id: Ifffde239e68047f6419c6980eca70c4efa9822c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/28662
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
This CL extends the function AST object to allow function decorations to
be attached.
Change-Id: I5b16aa2e6927792368b7f88047c0229e258b5bd3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/28661
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
This CL adds a function which returns the last statement of a ast::Function
Change-Id: I1dc68b7f4669c17a24a62c03a87dcc95866a428d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26720
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
This CL converts the Function class to using a BlockStatement
internally. All usages have been updated execept for the two readers.
Bug: tint:130
Change-Id: I7159cf2d3ed5cb8a34d51fbe848b88f0e5479605
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25720
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
This CL removes the KillStatement from the AST and replaces all test
usage with DiscardStatement.
Bug: tint:169
Change-Id: Ie68dd3cdd54056f144d10506f05cc1f6903d1cda
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25605
Reviewed-by: David Neto <dneto@google.com>
Couple of little style issues, and a path to a file that was incorrect.
Change-Id: I21d1a4ca41bc36163602378086989ac712f54469
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25265
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This CL adds a helper to get referenced builtins and referenced
locations from the ast::Function.
Change-Id: I95cf7efd6b0fe9569c5e514d3245112e78147ffe
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/24783
Reviewed-by: David Neto <dneto@google.com>
This Cl updates the type determiner to annotate each function with the
name of any entry points which call into the given function. This will
allow determining in the backends if we need to duplicate the function
due to differing entry point parameter requirements.
Bug: tint:8
Change-Id: Icd7c4ccab72dd6eabcf0abaf1159319949c4ecf5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/24760
Reviewed-by: David Neto <dneto@google.com>
