[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno requested changes to this revision.
bruno added a comment.
This revision now requires changes to proceed.

Nice, thanks for adding the builtin layer.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:7808
+/// Handle __builtin_assume_separate_storage. For now this is a no-op, but
+/// eventually we expect an optional multi-arg variadic version (to handle an
+/// excluded range).
----------------
Can you prefix this with a `FIXME`/`TODO`?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7810
+/// excluded range).
+bool Sema::SemaBuiltinAssumeSeparateStorage(CallExpr *TheCall) { return false; }
+
----------------
IIUC, seems like there a two pieces here you can already cover as part of this patch:

- Check for exact two arguments. If in the future an optional multi-arg variadic version is to be supported the checking could be enhanced/changed to support that.
- Check both are pointers. I noticed that in the testcase you rely on `incompatible integer to pointer conversion` and friends to catch some of these, but a more user friendly diagnostic could be explicit about the expectations. In case these current error diagnostics you are relying run before this sema check, perhaps a note diagnostic could help clarify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136515