[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Congcong Cai via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5df593a35471: [Sema] cast to CXXRecordDecl correctly when 
diag a default comparison method (authored by HerrCai0907).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
@@ -227,5 +227,5 @@
 A a;
 std::strong_ordering operator<=>(const B&) const = default; // 
expected-error {{call to deleted constructor of 'A'}}
   };
-  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'Preference::B' first required here}}
+  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'B' first required here}}
 }
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,13 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType = FD->getParamDecl(0)
+  ->getType()
+  .getNonReferenceType()
+  .getUnqualifiedType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
-<< (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< (int)DFK.asComparison() << RecordType;
   }
   break;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -445,6 +445,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo 

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 525754.
HerrCai0907 added a comment.

remove not related change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
@@ -227,5 +227,5 @@
 A a;
 std::strong_ordering operator<=>(const B&) const = default; // 
expected-error {{call to deleted constructor of 'A'}}
   };
-  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'Preference::B' first required here}}
+  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'B' first required here}}
 }
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,13 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType = FD->getParamDecl(0)
+  ->getType()
+  .getNonReferenceType()
+  .getUnqualifiedType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
-<< (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< (int)DFK.asComparison() << RecordType;
   }
   break;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator fo

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 525752.
HerrCai0907 added a comment.

avoid desurger


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
@@ -227,5 +227,5 @@
 A a;
 std::strong_ordering operator<=>(const B&) const = default; // 
expected-error {{call to deleted constructor of 'A'}}
   };
