Skip to content

Fix Clang and ld warnings#8634

Merged
expipiplus1 merged 6 commits intoshader-slang:masterfrom
ncelikNV:fix-clang-warnings
Nov 11, 2025
Merged

Fix Clang and ld warnings#8634
expipiplus1 merged 6 commits intoshader-slang:masterfrom
ncelikNV:fix-clang-warnings

Conversation

@ncelikNV
Copy link
Copy Markdown
Contributor

@ncelikNV ncelikNV commented Oct 8, 2025

Fixes the following ld warning:

/usr/bin/ld: tools/CMakeFiles/slang-fiddle.dir/Debug/slang-fiddle/slang-fiddle-lua.cpp.o: in function `os_tmpname(lua_State*)':
/home/ncelik/projects/slang/external/lua/loslib.c:174:(.text+0x33ad1): warning: the use of `tmpnam' is dangerous, better use `mkstemp'

Fixes compiler warnings of the following kinds, emitted when compiling
with Clang 20:

  • warning: implicit conversion from 'int' to 'float' changes value from 2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion]
  • warning: first argument in call to 'memset' is a pointer to non-trivially copyable type '...' [-Wnontrivial-memcall]

Disables -Wunused-but-set-variable warnings (after discussion with
Slang maintainers), e.g.,

/home/ncelik/projects/slang/source/slang/slang-ast-val.h:382:28: warning: variable 'thatGenParam' set but not used [-Wunused-but-set-variable]
  382 |             if (const auto thatGenParam = as<DeclRefIntVal>(other.getParam()))
      |                            ^

@ncelikNV ncelikNV requested a review from a team as a code owner October 8, 2025 13:23
@ncelikNV ncelikNV added the pr: non-breaking PRs without breaking changes label Oct 8, 2025
Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

I left a few comments.

The PR description says something about memset.

warning: first argument in call to 'memset' is a pointer to non-trivially copyable type '...' [-Wnontrivial-memcall]

But I am not sure which lines are related to that.

@ncelikNV
Copy link
Copy Markdown
Contributor Author

ncelikNV commented Oct 9, 2025

warning: first argument in call to 'memset' is a pointer to non-trivially copyable type '...' [-Wnontrivial-memcall]

But I am not sure which lines are related to that.

The lines changed in slang-ir.cpp are. I replaced calls to memset with value-initialization ({}).

@ncelikNV
Copy link
Copy Markdown
Contributor Author

ncelikNV commented Oct 9, 2025

I forgot to mention, I also got 6 -Wnontrivial-memcall warnings in ImGui and an ld warning in Lua about tmpnam being insecure, but I'm not sure we should try to silence those.

Fixes the following ld warning:

```
/usr/bin/ld: tools/CMakeFiles/slang-fiddle.dir/Debug/slang-fiddle/slang-fiddle-lua.cpp.o: in function `os_tmpname(lua_State*)':
/home/ncelik/projects/slang/external/lua/loslib.c:174:(.text+0x33ad1): warning: the use of `tmpnam' is dangerous, better use `mkstemp'
```

Fixes compiler warnings of the following kinds, emitted when compiling
with Clang 20:

- `warning: implicit conversion from 'int' to 'float' changes value from 2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion]`
- `warning: first argument in call to 'memset' is a pointer to non-trivially copyable type '...' [-Wnontrivial-memcall]`

Disables `-Wunused-but-set-variable` warnings (after discussion with
Slang maintainers), e.g.,

```
/home/ncelik/projects/slang/source/slang/slang-ast-val.h:382:28: warning: variable 'thatGenParam' set but not used [-Wunused-but-set-variable]
  382 |             if (const auto thatGenParam = as<DeclRefIntVal>(other.getParam()))
      |                            ^
```
@ncelikNV ncelikNV changed the title Fix Clang warnings Fix Clang and ld warnings Oct 30, 2025
jkwak-work
jkwak-work previously approved these changes Oct 31, 2025
Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

Please keep the comment.

@jkwak-work jkwak-work self-assigned this Nov 4, 2025
@expipiplus1
Copy link
Copy Markdown
Collaborator

I think that we should actually remove -Wno-error=unused-but-set-variable. I think in the past I went through and fixed all of these by prefixing them with const which also silences the warning, this seems to be a more subtle way of fixing them.

although to be honest I think that we should definitely discourage this pattern, especially given the surprising scoping rules from variables declared in that position.

@ncelikNV ncelikNV requested a review from jkwak-work November 7, 2025 16:26
Copy link
Copy Markdown
Collaborator

@expipiplus1 expipiplus1 left a comment

Choose a reason for hiding this comment

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

Thanks!

@expipiplus1 expipiplus1 added this pull request to the merge queue Nov 11, 2025
auto-merge was automatically disabled November 11, 2025 04:59

Pull Request is not mergeable

Merged via the queue into shader-slang:master with commit ef580c1 Nov 11, 2025
37 checks passed
juliusikkala pushed a commit to juliusikkala/slang that referenced this pull request Nov 12, 2025
Fixes the following ld warning:

```
/usr/bin/ld: tools/CMakeFiles/slang-fiddle.dir/Debug/slang-fiddle/slang-fiddle-lua.cpp.o: in function `os_tmpname(lua_State*)':
/home/ncelik/projects/slang/external/lua/loslib.c:174:(.text+0x33ad1): warning: the use of `tmpnam' is dangerous, better use `mkstemp'
```

Fixes compiler warnings of the following kinds, emitted when compiling
with Clang 20:

- `warning: implicit conversion from 'int' to 'float' changes value from
2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion]`
- `warning: first argument in call to 'memset' is a pointer to
non-trivially copyable type '...' [-Wnontrivial-memcall]`

Disables `-Wunused-but-set-variable` warnings (after discussion with
Slang maintainers), e.g.,

```
/home/ncelik/projects/slang/source/slang/slang-ast-val.h:382:28: warning: variable 'thatGenParam' set but not used [-Wunused-but-set-variable]
  382 |             if (const auto thatGenParam = as<DeclRefIntVal>(other.getParam()))
      |                            ^
```

---------

Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
ncelikNV added a commit to ncelikNV/slang that referenced this pull request Dec 10, 2025
Fixes the following warning emitted by Clang 20:

```
warning: first argument in call to 'memset' is a pointer to non-trivially copyable type 'IRConstant' [-Wnontrivial-memcall]
```

See shader-slang#8634.
github-merge-queue bot pushed a commit that referenced this pull request Dec 16, 2025
Fixes the following warning emitted by Clang 20:

```
warning: first argument in call to 'memset' is a pointer to non-trivially copyable type 'IRConstant' [-Wnontrivial-memcall]
```

See #8634.

Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants