bassiounix wrote: > 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.
`Sema` or `SemaBase`? > 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'll split the implementation in 2 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... Since there is a common functionalities, I think I'll go with the first approach, that is, declaring them in a common class. This class is `Sema` for now as mentioned in the review. 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
