[PATCH] D86964: [ASTMatchers] Avoid recursion in ancestor matching to save stack space.

2020-09-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:705
+  // Unlike matchesAnyAncestorOf there's no memoization: it doesn't save much.
+  bool matchesParentOf(const DynTypedNode &Node, const DynTypedMatcher 
&Matcher,
+   BoundNodesTreeBuilder *Builder) {

this API makes sense, I'm wondering whether we should do it further (more 
aligned with `matchesChildOf` pattern):

- make `matchesParentOf` public;
- introduce a new `matchesParentOf` interface in ASTMatcherFinder, which calls 
this function here;
- then we can remove the special parent case in 
`MatchASTVisitor::matchesAncestorOf`, and the `AncestorMatchMode` enum is also 
not needed anymore;

I'm also ok with the current way.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:738
+// Memoization keys that must be updated with the result.
+std::vector Keys;
+// When returning, update the memoization cache.

nit: I think we should explicitly describe (either in the comment or name) 
these keys are for the single-parent *chain*. It took me a while to understand 
what are these keys for?



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:740
+// When returning, update the memoization cache.
+auto Finish = [&](bool Result) {
+  for (const auto &Key : Keys) {

maybe

Finish => Memorize
Result => IsMatched



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:807
+  // Memoization of newly visited nodes is not possible (but we still 
update
+  // results for the elements in the chain we found above).
   std::deque Queue(Parents.begin(), Parents.end());

nit: assert(Parents.size() > 1).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86964

___
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-03 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> `len + 1 < 0`

What are types of each term in this expression?

Like, if everything here was promoted to `size_t`, as it should be according to 
the language rules, then this comparison is trivially false and our constraint 
manager is definitely smart enough to figure this out.

> into `len < -1`

And this is an incorrect transformation assuming everything is of type 
`size_t`. The original statement was trivially false whereas this statement is 
true for almost every value - except, well, `SIZE_MAX`. If it's the checker 
doing this, it's doing something wrong.


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] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

The builtins with custom type-checking are all true intrinsics like 
`__builtin_operator_new` and so on.  They really can't be validly declared by 
the user program.  The thing that seems most likely to avoid random compiler 
crashes would be to either forbid explicit declarations of them or treat those 
as no longer being builtins.  If we need to maintain compatibility with people 
making custom declarations, we would need to always treat them as builtins and 
run the risk of crashing if someone declares one with a bad signature.  But I 
don't think it's unfair of us to break those people; that is seriously not 
reasonable user behavior.

It's possible that custom declarations are people trying to create 
compatibility shims for compilers that don't provide these as builtins.  Those 
people should be guarding their custom declarations, preferably with 
`__has_builtin`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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


[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-03 Thread Rainer Orth via Phabricator via cfe-commits
ro updated this revision to Diff 289658.
ro added a comment.

Add FIXME for SparcV8 `MaxAtomicInlineWidth`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

Files:
  clang/lib/Basic/Targets/Sparc.cpp
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
  compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp

Index: compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
@@ -11,9 +11,6 @@
 // FIXME: not %run %t 8 2>&1 | FileCheck %s --check-prefix=CHECK-8
 // RUN: not %run %t 9 2>&1 | FileCheck %s --check-prefix=CHECK-9
 
-// Bug 42535
-// XFAIL: sparc-target-arch
-
 // This test assumes float and double are IEEE-754 single- and double-precision.
 
 #if defined(__APPLE__)
Index: compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
===
--- compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
+++ compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
@@ -10,9 +10,6 @@
 RUN: llvm-cov gcov instrprof-gcov-parallel.target.gcda
 RUN: FileCheck --input-file instrprof-gcov-parallel.target.c.gcov %s
 
-# Bug 42535
-# XFAIL: sparc-target-arch
-
 # Test if the .gcda file is correctly created from one of child processes
 # and counters of all processes are recorded correctly.
 # 707 = CHILDREN * COUNT
Index: clang/test/Preprocessor/predefined-arch-macros.c
===
--- clang/test/Preprocessor/predefined-arch-macros.c
+++ clang/test/Preprocessor/predefined-arch-macros.c
@@ -3233,9 +3233,26 @@
 // RUN: -target sparc-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
 // CHECK_SPARC-V9-NOT: #define __sparcv8 1
+// CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
 // CHECK_SPARC-V9: #define __sparc_v9__ 1
 // CHECK_SPARC-V9: #define __sparcv9 1
-// CHECK_SPARC-V9-NOT: #define __sparcv8 1
+// CHECK_SPARC-V9: #define __sparcv9__ 1
+
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN: -target sparc-sun-solaris \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC_SOLARIS_GCC_ATOMICS
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
+
+// RUN: %clang -mcpu=v8 -E -dM %s -o - 2>&1 \
+// RUN: -target sparc-sun-solaris \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN: -target sparcel-unknown-linux \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -347,6 +347,8 @@
   case llvm::Triple::sparcv9:
 if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
   return A->getValue();
+if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris())
+  return "v9";
 return "";
 
   case llvm::Triple::x86:
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -166,10 +166,15 @@
   PtrDiffType = SignedLong;
   break;
 }
-// Up to 32 bits are lock-free atomic, but we're willing to do atomic ops
-// on up to 64 bits.
+// Up to 32 bits (V8) or 64 bits (V9) are lock-free atomic, but we're
+// willing to do atomic ops on up to 64 bits.
 MaxAtomicPromoteWidth = 64;
-MaxAtomicInlineWidth = 32;
+if (getCPUGeneration(CPU) == CG_V9)
+  MaxAtomicInlineWidth = 64;
+else
+  // FIXME: This isn't correct for plain V8 which lacks CAS,
+  // only for LEON 3+ and Myriad.
+  MaxAtomicInlineWidth = 32;
   }
 
   void getTargetDefines(const LangOptions &Opts,
Index: clang/lib/Basic/Targets/Sparc.cpp
===
--- clang/lib/Basic/Targets/Sparc.cpp
+++ clang/lib/Basic/Targets/Sparc.cpp
@@ -147,19 +147,20 @@
 void Sparc

[PATCH] D86720: [clang][aarch64] Drop experimental from __ARM_FEATURE_SVE_BITS macro

2020-09-03 Thread Cullen Rhodes 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 rGf9091e56d34f: [clang][aarch64] Drop experimental from  
__ARM_FEATURE_SVE_BITS macro (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D86720?vs=288352&id=289668#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86720

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
  clang/test/CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
  clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -44,12 +44,12 @@
 // CHECK-NOT: __ARM_BF16_FORMAT_ALTERNATIVE 1
 // CHECK-NOT: __ARM_FEATURE_BF16 1
 // CHECK-NOT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 0
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 512
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 1024
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 2048
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 0
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 128
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 256
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 512
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 1024
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 2048
 
 // RUN: %clang -target aarch64_be-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-BIGENDIAN
 // CHECK-BIGENDIAN: __ARM_BIG_ENDIAN 1
@@ -444,10 +444,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// NOTE: The __ARM_FEATURE_SVE_BITS feature macro is experimental until the
-// feature is complete.
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 512
-// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 1024
-// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 2048
+// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS 128
+// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS 256
+// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS 512
+// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS 1024
+// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS 2048
Index: clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp
===

[clang] f9091e5 - [clang][aarch64] Drop experimental from __ARM_FEATURE_SVE_BITS macro

2020-09-03 Thread Cullen Rhodes via cfe-commits

Author: Cullen Rhodes
Date: 2020-09-03T09:39:37Z
New Revision: f9091e56d34fc1a14fe4640b95a691d9ac7afcc4

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

LOG: [clang][aarch64] Drop experimental from  __ARM_FEATURE_SVE_BITS macro

The __ARM_FEATURE_SVE_BITS feature macro is specified in the Arm C
Language Extensions (ACLE) for SVE [1] (version 00bet5). From the spec,
where __ARM_FEATURE_SVE_BITS==N:

When N is nonzero, indicates that the implementation is generating
code for an N-bit SVE target and that the arm_sve_vector_bits(N)
attribute is available.

This was defined in D83550 as __ARM_FEATURE_SVE_BITS_EXPERIMENTAL and
enabled under the -msve-vector-bits flag to simplify initial tests.
This patch drops _EXPERIMENTAL now there is support for the feature.

[1] https://developer.arm.com/documentation/100987/latest

Reviewed By: david-arm

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

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td
clang/lib/Basic/Targets/AArch64.cpp
clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
clang/test/CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp
clang/test/Preprocessor/aarch64-target-features.c
clang/test/Sema/attr-arm-sve-vector-bits.c
clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 3a28cf245656..d6d5567c7924 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4944,10 +4944,6 @@ to the SVE predicate type ``svbool_t``, this excludes 
tuple types such as
 ``N==__ARM_FEATURE_SVE_BITS``, the implementation defined feature macro that is
 enabled under the ``-msve-vector-bits`` flag.
 
-NOTE: This feature is currently WIP, the ``-msve-vector-bits=`` flag defines
-the ``__ARM_FEATURE_SVE_BITS_EXPERIMENTAL`` macro. This feature is complete
-when experimental is dropped.
-
 For more information See `Arm C Language Extensions for SVE
 `_ for more information.
 }];

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 6fd97d4e5786..7f0a0f0d86dc 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -378,8 +378,7 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
&Opts,
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
 
   if (Opts.ArmSveVectorBits)
-Builder.defineMacro("__ARM_FEATURE_SVE_BITS_EXPERIMENTAL",
-Twine(Opts.ArmSveVectorBits));
+Builder.defineMacro("__ARM_FEATURE_SVE_BITS", 
Twine(Opts.ArmSveVectorBits));
 }
 
 ArrayRef AArch64TargetInfo::getTargetBuiltins() const {

diff  --git a/clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c 
b/clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
index f6b8b1be1e76..cab424c3dbe1 100644
--- a/clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
+++ b/clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
@@ -6,7 +6,7 @@
 
 #include 
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef svint64_t fixed_int64_t __attribute__((arm_sve_vector_bits(N)));
 typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));

diff  --git a/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c 
b/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
index 412923f1e898..490ec92dfdeb 100644
--- a/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
+++ b/clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
@@ -4,7 +4,7 @@
 
 #include 
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
 typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));

diff  --git a/clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c 
b/clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
index 6c7edf9033f7..13d8f14f991a 100644
--- a/clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
+++ b/clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
@@ -4,7 +4,7 @@
 
 #include 
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
 typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));

diff  --git a/clang/test/CodeGen/attr-arm-sv

[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

2020-09-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.
Herald added a project: clang.
hokein requested review of this revision.

heap-allocate the Template kind, this would reduce AST memory usage

- TemplateArgumentLocInfo from 24 bytes to 8 bytes;
- TemplateARgumentLoc from 56 bytes to 40 bytes;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87080

Files:
  clang/include/clang/AST/TemplateBase.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -7101,15 +7101,15 @@
 NestedNameSpecifierLoc QualifierLoc =
   readNestedNameSpecifierLoc();
 SourceLocation TemplateNameLoc = readSourceLocation();
-return TemplateArgumentLocInfo(QualifierLoc, TemplateNameLoc,
-   SourceLocation());
+return TemplateArgumentLocInfo(getASTContext(), QualifierLoc,
+   TemplateNameLoc, SourceLocation());
   }
   case TemplateArgument::TemplateExpansion: {
 NestedNameSpecifierLoc QualifierLoc = readNestedNameSpecifierLoc();
 SourceLocation TemplateNameLoc = readSourceLocation();
 SourceLocation EllipsisLoc = readSourceLocation();
-return TemplateArgumentLocInfo(QualifierLoc, TemplateNameLoc,
-   EllipsisLoc);
+return TemplateArgumentLocInfo(getASTContext(), QualifierLoc,
+   TemplateNameLoc, EllipsisLoc);
   }
   case TemplateArgument::Null:
   case TemplateArgument::Integral:
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3545,12 +3545,12 @@
 }
 
 case TemplateArgument::Template:
-  return TemplateArgumentLoc(TemplateArgument(
-  Pattern.getArgument().getAsTemplate(),
-  NumExpansions),
- Pattern.getTemplateQualifierLoc(),
- Pattern.getTemplateNameLoc(),
- EllipsisLoc);
+  return TemplateArgumentLoc(
+  SemaRef.Context,
+  TemplateArgument(Pattern.getArgument().getAsTemplate(),
+   NumExpansions),
+  Pattern.getTemplateQualifierLoc(), Pattern.getTemplateNameLoc(),
+  EllipsisLoc);
 
 case TemplateArgument::Null:
 case TemplateArgument::Integral:
@@ -4288,8 +4288,8 @@
 if (Template.isNull())
   return true;
 
-Output = TemplateArgumentLoc(TemplateArgument(Template), QualifierLoc,
- Input.getTemplateNameLoc());
+Output = TemplateArgumentLoc(SemaRef.Context, TemplateArgument(Template),
+ QualifierLoc, Input.getTemplateNameLoc());
 return false;
   }
 
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1095,7 +1095,7 @@
   case TemplateArgument::TemplateExpansion:
 Ellipsis = OrigLoc.getTemplateEllipsisLoc();
 NumExpansions = Argument.getNumTemplateExpansions();
-return TemplateArgumentLoc(Argument.getPackExpansionPattern(),
+return TemplateArgumentLoc(Context, Argument.getPackExpansionPattern(),
OrigLoc.getTemplateQualifierLoc(),
OrigLoc.getTemplateNameLoc());
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2886,7 +2886,7 @@
 if (!TName.isNull())
   Param->setDefaultArgument(
   SemaRef.Context,
-  TemplateArgumentLoc(TemplateArgument(TName),
+  TemplateArgumentLoc(SemaRef.Context, TemplateArgument(TName),
   D->getDefaultArgument().getTemplateQualifierLoc(),
   D->getDefaultArgument().getTemplateNameLoc()));
   }
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2670,11 +2670,11 @@
 Builder.MakeTrivial(Context, QTN->getQualifier(), Loc);
 
   if (Arg.getKind() == TemplateArgument::Template)
-

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

2020-09-03 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

I was indeed on vacation, so thanks for committing it, @NoQ! I was waiting for 
agreement for the prerequisite patch then I forgot to notify you that I was 
going on vacation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

___
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-03 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: steakhal, balazske, Szelethus, NoQ, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, 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.

Add the BufferSize argument constraint to fread and fwrite. This change
itself makes it possible to discover a security critical case, described
in SEI-CERT ARR38-C.

We also add the not-null constraint on the 3rd arguments.

In this patch, I also remove those lambdas that don't take any
parameters (Fwrite, Fread, Getc), thus making the code better
structured.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87081

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

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -194,6 +194,22 @@
 // bugpath-warning{{Function argument constraint is not satisfied}} \
 // bugpath-note{{Function argument constraint is not satisfied}}
 }
+typedef __WCHAR_TYPE__ wchar_t;
+// This is one test case for the ARR38-C SEI-CERT rule.
+void ARR38_C_F(FILE *file) {
+  enum { BUFFER_SIZE = 1024 };
+  wchar_t wbuf[BUFFER_SIZE]; // bugpath-note{{'wbuf' initialized here}}
+
+  const size_t size = sizeof(*wbuf);
+  const size_t nitems = sizeof(wbuf);
+
+  // The 3rd parameter should be the number of elements to read, not
+  // the size in bytes.
+  fread(wbuf, size, nitems, file); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+}
 
 int __two_constrained_args(int, int);
 void test_constraints_on_multiple_args(int x, int y) {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -989,6 +989,12 @@
   for (const Summary &S : Summaries)
 operator()(Name, S);
 }
+// Add the same summary for different names with the Signature explicitly
+// given.
+void operator()(std::vector Names, Signature Sign, Summary Sum) {
+  for (StringRef Name : Names)
+operator()(Name, Sign, Sum);
+}
   } addToFunctionSummaryMap(ACtx, FunctionSummaryMap, DisplayLoadedSummaries);
 
   // Below are helpers functions to create the summaries.
@@ -1030,35 +1036,12 @@
   Optional FilePtrRestrictTy = getRestrictTy(FilePtrTy);
 
   // Templates for summaries that are reused by many functions.
-  auto Getc = [&]() {
-return Summary(ArgTypes{FilePtrTy}, RetType{IntTy}, NoEvalCall)
-.Case({ReturnValueCondition(WithinRange,
-{{EOFv, EOFv}, {0, UCharRangeMax}})});
-  };
   auto Read = [&](RetType R, RangeInt Max) {
 return Summary(ArgTypes{Irrelevant, Irrelevant, SizeTy}, RetType{R},
NoEvalCall)
 .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
ReturnValueCondition(WithinRange, Range(-1, Max))});
   };
-  auto Fread = [&]() {
-return Summary(
-   ArgTypes{VoidPtrRestrictTy, SizeTy, SizeTy, FilePtrRestrictTy},
-   RetType{SizeTy}, NoEvalCall)
-.Case({
-ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-})
-.ArgConstraint(NotNull(ArgNo(0)));
-  };
-  auto Fwrite = [&]() {
-return Summary(ArgTypes{ConstVoidPtrRestrictTy, SizeTy, SizeTy,
-FilePtrRestrictTy},
-   RetType{SizeTy}, NoEvalCall)
-.Case({
-ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-})
-.ArgConstraint(NotNull(ArgNo(0)));
-  };
   auto Getline = [&](RetType R, RangeInt Max) {
 return Summary(ArgTypes{Irrelevant, Irrelevant, Irrelevant}, RetType{R},
NoEvalCall)
@@ -1223,19 +1206,45 @@
  0U, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}})));
 
   // The getc() family of functions that returns either a char or an EOF.
-addToFunctionSummaryMap("getc", Getc());
-addToFunctionSummaryMap("fgetc", Getc());
+  addToFunctionSummaryMap(
+  {"getc", "fgetc"}, Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+  Summary(NoEvalCall)
+  .Case({ReturnValueCondition(WithinRange,
+  {{EOFv, EOFv}, {0, UCharRangeMax}})}));
   addToFunctionSummaryMap(
   "getchar", Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
   

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

2020-09-03 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

Is there a way this test case can somehow be broken up into multiple files? The 
test case takes a very long time to compile which causes intermittent but 
frequent failures on one of our bots that runs on a fairly small VM. Most of 
the failures listed here: 
http://lab.llvm.org:8011/builders/clang-ppc64le-linux?numbuilds=100 are timeout 
failures when compiling this test. It seems unfortunate to have to increase the 
timeout limit on the bot just for this - especially if this can be broken up 
into multiple files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82485

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


[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 289623.
ymandel added a comment.
Herald added a subscriber: JDevlieghere.

fix diff base; make small clang-tidy suggested change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87031

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -25,6 +25,7 @@
 using ::testing::IsEmpty;
 using transformer::cat;
 using transformer::changeTo;
+using transformer::rewriteDescendants;
 using transformer::RewriteRule;
 
 constexpr char KHeaderContents[] = R"cc(
@@ -568,6 +569,88 @@
   EXPECT_EQ(ErrorCount, 1);
 }
 
+//
+// We include one test per typed overload. We don't test extensively since that
+// is already covered by the tests above.
+//
+
+TEST_F(TransformerTest, RewriteDescendantsTypedStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("body");
+  assert(Node != nullptr && "body must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDecl) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("fun");
+  assert(Node != nullptr && "fun must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(
+  functionDecl(
+  hasName("f"),
+  hasParameter(0, varDecl(hasTypeLoc(typeLoc().bind("parmType"),
+  [&IntToChar](const MatchFinder::MatchResult &R) {
+const auto *Node = R.Nodes.getNodeAs("parmType");
+assert(Node != nullptr && "parmType must be bound");
+return transformer::detail::rewriteDescendants(*Node, IntToChar, R);
+  }),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDynTyped) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(
+  makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+   [&InlineX](const MatchFinder::MatchResult &R) {
+ auto It = R.Nodes.getMap().find("body");
+ assert(It != R.Nodes.getMap().end() && "body must be bound");
+ return transformer::detail::rewriteDescendants(It->second,
+InlineX, R);
+   }),
+  Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transfor

[PATCH] D87056: [POC] SVE/SVE2 implementation (LLVM 9)

2020-09-03 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dang, nikic, 
aaron.ballman, ecnelises, dantrushin, kerbowa, s.egerton, Jim, asbirlea, jfb, 
arphaman, dexonsmith, rogfer01, steven_wu, atanasyan, mgrang, zzheng, jrtc27, 
delcypher, simoncook, haicheng, kosarev, javed.absar, fedor.sergeev, kbarton, 
hiraditya, kristof.beyls, eraman, jgravelle-google, krytarowski, tschuett, 
sbc100, mgorny, nhaehnle, jvesely, nemanjai, sdardis, dylanmckay, dschuff, 
arsenm, qcolombet, MatzeB, emaste, jholewinski.
Herald added a reviewer: deadalnix.
Herald added a reviewer: alexshap.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: rengolin.
Herald added a reviewer: DavidTruby.
Herald added a reviewer: ctetreau.
Herald added a reviewer: sscalpone.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.
sdesmalen requested review of this revision.
Herald added subscribers: sstefan1, vkmr, ormris, MaskRay, aheejin.

DO NOT REVIEW; For reference only.

This patch contains Arm's changes to LLVM 9 to support SVE/SVE2. This
supports SVE/SVE2 CodeGen, scalable auto-vectorization and the ACLE
(C/C++ intrinsics interface).

While the patches are still based on LLVM 9, it should be a good
indication of the changes we've made to support scalable vectors.
These patches are meant for reference and are not intended to be
committed. This patch may help clarify some of the design choices we've
made when implementing scalable vectors for SVE, and it allows others
to experiment with our scalable-vector implementation.

The patches apply cleanly to the `llvmorg-9.0.0` tag in the Monorepo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87056

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypesSVE.def
  clang/include/clang/AST/CanonicalType.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/TargetTypes.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/BuiltinsSVE.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/Diagnostic.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/Sanitizers.def
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/Version.h
  clang/include/clang/Basic/arm_sve.td
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Phases.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/include/clang/Driver/Util.h
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/module.modulemap
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Version.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

2020-09-03 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

A gentle notification :-)


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

https://reviews.llvm.org/D77062

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


[PATCH] D78902: [Driver] Add output file to properties of Command

2020-09-03 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 289693.
sepavloff added a comment.

Rebased patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78902

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/lib/Driver/ToolChains/Ananas.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CloudABI.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/DragonFly.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/ToolChains/MSP430.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/Minix.cpp
  clang/lib/Driver/ToolChains/Myriad.cpp
  clang/lib/Driver/ToolChains/NaCl.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/XCore.cpp
  clang/unittests/Driver/ToolChainTest.cpp

Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -259,4 +259,34 @@
   EXPECT_STREQ(Res.DriverMode, "--driver-mode=cl");
   EXPECT_FALSE(Res.TargetIsValid);
 }
+
+TEST(ToolChainTest, CommandOutput) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new llvm::vfs::InMemoryFileSystem);
+
+  Driver CCDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+  InMemoryFileSystem);
+  CCDriver.setCheckInputsExist(false);
+  std::unique_ptr CC(
+  CCDriver.BuildCompilation({"/home/test/bin/clang", "foo.cpp"}));
+  const JobList &Jobs = CC->getJobs();
+
+  const auto &CmdCompile = Jobs.getJobs().front();
+  const auto &InFile = CmdCompile->getInputFilenames().front();
+  EXPECT_STREQ(InFile, "foo.cpp");
+  auto ObjFile = CmdCompile->getOutputFilenames().front();
+  EXPECT_TRUE(StringRef(ObjFile).endswith(".o"));
+
+  const auto &CmdLink = Jobs.getJobs().back();
+  const auto LinkInFile = CmdLink->getInputFilenames().front();
+  EXPECT_EQ(ObjFile, LinkInFile);
+  auto ExeFile = CmdLink->getOutputFilenames().front();
+  EXPECT_EQ("a.out", ExeFile);
+}
+
 } // end anonymous namespace.
Index: clang/lib/Driver/ToolChains/XCore.cpp
===
--- clang/lib/Driver/ToolChains/XCore.cpp
+++ clang/lib/Driver/ToolChains/XCore.cpp
@@ -53,7 +53,7 @@
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("xcc"));
   C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(),
- Exec, CmdArgs, Inputs));
+ Exec, CmdArgs, Inputs, Output));
 }
 
 void tools::XCore::Linker::ConstructJob(Compilation &C, const JobAction &JA,
@@ -82,7 +82,7 @@
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("xcc"));
   C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(),
- Exec, CmdArgs, Inputs));
+ Exec, CmdArgs, Inputs, Output));
 }
 
 /// XCore tool chain
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -114,8 +114,9 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  C.addCommand(std::make_unique(
-  JA, *this, ResponseFileSupport::AtFileCurCP(), Linker, CmdArgs, Inputs));
+  C.addCommand(std::make_unique(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Linker, CmdArgs, Inputs, Output));
 
   // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
@@ -139,7 +140,7 @@
 CmdArgs.push_back(Output.getFilename());
 C.addCommand(std::make_unique(

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13981
+bool overflow;
+llvm::APInt product(index);
+product += 1;

What if index is wider than AddrBits, but the active bits are fewer? Then you 
might miss out on triggering the overflow case in the multiplication.



Comment at: clang/lib/Sema/SemaChecking.cpp:13992
+  MaxElems =
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;

Should this not be AddrBits + 1 if you want to add 1 below?



Comment at: clang/lib/Sema/SemaChecking.cpp:13993
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())

Though, why is the +1 here? Isn't this already the maximum number of elements?



Comment at: clang/lib/Sema/SemaChecking.cpp:13994-13997
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())
+MaxElems = MaxElems.zext(apElemBytes.getBitWidth());
+  else if (apElemBytes.getBitWidth() < MaxElems.getBitWidth())
+apElemBytes = apElemBytes.zext(MaxElems.getBitWidth());

MaxElems should already be at a sufficient width here because of the earlier 
max. You can probably just do apElemBytes = 
apElemBytes.zextOrTrunc(MaxElems.getBitWidth())?



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto &ASTC = getASTContext();

chrish_ericsson_atx wrote:
> ebevhan wrote:
> > It might simplify the patch to move this condition out of the tree and just 
> > early return for the other case. That is:
> > 
> > ```
> > if (isUnboundedArray) {
> >   if (!(index.isUnsigned() || !index.isNegative()))
> > return;
> > 
> >   ...
> >   return;
> > }
> > 
> > if (index.isUnsigned() ...
> > ```
> There's a bit more code (starting at line 14094 in this patch set) that 
> applies in all cases, so an early return here would prevent the "Array 
> declared here" note from being generated.
Ah, the note.

I wonder if it wouldn't be cleaner (and avoid indenting the entire block) if 
that was just duplicated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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


[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+  // do not remove those when using the cl driver.
+  bool IsDependencyFileArg;
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))

shivanshu3 wrote:
> hans wrote:
> > Instead of a bool and if below, I'd suggest if-statements and early 
> > continues instead. Breaking up the old if-statement is nice though, and 
> > maybe the comment from above about what flags to ignore could be moved down 
> > here to those if statements. For example:
> > 
> > 
> > ```
> > // -M flags blah blah
> > if (Arg.startswith("-M") && !UsingClDriver)
> >   continue;
> > // MSVC flags blah blah
> > if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
> >   continue;
> > AdjustedArgs.push_back(Args[i]);
> > ```
> I realized that with my original change, we would skip the next argument 
> under cl driver mode when -MT was used, which would be wrong. The next 
> argument should only be skipped when `IsDependencyFileArg` is true. So I 
> think it might be cleaner to keep that extra boolean so the code is easy to 
> read and understand. What do you think?
I think having the boolean variable is still more confusing (it adds more state 
to keep track of) than just handling the cases with if-statements and early 
continue.

How about:

```
// -M flags that take an arg..
if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
  ++i;
  continue;
}
// -M flags blah blah
if (!UsingClDriver && Arg.startswith("-M"))
  continue;
// MSVC flags blah blah
if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
  continue;

AdjustedArgs.push_back(Args[i]);
```

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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


[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:578
+static std::pair
+getLikelihood(const Stmt *Stmt) {
+  if (const auto *AS = dyn_cast(Stmt))

Mordante wrote:
> rsmith wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > rsmith wrote:
> > > > > Sema doesn't care about any of this; can you move this code to 
> > > > > CodeGen and remove the code that stores likelihood data in the AST?
> > > > FWIW, I asked for it to be moved out of CodeGen and into Sema because 
> > > > the original implementation in CodeGen was doing a fair amount of work 
> > > > trying to interrogate the AST for this information. Now that we've 
> > > > switched the design to only be on the substatement of an if/else 
> > > > statement (rather than an arbitrary statement), it may be that CodeGen 
> > > > is a better place for this again (and if so, I'm sorry for the churn).
> > > At the moment the Sema cares about it to generate diagnostics about 
> > > conflicting annotations:
> > > ```
> > > if(i) [[ likely]] {}
> > > else [[likely]] {}
> > > ```
> > > This diagnostic only happens for an if statement, for a switch multiple 
> > > values can be considered likely.
> > > Do you prefer to have the diagnostic also in the CodeGen?
> > It looked to me like you'd reached agreement to remove that diagnostic. Are 
> > you intending to keep it?
> > 
> > If so, then I'd suggest you make `getLikelihood` a member of `Stmt` so that 
> > `CodeGen` and the warning here can both easily call it.
> @aaron.ballman I thought we wanted to keep this conflict warning, am I 
> correct?
> I'll add an extra function to the Stmt to test for a conflict and use that 
> for the diagnostic in the Sema. This allows me to use `LH_None` for no 
> attribute or a conflict of attributes in the CodeGen. Then there's no need 
> for `LH_Conflict` and it can be removed from the enumerate.
> @aaron.ballman I thought we wanted to keep this conflict warning, am I 
> correct?

I want to keep the property that any time the user writes an explicit 
annotation in their code but we decide to ignore the annotation (for whatever 
reason), the user gets some sort of diagnostic telling them their expectations 
may not be met. Because we're ignoring the attributes in the case where they're 
identical on both branches, I'd like to keep some form of diagnostic.


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

https://reviews.llvm.org/D85091

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


[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D85091#2252632 , @Mordante wrote:

> In D85091#2250657 , @rsmith wrote:
>
>> Looking specifically for attributes in the 'then' and 'else' cases of an 
>> `if` seems like a fine first pass at this, but I think this is the wrong way 
>> to lower these attributes in the longer term: we should have a uniform 
>> treatment of them that looks for the most recent prior branch and annotates 
>> it with weights. We could do that either by generating LLVM intrinsic calls 
>> that an LLVM pass lowers, or by having the frontend look backwards for the 
>> most recent branch and annotate it. I suppose it's instructive to consider a 
>> case such as:
>>
>>   void mainloop() noexcept; // probably terminates by calling `exit`
>>   void f() {
>> mainloop();
>> [[unlikely]];
>> somethingelse();
>>   }
>>
>> ... where the `[[unlikely]];` should probably cause us to split the 
>> `somethingelse()` out into a separate basic block that we mark as cold, but 
>> should not cause `f()` itself or its call to `mainloop()` to be considered 
>> unlikely -- except in the case where `mainloop()` can be proven to always 
>> terminate, in which case the implication is that it's unlikely that `f()` is 
>> invoked. Cases like this probably need the LLVM intrinsic approach to model 
>> them properly.
>
> We indeed considered similar cases and wondered whether it was really 
> intended to work this way. Since this approach seems a bit hard to explain to 
> users, I changed the code back to only allow the attribute on the 
> substatement of the `if` and `else`. For now I first want to implement the 
> simple approach, which I expect will be the most common use case. Once 
> finished we can see what the next steps will be.

+1 to the cautious approach. While I can understand how to implement what 
Richard is suggesting, I'm not convinced it results in readable code or that 
we're missing important optimization cases by using the more restrictive 
approach, so I'd like to get some field experience with users first.


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

https://reviews.llvm.org/D85091

___
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-03 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Okay, almost there..




Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

I don't think the if-statement is necessary. I thought we concluded we want to 
warn even for void*? Also note that "isMSVC" here is only checking the driver 
mode, i.e. just the user interface to the compiler. The user could still be 
compiling in MSVC mode.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:649
 
+  // Warns when typeid is used with RTTI disabled.
+  if (!getLangOpts().RTTIData)

s/RTTI/RTTI data/
(the "RTTI disabled" case is on line 646)



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = 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-}}
+}

Is the cast to void necessary? Same comment for the next file.


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] D67935: Add `#pragma clang deprecated`, used to deprecate macros

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D67935#2251145 , @erik.pilkington 
wrote:

