jroelofs marked an inline comment as done.
jroelofs added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46
bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
----------------
njames93 wrote:
> jroelofs wrote:
> > njames93 wrote:
> > > Doesn't belong in this patch, but this function should be changed in a
> > > follow up to return an `Optional<FileExtensionSet>`.
> > Looking at the uses, I'm not convinced this would be better in this
> > specific case.
> >
> > Compare:
> > ```
> > if (Optional<FileExtensionSet> HFE = parseFileExtensions(Extensions,
> > Delimiters)) {
> > HeaderFileExtensions = *HFE;
> > } else {
> > errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> >
> > With the status quo:
> > ```
> > if (!parseFileExtensions(Extensions, HeaderFileExtensions, Delimiters)) {
> > errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> >
> > Optional's explicit operator bool is great for when you want to guard on
> > the presence of the thing, but not so great when you want to check for it
> > not being there. I feel like we'd need some new utility to go with Optional
> > to make this nice:
> >
> > ```
> > if (not(HeaderFileExtensions) = parseFileExtensions(Extensions,
> > Delimiters)) {
> > errs() << "Invalid extensions: " << Extensions;
> > }
> > ```
> To be honest it should probably be `Expected` rather than `Optional`, but
> that still doesn't help solve the cleanliness issue.
Implementation of the `not` idea here:
https://gcc.godbolt.org/z/XZh39g
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75621/new/
https://reviews.llvm.org/D75621
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
