Skip to content

Conversation

minestarks
Copy link
Member

Fixes

  • Fixed package exports - Export items weren't being generated in HIR for non-aliased exports, breaking libraries like FixedPoint
  • Fixed relative namespace imports - import Bar from within namespace Foo now correctly resolves to Foo.Bar, matching the behavior of open Bar
  • Fixed namespace reexports from dependency packages - Namespace aliases exported from a dependency's Main namespace (like export A as Alias) are now properly accessible to consuming packages
  • Fixed declaration order dependencies - Import/export resolution now works regardless of the order namespaces are declared, using an iterative resolution approach

Breaking changes

  • Primitive type exports (Unit, Qubit, Int) removed from legacy APIs - these never actually worked
  • Parent namespace exports now explicitly banned (e.g. export Parent where Parent.Foo exists) - never properly worked
  • Cross-namespace exports from external packages disallowed as they serve no purpose
  • Duplicate imports/exports in namespace scope now consistently error - previously only some collisions would error
  • Import name collisions with callable/UDT declarations now error

Library Cleanup

  • Removed workaround aliases from FixedPoint and Rotations libraries that are no longer needed
  • Cleaned up Microsoft.Quantum.Core legacy API exports

Implementation Notes

HIR:

  • Removed redundant namespaces field from HIR - only resolver needs the namespace tree
  • All import/export items now get item IDs; all exports are lowered while imports are not
  • Export items now use Res type instead of ItemId to preserve even failed export items
  • Simplified globals enumeration in global.rs

Parser & AST:

  • Invalid patterns like import A.* as B, export C.*, and empty import; are now reported in the parser, reducing the validation that the resolver has to do
  • Changed from "glob" to "wildcard" throughout codebase to match public documentation

Resolver:

  • Iterative fixed-point algorithm resolves dependency chains between import/exports
  • Simplified lowerer by moving more logic to resolver
  • New public resolve() function encapsulates resolver API complexity

Language Service:

  • Updated completion logic to work with new ImportKind structure, but no real fixes yet (next PR)

Copy link

github-actions bot commented Aug 2, 2025

Change in memory usage detected by benchmark.

Memory Report for 723f6a5

Test This Branch On Main Difference
compile core + standard lib 24532482 bytes 25366182 bytes -833700 bytes

@ScottCarda-MS
Copy link
Contributor

ScottCarda-MS commented Aug 4, 2025

I think I found a bug:

file A:

operation Original() : Unit {}

file B:

import A.Original as Other;

file C:

import B.Other;

File C will error saying it can't find B.Other. This is avoided if I put an export in file B: export Other as ExportedOther. Then file C can import B.ExportedOther;, but that seems like it shouldn't be necessary. My understanding was that exporting should really only be necessary to expose content across the package boundary.

Copy link
Collaborator

@swernli swernli left a comment

Choose a reason for hiding this comment

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

Code overall looks good, signing off. Some feedback on from me (and Ian) that could address or discuss, but in general approach seems thorough and the new errors surfaced to the user are great.

Copy link

github-actions bot commented Aug 6, 2025

Change in memory usage detected by benchmark.

Memory Report for 5f1373f

Test This Branch On Main Difference
compile core + standard lib 24532482 bytes 25366182 bytes -833700 bytes

@minestarks minestarks requested a review from idavis August 6, 2025 18:27
@minestarks minestarks added this pull request to the merge queue Aug 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2025
Copy link
Contributor

@orpuente-MS orpuente-MS left a comment

Choose a reason for hiding this comment

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

Approved with minor comment. Thanks!

@minestarks minestarks added this pull request to the merge queue Aug 7, 2025
Merged via the queue into main with commit d5fc73f Aug 7, 2025
18 checks passed
@minestarks minestarks deleted the minestarks/imports branch August 7, 2025 20:55
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.

5 participants