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

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: arichardson.
Herald added a project: clang.

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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83074

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


Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===================================================================
--- clang/test/SemaCXX/builtin-align-cxx.cpp
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -52,6 +52,9 @@
   __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.
 }
 
 // The original fix for the issue above broke some legitimate code.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -237,8 +237,8 @@
   }
   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();



Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===================================================================
--- clang/test/SemaCXX/builtin-align-cxx.cpp
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -52,6 +52,9 @@
   __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.
 }
 
 // The original fix for the issue above broke some legitimate code.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -237,8 +237,8 @@
   }
   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();

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits