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

[SYCLomatic] Remove invalid json test for standard compilation db fields #583

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

Conversation

daiyaan-ahmed6
Copy link
Contributor

@daiyaan-ahmed6 daiyaan-ahmed6 commented Jan 4, 2024

Removed tests are check for 'Output' and 'Arguments' field in dpct to be invalid.

This is done for passing of feature PR: oneapi-src/SYCLomatic#1566

ngt-invalid-json7 and ngt-invalid-json8 test are removed.
ngt-invalid-json9 and ngt-invalid-json10 are renamed to ngt-invalid-json7 and ngt-invalid-json8 respectively.

Signed-off-by: Daiyaan Ahmed [email protected]

Copy link
Contributor

@zhimingwang36 zhimingwang36 left a comment

Choose a reason for hiding this comment

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

LGTM

if "directory_placeholder" in line:
ret.append(" \"output\": \"aaaaa\",\n")
line = line.replace("directory_placeholder", os.getcwd().replace("\\", "\\\\"))
iter += 1
Copy link
Contributor

@tomflinda tomflinda Jan 8, 2024

Choose a reason for hiding this comment

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

Sine 'Output' and 'Arguments' in compile_commands.json could be consumed by SYCLomatic, the test case should keep 'output' in compile_commands.json and also added 'Arguments' to verify this feature, instead of remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the new PR for this, I have changed the name of these tests to 'standard_compilation_db_json' and I am keeping 'Arguments' and 'Output' in compilation db to verify that dpct consumes these arguments.

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