Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

source_location 18.x regression from 8c2b0d4175dcfe669a43d0173fd00ed3 #81155

Closed
AaronBallman opened this issue Feb 8, 2024 · 3 comments · Fixed by #106925
Closed

source_location 18.x regression from 8c2b0d4175dcfe669a43d0173fd00ed3 #81155

AaronBallman opened this issue Feb 8, 2024 · 3 comments · Fixed by #106925
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" regression rejects-valid

Comments

@AaronBallman
Copy link
Collaborator

AaronBallman commented Feb 8, 2024

This was found internally at Intel from some customer code. The original test case was stripped down to:

template <bool> using enable_if_t = int;
template <class Ty> Ty Returns_exactly() noexcept;
struct InvokeBase {
  template <class Fx, class... Args>
  static auto Call(Fx func, Args... args) -> decltype(func(args...));
};
template <class Fx1, class Fx2 = Fx1, bool = false> struct Invoke;
template <class Fx1, class Fx2> struct Invoke<Fx1, Fx2, false> : InvokeBase {};
template <int> struct F { };
template <class...> struct InvocableR;
template <class Rx, class Callable, class... Args>
struct InvocableR<Rx, Callable, Args...> : 
  F<noexcept(Invoke<Rx>::Call(Returns_exactly<Rx>(), Returns_exactly<Callable>()))> {
  static constexpr bool value = false;
};
template <class Rx, class... Args> constexpr bool IsInvocableRV = InvocableR<Rx, Args...>::value;
template <class, class... Types> class FuncClass {
public:
  template <class Fx> using EnableIfCallable = enable_if_t<IsInvocableRV<Fx, Types...>>;
};
template <class> struct FuncImpl;
template <class Ret, class... Types> struct FuncImpl<Ret(Types...)> {
  using type = FuncClass<Ret, Types...>;
};
template <class FTy> class function {
  using Base = typename FuncImpl<FTy>::type;

public:
  template <class Fx, typename Base::template EnableIfCallable<Fx> = 0> function(Fx) {}
};

struct codeloc {
  static constexpr codeloc current_location(const char *name = __builtin_FUNCTION()) {
    return codeloc();
  }
  constexpr codeloc() {}
};
class buff {
public:
  buff(buff &, codeloc = codeloc::current_location());
};
void Help(function<void(buff)>) {
}
void Test() {
  Help([](buff) {});
}

https://godbolt.org/z/qao5jhv1e

And with help from @Endilll it was further reduced to:

struct buff {
  buff(buff &, const char * = __builtin_FUNCTION());
};

template <class Ty>
Ty declval();

template <class Fx>
auto Call(buff arg) -> decltype(Fx{}(arg));

template <typename>
struct F {};

template <class Fx>
struct InvocableR : F<decltype(Call<Fx>(declval<buff>()))> {
  static constexpr bool value = false;
};

template <class Fx, bool = InvocableR<Fx>::value>
void Help(Fx) {}

void Test() {
  Help([](buff) {});
}

https://godbolt.org/z/3n3K46WT8

It's possibly worth noting that the behavior may be different on Windows than on Linux.

CC @cor3ntin

@AaronBallman AaronBallman added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" regression rejects-valid labels Feb 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2024

@llvm/issue-subscribers-c-20

Author: Aaron Ballman (AaronBallman)

