Skip to content
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

[2201.8.x] Revise the definition API of the TypeReferenceTypeSymbol to exclude searching for virtual symbols in the scope #42626

Open
wants to merge 1 commit into
base: 2201.8.x
Choose a base branch
from

Conversation

nipunayf
Copy link
Contributor

@nipunayf nipunayf commented Apr 24, 2024

Purpose

$title as virtual symbols are not stored in the scope.

Fixes #42454

Approach

The current approach ensures that the definition API of the type references does not search for virtual symbols in the current scope.

Remarks

Although this PR fixed the semantic API issue, the tsymbol set for narrowed type symbols remains invalid, which is being tracked separately.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@@ -103,7 +104,7 @@ public Symbol definition() {
if (referredType.tag == TypeTags.PARAMETERIZED_TYPE || bType.tag == TypeTags.PARAMETERIZED_TYPE) {
this.definition = symbolFactory.getBCompiledSymbol(((BParameterizedType) this.tSymbol.type).paramSymbol,
this.name());
} else if (referredType.tag == TypeTags.INTERSECTION) {
} else if (referredType.tag == TypeTags.INTERSECTION || referredType.tsymbol.origin == SymbolOrigin.VIRTUAL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this approach, can't we check whether readonly flag is set or not?

Copy link
Contributor Author

@nipunayf nipunayf Apr 25, 2024

Choose a reason for hiding this comment

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

It is not clear at which position I should make your change. If you are suggesting replacing the virtual check with readonly, then it breaks the existing cases.

For instance, in the following case, the definition is correct as we are looking for the symbol through its name. However, with the mentioned change, it would use the tsymbol instead as the symbol is readonly, which will produce an incorrect type definition symbol.

type Student string;

function fn() {
    Student student = "A";
}

@dulajdilshan dulajdilshan self-assigned this Apr 29, 2024
Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

@github-actions github-actions bot added the Stale label May 14, 2024
@nipunayf nipunayf removed the Stale label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants