[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Is it worth using regex to ignore some operators:
`*::operator bool` to ignore all bool conversions etc.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst:12-14
+This check implements `C.46 
`_
+and `C.164 
`_
+from the CppCoreGuidelines.

The page generated from here will automatically redirect to the alias after 5 
seconds so any extra documentation in here won't be read.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst:75
+operator `operator B()` in class ``A`` would look as follows:
+``"A::operator B"``. The default list is empty.

double quotes aren't needed in options.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp:1-4
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'

Passing an explicitly empty config is unnecessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102779/new/

https://reviews.llvm.org/D102779

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap marked 4 inline comments as done.
gergap added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:369
 assert(Shift >= 0);
+if (Shift == 0)
+  continue;

HazardyKnusperkeks wrote:
> This is unrelated, isn't it?
> 
> If it is, I would like a separate commit. Otherwise an explanation why it is 
> now mandatory and does not infer with the other alignments.
I actually don't know. Just copied this from the original patch.



Comment at: clang/unittests/Format/FormatTest.cpp:14884
   Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  Alignment.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("float const a = 5;\n"

HazardyKnusperkeks wrote:
> Why change this?
It is already set to PAS_Right by default. But when reading the code you don't 
know this. I needed to debug this to find this out.
The following verifyFormat tests depend on PAS_Right style.



Comment at: clang/unittests/Format/FormatTest.cpp:15045
+  // PAS_RIGHT
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"

HazardyKnusperkeks wrote:
> I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I know 
> someone who will request that. :)
> 
> You should change that here and use that for your following tests.
I don't know, because I'm not the author of that code.
But I can change it to verifyFormat if this is what you prefer.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103358: [analyzer] MallocSizeof: sizeof pointer type is compatible with void*

[Xuanda Yang via Phabricator via cfe-commits]

TH3CHARLie created this revision.
TH3CHARLie added a reviewer: NoQ.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
TH3CHARLie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

source: https://bugs.llvm.org/show_bug.cgi?id=50214

Make sizeof pointer type is compatible with void* in MallocSizeofChecker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103358

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  clang/test/Analysis/malloc-sizeof.c


Index: clang/test/Analysis/malloc-sizeof.c
===
--- clang/test/Analysis/malloc-sizeof.c
+++ clang/test/Analysis/malloc-sizeof.c
@@ -26,6 +26,8 @@
   struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 
'calloc' is converted to a pointer of type 'struct A', which is incompatible 
with sizeof operand type 'struct B'}}
   struct A *ap6 = realloc(ap5, sizeof(struct A));
   struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result 
of 'realloc' is converted to a pointer of type 'struct A', which is 
incompatible with sizeof operand type 'struct B'}}
+
+  void **vpp1 = (void **)malloc(sizeof(struct A*)); // no warning
 }
 
 // Don't warn when the types differ only by constness.
Index: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -139,6 +139,10 @@
   if (B->isVoidPointerType() && A->getAs())
 return true;
 
+  // sizeof(pointer type) is compatible with void*
+  if (A->isVoidPointerType() && B->getAs())
+return true;
+
   while (true) {
 A = A.getCanonicalType();
 B = B.getCanonicalType();


Index: clang/test/Analysis/malloc-sizeof.c
===
--- clang/test/Analysis/malloc-sizeof.c
+++ clang/test/Analysis/malloc-sizeof.c
@@ -26,6 +26,8 @@
   struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 'calloc' is converted to a pointer of type 'struct A', which is incompatible with sizeof operand type 'struct B'}}
   struct A *ap6 = realloc(ap5, sizeof(struct A));
   struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result of 'realloc' is converted to a pointer of type 'struct A', which is incompatible with sizeof operand type 'struct B'}}
+
+  void **vpp1 = (void **)malloc(sizeof(struct A*)); // no warning
 }
 
 // Don't warn when the types differ only by constness.
Index: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -139,6 +139,10 @@
   if (B->isVoidPointerType() && A->getAs())
 return true;
 
