Bug #2067
C4927 warnings in Core with latest MSVC
Status: | Approved | Start date: | 09/13/2020 | |
---|---|---|---|---|
Priority: | High | Due date: | ||
Assignee: | Miroslav Fidler | % Done: | 0% | |
Category: | Core | Spent time: | - | |
Target version: | Release 2020.2 |
Description
It seems that compiling Core with latest MSVC produces a lot of warnings that have C4927. Screenshot with warnings attached.
Would be nice to fix for 2020.1.
History
#1 Updated by Miroslav Fidler about 3 years ago
Yeah, that is because MSVC is broken. Lucky that we have clang...
Mirek
#2 Updated by Miroslav Fidler about 3 years ago
- Assignee changed from Miroslav Fidler to Zbigniew Rebacz
Have you managed to fix this?
#3 Updated by Zbigniew Rebacz about 3 years ago
- Assignee changed from Zbigniew Rebacz to Miroslav Fidler
As we discuss we will keep it like it is and our recommendation is to do not use latest MSVC versions. If it will not be fixed in reasonable amount of time, we will remove all MSVC related code from TheIDE and we will base only open source solution such as clang ;) Open source compilers can be modify, so bugs like this can be fix easily ;)
Now to be serious. Maybe we should add explicit keyword to char* conversion operator of StringBuffer:
explicit operator char*() { return Begin(); }
This will make this warnings errors... Explicit casting will solve the issue in all cases:
static_cast<String>(sb);
errors are fixed.
You should returhing this whole situation and maybe we should apply explicit keywords in our code to avoid such situation.
Reference:
- https://google.github.io/styleguide/cppguide.html#Implicit_Conversions
Maybe from the usability point of view this change is heave, because each time you need to type casting operator. However from the code security point of view it is improvement.
#4 Updated by Miroslav Fidler about 3 years ago
- Status changed from New to In Progress
- Assignee changed from Miroslav Fidler to Zbigniew Rebacz
Well, that is not very helpful. I hoped that you fix it with String(r) solution and send me the diff or whatever so that I can decide whether it is worth it.
As for that explicit nonsense, do you plan to fix C++ std:: library too? What about boost?
#5 Updated by Zbigniew Rebacz about 3 years ago
I will try to generate patch in the weekend.
#6 Updated by Zbigniew Rebacz about 3 years ago
- File StringBufferPatch.diff
added
#7 Updated by Miroslav Fidler about 3 years ago
- Status changed from In Progress to Ready for QA
Thank you. I have decided to go with it, so please check that trunk now works... (if it does, close the task).
#8 Updated by Zbigniew Rebacz about 3 years ago
- Status changed from Ready for QA to Approved
#9 Updated by Zbigniew Rebacz about 3 years ago
- File Skylark.diff
added
- Status changed from Approved to In Progress
- Assignee changed from Zbigniew Rebacz to Miroslav Fidler
Two warnings in Skylark package would be good to fix...
#10 Updated by Zbigniew Rebacz about 3 years ago
- Status changed from In Progress to Approved