Emit sizeof/alignof values as notes when a static_assert fails

[arichardson]

Summary

Trying to debug a static_assert(sizeof(foo) == ...) failure can be rather
awkward since there is no easy way to print the value of the sizeof() at
compile-time without another compiler diagnostic involving an array size.
This patch emits additional notes when a static_assert fails for any
UnaryExprOrTypeTraitExpr expression inside the static_assert.

[aaron.ballman]

clang/lib/Sema/SemaTemplate.cpp

3598

clang/test/SemaCXX/static-assert.cpp

218

Can you also add a test where the unary expression isn't the reason for the failure? e.g.,

static_assert(is_same_v<float, decltype(sizeof(int))>);

Perhaps we'd want to silence the notes if the expression context for the call to sizeof is itself unevaluated?

[arichardson]

rebase and address feedback

[arichardson]

This is less useful now that 09117b21890c652994f7ada0229d309b35b44259 / D130894 has landed, but it might still be worth including (e.g. for expressions that include multiple sizeof() values?

[tbaeder]

From looking at the output in the test cases, the additional diagnostics seem unnecessary to me in almost all cases...?

[aaron.ballman]

In D108211#3748595, @tbaeder wrote:

From looking at the output in the test cases, the additional diagnostics seem unnecessary to me in almost all cases...?

+1; my observation is that the extra note repeats information because the new note says what the LHS and the RHS of the expression evaluates to. I could see this new note being more useful in a case like: static_assert(sizeof(foo) + sizeof(bar) == sizeof(baz) / 12); where there are multiple sizeof expressions involved and we could note what each constituent part evaluates to, but I'm not certain it's worth the extra complexity to go to those lengths (I'd want to see real world code where this would really help).

[arichardson]

Would require significant work to still be useful now that we print the value of expressions (e.g. only print for more complex expressions).

clang/include/clang/Basic/DiagnosticSemaKinds.td

 // C++ declarations
 def err_static_assert_expression_is_not_constant : Error<
   "static_assert expression is not an integral constant expression">;
 def err_constexpr_if_condition_expression_is_not_constant : Error<
   "constexpr if condition is not a constant expression">;
 def err_static_assert_failed : Error<"static_assert failed%select{ %1|}0">;
 def err_static_assert_requirement_failed : Error<
   "static_assert failed due to requirement '%0'%select{ %2|}1">;
+def note_static_assert_requirement_context : Note<"with '%0' equal to %1">;
 
 def ext_inline_variable : ExtWarn<
   "inline variables are a C++17 extension">, InGroup<CXX17>;
 def warn_cxx14_compat_inline_variable : Warning<
   "inline variables are incompatible with C++ standards before C++17">,
   DefaultIgnore, InGroup<CXXPre17Compat>;
 
 def warn_inline_namespace_reopened_noninline : Warning<

clang/include/clang/Sema/Sema.h

   /// Check the enable_if expressions on the given function. Returns the first
   /// failing attribute, or NULL if they were all successful.
   EnableIfAttr *CheckEnableIf(FunctionDecl *Function, SourceLocation CallLoc,
                               ArrayRef<Expr *> Args,
                               bool MissingImplicitThis = false);
 
   /// Find the failed Boolean condition within a given Boolean
   /// constant expression, and describe it with a string.
-  std::pair<Expr *, std::string> findFailedBooleanCondition(Expr *Cond);
+  std::pair<Expr *, std::string>
+  findFailedBooleanCondition(Expr *Cond,
+                             SmallVectorImpl<PartialDiagnosticAt> &Notes);
 
   /// Emit diagnostics for the diagnose_if attributes on Function, ignoring any
   /// non-ArgDependent DiagnoseIfAttrs.
   ///
   /// Argument-dependent diagnose_if attributes should be checked each time a
   /// function is used as a direct callee of a function call.
   ///
   /// Returns true if any errors were emitted.

clang/lib/Sema/SemaDeclCXX.cpp

if (!AssertExpr->isTypeDependent() && !AssertExpr->isValueDependent() &&
     if (!Failed && !Cond) {
       SmallString<256> MsgBuffer;
       llvm::raw_svector_ostream Msg(MsgBuffer);
       if (AssertMessage)
         AssertMessage->printPretty(Msg, nullptr, getPrintingPolicy());
 
       Expr *InnerCond = nullptr;
       std::string InnerCondDescription;
+      SmallVector<PartialDiagnosticAt, 2> Notes;
       std::tie(InnerCond, InnerCondDescription) =
-        findFailedBooleanCondition(Converted.get());
+          findFailedBooleanCondition(Converted.get(), Notes);
       if (InnerCond && isa<ConceptSpecializationExpr>(InnerCond)) {
         // Drill down into concept specialization expressions to see why they
         // weren't satisfied.
         Diag(StaticAssertLoc, diag::err_static_assert_failed)
           << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
         ConstraintSatisfaction Satisfaction;
         if (!CheckConstraintSatisfaction(InnerCond, Satisfaction))
           DiagnoseUnsatisfiedConstraint(Satisfaction);
       } else if (InnerCond && !isa<CXXBoolLiteralExpr>(InnerCond)
                            && !isa<IntegerLiteral>(InnerCond)) {
         Diag(StaticAssertLoc, diag::err_static_assert_requirement_failed)
           << InnerCondDescription << !AssertMessage
           << Msg.str() << InnerCond->getSourceRange();
       } else {
         Diag(StaticAssertLoc, diag::err_static_assert_failed)
           << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
       }
+      for (const PartialDiagnosticAt &Note : Notes)
+        Diag(Note.first, Note.second);
       Failed = true;
     }
   } else {
     ExprResult FullAssertExpr = ActOnFinishFullExpr(AssertExpr, StaticAssertLoc,
                                                     /*DiscardedValue*/false,
                                                     /*IsConstexpr*/true);
     if (FullAssertExpr.isInvalid())
       Failed = true;

clang/lib/Sema/SemaTemplate.cpp

 
 namespace {
 
 // A PrinterHelper that prints more helpful diagnostics for some sub-expressions
 // within failing boolean expression, such as substituting template parameters
 // for actual types.
 class FailedBooleanConditionPrinterHelper : public PrinterHelper {
 public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy &P)
-      : Policy(P) {}
+  explicit FailedBooleanConditionPrinterHelper(
+      const PrintingPolicy &P, Sema &S,
+      SmallVectorImpl<PartialDiagnosticAt> &Notes)
+      : Policy(P), S(S), Notes(Notes) {}
 
   bool handledStmt(Stmt *E, raw_ostream &OS) override {
     const auto *DR = dyn_cast<DeclRefExpr>(E);
     if (DR && DR->getQualifier()) {
       // If this is a qualified name, expand the template arguments in nested
       // qualifiers.
       DR->getQualifier()->print(OS, Policy, true);
       // Then print the decl itself.
       const ValueDecl *VD = DR->getDecl();
       OS << VD->getName();
       if (const auto *IV = dyn_cast<VarTemplateSpecializationDecl>(VD)) {
         // This is a template variable, print the expanded template arguments.
         printTemplateArgumentList(
             OS, IV->getTemplateArgs().asArray(), Policy,
             IV->getSpecializedTemplate()->getTemplateParameters());
       }
       return true;
+    } else if (auto *UE = dyn_cast<UnaryExprOrTypeTraitExpr>(E)) {
+      Expr::EvalResult Result;
+      if (UE->EvaluateAsConstantExpr(Result, S.Context) && Result.Val.isInt()) {
+        std::string ExprStr;
+        llvm::raw_string_ostream ExprStream(ExprStr);
+        UE->printPretty(ExprStream, nullptr, Policy);
+        ExprStream.flush();
+        Notes.push_back(PartialDiagnosticAt(
+            UE->getExprLoc(),
+            S.PDiag(diag::note_static_assert_requirement_context)
+                << ExprStr << toString(Result.Val.getInt(), 10)
+                << UE->getSourceRange()));
+      }
     }
     return false;
   }
 
 private:
   const PrintingPolicy Policy;
+  Sema &S;
+  SmallVectorImpl<PartialDiagnosticAt> &Notes;
 };
 
 } // end anonymous namespace
 
 std::pair<Expr *, std::string>
-Sema::findFailedBooleanCondition(Expr *Cond) {
+Sema::findFailedBooleanCondition(Expr *Cond,
+                                 SmallVectorImpl<PartialDiagnosticAt> &Notes) {
   Cond = lookThroughRangesV3Condition(PP, Cond);
 
   // Separate out all of the terms in a conjunction.
   SmallVector<Expr *, 4> Terms;
   collectConjunctionTerms(Cond, Terms);
 
   // Determine which term failed.
   Expr *FailedCond = nullptr;
Sema::findFailedBooleanCondition(Expr *Cond,
   if (!FailedCond)
     FailedCond = Cond->IgnoreParenImpCasts();
 
   std::string Description;
   {
     llvm::raw_string_ostream Out(Description);
     PrintingPolicy Policy = getPrintingPolicy();
     Policy.PrintCanonicalTypes = true;
-    FailedBooleanConditionPrinterHelper Helper(Policy);
+    FailedBooleanConditionPrinterHelper Helper(Policy, *this, Notes);
     FailedCond->printPretty(Out, &Helper, Policy, 0, "\n", nullptr);
   }
   return { FailedCond, Description };
 }
 
 QualType Sema::CheckTemplateIdType(TemplateName Name,
                                    SourceLocation TemplateLoc,
                                    TemplateArgumentListInfo &TemplateArgs) {
if (CanonType.isNull()) {
           if (*DeductionInfo &&
               (*DeductionInfo)->hasSFINAEDiagnostic() &&
               (*DeductionInfo)->peekSFINAEDiagnostic().second.getDiagID() ==
                 diag::err_typename_nested_not_found_enable_if &&
               TemplateArgs[0].getArgument().getKind()
                 == TemplateArgument::Expression) {
             Expr *FailedCond;
             std::string FailedDescription;
+            SmallVector<PartialDiagnosticAt, 2> Notes;
             std::tie(FailedCond, FailedDescription) =
-              findFailedBooleanCondition(TemplateArgs[0].getSourceExpression());
+                findFailedBooleanCondition(
+                    TemplateArgs[0].getSourceExpression(), Notes);
 
             // Remove the old SFINAE diagnostic.
             PartialDiagnosticAt OldDiag =
               {SourceLocation(), PartialDiagnostic::NullDiagnostic()};
             (*DeductionInfo)->takeSFINAEDiagnostic(OldDiag);
 
             // Add a new SFINAE diagnostic specifying which condition
             // failed.
             (*DeductionInfo)->addSFINAEDiagnostic(
               OldDiag.first,
               PDiag(diag::err_typename_nested_not_found_requirement)
                 << FailedDescription
                 << FailedCond->getSourceRange());
+            for (const PartialDiagnosticAt &Note : Notes)
+              (*DeductionInfo)->addSFINAEDiagnostic(Note.first, Note.second);
           }
         }
       }
 
       return QualType();
     }
   } else if (Name.isDependent() ||
              TemplateSpecializationType::anyDependentTemplateArguments(
case LookupResult::NotFound: {
     SourceRange CondRange;
     Expr *Cond = nullptr;
     if (Ctx && isEnableIf(QualifierLoc, II, CondRange, Cond)) {
       // If we have a condition, narrow it down to the specific failed
       // condition.
       if (Cond) {
         Expr *FailedCond;
         std::string FailedDescription;
+        SmallVector<PartialDiagnosticAt, 2> Notes;
         std::tie(FailedCond, FailedDescription) =
-          findFailedBooleanCondition(Cond);
+            findFailedBooleanCondition(Cond, Notes);
 
         Diag(FailedCond->getExprLoc(),
              diag::err_typename_nested_not_found_requirement)
           << FailedDescription
           << FailedCond->getSourceRange();
+        for (const PartialDiagnosticAt &Note : Notes)
+          Diag(Note.first, Note.second);
         return QualType();
       }
 
       Diag(CondRange.getBegin(),
            diag::err_typename_nested_not_found_enable_if)
           << Ctx << CondRange;
       return QualType();
     }

clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp

template<typename T> void f() {
     extern int arr1[];
     extern T arr1;
     extern T arr2;
     extern int arr2[];
     static_assert(sizeof(arr1) == 12, "");
     static_assert(sizeof(arr2) == 12, "");
 
     // Use a failing test to ensure the type isn't considered dependent.
-    static_assert(sizeof(arr2) == 13, ""); // expected-error {{failed}}
+    static_assert(sizeof(arr2) == 13, ""); // expected-error {{failed}} expected-note{{with 'sizeof (arr2)' equal to 12}}
   }
 
   void g() { f<int[3]>(); } // expected-note {{in instantiation of}}
 
   template<typename T> void h1() {
     extern T arr3;
     {
       int arr3;

clang/test/Parser/objc-static-assert.mm

@interface A {
   static_assert(1, "");
   _Static_assert(1, "");
 
   static_assert(0, ""); // expected-error {{static_assert failed}}
   _Static_assert(0, ""); // expected-error {{static_assert failed}}
 
   static_assert(a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
   static_assert(sizeof(a) == 4, "");
-  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}}
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}} expected-note{{with 'sizeof (a)' equal to 4}}
 }
 
 static_assert(1, "");
 _Static_assert(1, "");
 
 - (void)f;
 @end
 
 @implementation A {
   int b;
   static_assert(1, "");
   _Static_assert(1, "");
   static_assert(sizeof(b) == 4, "");
-  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}} expected-note{{with 'sizeof (b)' equal to 4}}
 }
 
 static_assert(1, "");
 
 - (void)f {
   static_assert(1, "");
 }
 @end
 
 @interface B
 @end
 
 @interface B () {
   int b;
   static_assert(sizeof(b) == 4, "");
-  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}} expected-note{{with 'sizeof (b)' equal to 4}}
 }
 @end
 
 #else
 
 #if __has_feature(objc_cxx_static_assert)
 #error failed
 #endif

