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

[HerrCai0907]

Summary

Fixed: https://github.com/llvm/llvm-project/issues/62791
Fixed: https://github.com/llvm/llvm-project/issues/62102
in c++20, default comparison is supported. getLexicalDeclContext maybe cannot
get the CXXRecord if default comparison defined out of CXXRecord.
This patch want to get these information from the first function argument.

[shafik]

Comments here are relavant: https://github.com/llvm/llvm-project/issues/62102

[erichkeane]

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

clang/lib/Sema/SemaTemplateInstantiate.cpp

970

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
...

[HerrCai0907]

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.

[erichkeane]

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.

[HerrCai0907]

change solution

[HerrCai0907]

format

[erichkeane]

clang/lib/Sema/SemaTemplateInstantiate.cpp

970

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.

[HerrCai0907]

getLocallyUnqualifiedSingleStepDesugaredType to find the whole record name

[HerrCai0907]

clang/lib/Sema/SemaTemplateInstantiate.cpp

970

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?

[erichkeane]

clang/lib/Sema/SemaTemplateInstantiate.cpp

968

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().

[HerrCai0907]

avoid desurger

[HerrCai0907]

remove not related change

[erichkeane]

LGTM, thanks!

clang/docs/ReleaseNotes.rst

 - Clang `TextNodeDumper` enabled through `-ast-dump` flag no longer evaluates the
   initializer of constexpr `VarDecl` if the declaration has a dependent type.
 - Match GCC's behavior for ``__builtin_object_size`` and
   ``__builtin_dynamic_object_size`` on structs containing flexible array
   members.
   (`#62789 <https://github.com/llvm/llvm-project/issues/62789>`_).
 - Fix a crash when instantiating a non-type template argument in a dependent scope.
   (`#62533 <https://github.com/llvm/llvm-project/issues/62533>`_).
+- Fix crash when diagnosing default comparison method.
+  (`#62791 <https://github.com/llvm/llvm-project/issues/62791>`_) and
+  (`#62102 <https://github.com/llvm/llvm-project/issues/62102>`_).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug where attribute annotations on type specifiers (enums, classes,
   structs, unions, and scoped enums) were not properly ignored, resulting in

clang/lib/Sema/SemaTemplateInstantiate.cpp

case CodeSynthesisContext::DefiningSynthesizedFunction: {
           FD ? getDefaultedFunctionKind(FD) : DefaultedFunctionKind();
       if (DFK.isSpecialMember()) {
         auto *MD = cast<CXXMethodDecl>(FD);
         Diags.Report(Active->PointOfInstantiation,
                      diag::note_member_synthesized_at)
             << 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<CXXRecordDecl>(FD->getLexicalDeclContext()));
+            << Context.getRecordType(RecordType->getAsRecordDecl());
       }
       break;
     }
 
     case CodeSynthesisContext::RewritingOperatorAsSpaceship:
       Diags.Report(Active->Entity->getLocation(),
                    diag::note_rewriting_operator_as_spaceship);
       break;

clang/test/SemaCXX/cxx20-default-compare.cpp

+// 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}}