Skip to content

Commit

Permalink
[AST] Accept identical TypeConstraint referring to other template
Browse files Browse the repository at this point in the history 
parameters.

The current implementation to judge the similarity of TypeConstraint in
ASTContext::isSameTemplateParameter is problematic, it couldn't handle
the following case:

```C++
template <__integer_like _Tp, C<_Tp> Sentinel>
constexpr _Tp operator()(_Tp &&__t, Sentinel &&last) const {
    return __t;
}
```

When we see 2 such declarations from different modules, we would judge
their similarity by `ASTContext::isSame*` methods. But problems come for
the TypeConstraint. Originally, we would profile each argument one by
one. But it is not right. Since the profiling result of `_Tp` would
refer to two different template type declarations. So it would get
different results. It is right since the `_Tp` in different modules
refers to different declarations indeed. So the same declaration in
different modules would meet incorrect our-checking results.

It is not the thing we want. We want to know if the TypeConstraint have
the same expression.

Reviewer: vsapsai, ilya-biryukov

Differential Revision: https://reviews.llvm.org/D129068
  • Loading branch information
@ChuanqiXu9
ChuanqiXu9 committed Jul 12, 2022
1 parent 5d41fe0 commit f6b0ae1
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 50 deletions.
13 changes: 13 additions & 0 deletions clang/include/clang/AST/ASTContext.h
View file
Expand Up @@ -130,6 +130,7 @@ class TemplateDecl;
class TemplateParameterList;
class TemplateTemplateParmDecl;
class TemplateTypeParmDecl;
class TypeConstraint;
class UnresolvedSetIterator;
class UsingShadowDecl;
class VarTemplateDecl;
Expand Down Expand Up @@ -2664,6 +2665,18 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// that they may be used in declarations of the same template.
bool isSameTemplateParameter(const NamedDecl *X, const NamedDecl *Y) const;

/// Determine whether two 'requires' expressions are similar enough that they
/// may be used in re-declarations.
///
/// Use of 'requires' isn't mandatory, works with constraints expressed in
/// other ways too.
bool isSameConstraintExpr(const Expr *XCE, const Expr *YCE) const;

/// Determine whether two type contraint are similar enough that they could
/// used in declarations of the same template.
bool isSameTypeConstraint(const TypeConstraint *XTC,
const TypeConstraint *YTC) const;

