Skip to content

Conversation

@farzonl
Copy link
Member

@farzonl farzonl commented Jan 13, 2025

In the below code B varies over the arg list via a loop. However, the diagnostics do not vary with the loop.
Fix so that diagnostics can vary with B.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Jan 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Farzon Lotfi (farzonl)

Changes

In the below code B varies over the arg list via a loop. However, the diagnostics do not vary with the loop.
Fix so that diagnostics can vary with B.


Full diff: https://github.com/llvm/llvm-project/pull/122772.diff

1 Files Affected:

  • (modified) clang/lib/Sema/SemaHLSL.cpp (+4-5)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 65ddee05a21512..169a6a6903891c 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1688,8 +1688,9 @@ static bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
   auto *VecTyA = ArgTyA->getAs<VectorType>();
   SourceLocation BuiltinLoc = TheCall->getBeginLoc();
 
+  ExprResult B;
   for (unsigned i = 1; i < TheCall->getNumArgs(); ++i) {
-    ExprResult B = TheCall->getArg(i);
+    B = TheCall->getArg(i);
     QualType ArgTyB = B.get()->getType();
     auto *VecTyB = ArgTyB->getAs<VectorType>();
     if (VecTyA == nullptr && VecTyB == nullptr)
@@ -1712,8 +1713,7 @@ static bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
         // HLSLVectorTruncation.
         S->Diag(BuiltinLoc, diag::err_vec_builtin_incompatible_vector)
             << TheCall->getDirectCallee() << /*useAllTerminology*/ true
-            << SourceRange(TheCall->getArg(0)->getBeginLoc(),
-                           TheCall->getArg(1)->getEndLoc());
+            << SourceRange(A.get()->getBeginLoc(), B.get()->getEndLoc());
         retValue = true;
       }
       return retValue;
@@ -1724,8 +1724,7 @@ static bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
   // requires a VectorSplat on Arg0 or Arg1
   S->Diag(BuiltinLoc, diag::err_vec_builtin_non_vector)
       << TheCall->getDirectCallee() << /*useAllTerminology*/ true
-      << SourceRange(TheCall->getArg(0)->getBeginLoc(),
-                     TheCall->getArg(1)->getEndLoc());
+      << SourceRange(A.get()->getBeginLoc(), B.get()->getEndLoc());
   return true;
 }
 

@llvm-beanz
Copy link
Collaborator

This is clearly a bug, and the fix looks right, but the absence of a test case seems wrong because this isn't an NFC change... it actually fixes a bug.

@farzonl
Copy link
Member Author

farzonl commented Jan 13, 2025

This is clearly a bug, and the fix looks right, but the absence of a test case seems wrong because this isn't an NFC change... it actually fixes a bug.

We have two builtins that have greater than 2 args clamp and lerp. so should be able to write a test case for this.

@farzonl farzonl changed the title [NFC] Fixed Diagnostics that assumed only two arguments [HLSL][Sema] Fixed Diagnostics that assumed only two arguments Jan 15, 2025
@farzonl farzonl force-pushed the nfc_hlsl_sema_typo branch from 9df164c to 5ca3dc1 Compare January 15, 2025 23:54
@farzonl farzonl merged commit 5a735a2 into llvm:main Jan 17, 2025
9 checks passed
@farzonl farzonl deleted the nfc_hlsl_sema_typo branch January 17, 2025 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants