This is an archive of the discontinued LLVM Phabricator instance.

Add _Optional as fast qualifier
Needs ReviewPublic

Authored by chrisbazley on Jan 27 2023, 8:43 AM.

Details

Reviewers
shafik
rriddle
Summary

A new pointee type qualifier for the purpose of
adding pointer nullability information to C
programs. Its goal is to provide value not only
for static analysis and documentation, but also
for compilers which report errors based only on
existing type-compatibility rules. The syntax
and semantics are designed to be as familiar (to
C programmers) and ergonomic as possible.

Diff Detail

Event Timeline

chrisbazley created this revision.Jan 27 2023, 8:43 AM
Herald added a reviewer: shafik. · View Herald Transcript
Herald added a reviewer: rriddle. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
chrisbazley requested review of this revision.Jan 27 2023, 8:43 AM
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptJan 27 2023, 8:43 AM

I don't understand the MLIR changes here, how are they relevant to the patch?

rsmith added a subscriber: rsmith.Jan 27 2023, 2:02 PM

Including a link to the RFC (https://discourse.llvm.org/t/rfc-optional-a-type-qualifier-to-indicate-pointer-nullability/68004/2) in each of the patches in this series would be helpful.

Assuming that we want to go in this direction, it seems quite expensive to model this as a fast qualifier rather than an extended qualifier. Are these annotations expected to be so common that it's better to increase the alignment of all types than perform extra allocations and indirections for _Optional qualifiers? Have you measured the memory impact of increasing the alignment of Type? I think that should be a prerequisite to adding any new kind of fast qualifier, and if we do add such a qualifier, we should select carefully which qualifier gets this precious bit in QualType.

Hi, thanks very much for looking at my patch. I added the link that you proposed to all of the patches in the stack.

Assuming that we want to go in this direction, it seems quite expensive to model this as a fast qualifier rather than an extended qualifier.

True, and I'm not at all wedded to the idea of _Optional being a fast qualifier. I added it in the simplest way I knew how, having no prior experience of the codebase. What would be really useful would be if you could show/explain how to add it in a less risky way. If not, I can look into it myself.

Are these annotations expected to be so common that it's better to increase the alignment of all types than perform extra allocations and indirections for _Optional qualifiers?

Given time, I hope so, but realistically not in the near future.

Have you measured the memory impact of increasing the alignment of Type?

No, because I didn't think it necessary for the purpose of prototyping. If there's any prospect of getting my patches merged then I'd be delighted to invest the time... but only if increasing the alignment is a necessary thing to do.

I think that should be a prerequisite to adding any new kind of fast qualifier, and if we do add such a qualifier, we should select carefully which qualifier gets this precious bit in QualType.

I believe that over time, _Optional would be a much more appropriate use of the precious bit currently occupied by volatile because I expect it to be used more heavily, but I can't make that argument yet.