`[[gnu::weak]]` disables constant evaluation of constexpr variables in C++14

[cor3ntin]

GCC and Clang disagree on their handling of [[gnu::weak]] before C++17 https://gcc.godbolt.org/z/s7q67vqjz

struct S { static constexpr int var = 0; };
[[gnu::weak]] constexpr int S::var;
static constexpr int usage = S::var;

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it, for consistency with GCC

In static bool evaluateVarDeclInit (ExprConstant.cpp), and in static bool CheckWeak (Interp.cpp), we should allow constant evaluation -without diagnostics, but not constant folding) - by looking at Info.EvalMode

[llvmbot]

@llvm/issue-subscribers-c-14

Author: cor3ntin (cor3ntin)

GCC and Clang disagree on their handling of `[[gnu::weak]]` before C++17 https://gcc.godbolt.org/z/s7q67vqjz

struct S { static constexpr int var = 0; };
[[gnu::weak]] constexpr int S::var;
static constexpr int usage = S::var;

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it, for consistency with GCC

In static bool evaluateVarDeclInit (ExprConstant.cpp), and in static bool CheckWeak (Interp.cpp), we should allow constant evaluation -without diagnostics, but not constant folding) - by looking at Info.EvalMode

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: cor3ntin (cor3ntin)

GCC and Clang disagree on their handling of `[[gnu::weak]]` before C++17 https://gcc.godbolt.org/z/s7q67vqjz

struct S { static constexpr int var = 0; };
[[gnu::weak]] constexpr int S::var;
static constexpr int usage = S::var;

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it, for consistency with GCC

In static bool evaluateVarDeclInit (ExprConstant.cpp), and in static bool CheckWeak (Interp.cpp), we should allow constant evaluation -without diagnostics, but not constant folding) - by looking at Info.EvalMode

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor is working on this issue. If someone is assigned to the issue or claimed to be working on it, ping the person. After one week without a response, the assignee may be changed.
2. Leave a comment indicating that you are working on the issue, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-c-11

Author: cor3ntin (cor3ntin)

GCC and Clang disagree on their handling of `[[gnu::weak]]` before C++17 https://gcc.godbolt.org/z/s7q67vqjz

struct S { static constexpr int var = 0; };
[[gnu::weak]] constexpr int S::var;
static constexpr int usage = S::var;

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it, for consistency with GCC

In static bool evaluateVarDeclInit (ExprConstant.cpp), and in static bool CheckWeak (Interp.cpp), we should allow constant evaluation -without diagnostics, but not constant folding) - by looking at Info.EvalMode

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: cor3ntin (cor3ntin)

GCC and Clang disagree on their handling of `[[gnu::weak]]` before C++17 https://gcc.godbolt.org/z/s7q67vqjz

struct S { static constexpr int var = 0; };
[[gnu::weak]] constexpr int S::var;
static constexpr int usage = S::var;

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it, for consistency with GCC

In static bool evaluateVarDeclInit (ExprConstant.cpp), and in static bool CheckWeak (Interp.cpp), we should allow constant evaluation -without diagnostics, but not constant folding) - by looking at Info.EvalMode

[efriedma-quic]

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it

What does this mean? We allow it in constant-evaluated contexts, but not non-constant-evaluated contexts? That seems wrong. If C++ ODR requires that the real definition is the same as the weak definition, then we should allow constant-folding. If, somehow, C++ ODR doesn't require that, then any sort of constant evaluation is broken.

[Mr-Anyone]

A patch like this seems to do the trick. I'm unsure if this is even correct, as the comment above seems to suggest otherwise:

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b20e2690d0ee..ac0bd38a64d3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3646,7 +3646,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 
   // Never use the initializer of a weak variable, not even for constant
   // folding. We can't be sure that this is the definition that will be used.
-  if (VD->isWeak()) {
+  if (VD->isWeak() && (Info.EvalMode == EvalInfo::EM_ConstantFold) ) {
     Info.FFDiag(E, diag::note_constexpr_var_init_weak) << VD;
     NoteLValueLocation(Info, Base);
     return false;

[erichkeane]

While it would be unsafe to constant-fold this code, it should be fine to constant-evaluate it

What does this mean? We allow it in constant-evaluated contexts, but not non-constant-evaluated contexts? That seems wrong. If C++ ODR requires that the real definition is the same as the weak definition, then we should allow constant-folding. If, somehow, C++ ODR doesn't require that, then any sort of constant evaluation is broken.

That is the suggestion, and I don't disagree with you perhaps? But not entirely. First, we should very much allow this in constant evaluated contexts, as demanded by the language. Using the initialized value there seems to be consistent with the language requirements here.

I don't think ODR really comes into play here, attribute 'weak' is a GCC modification of the ODR rules, and it isn't exactly clear HOW. To me, Constant EVALUATION is a situation where the user definitely expects that the evaluation will happen with the initializer value.

Constant FOLDING is a little different in this case, it is effectively a use of the 'as-if' rule to evaluate it 'early'. Whoever wrote that diagnostic seems to believe that means that we cannot count on 'weak' symbols to have that value during that evaluation.

I see 2 solutions here:

1- completely remove the diagnostic. We end up folding values under 'as-if' that might have been a different value at runtime, and ce-la-vie.
2- Remove the diagnostic during constexpr evaluation as the patch by @Mr-Anyone does. This is consistent with how 'constexpr' is supposed to work, plus maintains our existing behavior for 'weak'. It is lower risk, but also seems odd.

[efriedma-quic]

It's sort of counterintuitive, but the proposed patch changes the behavior of the following:

[[gnu::weak]] extern const int var = 0;
int usage = var;

Just allowing all constant-folding doesn't seem much worse...

---

My primary concern here is someone actually using weak to allow overriding the variable. Which is probably more common than people using it as a hack to get inline variables without C++17.

Maybe worth asking gcc developers to see what they think.

[philnik777]

FWIW IMO the only correct thing would be to reject [[gnu::weak]] constexpr variables. I don't see how they ever make any sense.