Skip to content

don't warn/error symbols in semGenericStmt/templates #24907

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 9 commits into from
Apr 29, 2025
Merged

don't warn/error symbols in semGenericStmt/templates #24907

merged 9 commits into from
Apr 29, 2025

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Apr 23, 2025

fixes #24905
fixes #24903
fixes #11805
fixes #15650

In the first phase of generic checking, we cannot warn/error symbols because they can belong a false branch of when or there is a push/pop options using open symbols. So we cannot decide whether to warn/error or not

@@ -72,7 +72,7 @@ proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule;
incl(s.flags, sfUsed)
markOwnerModuleAsUsed(c, s)
else:
markUsed(c, info, s)
markUsed(c, info, s, inGenerics = inGenerics)
Copy link
Collaborator

@metagn metagn Apr 23, 2025

Choose a reason for hiding this comment

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

Instead of modifying markUsed, just triggering the if branch above (i.e. if isField or inGenerics: rather than if isField:) should be enough.

Also this should apply to templates as well as generics. Another option might be to make the top if branch the default behavior for symChoice and call markUsed manually when the produced node is an nkSym. The only place where this isn't already done and isn't in generics/templates seems to be semexprs.semSymGenericInstantiation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to detect it early but then the when false case would still be a problem, especially with --styleCheck:error. Depends whichever behavior is less annoying.

@ringabout ringabout changed the title don't warn/error symbols in semGenericStmt don't warn/error symbols in semGenericStmt/templates Apr 23, 2025
@Araq Araq merged commit 0506d5b into devel Apr 29, 2025
18 checks passed
@Araq Araq deleted the pr_depre branch April 29, 2025 09:08
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 0506d5b

Hint: mm: orc; opt: speed; options: -d:release
179421 lines; 9.055s; 651.219MiB peakmem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants