Skip to content

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Jul 12, 2022

Now TypeRepr is a final class in the AST, which is more or less just
a type with a location in code.

As the frontend does not provide a direct way to get a type from a
type representation, this information must be provided when fetching
the label of a type repr.

This meant:

  • removing the type repr field from EnumIsCaseExpr: this is a virtual
    AST node introduced in place of some kinds of IsEpxr. The type
    repr is still available from the ConditionalCheckedCastExpr wrapped
    by this virtual node, and we will rebuild the original IsExpr with
    the IPA layer.
  • adding some logic to get the type of keypath roots from
    KeyPathExpr. This was done to keep the TypeRepr to Type relation
    total in the DB, but goes against the design of a dumb extractor. The
    logic could be moved to QL in the future
  • in the control flow library, TypeRepr children are now ignored. As
    far as I can tell, there is no runtime evaluation going on in
    TypeReprs, so it does not make much sense to have control flow
    through them.
  • when fetching a label for a swift::ASTNode, we cannot do it for TypeRepr,
    and we just skip fetching in that case (leading to an assertion). This behaviour
    relies on the fact that we only extract swift::ASTNode in the context of
    BraceStmt, where a standalone TypeRepr is actually impossible
    We should probably decouple AstNode from what can appear in a BraceStmt.

Now `TypeRepr` is a final class in the AST, which is more or less just
a type with a location in code.

As the frontend does not provide a direct way to get a type from a
type representation, this information must be provided when fetching
the label of a type repr.

This meant:
* removing the type repr field from `EnumIsCaseExpr`: this is a virtual
  AST node introduced in place of some kinds of `IsEpxr`. The type
  repr is still available from the `ConditionalCheckedCastExpr` wrapped
  by this virtual node, and we will rebuild the original `IsExpr` with
  the IPA layer.
* some logic to get the type of keypath roots has been added to
  `KeyPathExpr`. This was done to keep the `TypeRepr` to `Type` relation
  total in the DB, but goes against the design of a dumb extractor. The
  logic could be moved to QL in the future
* in the control flow library, `TypeRepr` children are now ignored. As
  far as I can tell, there is no runtime evaluation going on in
  `TypeRepr`s, so it does not make much sense to have control flow
  through them.
@redsun82 redsun82 requested a review from a team as a code owner July 12, 2022 08:49
@github-actions github-actions bot added the Swift label Jul 12, 2022
@rdmarsh2
Copy link
Contributor

QL code looks good to me - I'd appreciate a second set of eyes on the C++, though.

redsun82 added 3 commits July 14, 2022 06:18
A type representation may not have a type in unresolved things, which
for example pop up in inactive `#if` clauses.
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ changes LGTM. One small comment.

redsun82 and others added 2 commits July 22, 2022 13:34
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
@redsun82 redsun82 merged commit fe73601 into main Jul 25, 2022
@redsun82 redsun82 deleted the redsun82/swift-type-repr-collapse branch July 25, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants