Index expressible-by-literal expressions. #60432

allevato · 2022-08-06T07:50:10Z

When a value is initialized or coerced for a type that conforms to one of the ExpressibleBy*Literal protocols (or ExpressibleByStringInterpolation), this change records an implicit call to the corresponding init(...Literal:) in the indexstore, located at the beginning of the literal.

allevato · 2022-08-06T07:50:53Z

@swift-ci please test

bnbarham · 2022-08-09T20:23:21Z

lib/Index/Index.cpp

@@ -1691,6 +1708,25 @@ bool IndexSwiftASTWalker::initFuncDeclIndexSymbol(FuncDecl *D,
 return false;
 }

+bool IndexSwiftASTWalker::initLiteralInitRefIndexSymbol(ValueDecl *D,


This is almost initFuncRefIndexSymbol - is ide::isBeingCalled false and thus causing it to return early?

Yes, isBeingCalled is only checking for (Self)ApplyExpr. Should I update it to support LiteralExprs with non-null initializers?

And if I do that, should I update Expr->getReferencedDecl to return the initializer for subclasses of LiteralExpr? Right now they're all using the NO_REFERENCE macro to return null unconditionally.

That seems reasonable to me. That combined with SemaAnnotator walking this should mean that IndexSwiftASTWalker will work as is, I believe.

Ok, I think I've got this rewritten the way you described now; PTAL!

bnbarham · 2022-08-09T20:38:43Z

lib/Index/Index.cpp

@@ -698,12 +698,27 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
 }
 }

+ void handleLiteralInitRefs(Expr *E) {


I assume this isn't being walked by SemaAnnotator because it's implicit? Feels odd to me that visitDeclReference takes a ReferenceMetaData which has IsImplicit and some expressions we walk regardless, but not others. Though that bit looks to have been added a bit later on. Perhaps we should walk LiteralExpr despite being implicit as well?

Any thoughts @benlangmuir?

Seem reasonable to me, I don't recall if there's any reason for the current behaviour other than historical.

Fair enough, I'd personally prefer doing this in SemaAnnotator then - IMO IndexSwiftASTWalker should be much simpler than it is right now, with all the actual walking done in SemaAnnotator instead.

This function is now removed; the changes above made this work without special handling here.

rintaro · 2022-08-09T20:45:28Z

Could you add test cases for SE-0213 Literal initialization via coercion? i.e. CustomInteger(42) implicitly calls CustomInteger.init(integerLiteral:)

rintaro · 2022-08-09T20:55:05Z

Also does it make sense to do similar to ExpressibleByArrayLiteral/ExpressibleByDictionaryLiteral too?

bnbarham · 2022-08-11T22:48:49Z

Also does it make sense to do similar to ExpressibleByArrayLiteral/ExpressibleByDictionaryLiteral too?

There's actually an open bug for this as well - #56165. So @ileitch may be interested in this PR.

When a value is initialized or coerced for a type that conforms to one of the `ExpressibleBy*Literal` protocols (or `ExpressibleByStringInterpolation`), this change records an implicit call to the corresponding `init(...Literal:)` in the indexstore, located at the beginning of the literal.

allevato · 2022-08-28T03:09:34Z

Also does it make sense to do similar to ExpressibleByArrayLiteral/ExpressibleByDictionaryLiteral too?

There's actually an open bug for this as well - #56165. So @ileitch may be interested in this PR.

I've added these two conformances as well now—thanks for mentioning them!

allevato · 2022-08-28T03:09:42Z

@swift-ci please test

allevato · 2022-08-28T16:18:40Z

Could you add test cases for SE-0213 Literal initialization via coercion? i.e. CustomInteger(42) implicitly calls CustomInteger.init(integerLiteral:)

Forgot to mention, I also added tests for this. An expression like CustomInteger(42) indexes as an implicit call to CustomInteger(integerLiteral:) at the location of the 42 token due to the way the AST transforms the expression. That might seem a little odd since it looks like an explicit call, but since it's an accurate representation of what's happening here at the language level, I don't know if it's worth treating differently.

bnbarham · 2022-08-30T00:21:22Z

Forgot to mention, I also added tests for this. An expression like CustomInteger(42) indexes as an implicit call to CustomInteger(integerLiteral:) at the location of the 42 token due to the way the AST transforms the expression. That might seem a little odd since it looks like an explicit call, but since it's an accurate representation of what's happening here at the language level, I don't know if it's worth treating differently.

Seems reasonable for now, we can always change later if necessary.

bnbarham

Thanks Tony, LGTM!

bnbarham · 2022-08-30T00:32:13Z

lib/AST/Expr.cpp

+ SIMPLE_REFERENCE(NilLiteral, getInitializer);
+ SIMPLE_REFERENCE(IntegerLiteral, getInitializer);
+ SIMPLE_REFERENCE(FloatLiteral, getInitializer);
+ SIMPLE_REFERENCE(BooleanLiteral, getInitializer);
+ SIMPLE_REFERENCE(StringLiteral, getInitializer);
+ SIMPLE_REFERENCE(InterpolatedStringLiteral, getInitializer);


Does this change cursor info at all? Could add some tests for that (though I'm not sure anyone would think to jump to definition on a literal 😅). It would also be a little weird for eg. the collection cases since I (think) only the first character (eg. [) would actually return a result.

It looks like it doesn't; a quick check adding one of these to SourceKit/CursorInfo/cursor_info_expressions came back with <empty cursor info; internal diagnostic: "Resolved to incomplete expression or statement."> at the literal's location.

allevato requested a review from bnbarham August 6, 2022 07:50

bnbarham reviewed Aug 9, 2022

View reviewed changes

bnbarham requested a review from benlangmuir August 9, 2022 20:39

allevato force-pushed the index-expressible-by-literal branch 2 times, most recently from df687ca to 6e51655 Compare August 28, 2022 03:05

allevato force-pushed the index-expressible-by-literal branch from 6e51655 to 31d1b3a Compare August 28, 2022 03:06

bnbarham approved these changes Aug 30, 2022

View reviewed changes

allevato merged commit 9c84149 into swiftlang:main Aug 30, 2022

allevato deleted the index-expressible-by-literal branch August 30, 2022 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index expressible-by-literal expressions. #60432

Index expressible-by-literal expressions. #60432

allevato commented Aug 6, 2022

allevato commented Aug 6, 2022

bnbarham Aug 9, 2022

allevato Aug 19, 2022

allevato Aug 19, 2022

bnbarham Aug 22, 2022

allevato Aug 28, 2022

bnbarham Aug 9, 2022

benlangmuir Aug 9, 2022

bnbarham Aug 9, 2022

allevato Aug 28, 2022

rintaro commented Aug 9, 2022 •

edited

Loading

rintaro commented Aug 9, 2022

bnbarham commented Aug 11, 2022

allevato commented Aug 28, 2022

allevato commented Aug 28, 2022

allevato commented Aug 28, 2022

bnbarham commented Aug 30, 2022

bnbarham left a comment

bnbarham Aug 30, 2022

allevato Aug 30, 2022

Index expressible-by-literal expressions. #60432

Index expressible-by-literal expressions. #60432

Conversation

allevato commented Aug 6, 2022

allevato commented Aug 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rintaro commented Aug 9, 2022 • edited Loading

rintaro commented Aug 9, 2022

bnbarham commented Aug 11, 2022

allevato commented Aug 28, 2022

allevato commented Aug 28, 2022

allevato commented Aug 28, 2022

bnbarham commented Aug 30, 2022

bnbarham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rintaro commented Aug 9, 2022 •

edited

Loading