This is an archive of the discontinued LLVM Phabricator instance.

[LLVM] Replace `dyn_cast_or_null` with `dyn_cast_if_present`, NFC.
AbandonedPublic

Authored by bzcheeseman on Aug 31 2022, 9:29 PM.

Details

Summary

dyn_cast_if_present is simply the new version of dyn_cast_or_null - indeed dyn_cast_or_null calls dyn_cast_if_present. The if_present suffix more closely represents the capabilities of the function now, as it can correctly handle optionals and things that are not "null".

Diff Detail

Event Timeline

bzcheeseman created this revision.Aug 31 2022, 9:29 PM
bzcheeseman requested review of this revision.Aug 31 2022, 9:29 PM
nikic added a subscriber: nikic.Sep 1 2022, 12:29 AM

While the new name represents the capabilities better, the way these functions are used in LLVM, we are actually working with null values. I find the original names (both here and in D133090) substantially clearer. I don't think we should do this rename.

bzcheeseman added a comment.EditedSep 1 2022, 7:44 AM

It's also worth noting that the _or_null variants are soft-deprecated (see comments on _or_null) and this is in preparation for actually marking them deprecated. IMO, we want one way to do the "if this value exists, do the dyn_cast/cast" and the if_present naming can apply equally well to pointers and optionals.

I think it'd better to discuss such topics in https://discourse.llvm.org/ to involve more people in (Although there are many developers got subscribed : )

Discourse thread here for folks that want to discuss more long-form: https://discourse.llvm.org/t/psa-swapping-out-or-null-with-if-present/65018

nhaehnle removed a subscriber: nhaehnle.Oct 6 2022, 2:46 AM
bzcheeseman abandoned this revision.Oct 14 2022, 8:38 AM

No consensus, closing this.