/// Determine whether two default template arguments are similar enough
/// that they may be used in declarations of the same template.
bool isSameDefaultTemplateArgument(const NamedDecl *X,
Expand Down
105 changes: 56 additions & 49 deletions clang/lib/AST/ASTContext.cpp
View file
Expand Up @@ -6218,6 +6218,57 @@ bool ASTContext::hasSameTemplateName(const TemplateName &X,
getCanonicalTemplateName(Y).getAsVoidPointer();
}

bool ASTContext::isSameConstraintExpr(const Expr *XCE, const Expr *YCE) const {
if (!XCE != !YCE)
return false;

if (!XCE)
return true;

llvm::FoldingSetNodeID XCEID, YCEID;
XCE->Profile(XCEID, *this, /*Canonical=*/true);
YCE->Profile(YCEID, *this, /*Canonical=*/true);
return XCEID == YCEID;
}

bool ASTContext::isSameTypeConstraint(const TypeConstraint *XTC,
const TypeConstraint *YTC) const {
if (!XTC != !YTC)
return false;

if (!XTC)
return true;

auto *NCX = XTC->getNamedConcept();
auto *NCY = YTC->getNamedConcept();
if (!NCX || !NCY || !isSameEntity(NCX, NCY))
return false;
if (XTC->hasExplicitTemplateArgs() != YTC->hasExplicitTemplateArgs())
return false;
if (XTC->hasExplicitTemplateArgs())
if (XTC->getTemplateArgsAsWritten()->NumTemplateArgs !=
YTC->getTemplateArgsAsWritten()->NumTemplateArgs)
return false;

// Compare slowly by profiling.
//
// We couldn't compare the profiling result for the template
// args here. Consider the following example in different modules:
//
// template <__integer_like _Tp, C<_Tp> Sentinel>
// constexpr _Tp operator()(_Tp &&__t, Sentinel &&last) const {
// return __t;
// }
//
// When we compare the profiling result for `C<_Tp>` in different
// modules, it will compare the type of `_Tp` in different modules.
// However, the type of `_Tp` in different modules refer to different
// types here naturally. So we couldn't compare the profiling result
// for the template args directly.
return isSameConstraintExpr(XTC->getImmediatelyDeclaredConstraint(),
YTC->getImmediatelyDeclaredConstraint());
}

bool ASTContext::isSameTemplateParameter(const NamedDecl *X,
const NamedDecl *Y) const {
if (X->getKind() != Y->getKind())
Expand All @@ -6229,32 +6280,8 @@ bool ASTContext::isSameTemplateParameter(const NamedDecl *X,
return false;
if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
return false;
const TypeConstraint *TXTC = TX->getTypeConstraint();
const TypeConstraint *TYTC = TY->getTypeConstraint();
if (!TXTC != !TYTC)
return false;
if (TXTC && TYTC) {
auto *NCX = TXTC->getNamedConcept();
auto *NCY = TYTC->getNamedConcept();
if (!NCX || !NCY || !isSameEntity(NCX, NCY))
return false;
if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
return false;
if (TXTC->hasExplicitTemplateArgs()) {
auto *TXTCArgs = TXTC->getTemplateArgsAsWritten();
auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
return false;
llvm::FoldingSetNodeID XID, YID;
for (auto &ArgLoc : TXTCArgs->arguments())
ArgLoc.getArgument().Profile(XID, X->getASTContext());
for (auto &ArgLoc : TYTCArgs->arguments())
ArgLoc.getArgument().Profile(YID, Y->getASTContext());
if (XID != YID)
return false;
}
}
return true;
return isSameTypeConstraint(TX->getTypeConstraint(),
TY->getTypeConstraint());
}

if (auto *TX = dyn_cast<NonTypeTemplateParmDecl>(X)) {
Expand All @@ -6279,19 +6306,7 @@ bool ASTContext::isSameTemplateParameterList(
if (!isSameTemplateParameter(X->getParam(I), Y->getParam(I)))
return false;

const Expr *XRC = X->getRequiresClause();
const Expr *YRC = Y->getRequiresClause();
if (!XRC != !YRC)
return false;
if (XRC) {
llvm::FoldingSetNodeID XRCID, YRCID;
XRC->Profile(XRCID, *this, /*Canonical=*/true);
YRC->Profile(YRCID, *this, /*Canonical=*/true);
if (XRCID != YRCID)
return false;
}

return true;
return isSameConstraintExpr(X->getRequiresClause(), Y->getRequiresClause());
}

bool ASTContext::isSameDefaultTemplateArgument(const NamedDecl *X,
Expand Down Expand Up @@ -6489,17 +6504,9 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
return false;
}

const Expr *XRC = FuncX->getTrailingRequiresClause();
const Expr *YRC = FuncY->getTrailingRequiresClause();
if (!XRC != !YRC)
if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
FuncY->getTrailingRequiresClause()))
return false;
if (XRC) {
llvm::FoldingSetNodeID XRCID, YRCID;
XRC->Profile(XRCID, *this, /*Canonical=*/true);
YRC->Profile(YRCID, *this, /*Canonical=*/true);
if (XRCID != YRCID)
return false;
}

auto GetTypeAsWritten = [](const FunctionDecl *FD) {
// Map to the first declaration that we've already merged into this one.
Expand Down
44 changes: 43 additions & 1 deletion clang/test/Modules/concept.cppm
View file
Expand Up @@ -3,6 +3,7 @@
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t -DDIFFERENT %t/B.cppm -verify
// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/B.cppm -verify

//--- foo.h
Expand All @@ -18,6 +19,9 @@ concept __integer_like = true;
template <class _Tp>
concept __member_size = requires(_Tp &&t) { t.size(); };

template <class First, class Second>
concept C = requires(First x, Second y) { x + y; };

struct A {
public:
template <Range T>
Expand All @@ -29,6 +33,29 @@ struct __fn {
constexpr __integer_like auto operator()(_Tp&& __t) const {
return __t.size();
}

template <__integer_like _Tp, C<_Tp> Sentinel>
constexpr _Tp operator()(_Tp &&__t, Sentinel &&last) const {
return __t;
}

template <template <class> class H, class S, C<H<S>> Sentinel>
constexpr H<S> operator()(H<S> &&__s, Sentinel &&last) const {
return __s;
}

// Tests that we could find different concept definition indeed.
#ifndef DIFFERENT
template <__integer_like _Tp, __integer_like _Up, C<_Tp> Sentinel>
constexpr _Tp operator()(_Tp &&__t, _Up _u, Sentinel &&last) const {
return __t;
}
#else
template <__integer_like _Tp, __integer_like _Up, C<_Up> Sentinel>
constexpr _Tp operator()(_Tp &&__t, _Up _u, Sentinel &&last) const {
return __t;
}
#endif
};
#endif

Expand All @@ -38,17 +65,32 @@ module;
export module A;

//--- B.cppm
// expected-no-diagnostics
module;
#include "foo.h"
export module B;
import A;

#ifdef DIFFERENT
// expected-error@foo.h:41 {{'__fn::operator()' from module 'A.<global>' is not present in definition of '__fn' provided earlier}}
// expected-note@* 1+{{declaration of 'operator()' does not match}}
#else
// expected-no-diagnostics
#endif

template <class T>
struct U {
auto operator+(U) { return 0; }
};

void foo() {
A a;
struct S {
int size() { return 0; }
auto operator+(S s) { return 0; }
};
__fn{}(S());
__fn{}(S(), S());
__fn{}(S(), S(), S());

__fn{}(U<int>(), U<int>());
}

0 comments on commit f6b0ae1

Please sign in to comment.