mizvekov added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:5425
Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
- if (CCGenDiagnostics && A) {
- SmallString<128> CrashDirectory(A->getValue());
+ const char *CrashDirectory = CCGenDiagnostics && A
+ ? A->getValue()
----------------
erichkeane wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > `StringRef` would be better here instead, which should mean you don't
> > > have to create the SmallString below, and could just work in `Twine`s for
> > > everything.
> > It seems that `llvm::sys::path::append` takes twine as inputs, but it only
> > outputs to a SmallVectorImpl.
> > Unless there is something else I could use?
> Ah, yikes, I missed that was what was happening :/ THAT is still necessary.
> A Twine can be created from a stringref (and I'd still prefer one anyway),
> but you DO still have to make that SmallString unfortunately.
The Twine also can't take a nullptr (such as the case no flag passed and no env
variable), so we would have to use something like `std::optional<Twine>`, and
make the needed conversions here.
And the Twine to SmallString copy looks unergonomic as well.
So it seems we would need to make some little changes out of scope of this MR
to get that to look nicely.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133082/new/
https://reviews.llvm.org/D133082
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