This was found internally at Intel from some customer code. The original test case was stripped down to: ``` template <bool> using enable_if_t = int; template <class Ty> Ty Returns_exactly() noexcept; struct InvokeBase { template <class Fx, class... Args> static auto Call(Fx func, Args... args) -> decltype(func(args...)); }; template <class Fx1, class Fx2 = Fx1, bool = false> struct Invoke; template <class Fx1, class Fx2> struct Invoke<Fx1, Fx2, false> : InvokeBase {}; template <int> struct F { }; template <class...> struct InvocableR; template <class Rx, class Callable, class... Args> struct InvocableR<Rx, Callable, Args...> : F<noexcept(Invoke<Rx>::Call(Returns_exactly<Rx>(), Returns_exactly<Callable>()))> { static constexpr bool value = false; }; template <class Rx, class... Args> constexpr bool IsInvocableRV = InvocableR<Rx, Args...>::value; template <class, class... Types> class FuncClass { public: template <class Fx> using EnableIfCallable = enable_if_t<IsInvocableRV<Fx, Types...>>; }; template <class> struct FuncImpl; template <class Ret, class... Types> struct FuncImpl<Ret(Types...)> { using type = FuncClass<Ret, Types...>; }; template <class FTy> class function { using Base = typename FuncImpl<FTy>::type;

public:
template <class Fx, typename Base::template EnableIfCallable<Fx> = 0> function(Fx) {}
};

struct codeloc {
static constexpr codeloc current_location(const char *name = __builtin_FUNCTION()) {
return codeloc();
}
constexpr codeloc() {}
};
class buff {
public:
buff(buff &, codeloc = codeloc::current_location());
};
void Help(function<void(buff)>) {
}
void Test() {
Help( {});
}

https://godbolt.org/z/qao5jhv1e

And with help from @<!-- -->Endilll it was further reduced to:

struct buff {
buff(buff &, const char * = __builtin_FUNCTION());
};

template <class Ty>
Ty declval();

template <class Fx>
auto Call(buff arg) -> decltype(Fx{}(arg));

template <typename>
struct F {};

template <class Fx>
struct InvocableR : F<decltype(Call<Fx>(declval<buff>()))> {
static constexpr bool value = false;
};

template <class Fx, bool = InvocableR<Fx>::value>
void Help(Fx) {}

void Test() {
Help( {});
}

https://godbolt.org/z/3n3K46WT8

It's possibly worth noting that the behavior may be different on Windows than on Linux.

CC @<!-- -->cor3ntin 
</details>

@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2024

@llvm/issue-subscribers-clang-frontend

Author: Aaron Ballman (AaronBallman)

This was found internally at Intel from some customer code. The original test case was stripped down to: ``` template <bool> using enable_if_t = int; template <class Ty> Ty Returns_exactly() noexcept; struct InvokeBase { template <class Fx, class... Args> static auto Call(Fx func, Args... args) -> decltype(func(args...)); }; template <class Fx1, class Fx2 = Fx1, bool = false> struct Invoke; template <class Fx1, class Fx2> struct Invoke<Fx1, Fx2, false> : InvokeBase {}; template <int> struct F { }; template <class...> struct InvocableR; template <class Rx, class Callable, class... Args> struct InvocableR<Rx, Callable, Args...> : F<noexcept(Invoke<Rx>::Call(Returns_exactly<Rx>(), Returns_exactly<Callable>()))> { static constexpr bool value = false; }; template <class Rx, class... Args> constexpr bool IsInvocableRV = InvocableR<Rx, Args...>::value; template <class, class... Types> class FuncClass { public: template <class Fx> using EnableIfCallable = enable_if_t<IsInvocableRV<Fx, Types...>>; }; template <class> struct FuncImpl; template <class Ret, class... Types> struct FuncImpl<Ret(Types...)> { using type = FuncClass<Ret, Types...>; }; template <class FTy> class function { using Base = typename FuncImpl<FTy>::type;

public:
template <class Fx, typename Base::template EnableIfCallable<Fx> = 0> function(Fx) {}
};

struct codeloc {
static constexpr codeloc current_location(const char *name = __builtin_FUNCTION()) {
return codeloc();
}
constexpr codeloc() {}
};
class buff {
public:
buff(buff &, codeloc = codeloc::current_location());
};
void Help(function<void(buff)>) {
}
void Test() {
Help( {});
}

https://godbolt.org/z/qao5jhv1e

And with help from @<!-- -->Endilll it was further reduced to:

struct buff {
buff(buff &, const char * = __builtin_FUNCTION());
};

template <class Ty>
Ty declval();

template <class Fx>
auto Call(buff arg) -> decltype(Fx{}(arg));

template <typename>
struct F {};

template <class Fx>
struct InvocableR : F<decltype(Call<Fx>(declval<buff>()))> {
static constexpr bool value = false;
};

template <class Fx, bool = InvocableR<Fx>::value>
void Help(Fx) {}

void Test() {
Help( {});
}

https://godbolt.org/z/3n3K46WT8

It's possibly worth noting that the behavior may be different on Windows than on Linux.

CC @<!-- -->cor3ntin 
</details>

@cor3ntin
Copy link
Contributor

I spent some time looking into this, I confirm the bug but I struggle finding a solution, or an exact root cause.
Considering __builtin_FUNCTION value dependent causes AddTemplateOverloadCandidate (for Call) to fail.

I was not able to find where we look at value dependence of default values

@cor3ntin cor3ntin changed the title Potential 18.x regression from 8c2b0d4175dcfe669a43d0173fd00ed3 source_location 18.x regression from 8c2b0d4175dcfe669a43d0173fd00ed3 Jul 14, 2024
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 1, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 1, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 2, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 4, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they are first
used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template arguments
- They morally depend on the context in which they are used (rather than
called from).

It's fair to say that this is quite novels and confuses clang. In
particular, in some cases, we used to create dependent SourceLocExpr and
never subsequently transform them, leaving dependent objects in
instantiated functions types. To work around that we avoid replacing
SourceLocExpr when we think they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs, and
seem to only affect scenarios in which the value of the SourceLocExpr
does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
Fixes llvm#80210
Fixes llvm#85373

---------

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
tru pushed a commit to cor3ntin/llvm-project that referenced this issue Sep 10, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they are first
used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template arguments
- They morally depend on the context in which they are used (rather than
called from).

It's fair to say that this is quite novels and confuses clang. In
particular, in some cases, we used to create dependent SourceLocExpr and
never subsequently transform them, leaving dependent objects in
instantiated functions types. To work around that we avoid replacing
SourceLocExpr when we think they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs, and
seem to only affect scenarios in which the value of the SourceLocExpr
does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
Fixes llvm#80210
Fixes llvm#85373

---------

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" regression rejects-valid
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants