This is an archive of the discontinued LLVM Phabricator instance.

Pedantically rename all Tool subclasses to be nouns, not verbs. NFC
ClosedPublic

Authored by dougk on Jun 21 2015, 7:17 PM.

Details

Summary

Classes in Tools.h inherit ultimately from Tool, possibly via GnuTool.
The problem: "Assemble", "Compile", etc are verbs, whereas "Tool" is a noun (and "Link" is both, but in this context, a verb).
Had the classes been named for their command-line (literally ld, etc), this would be rational in that ld is a tool per se.
But they're not- they're named by the conceptual tool's operation.

With the patch, a constructor call - formerly "Compile(args...)" - loses the deceptive appearance of doing that at the time of call and is more symmetric with the factory invocation, e.g. BuildLinker() returns a "new namespace::Linker(...)", not "new namespace::Link(...)".

Exceptions: Clang and ClangAs are un-renamed. Those are their rightful names. And there is no particulary great way to name the "Lipo-er" and a few others.

Diff Detail

Event Timeline

dougk updated this revision to Diff 28093.Jun 21 2015, 7:17 PM
dougk retitled this revision from to Pedantically rename all Tool subclasses to be nouns, not verbs. NFC.
dougk updated this object.
dougk edited the test plan for this revision. (Show Details)
dougk added a subscriber: Unknown Object (MLST).
chandlerc added inline comments.
lib/Driver/Tools.h
173

Above you renamed "gcc::Preprocess" to "gcc::Preprocessor", but here (and almost everywhere else in my spot check) the text names were left alone.

I'm not actually sure which is right. These seem like documentation or logging generation strings, so maybe they should all be updated?

dougk updated this revision to Diff 28110.Jun 22 2015, 8:02 AM

Renamed most Tools' debug string names, except "clang" which has special significance.

dougk added inline comments.Jun 22 2015, 8:03 AM
lib/Driver/Tools.h
173

a few of these strings are tested for in 'lit' tests, so I wanted to avoid changing them; but that was just undue paranoia. I put a cautionary note at the one that matters - "clang" must remain literally as-is; and changed others.

chandlerc accepted this revision.Jun 23 2015, 12:34 PM
chandlerc added a reviewer: chandlerc.

LGTM, thanks for the cleanup!

lib/Driver/Tools.h
196–197

This seems like an unrelated formatting change? I don't really care about it though.

This revision is now accepted and ready to land.Jun 23 2015, 12:34 PM
This revision was automatically updated to reflect the committed changes.