cor3ntin marked 2 inline comments as done.
cor3ntin added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7260-7265
+def ext_deducing_this : ExtWarn<
+ "explicit object parameters are a C++2b extension">,
+ InGroup<CXX23>;
+def warn_cxx20_ext_deducing_this : Warning<
+ "explicit object parameters are incompatible with C++ standards before
C++2b">,
+ DefaultIgnore, InGroup<CXXPre23Compat>;
----------------
aaron.ballman wrote:
> Missing test coverage for both of these.
>
> What is the rationale for exposing this as an extension in earlier language
> modes instead of leaving this a C++26 and later feature?
Can you clarify, what do you think is missing test coverage?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1397
+ if (!HaveCorrespondingObjectParameters) {
+ DiagnoseInconsistentRefQualifiers();
+ // CWG2554
----------------
aaron.ballman wrote:
> Should you be looking at the return value here?
Might as well. Not sure it changes anything.
================
Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:192
+ f(); // expected-error{{call to non-static member function
without an object argument}}
+ f(Over_Call_Func_Example{}); // expected-error{{call to non-static
member function without an object argument}}
+ Over_Call_Func_Example{}.f(); // ok
----------------
aaron.ballman wrote:
> Errr, this diagnostic seems likely to be hard for users to act on: this
> non-static member function has an object argument! And it's even the right
> type!
>
> If the model for the feature is "these are just static functions with funky
> lookup rules", I kind of wonder if this should be accepted instead of
> rejected. But if it needs to be rejected, I think we should have a better
> diagnostic, perhaps along the lines of "a call to an explicit object member
> function cannot specify the explicit object argument" with a fix-it to remove
> that argument. WDYT?
UGH, phab is confused, I'm no longer sure which diag you are concerned about...
================
Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:29-30
+ S(this auto); // expected-error {{an explicit object parameter cannot
appear in a constructor}}
+ ~S(this S) {} // expected-error {{an explicit object parameter cannot
appear in a destructor}} \
+ // expected-error {{destructor cannot have any parameters}}
+};
----------------
aaron.ballman wrote:
> If possible, it would be nicer if we only issued one warning as the root
> cause is the same for both diagnostics.
Should we simply remove the newly added diagnostic then?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140828/new/
https://reviews.llvm.org/D140828
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
