date:20200907

[clang] 7907e55 - [Sema] fix /gr warning test case

2020-09-07 Thread Zequan Wu via cfe-commits


Author: Zequan Wu
Date: 2020-09-07T20:55:11-07:00
New Revision: 7907e5516a418fec29137beed3ff985f40e04f17

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

LOG: [Sema] fix /gr warning test case

Added: 


Modified: 
clang/test/SemaCXX/no-rtti.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/no-rtti.cpp b/clang/test/SemaCXX/no-rtti.cpp
index e0b57153c24c..f8487a0902dd 100644
--- a/clang/test/SemaCXX/no-rtti.cpp
+++ b/clang/test/SemaCXX/no-rtti.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fno-rtti %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify -fno-rtti 
%s
 
 namespace std {
   class type_info;

diff  --git a/clang/test/SemaCXX/no_dynamic_cast.cpp 
b/clang/test/SemaCXX/no_dynamic_cast.cpp
index 4db21d36f4a9..074b02f4668b 100644
--- a/clang/test/SemaCXX/no_dynamic_cast.cpp
+++ b/clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+// RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -fno-rtti-data -fsyntax-only 
-verify
 
 namespace std {
 struct type_info {};



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

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-09-07 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies added a comment.

In D66564#2257635 , @aaron.ballman 
wrote:

> In D66564#2256482 , @ffrankies wrote:
>
>> In D66564#2256424 , @Eugene.Zelenko 
>> wrote:
>>
>>> In D66564#2256423 , @ffrankies 
>>> wrote:
>>>
 @Eugene.Zelenko I don't have commit access to the repository, could you 
 please commit this check on our behalf?
>>>
>>> Sorry, I don't have it either. @aaron.ballman or @njames93 could do this 
>>> for you.
>>
>> No problem. @aaron.ballman @njames93 Could one of you please commit this 
>> check on our behalf?
>
> Sorry for the hassle, but could you rebase again? I'm getting errors when I 
> try to apply the patch.

Done. After rebasing, fixing the include statement in `StructPackAlignCheck.h` 
got rid of the errors that I saw.


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

https://reviews.llvm.org/D66564

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

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-09-07 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies updated this revision to Diff 290396.
ffrankies added a comment.

Rebased, changed import in `StructPackAlignCheck.h` from `../ClangTidy.h` to 
`../ClangTidyCheck.h`


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

https://reviews.llvm.org/D66564

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-struct-pack-align.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s altera-struct-pack-align %t -- -header-filter=.*
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error' is inefficient due to padding; only needs 10 bytes but is using 24 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((packed))" to reduce the amount of padding applied to struct 'error'
+// CHECK-MESSAGES: :[[@LINE-7]]:8: warning: accessing fields in struct 'error' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-8]]:8: note: use "__attribute__((aligned(16)))" to align struct 'error' to 16 bytes
+// CHECK-FIXES: __attribute__((packed))
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error_packed' is inefficient due to poor alignment; currently aligned to 1 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'error_packed' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)))
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: accessing fields in struct 'align_only' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-9]]:8: note: use "__attribute__((aligned(16)))" to align struct 'align_only' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align2' is inefficient due to poor alignment; currently aligned to 32 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align2' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align3' is inefficient due to poor alignment; currently aligned to 4 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((aligned(16)))" to align struct 'bad_align3' to 16 bytes
+// CHECK-FIXES: __attribute__((aligned(16)));
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute__((packed)) __attribute__((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+  int a;
+  int b;
+  int c;
+}

[PATCH] D82725: [PowerPC] Implement Move to VSR Mask builtins in LLVM/Clang

2020-09-07 Thread Qing Shan Zhang via Phabricator via cfe-commits

steven.zhang added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10054
+
+  case Intrinsic::ppc_altivec_mtvsrbm: {
+// The llvm.ppc.altivec.mtvsrbm intrinsic can correspond to two different

Can we handle this inside the .td ? i.e. change the definition of the instr as:
```
  def MTVSRBMI : DXForm<4, 10, (outs vrrc:$vD), (ins u8imm64:$D),
"mtvsrbmi $vD, $D", IIC_VecGeneral,
[(set v16i8:$vD,
  (int_ppc_altivec_mtvsrbm imm:$D))]>;
```
And add the missing u8imm64 as what we did for u16imm64 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82725

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

[PATCH] D87218: [builtins] Inline paritysi2 into paritydi2 and inline paritydi2 into parityti2.

2020-09-07 Thread Craig Topper via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35f708a3c9ff: [builtins] Inline __paritysi2 into __paritydi2 
and inline __paritydi2 into… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87218

Files:
  compiler-rt/lib/builtins/paritydi2.c
  compiler-rt/lib/builtins/parityti2.c


Index: compiler-rt/lib/builtins/parityti2.c
===
--- compiler-rt/lib/builtins/parityti2.c
+++ compiler-rt/lib/builtins/parityti2.c
@@ -18,8 +18,14 @@
 
 COMPILER_RT_ABI int __parityti2(ti_int a) {
   twords x;
+  dwords x2;
   x.all = a;
-  return __paritydi2(x.s.high ^ x.s.low);
+  x2.all = x.s.high ^ x.s.low;
+  su_int x3 = x2.s.high ^ x2.s.low;
+  x3 ^= x3 >> 16;
+  x3 ^= x3 >> 8;
+  x3 ^= x3 >> 4;
+  return (0x6996 >> (x3 & 0xF)) & 1;
 }
 
 #endif // CRT_HAS_128BIT
Index: compiler-rt/lib/builtins/paritydi2.c
===
--- compiler-rt/lib/builtins/paritydi2.c
+++ compiler-rt/lib/builtins/paritydi2.c
@@ -17,5 +17,9 @@
 COMPILER_RT_ABI int __paritydi2(di_int a) {
   dwords x;
   x.all = a;
-  return __paritysi2(x.s.high ^ x.s.low);
+  su_int x2 = x.s.high ^ x.s.low;
+  x2 ^= x2 >> 16;
+  x2 ^= x2 >> 8;
+  x2 ^= x2 >> 4;
+  return (0x6996 >> (x2 & 0xF)) & 1;
 }


Index: compiler-rt/lib/builtins/parityti2.c
===
--- compiler-rt/lib/builtins/parityti2.c
+++ compiler-rt/lib/builtins/parityti2.c
@@ -18,8 +18,14 @@
 
 COMPILER_RT_ABI int __parityti2(ti_int a) {
   twords x;
+  dwords x2;
   x.all = a;
-  return __paritydi2(x.s.high ^ x.s.low);
+  x2.all = x.s.high ^ x.s.low;
+  su_int x3 = x2.s.high ^ x2.s.low;
+  x3 ^= x3 >> 16;
+  x3 ^= x3 >> 8;
+  x3 ^= x3 >> 4;
+  return (0x6996 >> (x3 & 0xF)) & 1;
 }
 
 #endif // CRT_HAS_128BIT
Index: compiler-rt/lib/builtins/paritydi2.c
===
--- compiler-rt/lib/builtins/paritydi2.c
+++ compiler-rt/lib/builtins/paritydi2.c
@@ -17,5 +17,9 @@
 COMPILER_RT_ABI int __paritydi2(di_int a) {
   dwords x;
   x.all = a;
-  return __paritysi2(x.s.high ^ x.s.low);
+  su_int x2 = x.s.high ^ x.s.low;
+  x2 ^= x2 >> 16;
+  x2 ^= x2 >> 8;
+  x2 ^= x2 >> 4;
+  return (0x6996 >> (x2 & 0xF)) & 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits

bc-lee updated this revision to Diff 290381.
bc-lee added a comment.

Fix the example to match the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is separated by a newline. Static imports will also follow the
-  /// same grouping convention above all non-static imports. One group's prefix
-  /// can be a subset of another - the longest prefix is always matched. Within
-  /// a group, the imports are ordered lexicographically.
+  /// One group's prefix can be a subset of another - the longest prefix is
+  /// always matched. Within a group, the imports

[PATCH] D87257: [clang] Traverse init-captures while indexing

2020-09-07 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 290379.
nridge added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87257

Files:
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/Index/IndexBody.cpp


Index: clang/lib/Index/IndexBody.cpp
===
--- clang/lib/Index/IndexBody.cpp
+++ clang/lib/Index/IndexBody.cpp
@@ -391,11 +391,13 @@
 if (C->capturesThis() || C->capturesVLAType())
   return true;
 
+if (!base::TraverseStmt(Init))
+  return false;
+
 if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
   return IndexCtx.handleReference(C->getCapturedVar(), C->getLocation(),
   Parent, ParentDC, SymbolRoleSet());
 
-// FIXME: Lambda init-captures.
 return true;
   }
 
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1589,6 +1589,12 @@
 [[Vector]] x2;
 Vector y;
   )cpp",
+  R"cpp(// Lambda capture initializer
+void foo() {
+  int [[w^aldo]] = 42;
+  auto lambda = [x = [[waldo]]](){};
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);


Index: clang/lib/Index/IndexBody.cpp
===
--- clang/lib/Index/IndexBody.cpp
+++ clang/lib/Index/IndexBody.cpp
@@ -391,11 +391,13 @@
 if (C->capturesThis() || C->capturesVLAType())
   return true;
 
+if (!base::TraverseStmt(Init))
+  return false;
+
 if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
   return IndexCtx.handleReference(C->getCapturedVar(), C->getLocation(),
   Parent, ParentDC, SymbolRoleSet());
 
-// FIXME: Lambda init-captures.
 return true;
   }
 
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1589,6 +1589,12 @@
 [[Vector]] x2;
 Vector y;
   )cpp",
+  R"cpp(// Lambda capture initializer
+void foo() {
+  int [[w^aldo]] = 42;
+  auto lambda = [x = [[waldo]]](){};
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87257: [clang] Traverse init-captures while indexing

2020-09-07 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a reviewer: hokein.
nridge added a comment.

I did try to add a test to `clang/test/Index/Core/index-source.cpp`, however 
the output of `c-index-test` does not seem to be changed by adding this 
reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87257

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

[PATCH] D87257: [clang] Traverse init-captures while indexing

2020-09-07 Thread Nathan Ridge via Phabricator via cfe-commits

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87257

Files:
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/Index/IndexBody.cpp


Index: clang/lib/Index/IndexBody.cpp
===
--- clang/lib/Index/IndexBody.cpp
+++ clang/lib/Index/IndexBody.cpp
@@ -391,11 +391,13 @@
 if (C->capturesThis() || C->capturesVLAType())
   return true;
 
+// if (!base::TraverseStmt(Init))
+//  return false;
+
 if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
   return IndexCtx.handleReference(C->getCapturedVar(), C->getLocation(),
   Parent, ParentDC, SymbolRoleSet());
 
-// FIXME: Lambda init-captures.
 return true;
   }
 
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1589,6 +1589,12 @@
 [[Vector]] x2;
 Vector y;
   )cpp",
+  R"cpp(// Lambda capture initializer
+void foo() {
+  int [[w^aldo]] = 42;
+  auto lambda = [x = [[waldo]]](){};
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);


Index: clang/lib/Index/IndexBody.cpp
===
--- clang/lib/Index/IndexBody.cpp
+++ clang/lib/Index/IndexBody.cpp
@@ -391,11 +391,13 @@
 if (C->capturesThis() || C->capturesVLAType())
   return true;
 
+// if (!base::TraverseStmt(Init))
+//  return false;
+
 if (C->capturesVariable() && IndexCtx.shouldIndexFunctionLocalSymbols())
   return IndexCtx.handleReference(C->getCapturedVar(), C->getLocation(),
   Parent, ParentDC, SymbolRoleSet());
 
-// FIXME: Lambda init-captures.
 return true;
   }
 
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1589,6 +1589,12 @@
 [[Vector]] x2;
 Vector y;
   )cpp",
+  R"cpp(// Lambda capture initializer
+void foo() {
+  int [[w^aldo]] = 42;
+  auto lambda = [x = [[waldo]]](){};
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87218: [builtins] Inline paritysi2 into paritydi2 and inline paritydi2 into parityti2.

2020-09-07 Thread Eli Friedman via Phabricator via cfe-commits

efriedma accepted this revision.
efriedma added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87218

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

[PATCH] D87218: [builtins] Inline paritysi2 into paritydi2 and inline paritydi2 into parityti2.

2020-09-07 Thread Fangrui Song via Phabricator via cfe-commits

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

LG. Please wait a bit for @efriedma's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87218

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-07 Thread Zequan Wu 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 rG3e782bf8090c: [Sema][MSVC] warn at dynamic_cast when /GR- is 
given (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D86369?vs=289802=290374#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/ms_no_dynamic_cast.cpp
  clang/test/SemaCXX/no_dynamic_cast.cpp

Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B* b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+  void* v = dynamic_cast(b);
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B* b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+  void* v = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
+  // Warns when typeid is used with RTTI data disabled.
+  if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,18 @@
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI data disabled.
+  if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+  << isClangCL;
+  }
+
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7438,6 +7438,12 @@
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_rtti_disabled: Warning<
+  "dynamic_cast will not work since RTTI data is disabled by " 
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+  "typeid will not work since RTTI data is disabled by "
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1235,3 +1235,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list

[clang] 3e782bf - [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-07 Thread Zequan Wu via cfe-commits


Author: Zequan Wu
Date: 2020-09-07T16:46:58-07:00
New Revision: 3e782bf8090c80e6d75e62cd52c9ed32715cbcdd

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

LOG: [Sema][MSVC] warn at dynamic_cast when /GR- is given

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

Added: 
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 6b4dcc850612..a9bd52b8afcd 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1235,3 +1235,5 @@ in addition with the pragmas or -fmax-tokens flag to get 
any warnings.
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d856f784e0ee..e1601da74b73 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7438,6 +7438,12 @@ def err_no_typeid_with_fno_rtti : Error<
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_rtti_disabled: Warning<
+  "dynamic_cast will not work since RTTI data is disabled by " 
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+  "typeid will not work since RTTI data is disabled by "
+  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 726900c59f20..b213fb756a65 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,18 @@ void CastOperation::CheckDynamicCast() {
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI data disabled.
+  if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() 
==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+  << isClangCL;
+  }
+
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d1fcdf354527..8f8847e63804 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@ Sema::ActOnCXXTypeid(SourceLocation OpLoc, SourceLocation 
LParenLoc,
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
+  // Warns when typeid is used with RTTI data disabled.
+  if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {

diff  --git a/clang/test/SemaCXX/ms_no_dynamic_cast.cpp 
b/clang/test/SemaCXX/ms_no_dynamic_cast.cpp
new file mode 100644
index ..d2c007fd8c29
--- /dev/null
+++ b/clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows -fdiagnostics-format msvc 
-fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+  ~D1() = default;
+};
+
+void f() {
+  B* b = new D1();
+  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not 
work since RTTI data is disabled by /GR-}}
+  void* v = dynamic_cast(b); // expected-warning{{dynamic_cast will 
not work since RTTI data is disabled by /GR-}}
+  (void)typeid(int);  // expected-warning{{typeid will not work 
since RTTI data is disabled by /GR-}}
+}

diff  --git a/clang/test/SemaCXX/no_dynamic_cast.cpp 
b/clang/test/SemaCXX/no_dynamic_cast.cpp
new file mode 100644
index ..4db21d36f4a9
--- /dev/null
+++ b/clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+  virtual

[PATCH] D82726: [PowerPC] Implement Vector Count Mask Bits builtins in LLVM/Clang

2020-09-07 Thread Qing Shan Zhang via Phabricator via cfe-commits

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82726

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

[PATCH] D87218: [builtins] Inline paritysi2 into paritydi2 and inline paritydi2 into parityti2.

2020-09-07 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

In D87218#2259972 , @MaskRay wrote:

> I think this is correct, but does their performance matter? 
> `llvm/IR/RuntimeLibcalls.def` does not define them (they cannot be produced 
> by llvm). Targets either emit popcount & 1 or detect the idiom and emit an 
> optimized parity (x86 after PR46954)

Performance probably doesn't matter. Just one of the things a certain someone 
wasn't amused by.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87218

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

[PATCH] D87218: [builtins] Inline paritysi2 into paritydi2 and inline paritydi2 into parityti2.

2020-09-07 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I think this is correct, but does their performance matter? 
llvm/IR/RuntimeLibcalls.def do not define them (they cannot be produced by 
llvm). Targets either emit popcount & 1 or detect the idiom and emit an 
optimized parity (x86 after PR46954)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87218

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

[PATCH] D87256: [clangd] Avoid relations being overwritten in a header shard

2020-09-07 Thread Nathan Ridge via Phabricator via cfe-commits

nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Fixes https://github.com/clangd/clangd/issues/510


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87256

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -547,6 +547,8 @@
   Sym2.CanonicalDeclaration.FileURI = BHeaderUri.c_str();
   Sym2.Definition.FileURI = BSourceUri.c_str();
 
+  auto Sym3 = symbol("3"); // not stored
+
   IndexFileIn IF;
   {
 SymbolSlab::Builder B;
@@ -562,12 +564,12 @@
   }
   {
 RelationSlab::Builder B;
-// Should be stored in a.h
-B.insert(Relation{Sym1.ID, RelationKind::BaseOf, Sym2.ID});
 // Should be stored in b.h
+B.insert(Relation{Sym1.ID, RelationKind::BaseOf, Sym2.ID});
+// Should be stored in a.h
 B.insert(Relation{Sym2.ID, RelationKind::BaseOf, Sym1.ID});
-// Dangling relation should be dropped.
-B.insert(Relation{symbol("3").ID, RelationKind::BaseOf, Sym1.ID});
+// Should be stored in a.h even though it's dangling
+B.insert(Relation{Sym3.ID, RelationKind::BaseOf, Sym1.ID});
 IF.Relations.emplace(std::move(B).build());
   }
 
@@ -605,7 +607,8 @@
 EXPECT_THAT(*Shard->Refs, IsEmpty());
 EXPECT_THAT(
 *Shard->Relations,
-UnorderedElementsAre(Relation{Sym1.ID, RelationKind::BaseOf, Sym2.ID}));
+UnorderedElementsAre(Relation{Sym2.ID, RelationKind::BaseOf, Sym1.ID},
+ Relation{Sym3.ID, RelationKind::BaseOf, Sym1.ID}));
 ASSERT_THAT(Shard->Sources->keys(), UnorderedElementsAre(AHeaderUri));
 EXPECT_THAT(Shard->Sources->lookup(AHeaderUri).DirectIncludes, IsEmpty());
 EXPECT_TRUE(Shard->Cmd.hasValue());
@@ -617,7 +620,7 @@
 EXPECT_THAT(*Shard->Refs, IsEmpty());
 EXPECT_THAT(
 *Shard->Relations,
-UnorderedElementsAre(Relation{Sym2.ID, RelationKind::BaseOf, Sym1.ID}));
+UnorderedElementsAre(Relation{Sym1.ID, RelationKind::BaseOf, Sym2.ID}));
 ASSERT_THAT(Shard->Sources->keys(),
 UnorderedElementsAre(BHeaderUri, AHeaderUri));
 EXPECT_THAT(Shard->Sources->lookup(BHeaderUri).DirectIncludes,
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -229,6 +229,49 @@
FileURI("unittest:///root/B.cc")}));
 }
 
+TEST_F(BackgroundIndexTest, RelationsMultiFile) {
+  MockFS FS;
+  FS.Files[testPath("root/RAV.h")] = "template  class RAV {};";
+  FS.Files[testPath("root/A.cc")] = R"cpp(
+#include "RAV.h"
+class A : public RAV {};
+  )cpp";
+  FS.Files[testPath("root/B.cc")] = R"cpp(
+#include "RAV.h"
+class B : public RAV {};
+  )cpp";
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Index(FS, CDB, [&](llvm::StringRef) { return  },
+/*Opts=*/{});
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", Cmd.Filename};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+  ASSERT_TRUE(Index.blockUntilIdleForTest());
+
+  Cmd.Filename = testPath("root/B.cc");
+  Cmd.CommandLine = {"clang++", Cmd.Filename};
+  CDB.setCompileCommand(testPath("root/B.cc"), Cmd);
+  ASSERT_TRUE(Index.blockUntilIdleForTest());
+
+  auto HeaderShard = MSS.loadShard(testPath("root/RAV.h"));
+  EXPECT_NE(HeaderShard, nullptr);
+  SymbolID RAV = findSymbol(*HeaderShard->Symbols, "RAV").ID;
+
+  RelationsRequest Req;
+  Req.Subjects.insert(RAV);
+  Req.Predicate = RelationKind::BaseOf;
+  uint32_t Results = 0;
+  Index.relations(Req, [&](const SymbolID &, const Symbol &) { ++Results; });
+  EXPECT_EQ(Results, 2u);
+}
+
 TEST_F(BackgroundIndexTest, MainFileRefs) {
   MockFS FS;
   FS.Files[testPath("root/A.h")] = R"cpp(
@@ -346,13 +389,13 @@
 UnorderedElementsAre(FileURI("unittest:///root/A.cc")));
 
   // The BaseOf relationship between A_CC and B_CC is stored in the file
