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

[llvm-c-test] Rename --test-dibuilder-debuginfo-format to --test-dibuilder #105702

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

Conversation

vadorovsky
Copy link
Contributor

@vadorovsky vadorovsky commented Aug 22, 2024

The former name was introduced during the split between debug info intrinsic and DbgRecord. Before the split, it was named --test-dibuilder.

However, the full migration to DbgRecord happened already and we have just one test suite related to building debug info using LLVM-C API. Therefore, it makes sense to rename it back to --test-dibuilder.

@vadorovsky vadorovsky force-pushed the llvm-c-test-help branch 2 times, most recently from ae1bd2a to ecccadb Compare August 24, 2024 03:59
Copy link
Contributor

@weliveindetail weliveindetail left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM

@vadorovsky
Copy link
Contributor Author

@weliveindetail @OCHyams do you mind merging this one as well?

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

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

Thanks, good spot.

Looking at it though, I think the original name --test-dibuilder from before all the renaming makes the most sense in today's state. I think we probably should've restored that name in the patch you linked - I probably just missed that at the time.

Could I be cheeky and ask that you invert your patch; change the flag name to --test-dibuilder (keep the existing usage message) and update debug_info_new_format.ll to use that flag?

@vadorovsky vadorovsky changed the title [llvm-c-test] Fix the usage message [llvm-c-test] Rename --test-dibuilder-debuginfo-format to --test-dibuilder Sep 21, 2024
@vadorovsky
Copy link
Contributor Author

@OCHyams done

Copy link

github-actions bot commented Sep 21, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

…ibuilder`

The former name was introduced during the split between debug info
intrinsic and `DbgRecord`. Before the split, it was named
`--test-dibuilder`.

However, the full migration to `DbgRecord` happened already and we have
just one test suite related to building debug info using LLVM-C API.
Therefore, it makes sense to rename it back to `--test-dibuilder`.
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