[PATCH] D157029: [llvm] Construct option's prefixed name at compile-time
Jan Svoboda via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 4 11:59:03 PDT 2023
jansvoboda11 added inline comments.
================
Comment at: llvm/include/llvm/Option/Option.h:103
+ StringLiteral getSpelling() const {
+ assert(Info && "Must have a valid info!");
----------------
benlangmuir wrote:
> This could use a doc comment to differentiate it from other string representations.
>
> How does this compare with `Arg::getSpelling`? With `Arg`, IIUC the "spelling" is how it was actually written rather than a canonical form. That might be confusing if this one is canonical; so we should at least clearly document it or maybe put "canonical" in the API name?
You're right `Arg::getSpelling()` is the as-written prefix and name, while `Option::getSpelling()` was the canonical prefix and name. I noticed there's also `Option::getPrefixedName()` which used to return `std::string`. I decided to reuse that and return `StringLiteral` instead. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157029/new/
https://reviews.llvm.org/D157029
More information about the cfe-commits
mailing list