-  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'Preference::B' first required here}}
+  bool x = B() < B(); // expected-note {{in defaulted three-way comparison 
operator for 'B' first required here}}
 }
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,13 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType = FD->getParamDecl(0)
+  ->getType()
+  .getNonReferenceType()
+  .getUnqualifiedType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
-<< (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< (int)DFK.asComparison() << RecordType;
   }
   break;
 }
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -163,6 +163,7 @@
 bool WebAssemblyTargetInfo::handleTargetFeatures(
 std::vector &Features, DiagnosticsEngine &Diags) {
   for (const auto &Feature : Features) {
+llvm::errs() << Feature << "\n";
 if (Feature == "+simd128") {
   SIMDLevel = std::max(SIMDLevel, SIMD128);
   continue;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// 

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:968
+.getNonReferenceType()
+->getLocallyUnqualifiedSingleStepDesugaredType();
 Diags.Report(Active->PointOfInstantiation,

This doesn't seem right to me?  I think you just want the unqualified, 
non-reference type here.  I suspect you don't really want to do Desugaring at 
all, since then you lose the typedef information (so something like std::string 
will be confusing).

So I think you just want: `.getNonReferenceType().getUnqualifiedType()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:969
 << (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< Context.getRecordType(RecordType->getAsRecordDecl());
   }

erichkeane wrote:
> What is going on here?  You already have the `RecordType` in the RecordType 
> variable.  You don't need to go to RecordDecl and back?  You should just be 
> able to put the `RecordType` variable directly.  
I am not very familiar with QualType's conversion. Do you think it is the 
correct path to remove all qualifier and get the full name for class?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 525718.
HerrCai0907 added a comment.
Herald added subscribers: pmatos, asb, aheejin, jgravelle-google, sbc100, 
dschuff.

getLocallyUnqualifiedSingleStepDesugaredType to find the whole record name


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,14 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType =
+FD->getParamDecl(0)
+->getType()
+.getNonReferenceType()
+->getLocallyUnqualifiedSingleStepDesugaredType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
-<< (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< (int)DFK.asComparison() << RecordType;
   }
   break;
 }
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -163,6 +163,7 @@
 bool WebAssemblyTargetInfo::handleTargetFeatures(
 std::vector &Features, DiagnosticsEngine &Diags) {
   for (const auto &Feature : Features) {
+llvm::errs() << Feature << "\n";
 if (Feature == "+simd128") {
   SIMDLevel = std::max(SIMDLevel, SIMD128);
   continue;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defa

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:969
 << (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< Context.getRecordType(RecordType->getAsRecordDecl());
   }

What is going on here?  You already have the `RecordType` in the RecordType 
variable.  You don't need to go to RecordDecl and back?  You should just be 
able to put the `RecordType` variable directly.  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-24 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 525452.
HerrCai0907 added a comment.

format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,12 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType =
+FD->getParamDecl(0)->getType().getNonReferenceType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
 << (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< Context.getRecordType(RecordType->getAsRecordDecl());
   }
   break;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,12 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType =
+FD->getParamDecl(0)->getType().getNonReferenceType();

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-24 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 525333.
HerrCai0907 added a comment.

change solution


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/cxx20-default-compare.cpp


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning 
{{comparing floating point with == or != is unsafe}} expected-note {{in 
defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. 
Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating 
point with == or != is unsafe}} expected-note {{in defaulted equality 
comparison operator for 'Foo' first required here}}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,13 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType = FD->getParamDecl(0)
+  ->getType()
+  .getNonReferenceType();
 Diags.Report(Active->PointOfInstantiation,
  diag::note_comparison_synthesized_at)
 << (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< Context.getRecordType(RecordType->getAsRecordDecl());
   }
   break;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,9 @@
   (`#62789 `_).
 - Fix a crash when instantiating a non-type template argument in a dependent 
scope.
   (`#62533 `_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 `_) and
+  (`#62102 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/cxx20-default-compare.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
+
+struct Foo {
+  float val;
+  bool operator==(const Foo &) const;
+  friend bool operator==(const Foo &, const Foo &);
+  friend bool operator==(Foo, Foo );
+};
+
+// Declare the defaulted comparison function as a member function.
+bool Foo::operator==(const Foo &) const = default; // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function.
+bool operator==(const Foo &, const Foo &) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+// Declare the defaulted comparison function as a non-member function. Arguments are passed by value.
+bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -961,11 +961,13 @@
 << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
 << Context.getTagDeclType(MD->getParent());
   } else if (DFK.isComparison()) {
+QualType RecordType = FD->get

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D151365#4369650 , @HerrCai0907 
wrote:

> In D151365#4369605 , @erichkeane 
> wrote:
>
>> As Richard says, we should just be taking the first parameter type and using 
>> that instead (obviously de-qualified and removing reference if necessary).
>
> But we still can find the correct RecordDecl by `redecls`.

That ends up being an absurdly larger amount of work, and in the case of a 
'friend' declaration can result in the wrong answer, correct?  Consider 
something like:

  struct Foo;
  
  struct Bar {
  friend bool operator==(const Foo&, const Foo&);
  };
  
  struct Foo {
  friend bool operator==(const Foo&, const Foo&);
  };
  
  bool operator==(const Foo&, const Foo&) = default;

or even:

  struct Foo {
  bool operator==(const Foo&) const;
  };
  
  struct Bar {
  friend bool Foo::operator==(const Foo&) const;
  };
  
  
  bool operator==(Foo, Foo) = default;

In the 1st one, the lexical context could be 'Bar'.  Searching 'redecls' will 
end up with the 'first one first', so at least the 2nd example isn't as 
problematic, but I'm sure someone more clever than I could come up with a case 
where that could be declared in a different order.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-24 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 added a comment.

In D151365#4369605 , @erichkeane 
wrote:

> As Richard says, we should just be taking the first parameter type and using 
> that instead (obviously de-qualified and removing reference if necessary).

But we still can find the correctly RecordDecl by `redecls`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

2023-05-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

As Richard says, we should just be taking the first parameter type and using 
that instead (obviously de-qualified and removing reference if necessary).




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:964
   } else if (DFK.isComparison()) {
-Diags.Report(Active->PointOfInstantiation,
- diag::note_comparison_synthesized_at)
-<< (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+CXXRecordDecl *RD = nullptr;
+for (Decl *D : FD->redecls()) {

According to the comment on the related bug:

```
This code predates P2085R0, prior to which a comparison function could only be 
defaulted lexically within the class. Given that we can't use the 
lexically-enclosing class any more, we can instead look at the (first) 
parameter type to determine which class's comparison is being defaulted.
...
since the lexical context isn't the right thing for us to be looking at any more
...
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151365/new/

https://reviews.llvm.org/D151365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits