-
Notifications
You must be signed in to change notification settings - Fork 51
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
Fixing code for MSVC #23
Conversation
Used const std::string& for vm_brand parameter in memoize function to avoid unnecessary string copies. Updated the conditions in memoize to use bitwise AND (&) instead of logical AND (&&) for better efficiency. Added comments to explain the purpose of the code.
The compilation error in the build test is because the unicode character set that I have explained, "Remember that most issues with compilation errors are because Unicode compatibility issues. Therefore, you need to enfornce "Use Unicode Character Set"". Its just one problematic line thought. |
oh wow, that’s a massive PR! I was sleeping until I heard an email notification on my phone from github about this. I’ll go back to sleep right now since my head is feeling fuzzy at the moment but I will review it whenever I wake up. I genuinely appreciate the PRs you’ve made recently, it really means a lot :) |
hey, i made a PR on your fork here a few days ago. Just letting you know |
Merged, good job :)
fixed the conflict in this PR. |
nice, I will review the code when I have time, everything seems ok tho and I'm agree with everything you mentioned, including the indentation part |
take your time, there’s no rush :) |
Fixed conflicts and made indentation changes
Done, pull request merged. |
i'll try to get it fixed on my end, thanks for the effort btw! :) If you want to contribute more then feel free to share new detection ideas, there's no stupid suggestion. EDIT: fixed here, that was a mess up on my part |
perfect, I'll keep contributing to this project, I like it a lot Thanks for your effors too, good job |
Compiler flags:
Compilation errors found in the previous version:
To see all the changes made in the first commit, you should click on "Ignore whitespaces", like this:
Link: NotRequiem@600d8b7?diff=unified&w=1
Compilation warnings found in the previous version:
Fixed some IntelliSense warnings about buffer overruns, variants not being initialized with VariantInit and 'dll could be 0'.
How did I fix
Normally in large source files, when fixing errors in a source file using Visual Studio, you may find no errors when you finish. But if you copy and paste the same source code with the same compiler flags in other project, using Visual Studio, more errors/warnings may be shown. Example of pasting the same code in another project:
So, if you constantly fix errors and warnings until you find no errors in other projects, then it will compile succesfully for everyone.
Link: NotRequiem@ccfd50e?diff=unified&w=0
Remember that most issues with compilation errors are because Unicode compatibility issues. Therefore, you need to enfornce "Use Unicode Character Set"
Using unicode character set:
Not using it:
Testing
The final code works for C++ 17:
The final code works for C++ 14:
The final code compiles correctly on other projects:
Final result: