[clang] 283c8f7 - [clang] Check ValueDependent instead of InstantiationDependent before executing the align expr for builtin align functions.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-03T09:02:12+02:00
New Revision: 283c8f7f5ad0328ed99ec189ce3d6b3c7c877d36

URL: https://github.com/llvm/llvm-project/commit/283c8f7f5ad0328ed99ec189ce3d6b3c7c877d36
DIFF: https://github.com/llvm/llvm-project/commit/283c8f7f5ad0328ed99ec189ce3d6b3c7c877d36.diff

LOG: [clang] Check ValueDependent instead of InstantiationDependent before executing the align expr for builtin align functions.

in general, value dependent is a subset of instnatiation dependent. This
would allows us to produce diagnostics for the align expression (which
is instantiation dependent but not value dependent).

Differential Revision: https://reviews.llvm.org/D83074

Added: 
    

Modified: 
    clang/lib/Sema/SemaChecking.cpp
    clang/test/SemaCXX/builtin-align-cxx.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 672fe77a3359..0ce84ea382b5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -237,8 +237,8 @@ static bool SemaBuiltinAlignment(Sema &S, CallExpr *TheCall, unsigned ID) {
   }
   Expr::EvalResult AlignResult;
   unsigned MaxAlignmentBits = S.Context.getIntWidth(SrcTy) - 1;
-  // We can't check validity of alignment if it is type dependent.
-  if (!AlignOp->isInstantiationDependent() &&
+  // We can't check validity of alignment if it is value dependent.
+  if (!AlignOp->isValueDependent() &&
       AlignOp->EvaluateAsInt(AlignResult, S.Context,
                              Expr::SE_AllowSideEffects)) {
     llvm::APSInt AlignValue = AlignResult.Val.getInt();

diff  --git a/clang/test/SemaCXX/builtin-align-cxx.cpp b/clang/test/SemaCXX/builtin-align-cxx.cpp
index 190f352c26d4..848ed86525bb 100644
--- a/clang/test/SemaCXX/builtin-align-cxx.cpp
+++ b/clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -37,7 +37,7 @@ void test() {
   // expected-note at -1{{in instantiation of function template specialization 'test_templated_arguments<int, 7, 16>'}}
 }
 
-template <typename T>
+template <typename T, long ArraySize>
 void test_incorrect_alignment_without_instatiation(T value) {
   int array[32];
   static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
@@ -52,6 +52,10 @@ void test_incorrect_alignment_without_instatiation(T value) {
   __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
   __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
   __builtin_align_up(value);       // Same here
+
+  __builtin_align_up(array, sizeof(sizeof(value)) - 1); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, value); // no diagnostic as the alignment is value dependent.
+  (void)__builtin_align_up(array, ArraySize); // The same above here
 }
 
 // The original fix for the issue above broke some legitimate code.