Skip to content

[clang] consteval constructor executes at runtime when invoked by escalated immediate function (or fails to link) #112677

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

Closed
rkjnsn opened this issue Oct 17, 2024 · 7 comments · Fixed by #112860
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval

Comments

@rkjnsn
Copy link

rkjnsn commented Oct 17, 2024

Consider the following code:

#include <iostream>
#include <optional>


class ConstEval {
 public:
  consteval ConstEval(const char* value) {
    const char* current_char = value;
    while (*current_char) {
      if (*current_char == 'a') {
        throw "Disallowed character!";
      }
      ++current_char;
    }
    value_ = value;
  }
  constexpr const char* value() { return value_; }
 private:
  const char* value_;
};

void TakesOptional(std::optional<ConstEval> maybe_value) {
  if (maybe_value.has_value()) {
    std::cout << "Value: " << maybe_value->value() << std::endl;
  } else {
    std::cout << "No value" << std::endl;
  }
}

int main(int argc, const char*argv[]) {
  // 1
  TakesOptional("testing 123");
  // 2
  std::optional<ConstEval> foo2 = "testing 123";
  // 3
  constexpr std::optional<ConstEval> foo3 = "testing 123";
  // 4
  TakesOptional(ConstEval{"testing 123"});
}

// 5
std::optional<ConstEval> foo5 = "testing 123";
// 6
constinit std::optional<ConstEval> foo6 = "testing 123";

Assuming I'm reading things correctly, I would expect the std::optional constructor to be an immediate-escalating function (because it is "a function that results from the instantiation of a templated entity defined with the constexpr specifier"), and a contained call to ConstEval's constructor to be an immediate-escalating expression, causing the std::optional constructor to become an immediate function.

Thus, I would expect lines 1, 2, 3, 5, and 6 all to immediately invoke the std::optional constructor to create a compile-time constant std::optional (indirectly invoking the ConstEval constructor), while 4 would immediately invoke the ConstEval constructor to create a constant ConstEval, which would then passed to the (not immediate, in this case) std::optional constructor.

Given that, I would further expect that changing "testing" to "tasting" on any of the 6 lines would result in a compiler failure (specifically, one informing me that throw "Disallowed character!" is not valid in a constant expression).

However, with clang built from revision 3dbd929, only lines 3, 4, and 6 fail to compile if "testing" is changed to "tasting". Lines 1, 2, and 5 unexpectedly continue to compile if "testing" in changed to "tasting", and instead cause an abort at runtime due to the uncaught exception. Thus, it appears that while clang properly considers the invocation of the explicitly-immediate ConstEval constructor to be a constant expression in 4, it erroneously does not consider the invocation of the escalated-to-immediate std::optional constructor to be a constant expression in 1, 2, and 5. Meanwhile, 3 and 6 invoke the constructor within an explicit constant expression, so those work as expected.

A further note: the observed behavior of ConstEval's constructor executing (and crashing if "testing" is changed to "tasting") at runtime for 1, 2, and 5 appears only to happen if it's small enough to inline into the generated std::optional specialization. If the constructor is more complicated and doesn't get inlined, lines 1, 2, and 5 will instead generate a linker error due to the symbol ConstEval::ConstEval(char const*) not being defined.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Oct 17, 2024
@EugeneZelenko EugeneZelenko added the consteval C++20 consteval label Oct 17, 2024
@efriedma-quic
Copy link
Collaborator

Needs reduced testcase that doesn't involve the full std::optional. (Looked briefly, but the issue seems to be deep inside the constructor for std::optional.)

@EugeneZelenko EugeneZelenko added the needs-reduction Large reproducer that should be reduced into a simpler form label Oct 17, 2024
@rkjnsn
Copy link
Author

rkjnsn commented Oct 17, 2024

Unfortunately, the issue doesn't reproduce with a simple optional analog, so it's something within the complexity of std::optional's implementation that is triggering the bug.

@efriedma-quic
Copy link
Collaborator

class ConstEval {
 public:
  consteval ConstEval(const char* value) {
    throw "Disallowed character!";
  }
};
struct FakeOptionalBase {
    ConstEval val;
    template <class Anything = int> constexpr
    FakeOptionalBase(const char (&arg)[12]) : val(arg) {}
};
struct FakeOptional : FakeOptionalBase {
    using FakeOptionalBase::FakeOptionalBase;
};
void TakesOptional(FakeOptional maybe_value) {
}
int main(int argc, const char*argv[]) {
  TakesOptional(FakeOptional("tasting 123"));
}

@efriedma-quic efriedma-quic removed the needs-reduction Large reproducer that should be reduced into a simpler form label Oct 18, 2024
@efriedma-quic
Copy link
Collaborator

The key here seems to be that the inherited constructor isn't marked as an immediate function in the AST.

@efriedma-quic
Copy link
Collaborator

(CC @cor3ntin)

cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Oct 18, 2024
@cor3ntin cor3ntin added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Oct 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/issue-subscribers-clang-frontend

