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

Fix some styleCheck bugs #20095

Merged
merged 1 commit into from
May 6, 2023
Merged

Conversation

quantimnot
Copy link
Contributor

@quantimnot quantimnot commented Jul 26, 2022

refs #19822

Fixes these bugs (look at the added tests for details):

  • Style check violations in generics defined in foreign packages are raised.
  • Builtin pragma usage style check violations in foreign packages are raised.
  • User pragma definition style check violations are not raised.
  • Inconsistent usage style of foreign symbols within a module are not raised. #19822#issuecomment-1227125459
    (This is still an issue, but my approach isn't accepted, so it needs addressed later.)

@quantimnot quantimnot changed the title Fix some styleCheck bugs [skip ci] Fix some styleCheck bugs Jul 26, 2022
compiler/semdata.nim Outdated Show resolved Hide resolved
@Varriount
Copy link
Contributor

@Araq Is this ok to merge?

@Araq
Copy link
Member

Araq commented Sep 22, 2022

@Varriount No.

@Varriount Varriount added the Requires Araq To Merge PR should only be merged by Araq label Oct 5, 2022
refs nim-lang#19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.
@Araq Araq merged commit 365a753 into nim-lang:devel May 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

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

Hint: mm: orc; opt: speed; options: -d:release
166996 lines; 8.370s; 612.391MiB peakmem

@quantimnot quantimnot deleted the pr_stylecheck_bugs branch May 6, 2023 21:14
capocasa pushed a commit to capocasa/Nim that referenced this pull request May 15, 2023
refs nim-lang#19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
capocasa pushed a commit to capocasa/Nim that referenced this pull request May 16, 2023
refs nim-lang#19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
narimiran pushed a commit that referenced this pull request Jun 15, 2023
refs #19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
(cherry picked from commit 365a753)
narimiran pushed a commit that referenced this pull request Jun 15, 2023
refs #19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
(cherry picked from commit 365a753)
narimiran pushed a commit that referenced this pull request Jun 15, 2023
refs #19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
(cherry picked from commit 365a753)
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
refs nim-lang#19822

Fixes these bugs:

* Style check violations in generics defined in foreign packages are raised.
* Builtin pragma usage style check violations in foreign packages are raised.
* User pragma definition style check violations are not raised.

Co-authored-by: quantimnot <[email protected]>
Araq pushed a commit that referenced this pull request Oct 11, 2024
fixes #24269, refs #20095

Instead of checking the package of the *used sym* to determine whether a
stylecheck should trigger, we check the package of the lineinfo instead.
Before #20095 this checked for the current compilation context module
instead which caused issues with generic procs, but the lineinfo should
more closely match the AST.

I figured this might cause issues with includes etc but the foreign
package test specifically tests for an include and passes, so maybe the
package determining logic accounts for this already. This still might
not be the correct logic, I'm not too familiar with the package handling
in the compiler.

Package PRs, both merged:

- json_rpc: status-im/nim-json-rpc#226
- json_serialization:
status-im/nim-json-serialization#99
narimiran pushed a commit that referenced this pull request Oct 11, 2024
fixes #24269, refs #20095

Instead of checking the package of the *used sym* to determine whether a
stylecheck should trigger, we check the package of the lineinfo instead.
Before #20095 this checked for the current compilation context module
instead which caused issues with generic procs, but the lineinfo should
more closely match the AST.

I figured this might cause issues with includes etc but the foreign
package test specifically tests for an include and passes, so maybe the
package determining logic accounts for this already. This still might
not be the correct logic, I'm not too familiar with the package handling
in the compiler.

Package PRs, both merged:

- json_rpc: status-im/nim-json-rpc#226
- json_serialization:
status-im/nim-json-serialization#99

(cherry picked from commit aaf6c40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires Araq To Merge PR should only be merged by Araq
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants