Page MenuHomePhabricator

[clangd] More complete fix for hover crashes on invalid record.

Authored by hokein on Jul 5 2020, 11:35 PM.



We should not call getFieldOffset on invalid record decls.

Diff Detail

Event Timeline

hokein created this revision.Jul 5 2020, 11:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2020, 11:35 PM

thanks for doing this !


i think we should bail out for invalid decls in here too. it won't crash but will provide spurious results. so i think an early exit for ND.isInvalidDecl() at the beginning of the function would be nice.


could you move this out of the if statement.

hokein updated this revision to Diff 275664.Jul 6 2020, 4:44 AM
hokein marked 2 inline comments as done.

address comment.


I'm not sure whether Size and Offset should be be independent, IIUC your fix seems to change it as a side effect, this just preserves the old behavior.

kadircet added inline comments.Jul 6 2020, 7:51 AM

it wasn't a side effect, it was intentional and stamped (and explicitly spelled out in the description)

kadircet accepted this revision.Jul 6 2020, 8:04 AM
This revision is now accepted and ready to land.Jul 6 2020, 8:04 AM
hokein marked an inline comment as done.Jul 6 2020, 8:07 AM
hokein added inline comments.

ah, OK, changed it back.

hokein updated this revision to Diff 275718.Jul 6 2020, 8:08 AM

address comment.

This revision was automatically updated to reflect the committed changes.