-  // containing the definition of the subject (A_CC)
+  // containing the definition of the object (B_CC)
   SymbolID A = findSymbol(*ShardHeader->Symbols, "A_CC").ID;
   SymbolID B = findSymbol(*ShardSource->Symbols, "B_CC").ID;
-

[PATCH] D84599: [Index/USRGeneration] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

2020-09-07 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

To make this patch more acceptable I could also add a `Visit` function for 
`DecompositionDecl` and `MSGuidDecl` such that the current behaviour is 
preserved (I won't be able to test it though since these implicit AST nodes are 
not visited).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84599

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

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-07 Thread Jake Merdich via Phabricator via cfe-commits

JakeMerdichAMD added inline comments.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

bc-lee wrote:
> JakeMerdichAMD wrote:
> > 3 things here:
> > 
> > 1. Did you mix up the true and false cases?
> > 2. (nit) You should also note that if this option is false, all static 
> > imports are before all non-static imports (as opposed to mixed together 
> > alphabetically). I was confused on first glance.
> > 3. Please add this config option to 
> > FormatTests.cpp:ParsesConfigurationBools.
> I understand. The description is somewhat misleading. 
New phrasing makes it clear, but true and false are still inverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

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

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

2020-09-07 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

In D86089#2256139 , 
@richard.barton.arm wrote:

> Another random thought that just came to me: what does the new driver do when 
> you invoke it with no input files or options? I could imagine a few sensible 
> outcomes (error: no input (clang and gcc/gfortran behaviour), print version 
> and exit, print help and exit, etc.) and I don't have a strong preference 
> over which we chose here but I think there should be a test for it in 
> test/Driver

`flang-new` inherits generic things like this from libclangDriver. In this case:

  error: no input files

This is consistent with `clang`. I've added a test for this. As for `flang-new 
-fc1` - it in general does very little right now :) `clang -cc1` just sits 
there waiting




Comment at: flang/test/CMakeLists.txt:8
 
+llvm_canonicalize_cmake_booleans(FLANG_BUILD_NEW_DRIVER)
+

richard.barton.arm wrote:
> So do the other bools like LINK_WITH_FIR also need this treatment and this is 
> a latent bug? Seems amazing we've not hit that before now.
> 
> From an offline conversation ISTR you saying this was to do with how the 
> variable is translated into the lit config? If that is the case then I think 
> there is a function you can use called lit.util.pythonize_bool which converts 
> various strings that mean "True/False" into a real bool for python. That 
> would also let you clean up the lit.cfg.py later on (separate comments).
AFAIK, `LINK_WITH_FIR` is not used in LIT tests.

I switched to `lit.util.pythonize_bool `, thanks for the suggestion!



Comment at: flang/test/Flang-Driver/driver-error-cc1.c:7
+
+// CHECK:error: unknown integrated tool '-cc1'. Valid tools include '-fc1'.

richard.barton.arm wrote:
> I am surprised that you don't need to prefix this run line with 'not' 
> indicating that flang-new returns 0 exit code in this instance, which seems 
> wrong to me.
Good catch, thanks! Fixed in the latest revision.



Comment at: flang/test/Flang-Driver/driver-error-cc1.cpp:1
+// RUN: %flang-new %s 2>&1 | FileCheck %s
+

richard.barton.arm wrote:
> Same comment as above on exit code.
Good catch, thanks! Fixed in the latest revision.



Comment at: flang/test/Flang-Driver/emit-obj.f90:2
+! RUN: %flang-new  %s 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: %flang-new  -fc1 -emit-obj %s 2>&1 | FileCheck %s --check-prefix=ERROR
+

richard.barton.arm wrote:
> Seems like it would be helpful to have also implemented `-###` in this patch 
> so that you don't need to write tests like this. You could simply run 
> flang-new -### then check the -fc1 line that would have been emitted for the 
> presence of -emit-obj.
> 
> Same comment above regarding exit code.
> Seems like it would be helpful to have also implemented -### in this patch

As`flang-new` is based on libclangDriver, we get `-###` for free (i.e. it's 
already supported).

However, there's a catch: 
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L369-L370.
 Currently `flang-new --help` will not display `-###`  because the the 
corresponding option definition hasn't been updated (i.e. it is not flagged as 
a Flang option):
```
def _HASH_HASH_HASH : Flag<["-"], "###">, Flags<[DriverOption, CoreOption]>,
HelpText<"Print (but do not run) the commands to run for this compilation">;
``` 
We have three options:
1. Make `flang-new --help` display options that are flagged as `DriverOption` 
or `CoreOption`. Note this will include many other options (currently 
unsupported) and extra filtering would be required.
2. Add `FlangOption` to the definition of `_HASH_HASH_HASH`, but IMO that's a 
bit controversial. Is that what we want to do long-term? Or shall we create a 
separate category for generic driver options that apply to both Clang and Flang?
3. Delay the decision until there is more code to base our design on.

I'm in favor of Option 3.



Comment at: flang/test/lit.cfg.py:39
 # directories.
+# exclude the tests for flang_new driver while there are two drivers
 config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']

richard.barton.arm wrote:
> This comment should be on line 41
Updated, ta!



Comment at: flang/test/lit.site.cfg.py.in:14
+# control the regression test for flang-new driver
+config.include_flang_new_driver_test="@FLANG_BUILD_NEW_DRIVER@"
+

richard.barton.arm wrote:
> awarzynski wrote:
> > `FLANG_BUILD_NEW_DRIVER` should be canonicalized in CMake first: 
> > https://github.com/llvm/llvm-project/blob/c79a366ec0f87eafca123138b550b039618bf880/llvm/cmake/modules/AddLLVM.cmake#L1424-L1435
> I think it might be better to use lit.util.pythonize_bool to do the 
> canonicalisation. It would let you use config.include_flang_new_driver_test 
> as a real bool later in the file rather

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

2020-09-07 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski updated this revision to Diff 290337.
awarzynski marked 2 inline comments as done.
awarzynski added a comment.

- Adddressed comments from @richard.barton.arm
- Added FC1Option to ClangFlags (and made other related changes)
- Added missing code to check return codes from subcommands (flang-new)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/unittests/Driver/SanitizerArgsTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/CMakeLists.txt
  flang/README.md
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/FrontendTool/Utils.h
  flang/lib/CMakeLists.txt
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/CMakeLists.txt
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/CMakeLists.txt
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/emit-obj.f90
  flang/test/Flang-Driver/missing-input.f90
  flang/test/lit.cfg.py
  flang/test/lit.site.cfg.py.in
  flang/tools/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/CMakeLists.txt
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/CompilerInstanceTest.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -610,8 +610,10 @@
   continue;
 
 unsigned Flags = getInfo(Id).Flags;
+
 if (FlagsToInclude && !(Flags & FlagsToInclude))
   continue;
+
 if (Flags & FlagsToExclude)
   continue;
 
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -48,7 +48,7 @@
 unsigned ID;
 unsigned char Kind;
 unsigned char Param;
-unsigned short Flags;
+unsigned int Flags;
 unsigned short GroupID;
 unsigned short AliasID;
 const char *AliasArgs;
@@ -225,7 +225,8 @@
   /// \param Usage - USAGE: Usage
   /// \param Title - OVERVIEW: Title
   /// \param FlagsToInclude - If non-zero, only include options with any
-  /// of these flags set.
+  /// of these flags set. Takes precedence over
+  /// FlagsToExclude.
   /// \param FlagsToExclude - Exclude options with any of these flags set.
   /// \param ShowAllAliases - If true, display all options including aliases
   /// that don't have help texts. By default, we display
Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -0,0 +1,52 @@
+//===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+using namespace llvm;
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  // 1. Set-up a basic DiagnosticConsumer
+  std::string diagnosticOutput;
+  llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  auto diagPrinter = std::make_unique(
+  diagnosticsOS, new clang::DiagnosticOptions());
+
+  // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
+  CompilerInstance compInst;
+
+  // 3. Set-up DiagnosticOptions
+  auto

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-07 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

> This change allow a CallExpr to have optional FPOptionsOverride object,

Should this be `CastExpr` instead?

> stored in trailing storage. The implementaion is made similar to the way
> used in CallExpr.

Nit, but that's not really similar. In the `CallExpr` case the trailing objects 
are directly managed by `CallExpr` without using `TrailingObjects` in the 
sub-classes.




Comment at: clang/include/clang/AST/Expr.h:3471
 
+  /// Return a pointer to the trailing FPOptions.
+  FPOptionsOverride *getTrailingFPFeatures();

Can you document that `hasStoredFPFeatures()` must  be true as a pre-condition?



Comment at: clang/include/clang/AST/Expr.h:3612
 
+  unsigned numTrailingObjects(OverloadToken) const {
+return path_size();

Here and elsewhere: `numTrailingObjects` is not part of the public interface.



Comment at: clang/include/clang/AST/ExprCXX.h:475
   : CXXNamedCastExpr(CXXDynamicCastExprClass, ty, VK, kind, op, pathSize,
- writtenTy, l, RParenLoc, AngleBrackets) {}
+ false, writtenTy, l, RParenLoc, AngleBrackets) {}
 

`/*frob=*/false` here and elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-07 Thread Alain Mosnier via Phabricator via cfe-commits

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

I'm not sure I'm supposed to accept a revision, but apparently I'm authorized 
to do so. And I'm obviously biased since I wrote the bug report. While looking 
for candidates close to the target file might often sound reasonable, it does 
not feel particularly adapted to a whole category of files: template 
declarations. Why would the files that include those be anywhere close to them? 
Also, fetching compile commands from a file written in another language pretty 
much always seems like a bad idea. If I understand correctly, with this commit, 
an arbitrary file of the same language will be picked, every time one such file 
is found. It would of course be even better to pick one that includes the 
target file (in case of a header file), but I can easily imagine that it would 
be much more difficult to implement.
Based on the previous analysis, I approve this commit.
I would also like to express how impressed I am by this project's efficiency!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87253

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

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

2020-09-07 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 added a comment.

Please also add tests for calls to constructors without arguments and calls to 
implicit constructors 1 argument, as requested in 
https://reviews.llvm.org/D86700.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87249

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

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-07 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1137
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())

eduucaldas wrote:
> gribozavr2 wrote:
> > I don't quite understand which test is checking the CXXDefaultArgExpr case 
> > -- could you help me?
> `CXXDefaultArgExpr` is in another [[ https://reviews.llvm.org/D87249 | patch 
> ]]. Although I discovered while working on this it is as linked to 
> Constructors as it is linked to function calls. As such I decided to add that 
> in a separate patch
OK, added a comment in that review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-07 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Minor request, but otherwise LGTM.




Comment at: clang/lib/Analysis/BodyFarm.cpp:188
+  const_cast(Arg), nullptr, VK_RValue,
+  FPOptionsOverride());
 }

Can these call sites just be changed to use `makeImplicitCast`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz added a reviewer: kadircet.
adamcz added a comment.

Hey Kadir

What do you think about this change? This addresses 
https://github.com/clangd/clangd/issues/519, but I'm not entirely convinced 
this is a good idea. Let me know if you have an opinion on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87253

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

[PATCH] D87250: [OpenMP] Fix typo in CodeGenFunction::EmitOMPWorksharingLoop (PR46412)

2020-09-07 Thread Johannes Doerfert via Phabricator via cfe-commits

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

LGTM thx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87250

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

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
adamcz requested review of this revision.

When looking for compile commands for file of type X, other files of
type X will be preferred over files of any other type, regardless of
score.

However, when gathering the set of candidates, we only look for files
with similar name and files close to the requested one. If it happens
that none of them are in language X, we previously (before this change)
would choose one of the existing candidates of some other type.

