Skip to content

AST: Rename VarDecl::getType() to VarDecl::getTypeInContext() #67739

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 1 commit into from
Aug 5, 2023

Conversation

slavapestov
Copy link
Contributor

This is a futile attempt to discourage future use of getType() by giving it a "scary" name.

We want people to use getInterfaceType() like with the other decl kinds.

@slavapestov
Copy link
Contributor Author

#67737
swiftlang/llvm-project#7150
@swift-ci Please smoke test

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Cool!

Comment on lines 149 to +152
llvm::SmallVector<const ParamDecl *, 8> silParamMapping;
for (auto param : *fd->getParameters()) {
if (auto *tuple =
param->getType()->getDesugaredType()->getAs<TupleType>()) {
param->getInterfaceType()->getAs<TupleType>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected this won't handle the following? e.g

protocol P {
  associatedtype X
}
extension P where X == (Int, Int) {
  func foo(_ x: X) {}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this is a problem with anything that looks at the interface type of a parameter. I think type resolution should just map generic parameters to concrete types in this case since it's a systemic issue throughout the compiler.

However this particular check looks like some kind of hack already?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that makes sense, just wanted to make sure the change was intentional

@@ -2268,7 +2268,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
// NOTE: We do this here instead of TypeCheckAttr since types are not
// completely type checked at that point.
if (auto attr = VD->getAttrs().getAttribute<NoImplicitCopyAttr>()) {
if (auto *nom = VD->getType()->getCanonicalType()->getNominalOrBoundGenericNominal()) {
if (auto *nom = VD->getInterfaceType()->getNominalOrBoundGenericNominal()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question

Comment on lines 3967 to +3968
if (auto attr = param->getAttrs().getAttribute<NoImplicitCopyAttr>()) {
if (auto *nom = param->getType()->getCanonicalType()->getNominalOrBoundGenericNominal()) {
if (auto *nom = param->getInterfaceType()->getNominalOrBoundGenericNominal()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question

This is a futile attempt to discourage future use of getType() by
giving it a "scary" name.

We want people to use getInterfaceType() like with the other decl kinds.
@slavapestov slavapestov force-pushed the var-decl-type-in-context branch from 2b6b663 to 9ebb5f2 Compare August 4, 2023 18:19
@slavapestov
Copy link
Contributor Author

swiftlang/llvm-project#7150
@swift-ci Please smoke test

@slavapestov slavapestov merged commit 6ae2a1d into swiftlang:main Aug 5, 2023
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.

2 participants