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>
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>
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>
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>
This includes a couple of position fixes in `lexer.cc`.
The source for identifiers (vars, params, etc) now refer to the identifier, not the first token of the construct.
Bug: tint:282
Change-Id: I58cb8422a4af1c7dc5f84431fd7f06b823b514c5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31444
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Use `source().range.begin.line` and `source().range.begin.column` instead.
Bug: tint:282
Change-Id: I6c9bf8766d6db2c9d8e7e1b8bafb2eea93e065d8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31441
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 moves all test files to the anonymous namespace.
Change-Id: Ie5ae0efe907c03eeb441e90f31c976844172b594
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/17900
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
This CL adds tests for the fallthrough statement.
Bug: tint:11
Change-Id: I1bc8677e00aa9eb446a096bd2600104550480d77
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/16500
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Ben Clayton
Ryan Harrison
dan sinclair