This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] [docs] Improve description of NAME in Programmer's Reference
ClosedPublic

Authored by Paul-C-Anagnostopoulos on Apr 20 2021, 10:09 AM.

Details

Summary

This revision improves the description of the implicit template argument NAME in the TableGen "Programmer's Reference."

It also uses the term "parent class" consistently.

Diff Detail

Event Timeline

Paul-C-Anagnostopoulos requested review of this revision.Apr 20 2021, 10:09 AM
Paul-C-Anagnostopoulos created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2021, 10:09 AM
dblaikie added inline comments.Apr 20 2021, 10:27 AM
llvm/docs/TableGen/ProgRef.rst
65–67

Not sure how the wording is used in other parts of the TableGen docs, but since LLVM's a C++ oriented project, maybe we should standardize/lean towards the C++ terminology as being the canonical one, which would be "base" class?

llvm/docs/TableGen/ProgRef.rst
65–67

Here we are talking about a record inheriting from a class. I can't find any standard name for that class. Is "base class" the standard? As far as I can tell, "base," "parent," and "super" all pertain to the class hierarchy.

dblaikie added inline comments.Apr 20 2021, 4:35 PM
llvm/docs/TableGen/ProgRef.rst
65–67

In the C++ standard "base" is the term that's used - that's visible also in the C++ API with type traits like std::is_base_of ( https://en.cppreference.com/w/cpp/types/is_base_of )

llvm/docs/TableGen/ProgRef.rst
65–67

std::is_base_of() takes two classes. Here we need a term for the class that a record inherits from, not a term involving the class hierarchy. In C++ we would be talking about an instance's class, for which the term is just "class." But we don't think of records as instances.

I suppose "base class" is just as good as "parent class" and would be the correct term when we are talking about one class inheriting from another class. Okay, let me reword the text with "base class" as the primary term.

dblaikie added inline comments.Apr 20 2021, 5:03 PM
llvm/docs/TableGen/ProgRef.rst
65–67

Here we need a term for the class that a record inherits from,

Hmm, OK - maybe there isn't a good analogy with C++, then.

What if the wording here said "an optional list of classes" (rather than base/parent/super)? I guess that doesn't apply to all the other places.

I guess: How'd you pick parent rather than super or base (but not completely consistently)? Were there more uses of parent than any other, so you decided to standardize on that name?

llvm/docs/TableGen/ProgRef.rst
65–67

Yes, I tried to go with the more common term. But I also avoided superclass, which really suggests a class-to-class relationship.

The more I think about it, the more I like "parent class" rather than "base class." It's the only term that seems like it could apply to a record's class. Let me look at the uses and see if some or most can just use the term "class."

I'm going to stick with "parent class," but if there are any sentences that talk only about records, I will try to use just "class." I will add a note at the appropriate spot explaining this.

dblaikie accepted this revision.Apr 21 2021, 10:18 AM

I'm going to stick with "parent class," but if there are any sentences that talk only about records, I will try to use just "class." I will add a note at the appropriate spot explaining this.

Fair enough. I wouldn't mind @lattner 's take on this since he's probably got some history & more context with the naming.

This revision is now accepted and ready to land.Apr 21 2021, 10:18 AM

I added a note about the term "parent class." It proved futile to try to remove the word "parent" in certain sentences. Many pertain to both classes and records. And those that don't just seemed to become inconsistent when I tried to remove the word.

This revision was landed with ongoing or failed builds.Apr 23 2021, 6:49 AM
This revision was automatically updated to reflect the committed changes.