clang/test/Sema/static-assert.c

#define UNION(T1, T2) union { \
     _Static_assert(ASSERT_IS_TYPE(T1), "T1 is not a type"); \
     _Static_assert(ASSERT_IS_TYPE(T2), "T2 is not a type"); \
     _Static_assert(sizeof(T1) == sizeof(T2), "type size mismatch"); \
   }
 
 typedef UNION(unsigned, struct A) U1; // ext-warning 3 {{'_Static_assert' is a C11 extension}}
 UNION(char[2], short) u2 = { .one = { 'a', 'b' } }; // ext-warning 3 {{'_Static_assert' is a C11 extension}} cxx-warning {{designated initializers are a C++20 extension}}
 typedef UNION(char, short) U3; // expected-error {{static_assert failed due to requirement 'sizeof(char) == sizeof(short)' "type size mismatch"}} \
-                               // ext-warning 3 {{'_Static_assert' is a C11 extension}}
+                               // ext-warning 3 {{'_Static_assert' is a C11 extension}} \
+                               // expected-note {{with 'sizeof(char)' equal to 1}} \
+                               // expected-note {{with 'sizeof(short)' equal to 2}}
 typedef UNION(float, 0.5f) U4; // expected-error {{expected a type}} \
                                // ext-warning 3 {{'_Static_assert' is a C11 extension}}
 
 // After defining the assert macro in MS-compatibility mode, we should
 // no longer warn about including <assert.h> under the assumption the
 // user already did that.
 #ifdef MS
 #define assert(expr)
 static_assert(1, "1 is nonzero"); // ok
 
 // But we should still warn if the user did something bonkers.
 #undef static_assert
 static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
 #endif

