[PATCH] D126324: [clang] Allow const variables with weak attribute to be overridden

Wed May 25 07:51:37 PDT 2022

jyknight added a comment.

In D126324#3536785 <https://reviews.llvm.org/D126324#3536785>, @aaron.ballman wrote:

> The changes so far look sensible, but I think we should add some more tests for a few situations. 1) Using a const weak symbol as a valid initializer should be diagnosed (with a warning? with an error?) so users are alerted to the behavioral quirks.

I don't feel this is really necessary.

> 2. Using a const weak symbol in a constant expression context should probably be an error, right? e.g.,

That's already the case (except, using a VLA is not an error in either language mode). Clang previously _only_ treated weak symbols as compile-time-known in the backend; the frontend has treated it as an non-constant-evaluable variable for ages, and that behavior is unchanged by this patch.

> Also, this definitely could use a release note so users know about the behavioral change.
>
> Do you have any ideas on how much code this change is expected to break? (Is it sufficient enough that we want to have an explicit deprecation period of the existing behavior before we switch to the new behavior?)

Because it's only changing the optimizer behavior not the frontend constant evaluator, I think it's pretty safe (which is also why I'm OK with it).

================
Comment at: clang/include/clang/Basic/AttrDocs.td:6527
+If an external declaration is marked weak and that symbol does not
+exist during linking (possibly dynamic) the linker will replace that
+with NULL.
----------------
Better said:
"""
If an external declaration is marked weak and that symbol does not
exist during linking (possibly dynamic) the address of the symbol
will evaluate to NULL.
"""

(function decls are just weird in that `&f` is treated as equivalent to `f`)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126324/new/

https://reviews.llvm.org/D126324