chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.8k stars 422 forks source link

Rename isPrivatized() #22890

Open bradcray opened 1 year ago

bradcray commented 1 year ago

isPrivatized() is a confusing (developer-oriented) query in that it sounds as though it's checking whether a given class is privatized or not, and it is, but it also checks to see whether we've compiled for a single locale or not, which the name doesn't really suggest. This issue proposes that we rename it to something that doesn't lead to that sort of confusion, though I don't have a proposal in hand at the moment.

This caused me a fair amount of confusion in working on #22657 until @ronawho swept in to save the day. Maybe he was just trying to make me feel better, but he said something about this causing him (or others?) confusion as well, motivating this issue. (I'd hoped to implement this as part of #22657, but have been having trouble wrapping up that PR, so am opening this issue instead).

ronawho commented 1 year ago

I don't even think it checks if the current instance has been privatized. I think it's exclusively a "should I privatize this type" check (note that it's a param function.)

Checking the code, it does:

  proc _isPrivatized(value) param do
    return !compiledForSingleLocale() &&
           ((_privatization && value!.dsiSupportsPrivatization()) ||
            value!.dsiRequiresPrivatization());

Not that I'm any good at naming, but I'd default to something like shouldPrivatize