[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