More conforming to C++ standards
- Can't mix char* and const char*
- Use this-> to represent member from parent template classes
- Can't do double implicit conversion
:white_check_mark: Build DirectXShaderCompiler 1.0.1790 completed (commit https://github.com/microsoft/DirectXShaderCompiler/commit/b391509d41 by @gongminmin)
These all look like reasonable changes to me.
Is there a set of compiler warnings you enabled when developing this patch? Might be worth enabling extra warnings in the future to prevent regression.
It's a part of efforts when I trying to compile dxc with clang-cl. These issues could be caught by msvc with some warnings on. But I'm not sure which ones.
:white_check_mark: Build DirectXShaderCompiler 1.0.1982 completed (commit https://github.com/microsoft/DirectXShaderCompiler/commit/503f910adc by @gongminmin)
Looks good.
I don't see any example of some of the things mentioned in the commit message.
2. Use this-> to represent member from parent template classes I don't see anything similar to this. 3. Can't do double implicit conversionMaybe not this either? There are some places that lose casting. I don't know if this is in reference to double type casting or two casts that form a (double)
Perhaps these comments represent an earlier version of the change?
Right. 2) and 3) are already fixed in main branch in the last 15 months. I've updated the commit message and the PR description.
PR contains clang-format violations. First 50 lines of the diff:
--- projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp (before formatting)
+++ projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp (after formatting)
@@ -567,11 +567,11 @@
memcpy(&P, pParamBase + iElement * uElemSize, uElemSize);
break;
case sizeof(D3D10_INTERNALSHADER_PARAMETER):
- static_assert(sizeof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS) ==
- sizeof(D3D10_INTERNALSHADER_PARAMETER) +
- offsetof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS,
- SemanticName),
- "Incorrect assumptions about field offset");
+ static_assert(
+ sizeof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS) ==
+ sizeof(D3D10_INTERNALSHADER_PARAMETER) +
+ offsetof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS, SemanticName),
+ "Incorrect assumptions about field offset");
memcpy(&P.SemanticName, pParamBase + iElement * uElemSize, uElemSize);
break;
default:
--- tools/dxexp/dxexp.cpp (before formatting)
+++ tools/dxexp/dxexp.cpp (after formatting)
@@ -336,7 +336,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
@@ -356,7 +357,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
See action log for the full diff.
PR contains clang-format violations. First 50 lines of the diff:
--- projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp (before formatting)
+++ projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp (after formatting)
@@ -567,11 +567,11 @@
memcpy(&P, pParamBase + iElement * uElemSize, uElemSize);
break;
case sizeof(D3D10_INTERNALSHADER_PARAMETER):
- static_assert(sizeof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS) ==
- sizeof(D3D10_INTERNALSHADER_PARAMETER) +
- offsetof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS,
- SemanticName),
- "Incorrect assumptions about field offset");
+ static_assert(
+ sizeof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS) ==
+ sizeof(D3D10_INTERNALSHADER_PARAMETER) +
+ offsetof(D3D11_INTERNALSHADER_PARAMETER_FOR_GS, SemanticName),
+ "Incorrect assumptions about field offset");
memcpy(&P.SemanticName, pParamBase + iElement * uElemSize, uElemSize);
break;
default:
--- tools/dxexp/dxexp.cpp (before formatting)
+++ tools/dxexp/dxexp.cpp (after formatting)
@@ -336,7 +336,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
@@ -356,7 +357,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
See action log for the full diff.
PR contains clang-format violations. First 50 lines of the diff:
--- tools/dxexp/dxexp.cpp (before formatting)
+++ tools/dxexp/dxexp.cpp (after formatting)
@@ -336,7 +336,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
@@ -356,7 +357,8 @@
hRuntime = LoadLibraryW(L"d3d12.dll");
if (hRuntime == NULL) {
err = GetLastError();
- printf("Failed to load library d3d12.dll - Win32 error %u\n", (unsigned int)err);
+ printf("Failed to load library d3d12.dll - Win32 error %u\n",
+ (unsigned int)err);
return 1;
}
See action log for the full diff.