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

CConditionalScanner does not detect included header from a command line macro #4193

Closed
mwichmann opened this issue Jul 19, 2022 · 5 comments · Fixed by #4329
Closed

CConditionalScanner does not detect included header from a command line macro #4193

mwichmann opened this issue Jul 19, 2022 · 5 comments · Fixed by #4329
Labels

Comments

@mwichmann
Copy link
Collaborator

mwichmann commented Jul 19, 2022

This issue is generated as a result of investigating using the CConditionalScanner to detect dependencies when the #include uses a macro, after a Discord discussion here: https://discord.com/channels/571796279483564041/571796280146133047/998690474216927345. This base problem has actually been recorded in several past issues, including #344 #2601 #3038.

The investigation shows that if the macro is defined in the source file, like this:

#define HEADER "foo.h"
#include HEADER

then CConditionalScanner indeed produces the dependency, but if it is defined as a compiler command-line option, it is not.

env = Environment(CPPDEFINES=['HEADER=\\"foo.h\\"'])

Attaching a diff to the unit test file for the C scanner, which demonstrates this behavior (unfortunately had to make into a file type github would accept)

CTests-diff.zip

@ivankravets
Copy link
Contributor

Hi @mwichmann, do you use the latest development version? It should be fixed by d9192db

@mwichmann
Copy link
Collaborator Author

Doesn't seem to be... which doesn't mean my test isn't faulty. I still get:

AssertionError: False is not true : expect ['f9.h'] != scanned []

@mwichmann
Copy link
Collaborator Author

I think this may have been fixed by #4263 - not an issue with the conditional scanner, but with the way processDefines emitted a single string-valued macro. I'll need to swing by and check in more detail, but we may be able to close this.

@mwichmann
Copy link
Collaborator Author

Okay, reran the test case attached here, it still fails, but can be made to pass by:

-        env = DummyEnvironment(CPPDEFINES=['HEADER=\\"f9.h\\"'])
+        env = DummyEnvironment(CPPDEFINES=[('HEADER', '\\"f9.h\\"')])

So there may still be a CPPDEFINES hole here.

@mwichmann
Copy link
Collaborator Author

The hole is in SCons/Scanner/C/dictifyCPPDEFINES() - it does its own processing of defines, not using the processDefines routine the rest of SCons uses, and did not deal with "NAME=VALUE" strings, either as a solo value, or as a member of a sequence. (note this problem is not related to the large CPPDEFINES refactor, it was always possible to assign a single string or a string-in-a-list to CPPDEFINES, only after type conversion is triggered might you not have those anymore). I have a patch and some additional tests.

mwichmann added a commit to mwichmann/scons that referenced this issue Mar 21, 2023
CPPDEFINES can contain name=value strings, either a single one, or one or
more in a sequence type.  After conversion (subsequent Append/Prepend to
CPPDEFINES), these will be stored as tuples, but it is possible to hit
cases where the type conversion has never been triggered. The C Scanner
has its own routine to process CPPDEFINES, and missed these cases,
which are now handled.

The testcases in issue 4193 which showed this problem are now included
in the C scanner unit tests, and the test for dictifyCPPDEFINES is
expanded to check these two forms.

Fixes SCons#4193

Signed-off-by: Mats Wichmann <mats@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants