aaron.ballman added a comment.
Precommit CI found a relevant failure that should be addressed.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593
+ const FunctionProtoType *FnType =
FD->getType()->castAs<FunctionProtoType>();
+ if (FnType->isVolatile()) {
----------------
ilya-biryukov wrote:
> NIT: this version is simpler and more aligned with the code below.
> Also, could you move the `volatile` handling after the check for `const`?
>
>
> Additionally, we seem to recover in case of `const` by adding it to the type
> and suggesting a fix-it to add it in the code.
> Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a
> fix-it to remove the `ref-qualifier` and `volatile` and change the
> corresponding type to make AST pretend they were never there?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593-8601
+ const FunctionProtoType *FnType =
FD->getType()->castAs<FunctionProtoType>();
+ if (FnType->isVolatile()) {
+ Diag(FD->getLocation(), diag::err_volatile_comparison_operator);
+ return true;
+ }
+ if (FnType->getRefQualifier() == RQ_RValue) {
+ Diag(FD->getLocation(), diag::err_ref_qualifier_comparison_operator);
----------------
aaron.ballman wrote:
> ilya-biryukov wrote:
> > NIT: this version is simpler and more aligned with the code below.
> > Also, could you move the `volatile` handling after the check for `const`?
> >
> >
> > Additionally, we seem to recover in case of `const` by adding it to the
> > type and suggesting a fix-it to add it in the code.
> > Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a
> > fix-it to remove the `ref-qualifier` and `volatile` and change the
> > corresponding type to make AST pretend they were never there?
>
Note, you can shorten it further with:
```
return Diag(...);
```
because that will return true for you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148924/new/
https://reviews.llvm.org/D148924
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
