[llvm] [YAML] Init local var not set by some branches (PR #123137)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 16 12:26:12 PST 2025


vitalybuka wrote:

> This change is incorrect. For starters, `ELF_SHT` is a section header type, but you're initialising it with an ELF type, which is an orthogonal concept.

I am sorry, I didn't realize this. I thought fallback to default is intentional.

> 
> I do agree that there is an unitialised variable usage. However, I don't think initialising the value like you have (even if it were a valid value to use) is correct. Please revert this PR and create a new one with the correct fix. (If necessary, I'll revert this PR, but would prefer it if the original contributor/reviewer did).

Will do.

> 
> The "Type" field in the YAML is a mandatory field for sections. It doesn't make sense to try to parse the rest of the Section block if it doesn't have one, because the contents are meaningless. Instead, if the field has been found as unset when calling `getStringValue` at line 1601 in this PR, we should bail out of the code. This could be detected either by `TypeStr` being checked for being empty, or by checking the value of (I think) `IO.EC`. I think I'd prefer the former, because this would mean (valid) section parsing isn't aborted because of unrelated errors elsewhere in the document.

Thanks for the advice!



https://github.com/llvm/llvm-project/pull/123137


More information about the llvm-commits mailing list