> @aaron.ballman: Did you happen to get any feedback on macro attributes? There 
> are a growing number of macros that we'd like to be able to deprecate, and 
> having a workable solution would be useful for us.

Thank you for bringing this back up! I've worked on a patch to add preprocessor 
attributes to clang but have set it aside because it feels like it may be an 
awkward fit because of attribute arguments -- for instance, the preprocessor 
has no type system or AST, so we track values for things with `APValue` objects 
and there is no string `APValue` type, so it would be a fair amount of work to 
support `# [[deprecated("don't use baz")]] define BAZ`, let alone the more 
esoteric situations for arbitrary attributes. Another issue is that the 
preprocessor is sometimes shared between C/C++ frontends and, say, a FORTRAN 
frontend, which could suddenly introduce a new feature into a FORTRAN compiler 
without extra work.

Based on all of that, I think we should go with your approach of using a 
`#pragma` as it does solve a problem and isn't quite as novel. However, I am 
wondering about the design a bit -- why are you using a string literal to 
supply the macro name? It's my understanding that all preprocessing directives 
are executed at the same phase of translation (so you don't have to worry about 
`#define` changing the behavior of `#pragma` or `_Pragma`), so I would have 
expected a design more like:

  #define FOOBAR(x) whatever(x)
  #pragma clang deprecated(FOOBAR, "don't use FOOBAR, it will do bad things")

This feels like a more approachable way to expose the feature, to me, but I 
wonder if I'm missing something.

Other feedback is: should we diagnose if the pp-token for the macro identifier 
doesn't actually match the name of a macro? Or are we allowing constructs like:

  #pragma clang deprecated(FOOBAR, "don't use FOOBAR, twelve is a terrible 
number")
  #define FOOBAR 12




