[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 11 23:22:10 PDT 2020
rsmith added a comment.
In D89212#2324127 <https://reviews.llvm.org/D89212#2324127>, @rjmccall wrote:
> This seems like a good idea in the abstract; we'll need to figure out what the practical consequences are.
Looks like it triggers warnings in Abseil at least; there, the code looks like this:
// spinlock.h
void AbslInternalSpinLockDelay();
inline void lock(...) {
AbslInternalSpinLockDelay();
}
// spinlock.cc
__attribute__((weak)) void AbslInternalSpinLockDelay() { ... }
... and adding the `weak` attribute to the declaration would change the meaning of the program (we want an `external` reference, not an `extern_weak` reference).
Perhaps we should only warn if the function is not defined in the same translation unit? If it is defined, then I think its address can never be null, and CodeGen will emit it with the correct linkage. We still have the problem that `==` comparisons against pointers to the function can incorrectly evaluate to `false`, but I think that's really a problem with aliases rather than with weakness. (C++ requires that `void f(), g(); bool b = f == g;` initializes `b` to `false` even though the only correct answer is that we don't know; we refuse to evaluate the comparison if either `f` or `g` is weak, but I think that's only really addressing the problem that they could both be null, not that they could have the same non-null address, since the latter problem isn't specific to weak declarations. I think the constant evaluator is being overly-conservative, and could evaluate `f == g` to `false` if `f` and `g` are both weak but at least one of them is defined locally, under the language-guaranteed assumption that distinct functions have different addresses. And perhaps we should have a way of declaring a function as potentially aliasing another function without actually defining the function as an alias, as a way to turn off that assumption independent of weakness?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89212/new/
https://reviews.llvm.org/D89212
More information about the cfe-commits
mailing list