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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