clang/test/SemaCXX/constant-expression-cxx11.cpp

namespace VirtualFromBase {
 
   // Virtual f(), not OK.
   constexpr X<X<S1>> xxs1;
   constexpr X<S1> *p = const_cast<X<X<S1>>*>(&xxs1);
   static_assert(p->f() == sizeof(X<S1>), "");
   // cxx11-error@-1    {{not an integral constant expression}}
   // cxx11-note@-2     {{call to virtual function}}
   // cxx20_2b-error@-3 {{static_assert failed}}
+  // cxx20_2b-note@-4  {{with 'sizeof(VirtualFromBase::X<VirtualFromBase::S1>)' equal to 16}}
 
   // Non-virtual f(), OK.
   constexpr X<X<S2>> xxs2;
   constexpr X<S2> *q = const_cast<X<X<S2>>*>(&xxs2);
   static_assert(q->f() == sizeof(S2), ""); // cxx20_2b-error {{static_assert failed}}
+  // cxx20_2b-note@-1 {{with 'sizeof(VirtualFromBase::S2)' equal to 8}}
 }
 
 namespace ConstexprConstructorRecovery {
   class X {
   public:
       enum E : short {
           headers = 0x1,
           middlefile = 0x2,

clang/test/SemaCXX/static-assert-cxx17.cpp

void foo6() {
   static_assert(X<typename T::T>({1, 2}));
   // expected-error@-1{{static_assert failed due to requirement 'X<int>({1, 2})'}}
   static_assert(typename T::T{0});
   // expected-error@-1{{static_assert failed due to requirement 'int{0}'}}
   static_assert(typename T::T(0));
   // expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
   static_assert(sizeof(X<typename T::T>) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X<int>) == 0'}}
+  // expected-note@-2{{with 'sizeof(X<int>)' equal to 8}}
   static_assert((const X<typename T::T> *)nullptr);
   // expected-error@-1{{static_assert failed due to requirement '(const X<int> *)nullptr'}}
   static_assert(static_cast<const X<typename T::T> *>(nullptr));
   // expected-error@-1{{static_assert failed due to requirement 'static_cast<const X<int> *>(nullptr)'}}
   static_assert((const X<typename T::T>[]){} == nullptr);
   // expected-error@-1{{static_assert failed due to requirement '(X<int> const[0]){} == nullptr'}}
   static_assert(sizeof(X<decltype(X<typename T::T>().X<typename T::T>::~X())>) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X<void>) == 0'}}
+  // expected-note@-2{{with 'sizeof(X<void>)' equal to 8}}
   static_assert(constexpr_return_false<typename T::T, typename T::U>());
   // expected-error@-1{{static_assert failed due to requirement 'constexpr_return_false<int, float>()'}}
 }
 template void foo6<ExampleTypes>();
 // expected-note@-1{{in instantiation of function template specialization 'foo6<ExampleTypes>' requested here}}

clang/test/SemaCXX/static-assert.cpp

 
 template<int N> struct T {
     static_assert(N == 2, "N is not 2!"); // expected-error {{static_assert failed due to requirement '1 == 2' "N is not 2!"}}
 };
 
 T<1> t1; // expected-note {{in instantiation of template class 'T<1>' requested here}}
 T<2> t2;
 
-template<typename T> struct S {
-    static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static_assert failed due to requirement 'sizeof(char) > sizeof(char)' "Type not big enough!"}}
+template <typename T> struct S {
+                                                                   // expected-note@-1{{with 'sizeof(char)' equal to 1}}
+                                                                   // expected-note@-2{{with 'sizeof(char)' equal to 1}}
 };
 
 S<char> s1; // expected-note {{in instantiation of template class 'S<char>' requested here}}
 S<int> s2;
 
 static_assert(false, L"\xFFFFFFFF"); // expected-error {{static_assert failed L"\xFFFFFFFF"}}
 static_assert(false, u"\U000317FF"); // expected-error {{static_assert failed u"\U000317FF"}}
 // FIXME: render this as u8"\u03A9"

 constexpr int *p = 0;
 static_assert(p, "message"); // expected-error {{static_assert failed}}
 
 struct NotBool {
 } notBool;
 constexpr NotBool constexprNotBool;
 static_assert(notBool, "message");          // expected-error {{value of type 'struct NotBool' is not contextually convertible to 'bool'}}
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
+
+struct IntAndPointer {
+  int i;
+  void *p;
+};
+static_assert(sizeof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'sizeof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'sizeof(IntAndPointer)' equal to 16}}
+static_assert(alignof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+static_assert(alignof(IntAndPointer) == sizeof(IntAndPointer), "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+// expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}

clang/test/SemaTemplate/overload-candidates.cpp

 template<> struct is_int<int> { enum { value = true }; };
 template<typename T> typename llvm::enable_if<is_int<T> >::type if_int(); // expected-note{{candidate template ignored: disabled by 'enable_if' [with T = char]}}
 void test_if_int() {
   if_int<char>(); // expected-error{{no matching function}}
 }
 
 template<typename T> struct NonTemplateFunction {
   typename boost::enable_if<sizeof(T) == 4, int>::type f(); // expected-error{{failed requirement 'sizeof(char) == 4'; 'enable_if' cannot be used to disable this declaration}}
+  // expected-note@-1{{with 'sizeof(char)' equal to 1}}
 };
 NonTemplateFunction<char> NTFC; // expected-note{{here}}
 
 namespace NS1 {
   template <class A>
   class array {};
 }