venk-ks wrote: Thanks for updating the PR to use `Builtins.td` and `switch (BuiltinID)`. This looks much cleaner.
Two small suggestions based on @nickdesaulniers's recent review feedback on #223520 and #226261: 1. In `SemaChecking.cpp`, when `Prototype = ""` is unset in `Builtins.td`, @nickdesaulniers requested also checking the types of the remaining arguments at the call site (see https://github.com/llvm/llvm-project/pull/226261#discussion_r4107750442): - `arg 0` (`fd`) is `isIntegerType()` for `read`, `write`, `pread`, `pread64`, `pwrite`, `pwrite64`, and `readlinkat` - `arg 3` (`offset`) is `isIntegerType()` for `pread`, `pread64`, `pwrite`, and `pwrite64` - `path` (`arg 0` for `readlink`, `arg 1` for `readlinkat`) is `isPointerType()` 2. In `Builtins.td`, could you add a comment above `let Prototype = "";` on each definition showing the C signature and which types are target-specific (e.g., `// ssize_t(int, void*, size_t); ssize_t is target-specific`)? Also, `PRead`/`PRead64` and `PWrite`/`PWrite64` can optionally be combined using `let Spellings = ["pread", "pread64"];` and `let Spellings = ["pwrite", "pwrite64"];`. https://github.com/llvm/llvm-project/pull/224979 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
