[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

Wed Mar 23 09:47:36 PDT 2022

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you for the fix!

================
Comment at: clang/include/clang/Sema/Weak.h:62
+        return false;
+      return LHS.getAlias()->getName() == RHS.getAlias()->getName();
+    }
----------------
hubert.reinterpretcast wrote:
> aaron.ballman wrote:
> > Previously we cared about the source locations being the same. But... is there a situation where the aliases are the same but the source locations are different? I can't think of any, so perhaps that's worth an assert that the locations match?
> We didn't really case about the source locations being the same. The removed operators were just dead code (featuring such things as compare-by-pointer-value). I have verified that splitting the deletion of them to a separate patch does not cause any build or LIT `check-all` failures. I can commit that part as an NFC patch first if you prefer.
> 
> Also, the aliases //can// be the same with different source locations. The location comes from where the `#pragma weak` is (from the parser by way of `ActOnPragmaWeakAlias`). That duplicates are ignored (meaning we only diagnose one instance) is mentioned in the patch description.
> 
> It is already the status quo that having the same alias specified with different targets (at least those not yet declared) neither produces a diagnostic nor operates by "last one wins". Instead, Clang prefers the first declared target and GCC just emits both and leaves it to the assembler to figure out (https://godbolt.org/z/EasK375j3).
Ah, thank you for the explanation, that makes sense to me.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121927