Commit Graph

26 Commits

Author SHA1 Message Date
Ben Clayton e6995de232 Add ProgramID feed it into all ast::Nodes
This will be used to detect accidental leaks of program objects between programs.

Bug: tint:709
Change-Id: I20f784a2c673d19a04a880b3ec91dfe2eb743bdb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47622
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>
2021-04-13 23:27:27 +00:00
Ben Clayton dba65b7a34 Add semantic::Statement::Block() for getting the owning BlockStatement
Required special casing the ElseStatement, as this isn't actually owned by a BlockStatement.

Change-Id: Ic33c207598b838a12b865a7694e596b2629c9208
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46443
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-03-31 20:35:46 +00:00
Ben Clayton 8454d824d4 ast: Replace IsValid() with TINT_ASSERT()
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>
2021-03-10 11:41:49 +00:00
Ben Clayton 5b36d2c612 Remove all unnecessary includes
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>
2021-03-09 11:11:17 +00:00
Antonio Maiorano fd31bbd3f1 TD: validate continue statements bypassing body variables
We now keep track of scopes as a tree of BlockInfos that track variables
declared in each scope. For loop scopes, we store the index of the first
variable (if any) that follows the first continue statement. Using this
data structure, when parsing expressions, we validate that used
variables in continuing blocks are not bypassed by a continue statement
in the parent loop block.

Also:
* Validate that continue statements are in a loop in TD. This error is
already caught by the spir-v writer, but better to catch it here.
* Add more utility functions to ProgramBuilder to make it easier to
write tests

Fixed: tint:17
Change-Id: I967bf2cfb63062bac8dcca113d074ba0fe2152e2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44120
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2021-03-09 10:26:57 +00:00
Ben Clayton 4602ce7195 ast: Remove @notes about semantic info not being cloned
Semantic info is no longer part of the ast, so it is now odd to mention semantic info on a clone method for the AST.

Improve the documentation around cloning on the Program methods and the CloneContext.

Change-Id: Ib1cf255acfd994521aaa5add2789e5117db6b072
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41548
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
2021-02-17 01:01:03 +00:00
Ben Clayton dd1b6fca9f Introduce semantic::Info
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>
2021-01-29 10:55:40 +00:00
Ben Clayton be96376d8e ast: Make all non-semantic fields const
Annotate those that are set by the TypeDeterminer as "Semantic Info"

Bug: tint:396
Bug: tint:390
Change-Id: I0705c64e8e23d97a6430230728f82e64dd92efb7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35165
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-15 14:52:38 +00:00
Ben Clayton d408f2465a Remove BlockStatement::insert()
Bug: tint:396
Bug: tint:390
Change-Id: I719b84804164fa801ded505ed56717948f06c7a7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35502
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-14 20:31:17 +00:00
Ben Clayton db5ce658b5 Remove BlockStatement::append()
Bug: tint:396
Bug: tint:390
Change-Id: I3b558a8961f9890f24d1aa3d6647ec095e5fe1cb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35421
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-14 20:25:27 +00:00
Ben Clayton b833f1572b reader/spirv: Remove use of BlockStatement::append()
Introduce `StatementBuilder`s , which may hold mutable state, before being converted into the immutable AST node on completion of the `BlockStatement`.

Bug: tint:396
Bug: tint:390
Change-Id: I0381c4ae7948be0de02bc13e54e0037a72baaf0c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35506
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
2020-12-14 19:48:47 +00:00
Ben Clayton bbefff63a3 ast: Remove statement constructors that don't take a Source
Parsers need fixing up.

Bug: tint:396
Bug: tint:390
Change-Id: I137f1017ca56125cf3d52ecbef2ff46d0574338b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35161
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-12-12 11:58:44 +00:00
Ben Clayton f8971ae74d Fixup all doxygen tags
We've been using |blah| in various places to markup code, however this is not doxygen markup. So instead:

* If the code links to a parameter, use `blah`.
* If the code links to a member field, use #blah.
* If the code links to a method use blah().
* If the code is somewhere unlinkable use `blah`.

Change-Id: Idac748a4c2531b5bae77e1a335e3d3ef6fab48b6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33787
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
2020-12-02 15:31:08 +00:00
Ben Clayton ed2b97811e ast: Add Module.Clone()
Deep-clones all `Node`s and `Type`s into a new module.

