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

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

Reply via email to