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

[consteval] clang++ sometimes reports false-positive errors for static consteval member functions #56246

Closed
ldalessa opened this issue Jun 27, 2022 · 13 comments
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval

Comments

@ldalessa
Copy link

When I have a CRTP that uses it's client's function, if that client's function happens to be static consteval, clang++ reports an error.

template <class T>
struct base
{
    constexpr auto foobar() const -> int {
        return static_cast<T const*>(this)->bar(); // error: call to consteval function 'foo::bar' is not a constant expression
    }
};

struct foo : base<foo> {
    static consteval auto bar() -> int {
        return 0;
    }
};

foo x;
int i = x.foobar();

live: https://godbolt.org/z/KG5oT6GTj

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" c++23 and removed new issue labels Jun 27, 2022
@llvmbot
Copy link
Member

llvmbot commented Jun 27, 2022

@llvm/issue-subscribers-clang-frontend

@llvmbot
Copy link
Member

llvmbot commented Jun 27, 2022

@llvm/issue-subscribers-c-2b

@usx95 usx95 added c++20 and removed c++23 labels Aug 19, 2022
@llvmbot
Copy link
Member

llvmbot commented Aug 19, 2022

@llvm/issue-subscribers-c-20

@usx95 usx95 closed this as completed Aug 19, 2022
@usx95 usx95 reopened this Aug 19, 2022
@usx95
Copy link
Contributor

usx95 commented Aug 19, 2022

This does not work even for simpler cases of this and consteval interactions.

struct S {
    static consteval int hello() { return 1;}

    int foo() const { 
        return this->hello(); 
        // ^ error: call to consteval function 'S::hello' is not a constant expression
        // ^ note: use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
    }
    constexpr int bar() const {
        return this->hello(); // same error and note as above.
    }
    consteval int baz() const {
        // works fine.
        return this->hello();
    }
} s;
static_assert(s.baz() == 1);

Clang errors and diverges from all the rest of the compilers. https://godbolt.org/z/93WTdbK1v

@usx95
Copy link
Contributor

usx95 commented Aug 19, 2022

9fcca8b#diff-f10defc3f20fb095bf22b3a79bead200d494bde9d503e283067a57aff483936c

I think this patch fixed a crash but clang still produces an error.

struct A {
  consteval int a() const { return 1; }
  void c() {
    a() + d(); // expected-error {{call to consteval function 'PR48235::A::a' is not a constant expression}} \
               // expected-note {{use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function}}
  }

My reading of https://eel.is/c++draft/dcl.constexpr is that consteval members are valid and using them in any context (constant or unevaluated) show also be legal.

Would be interested in knowing thoughts of @AaronBallman

@AaronBallman
Copy link
Collaborator

9fcca8b#diff-f10defc3f20fb095bf22b3a79bead200d494bde9d503e283067a57aff483936c

I think this patch fixed a crash but clang still produces an error.

struct A {
  consteval int a() const { return 1; }
  void c() {
    a() + d(); // expected-error {{call to consteval function 'PR48235::A::a' is not a constant expression}} \
               // expected-note {{use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function}}
  }

My reading of https://eel.is/c++draft/dcl.constexpr is that consteval members are valid and using them in any context (constant or unevaluated) show also be legal.

Would be interested in knowing thoughts of @AaronBallman

I believe http://eel.is/c++draft/expr.prim.id.general#4 is the cause of that diagnostic. https://godbolt.org/z/PP51G3WWW sure suggests everyone implements this the same way, too.

@usx95
Copy link
Contributor

usx95 commented Aug 19, 2022

Interesting. The above example looks right now.

Coming back to

struct S {
    static consteval int hello() { return 1;}

    int foo() const { 
        return this->hello(); 
        // ^ error: call to consteval function 'S::hello' is not a constant expression
        // ^ note: use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
    }
    constexpr int bar() const {
        return this->hello(); // same error and note as above.
    }
    consteval int baz() const {
        // works fine.
        return this->hello();
    }
} s;
static_assert(s.baz() == 1);

I guess clang is almost right but it should consider this->hello() in both foo() and bar() to be a constant expression according to http://eel.is/c++draft/dcl.constexpr#5.1 and http://eel.is/c++draft/expr.const#5.1

@AaronBallman
Copy link
Collaborator

I think you're correct because of http://eel.is/c++draft/expr.const#5.1.2 specifically. While the this isn't necessary for the member access expression (because the expression denotes a static function), that this is still evaluated per https://eel.is/c++draft/expr.ref#1.sentence-2 (see the associated footnote as well), so it is being used as the postfix-expression and so it should be fine as a core constant expression.

So I think the diagnostic in foo() is correct, but the one in bar() is incorrect, and accepting the code in baz() is correct. Do you come to the same conclusion?

@usx95 usx95 moved this from Todo to In Progress in Core C++20 in Clang Aug 19, 2022
@usx95 usx95 added the consteval C++20 consteval label Aug 20, 2022
@usx95
Copy link
Contributor

usx95 commented Aug 20, 2022

I come to the conclusion that
this->static_consteval_member() is a core constant expression and thus the invocation inside foo() is also valid and clang diagnostic is incorrect.
(http://eel.is/c++draft/dcl.constexpr#5.1 : an invocation of a constexpr function can appear in a constant expression [expr.const])

I agree with the rest of the conclusion (bar() and baz()).

@usx95
Copy link
Contributor

usx95 commented Aug 20, 2022

Although I suspect I might be wrong again due to http://eel.is/c++draft/expr.prim.id.general#4. I am mostly confused about the definition of id-expression. Basically:

consteval int func() {return 0;}
struct S {
    static consteval int hello() { return 1;}
    int foo() const { 
        return this->hello(); // Why this should be invalid
        // return func() ; // but this is valid. Both are core constant expressions!
    }
}

Is func() not an id-expression and http://eel.is/c++draft/expr.prim.id.general#4 does not apply to it ?

@AaronBallman
Copy link
Collaborator

func is an id-expression (it's the primary-expression component of the postfix-expression and the () denotes a function call expression http://eel.is/c++draft/expr.compound#nt:postfix-expression). But id-expression is also used in a postfix-expression denoting a member access in the same paragraph:

postfix-expression . template_opt id-expression
postfix-expression -> template_opt id-expression

http://eel.is/c++draft/expr.post#expr.ref-6 determines what is denoted by the member access expression, and that id-expression will denote an immediate function whether called through this->func() or func().

@cor3ntin
Copy link
Contributor

cor3ntin commented Jun 6, 2023

Should be fixed by #63139

@shafik
Copy link
Collaborator

shafik commented Jun 13, 2024

This currently working in trunk: https://godbolt.org/z/4P8GdPhWM

@shafik shafik closed this as completed Jun 13, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Core C++20 in Clang Jun 13, 2024
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" consteval C++20 consteval
Projects
Status: Done
Development

No branches or pull requests

8 participants
@cor3ntin @AaronBallman @shafik @usx95 @ldalessa @EugeneZelenko @llvmbot and others