Instead of writing a million standalone tests that'll only ever test the
existing fields of each type, I've opted to write the tests using
wgsl<->ast<->wgsl conversion. This means the tests require the enabling
of TINT_BUILD_WGSL_READER and TINT_BUILD_WGSL_WRITER, but I believe this
is much easier to maintain.

I'm aware there are probably gaps in the tests, and that even full
coverage is likely to rapidly rot, so I've also added
fuzzers/tint_ast_clone_fuzzer.cc - a fuzzer based test that ensures that
all AST modules can be cloned with identical reproduction.

I've run this across 100 cores of a 3990x for 4 hours, fixing the
single issue it detected.

Note: Expressions do not currently clone their `TypeManager` determined
types. This is for two reasons:
(a) This initial CL is mahoosive enough.
(b) I'm uncertain whether we actually want to clone this info, or to
    re-run the `TypeDeterminer` after each AST transform. Maybe it should
    be optional. Time will tell.

Fixed: tint:307
Change-Id: Id90fab06aaa740c805d12b66f3f11d1f452c6805
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33300
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
2020-12-01 18:04:17 +00:00
Ben Clayton 03ae9a397f Cleanup: Remove unnecessary namespace prefixes
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>
2020-11-30 23:30:58 +00:00
Ben Clayton 1d8098ae94 Replace Statement::(Is|As)* with Castable
Change-Id: I5520752a4b5844be0ecac7921616893d123b246a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34315
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-11-30 23:30:58 +00:00
Ben Clayton e319d7f0e9 Derive all ast::Node from Castable
The hand-rolled `AsBlah()`, `IsBlah()` methods will be migrated in future changes.

Change-Id: I078c100b561b50018771cc38c1cac4379c393424
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34301
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-11-30 23:30:58 +00:00
Ben Clayton b053acf796 Replace use of std::unique_ptr<T> with T* for AST nodes
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>
2020-11-16 16:31:07 +00:00
Ben Clayton f8bd106041 Fix all warnings when building with CMake + clang
Change-Id: I987b4580f5f99584649b2b189369a0b962b0c3d4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31721
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
2020-11-03 17:46:49 +00:00
dan sinclair d5fd7e02ba Fixup lint errors.
This CL fixes up the various lint errors.

Change-Id: If4d3077b55aadec33980452c43917194d803fac6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31680
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
2020-11-03 16:26:09 +00:00
dan sinclair 753f40b869 Fixup build warnings
Change-Id: I7a61bd4363b01bf186e235bf76e97bd513d2fd12
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26081
Reviewed-by: Ryan Harrison <rharrison@google.com>
2020-07-30 18:18:46 +00:00
Idan Raiter a8e4e2ad5d Add BlockStatement insert and non-const global_variables
Insert will be used for adding code at the beginning of a function, and
non-const global_variables to edit decorations.

Bug: dawn:480
Change-Id: I9fbb0210a95b8488c9ce6d9a536a68f169b0cc33
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25850
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
2020-07-30 13:59:00 +00:00
dan sinclair cfdc5995ec [spirv-reader] Update to create BlockStatements
This CL updates the SPIR-V Reader to create BlockStatements instead of
StatementLists.

Bug: tint:136
Change-Id: I957019446ca00306187de701f86ae3e0dd5c5eb8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25740
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
2020-07-27 15:25:00 +00:00
dan sinclair 7c2fa1e7bc Convert CaseStatement to use BlockStatement.
This CL converts the CaseStatement class to using a BlockStatement
internally. All usages have been updated execept for the two readers.

Bug: tint:130
Change-Id: Idb9114230f7d9eb0f2208af635316edb0a0e8f99
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25721
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
2020-07-27 15:25:00 +00:00
dan sinclair 4069f3357d Convert Function to use BlockStatement.
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>
2020-07-27 15:25:00 +00:00
dan sinclair 775cb51794 [ast] Add BlockStatement
This CL adds a BlockStatement to wrap the statements in a given block.

Bug: tint:130
Change-Id: Idc2389e001d9d87ef7f45dcd8aa90bbd27ff7dce
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25606
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
2020-07-27 15:25:00 +00:00