[clang] [Clang][FIX] Fix type qualifiers on vector builtins (PR #160185)

[John McCall via cfe-commits]

================
@@ -2268,7 +2268,8 @@ static bool BuiltinCountZeroBitsGeneric(Sema &S, CallExpr *TheCall) {
 }
 
 static bool CheckMaskedBuiltinArgs(Sema &S, Expr *MaskArg, Expr *PtrArg,
-                                   unsigned Pos) {
+                                   unsigned Pos, bool AllowConst,
+                                   bool AllowAS) {
----------------
rjmccall wrote:

I'm curious what cases need to disallow address spaces. If there's a good reason for it, okay, but I'd like to make sure we're not accidentally restricting things that LLVM is perfectly capable of supporting.

My comment about l-value conversion was actually saying that you're missing something here; sorry for not being clearer about that. Most expressions in C undergo a specific set of conversions — l-value conversion, array-to-pointer conversion, and function-to-pointer conversion. Clang normally always does these conversions for call arguments, but builtins with custom typechecking specifically disable this, so the checking has to request them to be done explicitly by calling `DefaultFunctionArrayLvalueConversion`. You'll see a bunch of other builtins in SemaChecking.cpp that have to do this.

Performing these conversions correctly has a number of consequences, most of which are pretty subtle. The easiest one to test for is an Objective-C property reference expression: basically, whether `obj.prop` calls a getter or a setter depends on whether you use in on the LHS of an assignment operator, and `DefaultFunctionArrayLvalueConversion` decides that it *hasn't* been used that way and so resolves it to a getter. Testing whether you handle ObjC property references correctly therefore functions as a test that you're calling `DefaultFunctionArrayLvalueConversion` when you're supposed to. Check out `test/SemaObjC/matrix-type-builtins.m` for the test pattern, and you can look at e.g. `BuiltinMatrixColumnMajorLoad` for what the Sema code should look like.

Another thing to test for is that clients should be able to just use the name of an array variable as the pointer operand of your builtin. You don't check for this specifically in your builtin type-checking; you just call `DefaultFunctionArrayLvalueConversion` like you're supposed to, and it adds an array-to-pointer conversion, and now your operand has pointer type.

Oh, and looking at `BuiltinMatrixColumnMajorLoad` reminds me that you're not handling templates correctly. If any of the argument expressions are type-dependent, you need to just mark the builtin call as having a dependent type and bail out.

I know this all seems like a lot of corner-case complexity, but it's the basic expectation of how language feature development works in Clang.

https://github.com/llvm/llvm-project/pull/160185