[clang] Fix CodeComplete crash involving CWG1432 #129436
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This skips the provisional resolution of CWG1432 just when ordering the candidates for function call code completion, as otherwise this breaks some assumptions the implementation makes about how closely related the candidates are. As a drive-by, deduplicate the implementation with the one used for class template partial ordering, and strenghten an assertion which was previosuly dependent on the order of candidates. Also add a test for the fix for CWG1432 when partial ordering function templates, which was otherwise untested.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis skips the provisional resolution of CWG1432 just when ordering the candidates for function call code completion, as otherwise this breaks some assumptions the implementation makes about how closely related the candidates are. As a drive-by, deduplicate the implementation with the one used for class template partial ordering, and strenghten an assertion which was previosuly dependent on the order of candidates. Also add a test for the fix for CWG1432 when partial ordering function templates, which was otherwise untested. Full diff: https://github.com/llvm/llvm-project/pull/129436.diff 8 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c97bbc372fcb7..0046bbf9901a3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,7 +159,7 @@ related warnings within the method body.
print_status("%s (%#08x)\n");
// order of %s and %x is swapped but there is no diagnostic
}
-
+
Before the introducion of ``format_matches``, this code cannot be verified
at compile-time. ``format_matches`` plugs that hole:
@@ -227,6 +227,8 @@ Bug Fixes in This Version
signed char values (#GH102798).
- Fixed rejects-valid problem when #embed appears in std::initializer_list or
when it can affect template argument deduction (#GH122306).
+- Fix crash on code completion of function calls involving partial order of function templates
+ (#GH125500).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index c03ec00d03dc5..6e08762dcc6d7 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -1265,11 +1265,11 @@ class Sema;
};
- bool isBetterOverloadCandidate(Sema &S,
- const OverloadCandidate &Cand1,
+ bool isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1,
const OverloadCandidate &Cand2,
SourceLocation Loc,
- OverloadCandidateSet::CandidateSetKind Kind);
+ OverloadCandidateSet::CandidateSetKind Kind,
+ bool PartialOverloading = false);
struct ConstructorInfo {
DeclAccessPair FoundDecl;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3b2be86a88e82..5b5cee5810488 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12617,7 +12617,8 @@ class Sema final : public SemaBase {
FunctionTemplateDecl *getMoreSpecializedTemplate(
FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
- QualType RawObj1Ty = {}, QualType RawObj2Ty = {}, bool Reversed = false);
+ QualType RawObj1Ty = {}, QualType RawObj2Ty = {}, bool Reversed = false,
+ bool PartialOverloading = false);
/// Retrieve the most specialized of the given function template
/// specializations.
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 80ae87e7c5725..db467d76b5d32 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -6229,8 +6229,8 @@ static void mergeCandidatesWithResults(
// Sort the overload candidate set by placing the best overloads first.
llvm::stable_sort(CandidateSet, [&](const OverloadCandidate &X,
const OverloadCandidate &Y) {
- return isBetterOverloadCandidate(SemaRef, X, Y, Loc,
- CandidateSet.getKind());
+ return isBetterOverloadCandidate(SemaRef, X, Y, Loc, CandidateSet.getKind(),
+ /*PartialOverloading=*/true);
});
// Add the remaining viable overload candidates as code-completion results.
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index c344b6fff40c6..d3c0534b4dd0b 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10429,7 +10429,8 @@ getMorePartialOrderingConstrained(Sema &S, FunctionDecl *Fn1, FunctionDecl *Fn2,
/// candidate is a better candidate than the second (C++ 13.3.3p1).
bool clang::isBetterOverloadCandidate(
Sema &S, const OverloadCandidate &Cand1, const OverloadCandidate &Cand2,
- SourceLocation Loc, OverloadCandidateSet::CandidateSetKind Kind) {
+ SourceLocation Loc, OverloadCandidateSet::CandidateSetKind Kind,
+ bool PartialOverloading) {
// Define viable functions to be better candidates than non-viable
// functions.
if (!Cand2.Viable)
@@ -10666,7 +10667,7 @@ bool clang::isBetterOverloadCandidate(
: QualType{},
Obj2Context ? QualType(Obj2Context->getTypeForDecl(), 0)
: QualType{},
- Cand1.isReversed() ^ Cand2.isReversed())) {
+ Cand1.isReversed() ^ Cand2.isReversed(), PartialOverloading)) {
return BetterTemplate == Cand1.Function->getPrimaryTemplate();
}
}
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index dbd73ead8a63f..9769848840d87 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5862,10 +5862,42 @@ static bool isAtLeastAsSpecializedAs(
return true;
}
+enum class MoreSpecializedTrailingPackTieBreakerResult { Equal, Less, More };
+
+// This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
+// there is no wording or even resolution for this issue.
+static MoreSpecializedTrailingPackTieBreakerResult
+getMoreSpecializedTrailingPackTieBreaker(
+ const TemplateSpecializationType *TST1,
+ const TemplateSpecializationType *TST2) {
+ ArrayRef<TemplateArgument> As1 = TST1->template_arguments(),
+ As2 = TST2->template_arguments();
+ const TemplateArgument &TA1 = As1.back(), &TA2 = As2.back();
+ bool IsPack = TA1.getKind() == TemplateArgument::Pack;
+ assert(IsPack == (TA2.getKind() == TemplateArgument::Pack));
+ if (!IsPack)
+ return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+ assert(As1.size() == As2.size());
+
+ unsigned PackSize1 = TA1.pack_size(), PackSize2 = TA2.pack_size();
+ bool IsPackExpansion1 =
+ PackSize1 && TA1.pack_elements().back().isPackExpansion();
+ bool IsPackExpansion2 =
+ PackSize2 && TA2.pack_elements().back().isPackExpansion();
+ if (PackSize1 == PackSize2 && IsPackExpansion1 == IsPackExpansion2)
+ return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+ if (PackSize1 > PackSize2 && IsPackExpansion1)
+ return MoreSpecializedTrailingPackTieBreakerResult::More;
+ if (PackSize1 < PackSize2 && IsPackExpansion2)
+ return MoreSpecializedTrailingPackTieBreakerResult::Less;
+ return MoreSpecializedTrailingPackTieBreakerResult::Equal;
+}
+
FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
FunctionTemplateDecl *FT1, FunctionTemplateDecl *FT2, SourceLocation Loc,
TemplatePartialOrderingContext TPOC, unsigned NumCallArguments1,
- QualType RawObj1Ty, QualType RawObj2Ty, bool Reversed) {
+ QualType RawObj1Ty, QualType RawObj2Ty, bool Reversed,
+ bool PartialOverloading) {
SmallVector<QualType> Args1;
SmallVector<QualType> Args2;
const FunctionDecl *FD1 = FT1->getTemplatedDecl();
@@ -6001,34 +6033,27 @@ FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
return FT1;
}
- // This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
- // there is no wording or even resolution for this issue.
- for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+ // Skip this tie breaker if we are performing overload resolution with partial
+ // arguments, as this breaks some assumptions about how closely related the
+ // candidates are.
+ for (int i = 0, e = std::min(NumParams1, NumParams2);
+ !PartialOverloading && i < e; ++i) {
QualType T1 = Param1[i].getCanonicalType();
QualType T2 = Param2[i].getCanonicalType();
auto *TST1 = dyn_cast<TemplateSpecializationType>(T1);
auto *TST2 = dyn_cast<TemplateSpecializationType>(T2);
if (!TST1 || !TST2)
continue;
- const TemplateArgument &TA1 = TST1->template_arguments().back();
- if (TA1.getKind() == TemplateArgument::Pack) {
- assert(TST1->template_arguments().size() ==
- TST2->template_arguments().size());
- const TemplateArgument &TA2 = TST2->template_arguments().back();
- assert(TA2.getKind() == TemplateArgument::Pack);
- unsigned PackSize1 = TA1.pack_size();
- unsigned PackSize2 = TA2.pack_size();
- bool IsPackExpansion1 =
- PackSize1 && TA1.pack_elements().back().isPackExpansion();
- bool IsPackExpansion2 =
- PackSize2 && TA2.pack_elements().back().isPackExpansion();
- if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
- if (PackSize1 > PackSize2 && IsPackExpansion1)
- return FT2;
- if (PackSize1 < PackSize2 && IsPackExpansion2)
- return FT1;
- }
+ switch (getMoreSpecializedTrailingPackTieBreaker(TST1, TST2)) {
+ case MoreSpecializedTrailingPackTieBreakerResult::Less:
+ return FT1;
+ case MoreSpecializedTrailingPackTieBreakerResult::More:
+ return FT2;
+ case MoreSpecializedTrailingPackTieBreakerResult::Equal:
+ continue;
}
+ llvm_unreachable(
+ "unknown MoreSpecializedTrailingPackTieBreakerResult value");
}
if (!Context.getLangOpts().CPlusPlus20)
@@ -6375,28 +6400,15 @@ getMoreSpecialized(Sema &S, QualType T1, QualType T2, TemplateLikeDecl *P1,
if (!Better1 && !Better2)
return nullptr;
- // This a speculative fix for CWG1432 (Similar to the fix for CWG1395) that
- // there is no wording or even resolution for this issue.
- auto *TST1 = cast<TemplateSpecializationType>(T1);
- auto *TST2 = cast<TemplateSpecializationType>(T2);
- const TemplateArgument &TA1 = TST1->template_arguments().back();
- if (TA1.getKind() == TemplateArgument::Pack) {
- assert(TST1->template_arguments().size() ==
- TST2->template_arguments().size());
- const TemplateArgument &TA2 = TST2->template_arguments().back();
- assert(TA2.getKind() == TemplateArgument::Pack);
- unsigned PackSize1 = TA1.pack_size();
- unsigned PackSize2 = TA2.pack_size();
- bool IsPackExpansion1 =
- PackSize1 && TA1.pack_elements().back().isPackExpansion();
- bool IsPackExpansion2 =
- PackSize2 && TA2.pack_elements().back().isPackExpansion();
- if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
- if (PackSize1 > PackSize2 && IsPackExpansion1)
- return GetP2()(P1, P2);
- if (PackSize1 < PackSize2 && IsPackExpansion2)
- return P1;
- }
+ switch (getMoreSpecializedTrailingPackTieBreaker(
+ cast<TemplateSpecializationType>(T1),
+ cast<TemplateSpecializationType>(T2))) {
+ case MoreSpecializedTrailingPackTieBreakerResult::Less:
+ return P1;
+ case MoreSpecializedTrailingPackTieBreakerResult::More:
+ return GetP2()(P1, P2);
+ case MoreSpecializedTrailingPackTieBreakerResult::Equal:
+ break;
}
if (!S.Context.getLangOpts().CPlusPlus20)
diff --git a/clang/test/CXX/drs/cwg14xx.cpp b/clang/test/CXX/drs/cwg14xx.cpp
index 51bc9614299a5..759a402b5f9d6 100644
--- a/clang/test/CXX/drs/cwg14xx.cpp
+++ b/clang/test/CXX/drs/cwg14xx.cpp
@@ -59,22 +59,34 @@ namespace cwg1423 { // cwg1423: 11
namespace cwg1432 { // cwg1432: 16
#if __cplusplus >= 201103L
- template<typename T> T declval();
+ namespace class_template_partial_spec {
+ template<typename T> T declval();
- template <class... T>
- struct common_type;
+ template <class... T>
+ struct common_type;
- template <class T, class U>
- struct common_type<T, U> {
- typedef decltype(true ? declval<T>() : declval<U>()) type;
- };
+ template <class T, class U>
+ struct common_type<T, U> {
+ typedef decltype(true ? declval<T>() : declval<U>()) type;
+ };
- template <class T, class U, class... V>
- struct common_type<T, U, V...> {
- typedef typename common_type<typename common_type<T, U>::type, V...>::type type;
- };
+ template <class T, class U, class... V>
+ struct common_type<T, U, V...> {
+ typedef typename common_type<typename common_type<T, U>::type, V...>::type type;
+ };
+
+ template struct common_type<int, double>;
+ } // namespace class_template_partial_spec
+ namespace function_template {
+ template <int I, class... Ts> struct A {};
- template struct common_type<int, double>;
+ template <int I, class... Ts> void f(A<I, Ts...>) = delete;
+ template <int I> void f(A<I>);
+
+ void test() {
+ f(A<0>());
+ }
+ } // namespace function_template
#endif
} // namespace cwg1432
diff --git a/clang/test/CodeCompletion/GH125500.cpp b/clang/test/CodeCompletion/GH125500.cpp
new file mode 100644
index 0000000000000..d2d58469084dc
--- /dev/null
+++ b/clang/test/CodeCompletion/GH125500.cpp
@@ -0,0 +1,38 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+template <int...> struct B {};
+template <int> class C;
+
+namespace method {
+ struct S {
+ template <int Z>
+ void waldo(C<Z>);
+
+ template <int... Is, int Z>
+ void waldo(B<Is...>, C<Z>);
+ };
+
+ void foo() {
+ S().waldo(/*invoke completion here*/);
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-1):15 %s -o - | FileCheck -check-prefix=CHECK-METHOD %s
+ // CHECK-METHOD-LABEL: OPENING_PAREN_LOC:
+ // CHECK-METHOD-NEXT: OVERLOAD: [#void#]waldo(<#C<Z>#>)
+ // CHECK-METHOD-NEXT: OVERLOAD: [#void#]waldo(<#B<>#>, C<Z>)
+ }
+} // namespace method
+namespace function {
+ template <int Z>
+ void waldo(C<Z>);
+
+ template <int... Is, int Z>
+ void waldo(B<Is...>, C<Z>);
+
+ void foo() {
+ waldo(/*invoke completion here*/);
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-1):11 %s -o - | FileCheck -check-prefix=CHECK-FUNC %s
+ // CHECK-FUNC-LABEL: OPENING_PAREN_LOC:
+ // CHECK-FUNC-NEXT: OVERLOAD: [#void#]waldo(<#B<>#>, C<Z>)
+ // CHECK-FUNC-NEXT: OVERLOAD: [#void#]waldo(<#C<Z>#>)
+ }
+} // namespace function
|
Endilll
reviewed
Mar 2, 2025
Yep, it was really just untested for that case, and we could delete the copy of the implementation for function templates, and all tests still passed. |
erichkeane
approved these changes
Mar 3, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/8682 Here is the relevant piece of the build log for the reference |
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Mar 21, 2025
This skips the provisional resolution of CWG1432 just when ordering the candidates for function call code completion, as otherwise this breaks some assumptions the implementation makes about how closely related the candidates are. As a drive-by, deduplicate the implementation with the one used for class template partial ordering, and strenghten an assertion which was previosuly dependent on the order of candidates. Also add a test for the fix for CWG1432 when partial ordering function templates, which was otherwise untested. Fixes llvm#125500
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This skips the provisional resolution of CWG1432 just when ordering the candidates for function call code completion, as otherwise this breaks some assumptions the implementation makes about how closely related the candidates are.
As a drive-by, deduplicate the implementation with the one used for class template partial ordering, and strenghten an assertion which was previosuly dependent on the order of candidates.
Also add a test for the fix for CWG1432 when partial ordering function templates, which was otherwise untested.
Fixes #125500