LLVM warning: Value stored to '*' is never read

[Echelon9]

LLVM compiler warnings of the form -- Value stored to '*' is never read -- have been highlighted in the attached patch.

Each line marked is an assignment to a variable which is then never subsequently read. Some appear redundant, whereas others like ok = JS_CallFunctionValue() are valid function calls where there is no need to save the return value, if we aren't using it later in the codebase.

Any concerns against adjusting or removing each dead store variable accordingly?

[Echelon9]

Patch highlighting the lines LLVM reported

[historic_bruno]

Replying to Echelon9:

Any concerns against adjusting or removing each dead store variable accordingly?

I'd agree if they were causing problems, like a noticeable impact on performance. Otherwise, they could easily be bugs where we should but don't do something with the value, and if we remove the assignments then the bug becomes less obvious.

[historic_bruno]

Also, might an optimizing compiler catch these cases and just not perform the assignment in the first place?

[Philip]

For the two JS_CallFunctionValue, we don't need to report errors on returning false (since it'll already be obvious if they failed), so the "ok =" should be removed.

For CInput.cpp, I've never understood its logic and would be reluctant to change it even if it's clearly wrong (unless the change is to rewrite the whole thing to be much clearer).

For the other two files, the assignments look like obsolete code and should be removed.

Replying to historic_bruno:

Also, might an optimizing compiler catch these cases and just not perform the assignment in the first place?

Yes, which is how LLVM is discovering it should complain about them :-) . But performance is irrelevant for all this code anyway - the problem is mostly that redundant assignments make the intent of the code less clear, so it's good to clean them up.

[Echelon9]

I agree with each of Philip's points.