https://github.com/Sirraide requested changes to this pull request.
I think there may be some code duplication here, so moving *some* of this code around *might* make sense, but I see a few issues with how this is done at the moment: 1. If we were to introduce new helpers, then instead of making a new header for this, these functions should just become member functions of the `Sema` class. 2. The new version of `CheckAllArgTypesAreCorrect()` is... way too complicated, in my opinion; I have trouble trying to figure out what two parameters that are a variant and a pair are supposed to mean from looking at the function, and it does feel like this function wants to be at least two or three separate functions... I think a better approach would be to leave the HLSL functions alone and just factor out a separate helper and put it in `SemaSPIRV.cpp`, because as-is, this pr isn’t exactly simplifying things... https://github.com/llvm/llvm-project/pull/125045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
