[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