+  // sizeof(pointer type) is compatible with void*
+  if (A->isVoidPointerType() && B->getAs())
+return true;
+
   while (true) {
 A = A.getCanonicalType();
 B = B.getCanonicalType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap marked an inline comment as done.
gergap added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:15045
+  // PAS_RIGHT
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"

gergap wrote:
> HazardyKnusperkeks wrote:
> > I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I know 
> > someone who will request that. :)
> > 
> > You should change that here and use that for your following tests.
> I don't know, because I'm not the author of that code.
> But I can change it to verifyFormat if this is what you prefer.
> 
The verifyFormat() call with one code parameter does not work with this test 
pattern, because the internal messUpCode function removes the newlines.
The consecutive alignments are interrupted by newlines, which lead to different 
indent for each section. This breaks with messUpCode.

However, there is a verifyFormat function with two code arguments, wich behaves 
similar to the existing EXPECT_EQ. This way it works.
I go for this option now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14884
   Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  Alignment.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("float const a = 5;\n"

gergap wrote:
> HazardyKnusperkeks wrote:
> > Why change this?
> It is already set to PAS_Right by default. But when reading the code you 
> don't know this. I needed to debug this to find this out.
> The following verifyFormat tests depend on PAS_Right style.
Then make it an EXPECT_EQ.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:15045
+  // PAS_RIGHT
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"

gergap wrote:
> gergap wrote:
> > HazardyKnusperkeks wrote:
> > > I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I 
> > > know someone who will request that. :)
> > > 
> > > You should change that here and use that for your following tests.
> > I don't know, because I'm not the author of that code.
> > But I can change it to verifyFormat if this is what you prefer.
> > 
> The verifyFormat() call with one code parameter does not work with this test 
> pattern, because the internal messUpCode function removes the newlines.
> The consecutive alignments are interrupted by newlines, which lead to 
> different indent for each section. This breaks with messUpCode.
> 
> However, there is a verifyFormat function with two code arguments, wich 
> behaves similar to the existing EXPECT_EQ. This way it works.
> I go for this option now.
Unfortunately, verifyFormat does not work with this test pattern.
As you can see in line 80 below there is another mussUp(Code) call for the 
Objective-C++ checks.
This again screws up the newlines and it does not help to use the verifyFormat 
overload with two code arguments.

I think this is a general limitation. Clang-format distinguishes between 
ACS_Consecutive and ACS_AcrossEmptyLines.
Using verifyFormat you cannot use empty lines, so you cannot test if 
ACS_Consecutive does NOT align across empty lines.

IMO, it is better to keep the EXPECT_EQ tests. The other solution would be to 
remove the empty lines from the test patterns.

```
   68   void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
   69  llvm::StringRef Code,
   70  const FormatStyle &Style = getLLVMStyle()) {
   71 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
   72 EXPECT_EQ(Expected.str(), format(Expected, Style))
   73 << "Expected code is not stable";
   74 EXPECT_EQ(Expected.str(), format(Code, Style));
   75 if (Style.Language == FormatStyle::LK_Cpp) {
   76   // Objective-C++ is a superset of C++, so everything checked for C++
   77   // needs to be checked for Objective-C++ as well.
   78   FormatStyle ObjCStyle = Style;
   79   ObjCStyle.Language = FormatStyle::LK_ObjC;
   80   EXPECT_EQ(Expected.str(), format(test::messUp(Code), ObjCStyle));
   81 }
   82   }
```



Comment at: clang/unittests/Format/FormatTest.cpp:15068
Alignment));
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"

curdeius wrote:
> Is there a reason why we don't use `verifyFormat` in these tests?
same answer as above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap updated this revision to Diff 348625.
gergap added a comment.

fix review findings

- fix case of comments
- rename for loop variable name
- use preincrement instead of postincrement


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14881,6 +14881,7 @@
   FormatStyle Alignment = getLLVMStyle();
   Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  Alignment.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
