Skip to content

Conversation

@vadorovsky
Copy link
Contributor

Add LLVMGetBircodeProducerString function in LLVM C API that binds to BitcodeReader::getBitcodeProducerString in LLVM C++ API.

@github-actions
Copy link

github-actions bot commented Nov 2, 2025

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

return 1;
}

if (Producer) {
Copy link
Member

Choose a reason for hiding this comment

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

is it not an error for Producer to be NULL here (and for Err to be non-NULL)?

Copy link
Contributor Author

@vadorovsky vadorovsky Nov 3, 2025

Choose a reason for hiding this comment

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

You're right. I extended the predicate in the if statement above to Ret || Err and handled the lack of Producer as an error.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is what I meant. I was thinking we'd want the test to be as strict as possible - to me that means no || anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, how does it look for you now?

@vadorovsky vadorovsky force-pushed the llvm-c-bc-producer-string branch 2 times, most recently from ca7f55e to fdebb42 Compare November 4, 2025 07:39
Add `LLVMGetBircodeProducerString` function in LLVM C API that binds to
`BitcodeReader::getBitcodeProducerString` in LLVM C++ API.
@vadorovsky vadorovsky force-pushed the llvm-c-bc-producer-string branch from fdebb42 to e91bd0f Compare November 4, 2025 07:41
Comment on lines +154 to +161
if (Producer)
fprintf(stderr,
"LLVMGetBitcodeProducerString returned %d, but Producer is not "
"NULL: %s",
Res, Producer);
if (Err)
fprintf(stderr, "LLVMGetBitcodeProducerSring returned %d, error: %s\n",
Res, Err);
Copy link
Member

Choose a reason for hiding this comment

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

This can just be outside the if (Res), right? you always want to print these if they are non-null. Then you can immediately dispose them, and then you can:

int Ret = 0;
if (Res) {
  fprintf(stderr, "LLVMGetBitcodeProducerString(...) returned %d", Res);
  Ret = 1;
}
if (!Producer) {
  fprintf(...)
  Ret = 1;
}
if (Err) {
  ...
}
return Ret;

by the way, do we need a negative test, for the error case? It is not currently exercised.

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.

2 participants