Rename /P to /Po; make /P match cl.exe behavior#8165
Rename /P to /Po; make /P match cl.exe behavior#8165damyanp wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Rename the old FXC-style /P flag to /Po (deprecated) and make /P behave like cl.exe: preprocess to <input>.i by default, with /Fi to specify the output filename. The old /Po <filename> positional syntax is preserved with a deprecation warning directing users to /P /Fi. Fixes microsoft#4611 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR renames the old FXC-style /P preprocessing flag to /Po (with deprecation warnings) and makes /P behave like cl.exe, which preprocesses to <inputname>.i by default. The change maintains backward compatibility through /Po while aligning DXC's behavior with the widely-used Microsoft C/C++ compiler.
Changes:
- Added new
/Poflag preserving FXC-style preprocessing behavior (deprecated) - Updated
/Pto matchcl.exe: defaults to<input>.i, supports/Fito override - Maintained support for old
/Po <filename>positional syntax with deprecation warnings
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| include/dxc/Support/HLSLOptions.td | Adds /Po flag definition for backward compatibility |
| lib/DxcSupport/HLSLOptions.cpp | Implements separate logic for /P (cl.exe-style) and /Po (FXC-style with deprecation) |
| tools/clang/unittests/HLSL/OptionsTest.cpp | Adds comprehensive unit tests for both /P and /Po behaviors |
| tools/clang/test/DXC/preprocess.test | Adds integration tests verifying preprocessing with /P and /Po |
| docs/ReleaseNotes.md | Documents the command-line interface changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Verify /Po deprecation warning is emitted in preprocess.test - Clarify ReleaseNotes.md: positional syntax is deprecated and moved to /Po, not removed entirely Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| TEST_F(OptionsTest, TestPreprocessOption) { | ||
| // /P (cl.exe-compatible): preprocesses to <input>.i by default. | ||
| VerifyPreprocessOption("/T ps_6_0 -P input.hlsl", "input.i", ""); |
There was a problem hiding this comment.
The test verifies that /P defaults to <input>.i, but doesn't verify the actual file extension replacement logic when the input has a different extension (e.g., .fx, .hlsli). Consider adding a test case with a non-.hlsl input file to ensure the extension replacement works correctly.
Update test to use -P -Fi instead of the old -P <filename> syntax, which was changed to -Po in the previous commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Rename the old FXC-style /P flag to /Po (deprecated) and make /P behave like cl.exe: preprocess to .i by default, with /Fi to specify the output filename. The old /Po positional syntax is preserved with a deprecation warning directing users to /P /Fi.
Fixes #4611