[clang-tidy] Check for sizeof that call functions

[pfultz2]

Summary

A common mistake that I have found in our codebase is calling a function to get an integer or enum that represents the type such as:

int numBytes = numElements * sizeof(x.GetType());

So this extends the sizeof check to check for these cases. There is also a WarnOnSizeOfCall option so it can be disabled.

[lebedev.ri]

Please do bother to prefix with the appropriate [prefix], set the appropriate repo, and tags!

[hokein]

Can you elaborate a bit more about this? I think we also need to update the check document (adding proper section of this new behavior, and the new option).

[alexfh]

clang-tidy/misc/SizeofExpressionCheck.cpp

220 ↗

(On Diff #137499)

I'm not sure I understand the message "suspicious usage of ... to an integer". Specifically, what does the "to an integer" part mean?

[pfultz2]

Can you elaborate a bit more about this?

This catches problems when calling sizeof(f()) when f returns an integer(or enum). This is because, most likely, the integer represents the type to be chosen, however, sizeof is being computed for an integer and not the type the integer represents. That is, the user has an enum for the data_type:

enum data_type {
    float_type,
    double_type
};

At some point the user may call a function to get the data type(perhaps x.GetType()) and pass it on to sizeof, like sizeof(x.GetType()), which is incorrect.

I think we also need to update the check document (adding proper section of this new behavior, and the new option).

I will update the doc.

clang-tidy/misc/SizeofExpressionCheck.cpp

220 ↗

(On Diff #137499)

That could probably be worded better. It means the expr is an integer type. Maybe I should say suspicious usage of 'sizeof() on an expression that results in an integer?

[aaron.ballman]

In D44231#1031380, @pfultz2 wrote:

Can you elaborate a bit more about this?

This catches problems when calling sizeof(f()) when f returns an integer(or enum). This is because, most likely, the integer represents the type to be chosen, however, sizeof is being computed for an integer and not the type the integer represents. That is, the user has an enum for the data_type:

enum data_type {
    float_type,
    double_type
};

At some point the user may call a function to get the data type(perhaps x.GetType()) and pass it on to sizeof, like sizeof(x.GetType()), which is incorrect.

Can you point to some real world code that would benefit from this check? I don't see this as being a common issue with code and I am concerned about false positives. For instance, it's reasonable to write sizeof(func_call()) in a context where you don't want to repeat the type name in multiple places. I've seen this construct used in LLVM's code base (see Prologue::getLength() in DWARFDebugLine.h for one such instance).

[pfultz2]

Can you point to some real world code that would benefit from this check?

Yes, here:

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/master/src/convolution.cpp#L184

It is erroneously calling sizeof(yDesc.GetType()). The GetType function returns an enum that represents the float type. In this case it wants to compute the size of the float type not the size of the enum that represents the float type. There's actually many instances of this error in the codebase.

I don't see this as being a common issue with code and I am concerned about false positives.

I have seen it as a common issue. It only runs for a function that returns an integer type. Another possibility is to not warn for function calls that are dependent on a template parameter.

Of course, there is still the option to disable it.

For instance, it's reasonable to write sizeof(func_call()) in a context where you don't want to repeat the type name in multiple places.

Generally, a typedef is used to avoid repeating the type.

I've seen this construct used in LLVM's code base (see Prologue::getLength() in DWARFDebugLine.h for one such instance).

This is the first time I have seen this used legitimately outside of template metaprogramming. Alternatively, you could also write sizeof(decltype(expr)).

[aaron.ballman]

In D44231#1031503, @pfultz2 wrote:

Can you point to some real world code that would benefit from this check?

Yes, here:

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/master/src/convolution.cpp#L184

It is erroneously calling sizeof(yDesc.GetType()). The GetType function returns an enum that represents the float type. In this case it wants to compute the size of the float type not the size of the enum that represents the float type. There's actually many instances of this error in the codebase.

Thank you for the example.

I don't see this as being a common issue with code and I am concerned about false positives.

I have seen it as a common issue. It only runs for a function that returns an integer type. Another possibility is to not warn for function calls that are dependent on a template parameter.

Of course, there is still the option to disable it.

For instance, it's reasonable to write sizeof(func_call()) in a context where you don't want to repeat the type name in multiple places.

Generally, a typedef is used to avoid repeating the type.

The typedef doesn't avoid the repeating type.

uint16_t foo();
...
size_t s1 = sizeof(foo()); // No repeated type
size_t s2 = sizeof(uint16_t); // Repeated type

If the return type of foo() is changed, then the use for s1 will be automatically updated while the use for s2 will not. Your example of how to work around it using decltype isn't obvious and won't work for C users.

I've seen this construct used in LLVM's code base (see Prologue::getLength() in DWARFDebugLine.h for one such instance).

This is the first time I have seen this used legitimately outside of template metaprogramming. Alternatively, you could also write sizeof(decltype(expr)).

I think we need some data measurements over several large (>500k LoC) C and C++ code bases to see how frequently this check yields false positives. I have a hard time imagining that the true positives will outweigh the false positives, but without concrete data, it's just guesswork.

[pfultz2]

If the return type of foo() is changed, then the use for s1 will be automatically updated

But usually you write it as:

using foo_return = uint16_t;
foo_return foo();
...
size_t s1 = sizeof(foo());
size_t s2 = sizeof(foo_return);

So you just update the foo_return typedef, and it will update it everywhere that type is used.

I think we need some data measurements over several large (>500k LoC) C and C++ code bases to see how frequently this check yields false positives.

So I ran it on llvm. It mainly complained about areas where sizeof is used for building traits. I should add an extra check that the function is not overloaded.

It did complain once about sizeof in a detection idiom(in BitmaskEnum.h) and once with sizeof(getVersion()), like you mentioned.

[aaron.ballman]

In D44231#1032011, @pfultz2 wrote:

If the return type of foo() is changed, then the use for s1 will be automatically updated

But usually you write it as:

using foo_return = uint16_t;
foo_return foo();
...
size_t s1 = sizeof(foo());
size_t s2 = sizeof(foo_return);

So you just update the foo_return typedef, and it will update it everywhere that type is used.

Again, that only works for C++ and not C.

I think we need some data measurements over several large (>500k LoC) C and C++ code bases to see how frequently this check yields false positives.

So I ran it on llvm. It mainly complained about areas where sizeof is used for building traits. I should add an extra check that the function is not overloaded.

It did complain once about sizeof in a detection idiom(in BitmaskEnum.h) and once with sizeof(getVersion()), like you mentioned.

Did it report any true positives that would need correcting?

LLVM is a good start, but hardly representative. Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, rethinkdb, redis, and php-src? Did they produce true positives? How was the false positive rate?

[pfultz2]

Again, that only works for C++ and not C.

Typedef has always worked in C.

Did it report any true positives that would need correcting?

Not for LLVM, but it has in other projects like I mentioned.

Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, rethinkdb, redis, and php-src?

None of those projects use cmake, so it doesn't look easy to run clang-tidy on them. Do you have a script that will run clang-tidy on these repos?

Actually, PVS-Studio has a more general check for sizeof(expr):

https://www.viva64.com/en/examples/v568/

It shows an example of FCEUX doing sizeof(buf - 1), Asterisk doing sizeof(data[0] * 2) and ReactOS doing sizeof(UnknownError [0] - 20). I think it might be a good idea to expand this from just a function call to any integer expression.

[aaron.ballman]

In D44231#1032782, @pfultz2 wrote:

Again, that only works for C++ and not C.

Typedef has always worked in C.

This is true.

I think what I'm struggling to say is: I don't think this is a common pattern in either C or C++. It's also unnecessary because you can avoid repeating the type by just using sizeof on the function call result.

Did it report any true positives that would need correcting?

Not for LLVM, but it has in other projects like I mentioned.

Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, rethinkdb, redis, and php-src?

None of those projects use cmake, so it doesn't look easy to run clang-tidy on them. Do you have a script that will run clang-tidy on these repos?

I don't have a script for it. I've used "bear" with at least some of those projects because they use makefiles rather than cmake (https://github.com/rizsotto/Bear). I'm not tied to those projects specifically, either, so if you have a different corpus you'd prefer to use, that's fine. The important bit is testing it across a considerable amount of C code and C++ code to see whether this diagnostic is too chatty or not.

Actually, PVS-Studio has a more general check for sizeof(expr):

https://www.viva64.com/en/examples/v568/

It shows an example of FCEUX doing sizeof(buf - 1), Asterisk doing sizeof(data[0] * 2) and ReactOS doing sizeof(UnknownError [0] - 20). I think it might be a good idea to expand this from just a function call to any integer expression.

That won't catch many (most?) of the issues demonstrated by PVS-Studio; the rule their check follows are to warn on side-effecting operations (which Clang already does with -Wunevaluated-expression) and arithmetic expressions in sizeof. The latter might be a reasonable extension to the check -- I have less concerns about the false positive rate with that than I do with the function call case. Another possible extension to consider is sizeof(sizeof(foo)), which it seems PVS-Studio will diagnose as well.

[pfultz2]

I don't have a script for it. I've used "bear" with at least some of those projects because they use makefiles rather than cmake (https://github.com/rizsotto/Bear). I'm not tied to those projects specifically, either, so if you have a different corpus you'd prefer to use, that's fine. The important bit is testing it across a considerable amount of C code and C++ code to see whether this diagnostic is too chatty or not.

So I did a grep over these codebases(with grep -E '\bsizeof\([^()"*]+\([^()]*\)'). Most of them are macros which access elements(ie no function call) or are used in type traits. The only false positive I saw was here:

https://github.com/rethinkdb/rethinkdb/blob/v2.3.x/external/re2_20140111/re2/prog.cc#L317

So I dont think it will be too chatty.

That won't catch many (most?) of the issues demonstrated by PVS-Studio; the rule their check follows are to warn on side-effecting operations (which Clang already does with -Wunevaluated-expression) and arithmetic expressions in sizeof.

It finds function calls as well. I tried on MIOpen and it caught the errors like I mentioned earlier here:

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/master/src/convolution.cpp#L184

[pfultz2]

So I've updated the documentation on this. Are there any other updates needed for this to get it merged in?

[hokein]

As this patch can catch some mistakes, I'm fine with checking it in. I agree that it is reasonable to write normal code like sizeof(func_call()) (not false positive), maybe set the option to false by default?

[aaron.ballman]

In D44231#1039888, @hokein wrote:

As this patch can catch some mistakes, I'm fine with checking it in. I agree that it is reasonable to write normal code like sizeof(func_call()) (not false positive), maybe set the option to false by default?

I am okay with requiring people to enable this part of the check explicitly.

clang-tidy/misc/SizeofExpressionCheck.cpp

220 ↗

(On Diff #137499)

Missing a terminating quote around sizeof.

docs/clang-tidy/checks/misc-sizeof-expression.rst

28 ↗

(On Diff #137822)

It's not obvious what "the type" refers to in this sentence. Also, it is incorrect to state that "A common mistake is to query the size of an integer" -- that's an incredibly common practice. You may need a bit more setup in the explanation to give the reader context.

[pfultz2]

I have reworded the documentation. Hopefully it is clearer.

[pfultz2]

As this patch can catch some mistakes, I'm fine with checking it in. I agree that it is reasonable to write normal code like sizeof(func_call()) (not false positive), maybe set the option to false by default?

I have disabled it by default. We can decide later if we want it to enabled by default.

[aaron.ballman]

test/clang-tidy/misc-sizeof-expression.cpp

17 ↗

(On Diff #138791)

Can you add a C++11 test case using enum class -- I am worried that the isInteger() matcher will return false for that type.

Also, I think you should have a test case for bool; I think the check will diagnose bool types but I'm not certain there's value in diagnosing those. What do you think?

[alexfh]

LG modulo other reviewers' open comments.

[pfultz2]

So, I've added tests for class enums and bols.

[alexfh]

LG. Do you need someone to submit this for you?

test/clang-tidy/misc-sizeof-expression.cpp

17 ↗

(On Diff #138791)

I think the check will diagnose bool types but I'm not certain there's value in diagnosing those

It will be easier to decide once someone finds a few examples of those in real life code.

[pfultz2]

Do you need someone to submit this for you?

Yes

[pfultz2]

Is someone able to merge in my changes?

[hokein]

Sorry for the delay -- everyone was out because of the long Easter weekend. I'll commit for you.

[hokein]

pfultz2@, could you rebase this patch? The check has been moved to bugprone.

[pfultz2]

I have rebased.

[hokein]

Thanks, I have committed for you.

clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h

 public:
   SizeofExpressionCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   const bool WarnOnSizeOfConstant;
+  const bool WarnOnSizeOfIntegerExpression;
   const bool WarnOnSizeOfThis;
   const bool WarnOnSizeOfCompareToConstant;
 };
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIZEOFEXPRESSIONCHECK_H

clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp

 }
 
 } // namespace
 
 SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
+      WarnOnSizeOfIntegerExpression(
+          Options.get("WarnOnSizeOfIntegerExpression", 0) != 0),
       WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
       WarnOnSizeOfCompareToConstant(
           Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
 
 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
+  Options.store(Opts, "WarnOnSizeOfIntegerExpression",
+                WarnOnSizeOfIntegerExpression);
   Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
   Options.store(Opts, "WarnOnSizeOfCompareToConstant",
                 WarnOnSizeOfCompareToConstant);
 }
 
 void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
   const auto ConstantExpr = expr(ignoringParenImpCasts(
       anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
             binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr)))));
+  const auto IntegerCallExpr = expr(ignoringParenImpCasts(
+      callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
+               unless(isInTemplateInstantiation()))));
   const auto SizeOfExpr =
       expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr()))));
   const auto SizeOfZero = expr(
       sizeOfExpr(has(ignoringParenImpCasts(expr(integerLiteral(equals(0)))))));
 
   // Detect expression like: sizeof(ARRAYLEN);
   // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
   //       the sizeof size_t.
   if (WarnOnSizeOfConstant) {
     Finder->addMatcher(
         expr(sizeOfExpr(has(ignoringParenImpCasts(ConstantExpr))),
              unless(SizeOfZero))
             .bind("sizeof-constant"),
         this);
   }
 
