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

Handle _CROW_ILL or _CROW_ICD being NOTFOUND #720

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

DavidPetkovsek
Copy link
Contributor

@DavidPetkovsek DavidPetkovsek commented Nov 7, 2023

There is a big warning that may appear when compiling user code:

In file included from <built-in>:461:
<command line>:6:18: warning: ISO C99 requires whitespace after the macro name [-Wc99-extensions]
    6 | #define _CROW_ICD-NOTFOUND 1
      |

When using get_target_property in cmake and the property is not found it will define the content of the variable <var>-NOTFOUND and because these variables were being used as compile definitions it causes a bunch of warnings due to the - in the name.

Solution:

If the variable is NOTFOUND then set it to "". Then at the end after possible list appends. If the variable is not "" then update the target properties. This means we do not add the compile definition at all.

This gets rid of a warning. And since there is nowhere in the code looking for NOTFOUND it does not change functionality.

If the variable is NOTFOUND then set it to "". Then at the end after possible list appends. If the variable is not "" then update the target properties.

This gets rid of a warning
@DavidPetkovsek
Copy link
Contributor Author

Fixes #661

@DavidPetkovsek
Copy link
Contributor Author

For those who want the patch for the latest release v1.0+5

checkout this branch of the PR

@bmanga
Copy link

bmanga commented Dec 22, 2023

Any updates on this?

@DavidPetkovsek
Copy link
Contributor Author

The fix works for me. Lmk if there is an issue with anything.

@gittiver gittiver linked an issue Jan 29, 2024 that may be closed by this pull request
@gittiver gittiver added the bug Something isn't working label Jan 29, 2024
Copy link
Member

@gittiver gittiver left a comment

Choose a reason for hiding this comment

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

Have a look at the proposed simplified version.

@DavidPetkovsek
Copy link
Contributor Author

Have a look at the proposed simplified version.

Hi gittiver, thank you for the feed back!

We cannot follow through with those two changes.

For the first one. We need to set it to "" because depending on what the user configures, the options later down will append items to the variables as a list. This would mean the "not found" would be a list item.

For the second one, we still need to check if empty because in another project I found if you try to add "" it will cause something else to be added instead which depending on how cmake was used could cause an issue during configuration or building sometimes.

@gittiver
Copy link
Member

gittiver commented Feb 2, 2024

We cannot follow through with those two changes.

I will have a look on the complete cmake files, I had just reviewed the changes.

@gittiver gittiver self-assigned this Feb 2, 2024
@jbulow
Copy link

jbulow commented Feb 20, 2024

Any update on this? The library is not usable in its current state:

<command line>:4:18: error: ISO C99 requires whitespace after the macro name [-Werror,-Wc99-extensions]
    4 | #define _CROW_ICD-NOTFOUND 1
      |                  ^
1 error generated.

@gittiver
Copy link
Member

lets go with your solution.

@gittiver gittiver merged commit 1e864a2 into CrowCpp:master Feb 20, 2024
@nam20485
Copy link

Anyone looking for a workaround to use prior to the release can be found in a comment in the accompanying issue:

#661 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_CROW_ICD-NOTFOUND
5 participants