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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add a release note for the fix?



================
Comment at: clang/include/clang/Sema/Sema.h:1075-1077
   /// WeakUndeclaredIdentifiers - Identifiers contained in
   /// \#pragma weak before declared. rare. may alias another
   /// identifier, declared or undeclared
----------------
Should probably update this comment as the type just got much weirder than before.


================
Comment at: clang/include/clang/Sema/Weak.h:18-19
 #include "clang/Basic/SourceLocation.h"
 
+#include "llvm/ADT/DenseMapInfo.h"
+
----------------



================
Comment at: clang/include/clang/Sema/Weak.h:27-30
   IdentifierInfo *alias;  // alias (optional)
   SourceLocation loc;     // for diagnostics
-  bool used;              // identifier later declared?
 public:
+  WeakInfo() : alias(nullptr), loc(SourceLocation()) {}
----------------



================
Comment at: clang/include/clang/Sema/Weak.h:33
+      : alias(Alias), loc(Loc) {}
+  inline IdentifierInfo *getAlias() const { return alias; }
   inline SourceLocation getLocation() const { return loc; }
----------------
Would it be onerous to make the return type be `const IdentifierInfo *` given that the function is `const`? (If it is, just ignore the suggestion -- I love adding const correctness where we can get it basically for free.)


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121927