Alignment);
@@ -14916,8 +14917,8 @@
   verifyFormat("int  oneTwoThree{0}; // comment\n"
"unsigned oneTwo; // comment",
Alignment);
-  verifyFormat("unsigned int *  a;\n"
-   "int *   b;\n"
+  verifyFormat("unsigned int   *a;\n"
+   "int*b;\n"
"unsigned int Const *c;\n"
"unsigned int const *d;\n"
"unsigned int Const &e;\n"
@@ -15039,9 +15040,11 @@
"  doublebar();\n"
"};\n",
Alignment);
+
+  // PAS_Right
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"
-"  int * j   = 2;\n"
+"  int  *j   = 2;\n"
 "  int   big = 1;\n"
 "\n"
 "  unsigned oneTwoThree = 123;\n"
@@ -15062,6 +15065,140 @@
"int ll=1;\n"
"}",
Alignment));
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int **j   = 2, ***k;\n"
+"  int  &k   = i;\n"
+"  int &&l   = i + j;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int **j=2,***k;\n"
+   "int &k=i;\n"
+   "int &&l=i+j;\n"
+   " int big  =  1;\n"
+   "\n"
+   "unsigned oneTwoThree  =123;\n"
+   "int oneTwo = 12;\n"
+   "  method();\n"
+   "float k= 2;\n"
+   "int ll=1;\n"
+   "}",
+   Alignment));
+
+  // PAS_Left
+  FormatStyle AlignmentLeft = Alignment;
+  AlignmentLeft.PointerAlignment = FormatStyle::PAS_Left;
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int*  j   = 2;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int *j=2;\n"
+   " int big  =  1;\n"
+   "\n"
+   "unsigned oneTwoThree  =123;\n"
+   "int oneTwo = 12;\n"
+   "  method();\n"
+   "float k= 2;\n"
+   "int ll=1;\n"
+   "}",
+   AlignmentLeft));
+  EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+"  int const i   = 1;\n"
+"  int** j   = 2;\n"
+"  int&  k   = i;\n"
+"  int&& l   = i + j;\n"
+"  int   big = 1;\n"
+"\n"
+"  unsigned oneTwoThree = 123;\n"
+"  int  oneTwo  = 12;\n"
+"  method();\n"
+"  float k  = 2;\n"
+"  int   ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+   " int const  i= 1;\n"
+   "  int **j=2;\n"
+   "int &k=i;\n"
+   "int &&l=i+j;\n"
+   " int big  =  1;\n"
+   "\n"
+   "unsigned oneTwoThree  =123;\n"
+   "int oneTwo = 12;\n"
+   "  method();\n"
+   "float k= 2;\n"
+   "int ll=1;\n"
+ 

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. But please wait for @hazardyknusperkeks to land.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103319: [analyzer] Use Optional as a return type of StoreManager::castRegion

