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

Allow implicit top-level namespaces when parsing files #1416

Merged
merged 157 commits into from
May 20, 2024
Merged

Conversation

sezna
Copy link
Contributor

@sezna sezna commented Apr 23, 2024

This PR allows for implicit top-level namespaces.

Prior to this PR, all Q# had to be wrapped in namespace declarations:

namespace Foo {
  operation Main() : Unit {}
}

Now, users can optionally omit the top-level namespace declaration. This is nice, because it removes a level of necessary indentation.

If the explicit namespace declaration is omitted, the file path is used to determine the namespace name. For example, in ~/code/qsharp/project_dir/src/Foo/Bar/Baz.qs, the file:

operation Quux() : Unit {}

defines a callable in the namespace Foo.Bar.Baz.

We detect the "root" of the project by calculating the largest common prefix of the file paths. This is the most versatile approach, in my opinion, without having to introduce project-based logic into the compiler itself (that is, add a qsc_project dependency to qsc_frontend -- which would be cyclical via qsc_lint). It allows us to have a perfect UX for the happy path following a canonical project layout, and reasonable UX for other situations (e.g., manually evoking qsc on two files in different parts of your computer -- what is the "root"?).

Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

Made a first pass, will take another look after you've had a chance to address comments

compiler/qsc_frontend/src/compile.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/compile.rs Show resolved Hide resolved
compiler/qsc_parse/src/item.rs Outdated Show resolved Hide resolved
compiler/qsc_parse/src/lib.rs Outdated Show resolved Hide resolved
compiler/qsc_parse/src/item/tests.rs Outdated Show resolved Hide resolved
@DmitryVasilevsky
Copy link
Contributor

DmitryVasilevsky commented May 13, 2024

A few questions to the PR description.

"We detect the "root" of the project by calculating the largest common prefix of the file paths."

  1. What if we are compiling just one file and have no explicit project?
  2. What if we have a project, but ALL files happen to be in "src/they/are/here"?

"manually evoking qsc on two files in different parts of your computer -- what is the "root"?"

  1. What if we invoke qsc on two files that are on different drives/shares?
  2. If we use a file qs/x/y/z/a.qs in two qsc invocations - with qs/x/y/z1/b1.qs and qs/x/y2/b2.qs, the namespace assumed for a.qs will be different?

@sezna
Copy link
Contributor Author

sezna commented May 13, 2024

A few questions to the PR description.

"We detect the "root" of the project by calculating the largest common prefix of the file paths."

  1. What if we are compiling just one file and have no explicit project?

In this scenario, the namespace doesn't matter much because you will never refer to items in the current file from another file, due to being in single-file mode. That being said, the project root is detected as the whole path (although once again, this doesn't matter): you can see that in this test case.

  1. What if we have a project, but ALL files happen to be in "src/they/are/here"?

This is the one shortcoming of this PR. Mine mentioned it here. The implementation required for the alternative approach was very fragile and not worth it. We jointly decided that it would be better to surface a warning or error if your src directory does not contain any source files. I've filed #1516 to capture this work.

"manually evoking qsc on two files in different parts of your computer -- what is the "root"?"

  1. What if we invoke qsc on two files that are on different drives/shares?

qsc is not a "shipped" binary so exotic scenarios for it are not our primary use case. That being said, you would either have to use explicit namespaces or make one a dependency of the other using the yet-to-be-released dependencies feature. That way they would be different compile units.

  1. If we use a file qs/x/y/z/a.qs in two qsc invocations - with qs/x/y/z1/b1.qs and qs/x/y2/b2.qs, the namespace assumed for a.qs will be different?

Yes. Ideally you would have your source files organized inside of a project and use our soon-to-be-released dependency management solution to be on the happy path.

compiler/qsc_frontend/src/compile.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/compile.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/lower/tests.rs Outdated Show resolved Hide resolved
compiler/qsc_parse/src/item.rs Outdated Show resolved Hide resolved
compiler/qsc_parse/src/lib.rs Outdated Show resolved Hide resolved
compiler/qsc_parse/src/lib.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/compile.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/compile.rs Outdated Show resolved Hide resolved
pip/src/interpreter.rs Outdated Show resolved Hide resolved
Base automatically changed from alex/hns to main May 17, 2024 18:38
Copy link

Benchmark for 70d5d46

Click to view benchmark
Test Base PR %
Array append evaluation 337.6±21.61µs 337.5±7.02µs -0.03%
Array literal evaluation 189.2±1.00µs 186.3±1.27µs -1.53%
Array update evaluation 414.6±4.26µs 417.9±24.28µs +0.80%
Core + Standard library compilation 18.5±0.08ms 19.1±0.60ms +3.24%
Deutsch-Jozsa evaluation 5.1±0.32ms 5.0±0.06ms -1.96%
Large file parity evaluation 34.4±0.30ms 34.7±0.51ms +0.87%
Large input file compilation 12.4±0.66ms 12.6±0.62ms +1.61%
Large input file compilation (interpreter) 46.5±1.98ms 46.2±0.96ms -0.65%
Large nested iteration 32.8±0.17ms 32.8±0.48ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1549.7±33.84µs 1564.8±39.33µs +0.97%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.7±0.07ms 7.7±0.18ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1414.1±50.35µs 1422.4±39.19µs +0.59%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.3±0.42ms 27.6±1.50ms +1.10%
Teleport evaluation 90.7±9.15µs 87.2±3.52µs -3.86%

@sezna sezna added this pull request to the merge queue May 20, 2024
Merged via the queue into main with commit bd19ca1 May 20, 2024
16 checks passed
@sezna sezna deleted the alex/naked-namespace branch May 20, 2024 14:47
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