| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
| |
(not 100% NFC since it also fixes a bug by moving a line out of
a loop)
|
| |
|
|
|
|
| |
Update parsing of binary type sections to use TypeBuilder to support uses before
definitions. Now that both the binary and text parsers support out-of-order type
uses, this PR also relaxes the logic for emitting types to allow uses to be
emitted before definitions.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
singleton (#3581)
The fix here is to remove the code with
// maybe we don't need a block here?
That would remove a try's block if we thought it wasn't needed. However,
it just checked for exception branches, but not normal branches, which are
also possible.
At that location, we don't have a good way to find out if the block has other
branches to it aside from scanning its contents. So this PR just gives up on
doing so, which means we add an unnecessary block if the optimizer is not
run. If this matters we could make the binary parser more complicated by
remembering whether a block had branches in the past, but I'm not sure if
it's worth it.
|
| |
|
|
|
|
|
|
|
|
| |
We decided to change `catch_all`'s opcode from 0x05, which is the same
as `else`, to 0x19, to avoid some complicated handling in the tools.
See: https://github.com/WebAssembly/exception-handling/issues/147
lso this contains the original cpp file used to generate
dwarf_with_exceptions.wasm; instructions to generate the wasm from that
cpp file are in the comments.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I was previously mistaken about `rethrow`'s argument rule and thought
it only counted `catch`'s depth. But it turns out it follows the same
rule `delegate`'s label: the immediate argument follows the same rule as
when computing branch labels, but it only can target `try` labels
(semantically it targets that `try`'s corresponding `catch`); otherwise
it will be a validation failure. Unlike `delegate`, `rethrow`'s label
denotes not where to rethrow, but which exception to rethrow. For
example,
```wasm
try $l0
catch ($l0)
try $l1
catch ($l1)
rethrow $l0 ;; rethrow the exception caught by 'catch ($l0)'
end
end
```
Refer to this comment for the more detailed informal semantics:
https://github.com/WebAssembly/exception-handling/issues/146#issuecomment-777714491
---
This also reverts some of `delegateTarget` -> `exceptionTarget` changes
done in #3562 in the validator. Label validation rules apply differently
for `delegate` and `rethrow` for try-catch. For example, this is valid:
```wasm
try $l0
try
delegate $l0
catch ($l0)
end
```
But this is NOT valid:
```wasm
try $l0
catch ($l0)
try
delegate $l0
end
```
So `try`'s label should be used within try-catch range (not catch-end
range) for `delegate`s.
But for the `rethrow` the rule is different. For example, this is valid:
```wasm
try $l0
catch ($l0)
rethrow $l0
end
```
But this is NOT valid:
```wasm
try $l0
rethrow $l0
catch ($l0)
end
```
So the `try`'s label should be used within catch-end range instead.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
So far `Try`'s label is only targetted by `delegate`s, but it turns out
`rethrow` also has to follow the same rule as `delegate` so it needs to
target a `Try` label. So this renames variables like
`delegateTargetNames` to `exceptionTargetNames` and methods like
`replaceDelegateTargets` to `replaceExceptionTargets`.
I considered `tryTarget`, but the branch/block counterpart name we use
is not `blockTarget` but `branchTarget`, so I chose `exceptionTarget`.
The patch that fixes `rethrow`'s target will follow; this is the
preparation for that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds support for reading/writing of the new `delegate` instruction
in the folded wast format, the stack IR format, the poppy IR format, and
the binary format in Binaryen. We don't have a formal spec written down
yet, but please refer to WebAssembly/exception-handling#137 and
WebAssembly/exception-handling#146 for the informal semantics. In the
current version of spec `delegate` is basically a rethrow, but with
branch-like immediate argument so that it can bypass other
catches/delegates in between.
`delegate` is not represented as a new `Expression`, but it is rather
an option within a `Try` class, like `catch`/`catch_all`.
One special thing about `delegate` is, even though it is written
_within_ a `try` in the folded wat format, like
```wasm
(try
(do
...
)
(delegate $l)
)
```
In the unfolded wat format or in the binary format, `delegate` serves as
a scope end instruction so there is no separate `end`:
```wasm
try
...
delegate $l
```
`delegate` semantically targets an outer `catch` or `delegate`, but we
write `delegate` target as a `try` label because we only give labels to
block-like scoping expressions. So far we have not given `Try` a label
and used inner blocks or a wrapping block in case a branch targets the
`try`. But in case of `delegate`, it can syntactically only target `try`
and if it targets blocks or loops it is a validation failure.
So after discussions in #3497, we give `Try` a label but this label can
only be targeted by `delegate`s. Unfortunately this makes parsing and
writing of `Try` expression somewhat complicated. Also there is one
special case; if the immediate argument of `try` is the same as the
depth of control flow stack, this means the 'delegate' delegates to the
caller. To handle this case this adds a fake label
`DELEGATE_CALLER_TARGET`, and when writing it back to the wast format
writes it as an immediate value, unlike other cases in which we write
labels.
This uses `DELEGATE_FIELD_SCOPE_NAME_DEF/USE` to represent `try`'s label
and `delegate`'s target. There are many cases that `try` and
`delegate`'s labels need to be treated in the same way as block and
branch labels, such as for hashing or comparing. But there are routines
in which we automatically assume all label uses are branches. I thought
about adding a new kind of defines such as
`DELEGATE_FIELD_TRY_NAME_DEF/USE`, but I think it will also involve some
duplication of existing routines or classes. So at the moment this PR
chooses to use the existing `DELEGATE_FIELD_SCOPE_NAME_DEF/USE` for
`try` and `delegate` labels and makes only necessary amount of changes
in branch-utils. We can revisit this decision later if necessary.
Many of changes to the existing test cases are because now all `try`s
are automatically assigned a label. They will be removed in
`RemoveUnusedNames` pass in the same way as block labels if not targeted
by any delegates.
This only supports reading and writing and has not been tested against
any optimization passes yet.
---
Original unfolded wat file to generate test/try-delegate.wasm:
```wasm
(module
(event $e)
(func
try
try
delegate 0
catch $e
end)
(func
try
try
catch $e
i32.const 0
drop
try
delegate 1
end
catch $e
end
)
)
```
|
| |
|
| |
Adds support for modules with multiple tables. Adds a field for the table name to `CallIndirect` and updates the C/JS APIs accordingly.
|
| |
|
|
|
|
|
| |
Unordered maps will hash the pointer, while ordered ones will compare the
strings to find where to insert in the tree. I cannot confirm a speedup in time
from this, though others can, but I do see a consistent improvement of a
few % in perf stat results like number of instructions and cycles (and those
results have little noise). And it seems logical that this could be faster.
|
| |
|
|
|
|
|
|
| |
As proposed in https://github.com/WebAssembly/simd/pull/395. Note that the other
instructions in the proposal have not been implemented in LLVM or in V8, so
there is no need to implement them in Binaryen right now either. This PR
introduces a new expression class for the new instructions because they uniquely
take an immediate argument identifying which portion of the input vector to
widen.
|
| |
|
|
|
|
|
|
|
|
|
| |
This is only partial support, as br_on_null also has an extra optional
value in the spec. Implementing that is cumbersome in binaryen, and
there is ongoing spec discussions about it (see
https://github.com/WebAssembly/function-references/issues/45 ), so
for now we only support the simple case without the default value.
Also fix prefixed opcodes to be LEBs in RefAs, which was noticed here
as the change here made it noticeable whether the values were int8 or
LEBs.
|
| |
|
|
|
|
| |
This is different than the other RefAs variants in that it is part of the
typed functions proposal, and not GC. But it is part of GC prototype 3.
Note: This is not useful to us yet as we don't support non-nullable types.
|
| |
|
|
|
|
|
|
| |
This expands the existing BrOnCast into BrOn that can also handle the
func/data/i31 variants. This is not as elegant as RefIs / RefAs in that BrOnCast
has an extra rtt field, but I think it is still the best option. We already have optional
fields on Break (the value and condition), so making rtt optional is not odd. And
it allows us to share all the behavior of br_on_* which aside from the cast or the
check itself, is identical - returning the value if the branch is not taken, etc.
|
| |
|
|
|
|
|
| |
As a result, we cannot handle a br_on_cast with an unreachable RTT. The
binary format solves the problem by ignoring unreachable code, and this makes
the text format do the same.
A nice benefit of this is that we can remove the castType extra field.
|
| |
|
|
|
|
| |
wasm-finalize currently makes byte-wise copies of section data in the
user and data sections. If the section is large, that's extraordinarily
expensive. With a memcpy instead I see a speedup of 1.6 for a large
wasm binary with DWARF data.
|
| |
|
|
|
|
|
|
| |
These are similar to is, but instead of returning an i32 answer, they trap on
an invalid value, and return it otherwise.
These could in theory be in a single RefDoThing, with opcodes for both As
and Is, but as the return values are different, that would be a little odd, and
the name would be less clear.
|
| | |
|
| |
|
|
|
|
|
|
| |
This internal refactoring prepares us for ref.is_func/data/i31, by renaming
the node and adding an "op" field. For now that field must always be "Null"
which means it is a ref.is_null.
This adjusts the C API to match the new IR shape. The high-level JS API
is unchanged.
|
| |
|
|
|
|
|
|
| |
We now have multiple catches in each try, and a possible catch-all.
This changes our "extra delimiter" storage to store either an "else"
(unchanged from before) or an arbitrary list of things - we use that
for catches.
|
| |
|
|
|
|
|
|
|
|
| |
The binary spec
(https://docs.google.com/document/d/1yAWU3dbs8kUa_wcnnirDxUu9nEBsNfq0Xo90OWx6yuo/edit#)
lists `dataref` after `i31ref`, and `dataref` also comes after `i31ref`
in its binary code in the value-increasing order. This reorders these
two in wasm-type.h and other places, although in most of those places
the order is irrelevant.
This also adds C and JS API for `dataref`.
|
| |
|
|
|
| |
Some fields were removed, see
https://docs.google.com/document/d/1yAWU3dbs8kUa_wcnnirDxUu9nEBsNfq0Xo90OWx6yuo/edit#
|
| |
|
| |
This removes `exnref` type and `br_on_exn` instruction.
|
| |
|
|
|
| |
This is not 100% of everything, but is enough to get tests passing, which
includes full binary and text format support, getting all switches to compile
without error, and some additions to InstrumentLocals.
|
| |
|
|
| |
As proposed in https://github.com/WebAssembly/simd/pull/383, with opcodes
coordinated with the WIP V8 prototype.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
For now we don't support non-nullability, and can therefore lower a let into
simpler things. That is,
(let $x = ...
;;
)
=>
(block
$x = ...
;;
)
This lets us handle wasm binaries with let, so that we can optimize them
(with the current downside of losing non-nullability).
This is still not trivial to do, sadly, because the indexing of lets is somewhat
odd in the binary. A let modifies the indexes of other things declared before it,
which means that index "0" means different things at different times. And this
is trickier for us because we add more locals as needed for tuples and stacky
code. So this PR makes us track the absolute local indexes from which each
let started to allocate its locals.
The binary testcase was created from this wat using wasp:
(module
(type $vector (array (field (mut f64))))
(func $main
(local $x i32)
(local $y i32)
(drop (local.get $x)) ;; 0 is the index appearing in the binary
;; first let
(array.new_with_rtt $vector
(f64.const 3.14159)
(i32.const 1)
(rtt.canon $vector)
)
(let (local $v (ref $vector))
(drop (local.get $v)) ;; 0
(drop (local.get $x)) ;; 1
;; another one, nested
(array.new_with_rtt $vector
(f64.const 1234)
(i32.const 2)
(rtt.canon $vector)
)
(let (local $w (ref $vector))
(drop (local.get $v)) ;; 1
(drop (local.get $w)) ;; 0
(drop (local.get $x)) ;; 2
)
)
;; another one, later
(array.new_with_rtt $vector
(f64.const 2.1828)
(i32.const 3)
(rtt.canon $vector)
)
(let (local $v (ref $vector))
(drop (local.get $v)) ;; 0
(drop (local.get $x)) ;; 1
)
(drop (local.get $x)) ;; 0
)
)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This updates `try`-`catch`-`catch_all` and `rethrow` instructions to
match the new spec. `delegate` is not included. Now `Try` contains not a
single `catchBody` expression but a vector of catch
bodies and events.
This updates most existing routines, optimizations, and tests modulo the
interpreter and the CFG traversal. Because the interpreter has not been
updated yet, the EH spec test is temporarily disabled in check.py. Also,
because the CFG traversal for EH is not yet updated, several EH tests in
`rse_all-features.wast`, which uses CFG traversal, are temporarily
commented out.
Also added a few more tests in existing EH test functions in
test/passes. In the previous spec, `catch` was catching all exceptions
so it was assumed that anything `try` body throws is caught by its
`catch`, but now we can assume the same only if there is a `catch_all`.
Newly added tests test cases when there is a `catch_all` and cases there
are only `catch`es separately.
|
| |
|
|
|
|
|
|
|
| |
Noticed by comparing to V8 and Wasp. After this things are almost identical,
but there is also at least https://bugs.chromium.org/p/v8/issues/detail?id=11300
Test updates are due to having an instruction with two operands of which one is
unreachable. The new order puts the non-unreachable first, so it is not removed by
round-tripping through the binary format like before (which removes all unreachable
code).
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This lets us parse (ref null i31) and (ref i31) and not just i31ref.
It also fixes the parsing of i31ref, making it nullable for now, which we
need to do until we support non-nullability.
Fix some internal handling of i31 where we had just i31ref (which meant we
just handled the non-nullable type).
After fixing a bug in printing (where we didn't print out (ref null i31)
properly), I found some a simplification, to remove TypeName.
|
| |
|
|
| |
As proposed in https://github.com/WebAssembly/simd/pull/352, using the opcodes
used in the LLVM and V8 implementations.
|
| |
|
|
|
|
| |
As proposed in https://github.com/WebAssembly/simd/pull/380, using the opcodes
used in LLVM and V8. Since these opcodes overlap with the opcodes of
i64x2.all_true and i64x2.any_true, which have long since been removed from the
SIMD proposal, this PR also removes those instructions.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The tricky part here, as pointed out by aheejin in my previous attempt, is that
we need to know the type of the value we send if the branch is taken. We can
normally calculate that from the rtt parameter's type - we are casting to that
RTT, so we know what type that is - but if the rtt is unreachable, that's a problem.
To fix that, store the cast type on BrOnCast instructions.
This includes a test with a br_on_cast that succeeds and sends the cast value,
one that fails and passes through the uncast value, and also of one with an
unreachable RTT.
This includes a fix for Precompute, as noticed by that new test. If a break is
taken, with a ref as a value, we can't precompute it - for the same reasons
we can't precompute a ref in general, that it is a pointer to possibly shared
data.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is almost NFC, but it may emit slightly different IR in cases that
don't matter much. Specifically,
(block (result i32) ;; can also be unreachable
(unreachable)
(i32.const 1)
)
That can be finalized to have type unreachable or i32, as both are
valid. After this PR we should consistently do the same thing in all
places. (Either option would be ok - we prefer to keep the type if
there is one.)
In practice, DCE will remove all the dead code anyhow, leaving no
difference to matter. However, the IR is different without DCE, and
that may be noticeable in an unoptimized build - but it should have
no effect on behavior, just on the binary.
|
| |
|
|
|
| |
Previously we were using bools for both of these concepts, but using enums makes
the code clearer. In particular, the PR removes many instances of
`/*nullability=*/ true`.
|
| |
|
|
|
|
| |
- i64x2.eq (https://github.com/WebAssembly/simd/pull/381)
- i64x2 widens (https://github.com/WebAssembly/simd/pull/290)
- i64x2.bitmask (https://github.com/WebAssembly/simd/pull/368)
- signselect ops (https://github.com/WebAssembly/simd/pull/124)
|
| |
|
|
| |
This adds enough to read and write them and test that, but leaves
interpreter support for later.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
array.new/get/set/len - pretty straightforward after structs and all the
infrastructure for them.
Also fixes validation of the unnecessary heapType param in the
text and binary formats in structs as well as arrays.
Fixes printing of packed types in type names, which emitted i32
for them. That broke when we emitted the same name for an array
of i8 and i32 as in the new testing here.
Also fix a bug in Field::operator< which was wrong for packed
types; again, this was easy to notice with the new testing.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
With struct.new read/write support, we can start to do interesting
things! This adds a test of creating a struct and seeing that references
behave like references, that is, if we write to the value X refers to, and
if Y refers to the same thing, when reading from Y's value we see the
change as well.
The test is run through all of -O1, which uncovered a minor issue in
Precompute: We can't try to precompute a reference type, as we can't
replace a reference with a value.
Note btw that the test shows the optimizer properly running
CoalesceLocals on reference types, merging two locals.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds rtt.canon and rtt.sub together with RTT type support
that is necessary for them. Together this lets us test roundtripping the
instructions and types.
Also fixes a missing traversal over globals in collectHeapTypes,
which the example from the GC docs requires, as the RTTs are in
globals there.
This does not yet add full interpreter support and other things. It
disables initial contents on GC in the fuzzer, to avoid the fuzzer
breaking.
Renames the binary ID for exnref, which is being removed from
the spec, and which overlaps with the binary ID for rtt.
|
| |
|
|
|
|
|
|
|
| |
Interns HeapTypes using the same patterns and utilities already used to intern
Types. This allows HeapTypes to efficiently be compared for equality and hashed,
which may be important for very large struct types in the future. This change
also has the benefit of increasing symmetry between the APIs of Type and
HeapType, which will make the developer experience more consistent. Finally,
this change will make TypeBuilder (#3418) much simpler because it will no longer
have to introduce TypeInfo variants to refer to HeapTypes indirectly.
|
| |
|
|
|
|
|
|
|
|
| |
Mostly straightforward after struct.get.
This renames the value field in struct.get to ref. I think this makes
more sense because struct.set has both a reference to a thing, and a
value to set onto that thing. So calling the former ref seems more
consistent, giving us ref, value. This mirrors load/store for example
where we use ptr, value, and ref is playing the role of ptr here
basically.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is the first instruction that uses a GC Struct or Array, so it's where
we start to actually need support in the interpreter for those values, which
is added here.
GC data is modeled as a gcData field on a Literal, which is just a
Literals. That is, both a struct and an array are represented as an
array of values. The type which is alongside would indicate if it's a
struct or an array. Note that the data is referred to using a shared_ptr
so it should "just work", but we'll only be able to really test that once we
add struct.new and so can verify that references are by reference and
not value, etc.
As the first instruction to care about i8/16 types (which are only possible
in a Struct or Array) this adds support for parsing and emitting them.
This PR includes fuzz fixes for some minor things the fuzzer found, including
some bad printing of not having ResultTypeName in necessary places
(found by the text format roundtripping fuzzer).
|
| |
|
|
|
|
| |
This adds support in the text and binary format handling, which allows us
to have a full test of reading and writing the types.
This also adds a "name" field to struct fields, which the text format supports.
|
| |
|
|
|
|
|
|
|
| |
This will allow writing GC types in the future, which are non-signature
heap types.
To allow this PR to work, it adds operator< for HeapType so that it
can be used in the data structures that collect uses.
Drive-by fix of a weird hack with sending a Name* in Print.
|
| |
|
|
|
|
|
|
|
|
| |
Defined types in wasm are really one of the "heap types": a signature type, or
(with GC) a struct or an array type. This refactors the binary and text parsers
to load the defined types into an array of heap types, so that we can start to
parse GC types. This replaces the existing array of signature types (which
could not support a struct or an array).
Locally this PR can parse and print as text simple GC types. For that it was
necessary to also fix Type::getFeatures for GC.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
bugs (#3401)
* Count signatures in tuple locals.
* Count nested signature types (confirming @aheejin was right, that was missing).
* Inlining was using the wrong type.
* OptimizeInstructions should return -1 for unhandled types, not error.
* The fuzzer should check for ref types as well, not just typed function references,
similar to what GC does.
* The fuzzer now creates a function if it has no other option for creating a constant
expression of a function type, then does a ref.func of that.
* Handle unreachability in call_ref binary reading.
* S-expression parsing fixes in more places, and add a tiny fuzzer for it.
* Switch fuzzer test to just have the metrics, and not print all the fuzz output which
changes a lot. Also fix noprint handling which only worked on binaries before.
* Fix Properties::getLiteral() to use the specific function type properly, and make
Literal's function constructor require that, to prevent future bugs.
* Turn all input types into nullable types, for now.
|
| |
|
|
|
|
|
|
| |
Includes minimal support in various passes. Also includes actual optimization
work in Directize, which was easy to add.
Almost has fuzzer support, but the actual makeCallRef is just a stub so far.
Includes s-parser support for parsing typed function references types.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
types (#3388)
This adds the new feature and starts to use the new types where relevant. We
use them even without the feature being enabled, as we don't know the features
during wasm loading - but the hope is that given the type is a subtype, it should
all work out. In practice, if you print out the internal type you may see a typed
function reference-specific type for a ref.func for example, instead of a generic
funcref, but it should not affect anything else.
This PR does not support non-nullable types, that is, everything is nullable
for now. As suggested by @tlively this is simpler for now and leaves nullability
for later work (which will apparently require let or something else, and many
passes may need to be changed).
To allow this PR to work, we need to provide a type on creating a RefFunc. The
wasm-builder.h internal API is updated for this, as are the C and JS APIs,
which are breaking changes. cc @dcodeIO
We must also write and read function types properly. This PR improves
collectSignatures to find all the types, and also to sort them by the
dependencies between them (as we can't emit X in the binary if it depends
on Y, and Y has not been emitted - we need to give Y's index). This sorting
ends up changing a few test outputs.
InstrumentLocals support for printing function types that are not funcref
is disabled for now, until we figure out how to make that work and/or
decide if it's important enough to work on.
The fuzzer has various fixes to emit valid types for things (mostly
whitespace there). Also two drive-by fixes to call makeTrivial where it
should be (when we fail to create a specific node, we can't just try to make
another node, in theory it could infinitely recurse).
Binary writing changes here to replace calls to a standalone function to
write out a type with one that is called on the binary writer object itself,
which maintains a mapping of type indexes (getFunctionSignatureByIndex).
|
| |
|
|
|
|
|
|
|
| |
When Functions, Globals, Events, and Exports are added to a module, if they are
not already in std::unique_ptrs, they are wrapped in a new std::unique_ptr owned
by the Module. This adds an extra layer of indirection when accessing those
elements that can be avoided by allocating those elements as std::unique_ptrs.
This PR updates wasm-builder to allocate module elements via std::make_unique
rather than `new`. In the future, we should remove the raw pointer versions of
Module::add* to encourage using std::unique_ptrs more broadly.
|
| | |
|