[Denys Petrov via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfae3534b3056: [analyzer]  Use Optional as a return type of 
StoreManager::castRegion (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103319/new/

https://reviews.llvm.org/D103319

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/casts.c

Index: clang/test/Analysis/casts.c
===
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -251,18 +251,9 @@
 ;
 }
 
-// See PR50179.
-// Just don't crash.
-typedef struct taskS {
-  void *pJob;
-} taskS;
-
-typedef struct workS {
-  taskS *pTaskList;
-} workS;
-
-void *getTaskJob(unsigned jobId, workS *pWork, unsigned taskId) {
-  const taskS *pTask = pWork->pTaskList + taskId;
-  taskS task = *pTask;
-  return task.pJob;
+// PR50179.
+struct S {};
+void symbolic_offset(struct S *ptr, int i) {
+  const struct S *pS = ptr + i;
+  struct S s = *pS; // no-crash
 }
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -71,7 +71,8 @@
   return MRMgr.getElementRegion(T, idx, R, Ctx);
 }
 
-const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) {
+Optional StoreManager::castRegion(const MemRegion *R,
+ QualType CastToTy) {
   ASTContext &Ctx = StateMgr.getContext();
 
   // Handle casts to Objective-C objects.
@@ -88,7 +89,7 @@
 
 // We don't know what to make of it.  Return a NULL region, which
 // will be interpreted as UnknownVal.
-return nullptr;
+return None;
   }
 
   // Now assume we are casting from pointer to pointer. Other cases should
@@ -168,7 +169,7 @@
   // If we cannot compute a raw offset, throw up our hands and return
   // a NULL MemRegion*.
   if (!baseR)
-return nullptr;
+return None;
 
   CharUnits off = rawOff.getOffset();
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -753,16 +753,16 @@
 if (const auto *SR = dyn_cast(R)) {
   QualType SRTy = SR->getSymbol()->getType();
   if (!hasSameUnqualifiedPointeeType(SRTy, CastTy)) {
-R = StateMgr.getStoreManager().castRegion(SR, CastTy);
-return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(SR, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 }
   }
   // Next fixes pointer dereference using type different from its initial
   // one. See PR37503 and PR49007 for details.
   if (const auto *ER = dyn_cast(R)) {
-if ((R = StateMgr.getStoreManager().castRegion(ER, CastTy)))
-  return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(ER, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 
   return V;
@@ -807,8 +807,8 @@
 
 // Get the result of casting a region to a different type.
 const MemRegion *R = V.getRegion();
-if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-  return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 
   // Pointer to whatever else.
@@ -873,8 +873,8 @@
   if (!IsUnknownOriginalType && Loc::isLocType(CastTy) &&
   OriginalTy->isIntegralOrEnumerationType()) {
 if (const MemRegion *R = L.getAsRegion())
-  if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-return loc::MemRegionVal(R);
+  if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+return loc::MemRegionVal(*OptR);
 return L;
   }
 
@@ -890,8 +890,8 @@
   // Delegate to store manager to get the result of casting a region to a
   // different type. If the MemRegion* returned is NULL, this expression
   // Evaluates to UnknownVal.
-  if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-return loc::MemRegionVal(R);
+  if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+return loc::MemRegionVal(*OptR);
 }
   } else {
 if (Loc::isLocType(CastTy)) {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -181,7 +181,8 

[clang] fae3534 - [analyzer] Use Optional as a return type of StoreManager::castRegion

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-05-29T15:16:56+03:00
New Revision: fae3534b3056bb96d26a6d1b6e7d6a2ccaf4fab1

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

LOG: [analyzer]  Use Optional as a return type of StoreManager::castRegion

Summary: Make StoreManager::castRegion function usage safier. Replace `const 
MemRegion *` with `Optional`. Simplified one of related test 
cases due to suggestions in D101635.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/lib/StaticAnalyzer/Core/Store.cpp
clang/test/Analysis/casts.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index 947913ae4eee9..d2461705d1282 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -181,7 +181,8 @@ class StoreManager {
   /// castRegion - Used by ExprEngine::VisitCast to handle casts from
   ///  a MemRegion* to a specific location type.  'R' is the region being
   ///  casted and 'CastToTy' the result type of the cast.
-  const MemRegion *castRegion(const MemRegion *region, QualType CastToTy);
+  Optional castRegion(const MemRegion *region,
+ QualType CastToTy);
 
   virtual StoreRef removeDeadBindings(Store store, const StackFrameContext 
*LCtx,
   SymbolReaper &SymReaper) = 0;

diff  --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 0003c27513994..39787886cd7a5 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -753,16 +753,16 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, 
QualType CastTy,
 if (const auto *SR = dyn_cast(R)) {
   QualType SRTy = SR->getSymbol()->getType();
   if (!hasSameUnqualifiedPointeeType(SRTy, CastTy)) {
-R = StateMgr.getStoreManager().castRegion(SR, CastTy);
-return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(SR, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 }
   }
   // Next fixes pointer dereference using type 
diff erent from its initial
   // one. See PR37503 and PR49007 for details.
   if (const auto *ER = dyn_cast(R)) {
-if ((R = StateMgr.getStoreManager().castRegion(ER, CastTy)))
-  return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(ER, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 
   return V;
@@ -807,8 +807,8 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, 
QualType CastTy,
 
 // Get the result of casting a region to a 
diff erent type.
 const MemRegion *R = V.getRegion();
-if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-  return loc::MemRegionVal(R);
+if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+  return loc::MemRegionVal(*OptR);
   }
 
   // Pointer to whatever else.
@@ -873,8 +873,8 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, 
QualType CastTy,
   if (!IsUnknownOriginalType && Loc::isLocType(CastTy) &&
   OriginalTy->isIntegralOrEnumerationType()) {
 if (const MemRegion *R = L.getAsRegion())
-  if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-return loc::MemRegionVal(R);
+  if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+return loc::MemRegionVal(*OptR);
 return L;
   }
 
@@ -890,8 +890,8 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, 
QualType CastTy,
   // Delegate to store manager to get the result of casting a region to a
   // 
diff erent type. If the MemRegion* returned is NULL, this expression
   // Evaluates to UnknownVal.
-  if ((R = StateMgr.getStoreManager().castRegion(R, CastTy)))
-return loc::MemRegionVal(R);
+  if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy))
+return loc::MemRegionVal(*OptR);
 }
   } else {
 if (Loc::isLocType(CastTy)) {

diff  --git a/clang/lib/StaticAnalyzer/Core/Store.cpp 
b/clang/lib/StaticAnalyzer/Core/Store.cpp
index c563b44efc13e..b867b0746f90f 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -71,7 +71,8 @@ const ElementRegion *StoreManager::GetElementZeroRegion(const 
SubRegion *R,
   return MRMgr.getElementRegion(T, idx, R, Ctx);
 }
 
-const MemRegion *StoreManager::castRegion(const MemR

[clang] ffb48d4 - [clang-format] successive C# attributes cause line breaking issues

[via cfe-commits]

Author: mydeveloperday
Date: 2021-05-29T16:43:55+01:00
New Revision: ffb48d48e45c72ed81dda4983ccb06e800cdbbd0

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

LOG: [clang-format] successive C# attributes cause line breaking issues

{D74265} reduced the aggressiveness of line breaking following C# attributes, 
however this change removed any support for attributes on properties, causing 
significant ugliness to be introduced.

This revision goes some way to addressing that by re-introducing the more 
aggressive check to `mustBreakBefore()`, but constraining it to the most common 
cases where we use properties which should not impact the "caller info 
attributes"  or the "[In , Out]" decorations that are normally put on pinvoke

It does not address my additional concerns of the original change regarding 
multiple C# attributes, as these are somewhat incorrectly handled by virtue of 
the fact its not recognising the second attribute as an attribute at all. But 
instead thinking its an array.

The purpose of this revision is to get back to where we were for the most 
common of cases as a stepping stone to resolving this. However {D74265} has 
broken a lot of C# code and this revision will go someway alone to addressing 
the majority.

Reviewed By: jbcoe, HazardyKnusperkeks, curdeius

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestCSharp.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index daa624000ff6d..4bd9311ebadd4 100755
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3520,6 +3520,17 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   return false;
 if (Right.is(TT_CSharpGenericTypeConstraint))
   return true;
+
+// Break after C# [...] and before public/protected/private/internal.
+if (Left.is(TT_AttributeSquare) && Left.is(tok::r_square) &&
+(Right.isAccessSpecifier(/*ColonRequired=*/false) ||
+ Right.is(Keywords.kw_internal)))
+  return true;
+// Break between ] and [ but only when there are really 2 attributes.
+if (Left.is(TT_AttributeSquare) && Right.is(TT_AttributeSquare) &&
+Left.is(tok::r_square) && Right.is(tok::l_square))
+  return true;
+
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 // FIXME: This might apply to other languages and token kinds.
 if (Right.is(tok::string_literal) && Left.is(tok::plus) && Left.Previous &&

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp 
b/clang/unittests/Format/FormatTestCSharp.cpp
index 427f7257eeb9a..651b54cd342a7 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -247,6 +247,37 @@ TEST_F(FormatTestCSharp, Attributes) {
   verifyFormat("[TestMethod]\n"
"public string Host { set; get; }");
 
+  // Adjacent properties should not cause line wrapping issues
+  verifyFormat("[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "protected string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "internal string Bar { set; get; }");
+
+  // Multiple attributes should always be split (not just the first ones)
+  verifyFormat("[XmlIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[XmlIgnore]\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+   "[ScriptIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[XmlIgnore]\n"
+   "[ScriptIgnore]\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }");
+
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
"listening on provided host\")]\n"
"public string Host { set; get; }");
@@ -271,6 +302,34 @@ TEST_F(FormatTestCSharp, Attributes) {
"{\n"
"}");
 
+  verifyFormat("void MethodA([In, Out] ref double x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("void MethodA([In, Out] double[] x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("void MethodA([In] double[] x)\n"
+

[PATCH] D103307: [clang-format] successive C# attributes cause line breaking issues

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGffb48d48e45c: [clang-format] successive C# attributes cause 
line breaking issues (authored by MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D103307?vs=348495&id=348634#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103307/new/

https://reviews.llvm.org/D103307

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -247,6 +247,37 @@
   verifyFormat("[TestMethod]\n"
"public string Host { set; get; }");
 
+  // Adjacent properties should not cause line wrapping issues
+  verifyFormat("[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "protected string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "internal string Bar { set; get; }");
+
+  // Multiple attributes should always be split (not just the first ones)
+  verifyFormat("[XmlIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[XmlIgnore]\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }");
+
+  verifyFormat("[XmlIgnore]\n"
+   "[ScriptIgnore]\n"
+   "[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[XmlIgnore]\n"
+   "[ScriptIgnore]\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }");
+
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
"listening on provided host\")]\n"
"public string Host { set; get; }");
@@ -271,6 +302,34 @@
"{\n"
"}");
 
+  verifyFormat("void MethodA([In, Out] ref double x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("void MethodA([In, Out] double[] x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("void MethodA([In] double[] x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("void MethodA(int[] x)\n"
+   "{\n"
+   "}");
+  verifyFormat("void MethodA(int[][] x)\n"
+   "{\n"
+   "}");
+  verifyFormat("void MethodA([] x)\n"
+   "{\n"
+   "}");
+
+  verifyFormat("public void Log([CallerLineNumber] int line = -1, "
+   "[CallerFilePath] string path = null,\n"
+   "[CallerMemberName] string name = null)\n"
+   "{\n"
+   "}");
+
   // [] in an attribute do not cause premature line wrapping or indenting.
   verifyFormat(R"(//
 public class A
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3520,6 +3520,17 @@
   return false;
 if (Right.is(TT_CSharpGenericTypeConstraint))
   return true;
+
+// Break after C# [...] and before public/protected/private/internal.
+if (Left.is(TT_AttributeSquare) && Left.is(tok::r_square) &&
+(Right.isAccessSpecifier(/*ColonRequired=*/false) ||
+ Right.is(Keywords.kw_internal)))
+  return true;
+// Break between ] and [ but only when there are really 2 attributes.
+if (Left.is(TT_AttributeSquare) && Right.is(TT_AttributeSquare) &&
+Left.is(tok::r_square) && Right.is(tok::l_square))
+  return true;
+
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 // FIXME: This might apply to other languages and token kinds.
 if (Right.is(tok::string_literal) && Left.is(tok::plus) && Left.Previous &&


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -247,6 +247,37 @@
   verifyFormat("[TestMethod]\n"
"public string Host { set; get; }");
 
+  // Adjacent properties should not cause line wrapping issues
+  verifyFormat("[JsonProperty(\"foo\")]\n"
+   "public string Foo { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "public string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+   "protected string Bar { set; get; }\n"
+   "[JsonProperty(\"bar\")]\n"
+ 

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please just check `continue`, I would like to make it a separate commit, 
because it seems unrelated to me. Otherwise this is good.




Comment at: clang/lib/Format/WhitespaceManager.cpp:369
 assert(Shift >= 0);
+if (Shift == 0)
+  continue;

gergap wrote:
> HazardyKnusperkeks wrote:
> > This is unrelated, isn't it?
> > 
> > If it is, I would like a separate commit. Otherwise an explanation why it 
> > is now mandatory and does not infer with the other alignments.
> I actually don't know. Just copied this from the original patch.
Could you please check if it works without this?



Comment at: clang/unittests/Format/FormatTest.cpp:15045
+  // PAS_RIGHT
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
 "  int const i   = 1;\n"

gergap wrote:
> gergap wrote:
> > gergap wrote:
> > > HazardyKnusperkeks wrote:
> > > > I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I 
> > > > know someone who will request that. :)
> > > > 
> > > > You should change that here and use that for your following tests.
> > > I don't know, because I'm not the author of that code.
> > > But I can change it to verifyFormat if this is what you prefer.
> > > 
> > The verifyFormat() call with one code parameter does not work with this 
> > test pattern, because the internal messUpCode function removes the newlines.
> > The consecutive alignments are interrupted by newlines, which lead to 
> > different indent for each section. This breaks with messUpCode.
> > 
> > However, there is a verifyFormat function with two code arguments, wich 
> > behaves similar to the existing EXPECT_EQ. This way it works.
> > I go for this option now.
> Unfortunately, verifyFormat does not work with this test pattern.
> As you can see in line 80 below there is another mussUp(Code) call for the 
> Objective-C++ checks.
> This again screws up the newlines and it does not help to use the 
> verifyFormat overload with two code arguments.
> 
> I think this is a general limitation. Clang-format distinguishes between 
> ACS_Consecutive and ACS_AcrossEmptyLines.
> Using verifyFormat you cannot use empty lines, so you cannot test if 
> ACS_Consecutive does NOT align across empty lines.
> 
> IMO, it is better to keep the EXPECT_EQ tests. The other solution would be to 
> remove the empty lines from the test patterns.
> 
> ```
>68   void _verifyFormat(const char *File, int Line, llvm::StringRef 
> Expected,
>69  llvm::StringRef Code,
>70  const FormatStyle &Style = getLLVMStyle()) {
>71 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
>72 EXPECT_EQ(Expected.str(), format(Expected, Style))
>73 << "Expected code is not stable";
>74 EXPECT_EQ(Expected.str(), format(Code, Style));
>75 if (Style.Language == FormatStyle::LK_Cpp) {
>76   // Objective-C++ is a superset of C++, so everything checked for 
> C++
>77   // needs to be checked for Objective-C++ as well.
>78   FormatStyle ObjCStyle = Style;
>79   ObjCStyle.Language = FormatStyle::LK_ObjC;
>80   EXPECT_EQ(Expected.str(), format(test::messUp(Code), ObjCStyle));
>81 }
>82   }
> ```
A, now I see! The empty line between the blocks. Yeah, no verifyFormat for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103245/new/

https://reviews.llvm.org/D103245

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

[PATCH] D103286: [clang-format] Add PPIndentWidth option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.
This revision now requires changes to proceed.

Love it!

But this will result in unexpected (one might say breaking) behaviour, if 
someone set `IndentWidth` to a different value than his base style and update 
clang-format without adding a setting for `PPIndentWidth`. So in my opinion it 
should have a different default value, which always picks up `IndentWidth` 
until `PPIndentWidth` is explicitly set. Either some form of optional, or for 
me `-1` is also fine, but I know others are more reluctant to use `-1`.




Comment at: clang/unittests/Format/FormatTest.cpp:3438
+  style.PPIndentWidth = 1;
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+

How about the other `IndentPPDirectives` values?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103286/new/

https://reviews.llvm.org/D103286

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

[PATCH] D103286: [clang-format] Add PPIndentWidth option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:2336
+  /// \endcode
+  unsigned PPIndentWidth;
+

I prefer alphabetical sorting, I know there are some entries which aren't 
sorted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103286/new/

https://reviews.llvm.org/D103286

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

[PATCH] D103319: [analyzer] Use Optional as a return type of StoreManager::castRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ

> I guess another option is to put `loc::MemRegionVal()` inside `castRegion()`. 
> This way the return type `Optional` unambigously tells 
> that the region is always non-null if present (protected by the assertion in 
> the constructor of `loc::MemRegionVal`).

IMO we should keep `const MemRegion *` for `castRegion` as is, since it's 
intended for //cast// and should behave as //cast//. I'd better add a kinda 
wrapper`SValBuilder::getCastedMemRegionVal`. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103319/new/

https://reviews.llvm.org/D103319

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

[PATCH] D103358: [analyzer] MallocSizeof: sizeof pointer type is compatible with void*

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Amazing, perfect, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103358/new/

https://reviews.llvm.org/D103358

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

[clang] 620cef9 - [analyzer] MallocSizeof: sizeof pointer type is compatible with void*

[Xuanda Yang via cfe-commits]

Author: Xuanda Yang
Date: 2021-05-30T09:51:41+08:00
New Revision: 620cef91207bbeb570a529328976040e658a60ee

URL: 
https://github.com/llvm/llvm-project/commit/620cef91207bbeb570a529328976040e658a60ee
DIFF: 
https://github.com/llvm/llvm-project/commit/620cef91207bbeb570a529328976040e658a60ee.diff

LOG: [analyzer] MallocSizeof: sizeof pointer type is compatible with void*

source: https://bugs.llvm.org/show_bug.cgi?id=50214

Make sizeof pointer type compatible with void* in MallocSizeofChecker.

Reviewed By: NoQ

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
clang/test/Analysis/malloc-sizeof.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
index 71f593cb2b561..4b5206a102b87 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -139,6 +139,10 @@ static bool typesCompatible(ASTContext &C, QualType A, 
QualType B) {
   if (B->isVoidPointerType() && A->getAs())
 return true;
 
+  // sizeof(pointer type) is compatible with void*
+  if (A->isVoidPointerType() && B->getAs())
+return true;
+
   while (true) {
 A = A.getCanonicalType();
 B = B.getCanonicalType();

diff  --git a/clang/test/Analysis/malloc-sizeof.c 
b/clang/test/Analysis/malloc-sizeof.c
index ee104245b819a..22c4045b7bbd3 100644
--- a/clang/test/Analysis/malloc-sizeof.c
+++ b/clang/test/Analysis/malloc-sizeof.c
@@ -26,6 +26,8 @@ void foo(unsigned int unsignedInt, unsigned int readSize) {
   struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 
'calloc' is converted to a pointer of type 'struct A', which is incompatible 
with sizeof operand type 'struct B'}}
   struct A *ap6 = realloc(ap5, sizeof(struct A));
   struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result 
of 'realloc' is converted to a pointer of type 'struct A', which is 
incompatible with sizeof operand type 'struct B'}}
+
+  void **vpp1 = (void **)malloc(sizeof(struct A*)); // no warning
 }
 
 // Don't warn when the types 
diff er only by constness.



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

[PATCH] D103358: [analyzer] MallocSizeof: sizeof pointer type is compatible with void*

[Xuanda Yang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG620cef91207b: [analyzer] MallocSizeof: sizeof pointer type 
is compatible with void* (authored by TH3CHARLie).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103358/new/

https://reviews.llvm.org/D103358

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  clang/test/Analysis/malloc-sizeof.c


Index: clang/test/Analysis/malloc-sizeof.c
===
--- clang/test/Analysis/malloc-sizeof.c
+++ clang/test/Analysis/malloc-sizeof.c
@@ -26,6 +26,8 @@
   struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 
'calloc' is converted to a pointer of type 'struct A', which is incompatible 
with sizeof operand type 'struct B'}}
   struct A *ap6 = realloc(ap5, sizeof(struct A));
   struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result 
of 'realloc' is converted to a pointer of type 'struct A', which is 
incompatible with sizeof operand type 'struct B'}}
+
+  void **vpp1 = (void **)malloc(sizeof(struct A*)); // no warning
 }
 
 // Don't warn when the types differ only by constness.
Index: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -139,6 +139,10 @@
   if (B->isVoidPointerType() && A->getAs())
 return true;
 
+  // sizeof(pointer type) is compatible with void*
+  if (A->isVoidPointerType() && B->getAs())
+return true;
+
   while (true) {
 A = A.getCanonicalType();
 B = B.getCanonicalType();


Index: clang/test/Analysis/malloc-sizeof.c
===
--- clang/test/Analysis/malloc-sizeof.c
+++ clang/test/Analysis/malloc-sizeof.c
@@ -26,6 +26,8 @@
   struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 'calloc' is converted to a pointer of type 'struct A', which is incompatible with sizeof operand type 'struct B'}}
   struct A *ap6 = realloc(ap5, sizeof(struct A));
   struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result of 'realloc' is converted to a pointer of type 'struct A', which is incompatible with sizeof operand type 'struct B'}}
+
+  void **vpp1 = (void **)malloc(sizeof(struct A*)); // no warning
 }
 
 // Don't warn when the types differ only by constness.
Index: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -139,6 +139,10 @@
   if (B->isVoidPointerType() && A->getAs())
 return true;
 
+  // sizeof(pointer type) is compatible with void*
+  if (A->isVoidPointerType() && B->getAs())
+return true;
+
   while (true) {
 A = A.getCanonicalType();
 B = B.getCanonicalType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

This change is causing failures when the build is done from a working directory 
that has symlinks:
https://lab.llvm.org/staging/#/builders/126/builds/529/steps/5/logs/FAIL__Clang__hip-include-path_hip


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102723/new/

https://reviews.llvm.org/D102723

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