When ImplicitOpenExistentials was enabled (default in Swift language mode 6) the Instrumenter would crash the compiler when building logger calls. This was due to an incorrect assumption that the newly created apply expr wouldn't change type when type-checked. However, the type checker is free to change the kind of expression and did so in circumstances where the call expr was wrapped in an open existential expr.
Remove this bit from function decls and closures.
Instead, for closures, infer it from the presence
of a single return or single expression AST node
in the body, which ought to be equivalent, and
automatically takes result builders into
consideration. We can also completely drop this
query from AbstractFunctionDecl, replacing it
instead with a bit on ReturnStmt.
This is phase-1 of switching from llvm::Optional to std::optional in the
next rebranch. llvm::Optional was removed from upstream LLVM, so we need
to migrate off rather soon. On Darwin, std::optional, and llvm::Optional
have the same layout, so we don't need to be as concerned about ABI
beyond the name mangling. `llvm::Optional` is only returned from one
function in
```
getStandardTypeSubst(StringRef TypeName,
bool allowConcurrencyManglings);
```
It's the return value, so it should not impact the mangling of the
function, and the layout is the same as `std::optional`, so it should be
mostly okay. This function doesn't appear to have users, and the ABI was
already broken 2 years ago for concurrency and no one seemed to notice
so this should be "okay".
I'm doing the migration incrementally so that folks working on main can
cherry-pick back to the release/5.9 branch. Once 5.9 is done and locked
away, then we can go through and finish the replacement. Since `None`
and `Optional` show up in contexts where they are not `llvm::None` and
`llvm::Optional`, I'm preparing the work now by going through and
removing the namespace unwrapping and making the `llvm` namespace
explicit. This should make it fairly mechanical to go through and
replace llvm::Optional with std::optional, and llvm::None with
std::nullopt. It's also a change that can be brought onto the
release/5.9 with minimal impact. This should be an NFC change.
Add logging of function and closure parameter values when the playground transform is enabled and its "high-performance" mode is off.
MOTIVATION
The goal of the optional "playground transform" step in Sema is to instrument the code by inserting calls to `__builtin_log()` and similar log functions, in order to record the execution of the compiled code. Some IDEs (such as Xcode) enable this transform by passing -playground and provide implementations of the logger functions that record information that can then be shown in the IDE.
The playground transform currently logs variable assignments and return statements, but it doesn't log the input parameters received by functions and closures. Knowing these values can be very useful in order to understand the behaviour of functions and closures.
CHANGES
- add a `ParameterList` parameter to `InstrumenterSupport::transformBraceStmt()`
- this is an optional parameter list that, if given, specifies the parameters that should be logged at the start of the brace statement
- this has to be passed into the function because it comes from the owner of the BraceStmt
- adjust `PlaygroundTransform.cpp` to make use of this list
- the transform will insert calls to `__builtin_log()` for each of the parameters, in order
- adjust `PCMacro.cpp` to accept the parameter, though this instrumenter doesn't currently make use of the new information
- add two new unit tests (one for functions and one for closures)
- adjust four existing unit tests to account for the new log calls
REMARKS
- this is currently guarded by the existing "high performance" option (parameter logging is omitted in that case)
rdar://104974072
Provide ASTWalker with a customization point to specify whether to
check macro arguments (which are type checked but never emitted), the
macro expansion (which is the result of applying the macro and is
actually emitted into the source), or both. Provide answers for the
~115 different ASTWalker visitors throughout the code base.
Fixes rdar://104042945, which concerns checking of effects in
macro arguments---which we shouldn't do.
Replace the use of bool and pointer returns for
`walkToXXXPre`/`walkToXXXPost`, and instead use
explicit actions such as `Action::Continue(E)`,
`Action::SkipChildren(E)`, and `Action::Stop()`.
There are also conditional variants, e.g
`Action::SkipChildrenIf`, `Action::VisitChildrenIf`,
and `Action::StopIf`.
There is still more work that can be done here, in
particular:
- SourceEntityWalker still needs to be migrated.
- Some uses of `return false` in pre-visitation
methods can likely now be replaced by
`Action::Stop`.
- We still use bool and pointer returns internally
within the ASTWalker traversal, which could likely
be improved.
But I'm leaving those as future work for now as
this patch is already large enough.
Change the various AST transforms to use prebuilt DeclName constants more heavily rather than an ad-hoc mix of Identifiers, string literals, std::strings, and StringRefs with questionable memory management.
This change PCMacro and PlaygroundTransform to return an a moduleID and
fileID in addition to the source location information. The Frontend has
been changed to run PCMacro and PlaygroundTransform on all input files
instead of the main file only.
The tests have been updated to conform to these changes with an addition
of module and file ID specific tests. The Playgrounds related tests were
adjusted to make a module out of the stub interface files since those
files should not have PCMacro and PlaygroundTransform applied to them.
rdar://problem/50821146
Replace `NameOfType foo = dyn_cast<NameOfType>(bar)` with DRY version `auto foo = dyn_cast<NameOfType>(bar)`.
The DRY auto version is by far the dominant form already used in the repo, so this PR merely brings the exceptional cases (redundant repetition form) in line with the dominant form (auto form).
See the [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es11-use-auto-to-avoid-redundant-repetition-of-type-names) for a general discussion on why to use `auto` to avoid redundant repetition of type names.
Based off the PlaygroundTransform, this new ASTWalker leaves calls to __builtin_pc_before and __builtin_pc_after before and after a user would expect a program counter to enter a range of source code.
