Skip to content

Update what seems like an outdated comment #41369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Conversation

haikusw
Copy link
Contributor

@haikusw haikusw commented Feb 14, 2022

Totally minor, but I was taking a look at the source for educational purposes and noticed that it seems ASTNode evolved to be a union of a few additional types without the file comment being updated. Updated to reflect the code, if I understand it correctly.

Totally minor, but I was taking a look at the source for educational purposes and noticed that it seems ASTNode evolved to be a union of a few additional types without the file comment being updated.   Updated to reflect the code, if I understand it correctly.
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Thanks! This is my bad, I have added new members but forgot to update the comment...

@haikusw
Copy link
Contributor Author

haikusw commented Feb 15, 2022

Thanks! This is my bad, I have added new members but forgot to update the comment...

It's not a big deal but my silly brain noticed, so I figured might as well fix it.

I did have an additional related question that perhaps you know the answer to: I also noticed that the macro driven creation of the is{Type} utilities on lines 70-76:

    /// Provides some utilities to decide detailed node kind.
#define FUNC(T) bool is##T(T##Kind Kind) const;
    FUNC(Stmt)
    FUNC(Expr)
    FUNC(Decl)
    FUNC(Pattern)
#undef FUNC

has some of the new union members, but not all of them; perhaps the omitted ones aren't useful, but I thought someone more familiar with the code might want to consider this. (They obviously aren't needed currently since they aren't implemented :)).

@xedin
Copy link
Contributor

xedin commented Feb 15, 2022

@haikusw No worries! Yes, this is intentional because excluded members do have kinds like statements, expressions, decls, or patterns do i.e. isStmt(StmtKind::Return) could be used to determine whether ASTNode refers to a return statement. Conditions and case items are could be checked directly on the ASTNode.

@xedin
Copy link
Contributor

xedin commented Feb 15, 2022

@swift-ci please smoke test

@xedin
Copy link
Contributor

xedin commented Feb 17, 2022

@swift-ci please smoke test macOS platform

@xedin xedin merged commit da3856c into swiftlang:main Feb 18, 2022
@haikusw haikusw deleted the patch-1 branch February 28, 2022 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants