hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.


================
Comment at: clang/include/clang/Sema/Weak.h:62
+        return false;
+      return LHS.getAlias()->getName() == RHS.getAlias()->getName();
+    }
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121927

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to