-
Notifications
You must be signed in to change notification settings - Fork 174
Use Uri.LocalPath instead of Uri.AbsolutePath for file IDs #468
Conversation
|
this is a great idea. I started doing the exact same thing as part of #356 but then abandoned it as I wasn't sure what would I break in the runtime |
|
@filipw Sorry about that, I didn't realize your PR was so similar, or I would've mentioned you. :) Out of curiosity, is testing changes/making PRs across multiple repos something you are interested in? It is currently difficult for non-Microsoft-Quantum contributors to do this because our multi-repo integration repo is private at the moment, and I believe that only employees can publish packages to the alpha NuGet feed (which is how I updated the runtime package versions in this PR). I'm trying to advocate for making this integration repo/packaging process public sooner rather than later to help open-source contributors. :) |
yes, that would extremely helpful |
Great, thanks for the feedback! I'll pass this along. |
|
I have updated this PR:
Here is my reasoning for deprecating
|
* Use invariant culture for string conversions in tests (#470) * Prevent result equality comparisons for QPRGen0 runtime capabilities (#449) * Prevent result equality with QPRGen0 * Move more fields from SymbolTracker to ResolutionContext * Undo some changes to TypeChecking.cs * Move ResolutionContext up a bit * Better error message * Add basic tests * Add more tests (but not working right now...) * Fix tests * Add tests for not equals * Use static member for Create * Move ResolutionContext to SymbolTracker.fs * Update doc comment to ArgumentException Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Replace failwith with ArgumentException * Mention SymbolTracker versioning in ResolutionContext * Restore supportsEqualityComparison and use VerifyIsOneOf * Undo changes to VerificationTools.fs * Rename ResolutionContext to ScopeContext * Include execution target in error message * Make ScopeContext.ExecutionTarget non-nullable * Update error message Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Fix possible null reference in CompilationLoader (#481) * Revising the plugin mechanism for compiler steps (#472) * Classically Controlled Short Circuit Bug Fix (#478) Reordered the steps of the classically controlled rewrite to have the if-statement restructuring done first. * Don't execute rewrite steps if the validation fails (#473) * Use Uri.LocalPath instead of Uri.AbsolutePath for file IDs (#468) * Use Uri.LocalPath instead of Uri.AbsolutePath * Fix attribute reader test * Update packages to fix tests * Add GetFileId and deprecate TryGetFileId * Re-add null URI check Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Use OnCompilation in transformations. (#484) * Fix to reserve symbol in set statement crash (#486) * Add NoOp to Precondition Dependencies for Classically Controlled (#489) Added NoOp to precondition dependency list for classical control. * NoOp Removed from Dependency List (#490) Removed NoOp to precondition dependency list for classical control. Co-authored-by: Sarah Marshall <33814365+samarsha@users.noreply.github.com> Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Co-authored-by: Ryan Shaffer <ryan.shaffer@microsoft.com> Co-authored-by: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> Co-authored-by: César Zaragoza Cortés <cesar.zaragoza@outlook.com>
* Use invariant culture for string conversions in tests (#470) * Prevent result equality comparisons for QPRGen0 runtime capabilities (#449) * Prevent result equality with QPRGen0 * Move more fields from SymbolTracker to ResolutionContext * Undo some changes to TypeChecking.cs * Move ResolutionContext up a bit * Better error message * Add basic tests * Add more tests (but not working right now...) * Fix tests * Add tests for not equals * Use static member for Create * Move ResolutionContext to SymbolTracker.fs * Update doc comment to ArgumentException Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Replace failwith with ArgumentException * Mention SymbolTracker versioning in ResolutionContext * Restore supportsEqualityComparison and use VerifyIsOneOf * Undo changes to VerificationTools.fs * Rename ResolutionContext to ScopeContext * Include execution target in error message * Make ScopeContext.ExecutionTarget non-nullable * Update error message Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Fix possible null reference in CompilationLoader (#481) * Revising the plugin mechanism for compiler steps (#472) * Classically Controlled Short Circuit Bug Fix (#478) Reordered the steps of the classically controlled rewrite to have the if-statement restructuring done first. * Don't execute rewrite steps if the validation fails (#473) * Use Uri.LocalPath instead of Uri.AbsolutePath for file IDs (#468) * Use Uri.LocalPath instead of Uri.AbsolutePath * Fix attribute reader test * Update packages to fix tests * Add GetFileId and deprecate TryGetFileId * Re-add null URI check Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * Use OnCompilation in transformations. (#484) * Fix to reserve symbol in set statement crash (#486) * Add NoOp to Precondition Dependencies for Classically Controlled (#489) Added NoOp to precondition dependency list for classical control. * NoOp Removed from Dependency List (#490) Removed NoOp to precondition dependency list for classical control. * Language Server SEMVER (#497) * Bump Markdig version. (#502) * Add SemVer 2.0 data as AssemblyInformationalVersion (#501) * Add informational version to --version. * Fix `s. * Apply suggestions from code review * Update build/build.ps1 Co-authored-by: Ryan Shaffer <ryan.shaffer@microsoft.com> * Apply suggestions from code review Co-authored-by: Ryan Shaffer <ryan.shaffer@microsoft.com> * Fixed test.ps1. Co-authored-by: Ryan Shaffer <ryan.shaffer@microsoft.com> * Use correct versions for both VS and VSCode VSIX (#500) * Use Semver.Version for VS VSIX filename * Set Semver.Version in CI * Use SEMVER_VERSION env variable in .csproj * Undo unneeded changes * Add SemverVersion property to .csproj * Use #SEMVER_VERSION# for .csproj * Improve filter * Use task.setvariable * Update VSIX_VERSION correctly * Separate VSIX version for VS and VSCode extensions * Use SEMVER_VERSION directly for VSCode vsix * Distinguishing the processor architecture and the execution target (#503) * upgraded Roslyn to 3.6.0 (#499) Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * removing unnecessary package references (#505) Co-authored-by: Bettina Heim <beheim@microsoft.com> * Fix errors with exception handling during compilation and tests (#492) * Fix error handling in compilation loader/manager * Show original exceptions in tests * NoOp Reference Change (#494) Added NoOp to precondition verification for ClassicallyControlled. * Compile-time errors for Q# programs not supported by QPRGen1 runtime capabilities (#488) * Verify that result comparisons are in if condition * Add test cases for elif/else Result comparisons * Gen 1 result ifs can only be in operations * Un-skip QPRGen1 tests * Add new diagnostics * Messy, but most QPRGen1 tests passing * Fix diagnostic messages * Update diagnostic messages * Workaround to get diagnostics closer to the right place * Rename startOffset to rootPosition * Add test for return in nested scope * Replace FindStatements transformation with private function * Combine Offset and Range for return statements * Add QsStatement.RangeRelativeToRoot * Clean up verification * Use _ for discarded case value * Use QsStatement.ExtractAll * Use TypedExpression.Exists * Add new test cases, currently failing * Update mutable set verification * Update doc comment for TreeNode.GetRootPosition * Move verification functions to the top * Move else-block normalization to verify function * Use common base type in isResultComparison * Remove QsStatement.RangeRelativeToRoot * Add test for mutable set of tuple * Add SetReusedName test case to Unknown/Gen0 * Reformat new code in TryBuildIfStatement * Update diagnostic messages * Typo in doc comment * Remove QsNullable.ToOption * Formatting BoolLiteral * Don't use CommonBaseType * Add comment about no-derived-types assumption * Document assumptions and add tests for compound types * Remove struct tuple * Update references to ExecutionTarget * Move range conversion to CompilationManager * Fix conjugation diagnostic range Co-authored-by: Sarah Marshall <33814365+samarsha@users.noreply.github.com> Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Co-authored-by: Ryan Shaffer <ryan.shaffer@microsoft.com> Co-authored-by: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> Co-authored-by: César Zaragoza Cortés <cesar.zaragoza@outlook.com> Co-authored-by: Andres Paz <anpaz@microsoft.com> Co-authored-by: Chris Granade <chgranad@microsoft.com> Co-authored-by: Filip W <wojcieszyn@gmail.com> Co-authored-by: Bettina Heim <beheim@microsoft.com>
For a file path containing spaces, like
C:\Foo Bar\Baz.qs, that was converted to a URIfile://C:/Foo%20Bar/Baz.qs:AbsolutePathreturnsC:/Foo%20Bar/Baz.qsLocalPathreturns the originalC:\Foo Bar\Baz.qsSince the file ID is used as a path in C# generation with the
#lineannotations, usingAbsolutePathwas causing Q# projects to fail to build ifEmbedAllSourceswas set to true in the project file, because the C# compiler would follow those annotations. The path given byAbsolutePathis not equivalent to the original file path, so the file couldn't be found. I think usingLocalPathfor all file IDs makes more sense here.A corresponding PR microsoft/qsharp-runtime#263 is needed at the same time. The tests for this change are also there. Fixes #333.