Comment at: clang/lib/Lex/Pragma.cpp:1544
+PP.Lex(Tok);
+if (!PP.FinishLexStringLiteral(Tok, EntityString,
+   "#pragma clang deprecated",

Do we want to allow arbitrary string literals, or only narrow character string 
literals? e.g., should we disallow something like: `#pragma clang 
deprecated(L"oops", U"hahahaha")` ?



Comment at: clang/test/Lexer/pragma-deprecated.c:17-20
+#pragma clang deprecated("\"flerp.h\"", "use \"flarp.h\" instead")
+
+// expected-warning@+1{{'flerp' is deprecated}}
+#pragma clang deprecated("flerp")

What is the utility of unconditionally emitting a diagnostic like this? We've 
already got `#warning`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67935

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


[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87
+Mutex globalMutex;
+void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
+

Can you add a test that uses `!::globalMutex`? I'd like to verify that lookup 
rules are honored for naming the mutex, so more complex examples with name 
hiding would also be useful.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:91
+  Mutex namespaceMutex;
+  void f() EXCLUSIVE_LOCKS_REQUIRED(!namespaceMutex);
+}

Can you also add a test that uses `!ns::namespaceMutex`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84604

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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:37
+return false;
+  constexpr StringRef ExitFunctions[] = {"_Exit", "exit", "quick_exit"};
+  return llvm::is_contained(ExitFunctions, FD->getName());

`abort()` as well?

How about in C++ mode calling `std::terminate()`?

One last class of problem functions are ones that are attributed as not 
returning (this would include user-defined functions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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


[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

2020-09-03 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 289703.
baloghadamsoftware added a comment.

Tests separated.


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

https://reviews.llvm.org/D85351

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

Index: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
===
--- clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
+++ clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -51,22 +51,65 @@
 TEST(TestReturnValueUnderConstructionChecker,
  ReturnValueUnderConstructionChecker) {
   EXPECT_TRUE(runCheckerOnCode(
-  R"(class C {
- public:
-   C(int nn): n(nn) {}
-   virtual ~C() {}
- private:
-   int n;
- };
-
- C returnC(int m) {
-   C c(m);
-   return c;
- }
-
- void foo() {
-   C c = returnC(1); 
- })"));
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ C returnC(int m) {
+   C c(m);
+   return c;
+ }
+
+ void foo() {
+   C c = returnC(1);
+ })"));
+
+  EXPECT_TRUE(runCheckerOnCode(
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   explicit C(): C(0) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ C returnC() {
+   C c;
+   return c;
+ }
+
+ void foo() {
+   C c = returnC();
+ })"));
+
+  EXPECT_TRUE(runCheckerOnCode(
+  R"(class C {
+ public:
+   C(int nn): n(nn) {}
+   virtual ~C() {}
+ private:
+   int n;
+ };
+
+ class D: public C {
+ public:
+   D(int nn): C(nn) {}
+   virtual ~D() {}
+ };
+
+ D returnD(int m) {
+   D d(m);
+   return d;
+ }
+
+ void foo() {
+   D d = returnD(1); 
+ })"));
 }
 
 } // namespace
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -132,10 +132,11 @@
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
   const auto *Init = ICC->getCXXCtorInitializer();
-  assert(Init->isAnyMemberInitializer());
   const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
   Loc ThisPtr = SVB.getCXXThis(CurCtor, LCtx->getStackFrame());
   SVal ThisVal = State->getSVal(ThisPtr);
+  if (Init->isBaseInitializer() || Init->isDelegatingInitializer())
+return ThisVal;
 
   const ValueDecl *Field;
   SVal FieldVal;
@@ -364,6 +365,8 @@
 case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
+  const auto *Init = ICC->getCXXCtorInitializer();
+  assert(Init->isAnyMemberInitializer());
   return addObjectUnderConstruction(State, ICC->getCXXCtorInitializer(),
 LCtx, V);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

This looks reasonable to me.




Comment at: clang/docs/LanguageExtensions.rst:2370
 
+* ``bcmp``
 * ``memchr``

Can you mention the deprecation issue here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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


[PATCH] D87064: Thread safety analysis: Test and document release_generic_capability

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman 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/D87064/new/

https://reviews.llvm.org/D87064

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


[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for documenting this, that would be confusing without spelling 
it out explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87065

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


[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/docs/ThreadSafetyAnalysis.rst:922
+// Releases *this and all underlying capabilities, if they are still held.
+// There is no warning on double unlock.
 ~MutexLocker() RELEASE() {

This makes it sound like we're missing a diagnostic, but IIRC, this is intended 
behavior. Maybe we want to say there is purposefully no warning on double 
unlock and why? Or add a FIXME if I'm remembering wrong and this isn't intended 
behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87066

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


[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: rsmith.
sammccall added a comment.

Neither of the testcases look like the right behavior to me, and I think the 
bug is elsewhere in clang.

The crux is we're forcing `decltype(N)` to be a (unique) dependent type, which 
feels wrong.
This isn't specific to error-dependence, the same is true in

  template  void foo() {
constexpr int N = K;
decltype(K) // dependent?
  }

ASTContext::getDecltypeType() has to determine whether 
http://eel.is/c++draft/temp.type#4 applies.
There are three behaviors:

- standard up to C++14: traversing the expr looking for a template param to be 
lexically included (this is my reading of the standard)
- what clang actually does: check instantiation dependence, which I think pulls 
in too many cases
- standard after http://wg21.link/cwg2064: check type dependence

I think it's probably OK to adopt the C++17 behavior for all versions (if I'm 
right that the current behavior is a bug).
@rsmith It's your DR, what do you think :-)

(Per offline discussion, this was reduced from a related bug that didn't 
involve decltype, but I think we should treat that one as distinct)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86048

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


[PATCH] D84414: [RISCV] Support Shadow Call Stack

2020-09-03 Thread Z. Zheng via Phabricator via cfe-commits
zzheng added a comment.

ping..

@jrt, @lenary, @asb, IMHO, the patch is in good shape now, all concerns raised 
in comments has been addressed/answered, is there any additional comments 
before we can land it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

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


[clang] d4f3903 - [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2020-09-03T14:39:50Z
New Revision: d4f3903131292d36b3bc22c28798b8e9dae20af6

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

LOG: [libTooling] Provide overloads of `rewriteDescendants` that operate 
directly on an AST node.

The new overloads apply directly to a node, like the
`clang::ast_matchers::match` functions, Rather than generating an
`EditGenerator` combinator.

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 9700d1ff539d..4bdcc8d5c329 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -380,6 +380,38 @@ EditGenerator rewriteDescendants(std::string NodeId, 
RewriteRule Rule);
 // RewriteRule API.  Recast them as such.  Or, just declare these functions
 // public and well-supported and move them out of `detail`.
 namespace detail {
+/// The following overload set is a version of `rewriteDescendants` that
+/// operates directly on the AST, rather than generating a Transformer
+/// combinator. It applies `Rule` to all descendants of `Node`, although not
+/// `Node` itself. `Rule` can refer to nodes bound in `Result`.
+///
+/// For example, assuming that "body" is bound to a function body in 
MatchResult
+/// `Results`, this will produce edits to change all appearances of `x` in that
+/// body to `3`.
+/// ```
+/// auto InlineX =
+/// makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+/// const auto *Node = Results.Nodes.getNodeAs("body");
+/// auto Edits = rewriteDescendants(*Node, InlineX, Results);
+/// ```
+/// @{
+llvm::Expected>
+rewriteDescendants(const Decl &Node, RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected>
+rewriteDescendants(const Stmt &Node, RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected>
+rewriteDescendants(const TypeLoc &Node, RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected>
+rewriteDescendants(const DynTypedNode &Node, RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult &Result);
+/// @}
+
 /// Builds a single matcher for the rule, covering all of the rule's cases.
 /// Only supports Rules whose cases' matchers share the same base "kind"
 /// (`Stmt`, `Decl`, etc.)  Deprecated: use `buildMatchers` instead, which

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 594e22f56b87..03921e0ea7de 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -242,7 +242,7 @@ class ApplyRuleCallback : public MatchFinder::MatchCallback 
{
 } // namespace
 
 template 
-static llvm::Expected>
+llvm::Expected>
 rewriteDescendantsImpl(const T &Node, RewriteRule Rule,
const MatchResult &Result) {
   ApplyRuleCallback Callback(std::move(Rule));
@@ -252,10 +252,43 @@ rewriteDescendantsImpl(const T &Node, RewriteRule Rule,
   return std::move(Callback.Edits);
 }
 
+llvm::Expected>
+transformer::detail::rewriteDescendants(const Decl &Node, RewriteRule Rule,
+const MatchResult &Result) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const Stmt &Node, RewriteRule Rule,
+const MatchResult &Result) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const TypeLoc &Node, RewriteRule Rule,
+const MatchResult &Result) {
+  return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected>
+transformer::detail::rewriteDescendants(const DynTypedNode &DNode,
+RewriteRule Rule,
+const MatchResult &Result) {
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+  if (const auto *Node = DNode.get())
+return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+
+  return llvm::make_error(
+  llvm::errc::invalid_argument,
+  "type unsupp

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4f390313129: [libTooling] Provide overloads of 
`rewriteDescendants` that operate directly on… (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87031

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -25,6 +25,7 @@
 using ::testing::IsEmpty;
 using transformer::cat;
 using transformer::changeTo;
+using transformer::rewriteDescendants;
 using transformer::RewriteRule;
 
 constexpr char KHeaderContents[] = R"cc(
@@ -568,6 +569,88 @@
   EXPECT_EQ(ErrorCount, 1);
 }
 
+//
+// We include one test per typed overload. We don't test extensively since that
+// is already covered by the tests above.
+//
+
+TEST_F(TransformerTest, RewriteDescendantsTypedStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("body");
+  assert(Node != nullptr && "body must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDecl) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("fun");
+  assert(Node != nullptr && "fun must be bound");
+  return transformer::detail::rewriteDescendants(
+  *Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(
+  functionDecl(
+  hasName("f"),
+  hasParameter(0, varDecl(hasTypeLoc(typeLoc().bind("parmType"),
+  [&IntToChar](const MatchFinder::MatchResult &R) {
+const auto *Node = R.Nodes.getNodeAs("parmType");
+assert(Node != nullptr && "parmType must be bound");
+return transformer::detail::rewriteDescendants(*Node, IntToChar, R);
+  }),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDynTyped) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(
+  makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+   [&InlineX](const MatchFinder::MatchResult &R) {
+ auto It = R.Nodes.getMap().find("body");
+ assert(It != R.Nodes.getMap().end() && "body must be bound");
+ return transformer::detail::rewriteDescendants(It->second,
+InlineX, R);
+   }),
+  Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/Re

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

2020-09-03 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D86874#inline-803844 , 
@martong wrote:

> I really feel that this check would have a better place in the implementation 
> of `eval`. This seems really counter-intuitive to do this stuff at the 
> Checker's level. Is there any reason why we can't do this in `eval`?
> `evalBinOpNN` could return with Unknown, and the state should remain 
> unchanged. Am I missing something?

Ah, sort of.
To make everything clear, I have to provide examples, building-blocks, 
reasoning and stuff, so please don't be mad if it's too long.
**I hope you have a warm cup of tee to read through this - seriously - you will 
need that!**

Diving in
-

It is really important to make a clear distinction between evaluating an 
expression according to the semantics of the //meta-language// or of the 
//object-language//, because we might get different answers for the same 
expression.

In fact, `evalBinOpNN` evaluates expressions according to the semantics of the 
//object-language//.

In our context, meta-language is //mathematics//, and the //object-language// 
is the semantics of the abstract machine of the c/c++ language.
In mathematics, there is no such thing as overflow or value ranges, unlike in 
C++.

Let's see an example:
Assuming that `x` is in range `[1,UINT_MAX]`.
`x + 1` will be in range `[2,ULL_MAX+1]` in the //meta-language//.
`x + 1` will be in range `[0,0]u[2,UINT_MAX]` or in `[2,UINT_MAX+1]` weather 
the type of `x` is capable representing the (+1) value and the overflow is 
well-defined or not.

Another valuable comment is that non-contiguous ranges (range sets) are not 
really useful.
Knowing that `x` is in range `[0,0]u[2,UINT_MAX]` doesn't help much if you want 
to prove that:
`x < 5` holds for all possible interpretations of `x`.
We can not disproof that either.

So, overflow/underflow can really screw the value ranges, preventing us from 
evaluating expressions over relational operators.
Which is technically accurate, but not satisfiable in all cases - like in the 
checker `ArrayBoundCheckerV2`.

---

Before describing why is it so problematic in this checker, I should give an 
overview, how this checker works.

Overview of the logic of the ArrayBoundCheckerV2


The checker checks `Location` accesses (aka. pointer dereferences).
We construct the `RegionRawOffsetV2` object of the access (Which consists of 
the //base region//, and the symbolic //byte offset// expression of the access).

Then we check, whether we access an element //preceding// the **first valid 
index** of the //base// memory region.
In other words, we check if the //byte offset// symbolic expression is **less 
then** 0:

- If YES:   Report that we access an out-of-bounds element.
- If NO:Infer the range constraints on the symbol and add the constraint to 
make this array access valid.
- If MAYBE: Infer and constrain, just as you would do in the previous case.

Then we check, whether we access an element //exceeding// the **last valid 
index** of the memory region.
In other words, we check if the //byte offset// symbolic expression is greater 
then or equal to the //extent// of the region:

- If YES:   Report the bug...
- If NO:Infer & constrain...
- If MAYBE: Infer & constrain...

However, in the checker, we don't check `byte offset < 0` directly.
We //simplify// the left part of the `byte offset < 0` inequality by 
substracting/dividing both sides with the constant `C`, if the `byte offset` is 
a `SymintExpr` of `SymExpr OP C` over the plus or multiplication operator (OP).
We do this //simplification// recursively.
In the end, we will have a //symbolic root// (call it `RootSym`) expression and 
a `C'` constant over the right-hand side of the original relational operator.
So, after the //simplification// process we get the `RootSym < C'` inequality, 
which we check.

This //simplification// is the //infer// operation in the pseudo-code.
And the //constrain// step is where we assume that the negation of `RootSym < 
C'` holds.

**SPOILER**: This //simplification// process is only **valid** using the 
semantics of the //meta-language//.

ElementRegions
--

Pointer values, which describe a particular element of a memory region, can get 
quite complex.
Even more complex, when we reinterpret cast a pointer to a different type.
In such cases, we might wrap the `SubRegion` symbol within an `ElementRegion` 
with the given target type and offset `0`.
Eg:

  void foo(int x) {
int buf[10];
char *p = (char*)(buf + 2) + 3;
^-- 2*sizeof(int) in bytes which is 8.
*p = 42; // Not UB.
int *q = (int*)p; // unaligned pointer!
*q = 5; // UB.
  
char *r = (char*)(buf + x) + 3;
*r = 66; // Might be safe, if x has a value to make it so.
  }



RegionRawOffsetV2
-

In the previous example `p` would have the `&Element{buf,11 S64b,char}` 
`SymbolRegionV

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 3 inline comments as done.
chrish_ericsson_atx added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13981
+bool overflow;
+llvm::APInt product(index);
+product += 1;

ebevhan wrote:
> What if index is wider than AddrBits, but the active bits are fewer? Then you 
> might miss out on triggering the overflow case in the multiplication.
Line 13984 checks for active bits of product being less than AddrBits, which is 
the same case (since product, by definition, has same width as index).  So I 
think this is covered.  If I've misunderstood, please re-ping.



Comment at: clang/lib/Sema/SemaChecking.cpp:13992
+  MaxElems =
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;

ebevhan wrote:
> Should this not be AddrBits + 1 if you want to add 1 below?
AddrBits + 1 would also work.  I chose AddrBits << 1 figuring that the width 
would end up being a nice clean power of 2, but that's not necessarily true.  
Functionally, the effect of either approach is identical.  I suppose + 1 is a 
bit more obvious, though.  I'll make this change.



Comment at: clang/lib/Sema/SemaChecking.cpp:13993
+  MaxElems.zext(std::max(AddrBits << 1, apElemBytes.getBitWidth()));
+  MaxElems += 1;
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())

ebevhan wrote:
> Though, why is the +1 here? Isn't this already the maximum number of elements?
Initial value of MaxElems is APInt::getMaxValue(AddrBits), which is the index 
of the last addressable CharUnit in the address space.  Adding 1 makes it the 
total number of addressable CharUnits in the address space, which is what we 
want as the numerator for computing total number of elements of a given size 
that will fit in that address space.




Comment at: clang/lib/Sema/SemaChecking.cpp:13994-13997
+  if (MaxElems.getBitWidth() < apElemBytes.getBitWidth())
+MaxElems = MaxElems.zext(apElemBytes.getBitWidth());
+  else if (apElemBytes.getBitWidth() < MaxElems.getBitWidth())
+apElemBytes = apElemBytes.zext(MaxElems.getBitWidth());

ebevhan wrote:
> MaxElems should already be at a sufficient width here because of the earlier 
> max. You can probably just do apElemBytes = 
> apElemBytes.zextOrTrunc(MaxElems.getBitWidth())?
Yep -- you are right.  index and apElemBytes are already guaranteed to have 
equal width (>= AddrBits), and MaxElems is guaranteed to have the same width or 
double width.  So the single zext of apElemBytes will do fine.  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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


[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

2020-09-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Nit on the commit message: I think this is TemplateArgumentLoc 48 -> 32 bytes, 
and DynTypedNode 56 -> 40 bytes.




Comment at: clang/include/clang/AST/TemplateBase.h:415
 
-public:
-  constexpr TemplateArgumentLocInfo() : Template({nullptr, nullptr, 0, 0}) {}
+  llvm::PointerIntPair PointerAndKind;
 

Can you use PointerUnion, and save even more of the 
boilerplate?



Comment at: clang/include/clang/AST/TemplateBase.h:429
+  auto *T = getTemplate();
+  T->Ctx->Deallocate(T);
+}

this is a no-op, and thus not worth stashing a pointer to Ctx for!

It also doesn't delete T, and it's probably best to do that even if it's 
(currently) a no-op



Comment at: clang/include/clang/AST/TemplateBase.h:429
+  auto *T = getTemplate();
+  T->Ctx->Deallocate(T);
+}

sammccall wrote:
> this is a no-op, and thus not worth stashing a pointer to Ctx for!
> 
> It also doesn't delete T, and it's probably best to do that even if it's 
> (currently) a no-op
If you're going to destroy T in the destructor, then you can't have trivial 
copies, as the second one is pointing at deallocated memory.
(Well, apart from the fact that deallocation does nothing).

So I think we probably either want:
 - allocation on ASTContext, trivial copies, no deallocation
 - allocation on heap, copies reallocate
 - allocation on heap using shared_ptr
 - copies disallowed (but I think we rely on them being available)



Comment at: clang/include/clang/AST/TemplateBase.h:443
   SourceLocation EllipsisLoc) {
-Template.Qualifier = QualifierLoc.getNestedNameSpecifier();
-Template.QualifierLocData = QualifierLoc.getOpaqueData();
-Template.TemplateNameLoc = TemplateNameLoc.getRawEncoding();
-Template.EllipsisLoc = EllipsisLoc.getRawEncoding();
+T *Template = Ctx.Allocate();
+Template->Qualifier = QualifierLoc.getNestedNameSpecifier();

this is not "heap allocation" in the usual sense - allocating in the context's 
slab allocator is cheaper but can't be deallocated.

Not sure how this compares to simply `new T` instead.

If not, then as with delete, I'd prefer `new (Ctx) T` (I think it's correct 
without as long as T is trivial, but less obvious)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87080

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


[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment.

In D77491#2254065 , @rjmccall wrote:

> The builtins with custom type-checking are all true intrinsics like 
> `__builtin_operator_new` and so on.  They really can't be validly declared by 
> the user program.  The thing that seems most likely to avoid random compiler 
> crashes would be to either forbid explicit declarations of them or treat 
> those as no longer being builtins.  If we need to maintain compatibility with 
> people making custom declarations, we would need to always treat them as 
> builtins and run the risk of crashing if someone declares one with a bad 
> signature.  But I don't think it's unfair of us to break those people; that 
> is seriously not reasonable user behavior.
>
> It's possible that custom declarations are people trying to create 
> compatibility shims for compilers that don't provide these as builtins.  
> Those people should be guarding their custom declarations, preferably with 
> `__has_builtin`.

I fully agree.

However, I believe you forget to account for the example that I brought up.
In particular, MSVC's header `vadefs.h` includes a declaration of 
`__va_start()`, which would cause almost any code including standard headers to 
fail to compile on Windows.
This issue isn't isolated to some old MSVC version, as the declaration is still 
there in the latest Visual Studio Preview version 14.28.29213.

How about turning it into an error only on non-Windows?
Though keeping that as a followup might be even better, as this will probably 
be merged into 11.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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


[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

2020-09-03 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments.



Comment at: clang/docs/ThreadSafetyAnalysis.rst:908
+// Assumes mu is held, implicitly acquires *this and connects it to mu.
+MutexLocker(t_mutex &mu, adopt_lock_t) REQUIRES(mu) : mut(mu) {}
+

`s/t_mutex/Mutex/g`



Comment at: clang/docs/ThreadSafetyAnalysis.rst:922
+// Releases *this and all underlying capabilities, if they are still held.
+// There is no warning on double unlock.
 ~MutexLocker() RELEASE() {

aaron.ballman wrote:
> This makes it sound like we're missing a diagnostic, but IIRC, this is 
> intended behavior. Maybe we want to say there is purposefully no warning on 
> double unlock and why? Or add a FIXME if I'm remembering wrong and this isn't 
> intended behavior.
It is intentional (we have special handling for the destructor), and I also 
think it makes sense. I just wanted to clarify the difference between this and 
`Unlock`: calling `Unlock` twice is not allowed, but calling `Unlock` and then 
(implicitly) the destructor is allowed.

I could rephrase this as "There is no warning if the scope was already unlocked 
before." Could also add an "intentional" or "deliberate", but since it's 
documented I think the reader can infer that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87066

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


[clang] c9239b2 - [Analyzer][docs][NFC] Fix typo in code example

2020-09-03 Thread Jan Korous via cfe-commits

Author: Jan Korous
Date: 2020-09-03T09:28:34-07:00
New Revision: c9239b2bf5f00b58aaa431955f24013e0cada0a3

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

LOG: [Analyzer][docs][NFC] Fix typo in code example

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 3b378f735ebc..7a294f916bcf 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1747,7 +1747,7 @@ Check for integer to enumeration casts that could result 
in undefined values.
  void foo() {
TestEnum t = static_cast(-1);
// warn: the value provided to the cast expression is not in
-the valid range of values for the enum
+   //   the valid range of values for the enum
 
 .. _alpha-cplusplus-InvalidatedIterator:
 



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


[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Ahmed Bougacha via Phabricator via cfe-commits
ab created this revision.
Herald added subscribers: cfe-commits, danielkiss, cmtice, rupprecht, 
dexonsmith, steven_wu, hiraditya, kristof.beyls, arichardson.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.
ab requested review of this revision.
Herald added a subscriber: ormris.

This is the first of many Pointer Authentication-related patches we're happy to 
finally upstream =]
For a high-level overview, see our llvm-dev RFC: 
http://lists.llvm.org/pipermail/llvm-dev/2019-October/136091.html, as well as 
the devmtg talk we did at the same time last year.
For concrete code that builds on this, see the staging PR in 
apple/llvm-project: https://github.com/apple/llvm-project/pull/14.  Though 
we've made changes downstream since then, the general concepts and added 
constructs are mostly identical.

Per commit message:

  This also teaches MachO writers/readers about the MachO cpu subtype,
  beyond the minimal subtype reader support present at the moment.
  
  This also defines a preprocessor macro to allow users to distinguish
  __arm64__ from __arm64e__.
  
  arm64e defaults to an "apple-a12" CPU, which supports v8.3a, allowing
  pointer-authentication codegen.
  It also currently defaults to ios14+ and macos11+.

If it helps, we can obviously split this patch further, but I feel like this 
all belongs together, as the core boilerplate needed for new (darwin) targets.

Note that this adds a `Triple::isArm64e()`, which is a bit of a departure from 
prior subarches, in part because we check it more often.  The obvious 
alternative would be to compare `Triple::getArchName()` with "arm64e";  I'm 
fine with either.   However, I would like to preserve the "arm64e" naming, as 
we refer to that extensively in Darwin-land.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87095

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arclite-link.c
  clang/test/Driver/target-triple-deployment.c
  clang/test/Preprocessor/arm64e.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/BinaryFormat/MachO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/LTOModule.cpp
  llvm/lib/Object/MachOObjectFile.cpp
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
  llvm/test/MC/AArch64/arm64e-subtype.s
  llvm/test/MC/AArch64/arm64e.s
  llvm/test/MC/MachO/AArch64/arm-darwin-version-min-load-command.s
  llvm/test/tools/llvm-dwarfdump/AArch64/arm64e.ll
  llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
  llvm/test/tools/llvm-readobj/macho-arm64e.test
  llvm/unittests/ADT/TripleTest.cpp
  llvm/utils/UpdateTestChecks/asm.py

Index: llvm/utils/UpdateTestChecks/asm.py
===
--- llvm/utils/UpdateTestChecks/asm.py
+++ llvm/utils/UpdateTestChecks/asm.py
@@ -335,6 +335,7 @@
   'amdgcn': (scrub_asm_amdgpu, ASM_FUNCTION_AMDGPU_RE),
   'arm': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
   'arm64': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_RE),
+  'arm64e': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_DARWIN_RE),
   'arm64-apple-ios': (scrub_asm_arm_eabi, ASM_FUNCTION_AARCH64_DARWIN_RE),
   'armv7-apple-ios' : (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_IOS_RE),
   'armv7-apple-darwin': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_DARWIN_RE),
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1590,5 +1590,10 @@
 Triple T = Triple("aarch64_be");
 EXPECT_EQ(Triple::aarch64_be, T.getArch());
   }
+  {
+Triple T = Triple("arm64e");
+EXPECT_EQ(Triple::aarch64, T.getArch());
+EXPECT_EQ(Triple::AArch64SubArch_arm64e, T.getSubArch());
+  }
 }
 } // end anonymous namespace
Index: llvm/test/tools/llvm-readobj/macho-arm64e.test
===
--- /dev/null
+++ llvm/test/tools/llvm-readobj/macho-arm64e.test
@@ -0,0 +1,17 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-readobj -h %t.o | FileCheck %s
+
+# CHECK: Magic: Magic64 (0xFEEDFACF)
+# CHECK: CpuType: Arm64 (0x10C)
+# CHECK: CpuSubType: CPU_SUBTYPE_ARM64E (0x2)
+
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x010C
+  cpusubtype:  0x0002
+  filetype:0x0001
+  ncmds:   0
+  sizeofcmds:  0
+  flags:   0x
+  reserved:0x
Index: llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
===
--- llvm/test/tools/llvm-objdump/MachO/universal-arm64.test
+++ llvm/test/tools/llvm-objdump/MachO/universal-arm6

[PATCH] D87097: [analyzer][StdLibraryFunctionsChecker] Do not match based on the restrict qualifier in C++

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

The "restrict" keyword is illegal in C++, however, many libc
implementations use the "__restrict" compiler intrinsic in functions
prototypes. The "__restrict" keyword qualifies a type as a restricted type
even in C++.
In case of any non-C99 languages, we don't want to match based on the
restrict qualifier because we cannot know if the given libc implementation
qualifies the paramter type or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87097

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

Index: clang/test/Analysis/std-c-library-functions-restrict.cpp
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-restrict.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C++, however, we are more
+// indulgent and we do not match based on this qualifier. Thus, the given
+// signature should match for both of the declarations below, i.e the summary
+// should be loaded for both of them.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+// The below declaration is illegal, "restrict" is not a keyword in C++.
+// void __test_restrict_param_2(void *restrict p);
+
+// CHECK: Loaded summary for: void __test_restrict_param_0(void *p)
+// CHECK: Loaded summary for: void __test_restrict_param_1(void *__restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-restrict.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-restrict.c
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C the signature should match only
+// if the restrict qualifier is there on the parameter. Thus, the summary
+// should be loaded for the last two declarations only.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+void __test_restrict_param_2(void *restrict p);
+
+// CHECK-NOT: Loaded summary for: void __test_restrict_param_0
+// CHECK: Loaded summary for: void __test_restrict_param_1(void *restrict p)
+// CHECK: Loaded summary for: void __test_restrict_param_2(void *restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -744,21 +744,38 @@
 bool StdLibraryFunctionsChecker::Signature::matches(
 const FunctionDecl *FD) const {
   assert(!isInvalid());
-  // Check number of arguments:
+  // Check the number of arguments.
   if (FD->param_size() != ArgTys.size())
 return false;
 
-  // Check return type.
-  if (!isIrrelevant(RetTy))
-if (RetTy != FD->getReturnType().getCanonicalType())
+  // The "restrict" keyword is illegal in C++, however, many libc
+  // implementations use the "__restrict" compiler intrinsic in functions
+  // prototypes. The "__restrict" keyword qualifies a type as a restricted type
+  // even in C++.
+  // In case of any non-C99 languages, we don't want to match based on the
+  // restrict qualifier because we cannot know if the given libc implementation
+  // qualifies the paramter type or not.
+  auto RemoveRestrict = [&FD](QualType T) {
+if (!FD->getASTContext().getLangOpts().C99)
+  T.removeLocalRestrict();

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289753.
chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added a comment.

Addressed reviewer feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S bigblock[3276];
+};
+
+struct BQ bq[];
+
+void f5() {

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

2020-09-03 Thread Amy Kwan via Phabricator via cfe-commits
amyk added a comment.

Ping.


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] D82726: [PowerPC] Implement Vector Count Mask Bits builtins in LLVM/Clang

2020-09-03 Thread Amy Kwan via Phabricator via cfe-commits
amyk added a comment.

Ping.


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] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-03 Thread Daniel Grumberg via Phabricator via cfe-commits
dang added a comment.

Yes of course. This is a prerequisite for some other changes that are waiting 
to land and need polishing. Thanks for doing the revert. I will investigate the 
failures and recommit it when appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86290

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


[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto &ASTC = getASTContext();

ebevhan wrote:
> chrish_ericsson_atx wrote:
> > ebevhan wrote:
> > > It might simplify the patch to move this condition out of the tree and 
> > > just early return for the other case. That is:
> > > 
> > > ```
> > > if (isUnboundedArray) {
> > >   if (!(index.isUnsigned() || !index.isNegative()))
> > > return;
> > > 
> > >   ...
> > >   return;
> > > }
> > > 
> > > if (index.isUnsigned() ...
> > > ```
> > There's a bit more code (starting at line 14094 in this patch set) that 
> > applies in all cases, so an early return here would prevent the "Array 
> > declared here" note from being generated.
> Ah, the note.
> 
> I wonder if it wouldn't be cleaner (and avoid indenting the entire block) if 
> that was just duplicated.
Hard to say which is cleaner -- it's a tradeoff.  Nesting level would be 
shallower if that code was duplicated, but then again, the duplication 
increases the chance of an incomplete fix should an issue be discovered in that 
code.  Overall code would be slightly longer, as well (adding about 16 lines of 
code, but removing only 4).   To me, the current strategy feels more surgical, 
but I'll change it if you feel more strongly about it than I do.   Please 
re-ping if you do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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


[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

2020-09-03 Thread Albion Fung via Phabricator via cfe-commits
Conanap added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:993
+ [(set v16i8:$vD, 
(int_ppc_altivec_vexpandbm
+  v16i8:$vB))]>;
   def VEXPANDHM : VXForm_RD5_XO5_RS5<1602, 1, (outs vrrc:$vD), (ins vrrc:$vB),

Nit: Please make this indentation inline with the others


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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


[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I didn't see the specific example, sorry.  I agree that my description is more 
applicable to builtins in the `__builtin` namespace, which describes most of 
the builtins with custom typechecking.  Is the problem just `__va_start`?

If we have to treat all declarations as builtins for the custom-typechecking 
builtins just to land this patch, I don't think that's the worst result in the 
world, and we can incrementally go from there.  `__va_start` actually has a 
signature, it just effectively has optional arguments, which is something we 
could definitely teach the builtins database and signature-matcher about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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


[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

2020-09-03 Thread Albion Fung via Phabricator via cfe-commits
Conanap accepted this revision.
Conanap added a comment.
This revision is now accepted and ready to land.

Minor nit, okay if changed for commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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


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

2020-09-03 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon created this revision.
RKSimon added reviewers: craig.topper, spatel, nikic, lebedev.ri.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.
RKSimon requested review of this revision.

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 intrinsics to emit the generic equivalents 
instead of the icmp+sub+select code pattern.


Repository:
  rG LLVM Github Monorepo

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 @@
 ;

[PATCH] D86993: Document Clang's expectations of the C standard library.

2020-09-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a subscriber: t.p.northover.
rjmccall added a comment.

@t.p.northover says it's complicated.  `memcpy`, `memmove`, `memset`, and 
`bzero` are (I think) the only ones that LLVM will potentially synthesize from 
nothing and therefore need to be present even in freestanding builds; that's 
probably okay for us to guarantee.  That's probably also a good place to 
document the supported way to write those functions in libraries (just 
`-fno-builtin`, IIRC?).

In hosted builds, we should document that we assume the existence of the 
standard C library for the target platform, potentially including non-standard 
functions like `exp10`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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


[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, ahatanak, fhahn.
Herald added subscribers: ributzka, dexonsmith, jkorous.
erik.pilkington requested review of this revision.

Previously, this code discarded the result of CheckPlaceholderExpr for 
non-matrix subexpressions. Not only is this wasteful, but it was creating a 
Warc-repeated-use-of-weak false-positive on the attached testcase, since the 
discarded expression was still registered as a use of the weak property. (This 
was introduced in D76791 )

rdar://66162246


https://reviews.llvm.org/D87102

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/arc-repeated-weak.mm


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,23 @@
   base = result.get();
 }
   }
+
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

Hi, thanks for getting started on upstreaming this!

But I was wondering: shouldn't this be the *last* patch? I was imagining that 
you would first upstream support for the `-fptrauth-*` flags, and then at the 
end you would add an `arm64e` subarch that turns them on by default. Otherwise 
the upstream compiler will temporarily produce ABI-incompatible objects when 
targeting `arm64e`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87095

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


[PATCH] D86993: Document Clang's expectations of the C standard library.

2020-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/docs/Toolchain.rst:289
+
+C standard library implementations that do not guarantee these properties are
+incompatible with Clang and LLVM (and with several other major compilers).

While I think it's good that we're documenting this, it is really troubling 
that Clang community's perspective is "we can't work with a conforming C 
standard library" without filing any DRs to WG14 about why we cannot conform, 
especially if other major compilers are in a similar boat. Do you have any 
interest in filing a DR to see if we can change the C standard?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits
rahmanl updated this revision to Diff 289765.
rahmanl marked 5 inline comments as done.
rahmanl added a comment.

- Address +MaskRay's comments:
- Change the check prefix simply to "CHECK" for basic-block-sections-labels.ll
- Change the triple to x86_64 for this test.
- nits.
- Remove the "-LABEL" feature from basic-block-sections-labels.ll and use 
precise BB indices.
- Remove the assertion in emitBBAddrMapSection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"a

[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 289768.
akhuang added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,


Index: clang/test/CodeGenCXX/debug-info-class.cpp
===
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits
rahmanl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

MaskRay wrote:
> BBAddrMapSection is always non-null. Delete the if.
I believe we return null for non-ELF environment (Please refer to 
MCObjectFileInfo::getBBAddrMapSection).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

___
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-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

hans wrote:
> I don't think the if-statement is necessary. I thought we concluded we want 
> to warn even for void*? Also note that "isMSVC" here is only checking the 
> driver mode, i.e. just the user interface to the compiler. The user could 
> still be compiling in MSVC mode.
My bad. I thought you meant to use the previous version, so I reverted this 
region.
Like we concluded, we want to warn even for void*. This only applies to 
clang-cl, not clang, right? This is the purpose of this if. If it's in 
clang-cl, warn regardless of destination type. If it's in clang, don't warn for 
void*, like line 887 which doesn't emit error for void* destination type. 

If RTTI is false, RTTIData is also false 
(https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
 There is a test 
https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
 which should not be warned, right?



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = 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-}}
+}

hans wrote:
> Is the cast to void necessary? Same comment for the next file.
Yes, to suppress the warning of unused expression.


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] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

2020-09-03 Thread Ahmed Bougacha via Phabricator via cfe-commits
ab added a comment.

In D87095#2255010 , @pcc wrote:

> But I was wondering: shouldn't this be the *last* patch? I was imagining that 
> you would first upstream support for the `-fptrauth-*` flags, and then at the 
> end you would add an `arm64e` subarch that turns them on by default. 
> Otherwise the upstream compiler will temporarily produce ABI-incompatible 
> objects when targeting `arm64e`.

Good point, you're right.  ABI compatibility has obviously been a major issue 
as we keep iterating on this, so we added the concept of ptrauth ABI versions 
to the arm64e mach-o cpusubtype.  With this patch, upstream clang produces 
"unversioned" binaries, which are rejected on macOS11/iOS14.  Once we're done 
upstreaming, I'll make the final patch to match the ABI version.  In the 
meantime, that means upstream can't produce anything that runs on macOS, which 
should avoid this problem (and lets us use arm64e checks in the remaining 
patches, though I can obviously extract that to have the patches done the other 
way)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87095

___
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-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 289769.
zequanwu marked an inline comment as done.
zequanwu added a comment.

address comment.


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,20 @@
+// 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)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,20 @@
+// RUN: %clang_cc1 %s -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)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,16 @@
 return;
   }
 
+  // Warns when dynamic_cast is used with RTTI data disabled.
+  if (!Self.getLangOpts().RTTIData) {
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (isClangCL || !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
@@ -7433,6 +7433,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
@@ -1229,3 +1229,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: dblaikie, nemanjai.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay requested review of this revision.

Both tests operate on `%T/testbin`. If the two tests run concurrently,
one may fail.

This is likely the root cause of flaky failures reported by
https://lists.llvm.org/pipermail/llvm-dev/2020-September/144781.html

https://llvm.org/docs/CommandGuide/lit.html says:

`%T parent directory of %t (not unique, deprecated, do not use)`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87103

Files:
  clang/test/Driver/config-file3.c
  clang/test/Driver/target-override.c

Index: clang/test/Driver/target-override.c
===
--- clang/test/Driver/target-override.c
+++ clang/test/Driver/target-override.c
@@ -1,16 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/i386-clang
+// RUN: rm -rf %t && mkdir %t
+// RUN: ln -s %clang %t/i386-clang
 
 // Check if invocation of "foo-clang" adds option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
 // CHECK-TG1: Target: i386
 
 // Check if invocation of "foo-clang -target bar" overrides option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
 // CHECK-TG2: Target: x86_64
Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in different directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
 //
-// RUN: rm %T/te

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:388
+llvm::Expected>
+rewriteDescendants(const Decl &Node, RewriteRule Rule,
+   const ast_matchers::MatchFinder::MatchResult &Result);

ymandel wrote:
> gribozavr2 wrote:
> > I agree that these functions provide very useful functionality, however I 
> > feel uneasy about putting a function that returns an `EditGenerator` and a 
> > function that actually executes the refactoring into the same overload set. 
> > The first is a part of a DSL, the second is a regular function. Do you 
> > think this is a problem worth solving?
> > 
> > IDK what exactly to suggest though. A namespace for DSL functions like we 
> > have in AST matchers?
> That's a really good point. These new functions are definitely a part of an 
> (implicit) lower-level library that improves manipulation of the AST 
> directly.  We don't have any appropriate library yet for this -- SourceCode 
> is in this spirit but even simpler than RewriteRule, which in fact depends on 
> it.
> 
> For now,  I'll put these in the `detail` which is (in some sense) the 
> collection of low level functions for which we need to solve this same 
> problem. WDYT? (I wasn't quite clear about the comparison with ast matchers, 
> since both the matchers DSL and the `match` functions (on which this is 
> loosely based) are in the same namespace).
> I wasn't quite clear about the comparison with ast matchers, since both the 
> matchers DSL and the match functions (on which this is loosely based) are in 
> the same namespace

Oh right -- I mistakenly thought that `clang::ast_matchers` only contains the 
DSL. I think your choice to put functions into `detail` looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87031

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


[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

I feel like this may be trying to do too many things at once.
Was there an RFC?
There are several patches here:

1. langref
2. llvm side of the patch (maybe should be in the previous patch)
3. `SideEntry` changes - missing astdump changes, serialization/deserialization
4. the rest of the owl




Comment at: clang/lib/CodeGen/CGCleanup.cpp:777
+
+  // mark EHa scope end for fall-through flow
+  if (IsEHa && getInvokeDest())

Here and elsewhere, the comments start from Capital Letter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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


[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Aaron Smith via Phabricator via cfe-commits
asmith added a comment.

Yes there was an RFC and discussion and several requests for comments.
http://lists.llvm.org/pipermail/llvm-dev/2020-March/140541.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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


LLVM buildmaster will be updated and restarted tonight

2020-09-03 Thread Galina Kistanova via cfe-commits
 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM PST today.

Thanks

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


[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-09-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D80344#2255090 , @asmith wrote:

> Yes there was an RFC and discussion and several requests for comments.
> http://lists.llvm.org/pipermail/llvm-dev/2020-March/140541.html

It is probably good to state so in the patch's description.
But as far as i can tell the disscussion died down without an explicit 
acceptance?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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


[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

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


[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87103

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


[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-03 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added a comment.

ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

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


[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289785.
chrish_ericsson_atx added a comment.

Refactored as ebevhan suggested to simplify patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S bigblock[3276];
+};
+
+struct BQ bq[];
+
+void f5() {
+  ++bq[0].bigblock[0].a;
+  ++b

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked an inline comment as done.
chrish_ericsson_atx added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto &ASTC = getASTContext();

chrish_ericsson_atx wrote:
> ebevhan wrote:
> > chrish_ericsson_atx wrote:
> > > ebevhan wrote:
> > > > It might simplify the patch to move this condition out of the tree and 
> > > > just early return for the other case. That is:
> > > > 
> > > > ```
> > > > if (isUnboundedArray) {
> > > >   if (!(index.isUnsigned() || !index.isNegative()))
> > > > return;
> > > > 
> > > >   ...
> > > >   return;
> > > > }
> > > > 
> > > > if (index.isUnsigned() ...
> > > > ```
> > > There's a bit more code (starting at line 14094 in this patch set) that 
> > > applies in all cases, so an early return here would prevent the "Array 
> > > declared here" note from being generated.
> > Ah, the note.
> > 
> > I wonder if it wouldn't be cleaner (and avoid indenting the entire block) 
> > if that was just duplicated.
> Hard to say which is cleaner -- it's a tradeoff.  Nesting level would be 
> shallower if that code was duplicated, but then again, the duplication 
> increases the chance of an incomplete fix should an issue be discovered in 
> that code.  Overall code would be slightly longer, as well (adding about 16 
> lines of code, but removing only 4).   To me, the current strategy feels more 
> surgical, but I'll change it if you feel more strongly about it than I do.   
> Please re-ping if you do.
I reconsidered and made the changes you suggested.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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


[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song 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 rG6e09722b27ed: [test] Use %t instead of %T to remove race 
conditions between config-file3.c… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87103

Files:
  clang/test/Driver/config-file3.c
  clang/test/Driver/target-override.c

Index: clang/test/Driver/target-override.c
===
--- clang/test/Driver/target-override.c
+++ clang/test/Driver/target-override.c
@@ -1,16 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/i386-clang
+// RUN: rm -rf %t && mkdir %t
+// RUN: ln -s %clang %t/i386-clang
 
 // Check if invocation of "foo-clang" adds option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck -check-prefix CHECK-TG1 %s
 // CHECK-TG1: Target: i386
 
 // Check if invocation of "foo-clang -target bar" overrides option "-target foo".
 //
-// RUN: %T/testbin/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
+// RUN: %t/i386-clang -c -no-canonical-prefixes -target x86_64 %s -### 2>&1 | FileCheck -check-prefix CHECK-TG2 %s
 // CHECK-TG2: Target: x86_64
Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in different directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
 //
-// RUN: rm %T/testdmode/qqq-clang-g++.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+// RUN: rm %t/t

[clang] 6e09722 - [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-09-03T12:28:53-07:00
New Revision: 6e09722b27ed4d48dfc668b0efc2aed88d701ebf

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

LOG: [test] Use %t instead of %T to remove race conditions between 
config-file3.c and target-override.c

Both tests operate on `%T/testbin`. If the two tests run concurrently,
one may fail.

This is likely the root cause of flaky failures reported by
https://lists.llvm.org/pipermail/llvm-dev/2020-September/144781.html

https://llvm.org/docs/CommandGuide/lit.html says:

`%T parent directory of %t (not unique, deprecated, do not use)`

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/test/Driver/config-file3.c
clang/test/Driver/target-override.c

Removed: 




diff  --git a/clang/test/Driver/config-file3.c 
b/clang/test/Driver/config-file3.c
index 148646c2ebbf..fc5c286553ad 100644
--- a/clang/test/Driver/config-file3.c
+++ b/clang/test/Driver/config-file3.c
@@ -1,14 +1,15 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: rm -rf %t && mkdir %t
+
 //--- If config file is specified by relative path (workdir/cfg-s2), it is 
searched for by that path.
+
+// RUN: mkdir -p %t/workdir/subdir
+// RUN: echo "@subdir/cfg-s2" > %t/workdir/cfg-1
+// RUN: echo "-Wundefined-var-template" > %t/workdir/subdir/cfg-s2
 //
-// RUN: mkdir -p %T/workdir
-// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
-// RUN: mkdir -p %T/workdir/subdir
-// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
-//
-// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck 
%s -check-prefix CHECK-REL )
+// RUN: ( cd %t && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck 
%s -check-prefix CHECK-REL )
 //
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
@@ -16,12 +17,11 @@
 
 //--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
 //
-// RUN: rm -rf %T/testdmode
-// RUN: mkdir -p %T/testdmode
-// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
 //
 // FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 // FULL-NAME: -Wundefined-func-template
@@ -31,20 +31,20 @@
 // (As the clang executable and symlink are in 
diff erent directories, this
 // requires specifying the path via --config-*-dir= though.)
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= 
--config-user-dir=%T/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix 
SYMLINK
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= 
--config-user-dir=%t/testdmode -c %s -### 2>&1 | FileCheck %s -check-prefix 
SYMLINK
 //
 // SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
 //
 //--- File specified by --config overrides config inferred from clang 
executable.
 //
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config 
--config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | 
FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config 
--config-user-dir= --config i386-qqq -c -no-canonical-prefixes %s -### 2>&1 | 
FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 //
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if 
qqq-clang-g++.cfg is not found.
 //
-// RUN: rm %T/testdmode/qqq-clang-g++.cfg
-// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+// RUN: rm %t/testdmode/qqq-clang-g++.cfg
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c 
-no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
 //
 // SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
 // SHORT-NAME: -Werror
@@ -53,11 +53,10 @@
 
 //--- Config files are searched for in binary directory as well.
 //
-// RUN: rm -rf %T/testbin
-// RUN: mkdir -p %T/testbin
-// RUN: ln -s %clang %T/testbin/clang
-// RUN: echo "-Werror" > %T/testbin/aaa.cfg
-// RUN: %T/testbin/clan

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:2370-2398
+* ``bcmp``
 * ``memchr``
-* ``memcmp`` (and its deprecated BSD / POSIX alias ``bcmp``)
+* ``memcmp``
+* ``memcpy``
+* ``memmove``
+* ``memset``
+* ``strcat``

aaron.ballman wrote:
> Can you mention the deprecation issue here?
Do we really provide constant evaluation support for all of these functions?



Comment at: clang/docs/LanguageExtensions.rst:2381
+* ``strcspn``
+* ``strerror``
 * ``strlen``

Do we really provide `__builtin_strerror`? I don't see it below.



Comment at: clang/docs/LanguageExtensions.rst:2425-2435
-Clang provides constant expression evaluation support for builtin forms of the
-following functions from the C standard library headers
- and :
-
-* ``memcpy``
-* ``memmove``
-* ``wmemcpy``

You've moved this from the section where we describe the constant evaluation 
rules for these functions to a section where we describe the constant 
evaluation rules for string functions; those rules are quite different.



Comment at: clang/docs/LanguageExtensions.rst:2427-2428
+
+Clang provides support for builtins forms of the following functions from the C
+standard library header :
+

We should say how these "builtin forms" are named and what they're for: that we 
provide a `__builtin_`-prefixed version of each of these functions that has the 
same signature and semantics as the function from the C standard library but 
doesn't require a header to be included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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


[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

2020-09-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

It seems to me that adding new `__builtin_*` functions for GCC compatibility 
and adding new `LIBBUILTIN`s are unrelated changes, and it might be clearer to 
split them up into two commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

___
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-03 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:897
+  DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+  Self.Diag(OpRange.getBegin(),

zequanwu wrote:
> hans wrote:
> > I don't think the if-statement is necessary. I thought we concluded we want 
> > to warn even for void*? Also note that "isMSVC" here is only checking the 
> > driver mode, i.e. just the user interface to the compiler. The user could 
> > still be compiling in MSVC mode.
> My bad. I thought you meant to use the previous version, so I reverted this 
> region.
> Like we concluded, we want to warn even for void*. This only applies to 
> clang-cl, not clang, right? This is the purpose of this if. If it's in 
> clang-cl, warn regardless of destination type. If it's in clang, don't warn 
> for void*, like line 887 which doesn't emit error for void* destination type. 
> 
> If RTTI is false, RTTIData is also false 
> (https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
>  There is a test 
> https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
>  which should not be warned, right?
> Like we concluded, we want to warn even for void*. This only applies to 
> clang-cl, not clang, right?

Oh, right, we need to do the "even for void*" part only when using the 
Microsoft ABI. It's not about clang or clang-cl, those are just different user 
interfaces really, but about what platform (processor, abi, etc.) the compiler 
is targeting. It's possible to target Windows with regular clang, not just 
clang-cl.

Inside Sema, you can check Context.getTargetInfo().getCXXABI().isMicrosoft() to 
see if the microsoft abi is being targeted.

Apologies for my confusing comments here.



Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+  auto d = 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-}}
+}

zequanwu wrote:
> hans wrote:
> > Is the cast to void necessary? Same comment for the next file.
> Yes, to suppress the warning of unused expression.
Ah, okay. Didn't realize that warning was on by default :)


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] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

I think our abs intrinsic support is already sufficient to start canonicalizing 
to the intrinsic variant (the same is not true for min/max intrinsics). We 
might want to make the InstCombine change before this one, to make sure we 
don't lose CSE opportunities between the intrinsic and the expanded forms. 
Though if that's not a concern for these vector intrinsics, then this LG to me.


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


[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 updated this revision to Diff 289793.
shivanshu3 added a comment.

- Remove the bool `IsDependencyFileArg` in the implementation of 
`getClangStripDependencyFileAdjuster()` to make it simpler.
- Add an extra argument after -MT in the test case to ensure we do not strip 
arguments after -MT when using the cl driver mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

Files:
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -563,6 +563,40 @@
   EXPECT_TRUE(HasFlag("-c"));
 }
 
+// Check getClangStripDependencyFileAdjuster doesn't strip args when using the
+// MSVC cl.exe driver
+TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) {
+  FixedCompilationDatabase Compilations(
+  "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-O1", "-MTd", "-MP"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_TRUE(HasFlag("-MD"));
+  EXPECT_TRUE(HasFlag("-MDd"));
+  EXPECT_TRUE(HasFlag("-MT"));
+  EXPECT_TRUE(HasFlag("-O1"));
+  EXPECT_TRUE(HasFlag("-MTd"));
+  EXPECT_TRUE(HasFlag("-MP"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,6 +21,16 @@
 namespace clang {
 namespace tooling {
 
+static StringRef getDriverMode(const CommandLineArguments &Args) {
+  for (const auto &Arg : Args) {
+StringRef ArgRef = Arg;
+if (ArgRef.consume_front("--driver-mode=")) {
+  return ArgRef;
+}
+  }
+  return StringRef();
+}
+
 /// Add -fsyntax-only option and drop options that triggers output generation.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
@@ -93,20 +103,28 @@
 
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+auto UsingClDriver = (getDriverMode(Args) == "cl");
+
 CommandLineArguments AdjustedArgs;
 for (size_t i = 0, e = Args.size(); i < e; ++i) {
   StringRef Arg = Args[i];
-  // All dependency-file options begin with -M. These include -MM,
-  // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-  !Arg.startswith("-showIncludes")) {
-AdjustedArgs.push_back(Args[i]);
+
+  // These flags take an argument: -MX foo. Skip the next argument also.
+  if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
+++i;
 continue;
   }
+  // When not using the cl driver mode, dependency file generation options
+  // begin with -M. These include -MM, -MF, -MG, -MP, -MT, -MQ, -MD, and
+  // -MMD.
+  if (!UsingClDriver && Arg.startswith("-M"))
+continue;
+  // Under MSVC's cl driver mode, dependency file generation is controlled
+  // using /showIncludes
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
+continue;
 
-  if (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")
-// These flags take an argument: -MX foo. Skip the next argument also.
-++i;
+  AdjustedArgs.push_back(Args[i]);
 }
 return AdjustedArgs;
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked 2 inline comments as done.
shivanshu3 added inline comments.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+  // do not remove those when using the cl driver.
+  bool IsDependencyFileArg;
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))

hans wrote:
> shivanshu3 wrote:
> > hans wrote:
> > > Instead of a bool and if below, I'd suggest if-statements and early 
> > > continues instead. Breaking up the old if-statement is nice though, and 
> > > maybe the comment from above about what flags to ignore could be moved 
> > > down here to those if statements. For example:
> > > 
> > > 
> > > ```
> > > // -M flags blah blah
> > > if (Arg.startswith("-M") && !UsingClDriver)
> > >   continue;
> > > // MSVC flags blah blah
> > > if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
> > >   continue;
> > > AdjustedArgs.push_back(Args[i]);
> > > ```
> > I realized that with my original change, we would skip the next argument 
> > under cl driver mode when -MT was used, which would be wrong. The next 
> > argument should only be skipped when `IsDependencyFileArg` is true. So I 
> > think it might be cleaner to keep that extra boolean so the code is easy to 
> > read and understand. What do you think?
> I think having the boolean variable is still more confusing (it adds more 
> state to keep track of) than just handling the cases with if-statements and 
> early continue.
> 
> How about:
> 
> ```
> // -M flags that take an arg..
> if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
>   ++i;
>   continue;
> }
> // -M flags blah blah
> if (!UsingClDriver && Arg.startswith("-M"))
>   continue;
> // MSVC flags blah blah
> if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
>   continue;
> 
> AdjustedArgs.push_back(Args[i]);
> ```
> 
> ?
Yup I agree that looks better :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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


[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D87102

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


[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Florian Hahn via Phabricator via cfe-commits
fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks for addressing this! Please wait a bit with committing, in case 
there are additional comments.




Comment at: clang/lib/Sema/SemaExpr.cpp:4598
   }
   // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
   // a new MatrixSubscriptExpr.

I think it would be good to split the comment into 2. The case below is `base 
is a MatrixSubscriptExpr`, and further down we handle `base is a matrix type`.


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

https://reviews.llvm.org/D87102

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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-03 Thread Alexander Lanin via Phabricator via cfe-commits
AlexanderLanin added a comment.

Observed behavior:

Change: `for(string_view token : split_into_views(file_content, " \t\r\n"))` 
--> `for(string_view const token : split_into_views(file_content, " \t\r\n"))`.
Note: `std::vector split_into_views(string_view input, const char* 
separators);`
Problem: Now it triggers `error: loop variable 'token' of type 'const 
nonstd::sv_lite::string_view' (aka 'const basic_string_view') creates a 
copy from type 'const nonstd::sv_lite::string_view' 
[-Werror,-Wrange-loop-analysis]`
Fixed manually by making it `for(std::string_view& const token : 
split_into_views(file_content, " \t\r\n"))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

___
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-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 289802.
zequanwu added a comment.

address comment.


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,20 @@
+// 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)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,20 @@
+// RUN: %clang_cc1 %s -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)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
@@ -7433,6 +7433,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
@@ -1229,3 +1229,5 @@
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

2020-09-03 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik marked an inline comment as done.
shafik added a subscriber: rsmith.
shafik added a comment.

@martong I have been experimenting w/ how we pass around `ImportDefinitionKind` 
and pushing it through to more places does not help. I also tried defaulting 
most locations to `IDK_Everything` to experiment to see if maybe I was missing 
some crucial point and no luck. So while it seemed like a good direction it has 
not worked out, unless I missed something in your idea.

So my current approach may be the best we have until we can do some larger 
refactoring.

I had explored trying to fix this from the codegen and sema side of things but 
after discussing this w/ @rsmith there is no simple fix there that would not 
require some large refactoring on the LLDB side.


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

https://reviews.llvm.org/D86660

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


[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-03 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

Do you have open questions on whether some callsites passing "false" here, 
should be switched to true? Given what's here, I would say that it definitely 
does not makes sense to add this parameter everywhere.

So, for getting something committed: please send a new review which makes only 
the required changes, splitting by logical part of the code. E.g. one change to 
fix the new/delete alignment, one for the global-variable alignment, and so on 
if there are others.




Comment at: clang/lib/CodeGen/CGAtomic.cpp:814
+  std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(
+  AtomicTy, true /* NeedsPreferredAlignment */);
   uint64_t Size = sizeChars.getQuantity();

This is wrong.



Comment at: clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:252
+  std::tie(RetVal.Size, RetVal.Align) = Ctx.getTypeInfoInChars(
+  FD->getType(), true /* NeedsPreferredAlignment */);
   assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity()));

Not sure if this is right, since it's looking at individual fields in the 
struct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

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


[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-03 Thread Snehasish Kumar via Phabricator via cfe-commits
snehasish updated this revision to Diff 289805.
snehasish added a comment.

Add warning when option is enabled without profile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87047

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/split-machine-functions.c
  clang/test/Driver/fsplit-machine-functions.c

Index: clang/test/Driver/fsplit-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -0,0 +1,4 @@
+// RUN: %clang -### -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-fsplit-machine-functions"
+// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
Index: clang/test/CodeGen/split-machine-functions.c
===
--- /dev/null
+++ clang/test/CodeGen/split-machine-functions.c
@@ -0,0 +1,34 @@
+// REQUIRES: x86-registered-target
+
+// RUN: echo "foo"> %t.proftext
+// RUN: echo "# Func Hash:"   >> %t.proftext
+// RUN: echo "11262309905">> %t.proftext
+// RUN: echo "# Num Counters:">> %t.proftext
+// RUN: echo "2"  >> %t.proftext
+// RUN: echo "# Counter Values:"  >> %t.proftext
+// RUN: echo "100">> %t.proftext
+// RUN: echo "0"  >> %t.proftext
+// RUN: llvm-profdata merge -o %t.profdata %t.proftext
+// RUN: %clang_cc1 -triple x86_64 -O3 -S -fprofile-instrument-use-path=%t.profdata -fsplit-machine-functions -o - < %s | FileCheck %s
+
+__attribute__((noinline)) int foo(int argc) {
+  if (argc % 2 == 0) {
+exit(argc);
+  } else {
+return argc + 1;
+  }
+}
+
+int main(int argc, char *argv[]) {
+  int total = 0;
+  for (int i = 0; i < 100; ++i) {
+total += foo(argc);
+  }
+  printf("%d\n", total);
+}
+
+// CHECK: .section .text.hot.,"ax",@progbits
+// CHECK: foo:
+// CHECK: section .text.unlikely.foo,"ax",@progbits
+// CHECK: foo.cold:
+// CHECK: callq exit@PLT
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -998,6 +998,8 @@
   Opts.UniqueInternalLinkageNames =
   Args.hasArg(OPT_funique_internal_linkage_names);
 
+  Opts.SplitMachineFunctions = Args.hasArg(OPT_fsplit_machine_functions);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,6 +4255,8 @@
 options::OPT_fno_unique_section_names,
 options::OPT_funique_basic_block_section_names,
 options::OPT_fno_unique_basic_block_section_names,
+options::OPT_fsplit_machine_functions,
+options::OPT_fno_split_machine_functions,
 options::OPT_mrestrict_it,
 options::OPT_mno_restrict_it,
 options::OPT_mstackrealign,
@@ -4905,6 +4907,10 @@
options::OPT_fno_unique_basic_block_section_names, false))
 CmdArgs.push_back("-funique-basic-block-section-names");
 
+  if (Args.hasFlag(options::OPT_fsplit_machine_functions,
+   options::OPT_fno_split_machine_functions, false))
+CmdArgs.push_back("-fsplit-machine-functions");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -514,6 +514,13 @@
   Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
   }
 
+  if (CodeGenOpts.SplitMachineFunctions) {
+if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
+  Options.EnableMachineFunctionSplitter = true;
+else
+  Diags.Report(diag::warn_fe_machine_function_splitter_missing_profile);
+  }
+
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.

[PATCH] D87102: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f1be87e2947: [Sema] Fix a -Warc-repeated-use-of-weak 
false-positive by only calling… (authored by erik.pilkington).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D87102?vs=289764&id=289806#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87102

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/arc-repeated-weak.mm


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if (matSubscriptE) {
 if (CheckAndReportCommaError(idx))
@@ -4607,34 +4607,13 @@
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,24 @@
   base = result.get();
 }
   }
+
+  // If the base is a matrix type, try to create a new MatrixSubscriptExpr.
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();


Index: clang/test/SemaObjC/arc-repeated-weak.mm
===
--- clang/test/SemaObjC/arc-repeated-weak.mm
+++ clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if (matSubscr

[clang] 0f1be87 - [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling CheckPlaceholderExpr once

2020-09-03 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-09-03T16:56:35-04:00
New Revision: 0f1be87e294751a0941f1d9b7785ebf4d8072149

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

LOG: [Sema] Fix a -Warc-repeated-use-of-weak false-positive by only calling 
CheckPlaceholderExpr once

Previously, this code discarded the result of CheckPlaceholderExpr for
non-matrix subexpressions. Not only is this wasteful, but it was creating a
Warc-repeated-use-of-weak false-positive on the attached testcase, since the
discarded expression was still registered as a use of the weak property.

rdar://66162246

Differential revision: https://reviews.llvm.org/D87102

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/arc-repeated-weak.mm

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 450185788537..cd71ce70c70e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4595,8 +4595,8 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
 << SourceRange(base->getBeginLoc(), rbLoc);
 return ExprError();
   }
-  // If the base is either a MatrixSubscriptExpr or a matrix type, try to 
create
-  // a new MatrixSubscriptExpr.
+  // If the base is a MatrixSubscriptExpr, try to create a new
+  // MatrixSubscriptExpr.
   auto *matSubscriptE = dyn_cast(base);
   if (matSubscriptE) {
 if (CheckAndReportCommaError(idx))
@@ -4607,34 +4607,13 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
 return CreateBuiltinMatrixSubscriptExpr(
 matSubscriptE->getBase(), matSubscriptE->getRowIdx(), idx, rbLoc);
   }
-  Expr *matrixBase = base;
-  bool IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
-  if (!IsMSPropertySubscript) {
-ExprResult result = CheckPlaceholderExpr(base);
-if (!result.isInvalid())
-  matrixBase = result.get();
-  }
-  if (matrixBase->getType()->isMatrixType()) {
-if (CheckAndReportCommaError(idx))
-  return ExprError();
-
-return CreateBuiltinMatrixSubscriptExpr(matrixBase, idx, nullptr, rbLoc);
-  }
-
-  // A comma-expression as the index is deprecated in C++2a onwards.
-  if (getLangOpts().CPlusPlus20 &&
-  ((isa(idx) && cast(idx)->isCommaOp()) ||
-   (isa(idx) &&
-cast(idx)->getOperator() == OO_Comma))) {
-Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
-  << SourceRange(base->getBeginLoc(), rbLoc);
-  }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
+  bool IsMSPropertySubscript = false;
   if (base->getType()->isNonOverloadPlaceholderType()) {
 IsMSPropertySubscript = isMSPropertySubscriptExpr(*this, base);
 if (!IsMSPropertySubscript) {
@@ -4644,6 +4623,24 @@ Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base, 
SourceLocation lbLoc,
   base = result.get();
 }
   }
+
+  // If the base is a matrix type, try to create a new MatrixSubscriptExpr.
+  if (base->getType()->isMatrixType()) {
+if (CheckAndReportCommaError(idx))
+  return ExprError();
+
+return CreateBuiltinMatrixSubscriptExpr(base, idx, nullptr, rbLoc);
+  }
+
+  // A comma-expression as the index is deprecated in C++2a onwards.
+  if (getLangOpts().CPlusPlus20 &&
+  ((isa(idx) && cast(idx)->isCommaOp()) ||
+   (isa(idx) &&
+cast(idx)->getOperator() == OO_Comma))) {
+Diag(idx->getExprLoc(), diag::warn_deprecated_comma_subscript)
+<< SourceRange(base->getBeginLoc(), rbLoc);
+  }
+
   if (idx->getType()->isNonOverloadPlaceholderType()) {
 ExprResult result = CheckPlaceholderExpr(idx);
 if (result.isInvalid()) return ExprError();

diff  --git a/clang/test/SemaObjC/arc-repeated-weak.mm 
b/clang/test/SemaObjC/arc-repeated-weak.mm
index 4eec4d2fe69c..90388598c7b8 100644
--- a/clang/test/SemaObjC/arc-repeated-weak.mm
+++ b/clang/test/SemaObjC/arc-repeated-weak.mm
@@ -485,3 +485,17 @@ void foo1() {
 
 @class NSString;
 static NSString* const kGlobal = @"";
+
+@interface NSDictionary
+- (id)objectForKeyedSubscript:(id)key;
+@end
+
+@interface WeakProp
+@property (weak) NSDictionary *nd;
+@end
+
+@implementation WeakProp
+-(void)m {
+  (void)self.nd[@""]; // no warning
+}
+@end



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


[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-03 Thread Snehasish Kumar via Phabricator via cfe-commits
snehasish added a comment.

Thanks for the comments! PTAL.

In D87047#2252982 , @davidxl wrote:

> For x86 target, should it be turned on when -fprofile-use= option is 
> specified unless -fno-split-machine-function is specified?

I don't think we are there yet to turn it on for x86 profile builds. There is 
at least one unresolved issue with DebugInfo (D85085 
); once we have tested this some more 
internally we can revisit and make it the default.

> Also is it worth to give a warning if the option is specified but PGO is not 
> on?

Added a warning if no profile is specified but this option is turned on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87047

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


[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289815.
chrish_ericsson_atx added a comment.

NC.  Pushing null change in hopes of re-triggering testing.

Unit test that failed is low-level LLVM test, which doesn't even exercise the 
code I've changed here, so I'm assuming it's a spurious failure in the CI
machinery.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible ele

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 289814.
yaxunl added a comment.

Defer overload resolution diags only if there are wrong-sided candidates.


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

https://reviews.llvm.org/D84364

Files:
  clang/include/clang/Basic/Diagnostic.td
  clang/include/clang/Basic/DiagnosticAST.h
  clang/include/clang/Basic/DiagnosticAnalysis.h
  clang/include/clang/Basic/DiagnosticComment.h
  clang/include/clang/Basic/DiagnosticCrossTU.h
  clang/include/clang/Basic/DiagnosticDriver.h
  clang/include/clang/Basic/DiagnosticFrontend.h
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Basic/DiagnosticLex.h
  clang/include/clang/Basic/DiagnosticParse.h
  clang/include/clang/Basic/DiagnosticRefactoring.h
  clang/include/clang/Basic/DiagnosticSema.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/DiagnosticSerialization.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/deferred-oeverload.cu
  clang/test/TableGen/DiagnosticBase.inc
  clang/test/TableGen/deferred-diag.td
  clang/tools/diagtool/DiagnosticNames.cpp
  clang/utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1294,6 +1294,11 @@
 else
   OS << ", false";
 
+if (R.getValueAsBit("Deferrable"))
+  OS << ", true";
+else
+  OS << ", false";
+
 // Category number.
 OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap));
 OS << ")\n";
Index: clang/tools/diagtool/DiagnosticNames.cpp
===
--- clang/tools/diagtool/DiagnosticNames.cpp
+++ clang/tools/diagtool/DiagnosticNames.cpp
@@ -28,7 +28,7 @@
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
 #define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,   \
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\
+ SFINAE,NOWERROR,SHOWINSYSHEADER,DEFER,CATEGORY)\
   { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
Index: clang/test/TableGen/deferred-diag.td
===
--- /dev/null
+++ clang/test/TableGen/deferred-diag.td
@@ -0,0 +1,27 @@
+// RUN: clang-tblgen -gen-clang-diags-defs -I%S %s -o - 2>&1 | \
+// RUN:FileCheck --strict-whitespace %s
+include "DiagnosticBase.inc"
+
+// Test usage of Deferrable and NonDeferrable in diagnostics.
+
+def test_default : Error<"This error is non-deferrable by default">;
+// CHECK-DAG: DIAG(test_default, {{.*}}SFINAE_SubstitutionFailure, false, true, false, 0)
+
+def test_deferrable : Error<"This error is deferrable">, Deferrable;
+// CHECK-DAG: DIAG(test_deferrable, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+def test_non_deferrable : Error<"This error is non-deferrable">, NonDeferrable;
+// CHECK-DAG: DIAG(test_non_deferrable, {{.*}} SFINAE_SubstitutionFailure, false, true, false, 0)
+
+let Deferrable = 1 in {
+
+def test_let : Error<"This error is deferrable by let">;
+// CHECK-DAG: DIAG(test_let, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+// Make sure TextSubstitution is allowed in the let Deferrable block.
+def textsub : TextSubstitution<"%select{text1|text2}0">;
+
+def test_let2 : Error<"This error is deferrable by let %sub{textsub}0">;
+// CHECK-DAG: DIAG(test_let2, {{.*}} SFINAE_SubstitutionFailure, false, true, true, 0)
+
+}
\ No newline at end of file
Index: clang/test/TableGen/DiagnosticBase.inc
===
--- clang/test/TableGen/DiagnosticBase.inc
+++ clang/test/TableGen/DiagnosticBase.inc
@@ -45,6 +45,7 @@
   // diagnostics
   string Component = "";
   string CategoryName = "";
+  bit Deferrable = 0;
 }
 
 // Diagnostic Categories.  These can be applied to groups or individual
@@ -75,6 +76,7 @@
   bitAccessControl = 0;
   bitWarningNoWerror = 0;

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-09-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D84364#2201336 , @tra wrote:

> In D84364#2176091 , @yaxunl wrote:
>
>> I added a `Deferrable` bit to the diagnostics which can be specified in td 
>> files. This can be added to individual diagnostic defs or added to a bunch 
>> of diagnostic defs all together.
>>
>> This field is used to control whether a diagnostic message can be deferred.
>
> This may be a case of "too much, but not enough". It will be unnecessary for 
> most of the diagnostics we have. Overload resolution is likely to be the 
> primary beneficiary, inline asm and exceptions may be two other classes, but 
> I can't think of anything else ATM. 
> At the same time it may not be enough, because we also need to take into 
> account where and when particular diagnostic is emitted. I.e. the same 
> diagnostic may need to be postponed when we emit it from CUDA code, yet we 
> may want to *not* postpone it if it's in the code which has nothing to do 
> with CUDA. E.g. C++ code which has oveloading-related error in an inline 
> function which would not be codegen'ed. I would expect such error to be 
> reported as it would be if the same function was compiled in plain C++ mode.

I added a heuristics based rule for deferring overloading resolution related 
diagnostics. Basically clang will check if there are wrong-sided candidates 
when an overloading resolution error happens. This seems to be able to capture 
most of the host-ness related diagnostics which we want to defer whereas 
without affecting the diagnostics that are unrelated to host-ness.

>> For now I enabled this bit for the overloading resolution diagnostics since 
>> these have been seen as false alarms in host device functions. We could let 
>> more diagnostics be deferrable if we see it is necessary.
>
>
>
>> I just saw bugzilla bug https://bugs.llvm.org/show_bug.cgi?id=46922
>> my patch https://reviews.llvm.org/D77954 is supposed to fix this issue. 
>> However since implicit host device functions often cause overloading 
>> resolution diagnostics on the device side which are not deferred, my patch 
>> caused regressions and was reverted several times. Currently it was still 
>> reverted.
>>
>> I think to fix this issue we need to make overloading resolution diagnostics 
>> deferrable.
>
> I'm missing something here. How would deferred diagnostics help with the 
> issue in the bug 46922? The HD function there is codegen'ed on both sides, so 
> the only difference postponing would make is that we'd emit the diagnostic a 
> bit later.
> Do you mean that postponed diags patch is not the fix, but rather a 
> prerequisite for the overload resolution changes patch?

Right. We need to favor same sided candidates for host device functions to fix 
that issue, the obstacle to implement the correct host/device based overloading 
resolution is that implicit host device function causes diagnostics in device 
compilation, which should be deferred but currently are not. With this patch we 
may be able to fix the overloading resolution issue properly.


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

https://reviews.llvm.org/D84364

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


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

rahmanl wrote:
> MaskRay wrote:
> > BBAddrMapSection is always non-null. Delete the if.
> I believe we return null for non-ELF environment (Please refer to 
> MCObjectFileInfo::getBBAddrMapSection).
In that case emitBBAddrMapSection will not be called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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


[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289819.
chrish_ericsson_atx added a comment.

NC push did not resolve failed test.  Rebased in hopes that whatever has 
broken the build has been resolved in the intervening commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+struct BQ {
+  struct S 

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

2020-09-03 Thread Zixu Wang via Phabricator via cfe-commits
zixuw added a comment.

Okay so I think I traced down to the root problem: the diagnostic message 
prints out fine because `TextDiagnosticPrinter` uses `FullSourceLoc`, which 
actually embeds an appropriate `SourceManager` within it:

  TextDiag->emitDiagnostic(
  FullSourceLoc(Info.getLocation(), Info.getSourceManager()), Level,
  DiagMessageStream.str(), Info.getRanges(), Info.getFixItHints());

However, `FixItHint` is handled by a different path: a `FixItHint` only stores 
a trivial location (a `SourceRange`) which does not embed a `SourceManager`. 
And when a `FixItRewriter` constructs, it initializes a bunch of helper objects 
(Editor, Rewriter, Commit, etc.) using the current `SourceManager`. When trying 
to commit a fix-it hint, the helper objects use their (const ref to) 
`SourceManager`, which is used by the main compilation, to resolve the plain 
location in the `FixItHint`, which is generated in the new compiler instance 
for building a module. Therefore the location resolution is wrong.
To fix this, we can make `FixItHint` use `FullSourceLoc` instead, and also 
modify all relevant parties (`FixItRewriter`, `EditedSource`, `Rewriter`, 
`Commit`, ...) to use the embedded `SourceManager` instead of having a const 
ref to one initialized `SourceManager`.
My main concern is that I'm not very familiar with all the rewrite facilities, 
and changing `EditedSource` etc. might affect other users of the facilities.
What do you think? @arphaman @bruno @vsapsai


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82118

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


[PATCH] D86841: [clang] Add noprogress attribute deduction for infinite loops

2020-09-03 Thread Atmn Patel via Phabricator via cfe-commits
atmnpatel updated this revision to Diff 289821.
atmnpatel added a comment.
Herald added a subscriber: zzheng.

Renamed llvm loop metadata, changed deduction rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-noprogress.c
  clang/test/CodeGen/attr-noprogress.cpp
  clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp

Index: clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
===
--- clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
+++ clang/test/CodeGenCXX/fno-unroll-loops-metadata.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns -fno-unroll-loops | FileCheck --check-prefix=UNROLL_DISABLED_MD %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s -O3 -disable-llvm-optzns | FileCheck --check-prefix=NO_UNROLL_MD %s
 
-// NO_UNROLL_MD-NOT: llvm.loop
+// NO_UNROLL_MD-NOT: llvm.loop.unroll.disable
 
 // Verify unroll.disable metadata is added to while loop with -fno-unroll-loops
 // and optlevel > 0.
Index: clang/test/CodeGen/attr-noprogress.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noprogress.cpp
@@ -0,0 +1,188 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-attributes
+// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+
+int a = 0;
+int b = 0;
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z2f1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f1() {
+  for (; 1;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: ret void
+//
+void f2() {
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z1Fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK: br i1 true, label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK: br label [[FOR_COND]]
+// CHECK:   for.end:
+// CHECK: br label [[FOR_COND1:%.*]]
+// CHECK:   for.cond1:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[FOR_BODY2:%.*]], label [[FOR_END3:%.*]]
+// CHECK:   for.body2:
+// CHECK: br label [[FOR_COND1]], !llvm.loop [[LOOP2:!.*]]
+// CHECK:   for.end3:
+// CHECK: ret void
+//
+void F() {
+  for (; 1;) {
+  }
+  for (; a == b;) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z2w1v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_BODY:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_BODY]]
+//
+void w1() {
+  while (1) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2w2v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_COND:%.*]]
+// CHECK:   while.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHECK: br i1 [[CMP]], label [[WHILE_BODY:%.*]], label [[WHILE_END:%.*]]
+// CHECK:   while.body:
+// CHECK: br label [[WHILE_COND]]
+// CHECK:   while.end:
+// CHECK: ret void
+//
+void w2() {
+  while (a == b) {
+  }
+}
+
+// CHECK: Function Attrs: noinline nounwind optnone noprogress
+// CHECK-LABEL: @_Z1Wv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[WHILE_COND:%.*]]
+// CHECK:   while.cond:
+// CHECK: [[TMP0:%.*]] = load i32, i32* @a, align 4
+// CHECK: [[TMP1:%.*]] = load i32, i32* @b, align 4
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[TMP1]]
+// CHE

[PATCH] D86841: [clang] Add noprogress attribute deduction for infinite loops

2020-09-03 Thread Atmn Patel via Phabricator via cfe-commits
atmnpatel marked an inline comment as done.
atmnpatel added inline comments.



Comment at: clang/lib/CodeGen/CGLoopInfo.h:211
 llvm::ArrayRef Attrs, const llvm::DebugLoc &StartLoc,
-const llvm::DebugLoc &EndLoc);
+const llvm::DebugLoc &EndLoc, const bool NoProgress = false);
 

aaron.ballman wrote:
> I'd drop the top-level `const` on the declaration of `NoProgress` (that's not 
> a style we typically use in the project).
My bad, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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


[clang] e6393ee - Canonicalize declaration pointers when forming APValues.

2020-09-03 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-09-03T15:35:12-07:00
New Revision: e6393ee813178e9d3306b8e3c6949a4f32f8a2cb

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

LOG: Canonicalize declaration pointers when forming APValues.

References to different declarations of the same entity aren't different
values, so shouldn't have different representations.

Added: 


Modified: 
clang/include/clang/AST/APValue.h
clang/lib/AST/APValue.cpp
clang/lib/AST/ExprConstant.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
clang/test/OpenMP/ordered_messages.cpp

Removed: 




diff  --git a/clang/include/clang/AST/APValue.h 
b/clang/include/clang/AST/APValue.h
index 87e4bd7f84c1..485e6c2602cf 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -174,6 +174,7 @@ class APValue {
   return !(LHS == RHS);
 }
 friend llvm::hash_code hash_value(const LValueBase &Base);
+friend struct llvm::DenseMapInfo;
 
   private:
 PtrTy Ptr;
@@ -201,8 +202,7 @@ class APValue {
 
   public:
 LValuePathEntry() : Value() {}
-LValuePathEntry(BaseOrMemberType BaseOrMember)
-: Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
+LValuePathEntry(BaseOrMemberType BaseOrMember);
 static LValuePathEntry ArrayIndex(uint64_t Index) {
   LValuePathEntry Result;
   Result.Value = Index;

diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 2a8834b4db0c..7531229654cf 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -38,7 +38,7 @@ static_assert(
 "Type is insufficiently aligned");
 
 APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V)
-: Ptr(P), Local{I, V} {}
+: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr), Local{I, V} {}
 APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
 : Ptr(P), Local{I, V} {}
 
@@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase &LHS,
 const APValue::LValueBase &RHS) {
   if (LHS.Ptr != RHS.Ptr)
 return false;
-  if (LHS.is())
+  if (LHS.is() || LHS.is())
 return true;
   return LHS.Local.CallIndex == RHS.Local.CallIndex &&
  LHS.Local.Version == RHS.Local.Version;
 }
 }
 
+APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) {
+  if (const Decl *D = BaseOrMember.getPointer())
+BaseOrMember.setPointer(D->getCanonicalDecl());
+  Value = reinterpret_cast(BaseOrMember.getOpaqueValue());
+}
+
 namespace {
   struct LVBase {
 APValue::LValueBase Base;
@@ -113,14 +119,16 @@ APValue::LValueBase::operator bool () const {
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getEmptyKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getEmptyKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getEmptyKey();
+  return B;
 }
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getTombstoneKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getTombstoneKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getTombstoneKey();
+  return B;
 }
 
 namespace clang {
@@ -757,8 +765,10 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, 
bool IsDerivedMember,
   assert(isAbsent() && "Bad state change");
   MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData;
   Kind = MemberPointer;
-  MPD->MemberAndIsDerivedMember.setPointer(Member);
+  MPD->MemberAndIsDerivedMember.setPointer(
+  Member ? cast(Member->getCanonicalDecl()) : nullptr);
   MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember);
   MPD->resizePath(Path.size());
-  memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const 
CXXRecordDecl*));
+  for (unsigned I = 0; I != Path.size(); ++I)
+MPD->getPath()[I] = Path[I]->getCanonicalDecl();
 }

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e8f132dd4803..8e43b62662ee 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1978,18 +1978,11 @@ static bool HasSameBase(const LValue &A, const LValue 
&B) {
 return false;
 
   if (A.getLValueBase().getOpaqueValue() !=
-  B.getLValueBase().getOpaqueValue()) {
-const Decl *ADecl = GetLValueBaseDecl(A);
-if (!ADecl)
-  return false;
-const Decl *BDecl = GetLValueBaseDecl(B);
-if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl())
-  return false;
-  }
+  B.getLValueBase().getOpaqueValue())
+return false;
 
-  return IsGlobalLValue(A.getLValueBase()) ||
- (A.getLValueCallIndex() == B.getLValueCallIndex() &&
-  A.getLValueVersion() == B.getLValueVersion());
+  return A.getLValueCallIndex() == B.getLValueCallIndex() &&
+ A.getLValueVersion() == B.getLValueVersio

[clang] 052dbe2 - Remove unused and dangerous overload of PerformImplicitConversion.

2020-09-03 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-09-03T15:35:12-07:00
New Revision: 052dbe226cb3540c77cf0b3dc4a51a4ab7726b55

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

LOG: Remove unused and dangerous overload of PerformImplicitConversion.

Previously we had two overloads where the only real difference beyond
parameter order was whether a reference parameter is const, where one
overload treated the reference parameter as an in-parameter and the
other treated it as an out-parameter!

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 174b424bb996..ec449d6dd6be 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11224,10 +11224,6 @@ class Sema final {
   ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
AssignmentAction Action,
bool AllowExplicit = false);
-  ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
-   AssignmentAction Action,
-   bool AllowExplicit,
-   ImplicitConversionSequence& ICS);
   ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
const ImplicitConversionSequence& ICS,
AssignmentAction Action,

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 21a9ad04d500..71341e5688fe 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1494,17 +1494,9 @@ Sema::TryImplicitConversion(Expr *From, QualType ToType,
 /// converted expression. Flavor is the kind of conversion we're
 /// performing, used in the error message. If @p AllowExplicit,
 /// explicit user-defined conversions are permitted.
-ExprResult
-Sema::PerformImplicitConversion(Expr *From, QualType ToType,
-AssignmentAction Action, bool AllowExplicit) {
-  ImplicitConversionSequence ICS;
-  return PerformImplicitConversion(From, ToType, Action, AllowExplicit, ICS);
-}
-
-ExprResult
-Sema::PerformImplicitConversion(Expr *From, QualType ToType,
-AssignmentAction Action, bool AllowExplicit,
-ImplicitConversionSequence& ICS) {
+ExprResult Sema::PerformImplicitConversion(Expr *From, QualType ToType,
+   AssignmentAction Action,
+   bool AllowExplicit) {
   if (checkPlaceholderForOverload(*this, From))
 return ExprError();
 
@@ -1515,13 +1507,13 @@ Sema::PerformImplicitConversion(Expr *From, QualType 
ToType,
   if (getLangOpts().ObjC)
 CheckObjCBridgeRelatedConversions(From->getBeginLoc(), ToType,
   From->getType(), From);
-  ICS = ::TryImplicitConversion(*this, From, ToType,
-/*SuppressUserConversions=*/false,
-AllowExplicit ? AllowedExplicit::All
-  : AllowedExplicit::None,
-/*InOverloadResolution=*/false,
-/*CStyle=*/false, AllowObjCWritebackConversion,
-/*AllowObjCConversionOnExplicit=*/false);
+  ImplicitConversionSequence ICS = ::TryImplicitConversion(
+  *this, From, ToType,
+  /*SuppressUserConversions=*/false,
+  AllowExplicit ? AllowedExplicit::All : AllowedExplicit::None,
+  /*InOverloadResolution=*/false,
+  /*CStyle=*/false, AllowObjCWritebackConversion,
+  /*AllowObjCConversionOnExplicit=*/false);
   return PerformImplicitConversion(From, ToType, ICS, Action);
 }
 



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


[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-03 Thread Shivanshu Goyal via Phabricator via cfe-commits
shivanshu3 marked an inline comment as done.
shivanshu3 added a comment.

Note that I do not have commit access and this change will have to be committed 
by someone else on my behalf. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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


[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via Phabricator via cfe-commits
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1035
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);

akhuang wrote:
> dblaikie wrote:
> > When is a definition not a complete definition? (sounds like a line from 
> > Alice in Wonderland... but I'm genuinely curious/not sure I understand)
> ha, good question, I copied this without really looking at it. I am also not 
> sure when a definition would not be a complete definition.
Oh, I guess incomplete definition means it's in the process of being defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87062

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


[clang] aaf1a96 - [DebugInfo] Add size to class declarations in debug info.

2020-09-03 Thread Amy Huang via cfe-commits

Author: Amy Huang
Date: 2020-09-03T15:42:27-07:00
New Revision: aaf1a96408b1587b5fb80a3a7c424348cb09e577

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

LOG: [DebugInfo] Add size to class declarations in debug info.

This adds the size to forward declared class DITypes, if the size is known.

Fixes an issue where we determine whether to emit fragments based on the
type size, so fragments would sometimes be incorrectly emitted if there
was no size.

Bug: https://bugs.llvm.org/show_bug.cgi?id=47338

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-class.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 8a85a24910e4..1fdb6814c7bd 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@ CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType 
*Ty,
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,

diff  --git a/clang/test/CodeGenCXX/debug-info-class.cpp 
b/clang/test/CodeGenCXX/debug-info-class.cpp
index 94d5a0f1f082..e000532b8c3b 100644
--- a/clang/test/CodeGenCXX/debug-info-class.cpp
+++ b/clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@ int main(int argc, char **argv) {
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
-// CHECK-NOT:  size:
+// CHECK-SAME: size:
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-NOT:  identifier:
 // CHECK-SAME: ){{$}}



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


  1   2   >