Skip to content
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

importproject: fix reading of msvc std option #6769

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ludviggunne
Copy link
Contributor

@ludviggunne ludviggunne commented Sep 3, 2024

Might need more tests. The fix is a bit hacky but I thought I'd make as small of a change as possible.

@firewave
Copy link
Collaborator

firewave commented Sep 3, 2024

Will take a look later. That also lacks support for c++latest and clatest: https://learn.microsoft.com/en-us/cpp/build/reference/std-specify-language-standard-version?view=msvc-170.

The note about __cplusplus is also interesting. And it also has _MSVC_LANG as equivalent. Stupid quirks...

@ludviggunne
Copy link
Contributor Author

The note about __cplusplus is also interesting. And it also has _MSVC_LANG as equivalent. Stupid quirks...

The __cplusplus thing is quirky indeed... atleast with this change the correct substring is extracted from the command.

@ludviggunne ludviggunne marked this pull request as ready for review September 24, 2024 11:06
@firewave
Copy link
Collaborator

I will take a look after I finished up #6742 but that still requires some work as it exposes additional issues. So give it a day or two (or a week or two depending on how the reviews progress if they intersect).

if (startsWith(fval, "td:")) {
fs.standard = fval.substr(3);
} else {
fs.standard = readUntil(command, &pos, " ");
Copy link
Owner

Choose a reason for hiding this comment

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

hmm this is matching the old code. but it does not seem to me that it would handle a gcc command like this right?

gcc -std=c99 -c 1.c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants