Skip to content

Conversation

@slavapestov
Copy link
Contributor

I have some leftover patches from my work on ASTScope. A couple of places that create an ASTContext without a CompilerInvocation still had parser lookup enabled; swift-ide-test is one of them.

@slavapestov slavapestov requested a review from rintaro November 13, 2020 22:25
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

Comment on lines 404 to 407
Copy link
Member

@rintaro rintaro Nov 14, 2020

Choose a reason for hiding this comment

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

I think we can just return result here

Comment on lines 324 to 325
Copy link
Member

Choose a reason for hiding this comment

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

How about

if (WhereLoc.isInvalid())
  return SourceRange();
if (Requirements.empty())
  return WhereLoc;

We have two ways of knowing if we're inside of an inactive #if clause.
Refactor the only two places that called getScopeInfo().isInactiveConfigBlock()
to check InInactiveClauseEnvironment instead.

This removes the last remaining usage of Scope that's not related to
parse-time name lookup.
Also, store the end location of the where clause explicitly, so that
we can recover it even if there are no requirements.

This fixes one of the failing tests when parser lookup is disabled in
swift-ide-test by ensuring that the source range of the function
extends to the end of the 'where' clause, even though the 'where'
clause has a code completion token in it.
@slavapestov slavapestov force-pushed the swift-ide-test-parser-lookup branch from ca1e4a6 to 74e72a6 Compare November 16, 2020 21:53
@slavapestov
Copy link
Contributor Author

Thanks for the review, @rintaro!

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov merged commit 65bd39f into swiftlang:main Nov 17, 2020
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