+  // Detect sizeof(f())
+  if (WarnOnSizeOfIntegerExpression) {
+    Finder->addMatcher(
+        expr(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr))))
+            .bind("sizeof-integer-call"),
+        this);
+  }
+
   // Detect expression like: sizeof(this);
   if (WarnOnSizeOfThis) {
     Finder->addMatcher(
         expr(sizeOfExpr(has(ignoringParenImpCasts(expr(cxxThisExpr())))))
             .bind("sizeof-this"),
         this);
   }
 

 }
 
 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
   const ASTContext &Ctx = *Result.Context;
 
   if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
     diag(E->getLocStart(),
          "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
+  } else if (const auto *E =
+                 Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
+    diag(E->getLocStart(), "suspicious usage of 'sizeof()' on an expression "
+                           "that results in an integer");
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
     diag(E->getLocStart(),
          "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
     diag(E->getLocStart(),
          "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
   } else if (const auto *E =
                  Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst

 programmer was probably to simply get the integer and not its size.
 
 .. code-block:: c++
 
   #define BUFLEN 42
   char buf[BUFLEN];
   memset(buf, 0, sizeof(BUFLEN));  // sizeof(42) ==> sizeof(int)
 
+Suspicious usage of 'sizeof(expr)'
+----------------------------------
+
+In cases, where there is an enum or integer to represent a type, a common
+mistake is to query the ``sizeof`` on the integer or enum that represents the
+type that should be used by ``sizeof``. This results in the size of the integer
+and not of the type the integer represents:
+
+.. code-block:: c++
+
+  enum data_type {
+    FLOAT_TYPE,
+    DOUBLE_TYPE
+  };
+
+  struct data {
+    data_type type;
+    void* buffer;
+    data_type get_type() {
+      return type;
+    }
+  };
+
+  void f(data d, int numElements) {
+    // should be sizeof(float) or sizeof(double), depending on d.get_type()
+    int numBytes = numElements * sizeof(d.get_type());
+    ...
+  }
+
+
 Suspicious usage of 'sizeof(this)'
 ----------------------------------
 
 The ``this`` keyword is evaluated to a pointer to an object of a given type.
 The expression ``sizeof(this)`` is returning the size of a pointer. The
 programmer most likely wanted the size of the object and not the size of the
 pointer.
 

 Options
 -------
 
 .. option:: WarnOnSizeOfConstant
 
    When non-zero, the check will warn on an expression like
    ``sizeof(CONSTANT)``. Default is `1`.
 
+.. option:: WarnOnSizeOfIntegerExpression
+
+   When non-zero, the check will warn on an expression like ``sizeof(expr)``
+   where the expression results in an integer. Default is `0`.
+
 .. option:: WarnOnSizeOfThis
 
    When non-zero, the check will warn on an expression like ``sizeof(this)``.
    Default is `1`.
 
 .. option:: WarnOnSizeOfCompareToConstant
 
    When non-zero, the check will warn on an expression like
    ``sizeof(epxr) <= k`` for a suspicious constant `k` while `k` is `0` or
    greater than `0x8000`. Default is `1`.

clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp

-// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -config="{CheckOptions: [{key: bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression, value: 1}]}" --
 
 class C {
   int size() { return sizeof(this); }
   // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
 };
 
 #define LEN 8
 
 int X;
 extern int A[10];
 extern short B[10];
 
 #pragma pack(1)
 struct  S { char a, b, c; };
 
+enum E { E_VALUE = 0 };
+enum class EC { VALUE = 0 };
+
+bool AsBool() { return false; }
+int AsInt() { return 0; }
+E AsEnum() { return E_VALUE; }
+EC AsEnumClass() { return EC::VALUE; }
+S AsStruct() { return {}; }
+
+struct M {
+  int AsInt() { return 0; }
+  E AsEnum() { return E_VALUE; }
+  S AsStruct() { return {}; }
+};
+
+int ReturnOverload(int) { return {}; }
+S ReturnOverload(S) { return {}; }
+
+template <class T>
+T ReturnTemplate(T) { return {}; }
+
+template <class T>
+bool TestTrait1() {
+  return sizeof(ReturnOverload(T{})) == sizeof(A);
+}
+
+template <class T>
+bool TestTrait2() {
+  return sizeof(ReturnTemplate(T{})) == sizeof(A);
+}
+
+template <class T>
+bool TestTrait3() {
+  return sizeof(ReturnOverload(0)) == sizeof(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+template <class T>
+bool TestTrait4() {
+  return sizeof(ReturnTemplate(0)) == sizeof(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+bool TestTemplates() {
+  bool b = true;
+  b &= TestTrait1<int>();
+  b &= TestTrait1<S>();
+  b &= TestTrait2<int>();
+  b &= TestTrait2<S>();
+  b &= TestTrait3<int>();
+  b &= TestTrait3<S>();
+  b &= TestTrait4<int>();
+  b &= TestTrait4<S>();
+  return b;
+}
+
 int Test1(const char* ptr) {
   int sum = 0;
   sum += sizeof(LEN);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(AsBool());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsEnum());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsEnumClass());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(M{}.AsInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(M{}.AsEnum());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(LEN + sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(LEN + LEN + sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(LEN + (LEN + sizeof(X)));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'

 int ValidExpressions() {
   int A[] = {1, 2, 3, 4};
   static const char str[] = "hello";
   static const char* ptr[] { "aaa", "bbb", "ccc" };
   int sum = 0;
   if (sizeof(A) < 10)
     sum += sizeof(A);
   sum += sizeof(int);
+  sum += sizeof(AsStruct());
+  sum += sizeof(M{}.AsStruct());
   sum += sizeof(A[sizeof(A) / sizeof(int)]);
   sum += sizeof(&A[sizeof(A) / sizeof(int)]);
   sum += sizeof(sizeof(0));  // Special case: sizeof size_t.
   sum += sizeof(void*);
   sum += sizeof(void const *);
   sum += sizeof(void const *) / 4;
   sum += sizeof(str);
   sum += sizeof(str) / sizeof(char);
   sum += sizeof(str) / sizeof(str[0]);
   sum += sizeof(ptr) / sizeof(ptr[0]);
   sum += sizeof(ptr) / sizeof(*(ptr));
   return sum;
 }