[llvm] r288598 - [PM] Rename lookupPass to lookUpPass.

Justin Lebar via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 12 16:14:13 PST 2016


> This seems inconsistent with a fair amount of existing precedent in LLVM.

I would argue that these are also wrong.  :)

I freely admit that's elitist descriptivist crap, but this case really
does affect understanding, in that e.g. "lookup result" and "look up
result" have completely different meanings.  It's my hunch that (at
least native) speakers distinguish these meanings in spoken English
based on the existence of a pause between "look" and "up", so the
proposed written form matches how we speak.

Same for "setup" vs "set up", or "login" vs "log in".  For "setup/set
up", we seem to be trying to maintain some distinction in our writing:

$ git grep -iw 'set up' | wc -l
660
$ git grep -iw 'setup' | grep -v setUp | wc -l
754
$ git grep -hiw 'setup' | grep Up | wc -l
1109

(These are over the monorepo.)

In fairness, the stats are not nearly as strongly in my favor with
"look up" versus "lookup":

$ git grep -hiw 'look up' | wc -l
373
$ git grep -hi lookup | grep Up | wc -l
134
$ git grep -hi 'lookup' | grep -v Up | wc -l
8332

However, maybe many of those 8k are noun "lookup"s (ok in my book)
rather than verb "lookup"s (bad in my book)?  To test that, I looked
for tokens that look like function calls:

$ git grep -hi 'lookup[a-zA-Z0-9_]*('  | grep -v 'Up'   | wc -l
3466
$ git grep -hi 'lookup[a-zA-Z0-9_]*('  | grep 'Up'   | wc -l
83

Maybe that's a little better?  Most 62% of the "lookUp"s survived this
change, whereas only 41% of the "lookup"s survived when we restricted
to things that look like function calls.

Maybe "lookup" is a special case, different from "setup", "login",
"roundup/round up", "move-in date" (vs "move in"), and
"stand-up/standup comedy" (not "stand up comedy", which is what you do
when you don't attend :).  I don't tend to think it's a special case,
though, especially given the 373 occurrences of "look up", which
presumably all appear outside of code -- this suggests to me that the
special case is just our not capitalizing the "u" when we write code.

Anyway this patch is just a suggestion to scratch a (clearly) itchy
part of my brain.  If we don't like it, that's fine with me too.

-Justin

On Mon, Dec 12, 2016 at 3:29 PM, David Blaikie <dblaikie at gmail.com> wrote:
> This seems inconsistent with a fair amount of existing precedent in LLVM.
>
> we have loads of functions that use lookup as a verb (DenseMap::lookup,
> ScopedHashTable::lookup, IntervalMap::lookup, ImmutableMap::lookup,
> MapVector::lookup - just to name those in ADT, but they're littered across
> all parts of LLVM)
>
>
> On Sat, Dec 3, 2016 at 12:00 PM Justin Lebar via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: jlebar
>> Date: Sat Dec  3 13:49:35 2016
>> New Revision: 288598
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=288598&view=rev
>> Log:
>> [PM] Rename lookupPass to lookUpPass.
>>
>> Summary:
>> "Lookup" is a noun ("lookup table"), "look up" is a verb ("look up
>> 'table' in the dictionary").
>>
>> Reviewers: chandlerc
>>
>> Subscribers: silvas, llvm-commits, mehdi_amini
>>
>> Differential Revision: https://reviews.llvm.org/D27374
>>
>> Modified:
>>     llvm/trunk/include/llvm/IR/PassManager.h
>>
>> Modified: llvm/trunk/include/llvm/IR/PassManager.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PassManager.h?rev=288598&r1=288597&r2=288598&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/PassManager.h (original)
>> +++ llvm/trunk/include/llvm/IR/PassManager.h Sat Dec  3 13:49:35 2016
>> @@ -638,16 +638,16 @@ public:
>>    }
>>
>>  private:
>> -  /// \brief Lookup a registered analysis pass.
>> -  PassConceptT &lookupPass(AnalysisKey *ID) {
>> +  /// \brief Look up a registered analysis pass.
>> +  PassConceptT &lookUpPass(AnalysisKey *ID) {
>>      typename AnalysisPassMapT::iterator PI = AnalysisPasses.find(ID);
>>      assert(PI != AnalysisPasses.end() &&
>>             "Analysis passes must be registered prior to being queried!");
>>      return *PI->second;
>>    }
>>
>> -  /// \brief Lookup a registered analysis pass.
>> -  const PassConceptT &lookupPass(AnalysisKey *ID) const {
>> +  /// \brief Look up a registered analysis pass.
>> +  const PassConceptT &lookUpPass(AnalysisKey *ID) const {
>>      typename AnalysisPassMapT::const_iterator PI =
>> AnalysisPasses.find(ID);
>>      assert(PI != AnalysisPasses.end() &&
>>             "Analysis passes must be registered prior to being queried!");
>> @@ -665,7 +665,7 @@ private:
>>      // If we don't have a cached result for this function, look up the
>> pass and
>>      // run it to produce a result, which we then add to the cache.
>>      if (Inserted) {
>> -      auto &P = this->lookupPass(ID);
>> +      auto &P = this->lookUpPass(ID);
>>        if (DebugLogging)
>>          dbgs() << "Running analysis: " << P.name() << "\n";
>>        AnalysisResultListT &ResultList = AnalysisResultLists[&IR];
>> @@ -697,7 +697,7 @@ private:
>>        return;
>>
>>      if (DebugLogging)
>> -      dbgs() << "Invalidating analysis: " << this->lookupPass(ID).name()
>> +      dbgs() << "Invalidating analysis: " << this->lookUpPass(ID).name()
>>               << "\n";
>>      AnalysisResultLists[&IR].erase(RI->second);
>>      AnalysisResults.erase(RI);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list