QIR Profiles Selection via Project Field or EntryPoint argument #2591
Conversation
…pabilities or throws new error. Added is_single_file to Compilation and as param to prepare_package_store.
ScottCarda-MS
requested review from
billti,
idavis,
swernli,
minestarks and
DmitryVasilevsky
as code owners
ScottCarda-MS
changed the title
| let buildable_program = BuildableProgram::new(TargetCapabilityFlags::all(), pkg_graph, false); | ||
|
|
There was a problem hiding this comment.
An enum is more desciptive and readable than passing a bool
There was a problem hiding this comment.
Agreed... it's a little more hoop-jumping, but something like ProjectType::SingleFile vs ProjectType::Project would make it abundantly clear what is being used here and in other places where the option is passed.
There was a problem hiding this comment.
Ah, there is already a ProjectType, so another name, maybe ProjectStructure or something like that.
There was a problem hiding this comment.
Since moving this into the packageGraphSources struct, we only ever see this Boolean accompanied by the field name has_manifest. I'm hoping that is sufficient to provide clarity and readability now.
| let buildable_program = BuildableProgram::new(TargetCapabilityFlags::all(), pkg_graph, false); | ||
|
|
There was a problem hiding this comment.
Agreed... it's a little more hoop-jumping, but something like ProjectType::SingleFile vs ProjectType::Project would make it abundantly clear what is being used here and in other places where the option is passed.
| let buildable_program = BuildableProgram::new(TargetCapabilityFlags::all(), pkg_graph, false); | ||
|
|
There was a problem hiding this comment.
Ah, there is already a ProjectType, so another name, maybe ProjectStructure or something like that.
There was a problem hiding this comment.
I can't understand why these tests had to change so much. I can't find any meaningful changes in the language service that would have made these changes necessary?
There was a problem hiding this comment.
It is because before the changes, we were passing in the target profile as an extra argument to the compilation. Now target profile needs to be specified by the source code, either in the manifest or in an entrypoint argument.
source/vscode/src/qirGeneration.ts
Outdated
| ); | ||
| } else { | ||
| await setTarget(targetSupportsAdaptive ? "adaptive_ri" : "base"); | ||
| // Try to update the qsharp.json manifest automatically. |
There was a problem hiding this comment.
Was this behavior decided as part of a team discussion or something? It's weird. Can we just open the qsharp.json and let the user change the file if they want to?
There was a problem hiding this comment.
This was reflecting the existing behavior of that option, which updates the global profile setting. A button that says "Set the QIR profile to Base to continue" should do the actual setting, imho
This removes the workspace setting for the QIR profile and adds a setting for this for Q# project files instead, i.e.
"targetProfile": "base". Single-file Q# files are supported by adding an argument to the @entrypoint attribute where a user will specify what profile the file is meant to compile with, i.e.@EntryPoint(Adaptive_RI), or usesUnrestrictedas a default.