This patch adds all files in language X (regardless of name and
location) to the list of candidates if the initial set of candidates did
not contain any files of type X. This means we will always choose a file
of type X if one exists anywhere in the project.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87253

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -755,6 +755,7 @@
 }
 
 TEST_F(InterpolateTest, Language) {
+  add("aaa/bbb/ccc/ddd/file.c", "");
   add("dir/foo.cpp", "-std=c++17");
   add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
@@ -774,7 +775,10 @@
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  // even when it is far away.
+  EXPECT_EQ(getCommand("aaa/bbb/ccc/ddd/file.hpp"), "clang -D dir/aux.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
+  Entries.erase(path(StringRef("aaa/bbb/ccc/ddd/file.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
   // Prefer -x over -std when overriding language.
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -352,7 +352,7 @@
 types::ID PreferLanguage) const {
 assert(!empty() && "need at least one candidate!");
 std::string Filename = OriginalFilename.lower();
-auto Candidates = scoreCandidates(Filename);
+auto Candidates = scoreCandidates(Filename, PreferLanguage);
 std::pair Best =
 pickWinner(Candidates, Filename, PreferLanguage);
 
@@ -378,7 +378,8 @@
 
   // Award points to candidate entries that should be considered for the file.
   // Returned keys are indexes into paths, and the values are (nonzero) scores.
-  DenseMap scoreCandidates(StringRef Filename) const {
+  DenseMap scoreCandidates(StringRef Filename,
+types::ID PreferredLanguage) const {
 // Decompose Filename into the parts we care about.
 // /some/path/complicated/project/Interesting.h
 // [-prefix--][---dir---] [-dir-] [--stem---]
@@ -410,6 +411,25 @@
 // Award one more point if the whole rest of the path matches.
 if (sys::path::root_directory(Prefix) != Prefix)
   Award(1, indexLookup(Prefix, Paths));
+
+bool CandidateWithMatchingLanguageFound = false;
+for (const auto  : Candidates) {
+  if (Types[C.first] == PreferredLanguage) {
+CandidateWithMatchingLanguageFound = true;
+break;
+  }
+}
+if (!CandidateWithMatchingLanguageFound) {
+  // If none of the candidates we found so far use preferred language, try
+  // all other files, even if they are far away in the dir hierarchy.
+  for (size_t I = 0; I < OriginalPaths.size(); ++I) {
+// Increase score by one. The actual value doesn't matter, the point is
+// to have some candidates matching PreferredLanguage in the Candidates
+// set at all.
+if (Types[I] == PreferredLanguage)
+  Candidates[I] += 1;
+  }
+}
 return Candidates;
   }
 


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -755,6 +755,7 @@
 }
 
 TEST_F(InterpolateTest, Language) {
+  add("aaa/bbb/ccc/ddd/file.c", "");
   add("dir/foo.cpp", "-std=c++17");
   add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
@@ -774,7 +775,10 @@
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
+  // even when it is far away.
+  EXPECT_EQ(getCommand("aaa/bbb/ccc/ddd/file.hpp"), "clang -D dir/aux.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
+

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290331.
eduucaldas added a comment.

- Rename tests
- Add FIXMEs for init-declarators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,137 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test(){
+  // FIXME: 's...' is a declarator and '{...}' is initializer
+  [[S s0{}]];
+  [[S s1{1}]];
+  [[S s2{1, 2.}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s0'
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s1'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s2'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_EqualBrace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test() {
+  // FIXME: '= {...}' is initializer
+  [[S s0 = {}]];
+  [[S s1 = {1}]];
+  [[S s2 = {1, 2.}]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s0'
+  |-'='
+  `-UnknownExpression
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s1'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s2'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
@@ -3851,15 +3958,134 @@
   R"cpp(
 struct S {
   S(int);
+  S(int, float);
 };
-[[S s(1);]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
 )cpp",
   {R"txt(
 SimpleDeclaration
 |-'S'
 |-SimpleDeclarator Declarator
 | `-UnknownExpression
-|   |-'s'
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+void TakeX(const X&);
+void test() {
+  [[TakeX(1)]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'TakeX'
+|-'(' OpenParen

[PATCH] D87250: [OpenMP] Fix typo in CodeGenFunction::EmitOMPWorksharingLoop (PR46412)

2020-09-07 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon created this revision.
RKSimon added reviewers: ABataev, doctorpangloss.
Herald added subscribers: guansong, yaxunl.
Herald added a project: clang.
RKSimon requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Fixes issue noticed by static analysis where we appear to have a copy+paste 
typo, testing ScheduleKind.M1  twice instead of 
ScheduleKind.M2 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87250

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2982,7 +2982,7 @@
   ((ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
 ScheduleKind.Schedule == OMPC_SCHEDULE_unknown) &&
!(ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic ||
- ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
+ ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
   ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_monotonic ||
   ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_monotonic;
   if ((RT.isStaticNonchunked(ScheduleKind.Schedule,


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2982,7 +2982,7 @@
   ((ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
 ScheduleKind.Schedule == OMPC_SCHEDULE_unknown) &&
!(ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic ||
- ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
+ ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
   ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_monotonic ||
   ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_monotonic;
   if ((RT.isStaticNonchunked(ScheduleKind.Schedule,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79279: Add overloaded versions of builtin mem* functions

2020-09-07 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:4042
+
+TEST_P(SyntaxTreeTest, ExplicitConversion_ZeroArguments) {
+  if (!GetParam().isCXX()) {

gribozavr2 wrote:
> This is not a conversion, this is an explicit constructor call 
> (CXXTemporaryObjectExpr) -- so please rename the test. Same for other tests 
> below.
I am trying to follow the syntax to name things. And in the  [[ 
https://eel.is/c++draft/expr.type.conv | grammar ]] this is under "Explicit 
type conversion (functional notation)", which englobes as well things of the 
type `float(3)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

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

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1137
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())

gribozavr2 wrote:
> I don't quite understand which test is checking the CXXDefaultArgExpr case -- 
> could you help me?
`CXXDefaultArgExpr` is in another [[ https://reviews.llvm.org/D87249 | patch 
]]. Although I discovered while working on this it is as linked to Constructors 
as it is linked to function calls. As such I decided to add that in a separate 
patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

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

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290328.
eduucaldas marked 2 inline comments as done.
eduucaldas added a comment.

answer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit calls to default constructors.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit calls to default constructors.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290327.
eduucaldas marked 3 inline comments as done.
eduucaldas added a comment.

answer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,135 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test(){
+  [[S s0{}]];
+  [[S s1{1}]];
+  [[S s2{1, 2.}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s0'
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s1'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s2'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_EqualBrace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test() {
+  [[S s0 = {}]];
+  [[S s1 = {1}]];
+  [[S s2 = {1, 2.}]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s0'
+  |-'='
+  `-UnknownExpression
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s1'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s2'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
@@ -3851,15 +3956,133 @@
   R"cpp(
 struct S {
   S(int);
+  S(int, float);
 };
-[[S s(1);]]
+[[S s1(1);]]
+[[S s2(1, 2.);]]
 )cpp",
   {R"txt(
 SimpleDeclaration
 |-'S'
 |-SimpleDeclarator Declarator
 | `-UnknownExpression
-|   |-'s'
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+void TakeX(const X&);
+void test() {
+  [[TakeX(1)]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'TakeX'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest,

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas created this revision.
eduucaldas added a reviewer: gribozavr2.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

- Do not visit `CXXDefaultArgExpr`
- To build `CallArguments` nodes, just go through non-default arguments


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87249

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2744,6 +2744,54 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, CallExpression_DefaultArguments) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+void f(int i = 1, char c = '2');
+void test() {
+  [[f()]];
+  [[f(1)]];
+  [[f(1, '2')]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| |-IntegerLiteralExpression ListElement
+| | `-'1' LiteralToken
+| |-',' ListDelimiter
+| `-CharacterLiteralExpression ListElement
+|   `-''2'' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -4106,6 +4154,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParametersAndQualifiers_InFreeFunctions_Default_One) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func1([[int a = 1]]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+`-SimpleDeclaration ListElement
+  |-'int'
+  `-SimpleDeclarator Declarator
+|-'a'
+|-'='
+`-IntegerLiteralExpression
+  `-'1' LiteralToken
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest,
+   ParametersAndQualifiers_InFreeFunctions_Default_Multiple) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func2([[int *ap, int a = 1, char c = '2']]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'*'
+|   `-'ap'
+|-',' ListDelimiter
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'a'
+|   |-'='
+|   `-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-',' ListDelimiter
+`-SimpleDeclaration ListElement
+  |-'char'
+  `-SimpleDeclarator Declarator
+|-'c'
+|-'='
+`-CharacterLiteralExpression
+  `-''2'' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest,
ParametersAndQualifiers_InVariadicFunctionTemplate_ParameterPack) {
   if (!GetParam().isCXX11OrLater() || GetParam().hasDelayedTemplateParsing()) {
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -116,6 +116,13 @@
 };
 } // namespace
 
+static CallExpr::arg_range dropDefaultArgs(CallExpr::arg_range Args) {
+  auto firstDefaultArg = std::find_if(Args.begin(), Args.end(), [](auto it) {
+return isa(it);
+  });
+  return llvm::make_range(Args.begin(), firstDefaultArg);
+}
+
 static syntax::NodeKind getOperatorNodeKind(const CXXOperatorCallExpr ) {
   switch (E.getOperator()) {
   // Comparison
@@ -1073,7 +1080,11 @@
 return true;
   }
 
-  syntax::CallArguments *buildCallArguments(CallExpr::arg_range Args) {
+  /// Builds `CallArguments` syntax node from arguments that appear in source
+  /// code, i.e. not default arguments.
+  syntax::CallArguments *
+  buildCallArguments(CallExpr::arg_range ArgsAndDefaultArgs) {
+auto Args = dropDefaultArgs(ArgsAndDefaultArgs);
 for (const auto  : Args) {
   Builder.markExprChild(Arg, syntax::NodeRole::ListElement);
   const auto *DelimiterToken =
@@ -1187,6 +1198,8 @@
 }
   }
 
+  bool WalkUpFromCXXDefaultArgExpr(CXXDefaultArgExpr *S) { return true; }
+
   bool WalkUpFromNamespaceDecl(NamespaceDecl *S) {
 auto Tokens = Builder.getDeclarationRange(S);
 if (Tokens.front().kind() == tok::coloncolon) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-07 Thread Dave Lee via Phabricator via cfe-commits

kastiglione added a comment.

The `LLVM_ENABLE_WARNINGS` variable is read only within 
`HandleLLVMOptions.cmake`. Outside declarations/defaults have effect only when 
`HandleLLVMOptions` is loaded, one way or another.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

2020-09-07 Thread Craig Topper via Phabricator via cfe-commits

craig.topper abandoned this revision.
craig.topper added a comment.

This got replaced by D85765 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360

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

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:48
 
+// Ignores the implicit `CXXConstructExpr` for copy/move constructors generated
+// by the compiler, as well as in implicit conversions like the one wrapping 
`1`

eduucaldas wrote:
> Please give feedback on this comments and should I comment the rest of the 
> function?




Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:48-50
+// Ignores the implicit `CXXConstructExpr` for copy/move constructors generated
+// by the compiler, as well as in implicit conversions like the one wrapping 
`1`
+// in `X x = 1;`.

gribozavr2 wrote:
> eduucaldas wrote:
> > Please give feedback on this comments and should I comment the rest of the 
> > function?
> 
Seems straightforward to me.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:4042
+
+TEST_P(SyntaxTreeTest, ExplicitConversion_ZeroArguments) {
+  if (!GetParam().isCXX()) {

This is not a conversion, this is an explicit constructor call 
(CXXTemporaryObjectExpr) -- so please rename the test. Same for other tests 
below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-07 Thread Hans Wennborg via Phabricator via cfe-commits

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

LGTM

But please add a -triple parameter to the test files, and check the 
dynamic_cast behavior in each case before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-07 Thread Dave Lee via Phabricator via cfe-commits

kastiglione added a comment.

@lebedev.ri `clang/CMakeLists.txt` contains `include(HandleLLVMOptions)` for 
its standalone build, I think this covers the issue you're pointing out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-07 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

This is not nessesairy correct, at least the clang can be built in standalone 
mode, not as part of monorepo checkout (i.e. it's `clang/cmakelists.txt` is the 
root cmakelists, not `llvm/cmakelists.txt`'s)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-09-07 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

In D86671#2259443 , @dougpuob wrote:

> In D86671#2259364 , @njames93 wrote:
>
>> Did you upload this incorrectly again, context is missing and seems to be a 
>> relative diff from a previous version of this patch?
>
> Sorry for it, I think it's my bad. It is possible that I manually merged the 
> last master(github) with changes then updated them both via web interface ...
>
> Can I fix it if switch back to the base (`14948a0`) then merge all my 
> changes, then update the diff again via web interface? Or do you have any 
> better suggestion?
>
> I am curious about how do you know this mistake? You got error messages with 
> `arc patch D86671` ?

The no context is easy to spot as phab says context not available. Its easy to 
find knowing that there is no mention of hungarian notation in the trunk 
version of IdentifierNamingCheck.cpp, yet there is mention of that in the 
before diff.

The way I do my patches is I create a branch from the current master. Then all 
commits go into that branch. When its time to update the PR I can just do a 
diff from  to .
Though I do use arcanist for my patches

  arc diff master

arcanist will check to see if the current branch has tags for PR and 
automatically update that PR. Otherwise it will create a new PR.
If it goes to create a new PR instead of updating an existing one you can pass 
update

  arc diff master --update D86671

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D87244: [clang] Add fix-it for -Wreorder-ctor.

2020-09-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz updated this revision to Diff 290313.
adamcz added a comment.

Add a missing const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87244

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp

Index: clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
===
--- clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1  -fsyntax-only -Wreorder -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wreorder %s 2>&1 | FileCheck %s
 
 struct BB {};
 
@@ -122,6 +123,9 @@
 }
 
 namespace PR7179 {
+struct Foo {
+  Foo();
+};
   struct X
   {
 struct Y
@@ -129,6 +133,13 @@
   template  Y(T x) : X(x) { }
 };
   };
+
+  struct X2 {
+struct Y {
+  template  Y(T x) : a(), X(x) {}
+  Foo a;
+};
+  };
 }
 
 namespace test3 {
@@ -141,3 +152,23 @@
 }
   };
 }
+
+namespace fix {
+struct Foo {
+  Foo(int) {}
+};
+struct Bar {
+  Foo a, b, c;
+
+  Bar() : c(1), a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:17}:"a(2)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:23}:"b(3)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:25-[[@LINE-3]]:29}:"c(1)"
+
+  // Check that we do not generate fix it when comments are present. Ideally
+  // we would, but comments would need to be reordered as well and the current
+  // code can't do this.
+  Bar(int x) : c(x), /*foo*/ a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+};
+} // namespace fix
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5192,6 +5192,26 @@
   return Member->getAnyMember()->getCanonicalDecl();
 }
 
+// Returns true iff there are comments between Begin and End. Expects that both
+// Begin and End are in the same file.
+static bool RangeContainsComments(Sema , SourceLocation Begin,
+  SourceLocation End) {
+  auto  = SemaRef.getSourceManager();
+  auto BeginLocInfo = SrcMgr.getDecomposedLoc(Begin);
+  StringRef File = SrcMgr.getBufferData(BeginLocInfo.first);
+  Lexer Lex(SrcMgr.getLocForStartOfFile(BeginLocInfo.first),
+SemaRef.getLangOpts(), File.begin(),
+File.begin() + BeginLocInfo.second, File.end());
+  Lex.SetCommentRetentionState(true);
+  Token Tok;
+  while (!Lex.LexFromRawLexer(Tok) &&
+ SrcMgr.isBeforeInTranslationUnit(Tok.getLocation(), End)) {
+if (Tok.is(tok::comment))
+  return true;
+  }
+  return false;
+}
+
 static void DiagnoseBaseOrMemInitializerOrder(
 Sema , const CXXConstructorDecl *Constructor,
 ArrayRef Inits) {
@@ -5241,7 +5261,20 @@
   unsigned NumIdealInits = IdealInitKeys.size();
   unsigned IdealIndex = 0;
 
-  CXXCtorInitializer *PrevInit = nullptr;
+  // Instead of emitting diagnostic right away, we keep it here and only emit
+  // when we find a new one. The last one is emitted outside of this loop, with
+  // a FixIt attached.
+  PartialDiagnosticAt PDiag = {SourceLocation(),
+   PartialDiagnostic::NullDiagnostic()};
+  // Pairs {IdealIndex, pointer-to-initializer}, valid (i.e. found in
+  // IdealInitKeys) initializers only. Last value is used to generate
+  // diagnostics, the index is then used to sort valid initializers in
+  // initialization order.
+  SmallVector, 32>
+  InitsWithIdealIndex;
+  // MissingInIdeal[I] == true iff Inits[I] is not found in ideal list.
+  llvm::SmallBitVector MissingInIdeal(Inits.size());
+
   for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
 CXXCtorInitializer *Init = Inits[InitIndex];
 const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
@@ -5252,35 +5285,100 @@
   if (InitKey == IdealInitKeys[IdealIndex])
 break;
 
-// If we didn't find this initializer, it must be because we
-// scanned past it on a previous iteration.  That can only
-// happen if we're out of order;  emit a warning.
-if (IdealIndex == NumIdealInits && PrevInit) {
-  Sema::SemaDiagnosticBuilder D =
-SemaRef.Diag(PrevInit->getSourceLocation(),
- diag::warn_initializer_out_of_order);
+const bool IsOutOfOrder = IdealIndex == NumIdealInits;
+if (IsOutOfOrder) {
+  if (!InitsWithIdealIndex.empty()) {
+// This is not the first initializer we process. It's either not in the
+// ideal list at all, or it's out of order. Scan from the beginning.
+for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex)
+  if (InitKey ==

[PATCH] D87244: [clang] Add fix-it for -Wreorder-ctor.

2020-09-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz updated this revision to Diff 290312.
adamcz added a comment.

fixed comment typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87244

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp

Index: clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
===
--- clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1  -fsyntax-only -Wreorder -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wreorder %s 2>&1 | FileCheck %s
 
 struct BB {};
 
@@ -122,6 +123,9 @@
 }
 
 namespace PR7179 {
+struct Foo {
+  Foo();
+};
   struct X
   {
 struct Y
@@ -129,6 +133,13 @@
   template  Y(T x) : X(x) { }
 };
   };
+
+  struct X2 {
+struct Y {
+  template  Y(T x) : a(), X(x) {}
+  Foo a;
+};
+  };
 }
 
 namespace test3 {
@@ -141,3 +152,23 @@
 }
   };
 }
+
+namespace fix {
+struct Foo {
+  Foo(int) {}
+};
+struct Bar {
+  Foo a, b, c;
+
+  Bar() : c(1), a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:17}:"a(2)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:23}:"b(3)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:25-[[@LINE-3]]:29}:"c(1)"
+
+  // Check that we do not generate fix it when comments are present. Ideally
+  // we would, but comments would need to be reordered as well and the current
+  // code can't do this.
+  Bar(int x) : c(x), /*foo*/ a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+};
+} // namespace fix
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5192,6 +5192,26 @@
   return Member->getAnyMember()->getCanonicalDecl();
 }
 
+// Returns true iff there are comments between Begin and End. Expects that both
+// Begin and End are in the same file.
+static bool RangeContainsComments(Sema , SourceLocation Begin,
+  SourceLocation End) {
+  auto  = SemaRef.getSourceManager();
+  auto BeginLocInfo = SrcMgr.getDecomposedLoc(Begin);
+  StringRef File = SrcMgr.getBufferData(BeginLocInfo.first);
+  Lexer Lex(SrcMgr.getLocForStartOfFile(BeginLocInfo.first),
+SemaRef.getLangOpts(), File.begin(),
+File.begin() + BeginLocInfo.second, File.end());
+  Lex.SetCommentRetentionState(true);
+  Token Tok;
+  while (!Lex.LexFromRawLexer(Tok) &&
+ SrcMgr.isBeforeInTranslationUnit(Tok.getLocation(), End)) {
+if (Tok.is(tok::comment))
+  return true;
+  }
+  return false;
+}
+
 static void DiagnoseBaseOrMemInitializerOrder(
 Sema , const CXXConstructorDecl *Constructor,
 ArrayRef Inits) {
@@ -5241,7 +5261,20 @@
   unsigned NumIdealInits = IdealInitKeys.size();
   unsigned IdealIndex = 0;
 
-  CXXCtorInitializer *PrevInit = nullptr;
+  // Instead of emitting diagnostic right away, we keep it here and only emit
+  // when we find a new one. The last one is emitted outside of this loop, with
+  // a FixIt attached.
+  PartialDiagnosticAt PDiag = {SourceLocation(),
+   PartialDiagnostic::NullDiagnostic()};
+  // Pairs {IdealIndex, pointer-to-initializer}, valid (i.e. found in
+  // IdealInitKeys) initializers only. Last value is used to generate
+  // diagnostics, the index is then used to sort valid initializers in
+  // initialization order.
+  SmallVector, 32>
+  InitsWithIdealIndex;
+  // MissingInIdeal[I] == true iff Inits[I] is not found in ideal list.
+  llvm::SmallBitVector MissingInIdeal(Inits.size());
+
   for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
 CXXCtorInitializer *Init = Inits[InitIndex];
 const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
@@ -5252,35 +5285,100 @@
   if (InitKey == IdealInitKeys[IdealIndex])
 break;
 
-// If we didn't find this initializer, it must be because we
-// scanned past it on a previous iteration.  That can only
-// happen if we're out of order;  emit a warning.
-if (IdealIndex == NumIdealInits && PrevInit) {
-  Sema::SemaDiagnosticBuilder D =
-SemaRef.Diag(PrevInit->getSourceLocation(),
- diag::warn_initializer_out_of_order);
+bool IsOutOfOrder = IdealIndex == NumIdealInits;
+if (IsOutOfOrder) {
+  if (!InitsWithIdealIndex.empty()) {
+// This is not the first initializer we process. It's either not in the
+// ideal list at all, or it's out of order. Scan from the beginning.
+for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex)
+  if (InitKey ==

[PATCH] D87244: [clang] Add fix-it for -Wreorder-ctor.

2020-09-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz created this revision.
Herald added subscribers: cfe-commits, mgrang.
Herald added a project: clang.
adamcz requested review of this revision.

This version is very limited. It does not work when comments are present
anywhere in the initializer list, since we do not have a good way to
associate them to specific initalizer and move them around.

The fix-it is associated with the last ctor-order diagnostic and
reorders all initializers. This is better than providing fix for each
diagnostic generated, since user would have to apply them one-by-one.

This also fixes a bug that would trigger an assert() in dependent
classes, when initializer present in code might not be found in the
idealized list of initializers. That is related to PR7179.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87244

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp

Index: clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
===
--- clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1  -fsyntax-only -Wreorder -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wreorder %s 2>&1 | FileCheck %s
 
 struct BB {};
 
@@ -122,6 +123,9 @@
 }
 
 namespace PR7179 {
+struct Foo {
+  Foo();
+};
   struct X
   {
 struct Y
@@ -129,6 +133,13 @@
   template  Y(T x) : X(x) { }
 };
   };
+
+  struct X2 {
+struct Y {
+  template  Y(T x) : a(), X(x) {}
+  Foo a;
+};
+  };
 }
 
 namespace test3 {
@@ -141,3 +152,23 @@
 }
   };
 }
+
+namespace fix {
+struct Foo {
+  Foo(int) {}
+};
+struct Bar {
+  Foo a, b, c;
+
+  Bar() : c(1), a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:17}:"a(2)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:23}:"b(3)"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:25-[[@LINE-3]]:29}:"c(1)"
+
+  // Check that we do not generate fix it when comments are present. Ideally
+  // we would, but comments would need to be reordered as well and the current
+  // code can't do this.
+  Bar(int x) : c(x), /*foo*/ a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+};
+} // namespace fix
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5192,6 +5192,26 @@
   return Member->getAnyMember()->getCanonicalDecl();
 }
 
+// Returns true iff there are comments between Begin and End. Expects that both
+// Begin and End are in the same file.
+static bool RangeContainsComments(Sema , SourceLocation Begin,
+  SourceLocation End) {
+  auto  = SemaRef.getSourceManager();
+  auto BeginLocInfo = SrcMgr.getDecomposedLoc(Begin);
+  StringRef File = SrcMgr.getBufferData(BeginLocInfo.first);
+  Lexer Lex(SrcMgr.getLocForStartOfFile(BeginLocInfo.first),
+SemaRef.getLangOpts(), File.begin(),
+File.begin() + BeginLocInfo.second, File.end());
+  Lex.SetCommentRetentionState(true);
+  Token Tok;
+  while (!Lex.LexFromRawLexer(Tok) &&
+ SrcMgr.isBeforeInTranslationUnit(Tok.getLocation(), End)) {
+if (Tok.is(tok::comment))
+  return true;
+  }
+  return false;
+}
+
 static void DiagnoseBaseOrMemInitializerOrder(
 Sema , const CXXConstructorDecl *Constructor,
 ArrayRef Inits) {
@@ -5241,7 +5261,20 @@
   unsigned NumIdealInits = IdealInitKeys.size();
   unsigned IdealIndex = 0;
 
-  CXXCtorInitializer *PrevInit = nullptr;
+  // Instead of emitting diagnostic right away, we keep it here and only emit
+  // when we find a new one. The last one is emitted outsdie of this loop, with
+  // a FixIt attached.
+  PartialDiagnosticAt PDiag = {SourceLocation(),
+   PartialDiagnostic::NullDiagnostic()};
+  // Pairs {IdealIndex, pointer-to-initializer}, valid (i.e. found in
+  // IdealInitKeys) initializers only. Last value is used to generate
+  // diagnostics, the index is then used to sort valid initializers in
+  // initialization order.
+  SmallVector, 32>
+  InitsWithIdealIndex;
+  // MissingInIdeal[I] == true iff Inits[I] is not found in ideal list.
+  llvm::SmallBitVector MissingInIdeal(Inits.size());
+
   for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
 CXXCtorInitializer *Init = Inits[InitIndex];
 const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
@@ -5252,35 +5285,100 @@
   if (InitKey == IdealInitKeys[IdealIndex])
 break;
 
-// If we didn't find this initializer, it must be because we
-// scanned past it on a previous iteration.  That can only
-// happen if we're out of order;  emit a warning.
-if

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-07 Thread Dave Lee via Phabricator via cfe-commits

kastiglione created this revision.
Herald added subscribers: llvm-commits, libcxx-commits, cfe-commits, mgorny.
Herald added a reviewer: DavidTruby.
Herald added projects: clang, libunwind, LLVM.
Herald added a reviewer: libunwind.
kastiglione requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87243

Files:
  clang/CMakeLists.txt
  flang/CMakeLists.txt
  libunwind/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/runtimes/CMakeLists.txt


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -105,10 +105,6 @@
   # Avoid checking whether the compiler is working.
   set(LLVM_COMPILER_CHECKED ON)
 
-  # Enable warnings, otherwise -w gets added to the cflags by HandleLLVMOptions
-  # resulting in unjustified successes by check_cxx_compiler_flag.
-  set(LLVM_ENABLE_WARNINGS ON)
-
   # Handle common options used by all runtimes.
   include(AddLLVM)
   include(HandleLLVMOptions)
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -409,6 +409,8 @@
   endif()
 endif()
 
+option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
+
 if( MSVC )
   include(ChooseMSVCCRT)
 
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -399,7 +399,6 @@
 list(REMOVE_DUPLICATES LLVM_TARGETS_TO_BUILD)
 
 option(LLVM_ENABLE_PIC "Build Position-Independent Code" ON)
-option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
 option(LLVM_ENABLE_MODULES "Compile with C++ modules enabled." OFF)
 if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." ON)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -73,8 +73,6 @@
   endif()
 
   if (EXISTS ${LLVM_CMAKE_PATH})
-# Enable warnings, otherwise -w gets added to the cflags by 
HandleLLVMOptions.
-set(LLVM_ENABLE_WARNINGS ON)
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
 include("${LLVM_CMAKE_PATH}/AddLLVM.cmake")
 include("${LLVM_CMAKE_PATH}/HandleLLVMOptions.cmake")
Index: flang/CMakeLists.txt
===
--- flang/CMakeLists.txt
+++ flang/CMakeLists.txt
@@ -62,7 +62,6 @@
   if(LLVM_ENABLE_ZLIB)
 find_package(ZLIB REQUIRED)
   endif()
-  option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
   option(LLVM_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
   if(CMAKE_COMPILER_IS_GNUCXX)
 set(USE_NO_MAYBE_UNINITIALIZED 1)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -105,7 +105,6 @@
 set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
   endif()
 
-  option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
   option(LLVM_INSTALL_TOOLCHAIN_ONLY
 "Only include toolchain files in the 'install' target." OFF)
 


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -105,10 +105,6 @@
   # Avoid checking whether the compiler is working.
   set(LLVM_COMPILER_CHECKED ON)
 
-  # Enable warnings, otherwise -w gets added to the cflags by HandleLLVMOptions
-  # resulting in unjustified successes by check_cxx_compiler_flag.
-  set(LLVM_ENABLE_WARNINGS ON)
-
   # Handle common options used by all runtimes.
   include(AddLLVM)
   include(HandleLLVMOptions)
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -409,6 +409,8 @@
   endif()
 endif()
 
+option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
+
 if( MSVC )
   include(ChooseMSVCCRT)
 
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -399,7 +399,6 @@
 list(REMOVE_DUPLICATES LLVM_TARGETS_TO_BUILD)
 
 option(LLVM_ENABLE_PIC "Build Position-Independent Code" ON)
-option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
 option(LLVM_ENABLE_MODULES "Compile with C++ modules enabled." OFF)
 if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
   option(LLVM_ENABLE_MODULE_DEBUGGING "Compile with -gmodules." ON)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -73,8 +73,6 @@
   endif()
 
   if (EXISTS ${LLVM_CMAKE_PATH})
-# Enable

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

2020-09-07 Thread Raul Tambre via Phabricator via cfe-commits

tambre added a comment.

phosek: ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-07 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff added a comment.

Any feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-09-07 Thread Douglas Chen via Phabricator via cfe-commits

dougpuob added a comment.

In D86671#2259364 , @njames93 wrote:

> Did you upload this incorrectly again, context is missing and seems to be a 
> relative diff from a previous version of this patch?

Sorry for it, I think it's my bad. It is possible that I manually merged the 
last master(github) with changes then updated them both via web interface ...

Can I fix it if switch back to the base (`14948a0`) then merge all my changes, 
then update the diff again via web interface? Or do you have any better 
suggestion?

I am curious about how do you know this mistake? You got error messages with 
`arc patch D86671` ?

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits

bc-lee updated this revision to Diff 290303.
bc-lee added a comment.

Some comments have been corrected and a unittest has been added in 
FormatTest.ParsesConfigurationBools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp

Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
  "import org.c;\n"));
 }
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;
+
+  EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
 TEST_F(SortImportsTestJava, DeduplicateImports) {
   EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
 "import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
 IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
 IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+   Style.JavaStaticImportAfterImport);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
 JavaImportGroups.push_back(
 findJavaImportGroup(Style, Imports[i].Identifier));
   }
+  bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
   llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
 // Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
-   Imports[LHSI].Identifier) <
-   std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
-   Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+   std::make_tuple(!Imports[RHSI].IsStatic ^
+   StaticImportAfterNormalImport,
+   JavaImportGroups[RHSI], Imports[RHSI].Identifier);
   });
 
   // Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is separated by a newline. Static imports will also follow the
-  /// same grouping convention above all non-static imports. One group's prefix
-  /// can be a subset of another - the longest prefix is always matched. Within
-  /// a group, the imports are ordered lexicographically.
+  /// One group's prefix can be a subset of another - the longest

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-07 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1136
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&





Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1137
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())

I don't quite understand which test is checking the CXXDefaultArgExpr case -- 
could you help me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

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

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits

martong marked an inline comment as done.
martong added a comment.

In D87239#2259345 , @steakhal wrote:

> I completely agree with you.
> I plan to further refactor the CStringChecker, but the related patches are 
> pretty much stuck :D
>
> I think this workaround is fine for now.
> You might as well extend the corresponding parts of the CStringChecker to 
> make the modelling more precise.
> It shouldn't be much of a hassle.
> What do you say about that?

I think the modeling is well done and precise. I mean, it seems like all of the 
constraints that I am removing here are handled in CStringChecker. It checks 
the pointer arguments whether they are null. Also, the length is checked in 
case of `strncasecmp`, here:

  if (IsBounded) {
// Get the max number of characters to compare.
const Expr *lenExpr = CE->getArg(2);
SVal lenVal = state->getSVal(lenExpr, LCtx);

// If the length is known, we can get the right substrings.
if (const llvm::APSInt *len = svalBuilder.getKnownValue(state, lenVal)) {
  // Create substrings of each to compare the prefix.
  LeftStrRef = LeftStrRef.substr(0, (size_t)len->getZExtValue());
  RightStrRef = RightStrRef.substr(0, (size_t)len->getZExtValue());
  canComputeResult = true;
}
  } else {  }

So, the problem is rather that the constraint check should be done in 
checkPreCall, but that should be in an NFC refactoring.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87239

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

2020-09-07 Thread Alan Phipps via Phabricator via cfe-commits

alanphipps updated this revision to Diff 290301.
alanphipps added a comment.

- Rename isLeafCondition() to isInstrumentedCondition() and rephrase associated 
comments
- Add branch regions on C++ range-based loops


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

https://reviews.llvm.org/D84467

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp

Index: clang/test/CoverageMapping/macro-expressions.cpp
===
--- clang/test/CoverageMapping/macro-expressions.cpp
+++ clang/test/CoverageMapping/macro-expressions.cpp
@@ -79,7 +79,8 @@
   // CHECK-NEXT: File 0, [[@LINE+2]]:6 -> [[@LINE+2]]:8 = (#0 + #6)
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = (#0 + #6)
   do {} while (NEXPR(i));
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:8 -> [[@LINE+3]]:12 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:8 -> [[@LINE+4]]:12 = #0
+  // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:21 -> [[@LINE+3]]:22 = #7, #0
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:23 -> [[@LINE+2]]:26 = #0
   // CHECK: File 0, [[@LINE+1]]:42 -> [[@LINE+1]]:44 = #7
   for (DECL(int, j) : ARR(int, 1, 2, 3)) {}
Index: clang/test/CoverageMapping/loops.cpp
===
--- clang/test/CoverageMapping/loops.cpp
+++ clang/test/CoverageMapping/loops.cpp
@@ -4,7 +4,9 @@
 // CHECK: rangedFor
 void rangedFor() {  // CHECK-NEXT: File 0, [[@LINE]]:18 -> {{[0-9]+}}:2 = #0
   int arr[] = { 1, 2, 3, 4, 5 };
-  int sum = 0;  // CHECK: Gap,File 0, [[@LINE+1]]:20 -> [[@LINE+1]]:21 = #1
+  int sum = 0;  
+// CHECK-NEXT: Branch,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:15 = #1, (#0 - #3) 
+// CHECK: Gap,File 0, [[@LINE+1]]:20 -> [[@LINE+1]]:21 = #1
   for(auto i : arr) {   // CHECK: File 0, [[@LINE]]:21 -> [[@LINE+6]]:4 = #1
 if (i == 3)
   continue; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #2
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -792,20 +792,20 @@
 return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext()));
   }
 
-  /// Create a Branch Region around a leaf-level condition for code coverage
+  /// Create a Branch Region around an instrumentable condition for coverage
   /// and add it to the function's SourceRegions.  A branch region tracks a
-  /// "True" counter and a "False" counter for all leaf-level boolean
-  /// expressions that result in the generation of a branch.
+  /// "True" counter and a "False" counter for boolean expressions that
+  /// result in the generation of a branch.
   void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt) {
 // Check for NULL conditions.
 if (!C)
   return;
 
-// Ensure we are a leaf-level condition (i.e. no "&&" or "||").  Push
+// Ensure we are an instrumentable condition (i.e. no "&&" or "||").  Push
 // region onto RegionStack but immediately pop it (which adds it to the
 // function's SourceRegions) because it doesn't apply to any other source
 // code other than the Condition.
-if (CodeGenFunction::isLeafCondition(C)) {
+if (CodeGenFunction::isInstrumentedCondition(C)) {
   // If a condition can fold to true or false, the corresponding branch
   // will be removed.  Create a region with both counters hard-coded to
   // zero. This allows us to visualize them in a special way.
@@ -1250,6 +1250,10 @@
 addCounters(BC.BreakCount, subtractCounters(LoopCount, BodyCount));
 if (OutCount != ParentCount)
   pushRegion(OutCount);
+
+// Create Branch Region around condition.
+createBranchRegion(S->getCond(), BodyCount,
+   subtractCounters(LoopCount, BodyCount));
   }
 
   void VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S) {
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -212,7 +212,8 @@
   /// version so that we facilitate backward compatibility.
   bool VisitBinaryOperator(BinaryOperator *S) {
 if (ProfileVersion >= llvm::IndexedInstrProf::Version7)
-  if (S->isLogicalOp() && CodeGenFunction::isLeafCondition(S->getRHS()))
+  if (S->isLogicalOp() &&
+  CodeGenFunction::isInstrumentedCondition(S->getRHS()))
 CounterMap[S->getRHS()] = NextCounter++;
 return Base::VisitBinaryOperator(S);
   }

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

2020-09-07 Thread Alan Phipps via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359
+  /// condition (i.e. no "&&" or "||").
+  static bool isLeafCondition(const Expr *C);
+

vsk wrote:
> alanphipps wrote:
> > vsk wrote:
> > > alanphipps wrote:
> > > > vsk wrote:
> > > > > vsk wrote:
> > > > > > It might be helpful to either require that `C` be the RHS of a 
> > > > > > logical binop (e.g. by passing the binop expr in), or to document 
> > > > > > that requirement.
> > > > > Given a logical binop like `E = a && !(b || c)`, 
> > > > > `isLeafCondition(E->getRHS())` is true. That seems a bit 
> > > > > counter-intuitive, because `E->getRHS()` contains another leaf 
> > > > > condition. Would it make sense to rename the condition (perhaps to 
> > > > > something like 'InstrumentedCondition')? Have I misunderstood what a 
> > > > > leaf condition is?
> > > > Background: isLeafCondition() is an auxiliary routine that is used 
> > > > during 1.) counter allocation on binop RHS (CodeGenPGO.cpp), 2.) 
> > > > counter instrumentation (CodeGenFunction.cpp), and 3.) branch region 
> > > > generation (CoverageMappingGen.cpp).  In the #3 case, it isn't always 
> > > > looking at the RHS of a logical binop but can be used to assess whether 
> > > > any condition is instrumented/evaluated.
> > > > 
> > > > Given your example condition:
> > > > 
> > > > E = a && !(b || c)
> > > > 
> > > > You are correct that isLeafCondition(E->getRHS()) will return true, but 
> > > > I think it should actually return false here (and bypass logical-NOT), 
> > > > so I will adapt this. 
> > > > 
> > > > However, given a condition that includes an binary operator (like 
> > > > assign):
> > > > 
> > > > E = a && (x = !(b || c))
> > > > 
> > > > isLeafCondition(E->getRHS()) will also return true, and I think that is 
> > > > the correct behavior.  Given that, then I agree that maybe 
> > > > isLeafCondition() should be renamed to isInstrumentedCondition() since 
> > > > it's not technically just leaves that are tracked in the presence of 
> > > > operators.
> > > What is special about the assignment expression nested within "a && (x = 
> > > !(b || c))" that necessitates an extra counter compared to "a && !(b || 
> > > c)"?
> > I'm exempting the logical NOT operator basically to match the functionality 
> > of other coverage vendors (Bullseye, GCOV).  It's a simplistic operator in 
> > the sense that (as far as I can tell) it only affects the sense of the 
> > generated branch on a condition.
> > 
> > As for the assignment, we're effectively creating a new condition that is 
> > evaluatable (even if a branch may technically not be generated), and so 
> > creating a counter for it is more interesting (and matches other vendor 
> > behavior).
> > 
> > But I'm open to persuasion.  We could just be more conservative and create 
> > counters for logical NOT, but I think there's value in matching some of the 
> > expected behavior of other vendors.  This is also something we could 
> > continue to fine-tune in future patches.
> I agree that there isn't a need to insert a fresh counter to accommodate a 
> logical NOT operator in a condition. But I don't see how an assignment 
> expression effectively creates a new, evaluatable condition. It seems like 
> the count for the assignment expr can be derived by looking up the count for 
> its parent region.
> 
> At this stage I'm more interested in understanding the high-level design. If 
> the question of whether/not to add fresh counters for assignment exprs in a 
> condition is effectively about optimization, then I'm fine with tabling it.
I just realized I didn't make this name change to isInstrumentedCondition(). I 
will do that.


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

https://reviews.llvm.org/D84467

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

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D87081#2258637 , @Szelethus wrote:

> The patch looks great, in fact, it demonstrates how well thought out your 
> summary crafting machinery is.
>
> In D87081#2258579 , @martong wrote:
>
>> However, in a similar case with the CallAndMessage Checker, we decided to 
>> list the more specific Checker as a dependency.
>
> We got the answer to D77061#2057063 
> ! We should turn it into a weak 
> dependency though (D80905 ).
>
> In D87081#2256636 , @balazske wrote:
>
>> This checker will make an additional assumption on `fread` and `fwrite` with 
>> the ReturnValueCondition. The return value is constrained by `StreamChecker` 
>> too but it splits the error (if returned value is less that arg 3) and 
>> non-error cases into separate branches. I think this causes no problem 
>> because it will refine the assumption made here (if this assumption is made 
>> first) or the assumption here has no effect (if the split happened already).
>
> Be sure to triple check whether the `ExplodedGraph` looks okay with both 
> checkers enabled.

I'll try to create tests that check the state in both order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87081

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

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

2020-09-07 Thread Gabor Marton 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 rG8248c2af9497: [analyzer][StdLibraryFunctionsChecker] Have 
proper weak dependencies (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D87240?vs=290294=290299#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87240

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
  clang/test/Analysis/std-c-library-functions-arg-weakdeps.c

Index: clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -0,0 +1,64 @@
+// Check that the more specific checkers report and not the generic
+// StdCLibraryFunctionArgs checker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify
+
+
+// Make sure that all used functions have their summary loaded.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple x86_64-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: int isalnum(int)
+// CHECK: Loaded summary for: unsigned long fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)))
+// CHECK: Loaded summary for: int fileno(FILE *stream)
+
+void initializeSummaryMap();
+// We analyze this function first, and the call expression inside initializes
+// the summary map. This way we force the loading of the summaries. The
+// summaries would not be loaded without this because during the first bug
+// report in WeakDependency::checkPreCall we stop further evaluation. And
+// StdLibraryFunctionsChecker lazily initializes its summary map from its
+// checkPreCall.
+void analyzeThisFirst() {
+  initializeSummaryMap();
+}
+
+typedef __typeof(sizeof(int)) size_t;
+struct FILE;
+typedef struct FILE FILE;
+
+int isalnum(int);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)));
+int fileno(FILE *stream);
+
+void test_uninit_arg() {
+  int v;
+  int r = isalnum(v); // \
+  // expected-warning{{1st function call argument is an uninitialized value [core.CallAndMessage]}}
+  (void)r;
+}
+
+void test_notnull_arg(FILE *F) {
+  int *p = 0;
+  fread(p, sizeof(int), 5, F); // \
+  expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
+}
+
+void test_notnull_stream_arg() {
+  fileno(0); // \
+  // expected-warning{{Stream pointer might be NULL [alpha.unix.Stream]}}
+}
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -1,4 +1,13 @@
+// Here we test the order of the Checkers when StdCLibraryFunctionArgs is
+// enabled.
+
 // RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-checker=core \
+// RUN:   -Xclang -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -Xclang -analyzer-config \
+// RUN:  -Xclang apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.Stream \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
@@ -7,14 +16,16 @@
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT: core.CallAndMessageModeling
+// CHECK-NEXT: core.CallAndMessage
+// CHECK-NEXT: core.NonNullParamChecker
+// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: alpha.unix.StdCLibraryFunctionArgs
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
-// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT:

[clang] 8248c2a - [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

2020-09-07 Thread Gabor Marton via cfe-commits


Author: Gabor Marton
Date: 2020-09-07T17:56:26+02:00
New Revision: 8248c2af94975912b14e7e0cb414fcbb82c77123

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

LOG: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

We want the generice StdLibraryFunctionsChecker to report only if there
are no specific checkers that would handle the argument constraint for a
function.

Note, the assumptions are still evaluated, even if the arguement
constraint checker is set to not report. This means that the assumptions
made in the generic StdLibraryFunctionsChecker should be an
over-approximation of the assumptions made in the specific checkers. But
most importantly, the assumptions should not contradict.

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

Added: 
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
clang/test/Analysis/std-c-library-functions-arg-weakdeps.c

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/test/Analysis/analyzer-enabled-checkers.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index a444843c5006..a61af4523134 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -349,7 +349,6 @@ let ParentPackage = APIModeling in {
 
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
-  Dependencies<[CallAndMessageModeling]>,
   CheckerOptions<[
 CmdLineOption,
"such as whether the parameter of isalpha is in the range [0, 255] "
"or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
-  WeakDependencies<[NonNullParamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, 
StreamChecker]>,
   Documentation;
 
 } // end "alpha.unix"

diff  --git a/clang/test/Analysis/analyzer-enabled-checkers.c 
b/clang/test/Analysis/analyzer-enabled-checkers.c
index 7c00e78c16ac..bef786a1a59b 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -6,11 +6,11 @@
 
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
-// CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation

diff  --git 
a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c 
b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
new file mode 100644
index ..9ad1be053851
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -0,0 +1,66 @@
+// Here we test the order of the Checkers when StdCLibraryFunctionArgs is
+// enabled.
+
+// RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-checker=core \
+// RUN:   -Xclang -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -Xclang -analyzer-config \
+// RUN:  -Xclang apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.Stream \
+// RUN:   -Xclang -analyzer-list-enabled-checkers \
+// RUN:   -Xclang -analyzer-display-progress \
+// RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
+// RUN:   --implicit-check-not=\.
+
+// CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
+// CHECK-EMPTY:
+// CHECK-NEXT: core.CallAndMessageModeling
+// CHECK-NEXT: core.CallAndMessage
+// CHECK-NEXT: core.NonNullParamChecker
+// CHECK-NEXT: alpha.unix.Stream
+// CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: alpha.unix.StdCLibraryFunctionArgs
+// CHECK-NEXT: apiModeling.TrustNonnull
+// CHECK-NEXT: apiModeling.llvm.CastValue
+// CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.DivideZero
+// CHECK-NEXT: core.DynamicTypePropagation
+// CHECK-NEXT: core.NonnilStringConstants
+// CHECK-NEXT: core.NullDereference
+// CHECK-NEXT: core.StackAddrEscapeBase
+// CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.UndefinedBinaryOperatorResult
+// CHECK-NEXT: core.VLASize
+// CHECK-NEXT: core.builtin.BuiltinFunctions
+// CHECK-NEXT: core.builtin.NoReturnFunctions
+// CHECK-NEXT: core.uninitialized.ArraySubscript
+// CHECK-NEXT: core.uninitialized.Assign
+// CHECK-NEXT: core.uninitialized.Branch
+// CHECK-NEXT:

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-07 Thread Byoungchan Lee via Phabricator via cfe-commits

bc-lee added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2027
+
+ .. code-block:: java
+ true:

MyDeveloperDay wrote:
> The ClangFormatStyleOptions.rst is generated using 
> doc/tools/dump_format_style.py which reads Format.h and generates this,
> 
> If this code block in not in the Format.h it will get removed the next time 
> the script is run, please don't change ClangFormatStyleOption.rst by hand use 
> the script, so add the code block to the Format.h file (see others options 
> for now to do this)
Done.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

JakeMerdichAMD wrote:
> 3 things here:
> 
> 1. Did you mix up the true and false cases?
> 2. (nit) You should also note that if this option is false, all static 
> imports are before all non-static imports (as opposed to mixed together 
> alphabetically). I was confused on first glance.
> 3. Please add this config option to FormatTests.cpp:ParsesConfigurationBools.
I understand. The description is somewhat misleading. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd01280587d97: [analyzer][StdLibraryFunctionsChecker] Add 
POSIX pthread handling functions (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D84415?vs=289970=290296#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -108,6 +108,20 @@
 // CHECK: Loaded summary for: struct tm *gmtime(const time_t *tp)
 // CHECK: Loaded summary for: int clock_gettime(clockid_t clock_id, struct timespec *tp)
 // CHECK: Loaded summary for: int getitimer(int which, struct itimerval *curr_value)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -259,6 +273,34 @@
 struct itimerval;
 int getitimer(int which, struct itimerval *curr_value);
 
+typedef union {
+  int x;
+} pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union {
+  int x;
+} pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union {
+  int x;
+} pthread_mutex_t;
+typedef union {
+  int x;
+} pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -951,6 +951,8 @@
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -2182,6 +2184,99 @@
 Summary(ArgTypes{IntTy, StructItimervalPtrTy},
 RetType{IntTy}, NoEvalCall)
 .ArgConstraint(NotNull(ArgNo(1;
+
+Optional Pthread_cond_tTy = lookupTy("pthread_cond_t");
+Optional Pthread_cond_tPtrTy = getPointerTy(Pthread_cond_tTy);
+Optional Pthread_tTy =

[clang] d012805 - [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

2020-09-07 Thread Gabor Marton via cfe-commits


Author: Gabor Marton
Date: 2020-09-07T17:47:01+02:00
New Revision: d01280587d97eb02d37da37666afd3e4d57c9336

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

LOG: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-POSIX.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index ddde629f44a5..b71c19a80da9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -951,6 +951,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -2182,6 +2184,99 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 Summary(ArgTypes{IntTy, StructItimervalPtrTy},
 RetType{IntTy}, NoEvalCall)
 .ArgConstraint(NotNull(ArgNo(1;
+
+Optional Pthread_cond_tTy = lookupTy("pthread_cond_t");
+Optional Pthread_cond_tPtrTy = getPointerTy(Pthread_cond_tTy);
+Optional Pthread_tTy = lookupTy("pthread_t");
+Optional Pthread_tPtrTy = getPointerTy(Pthread_tTy);
+Optional Pthread_tPtrRestrictTy = getRestrictTy(Pthread_tPtrTy);
+Optional Pthread_mutex_tTy = lookupTy("pthread_mutex_t");
+Optional Pthread_mutex_tPtrTy = getPointerTy(Pthread_mutex_tTy);
+Optional Pthread_mutex_tPtrRestrictTy =
+getRestrictTy(Pthread_mutex_tPtrTy);
+Optional Pthread_attr_tTy = lookupTy("pthread_attr_t");
+Optional Pthread_attr_tPtrTy = getPointerTy(Pthread_attr_tTy);
+Optional ConstPthread_attr_tPtrTy =
+getPointerTy(getConstTy(Pthread_attr_tTy));
+Optional ConstPthread_attr_tPtrRestrictTy =
+getRestrictTy(ConstPthread_attr_tPtrTy);
+Optional Pthread_mutexattr_tTy = lookupTy("pthread_mutexattr_t");
+Optional ConstPthread_mutexattr_tPtrTy =
+getPointerTy(getConstTy(Pthread_mutexattr_tTy));
+Optional ConstPthread_mutexattr_tPtrRestrictTy =
+getRestrictTy(ConstPthread_mutexattr_tPtrTy);
+
+QualType PthreadStartRoutineTy = getPointerTy(
+ACtx.getFunctionType(/*ResultTy=*/VoidPtrTy, /*Args=*/VoidPtrTy,
+ FunctionProtoType::ExtProtoInfo()));
+
+// int pthread_cond_signal(pthread_cond_t *cond);
+// int pthread_cond_broadcast(pthread_cond_t *cond);
+addToFunctionSummaryMap(
+{"pthread_cond_signal", "pthread_cond_broadcast"},
+Signature(ArgTypes{Pthread_cond_tPtrTy}, RetType{IntTy}),
+Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+
+// int pthread_create(pthread_t *restrict thread,
+//const pthread_attr_t *restrict attr,
+//void *(*start_routine)(void*), void *restrict arg);
+addToFunctionSummaryMap(
+"pthread_create",
+Signature(ArgTypes{Pthread_tPtrRestrictTy,
+   ConstPthread_attr_tPtrRestrictTy,
+   PthreadStartRoutineTy, VoidPtrRestrictTy},
+  RetType{IntTy}),
+Summary(NoEvalCall)
+.ArgConstraint(NotNull(ArgNo(0)))
+.ArgConstraint(NotNull(ArgNo(2;
+
+// int pthread_attr_destroy(pthread_attr_t *attr);
+// int pthread_attr_init(pthread_attr_t *attr);
+addToFunctionSummaryMap(
+{"pthread_attr_destroy", "pthread_attr_init"},
+Signature(ArgTypes{Pthread_attr_tPtrTy}, RetType{IntTy}),
+Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0;
+
+// int pthread_attr_getstacksize(const pthread_attr_t *restrict attr,
+//   size_t *restrict stacksize);
+// int pthread_attr_getguardsize(const pthread_attr_t *restrict attr,
+//   size_t *restrict guardsize);
+addToFunctionSummaryMap(
+{"pthread_attr_getstacksize", "pthread_attr_getguardsize"},
+Signature(ArgTypes{ConstPthread_attr_tPtrRestrictTy, 
SizePtrRestrictTy},
+  RetType{IntTy}),
+Summary(NoEvalCall)
+.ArgConstraint(NotNull(ArgNo(0)))
+

[PATCH] D85611: Improve dynamic AST matching diagnostics for conversion errors

2020-09-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D85611#2230481 , @aaron.ballman 
wrote:

> In D85611#2218144 , @aaron.ballman 
> wrote:
>
>> Ping
>
> Ping x2

I realize it's performance review time for some folks and their schedules may 
be busier than usual, gently pinging this so it doesn't fall through the cracks.

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

https://reviews.llvm.org/D85611

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

In D86135#2259325 , @steakhal wrote:

> Perfectly clear, thank you. However, I would still rely on the others to 
> accept this :|
>
> BTW why does the `plist-macros-with-expansion.cpp.plist` change? It makes the 
> diff somewhat noisy :s

Well, I removed a line, so every other entry about file position is changed in 
the plist file. I think I could just remove the entire thing altogether, its 
not like the actual plist output is what we're looking for, at least not in its 
many-thousand-line entirety.

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

https://reviews.llvm.org/D86135

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-09-07 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Did you upload this incorrectly again, context is missing and seems to be a 
relative diff from a previous version of this patch?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

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

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits

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

This is exactly how I imagines weak dependencies to work. LGTM on my end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87240

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

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-07 Thread Jake Merdich via Phabricator via cfe-commits

JakeMerdichAMD requested changes to this revision.
JakeMerdichAMD added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Format/Format.h:1705
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

3 things here:

1. Did you mix up the true and false cases?
2. (nit) You should also note that if this option is false, all static imports 
are before all non-static imports (as opposed to mixed together 
alphabetically). I was confused on first glance.
3. Please add this config option to FormatTests.cpp:ParsesConfigurationBools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

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

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits

martong created this revision.
martong added reviewers: Szelethus, balazske, NoQ, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a project: clang.
martong requested review of this revision.

We want the generice StdLibraryFunctionsChecker to report only if there
are no specific checkers that would handle the argument constraint for a
function.

Note, the assumptions are still evaluated, even if the arguement
constraint checker is set to not report. This means that the assumptions
made in the generic StdLibraryFunctionsChecker should be an
over-approximation of the assumptions made in the specific checkers. But
most importantly, the assumptions should not contradict.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87240

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
  clang/test/Analysis/std-c-library-functions-arg-weakdeps.c

Index: clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -0,0 +1,64 @@
+// Check that the more specific checkers report and not the generic
+// StdCLibraryFunctionArgs checker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify
+
+
+// Make sure that all used functions have their summary loaded.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple x86_64-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: int isalnum(int)
+// CHECK: Loaded summary for: unsigned long fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)))
+// CHECK: Loaded summary for: int fileno(FILE *stream)
+
+void initializeSummaryMap();
+// We analyze this function first, and the call expression inside initializes
+// the summary map. This way we force the loading of the summaries. The
+// summaries would not be loaded without this because during the first bug
+// report in WeakDependency::checkPreCall we stop further evaluation. And
+// StdLibraryFunctionsChecker lazily initializes its summary map from its
+// checkPreCall.
+void analyzeThisFirst() {
+  initializeSummaryMap();
+}
+
+typedef __typeof(sizeof(int)) size_t;
+struct FILE;
+typedef struct FILE FILE;
+
+int isalnum(int);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)));
+int fileno(FILE *stream);
+
+void test_uninit_arg() {
+  int v;
+  int r = isalnum(v); // \
+  // expected-warning{{1st function call argument is an uninitialized value [core.CallAndMessage]}}
+  (void)r;
+}
+
+void test_notnull_arg(FILE *F) {
+  int *p = 0;
+  fread(p, sizeof(int), 5, F); // \
+  expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
+}
+
+void test_notnull_stream_arg() {
+  fileno(0); // \
+  // expected-warning{{Stream pointer might be NULL [alpha.unix.Stream]}}
+}
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -1,4 +1,13 @@
+// Here we test the order of the Checkers when StdCLibraryFunctionArgs is
+// enabled.
+
 // RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-checker=core \
+// RUN:   -Xclang -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -Xclang -analyzer-config \
+// RUN:  -Xclang apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -Xclang -analyzer-checker=alpha.unix.Stream \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
@@ -7,14 +16,16 @@
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT:

[PATCH] D87138: [analyzer][NFC] Introduce refactoring of PthreadLockChecker

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal accepted this revision.
steakhal added a comment.

Seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87138

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

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

I completely agree with you.
I plan to further refactor the CStringChecker, but the related patches are 
pretty much stuck :D

I think this workaround is fine for now.
You might as well extend the corresponding parts of the CStringChecker to make 
the modelling more precise.
It shouldn't be much of a hassle.
What do you say about that?




Comment at: 
clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c:12
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify

I'm not sure if this is required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87239

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-07 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added inline comments.



Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817
+  return IsASCII ? "^" : (const char *)u8"\u2548";
 case LineChar::RangeMid:
-  return IsASCII ? "|" : u8"\u2503";
+  return IsASCII ? "|" : (const char *)u8"\u2503";
 case LineChar::RangeEnd:
-  return IsASCII ? "v" : u8"\u253b";
+  return IsASCII ? "v" : (const char *)u8"\u253b";
 case LineChar::LabelVert:
+  return IsASCII ? "|" : (const char *)u8"\u2502";

jhenderson wrote:
> This seems unrelated to comparison checking?
> This seems unrelated to comparison checking?

It is unrelated. But In C++20, `u8` literals become their own type so this no 
longer compiled and I wanted to ensure that I could actually run the tests. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-07 Thread Barry Revzin via Phabricator via cfe-commits

BRevzin added a comment.

In D78938#2258557 , @jhenderson wrote:

> Not that I have anything particularly against this, but won't this likely rot 
> fairly rapidly? It's not like LLVM is even on C++17 let alone C++20 yet, so 
> trying to make it work like the latter when it's just going to break again 
> seems a bit like wasted effort to me.

People will want to write C++20 programs that use LLVM headers, so I think it's 
important to help let them do that. Sure, it may rot, but incremental fixes 
down the line will be smaller.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits

martong created this revision.
martong added reviewers: Szelethus, balazske, NoQ, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a project: clang.
martong requested review of this revision.

There are 2 reasons to remove strcasecmp and strncasecmp.

1. They are also modeled in CStringChecker and the related argumentum 
contraints are checked there.
2. The argument constraints are checked in CStringChecker::evalCall. This is 
fundamentally flawed, they should be checked in checkPreCall. Even if we set up 
CStringChecker as a weak dependency for StdLibraryFunctionsChecker then the 
latter reports the warning always. Besides, CStringChecker fails to discover 
the constraint violation before the call, so, its evalCall returns with `true` 
and then StdCLibraryFunctions also tries to evaluate, this causes an assertion 
in CheckerManager.

Either we fix CStringChecker to handle the call prerequisites in
checkPreCall, or we must not evaluate any pure functions in
StdCLibraryFunctions that are also handled in CStringChecker.
We do the latter in this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87239

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c


Index: clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
@@ -0,0 +1,21 @@
+// This test case crashes if strncasecmp is modeled in StdCLibraryFunctions.
+// Either we fix CStringChecker to handle the call prerequisites in
+// checkPreCall, or we must not evaluate any pure functions in
+// StdCLibraryFunctions that are also handled in CStringChecker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=unix.cstring.NullArg \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify
+
+typedef __typeof(sizeof(int)) size_t;
+int strncasecmp(const char *s1, const char *s2, size_t n);
+
+int strncasecmp_null_argument(char *a, size_t n) {
+  char *b = 0;
+  return strncasecmp(a, b, n); // expected-warning{{Null pointer passed as 2nd 
argument to string comparison function}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1674,22 +1674,6 @@
   RetType{IntTy}, NoEvalCall)
   .ArgConstraint(NotNull(ArgNo(0;
 
-// int strcasecmp(const char *s1, const char *s2);
-addToFunctionSummaryMap("strcasecmp",
-Summary(ArgTypes{ConstCharPtrTy, ConstCharPtrTy},
-RetType{IntTy}, EvalCallAsPure)
-.ArgConstraint(NotNull(ArgNo(0)))
-.ArgConstraint(NotNull(ArgNo(1;
-
-// int strncasecmp(const char *s1, const char *s2, size_t n);
-addToFunctionSummaryMap(
-"strncasecmp", Summary(ArgTypes{ConstCharPtrTy, ConstCharPtrTy, 
SizeTy},
-   RetType{IntTy}, EvalCallAsPure)
-   .ArgConstraint(NotNull(ArgNo(0)))
-   .ArgConstraint(NotNull(ArgNo(1)))
-   .ArgConstraint(ArgumentCondition(
-   2, WithinRange, Range(0, SizeMax;
-
 // int fileno(FILE *stream);
 addToFunctionSummaryMap(
 "fileno", Summary(ArgTypes{FilePtrTy}, RetType{IntTy}, NoEvalCall)


Index: clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
@@ -0,0 +1,21 @@
+// This test case crashes if strncasecmp is modeled in StdCLibraryFunctions.
+// Either we fix CStringChecker to handle the call prerequisites in
+// checkPreCall, or we must not evaluate any pure functions in
+// StdCLibraryFunctions that are also handled in CStringChecker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=unix.cstring.NullArg \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:

[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

2020-09-07 Thread Jake Merdich via Phabricator via cfe-commits

JakeMerdichAMD added a comment.

I can see the use of this, but I am also wary that ignoring style options will 
lead to people producing different results on different versions of 
clang-format. This is both because having set-or-unset an option will naturally 
lead to different code and also that newer options are a de-facto check that 
clang-format is at least a certain version (we have minor differences between 
major versions as bugs are fixed). In any case, I see this being *very* easy to 
misuse and the documentation should have a warning reflecting that.

As far as docs go, the bulk are in clang/docs/ClangFormat.rst and require no 
additional 'publish' step but definitely should be updated. The command line 
options are implicitly generated for the cli tool's --help flag, but the docs 
for them are all in that rst file and manually maintained.

Comment at: clang/tools/clang-format/ClangFormat.cpp:108
+static cl::opt
+IgnoreUnkownOptions("ignore-unknown-options",
+cl::desc("If set, unknown format options are 
ignored."),

fodinabor wrote:
> MyDeveloperDay wrote:
> > feels like a mouthful is there nothing shorter we could use?  -Wignore (or 
> > something)
> hmm... `-Wunknown`
> but the `-W` does not really make it clear that the default "errors" should 
> now be treated as warnings instead. From compiler conventions, I'd expect the 
> `-W` to enable a warning ... 
> 
> and something like `-Wno-error=unknown` is not really shorter...
I personally like -Wno-error=unknown if the behavior is to emit warnings and 
-Wno-unknown if the behavior is to be silent, for consistency with clang/gcc.

I would also put a note in the description saying that ignoring options can 
lead to dramatically different output between versions that do and don't 
support a given option, so it should be used with care.

Comment at: llvm/lib/Support/YAMLTraits.cpp:199
+  if (IgnoreUnkown)
+return;
   for (const auto  : MN->Mapping) {

grimar wrote:
> fodinabor wrote:
> > MyDeveloperDay wrote:
> > > do we want to flat out ignore or just report but not fatally. (just a 
> > > thought) silent failures are hard to diagnose
> > true.. don't know what's the best option?
> > 
> > keep it as a printed out error and just don't return an error code on exit? 
> > This option would make it a clang-format only change, but feels really 
> > dirty.
> > 
> > Otherwise I'd have to dig my way through to 
> > `llvm::yaml::Stream::printError` (or maybe rather add a `printWarning`) to 
> > conditionally change the message type for the ignore case.
> Yes, I think we might want to introduce a method, like `Input::reportWarning`
> which will call ` Strm->printError(node, message);`, but will not set a `EC`.
> Also, it should print "warning: ..." instead of "error: ..." prefix somehow.
> 
Something like how clang/gcc only report unknown -Wno-foo options if there's 
another error is an idea, but clang-format almost never fails unless there's a 
bad config or input file so that's not too useful.

I'm fine with either warning or being silent. If the user has opted-into 
ignoring missing options, we can assume they're willing to accept the 
consequences of such.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Perfectly clear, thank you. However, I would still rely on the others to accept 
this :|

BTW why does the `plist-macros-with-expansion.cpp.plist` change? It makes the 
diff somewhat noisy :s


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

https://reviews.llvm.org/D86135

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

I have checked only your test, but the readability of the reports should be 
improved.
You frequently refer to previous events, such as `This lock has already been 
unlocked`, `This lock has already been acquired`, etc.
It isn't clear to the reader where do you refer to. IMO you should put a 
//NoteTag// at the interesting locations to achieve more readable diagnostics.

Such as:

  void stms_bad2() {
stm1.lock();// expected-note {{Previously locked here}}
stm1.lock_shared(); // expected-warning {{This lock has already been 
acquired}}
  }

  void stm_bad3() {
stm1.lock();   // hmm, might be a good idea to put one note here too
stm2.lock();   // expected-note {{Previously locked mutex}}
stm1.unlock(); // expected-warning {{This was not the most recently 
acquired lock. Possible lock order reversal}}
stm2.unlock(); // no-warning
  }


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

https://reviews.llvm.org/D85984

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

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Ping @OikawaKirie .
How should we proceed?
I would happily participate in creating a minimal repro for this, but I need at 
least one crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660

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

[PATCH] D83088: Introduce CfgTraits abstraction

2020-09-07 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle added inline comments.



Comment at: llvm/include/llvm/Support/CfgTraits.h:51
+
+  operator bool() const { return ptr != nullptr; }
+

dblaikie wrote:
> `operator bool` should be `explicit`
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:53-54
+
+  bool operator==(CfgOpaqueType other) const { return ptr == other.ptr; }
+  bool operator!=(CfgOpaqueType other) const { return ptr != other.ptr; }
+};

dblaikie wrote:
> Preferably make any operator overload that can be a non-member, a non-member 
> - this ensures equal conversion handling on both the left and right hand side 
> of symmetric operators like these. (they can be friends if needed, but 
> doesn't look like it in this case - non-friend, non-members that call get() 
> should be fine here)
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:90
+/// operations such as traversal of the CFG.
+class CfgTraitsBase {
+protected:

dblaikie wrote:
> Not sure if this benefits from being inherited from, versus being freely 
> accessible?
The idea here is to enforce via the type system that people use 
CfgTraits::{wrap,unwrap}Ref instead of having makeOpaque calls freely in random 
code.



Comment at: llvm/include/llvm/Support/CfgTraits.h:271-273
+template  struct CfgTraitsFor {
+  // using CfgTraits = ...;
+};

dblaikie wrote:
> This probably shouldn't be defined if it's only needed for specialization,  
> instead it can be declared:
> ```
> template struct CfgTraitsFor;
> ```
Interesting. So GraphTraits should arguably be changed similarly.



Comment at: llvm/include/llvm/Support/CfgTraits.h:287
+public:
+  virtual ~CfgInterface() {}
+

dblaikie wrote:
> prefer `= default` where possible
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:337
+return Printable(
+[this, block](raw_ostream ) { printBlockName(out, block); });
+  }

dblaikie wrote:
> generally capture everything by ref `[&]` if the lambda is only used 
> locally/within the same expression or block
The lambda is returned via `Printable`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D86874#2259105 , @Szelethus wrote:

> I can only imagine how long it took for you to write all this, because 
> reading it wasn't that short either. Thank you so much! It really gave me a 
> perspective on how you see this problem, as well as what is actually 
> happening (and should happen) in the checker.

Thank you very much. I tied to make it as short as possible while keeping all 
the necessary details to make my reasoning perfectly clean. Now I think I was 
on a bad track, but @martong helped me to pinpoint some issues.

> In D86874#2254638 , @steakhal wrote:
>
>>   void foo(int x) {
>> int buf[10];
>>   
>> char *r = (char*)(buf + x) + 3;
>> *r = 66; // Might be safe, if x has a value to make it so.
>>   }
>
> I suppose you mean that it might be unsafe on the grounds of out-of-bounds 
> addressing, but alignment-wise its okay?

You can safely access any valid memory region via `char`, `unsigned char` and 
`std::byte` pointers. And as integers are using all bits in their 
representation, I suspect that this code snippet is safe - given that the 
pointer points to a valid region.
(aka. no out-of-bound access happens). Alignment does not play any role here.

> In D86874#2256249 , @martong wrote:
>
>>>   The resulting `RegionRawOffsetV2` for `p`, `q` will be:
>>>   p: {BaseRegion: `buf`, ByteOffset: `20 S64b`}
>>>   q: {BaseRegion: `buf`, ByteOffset: `(((reg_$0) + 1) * 12) + 24`}
>>> ^^^  ^^
>>>  |   |
>>>   `@a` This is an //object-language// expression. --/|
>>> /
>>>   `@b` The rest should be //meta-language// expression. ---/
>>>
>>> SPOILER: This distinction is really important here.
>>>
>>> So, we made an expression, we should not evaluate in either of the worlds.
>>> We should not evaluate it using the semantics of the //object-language// 
>>> since only the `@a` is of that world.
>>> We should model overflow in `@a` if that expression is of //unsigned// type 
>>> (OR signed but `-fwrapv`...) etc. according to the //abstract machine//.
>>> BUT modeling the possibility of an overflow of the rest of the expression 
>>> (`@b`) would be a flaw.
>>
>> Why? I'd expect that the value of the whole expression `@a@b` could overflow.
>
> That wasn't all too clear to me either. It seems like there is extra work to 
> do if we do the calculation according to C++ rules, but not the other way 
> around, so we could just regard everything as if it was under the 
> object-language ruleset.

Let's forget it. My statements in this regard are going in a bad direction. 
Sorry for the dead-end, I thought I was on the right track.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86874

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

[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

2020-09-07 Thread George Rimar via Phabricator via cfe-commits

grimar added a comment.

I am not familar with `clang-format`, but have a few comments inlined about the 
rest.
I think the new `setIgnoreUnknown` YAMLlib API is probably OK generally.
I'd perhaps call it differently, e.g. `setAllowUnknownKeys` though.

Also, I think you need to add a unit testing for the new functionality.
It seems appropriate places are: `clang\unittests\Format\FormatTest.cpp` (to 
test clang-format)
and `llvm\unittests\ObjectYAML\YAMLTest.cpp` (to test new YAML 
`Input::setIgnoreUnknown` API)

I'll add people who might have something useful to say too.

Comment at: llvm/include/llvm/Support/YAMLTraits.h:1508

+  void setIgnoreUnknown(bool) override;
+

I'd add a parameter name here. Seems the style is mixed in this file,
but it is a public member, and having no names for arguments actually
reads bad I think. Not sure why it was done initially.

Probably the same applies for `setIgnoreUnknown` in the base class.

Comment at: llvm/lib/Support/YAMLTraits.cpp:199
+  if (IgnoreUnkown)
+return;
   for (const auto  : MN->Mapping) {

fodinabor wrote:
> MyDeveloperDay wrote:
> > do we want to flat out ignore or just report but not fatally. (just a 
> > thought) silent failures are hard to diagnose
> true.. don't know what's the best option?
> 
> keep it as a printed out error and just don't return an error code on exit? 
> This option would make it a clang-format only change, but feels really dirty.
> 
> Otherwise I'd have to dig my way through to `llvm::yaml::Stream::printError` 
> (or maybe rather add a `printWarning`) to conditionally change the message 
> type for the ignore case.
Yes, I think we might want to introduce a method, like `Input::reportWarning`
which will call ` Strm->printError(node, message);`, but will not set a `EC`.
Also, it should print "warning: ..." instead of "error: ..." prefix somehow.

Comment at: llvm/lib/Support/YAMLTraits.cpp:742

+void Output::setIgnoreUnknown(bool Value) {}
+

You don't actually need it for `Output`, right?
I think instead you can have a default implementation in the base class, which 
should call `llvm_unreachable`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

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

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

2020-09-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D86874#2256249 , @martong wrote:

>> Calculation of the RegionRawOffsetV2
>> 
>>
>> Let's see how does these subscript expressions used during the calculation 
>> of the `RegionRawOffsetV2`:
>> For multi-dimension arrays we //fold// the index expressions of the nested 
>> `ElementRegion`s.
>> We are doing that by simply calculating the byte-offset of the nested 
>> region, and adding the current level's //index*sizeof(Type)//.
>> So, the `ByteOffset` that we build up will contain mostly multiplications by 
>> a constant OR additions of symbolic expressions (like the `x+1` in the 
>> example).
>
> We have these lines in the case of handling the ElementRegion:
>
>   if (!index.getAs())
> return RegionRawOffsetV2();
>
> So, if the index is symbolic we give up, don't we? So, how could this work 
> with `x+1`? What am I missing here?

In my example, I'm speaking about the case when it's `nonloc::SymbolVal` 
wrapping the expression `x+1`. So not every `NonLoc` is `ConcreteInt`.

>>   The resulting `RegionRawOffsetV2` for `p`, `q` will be:
>>   p: {BaseRegion: `buf`, ByteOffset: `20 S64b`}
>>   q: {BaseRegion: `buf`, ByteOffset: `(((reg_$0) + 1) * 12) + 24`}
>> ^^^  ^^
>>  |   |
>>   `@a` This is an //object-language// expression. --/|
>> /
>>   `@b` The rest should be //meta-language// expression. ---/
>>
>> SPOILER: This distinction is really important here.
>>
>> So, we made an expression, we should not evaluate in either of the worlds.
>> We should not evaluate it using the semantics of the //object-language// 
>> since only the `@a` is of that world.
>> We should model overflow in `@a` if that expression is of //unsigned// type 
>> (OR signed but `-fwrapv`...) etc. according to the //abstract machine//.
>> BUT modeling the possibility of an overflow of the rest of the expression 
>> (`@b`) would be a flaw.
>
> Why? I'd expect that the value of the whole expression `@a@b` could overflow.

Hm, you might be right about that.
This //simplification// (over the plus operator) is only valid if no overflow 
can happen.
We could add the necessary constraints during this constant folding operation 
such as:
 Simplifying the expression `x+1 < 0` into `x<-1` //assuming that// `x <= 
numeric_limit_max(typeof(x)) - 1`
We could repeat this, while we have a //state// where all such assumptions are 
true.
I'm looking forward to implementing it.

>> Simplify step, again
>> 
>>
>>   Simplification of the `(((reg_$0) + 1) * 12) + 24` < `0` 
>> inequality...
>> ^^
>>`@b`
>>   Result:  `reg_$0 < -3 S64b`
>> ^   ^^^
>>   `RootSym` --/|
>>   /
>>   `C'` --/
>>
>> `#1`: This step supposed to fold **ONLY** the //meta-language// expression 
>> parts (`@b`) of the previously aquired `ByteOffset`.
>> `#2`: This step requires the expression under simplified to be of 
>> //meta-language// to be able to reorder the constants. (aka. to fold into 
>> the right hand side's constant).
>> `#3`: This also requires the right-hand side to be of the //meta-language//.
>
> Do I understand this correctly, that none of the binary operations can have a 
> symbolic RHS, because that would mean we have a VLA?

I think yes, you are right.
However, as I tried to build an example demonstrating this, it turned out that 
it does not handle VLAs quite well.

  void foo(int x, int y) {
int buf[x][y]; // VLA
clang_analyzer_dump([1][2]);
// {Element{buf,1 S64b,int [y]},2 S64b,int}
  
clang_analyzer_DumpRegionRawOffsetV2AndSimplify([1][2]);
// RawOffsetV2: {BaseRegion: buf, ByteOffset: 8 S64b}
// simplification result: {RootSymbol: 8 S64b, FoldedConstant: 0 S64b)
  }

The RawOffsetV2 value is wrong. It supposed to have the ByteOffset: `(1 * 
sizeof(int[y])) + 8`.

>> We treat the complete expression under //simplification// as an expression 
>> of the //meta-language//.
>> I'm not changing this, but I probably should though.
>
> Again, I don't understand why you are sure that the value of //whole// 
> expression cannot overflow.
>
>> Ideally, after //simplification// we should have this inequality: `x+1 < -2`
>> That way we would not fold any subexpression of the //object-language//, so 
>> the `#1` requirement would be preserved.
>> The right-hand side is of an expression of the //meta-language//.
>> It makes sense, to evaluate the `operator<` as the semantics of the 
>> //meta-language//.
>> But the left-hand side is an expression of the //object-language//.
>> We need some sort of //logical// conversion here.
>

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

2020-09-07 Thread Dmitry Antipov via Phabricator via cfe-commits

dmantipov updated this revision to Diff 290281.
dmantipov added a comment.

Add trivial {/etc,/usr/lib}/os-release parser and fix tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87187

Files:
  clang/include/clang/Driver/Distro.h
  clang/lib/Driver/Distro.cpp

Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -19,30 +19,42 @@
 using namespace clang::driver;
 using namespace clang;
 
-static Distro::DistroType DetectDistro(llvm::vfs::FileSystem ,
-   const llvm::Triple ) {
-  // If we don't target Linux, no need to check the distro. This saves a few
-  // OS calls.
-  if (!TargetOrHost.isOSLinux())
-return Distro::UnknownDistro;
+static Distro::DistroType DetectOsRelease(llvm::vfs::FileSystem ) {
+  Distro::DistroType Version = Distro::UnknownDistro;
+  llvm::ErrorOr> File =
+  VFS.getBufferForFile("/etc/os-release");
+  if (!File)
+File = VFS.getBufferForFile("/usr/lib/os-release");
 
-  // If the host is not running Linux, and we're backed by a real file system,
-  // no need to check the distro. This is the case where someone is
-  // cross-compiling from BSD or Windows to Linux, and it would be meaningless
-  // to try to figure out the "distro" of the non-Linux host.
-  IntrusiveRefCntPtr RealFS =
-  llvm::vfs::getRealFileSystem();
-  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
-  if (!HostTriple.isOSLinux() &&  == RealFS.get())
-return Distro::UnknownDistro;
+  if (File) {
+SmallVector Lines;
+File.get()->getBuffer().split(Lines, "\n");
 
+// Obviously this can be improved a lot.
+for (StringRef Line : Lines)
+  if (Version == Distro::UnknownDistro && Line.startswith("ID="))
+Version = llvm::StringSwitch(Line.substr(3))
+  .Case("fedora", Distro::Fedora)
+  .Case("gentoo", Distro::Gentoo)
+  .Case("arch", Distro::ArchLinux)
+  // On SLES, /etc/os-release was introduced in SLES 11.
+  .Case("sles", Distro::OpenSUSE)
+  .Case("opensuse", Distro::OpenSUSE)
+  .Case("exherbo", Distro::Exherbo)
+  .Default(Distro::UnknownDistro);
+  }
+  return Version;
+}
+
+static Distro::DistroType DetectLsbRelease(llvm::vfs::FileSystem ) {
+  Distro::DistroType Version = Distro::UnknownDistro;
   llvm::ErrorOr> File =
   VFS.getBufferForFile("/etc/lsb-release");
+
   if (File) {
-StringRef Data = File.get()->getBuffer();
 SmallVector Lines;
-Data.split(Lines, "\n");
-Distro::DistroType Version = Distro::UnknownDistro;
+File.get()->getBuffer().split(Lines, "\n");
+
 for (StringRef Line : Lines)
   if (Version == Distro::UnknownDistro && Line.startswith("DISTRIB_CODENAME="))
 Version = llvm::StringSwitch(Line.substr(17))
@@ -73,11 +85,28 @@
   .Case("focal", Distro::UbuntuFocal)
   .Case("groovy", Distro::UbuntuGroovy)
   .Default(Distro::UnknownDistro);
-if (Version != Distro::UnknownDistro)
-  return Version;
   }
+  return Version;
+}
+
+static Distro::DistroType DetectDistro(llvm::vfs::FileSystem ) {
+  Distro::DistroType Version = Distro::UnknownDistro;
+
+  // Newer freedesktop.org's compilant systemd-based systems
+  // should provide /etc/os-release or /usr/lib/os-release.
+  Version = DetectOsRelease(VFS);
+  if (Version != Distro::UnknownDistro)
+return Version;
+
+  // Older systems might provide /etc/lsb-release.
+  Version = DetectLsbRelease(VFS);
+  if (Version != Distro::UnknownDistro)
+return Version;
+
+  // Otherwise try some distro-specific quirks for RedHat...
+  llvm::ErrorOr> File
+= VFS.getBufferForFile("/etc/redhat-release");
 
-  File = VFS.getBufferForFile("/etc/redhat-release");
   if (File) {
 StringRef Data = File.get()->getBuffer();
 if (Data.startswith("Fedora release"))
@@ -95,6 +124,7 @@
 return Distro::UnknownDistro;
   }
 
+  // ...for Debian
   File = VFS.getBufferForFile("/etc/debian_version");
   if (File) {
 StringRef Data = File.get()->getBuffer();
@@ -130,6 +160,7 @@
 .Default(Distro::UnknownDistro);
   }
 
+  // ...for SUSE
   File = VFS.getBufferForFile("/etc/SuSE-release");
   if (File) {
 StringRef Data = File.get()->getBuffer();
@@ -153,6 +184,7 @@
 return Distro::UnknownDistro;
   }
 
+  // ...and others.
   if (VFS.exists("/etc/exherbo-release"))
 return Distro::Exherbo;
 
@@ -168,5 +200,37 @@
   return Distro::UnknownDistro;
 }
 
+static Distro::DistroType GetDistro(llvm::vfs::FileSystem ,
+const llvm::Triple ) {
+  static Distro::DistroType Type = Distro::UninitializedDistro;
+
+  // If we don't target Linux, no need to

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus updated this revision to Diff 290280.
Szelethus marked 4 inline comments as done.
Szelethus added a comment.

Added a line about D78933 .


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

https://reviews.llvm.org/D86533

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -437,10 +437,77 @@
 
 - ...
 
+.. _release-notes-clang-static-analyzer:
+
 Static Analyzer
 ---
 
-- ...
+- Improved the analyzer's understanding of inherited C++ constructors.
+
+- Improved the analyzer's understanding of dynamic class method dispatching in
+  Objective-C.
+
+- Greatly improved the analyzer's constraint solver by better understanding
+  when constraints are imposed on multiple symbolic values that are known to be
+  equal or known to be non-equal. It will now also efficiently reject 
impossible
+  if-branches between known comparison expressions.
+
+- Added :ref:`on-demand parsing ` capability to Cross 
Translation
+  Unit (CTU) analysis.
+
+- Numerous fixes and improvements for the HTML output.
+
+- New checker: :ref:`alpha.core.C11Lock ` and
+  :ref:`alpha.fuchsia.Lock ` checks for their respective
+  locking APIs.
+
+- New checker: :ref:`alpha.security.cert.pos.34c `
+  finds calls to ``putenv`` where a pointer to an autmoatic variable is passed
+  as an argument.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` to enforce the existence of virtual
+  destructors for all uncounted types used as base classes.
+
+- New checker: :ref:`webkit.RefCntblBaseVirtualDtor
+  ` checks that only ref-counted types
+  are used as class members, not raw pointers and references to uncounted
+  types.
+
+- New checker: :ref:`alpha.cplusplus.SmartPtr ` check
+  for dereference of null smart pointers.
+
+- Moved ``PlacementNewChecker`` out of alpha, making it enabled by default.
+
+- Added support for multi-dimensional variadic arrays in ``core.VLASize``.
+
+- Added a check for insufficient storage in array placement new calls, as well
+  as support for alignment variants to ``cplusplus.PlacementNew``.
+
+- While still in alpha, ``alpha.unix.PthreadLock``, the iterator and container
+  modeling infrastructure, ``alpha.unix.Stream``, and taint analysis were
+  improved greatly. An ongoing, currently off-by-default improvement was made 
on
+  the pre-condition modeling of several functions defined in the POSIX 
standard.
+
+- Improved the warning messages of several checkers.
+
+- Fixed a few remaining cases of checkers emitting reports under incorrect
+  checker names, and employed a few restrictions to more easily identify and
+  avoid such errors.
+
+- Moved several non-reporting checkers (those that model, among other things,
+  standard functions, but emit no diagnostics) to be listed under
+  ``-analyzer-checker-help-developer`` instead of ``-analyzer-checker-help``.
+  Manually enabling or disabling checkers found on this list is not supported
+  in production.
+
+- Numerous fixes for crashes, false positives and false negatives in
+  ``unix.Malloc``, ``osx.cocoa.NSError``, and several other checkers.
+
+- Implemented a dockerized testing system to more easily determine the
+  correctness and performance impact of a change to the static analyzer itself.
+  The currently beta-version tool is found in
+  ``(llvm-project repository)/clang/utils/analyzer/SATest.py``.
 
 .. _release-notes-ubsan:
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -437,10 +437,77 @@
 
 - ...
 
+.. _release-notes-clang-static-analyzer:
+
 Static Analyzer
 ---
 
-- ...
+- Improved the analyzer's understanding of inherited C++ constructors.
+
+- Improved the analyzer's understanding of dynamic class method dispatching in
+  Objective-C.
+
+- Greatly improved the analyzer's constraint solver by better understanding
+  when constraints are imposed on multiple symbolic values that are known to be
+  equal or known to be non-equal. It will now also efficiently reject impossible
+  if-branches between known comparison expressions.
+
+- Added :ref:`on-demand parsing ` capability to Cross Translation
+  Unit (CTU) analysis.
+
+- Numerous fixes and improvements for the HTML output.
+
+- New checker: :ref:`alpha.core.C11Lock ` and
+  :ref:`alpha.fuchsia.Lock ` checks for their respective
+  locking APIs.
+
+- New checker: :ref:`alpha.security.cert.pos.34c `
+  finds calls to ``putenv`` where a pointer to an autmoatic variable is passed
+  as an argument.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` to enforce the existence of virtual
+  destructors for all uncounted types used as base classes.
+
+- New checker: :ref:`webkit.RefCntblBaseVirtualDtor
+  ` checks that only ref-counted

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:453
+  equal or known to be non-equal.
+
+- Added :ref:`on-demand parsing ` capability to Cross 
Translation

ASDenysPetrov wrote:
> I've added the patch "Reasoning about comparison expressions in 
> RangeConstraintManager". You can mention that as well.
I added the line you mentioned -- do you like how the notes look now?


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

https://reviews.llvm.org/D86533

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

[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

2020-09-07 Thread Joachim Meyer via Phabricator via cfe-commits

fodinabor added a subscriber: grimar.
fodinabor added a comment.

Thank you so far for the feedback!
maybe you can give further guidance on the comments on the comments :)

As of the git history it seems that @grimar did some work on YAML error 
handling..

Comment at: clang/tools/clang-format/ClangFormat.cpp:108
+static cl::opt
+IgnoreUnkownOptions("ignore-unknown-options",
+cl::desc("If set, unknown format options are 
ignored."),

MyDeveloperDay wrote:
> feels like a mouthful is there nothing shorter we could use?  -Wignore (or 
> something)
hmm... `-Wunknown`
but the `-W` does not really make it clear that the default "errors" should now 
be treated as warnings instead. From compiler conventions, I'd expect the `-W` 
to enable a warning ... 

and something like `-Wno-error=unknown` is not really shorter...

Comment at: llvm/include/llvm/Support/YAMLTraits.h:1520
   boolScalarMatchFound = false;
+  bool IgnoreUnkown = false;
 };

MyDeveloperDay wrote:
> is this clang-formatted?
the patch is... the original alignment of the members is not. (also see the 
`CurrentNode`'s formatting).
not sure what to do in this case, as the whole file seems rather unformatted?

Comment at: llvm/lib/Support/YAMLTraits.cpp:199
+  if (IgnoreUnkown)
+return;
   for (const auto  : MN->Mapping) {

MyDeveloperDay wrote:
> do we want to flat out ignore or just report but not fatally. (just a 
> thought) silent failures are hard to diagnose
true.. don't know what's the best option?

keep it as a printed out error and just don't return an error code on exit? 
This option would make it a clang-format only change, but feels really dirty.

Otherwise I'd have to dig my way through to `llvm::yaml::Stream::printError` 
(or maybe rather add a `printWarning`) to conditionally change the message type 
for the ignore case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-09-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits

ostannard added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:396
+/// according to the field declaring type width.
+CODEGENOPT(ForceNoAAPCSBitfieldWidth, 1, 0)
+

dnsampaio wrote:
> ostannard wrote:
> > Why is this a negative option, when the one above is positive?
> The enforcing of number of accesses would not be accepted if it was not an 
> opt-in option. This one I expect it should be accepted with a single opt-out 
> option.
My problem is with the name of the option (adding an extra negative just makes 
things more confusing), not with the default value. This could just be called 
`AAPCSBitfieldWidth`, (unless you think the `Force` is adding something), and 
default to true father than false.



Comment at: clang/include/clang/Driver/Options.td:2328
   HelpText<"Follows the AAPCS standard that all volatile bit-field write 
generates at least one load. (ARM only).">;
+def ForceNoAAPCSBitfieldWidth : Flag<["-"], "fno-AAPCSBitfieldWidth">, 
Group,
+  Flags<[DriverOption,CC1Option]>,

ostannard wrote:
> Command-line options are in kebab-case, so this should be something like 
> `fno-aapcs-bitfield-width`. This also applies to the `fAAPCSBitfieldLoad` 
> option above, assuming it's not too late to change that.
> 
> Please also add a positive version of this option (i.e. 
> `faapcs-bitfield-width`).
This still needs a positive version.



Comment at: clang/lib/CodeGen/CGRecordLayout.h:83
 
+  /// The offset within a contiguous run of bitfields that are represented as
+  /// a single "field" within the LLVM struct type. This offset is in bits.

These doc comments are copied from the ones above, they need changing.



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:279
+lowerUnion();
+return computeVolatileBitfields();
+  }

Returning `void` is confusing (yes I know it was already there), this should be 
a separate `return;` statement.



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:288
+  appendPaddingBytes(Size);
+  return computeVolatileBitfields();
+}

Same here.



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:535
+///
+/// Enforcing the width restriction can be disable using
+/// -fno-aapcs-bitfield-width.

s/disable/disabled/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

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

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

I can only imagine how long it took for you to write all this, because reading 
it wasn't that short either. Thank you so much! It really gave me a perspective 
on how you see this problem, as well as what is actually happening (and should 
happen) in the checker.

In D86874#2254638 , @steakhal wrote:

>   void foo(int x) {
> int buf[10];
>   
> char *r = (char*)(buf + x) + 3;
> *r = 66; // Might be safe, if x has a value to make it so.
>   }

I suppose you mean that it might be unsafe on the grounds of out-of-bounds 
addressing, but alignment-wise its okay?

In D86874#2256249 , @martong wrote:

>>   The resulting `RegionRawOffsetV2` for `p`, `q` will be:
>>   p: {BaseRegion: `buf`, ByteOffset: `20 S64b`}
>>   q: {BaseRegion: `buf`, ByteOffset: `(((reg_$0) + 1) * 12) + 24`}
>> ^^^  ^^
>>  |   |
>>   `@a` This is an //object-language// expression. --/|
>> /
>>   `@b` The rest should be //meta-language// expression. ---/
>>
>> SPOILER: This distinction is really important here.
>>
>> So, we made an expression, we should not evaluate in either of the worlds.
>> We should not evaluate it using the semantics of the //object-language// 
>> since only the `@a` is of that world.
>> We should model overflow in `@a` if that expression is of //unsigned// type 
>> (OR signed but `-fwrapv`...) etc. according to the //abstract machine//.
>> BUT modeling the possibility of an overflow of the rest of the expression 
>> (`@b`) would be a flaw.
>
> Why? I'd expect that the value of the whole expression `@a@b` could overflow.

That wasn't all too clear to me either. It seems like there is extra work to do 
if we do the calculation according to C++ rules, but not the other way around, 
so we could just regard everything as if it was under the object-language 
ruleset.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86874

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

[PATCH] D86097: [OpenMP][AMDGCN] Generate global variables and attributes for AMDGCN

2020-09-07 Thread Saiyedul Islam via Phabricator via cfe-commits

saiislam added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86097

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

[clang] 2853ae3 - [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-07 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-09-07T13:54:12+01:00
New Revision: 2853ae3c1b8174e3660424ffac45922601f700ee

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

LOG: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

We're now getting close to having the necessary analysis/combines etc. for the 
new generic llvm.abs.* intrinsics.

This patch updates the SSE/AVX ABS vector intrinsics to emit the generic 
equivalents instead of the icmp+sub+select code pattern.

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/avx2-builtins.c
clang/test/CodeGen/avx512bw-builtins.c
clang/test/CodeGen/avx512f-builtins.c
clang/test/CodeGen/avx512vl-builtins.c
clang/test/CodeGen/avx512vlbw-builtins.c
clang/test/CodeGen/ssse3-builtins.c
llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 42fab29ab8aa..0cb8f8f636f4 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -11314,16 +11314,6 @@ static Value *EmitX86ConvertIntToFp(CodeGenFunction 
,
   return EmitX86Select(CGF, Ops[2], Res, Ops[1]);
 }
 
-static Value *EmitX86Abs(CodeGenFunction , ArrayRef Ops) {
-
-  llvm::Type *Ty = Ops[0]->getType();
-  Value *Zero = llvm::Constant::getNullValue(Ty);
-  Value *Sub = CGF.Builder.CreateSub(Zero, Ops[0]);
-  Value *Cmp = CGF.Builder.CreateICmp(ICmpInst::ICMP_SGT, Ops[0], Zero);
-  Value *Res = CGF.Builder.CreateSelect(Cmp, Ops[0], Sub);
-  return Res;
-}
-
 static Value *EmitX86MinMax(CodeGenFunction , ICmpInst::Predicate Pred,
 ArrayRef Ops) {
   Value *Cmp = CGF.Builder.CreateICmp(Pred, Ops[0], Ops[1]);
@@ -13300,9 +13290,10 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
   case X86::BI__builtin_ia32_pabsb512:
   case X86::BI__builtin_ia32_pabsw512:
   case X86::BI__builtin_ia32_pabsd512:
-  case X86::BI__builtin_ia32_pabsq512:
-return EmitX86Abs(*this, Ops);
-
+  case X86::BI__builtin_ia32_pabsq512: {
+Function *F = CGM.getIntrinsic(Intrinsic::abs, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0], Builder.getInt1(false)});
+  }
   case X86::BI__builtin_ia32_pmaxsb128:
   case X86::BI__builtin_ia32_pmaxsw128:
   case X86::BI__builtin_ia32_pmaxsd128:

diff  --git a/clang/test/CodeGen/avx2-builtins.c 
b/clang/test/CodeGen/avx2-builtins.c
index 95659895eeaf..f3de6d1b8747 100644
--- a/clang/test/CodeGen/avx2-builtins.c
+++ b/clang/test/CodeGen/avx2-builtins.c
@@ -8,25 +8,19 @@
 
 __m256i test_mm256_abs_epi8(__m256i a) {
   // CHECK-LABEL: test_mm256_abs_epi8
-  // CHECK: [[SUB:%.*]] = sub <32 x i8> zeroinitializer, %{{.*}}
-  // CHECK: [[CMP:%.*]] = icmp sgt <32 x i8> %{{.*}}, zeroinitializer
-  // CHECK: select <32 x i1> [[CMP]], <32 x i8> %{{.*}}, <32 x i8> [[SUB]]
+  // CHECK: [[ABS:%.*]] = call <32 x i8> @llvm.abs.v32i8(<32 x i8> %{{.*}}, i1 
false)
   return _mm256_abs_epi8(a);
 }
 
 __m256i test_mm256_abs_epi16(__m256i a) {
   // CHECK-LABEL: test_mm256_abs_epi16
-  // CHECK: [[SUB:%.*]] = sub <16 x i16> zeroinitializer, %{{.*}}
-  // CHECK: [[CMP:%.*]] = icmp sgt <16 x i16> %{{.*}}, zeroinitializer
-  // CHECK: select <16 x i1> [[CMP]], <16 x i16> %{{.*}}, <16 x i16> [[SUB]]
+  // CHECK: [[ABS:%.*]] = call <16 x i16> @llvm.abs.v16i16(<16 x i16> %{{.*}}, 
i1 false)
   return _mm256_abs_epi16(a);
 }
 
 __m256i test_mm256_abs_epi32(__m256i a) {
   // CHECK-LABEL: test_mm256_abs_epi32
-  // CHECK: [[SUB:%.*]] = sub <8 x i32> zeroinitializer, %{{.*}}
-  // CHECK: [[CMP:%.*]] = icmp sgt <8 x i32> %{{.*}}, zeroinitializer
-  // CHECK: select <8 x i1> [[CMP]], <8 x i32> %{{.*}}, <8 x i32> [[SUB]]
+  // CHECK: [[ABS:%.*]] = call <8 x i32> @llvm.abs.v8i32(<8 x i32> %{{.*}}, i1 
false)
   return _mm256_abs_epi32(a);
 }
 

diff  --git a/clang/test/CodeGen/avx512bw-builtins.c 
b/clang/test/CodeGen/avx512bw-builtins.c
index c08b354d9519..cc173f1a9cfe 100644
--- a/clang/test/CodeGen/avx512bw-builtins.c
+++ b/clang/test/CodeGen/avx512bw-builtins.c
@@ -878,48 +878,36 @@ __m512i test_mm512_mask_blend_epi16(__mmask32 __U, 
__m512i __A, __m512i __W) {
 }
 __m512i test_mm512_abs_epi8(__m512i __A) {
   // CHECK-LABEL: @test_mm512_abs_epi8
-  // CHECK: [[SUB:%.*]] = sub <64 x i8> zeroinitializer, [[A:%.*]]
-  // CHECK: [[CMP:%.*]] = icmp sgt <64 x i8> [[A]], zeroinitializer
-  // CHECK: select <64 x i1> [[CMP]], <64 x i8> [[A]], <64 x i8> [[SUB]]
+  // CHECK: [[ABS:%.*]] = call <64 x i8> @llvm.abs.v64i8(<64 x i8> %{{.*}}, i1 
false)
   return _mm512_abs_epi8(__A); 
 }
 __m512i test_mm512_mask_abs_epi8(__m512i __W, __mmask64 __U, __m512i __A) {
   //

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-07 Thread Simon Pilgrim 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 rG2853ae3c1b81: [X86] Update SSE/AVX ABS intrinsics to emit 
llvm.abs.* (PR46851) (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87101

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/avx2-builtins.c
  clang/test/CodeGen/avx512bw-builtins.c
  clang/test/CodeGen/avx512f-builtins.c
  clang/test/CodeGen/avx512vl-builtins.c
  clang/test/CodeGen/avx512vlbw-builtins.c
  clang/test/CodeGen/ssse3-builtins.c
  llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
  llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll

Index: llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
===
--- llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
+++ llvm/test/CodeGen/X86/ssse3-intrinsics-fast-isel.ll
@@ -19,13 +19,11 @@
 ; AVX-NEXT:vpabsb %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <16 x i8>
-  %sub = sub <16 x i8> zeroinitializer, %arg
-  %cmp = icmp sgt <16 x i8> %arg, zeroinitializer
-  %sel = select <16 x i1> %cmp, <16 x i8> %arg, <16 x i8> %sub
-  %res = bitcast <16 x i8> %sel to <2 x i64>
+  %abs = call <16 x i8> @llvm.abs.v16i8(<16 x i8> %arg, i1 false)
+  %res = bitcast <16 x i8> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <16 x i8> @llvm.x86.ssse3.pabs.b.128(<16 x i8>) nounwind readnone
+declare <16 x i8> @llvm.abs.v16i8(<16 x i8>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_abs_epi16(<2 x i64> %a0) {
 ; SSE-LABEL: test_mm_abs_epi16:
@@ -38,13 +36,11 @@
 ; AVX-NEXT:vpabsw %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <8 x i16>
-  %sub = sub <8 x i16> zeroinitializer, %arg
-  %cmp = icmp sgt <8 x i16> %arg, zeroinitializer
-  %sel = select <8 x i1> %cmp, <8 x i16> %arg, <8 x i16> %sub
-  %res = bitcast <8 x i16> %sel to <2 x i64>
+  %abs = call <8 x i16> @llvm.abs.v8i16(<8 x i16> %arg, i1 false)
+  %res = bitcast <8 x i16> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <8 x i16> @llvm.x86.ssse3.pabs.w.128(<8 x i16>) nounwind readnone
+declare <8 x i16> @llvm.abs.v8i16(<8 x i16>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_abs_epi32(<2 x i64> %a0) {
 ; SSE-LABEL: test_mm_abs_epi32:
@@ -57,13 +53,11 @@
 ; AVX-NEXT:vpabsd %xmm0, %xmm0
 ; AVX-NEXT:ret{{[l|q]}}
   %arg = bitcast <2 x i64> %a0 to <4 x i32>
-  %sub = sub <4 x i32> zeroinitializer, %arg
-  %cmp = icmp sgt <4 x i32> %arg, zeroinitializer
-  %sel = select <4 x i1> %cmp, <4 x i32> %arg, <4 x i32> %sub
-  %res = bitcast <4 x i32> %sel to <2 x i64>
+  %abs = call <4 x i32> @llvm.abs.v4i32(<4 x i32> %arg, i1 false)
+  %res = bitcast <4 x i32> %abs to <2 x i64>
   ret <2 x i64> %res
 }
-declare <4 x i32> @llvm.x86.ssse3.pabs.d.128(<4 x i32>) nounwind readnone
+declare <4 x i32> @llvm.abs.v4i32(<4 x i32>, i1) nounwind readnone
 
 define <2 x i64> @test_mm_alignr_epi8(<2 x i64> %a0, <2 x i64> %a1) {
 ; SSE-LABEL: test_mm_alignr_epi8:
Index: llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
===
--- llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
+++ llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
@@ -10,13 +10,11 @@
 ; CHECK-NEXT:vpabsb %ymm0, %ymm0
 ; CHECK-NEXT:ret{{[l|q]}}
   %arg = bitcast <4 x i64> %a0 to <32 x i8>
-  %sub = sub <32 x i8> zeroinitializer, %arg
-  %cmp = icmp sgt <32 x i8> %arg, zeroinitializer
-  %sel = select <32 x i1> %cmp, <32 x i8> %arg, <32 x i8> %sub
-  %res = bitcast <32 x i8> %sel to <4 x i64>
+  %abs = call <32 x i8> @llvm.abs.v32i8(<32 x i8> %arg, i1 false)
+  %res = bitcast <32 x i8> %abs to <4 x i64>
   ret <4 x i64> %res
 }
-declare <32 x i8> @llvm.x86.avx2.pabs.b(<32 x i8>) nounwind readnone
+declare <32 x i8> @llvm.abs.v32i8(<32 x i8>, i1) nounwind readnone
 
 define <4 x i64> @test_mm256_abs_epi16(<4 x i64> %a0) {
 ; CHECK-LABEL: test_mm256_abs_epi16:
@@ -24,13 +22,11 @@
 ; CHECK-NEXT:vpabsw %ymm0, %ymm0
 ; CHECK-NEXT:ret{{[l|q]}}
   %arg = bitcast <4 x i64> %a0 to <16 x i16>
-  %sub = sub <16 x i16> zeroinitializer, %arg
-  %cmp = icmp sgt <16 x i16> %arg, zeroinitializer
-  %sel = select <16 x i1> %cmp, <16 x i16> %arg, <16 x i16> %sub
-  %res = bitcast <16 x i16> %sel to <4 x i64>
+  %abs = call <16 x i16> @llvm.abs.v16i16(<16 x i16> %arg, i1 false)
+  %res = bitcast <16 x i16> %abs to <4 x i64>
   ret <4 x i64> %res
 }
-declare <16 x i16> @llvm.x86.avx2.pabs.w(<16 x i16>) nounwind readnone
+declare <16 x i16> @llvm.abs.v16i16(<16 x i16>, i1) nounwind readnone
 
 define <4 x i64> @test_mm256_abs_epi32(<4 x i64> %a0) {
 ; CHECK-LABEL: test_mm256_abs_epi32:
@@ -38,13 +34,11 @@
 ; CHECK-NEXT:vpabsd %ymm0, %ymm0
 ; CHECK-NEXT:ret{{[l|q]}}
   %arg = bitcast <4 x i64> %a0 to <8 x i32>
-  %sub = sub <8 x i32>

[clang] 23f700c - Revert "[clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations."

2020-09-07 Thread Raphael Isemann via cfe-commits


Author: Raphael Isemann
Date: 2020-09-07T14:50:13+02:00
New Revision: 23f700c785a141355fa6d022552aafc73135bf5d

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

LOG: Revert "[clang] Prevent that Decl::dump on a CXXRecordDecl deserialises 
further declarations."

This reverts commit 0478720157f6413fad7595b8eff9c70d2d99b637. This probably
doesn't work when forcing deserialising while dumping (which the ASTDumper
optionally supports).

Added: 


Modified: 
clang/lib/AST/TextNodeDumper.cpp
clang/test/AST/ast-dump-lambda.cpp
clang/test/AST/ast-dump-records.cpp
clang/unittests/AST/CMakeLists.txt

Removed: 
clang/unittests/AST/ASTDumpTest.cpp



diff  --git a/clang/lib/AST/TextNodeDumper.cpp 
b/clang/lib/AST/TextNodeDumper.cpp
index 19b7b4c801d5..16c4c3736a4a 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1960,11 +1960,7 @@ void TextNodeDumper::VisitCXXRecordDecl(const 
CXXRecordDecl *D) {
   FLAG(hasTrivialDestructor, trivial);
   FLAG(hasNonTrivialDestructor, non_trivial);
   FLAG(hasUserDeclaredDestructor, user_declared);
-  // Avoid calls to the external source.
-  if (!D->hasExternalVisibleStorage()) {
-FLAG(hasConstexprDestructor, constexpr);
-  } else
-OS << " maybe_constexpr";
+  FLAG(hasConstexprDestructor, constexpr);
   FLAG(needsImplicitDestructor, needs_implicit);
   FLAG(needsOverloadResolutionForDestructor, needs_overload_resolution);
   if (!D->needsOverloadResolutionForDestructor())

diff  --git a/clang/test/AST/ast-dump-lambda.cpp 
b/clang/test/AST/ast-dump-lambda.cpp
index 302b93734459..37fb62ef9930 100644
--- a/clang/test/AST/ast-dump-lambda.cpp
+++ b/clang/test/AST/ast-dump-lambda.cpp
@@ -48,7 +48,7 @@ template  void test(Ts... a) {
 // CHECK-NEXT:|   | |-MoveConstructor exists simple trivial needs_implicit
 // CHECK-NEXT:|   | |-CopyAssignment simple trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT:|   | |-MoveAssignment exists simple trivial needs_implicit
-// CHECK-NEXT:|   | `-Destructor simple irrelevant trivial{{( 
maybe_constexpr)?}} needs_implicit
+// CHECK-NEXT:|   | `-Destructor simple irrelevant trivial needs_implicit
 // CHECK-NEXT:|   |-CXXRecordDecl {{.*}}  col:10{{( 
imported)?}} implicit struct V
 // CHECK-NEXT:|   `-CXXMethodDecl {{.*}}  
line:17:10{{( imported)?}} f 'void ()'
 // CHECK-NEXT:| `-CompoundStmt {{.*}} 
@@ -60,7 +60,7 @@ template  void test(Ts... a) {
 // CHECK-NEXT:|   | | | |-MoveConstructor exists simple trivial 
needs_implicit
 // CHECK-NEXT:|   | | | |-CopyAssignment trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT:|   | | | |-MoveAssignment
-// CHECK-NEXT:|   | | | `-Destructor simple irrelevant trivial{{( 
maybe_constexpr)?}} needs_implicit
+// CHECK-NEXT:|   | | | `-Destructor simple irrelevant trivial 
needs_implicit
 // CHECK-NEXT:|   | | |-CXXMethodDecl {{.*}}  col:7{{( 
imported)?}} operator() 'auto () const -> auto' inline
 // CHECK-NEXT:|   | | | `-CompoundStmt {{.*}} 
 // CHECK-NEXT:|   | | `-FieldDecl {{.*}}  col:8{{( imported)?}} 
implicit 'V *'
@@ -75,7 +75,7 @@ template  void test(Ts... a) {
 // CHECK-NEXT:| | | |-MoveConstructor exists simple trivial 
needs_implicit
 // CHECK-NEXT:| | | |-CopyAssignment trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT:| | | |-MoveAssignment
-// CHECK-NEXT:| | | `-Destructor simple irrelevant trivial{{( 
maybe_constexpr)?}} needs_implicit
+// CHECK-NEXT:| | | `-Destructor simple irrelevant trivial 
needs_implicit
 // CHECK-NEXT:| | |-CXXMethodDecl {{.*}}  col:7{{( 
imported)?}} operator() 'auto () const -> auto' inline
 // CHECK-NEXT:| | | `-CompoundStmt {{.*}} 
 // CHECK-NEXT:| | `-FieldDecl {{.*}}  col:8{{( imported)?}} 
implicit 'V'
@@ -94,7 +94,7 @@ template  void test(Ts... a) {
 // CHECK-NEXT:| | | |-MoveConstructor exists simple trivial needs_implicit
 // CHECK-NEXT:| | | |-CopyAssignment trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT:| | | |-MoveAssignment
-// CHECK-NEXT:| | | `-Destructor simple irrelevant trivial{{( 
maybe_constexpr)?}} needs_implicit
+// CHECK-NEXT:| | | `-Destructor simple irrelevant trivial needs_implicit
 // CHECK-NEXT:| | |-CXXMethodDecl {{.*}}  col:3{{( 
imported)?}} operator() 'auto () const' inline
 // CHECK-NEXT:| | | `-CompoundStmt {{.*}} 
 // CHECK-NEXT:| | |-CXXConversionDecl {{.*}}  col:3{{( 
imported)?}} implicit constexpr operator auto (*)() 'auto (*() const 
noexcept)()' inline
@@

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2020-09-07 Thread Raphael Isemann via Phabricator via cfe-commits

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

In D80878#2258942 , @riccibruno wrote:

> And what if deserialization is forced?

That's a good point. That should indeed continue to work with the ASTDumper and 
now (potentially) doesn't. I'll revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80878

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

[PATCH] D86631: [Fixed Point] Add fixed-point to floating point cast types and consteval.

2020-09-07 Thread Bevin Hansson via Phabricator via cfe-commits

ebevhan updated this revision to Diff 290266.
ebevhan added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86631

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Frontend/fixed_point_conversions_const.c
  clang/test/Frontend/fixed_point_errors.c
  clang/test/Frontend/fixed_point_unknown_conversions.c

Index: clang/test/Frontend/fixed_point_unknown_conversions.c
===
--- clang/test/Frontend/fixed_point_unknown_conversions.c
+++ clang/test/Frontend/fixed_point_unknown_conversions.c
@@ -22,16 +22,12 @@
   _Fract fract = accum; // ok
   _Accum *accum_ptr;
 
-  accum = f;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
-  accum = d;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   accum = dc;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   accum = ic;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   accum = s;   // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
   accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
   accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
 
-  f = accum;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
-  d = accum;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   dc = accum;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   ic = accum;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   s = accum;   // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -286,8 +286,3 @@
 
 // Division by zero
 short _Accum div_zero = 4.5k / 0.0lr;  // expected-error {{initializer element is not a compile-time constant}}
-
-void foo(void) {
-  _Accum x = 0.5k;
-  if (x == 0.5) {} // expected-error{{invalid operands to binary expression ('_Accum' and 'double')}}
-}
Index: clang/test/Frontend/fixed_point_conversions_const.c
===
--- clang/test/Frontend/fixed_point_conversions_const.c
+++ clang/test/Frontend/fixed_point_conversions_const.c
@@ -43,6 +43,37 @@
 short _Accum sa_const7 = -256;
 // CHECK-DAG: @sa_const7 = {{.*}}global i16 -32768, align 2
 
+// Fixed point to floating point
+float fl_const = 1.0hk;
+// CHECK-DAG: @fl_const = {{.*}}global float 1.00e+00, align 4
+float fl_const2 = -128.0k;
+// CHECK-DAG: @fl_const2 = {{.*}}global float -1.28e+02, align 4
+float fl_const3 = 0.0872802734375k;
+// CHECK-DAG: @fl_const3 = {{.*}}global float 0x3FB65800, align 4
+float fl_const4 = 192.5k;
+// CHECK-DAG: @fl_const4 = {{.*}}global float 1.925000e+02, align 4
+float fl_const5 = -192.5k;
+// CHECK-DAG: @fl_const5 = {{.*}}global float -1.925000e+02, align 4
+
+// Floating point to fixed point
+_Accum a_fl_const = 1.0f;
+// CHECK-DAG: @a_fl_const = {{.*}}global i32 32768, align 4
+_Accum a_fl_const2 = -128.0f;
+// CHECK-DAG: @a_fl_const2 = {{.*}}global i32 -4194304, align 4
+_Accum a_fl_const3 = 0.0872802734375f;
+// CHECK-DAG: @a_fl_const3 = {{.*}}global i32 2860, align 4
+_Accum a_fl_const4 = 0.0872802734375;
+// CHECK-DAG: @a_fl_const4 = {{.*}}global i32 2860, align 4
+_Accum a_fl_const5 = -0.0872802734375f;
+// CHECK-DAG: @a_fl_const5 = {{.*}}global i32 -2860, align 4
+_Fract f_fl_const = 0.5f;
+// CHECK-DAG: @f_fl_const = {{.*}}global i16 16384, align 2
+_Fract f_fl_const2 = -0.75;
+// CHECK-DAG: @f_fl_const2 = {{.*}}global i16 -24576, align 2
+unsigned short _Accum usa_fl_const = 48.75f;
+// SIGNED-DAG: @usa_fl_const = {{.*}}global i16 12480, align 2
+// UNSIGNED-DAG: @usa_fl_const = {{.*}}global i16 6240, align 2
+
 // Signedness
 unsigned short _Accum usa_const2 = 2.5hk;
 // SIGNED-DAG: @usa_const2  = {{.*}}global i16 640, align 2
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

2020-09-07 Thread Bevin Hansson via Phabricator via cfe-commits

ebevhan updated this revision to Diff 290267.
ebevhan added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/Frontend/fixed_point_compound.c
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_conversions_half.c
  llvm/include/llvm/IR/FixedPointBuilder.h

Index: llvm/include/llvm/IR/FixedPointBuilder.h
===
--- llvm/include/llvm/IR/FixedPointBuilder.h
+++ llvm/include/llvm/IR/FixedPointBuilder.h
@@ -120,6 +120,17 @@
 C.isSigned(), C.isSaturated(), BothPadded);
   }
 
+  /// Given a floating point type and a fixed-point semantic, return a floating
+  /// point type which can accommodate the fixed-point semantic. This is either
+  /// \p Ty, or a floating point type with a larger exponent than Ty.
+  Type *getAccommodatingFloatType(Type *Ty, const FixedPointSemantics ) {
+const fltSemantics *FloatSema = >getFltSemantics();
+while (!Sema.canAccommodateFloatSemantics(*FloatSema))
+  FloatSema = APFixedPoint::promoteFloatSemantics(FloatSema);
+// There's seemingly no way to convert fltSemantics to Type.
+return ConstantFP::get(Ty->getContext(), APFloat(*FloatSema))->getType();
+  }
+
 public:
   FixedPointBuilder(IRBuilderTy ) : B(Builder) {}
 
@@ -159,6 +170,55 @@
DstSema, false);
   }
 
+  Value *CreateFixedToFloating(Value *Src, const FixedPointSemantics ,
+   Type *DstTy) {
+Value *Result;
+Type *OpTy = getAccommodatingFloatType(DstTy, SrcSema);
+// Convert the raw fixed-point value directly to floating point. If the
+// value is too large to fit, it will be rounded, not truncated.
+Result = SrcSema.isSigned() ? B.CreateSIToFP(Src, OpTy)
+: B.CreateUIToFP(Src, OpTy);
+// Rescale the integral-in-floating point by the scaling factor. This is
+// lossless, except for overflow to infinity which is unlikely.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, -(int)SrcSema.getScale(;
+if (OpTy != DstTy)
+  Result = B.CreateFPTrunc(Result, DstTy);
+return Result;
+  }
+
+  Value *CreateFloatingToFixed(Value *Src, const FixedPointSemantics ) {
+bool UseSigned = DstSema.isSigned() || DstSema.hasUnsignedPadding();
+Value *Result = Src;
+Type *OpTy = getAccommodatingFloatType(Src->getType(), DstSema);
+if (OpTy != Src->getType())
+  Result = B.CreateFPExt(Result, OpTy);
+// Rescale the floating point value so that its significant bits (for the
+// purposes of the conversion) are in the integral range.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, DstSema.getScale(;
+
+Type *ResultTy = B.getIntNTy(DstSema.getWidth());
+if (DstSema.isSaturated()) {
+  Intrinsic::ID IID =
+  UseSigned ? Intrinsic::fptosi_sat : Intrinsic::fptoui_sat;
+  Result = B.CreateIntrinsic(IID, {ResultTy, OpTy}, {Result});
+} else {
+  Result = UseSigned ? B.CreateFPToSI(Result, ResultTy)
+ : B.CreateFPToUI(Result, ResultTy);
+}
+
+// When saturating unsigned-with-padding using signed operations, we may
+// get negative values. Emit an extra clamp to zero.
+if (DstSema.isSaturated() && DstSema.hasUnsignedPadding()) {
+  Constant *Zero = Constant::getNullValue(Result->getType());
+  Result =
+  B.CreateSelect(B.CreateICmpSLT(Result, Zero), Zero, Result, "satmin");
+}
+
+return Result;
+  }
+
   /// Add two fixed-point values and return the result in their common semantic.
   /// \p LHS - The left hand side
   /// \p LHSSema - The semantic of the left hand side
Index: clang/test/Frontend/fixed_point_conversions_half.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_conversions_half.c
@@ -0,0 +1,309 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
+short _Fract sf;
+long _Fract lf;
+
+short _Accum sa;
+long _Accum la;
+
+unsigned short _Accum usa;
+unsigned long _Accum ula;
+
+_Sat short _Fract sf_sat;
+_Sat long _Fract lf_sat;
+
+_Sat short _Accum sa_sat;
+_Sat long _Accum la_sat;
+
+_Sat unsigned short _Accum usa_sat;
+_Sat unsigned long _Accum ula_sat;
+
+_Float16 h;
+
+
+// CHECK-LABEL: @half_fix1(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load half, half* @h, align 2
+// CHECK-NEXT:[[TMP1:%.*]] = fmul half [[TMP0]], 0xH5800
+//

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:48-50
+// Ignores the implicit `CXXConstructExpr` for copy/move constructors generated
+// by the compiler, as well as in implicit conversions like the one wrapping 
`1`
+// in `X x = 1;`.

Please give feedback on this comments and should I comment the rest of the 
function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

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

[PATCH] D87229: [SyntaxTree] Ignore implicit `CXXFunctionalCastExpr` wrapping constructor

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290261.
eduucaldas added a comment.

Add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87229

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -4066,7 +4066,6 @@
   X(int);
 };
 X test() {
-  // FIXME: Remove `UnknownExpression` due to implicit `CXXFunctionalCastExpr`
   [[return X(1);]]
 }
 )cpp",
@@ -4074,12 +4073,11 @@
 ReturnStatement Statement
 |-'return' IntroducerKeyword
 |-UnknownExpression ReturnValue
-| `-UnknownExpression
-|   |-'X'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'X'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
 `-';'
 )txt"}));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -60,9 +61,25 @@
   return E;
 }
 
+// In:
+// struct X {
+//   X(int)
+// };
+// X x = X(1);
+// Ignores the implicit `CXXFunctionalCastExpr` that wraps
+// `CXXConstructExpr X(1)`.
+static Expr *IgnoreCXXFunctionalCastExprWrappingConstructor(Expr *E) {
+  if (auto *F = dyn_cast(E)) {
+if (F->getCastKind() == CK_ConstructorConversion)
+  return F->getSubExpr();
+  }
+  return E;
+}
+
 static Expr *IgnoreImplicit(Expr *E) {
   return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
- IgnoreImplicitConstructorSingleStep);
+ IgnoreImplicitConstructorSingleStep,
+ IgnoreCXXFunctionalCastExprWrappingConstructor);
 }
 
 LLVM_ATTRIBUTE_UNUSED


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -4066,7 +4066,6 @@
   X(int);
 };
 X test() {
-  // FIXME: Remove `UnknownExpression` due to implicit `CXXFunctionalCastExpr`
   [[return X(1);]]
 }
 )cpp",
@@ -4074,12 +4073,11 @@
 ReturnStatement Statement
 |-'return' IntroducerKeyword
 |-UnknownExpression ReturnValue
-| `-UnknownExpression
-|   |-'X'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'X'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
 `-';'
 )txt"}));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -60,9 +61,25 @@
   return E;
 }
 
+// In:
+// struct X {
+//   X(int)
+// };
+// X x = X(1);
+// Ignores the implicit `CXXFunctionalCastExpr` that wraps
+// `CXXConstructExpr X(1)`.
+static Expr *IgnoreCXXFunctionalCastExprWrappingConstructor(Expr *E) {
+  if (auto *F = dyn_cast(E)) {
+if (F->getCastKind() == CK_ConstructorConversion)
+  return F->getSubExpr();
+  }
+  return E;
+}
+
 static Expr *IgnoreImplicit(Expr *E) {
   return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
- IgnoreImplicitConstructorSingleStep);
+ IgnoreImplicitConstructorSingleStep,
+ IgnoreCXXFunctionalCastExprWrappingConstructor);
 }
 
 LLVM_ATTRIBUTE_UNUSED
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290260.
eduucaldas added a comment.

Add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit default constructs.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-07 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290259.
eduucaldas added a comment.

Add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,135 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test(){
+  [[S s0{}]];
+  [[S s1{1}]];
+  [[S s2{1, 2.}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s0'
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s1'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s2'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_EqualBrace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test() {
+  [[S s0 = {}]];
+  [[S s1 = {1}]];
+  [[S s2 = {1, 2.}]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s0'
+  |-'='
+  `-UnknownExpression
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s1'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s2'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
@@ -3851,15 +3956,133 @@
   R"cpp(
 struct S {
   S(int);
+  S(int, float);
 };
-[[S s(1);]]
+[[S s1(1);]]
+[[S s2(1, 2.);]]
 )cpp",
   {R"txt(
 SimpleDeclaration
 |-'S'
 |-SimpleDeclarator Declarator
 | `-UnknownExpression
-|   |-'s'
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+void TakeX(const X&);
+void test() {
+  [[TakeX(1)]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'TakeX'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Return) {
+  if (!GetParam().isCXX()) {

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-07 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added a comment.

In D87101#2256557 , @spatel wrote:

> I noticed one other LLVM codegen test that might want to be updated to use 
> the IR intrinsics to be more relevant -- 
> llvm-project/llvm/test/CodeGen/X86/combine-abs.ll. That can be done with this 
> patch or later.

I'm going to address that one in a followup that cleans up the x86 PABS 
intrinsic handling in AutoUpgrade.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87101

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

[clang] a8a9153 - [X86] Replace EmitX86AddSubSatExpr with EmitX86BinaryIntrinsic generic helper. NFCI.

2020-09-07 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-09-07T13:33:48+01:00
New Revision: a8a91533dd65041ced68ed5b9348b5d023837488

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

LOG: [X86] Replace EmitX86AddSubSatExpr with EmitX86BinaryIntrinsic generic 
helper. NFCI.

Feed the Intrinsic::ID value directly instead of via the IsSigned/IsAddition 
bool flags.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 1192fbdc1c9d..42fab29ab8aa 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -11543,13 +11543,9 @@ static Value *EmitX86SExtMask(CodeGenFunction , 
Value *Op,
   return CGF.Builder.CreateSExt(Mask, DstTy, "vpmovm2");
 }
 
-// Emit addition or subtraction with signed/unsigned saturation.
-static Value *EmitX86AddSubSatExpr(CodeGenFunction ,
-   ArrayRef Ops, bool IsSigned,
-   bool IsAddition) {
-  Intrinsic::ID IID =
-  IsSigned ? (IsAddition ? Intrinsic::sadd_sat : Intrinsic::ssub_sat)
-   : (IsAddition ? Intrinsic::uadd_sat : Intrinsic::usub_sat);
+// Emit binary intrinsic with the same type used in result/args.
+static Value *EmitX86BinaryIntrinsic(CodeGenFunction ,
+ ArrayRef Ops, Intrinsic::ID IID) 
{
   llvm::Function *F = CGF.CGM.getIntrinsic(IID, Ops[0]->getType());
   return CGF.Builder.CreateCall(F, {Ops[0], Ops[1]});
 }
@@ -14033,28 +14029,28 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
   case X86::BI__builtin_ia32_paddsw256:
   case X86::BI__builtin_ia32_paddsb128:
   case X86::BI__builtin_ia32_paddsw128:
-return EmitX86AddSubSatExpr(*this, Ops, true, true);
+return EmitX86BinaryIntrinsic(*this, Ops, Intrinsic::sadd_sat);
   case X86::BI__builtin_ia32_paddusb512:
   case X86::BI__builtin_ia32_paddusw512:
   case X86::BI__builtin_ia32_paddusb256:
   case X86::BI__builtin_ia32_paddusw256:
   case X86::BI__builtin_ia32_paddusb128:
   case X86::BI__builtin_ia32_paddusw128:
-return EmitX86AddSubSatExpr(*this, Ops, false, true);
+return EmitX86BinaryIntrinsic(*this, Ops, Intrinsic::uadd_sat);
   case X86::BI__builtin_ia32_psubsb512:
   case X86::BI__builtin_ia32_psubsw512:
   case X86::BI__builtin_ia32_psubsb256:
   case X86::BI__builtin_ia32_psubsw256:
   case X86::BI__builtin_ia32_psubsb128:
   case X86::BI__builtin_ia32_psubsw128:
-return EmitX86AddSubSatExpr(*this, Ops, true, false);
+return EmitX86BinaryIntrinsic(*this, Ops, Intrinsic::ssub_sat);
   case X86::BI__builtin_ia32_psubusb512:
   case X86::BI__builtin_ia32_psubusw512:
   case X86::BI__builtin_ia32_psubusb256:
   case X86::BI__builtin_ia32_psubusw256:
   case X86::BI__builtin_ia32_psubusb128:
   case X86::BI__builtin_ia32_psubusw128:
-return EmitX86AddSubSatExpr(*this, Ops, false, false);
+return EmitX86BinaryIntrinsic(*this, Ops, Intrinsic::usub_sat);
   }
 }
 



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

1 2 >

1 - 100 of 158 matches

Mail list logo