[PATCH] D101598: [clang][Sema] adds `[[clang::no_address]]` attribute

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:5972
+  Although opt-in, the attribute poisons the entire overload set, meaning that only one overload
+  needs the attribute per set (although such usage is discouraged). Until there is confidence that
+  this attribute doesn't have many sharp edges, usage is restricted to only entities in namespace
----------------
Could you make this an error?


================
Comment at: clang/lib/Sema/SemaOverload.cpp:12286
+    UnresolvedLookupExpr *OverloadSet = GetOverloadSet();
+    assert(OverloadSet != nullptr);
+    return llvm::any_of(OverloadSet->decls(), HasNoAddressAttr);
----------------
Nit: you can just `assert(OverloadSet && "...")`.

However, it looks like from the implementation of `GetOverloadSet` this might very well be null. Asserts crash compilers. If you're very confident this isn't going to be null, leave the assert but change the `dyn_cast`s in `GetOverloadSet` to `cast`s. Otherwise, provide some way to bail here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101598