Remove debugOptions from internal classes #3370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ncesario-lunarg
commented
Oct 21, 2023
| @@ -516,7 +516,7 @@ void ProcessGlobalBlockSettings(int& argc, char**& argv, std::string* name, unsi | |||
|
|
|||
| if (set) { | |||
| errno = 0; | |||
| int setVal = ::strtol(argv[curArg], nullptr, 10); | |||
| int setVal = static_cast<int>(::strtol(argv[curArg], nullptr, 10)); | |||
There was a problem hiding this comment.
These casts were necessary due to the addition of -Wshorten-64-to-32.
ncesario-lunarg
commented
Oct 21, 2023
Comment on lines
+333
to
+339
| GLSLANG_EXPORT int ShCompile(const ShHandle, const char* const shaderStrings[], const int numStrings, | ||
| const int* lengths, const EShOptimizationLevel, const TBuiltInResource* resources, | ||
| int, // debugOptions unused | ||
| int defaultVersion = 110, // use 100 for ES environment, overridden by #version in shader | ||
| bool forwardCompatible = false, // give errors for use of deprecated features | ||
| EShMessages messages = EShMsgDefault // warnings and errors | ||
| ); |
There was a problem hiding this comment.
These whitespace changes are coming from clang-format.
What is our policy on clang-format? It looks like we don't enforce clang-format changes, yet we have a .clang-format file checked into the repo. Should we either enforce clang-format (perhaps modifying the .clang-format file first), or just get rid of the .clang-format file if we don't plan on enforcing it?
The debug options passed down from the public ShConstruct* functions to internal code is unused. This change removes the internal debugOptions fields and attempts to make it more obvious these fields are not used. This change does not change the public-facing interface. This change also adds the -Wshorten-64-to-32 warning to the StandAlone build in order to avoid unwanted 64-to-32 bit conversions in the future. Closes KhronosGroup#3348.
ncesario-lunarg
force-pushed
the
issue-3348
branch
from
7ae762b to
00920ab
Compare
ncesario-lunarg
referenced
this pull request
Oct 21, 2023
Adds the --no-link option which outputs the compiled shader binaries without linking them. This is a first step towards allowing users to create SPIR-v binary, non-executable libraries. When using the --no-link option, all functions are decorated with the Export linkage attribute.
arcady-lunarg
approved these changes
Oct 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The debug options passed down from the public ShConstruct* functions to internal code is unused. This change removes the internal debugOptions fields and attempts to make it more obvious these fields are not used.
This change does not change the public-facing interface.
This change also adds the -Wshorten-64-to-32 warning to the StandAlone build in order to avoid unwanted 64-to-32 bit conversions in the future.
Closes #3348.