Page MenuHomePhabricator

[analyzer][ctu] fix unsortable diagnostics

Authored by r.stahl on Jun 22 2018, 1:19 AM.

Diff Detail

rC Clang

Event Timeline

r.stahl created this revision.Jun 22 2018, 1:19 AM

For my purposes I replaced the return statement of the compareCrossTUSourceLocs function with:

return XL.getFileID() < YL.getFileID();

A more correct fix would create only one unique diagnostic for both cases.

Thank you for the report, I could reproduce this after modifying the null dereference checker. My fear of using file IDs is that I don't know whether they are stable. So in subsequent runs, different paths might be chosen by the analyzer and this could be confusing to the user. I will think about a workaround that both stable and solves this assertion.

I see two possible ways to do the proper fix. One is to check explicitly for this case when the same function appears in multiple translation units. A better approach would be to have the ASTImporter handle this case. I think the proper fix is better addressed in a separate patch.

Here is my improper fix with test case. Since CTU is still an experimental feature and this is a rare case to encounter I believe it's okay to risk confusing the user, rather than keep it broken.

Unfortunately I will not have the time to work on a proper fix.

My fear of using file IDs is that I don't know whether they are stable

IIRC they are currently stable, but that's relying on implementation details.
Still better than not differentiating at all.

This revision is now accepted and ready to land.Jun 26 2018, 11:15 AM
This revision was automatically updated to reflect the committed changes.