Author: Erik Jensen (rkjnsn)

Consider the following code:
#include &lt;iostream&gt;
#include &lt;optional&gt;


class ConstEval {
 public:
  consteval ConstEval(const char* value) {
    const char* current_char = value;
    while (*current_char) {
      if (*current_char == 'a') {
        throw "Disallowed character!";
      }
      ++current_char;
    }
    value_ = value;
  }
  constexpr const char* value() { return value_; }
 private:
  const char* value_;
};

void TakesOptional(std::optional&lt;ConstEval&gt; maybe_value) {
  if (maybe_value.has_value()) {
    std::cout &lt;&lt; "Value: " &lt;&lt; maybe_value-&gt;value() &lt;&lt; std::endl;
  } else {
    std::cout &lt;&lt; "No value" &lt;&lt; std::endl;
  }
}

int main(int argc, const char*argv[]) {
  // 1
  TakesOptional("testing 123");
  // 2
  std::optional&lt;ConstEval&gt; foo2 = "testing 123";
  // 3
  constexpr std::optional&lt;ConstEval&gt; foo3 = "testing 123";
  // 4
  TakesOptional(ConstEval{"testing 123"});
}

// 5
std::optional&lt;ConstEval&gt; foo5 = "testing 123";
// 6
constinit std::optional&lt;ConstEval&gt; foo6 = "testing 123";

Assuming I'm reading things correctly, I would expect the std::optional constructor to be an immediate-escalating function (because it is "a function that results from the instantiation of a templated entity defined with the constexpr specifier"), and a contained call to ConstEval's constructor to be an immediate-escalating expression, causing the std::optional constructor to become an immediate function.

Thus, I would expect lines 1, 2, 3, 5, and 6 all to immediately invoke the std::optional constructor to create a compile-time constant std::optional (indirectly invoking the ConstEval constructor), while 4 would immediately invoke the ConstEval constructor to create a constant ConstEval, which would then passed to the (not immediate, in this case) std::optional constructor.

Given that, I would further expect that changing "testing" to "tasting" on any of the 6 lines would result in a compiler failure (specifically, one informing me that throw "Disallowed character!" is not valid in a constant expression).

However, with clang built from revision 3dbd929, only lines 3, 4, and 6 fail to compile if "testing" is changed to "tasting". Lines 1, 2, and 5 unexpectedly continue to compile if "testing" in changed to "tasting", and instead cause an abort at runtime due to the uncaught exception. Thus, it appears that while clang properly considers the invocation of the explicitly-immediate ConstEval constructor to be a constant expression in 4, it erroneously does not consider the invocation of the escalated-to-immediate std::optional constructor to be a constant expression in 1, 2, and 5. Meanwhile, 3 and 6 invoke the constructor within an explicit constant expression, so those work as expected.

A further note: the observed behavior of ConstEval's constructor executing (and crashing if "testing" is changed to "tasting") at runtime for 1, 2, and 5 appears only to happen if it's small enough to inline into the generated std::optional specialization. If the constructor is more complicated and doesn't get inlined, lines 1, 2, and 5 will instead generate a linker error due to the symbol ConstEval::ConstEval(char const*) not being defined.

@EugeneZelenko EugeneZelenko removed the clang Clang issues not falling into any other category label Oct 18, 2024
@rkjnsn
Copy link
Author

rkjnsn commented Nov 4, 2024

To bring some relevant PR discussion to the bug,

In @cor3ntin's PR #112860, @efriedma-quic provides the following test case:

struct ConstEval {
  consteval ConstEval(int) {}
};
struct SimpleCtor { constexpr SimpleCtor(int) {}};
struct TemplateCtor {
  template <class Anything = int> constexpr
  TemplateCtor (int arg) {}
};
struct ConstEvalMember1 : SimpleCtor {
    int y = 10;
    ConstEval x = y;
    using SimpleCtor::SimpleCtor;
};
struct ConstEvalMember2 : TemplateCtor {
    int y = 10;
    ConstEval x = y;
    using TemplateCtor::TemplateCtor;
};
void f() {
  ConstEvalMember1 i1(0);
  ConstEvalMember2 i2(0);
}

If the inherited derived class constructor copies the immediate-escalating property of the base class, this leads to the counter-intuitive behavior that i1 produces an error while i2 doesn't.

Intuitively, given that a compiler-generated default constructor is immediately-escalating, and an inheriting constructor in a derived class effectively acts as a compiler-generated default constructor except that it initializes the relevant base class with the inherited constructor, I would expect the generated derived-class constructor always to be immediate-escalating, and for any immediate field or base class initialization (including, but not limited to, the base class from which the constructor is inherited) to cause the compiler-generated constructor to escalate to being immediate. Thus, I would expect both i1 and i2 to compile.

That said, I was not able to find any language in the standard confirming or denying that that's how immediate escalation with inherited constructors should work. However, I'm far from a standards expert, so I very well might be missing something.

cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Dec 9, 2024
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Jan 22, 2025
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants