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 unsigned as a full type #6885

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pow2clk
Copy link
Member

@pow2clk pow2clk commented Aug 28, 2024

#106 disabled the unsigned keyword as a complete type specifier among other changes. This was to match FXC behavior which doesn't allow it. It's harmless and sure not to break any existing code. It also represents a discrepancy with the developing clang HLSL support that I'm not inclined to change there.

This restores the ability to promote a variable with just the unsigned keyword as an unsigned int and includes tests that affirm it's availability in a number of contexts and compatibility with uint

@pow2clk pow2clk requested a review from a team as a code owner August 28, 2024 21:22
Copy link
Contributor

github-actions bot commented Aug 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@pow2clk
Copy link
Member Author

pow2clk commented Aug 28, 2024

Oh that's a bit odd. The commit message contains less info than the description on github which says explicitly that it was to match FXC. So there's that.

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

I think there are a bunch of test cases that need to be written for this, particularly for sema cases.

Things like:

  • is_same<unsigned, uint>
  • is_same<unsigned int, uint>
  • is_same<unsigned int64_t, uint64_t>

What about long or short?

I had assumed we would actually remove the C keywords from Clang, so if we're not going to do that maybe we should discuss which ones we want to support in DXC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New
Development

Successfully merging this pull request may close these issues.

3 participants