[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

2021-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

@efriedma thank you for taking a look!
I agree that D99790  is basically guaranteed 
safe, and this "probably" isn't.

In D99791#2667047 , @efriedma wrote:

> This feels scary: the C standard technically allows this, but we haven't done 
> it in the past, and it could break otherwise functioning code.  (We've only 
> assumed alignment about pointers that are dereferenced/dereferenceable.)

I can add necessary UBSan plumbing beforehand, iff we can actually do this.

> For D99790 , we're already marking the 
> pointer dereferenceable, so also marking the alignment seems like a small 
> extra step.  (Really, it's a regression fix; we used to treat dereferenceable 
> as implying alignment. I guess I missed a spot when I was fixing that.)

Yep. Feel like stamping that one? :)

> But here, we're not assuming it's dereferenceable at the moment.  So there's 
> more potential to break code, and also the potential benefit is small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99791

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


[clang] 95f448a - [PGO, test] Fix typo in FileCheck var

2021-04-03 Thread Thomas Preud'homme via cfe-commits

Author: Thomas Preud'homme
Date: 2021-04-03T08:44:46+01:00
New Revision: 95f448aa86cd3680a9493b2311d9efb41c4d4c01

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

LOG: [PGO, test] Fix typo in FileCheck var

Reviewed By: xur

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

Added: 


Modified: 
clang/test/Profile/c-unreachable-after-switch.c

Removed: 




diff  --git a/clang/test/Profile/c-unreachable-after-switch.c 
b/clang/test/Profile/c-unreachable-after-switch.c
index 36a75449dbdf4..cfc111b2752e0 100644
--- a/clang/test/Profile/c-unreachable-after-switch.c
+++ b/clang/test/Profile/c-unreachable-after-switch.c
@@ -11,5 +11,5 @@ void foo() {
 return;
   }
   // We shouldn't emit the unreachable counter. This used to crash in 
GlobalDCE.
-  // CHECK-NOT: store {{.*}} @[[SWC]], i64 0, i64 1}
+  // CHECK-NOT: store {{.*}} @[[C]], i64 0, i64 1}
 }



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


[PATCH] D99821: [PGO, test] Fix typo in FileCheck var

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG95f448aa86cd: [PGO, test] Fix typo in FileCheck var 
(authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99821

Files:
  clang/test/Profile/c-unreachable-after-switch.c


Index: clang/test/Profile/c-unreachable-after-switch.c
===
--- clang/test/Profile/c-unreachable-after-switch.c
+++ clang/test/Profile/c-unreachable-after-switch.c
@@ -11,5 +11,5 @@
 return;
   }
   // We shouldn't emit the unreachable counter. This used to crash in 
GlobalDCE.
-  // CHECK-NOT: store {{.*}} @[[SWC]], i64 0, i64 1}
+  // CHECK-NOT: store {{.*}} @[[C]], i64 0, i64 1}
 }


Index: clang/test/Profile/c-unreachable-after-switch.c
===
--- clang/test/Profile/c-unreachable-after-switch.c
+++ clang/test/Profile/c-unreachable-after-switch.c
@@ -11,5 +11,5 @@
 return;
   }
   // We shouldn't emit the unreachable counter. This used to crash in GlobalDCE.
-  // CHECK-NOT: store {{.*}} @[[SWC]], i64 0, i64 1}
+  // CHECK-NOT: store {{.*}} @[[C]], i64 0, i64 1}
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99830: [DebugInfo, CallSites, test] Fix use of undef FileCheck var

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre created this revision.
thopre added reviewers: djtodoro, vsk.
thopre requested review of this revision.
Herald added a project: clang.

Clang test CodeGen/debug-info-extern-call.c tries to check for the
absence of a sequence of instructions with several CHECK-NOT with one of
those directives using a variable defined in another. However CHECK-NOT
are checked independently so that is using a variable defined in a
pattern that should not occur in the input.

This commit removes the CHECK-NOT for the retained line attribute
definition since the CHECK-NOT on the compile unit will already check
that there is no retained lines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99830

Files:
  clang/test/CodeGen/debug-info-extern-call.c


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -21,8 +21,7 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
-// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: 
![[RETTYPES:[0-9]+]]
-// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: !{{[0-9]+}}
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -21,8 +21,7 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
-// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
-// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: !{{[0-9]+}}
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99447: [OpenMP] Define omp_is_initial_device() variants in omp.h

2021-04-03 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Nice, thanks! I think the function in the devicertl is dead with this change, 
maybe remove that too?




Comment at: openmp/runtime/src/include/omp.h.var:479
+#   endif
+
 #   undef __KAI_KMPC_CONVENTION

jdoerfert wrote:
> hbae wrote:
> > jdoerfert wrote:
> > > Do we want them to be static? I would have expected odr linkage instead.
> > > 
> > > 
> > > 
> > Current clang fails to link two files that include omp.h, so I think we 
> > need static here.
> I would expect this to cause "unused function" warnings if you include omp.h.
> That said, given that clang might be the only impl. so far for begin/end 
> declare variant and it doesn't warn, we can go ahead.
'inline' is right for c++. Inline plus instantiation in library code right for 
c. Can we hit linkonce_odr from a clang attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99447

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


[PATCH] D99831: [HIP-Clang, test] Fix use of undef FileCheck var

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre created this revision.
thopre added reviewers: ashi1, yaxunl, MaskRay, hliao, pcc.
thopre requested review of this revision.
Herald added a project: clang.

Commit 8129521318accc44c2a009647572f6ebd3fc56dd changed a line defining
PREFIX in clang test CodeGenCUDA/device-stub.cu into a CHECK-NOT
directive. All following lines using PREFIX are therefore using an
undefined variable since the pattern defining PREFIX is not supposed to
occur and CHECK-NOT are checked independently.

This commit replaces all uses of PREFIX by the regex used to define it,
thereby avoiding the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99831

Files:
  clang/test/CodeGenCUDA/device-stub.cu


Index: clang/test/CodeGenCUDA/device-stub.cu
===
--- clang/test/CodeGenCUDA/device-stub.cu
+++ clang/test/CodeGenCUDA/device-stub.cu
@@ -266,13 +266,13 @@
 // CUDANOGLOBALS-NOT: @{{.*}} = private constant{{.*}}
 // HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// NOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
-// NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// NOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
-// NOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+// NOGLOBALS-NOT: define internal void @__{{cuda|hip}}_module_ctor
+// NOGLOBALS-NOT: 
call{{.*}}{{cuda|hip}}RegisterFatBinary{{.*}}__{{cuda|hip}}_fatbin_wrapper
+// NOGLOBALS-NOT: call void @__{{cuda|hip}}_register_globals
+// NOGLOBALS-NOT: define internal void @__{{cuda|hip}}_module_dtor
+// NOGLOBALS-NOT: call void @__{{cuda|hip}}UnregisterFatBinary
 
 // There should be no constructors/destructors if we have no GPU binary.
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_ctor
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_dtor
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_register_globals
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_module_ctor
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_module_dtor


Index: clang/test/CodeGenCUDA/device-stub.cu
===
--- clang/test/CodeGenCUDA/device-stub.cu
+++ clang/test/CodeGenCUDA/device-stub.cu
@@ -266,13 +266,13 @@
 // CUDANOGLOBALS-NOT: @{{.*}} = private constant{{.*}}
 // HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// NOGLOBALS-NOT: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
-// NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// NOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
-// NOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+// NOGLOBALS-NOT: define internal void @__{{cuda|hip}}_module_ctor
+// NOGLOBALS-NOT: call{{.*}}{{cuda|hip}}RegisterFatBinary{{.*}}__{{cuda|hip}}_fatbin_wrapper
+// NOGLOBALS-NOT: call void @__{{cuda|hip}}_register_globals
+// NOGLOBALS-NOT: define internal void @__{{cuda|hip}}_module_dtor
+// NOGLOBALS-NOT: call void @__{{cuda|hip}}UnregisterFatBinary
 
 // There should be no constructors/destructors if we have no GPU binary.
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_ctor
-// NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_dtor
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_register_globals
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_module_ctor
+// NOGPUBIN-NOT: define internal void @__{{cuda|hip}}_module_dtor
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99832: [HIP, test] Fix use of undef FileCheck var

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre created this revision.
thopre added reviewers: yaxunl, ashi1, MaskRay, hliao, pcc.
thopre requested review of this revision.
Herald added a project: clang.

Clang test CodeGenCUDA/kernel-stub-name.cu uses never defined DKERN
variable in a CHECK-NOT directive. This commit replace the variable by a
regex, thereby avoiding the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99832

Files:
  clang/test/CodeGenCUDA/kernel-stub-name.cu


Index: clang/test/CodeGenCUDA/kernel-stub-name.cu
===
--- clang/test/CodeGenCUDA/kernel-stub-name.cu
+++ clang/test/CodeGenCUDA/kernel-stub-name.cu
@@ -126,4 +126,4 @@
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HCKERN]]{{.*}}@[[CKERN]]
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HNSKERN]]{{.*}}@[[NSKERN]]
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HTKERN]]{{.*}}@[[TKERN]]
-// CHECK-NOT: call{{.*}}@__hipRegisterFunction{{.*}}@[[HDKERN]]{{.*}}@[[DKERN]]
+// CHECK-NOT: 
call{{.*}}@__hipRegisterFunction{{.*}}@[[HDKERN]]{{.*}}@{{[0-9]*}}


Index: clang/test/CodeGenCUDA/kernel-stub-name.cu
===
--- clang/test/CodeGenCUDA/kernel-stub-name.cu
+++ clang/test/CodeGenCUDA/kernel-stub-name.cu
@@ -126,4 +126,4 @@
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HCKERN]]{{.*}}@[[CKERN]]
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HNSKERN]]{{.*}}@[[NSKERN]]
 // CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[HTKERN]]{{.*}}@[[TKERN]]
-// CHECK-NOT: call{{.*}}@__hipRegisterFunction{{.*}}@[[HDKERN]]{{.*}}@[[DKERN]]
+// CHECK-NOT: call{{.*}}@__hipRegisterFunction{{.*}}@[[HDKERN]]{{.*}}@{{[0-9]*}}
___
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

2021-04-03 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 335068.
tentzen added a comment.

rebase the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCatchDotDotDot.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp
  clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
  clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp

Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -4415,6 +4415,10 @@
   Assert(
   !F->isIntrinsic() || isa(I) ||
   F->getIntrinsicID() == Intrinsic::donothing ||
+  F->getIntrinsicID() == Intrinsic::seh_try_begin ||
+  F->getIntrinsicID() == Intrinsic::seh_try_end ||
+  F->getIntrinsicID() == Intrinsic::seh_scope_begin ||
+  F->getIntrinsicID() == Intrinsic::seh_scope_end ||
   F->getIntrinsicID() == Intrinsic::coro_resume ||
   F->getIntrinsicID() == Intrinsic::coro_destroy ||
   F->getIntrinsicID() == Intrinsic::experimental_patchpoint_void ||
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2871,6 +2871,10 @@
   llvm_unreachable("Cannot invoke this intrinsic");
 case Intrinsic::donothing:
   // Ignore invokes to @llvm.donothing: jump directly to the next BB.
+case Intrinsic::seh_try_begin:
+case Intrinsic::seh_scope_begin:
+case Intrinsic::seh_try_end:
+case Intrinsic::seh_scope_end:
   break;
 case Intrinsic::experimental_patchpoint_void:
 case Intrinsic::experimental_patchpoint_i64:
@@ -6784,6 +6788,10 @@
   lowerCallToExternalSymbol(I, FunctionName);
 return;
   case Intrinsic::donothing:
+  case Intrinsic::seh_try_begin:
+  case Intrinsic::seh_scope_begin:
+  case Intrinsic::seh_try_end:
+  case Intrinsic::seh_scope_end:
 // ignore
 return;
   case Intrinsic::experimental_stackmap:
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -516,6 +516,16 @@
  [llvm_ptr_ty, llvm_ptr_ty],
  [IntrNoMem]>;
 
+// To mark the beginning/end of a try-scope for Windows SEH -EHa
+//  calls/invokes to these intrinsics are placed to model control flows
+//caused by HW exceptions under option -EHa.
+//  calls/invokes to these intrinsics will be discarded during a codegen pass
+//   after EH tables are generated
+def int_seh_try_begin : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_seh_try_end : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_seh_scope_begin : Intrinsic<[], [], [IntrNoMem]>;
+def int_seh_scope_end : Intrinsic<[], [], [IntrNoMem]>;
+
 // Note: we treat stacksave/stackrestore as writemem because we don't otherwise
 // model their dependencies on allocas.
 def int_stacksave : DefaultAttrsIntrinsic<[llvm_ptr_ty]>,
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -12252,6 +12252,68 @@
 the escaped allocas are allocated, which would break attempts to use
 '``llvm.localrecover``'.
 
+'``llvm.seh.try.begin``' and '``llvm.seh.try.end``' Intrinsics
+^^
+
+Syntax:
+"""
+
+::
+
+  declare void @llvm.seh.try.begin()
+  declare void @llvm.seh.try.end()
+
+Overview:
+"
+
+The '``llvm.seh.try.begin``' and '``llvm.seh.try.end``' intrinsics mark
+the boundary of a _try region for Windows SEH Asynchrous Exception Handling.
+
+Semantics:
+""
+
+When a C-function is compiled with Windows SEH Asynchrous Exception option,
+-feh_asynch (aka MSVC -EHa), these two intrinsics are injected to mar

[PATCH] D99797: [analyzer] Handle intersections and adjacency in RangeSet::Factory::add function

2021-04-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

In D99797#2666565 , @ASDenysPetrov 
wrote:

> @vsavchenko
> OK, what do you think of ***adjacency*** feature? I mean it simplifies such 
> ranges `[1,2][3,4][5,6]` to `[1,6]`. Is it worth for implementation?

I want to clarify my position here.  It's not that I am opposed to this change 
in principle, but I want to understand the motivation (a small example will be 
sufficient) and I don't want to sacrifice a single bit of performance 
efficiency of this part of code.
Even for the `O(N + M)` solution, I'd still be standing strong on keeping the 
old functions as is (except for maybe renaming them).  Range sets are small and 
asymptotics don't work that well when reasoning about the expected performance 
benefits and gains.
For this reason, whenever we can, we should have the simplest operation 
possible in terms of the number of instructions, ie the constant factor is very 
strong here.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:128
 ///
-/// Complexity: O(N + M)
+/// Complexity: O(N + Mlog(N))
 /// where N = size(LHS), M = size(RHS)

ASDenysPetrov wrote:
> vsavchenko wrote:
> > This most certainly can be done in `O(N + M)` the same way the intersection 
> > is done.
> Actually yes, it can. And it was, when I played with different solutions. But 
> the code was poor readable. So I decided make it easier to understand to 
> bring to review. Of couse I can move further to improve it and retain 
> readability. I'll do.
Bring it here and we'll see what can be done on that front. *makeover time!*



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:146
+///
+/// Complexity: O(N + log(N))
+/// where N = size(Original)

nit: `O(N + log(N)) == O(N)`


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

https://reviews.llvm.org/D99797

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


[PATCH] D99797: [analyzer] Handle intersections and adjacency in RangeSet::Factory::add function

2021-04-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:138-139
 
-  return makePersistent(std::move(Result));
-}
+  if (!Original.pin(From, To))
+return getEmptySet();
 

ASDenysPetrov wrote:
> This allows to add a RangeSet of any type. E.g. RangeSet(uchar) + 
> RangeSet(int) = valid, because of `pin`
> 
> I'm wondering whether we really need it here in practice?
We mix ranges of different types way more than one would expect.


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

https://reviews.llvm.org/D99797

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


[PATCH] D99797: [analyzer] Handle intersections and adjacency in RangeSet::Factory::add function

2021-04-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:114-115
+return RHS;
+  for (const Range &R : RHS)
+LHS = add(LHS, R);
+  return LHS;

This is REAL bad.  The main benefit of the new `RangeSet` over the old one is 
the fact that common operations that consist of many basic operations are done 
on mutable containers, i.e. when `RHS` has 10 elements this code will create 
and copy a new array 10 times discarding 9 of them.

That's why every implementation method here operates on mutable `ContainerType` 
and then makes is persistent.

Additionally, merging add can be done with one iteration over two containers, 
but instead we do `O(N)` operation here `M` times, so it is not `O(N + M)`, but 
`O(N * M)`.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:142
+  ContainerType::size_type InsertIdx = 0;
+  for (auto it = Original.begin(), e = Original.end(); it != e; ++it) {
+const llvm::APSInt &RFrom = it->From();

Is there a reason not to use range-based loop in this case?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:150
+if (!IsIntersected) {
+  auto One = APSIntType(From).getValue(1);
+  const bool isCurrentRangeOnTheLeft = RTo < From;

This should be done outside of the loop, we assume that all the ranges are of 
the same type.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:172
+  Range NewRange{ValueFactory.getValue(From), ValueFactory.getValue(To)};
+  Result.insert(Result.begin() + InsertIdx, NewRange);
+

This is a problem here.  This essentially doubles the work you did before.  
What can be done in one `O(N)` loop is done with two.

However, I don't really see a point in fixing this algorithm because the more 
generic `RangeSet` + `RangeSet` should be optimal `O(N + M)` and this one can 
be implemented as a special case.


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

https://reviews.llvm.org/D99797

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


[PATCH] D99837: [Windows] Turn off text mode correctly in Rewriter to stop CRLF translation

2021-04-03 Thread Abhina Sree via Phabricator via cfe-commits
abhina.sreeskantharajan created this revision.
abhina.sreeskantharajan added a reviewer: aganea.
abhina.sreeskantharajan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I incorrectly changed the RewriteTestAction::ExecuteAction's file to binary 
instead of the proper RewriteIncludesAction::BeginSourceFileAction in 
https://reviews.llvm.org/rGbc5d4bcc2deb71ab647270c9754a83484b3d6f87. In the 
original commit, I actually changed 
RewriteIncludesAction::BeginSourceFileAction in 
https://reviews.llvm.org/rGfdb640ea30d416368b76b68b106deda580c6aced. This 
should fix the issue @aganea is facing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99837

Files:
  clang/lib/Frontend/Rewrite/FrontendActions.cpp


Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -194,7 +194,7 @@
 void RewriteTestAction::ExecuteAction() {
   CompilerInstance &CI = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(/*Binary=*/true, 
getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(/*Binary=*/false, 
getCurrentFileOrBufferName());
   if (!OS) return;
 
   DoRewriteTest(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance &CI) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(/*Binary=*/false, 
getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(/*Binary=*/true, 
getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }


Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -194,7 +194,7 @@
 void RewriteTestAction::ExecuteAction() {
   CompilerInstance &CI = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(/*Binary=*/true, getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(/*Binary=*/false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   DoRewriteTest(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance &CI) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(/*Binary=*/false, getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(/*Binary=*/true, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99426: [SystemZ][z/OS][Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

2021-04-03 Thread Abhina Sree via Phabricator via cfe-commits
abhina.sreeskantharajan added a comment.

In D99426#2666341 , @aganea wrote:

> In D99426#2666141 , 
> @abhina.sreeskantharajan wrote:
>
>> In D99426#2665361 , @aganea wrote:
>>
>>> I am still concerned by the fact that this patch doesn't fix the issue 
>>> mentionned in https://reviews.llvm.org/D96363#2650460
>>> Was the intention to fix that issue? Will the fix be done in a subsequent 
>>> patch?
>>
>> I was fairly confident that if https://reviews.llvm.org/D96363 was the patch 
>> that was causing the issue for you
>
> That is the case! I've confirmed that `git checkout 
> fdb640ea30d416368b76b68b106deda580c6aced~1 && ninja clang -C build` generates 
> a `clang-cl.exe` that works with my above test case. `git revert 
> fdb640ea30d416368b76b68b106deda580c6aced` locally over ToT fixes the issue.

After scratching my head, I realized that I accidentally changed another 
function to Binary in clang/lib/Frontend/Rewrite/FrontendActions.cpp so that 
change was never properly reverted. I fixed this in 
https://reviews.llvm.org/D99837. Hopefully, this will fix your issue. Apologies 
for the confusion on my side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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


[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

2021-04-03 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a reviewer: steveire.
steveire added a comment.

Yes, please remove the line instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99715

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


[PATCH] D99037: [Matrix] Implement explicit type conversions for matrix types

2021-04-03 Thread Saurabh Jha via Phabricator via cfe-commits
SaurabhJha updated this revision to Diff 335083.
SaurabhJha added a comment.

Addressed most of the comments. I couldn't understand "..would also be good to 
have C++ tests that test casting with matrix types where some of the dimensions 
are template arguments...". When I tried this

"""
cx4x4 m1;

(void)(ix4x4)m1
"""

it gave me the error
"""
C-style cast from 'cx4x4' (aka 'char __attribute__((matrix_type(4, 4)))') to 
'ix4x4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed
"""
should I address it as part of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c

Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef struct test_struct {
+} test_struct;
+
+void f1() {
+  cx4x4 m1;
+  ix4x4 m2;
+  sx4x4 m3;
+  ix5x5 m4;
+  fx5x5 m5;
+  int i;
+  vec v;
+  test_struct *s;
+
+  m2 = (ix4x4)m1;
+  m3 = (sx4x4)m2;
+  m4 = (ix5x5)m3; // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and 'sx4x4' \
+(aka 'short __attribute__((matrix_type(4, 4)))') of different size}}
+  m5 = (ix5x5)m4; // expected-error {{assigning to 'fx5x5' (aka \
+'float __attribute__((matrix_type(5, 5)))') from incompatible type 'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))')}}
+  i = (int)m4;// expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'int'}}
+  m4 = (ix5x5)i;  // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'int'}}
+  v = (vec)m4;// expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'vec' (vector of 1 'int' value)}}
+  m4 = (ix5x5)v;  // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'vec' (vector of 1 'int' value)}}
+  s = (test_struct *)m3; // expected-error {{invalid conversion between matrix type \
+'sx4x4' (aka 'short __attribute__((matrix_type(4, 4)))') and non matrix type \
+'test_struct *' (aka 'struct test_struct *')}}
+  m3 = (sx4x4)s; // expected-error {{invalid conversion between matrix type \
+'sx4x4' (aka 'short __attribute__((matrix_type(4, 4)))') and non matrix type \
+'test_struct *' (aka 'struct test_struct *')}}
+
+  m4 = (ix5x5)m5;
+}
+
+typedef float float2_8x8 __attribute__((matrix_type(8, 8)));
+typedef double double_10x10 __attribute__((matrix_type(10, 10)));
+typedef double double_8x8 __attribute__((matrix_type(8, 8)));
+typedef signed int signed_int_12x12 __attribute__((matrix_type(12, 12)));
+typedef unsigned int unsigned_int_12x12 __attribute__((matrix_type(12, 12)));
+typedef unsigned int unsigned_int_10x10 __attribute__((matrix_type(10, 10)));
+
+void f2() {
+  float2_8x8 m1;
+  double_10x10 m2;
+  double_8x8 m3;
+  signed_int_12x12 m4;
+  unsigned_int_12x12 m5;
+  unsigned_int_10x10 m6;
+  float f;
+
+  m2 = (double_10x10)m1; // expected-error {{invalid conversion between matrix type \
+'double_10x10' (aka 'double __attribute__((matrix_type(10, 10)))') and 'float2_8x8' \
+(aka 'float __attribute__((matrix_type(8, 8)))') of different size}}
+  m3 = (double_8x8)m1;
+
+  m5 = (unsigned_int_12x12)m4;
+  m4 = (signed_int_12x12)m5;
+  m6 = (unsigned_int_10x10)m4; // expected-error {{invalid conversion between matrix type \
+'unsigned_int_10x10' (aka 'unsigned int __attribute__((matrix_type(10, 10)))') and 'signed_int_12x12' \
+(aka 'int __attribute__((matrix_type(12, 12)))') of different size}}
+  f = (float)m4;   // expected-error {{invalid conversion between matrix type \
+'signed_int_12x12' (aka 'int __attribute__((matrix_type(12, 12)))') and non matrix ty

[PATCH] D99037: [Matrix] Implement C-style explicit type conversions for matrix types

2021-04-03 Thread Saurabh Jha via Phabricator via cfe-commits
SaurabhJha updated this revision to Diff 335084.
SaurabhJha added a comment.

Update one inline comment in SemaCast.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99037

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/Sema/matrix-cast.c

Index: clang/test/Sema/matrix-cast.c
===
--- /dev/null
+++ clang/test/Sema/matrix-cast.c
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -fenable-matrix -fsyntax-only %s -verify
+
+typedef char cx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix4x4 __attribute__((matrix_type(4, 4)));
+typedef short sx4x4 __attribute__((matrix_type(4, 4)));
+typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+typedef int vec __attribute__((vector_size(4)));
+typedef struct test_struct {
+} test_struct;
+
+void f1() {
+  cx4x4 m1;
+  ix4x4 m2;
+  sx4x4 m3;
+  ix5x5 m4;
+  fx5x5 m5;
+  int i;
+  vec v;
+  test_struct *s;
+
+  m2 = (ix4x4)m1;
+  m3 = (sx4x4)m2;
+  m4 = (ix5x5)m3; // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and 'sx4x4' \
+(aka 'short __attribute__((matrix_type(4, 4)))') of different size}}
+  m5 = (ix5x5)m4; // expected-error {{assigning to 'fx5x5' (aka \
+'float __attribute__((matrix_type(5, 5)))') from incompatible type 'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))')}}
+  i = (int)m4;// expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'int'}}
+  m4 = (ix5x5)i;  // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'int'}}
+  v = (vec)m4;// expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'vec' (vector of 1 'int' value)}}
+  m4 = (ix5x5)v;  // expected-error {{invalid conversion between matrix type \
+'ix5x5' (aka 'int __attribute__((matrix_type(5, 5)))') and non matrix type \
+'vec' (vector of 1 'int' value)}}
+  s = (test_struct *)m3; // expected-error {{invalid conversion between matrix type \
+'sx4x4' (aka 'short __attribute__((matrix_type(4, 4)))') and non matrix type \
+'test_struct *' (aka 'struct test_struct *')}}
+  m3 = (sx4x4)s; // expected-error {{invalid conversion between matrix type \
+'sx4x4' (aka 'short __attribute__((matrix_type(4, 4)))') and non matrix type \
+'test_struct *' (aka 'struct test_struct *')}}
+
+  m4 = (ix5x5)m5;
+}
+
+typedef float float2_8x8 __attribute__((matrix_type(8, 8)));
+typedef double double_10x10 __attribute__((matrix_type(10, 10)));
+typedef double double_8x8 __attribute__((matrix_type(8, 8)));
+typedef signed int signed_int_12x12 __attribute__((matrix_type(12, 12)));
+typedef unsigned int unsigned_int_12x12 __attribute__((matrix_type(12, 12)));
+typedef unsigned int unsigned_int_10x10 __attribute__((matrix_type(10, 10)));
+
+void f2() {
+  float2_8x8 m1;
+  double_10x10 m2;
+  double_8x8 m3;
+  signed_int_12x12 m4;
+  unsigned_int_12x12 m5;
+  unsigned_int_10x10 m6;
+  float f;
+
+  m2 = (double_10x10)m1; // expected-error {{invalid conversion between matrix type \
+'double_10x10' (aka 'double __attribute__((matrix_type(10, 10)))') and 'float2_8x8' \
+(aka 'float __attribute__((matrix_type(8, 8)))') of different size}}
+  m3 = (double_8x8)m1;
+
+  m5 = (unsigned_int_12x12)m4;
+  m4 = (signed_int_12x12)m5;
+  m6 = (unsigned_int_10x10)m4; // expected-error {{invalid conversion between matrix type \
+'unsigned_int_10x10' (aka 'unsigned int __attribute__((matrix_type(10, 10)))') and 'signed_int_12x12' \
+(aka 'int __attribute__((matrix_type(12, 12)))') of different size}}
+  f = (float)m4;   // expected-error {{invalid conversion between matrix type \
+'signed_int_12x12' (aka 'int __attribute__((matrix_type(12, 12)))') and non matrix type \
+'float'}}
+  m4 = (signed_int_12x12)f;// expected-error {{invalid conversion between matrix type \
+'signed_int_12x12' (aka 'int __attribute__((matrix_type(12, 12)))') and non matrix type \
+'float'}}
+}
\ No newline at end of file
Index: clang/test/CodeGen/matrix-cast.c
===
--- /dev/null
+++ clang/test/CodeGen/matrix-cast.c
@@ -0,0 +1,100 @@
+//

[PATCH] D99838: [C++20, test] Fix use of undef FileCheck variable

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre created this revision.
thopre added reviewers: rsmith, EricWF, akhuang.
thopre requested review of this revision.
Herald added a project: clang.

Commit f495de43bd5da50286da6020e508d106cfc60f57 
 forgot 
two lines when
removing checks for strong and weak equality, resulting in the use of an
undefined FileCheck variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99838

Files:
  clang/test/CodeGenCXX/cxx2a-compare.cpp


Index: clang/test/CodeGenCXX/cxx2a-compare.cpp
===
--- clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -9,8 +9,6 @@
 // Ensure we don't emit definitions for the global variables
 // since the builtins shouldn't ODR use them.
 // CHECK-NOT: constant %[[SO]]
-// CHECK-NOT: constant %[[SE]]
-// CHECK-NOT: constant %[[WE]]
 // CHECK-NOT: constant %[[PO]]
 
 // CHECK-LABEL: @_Z11test_signedii


Index: clang/test/CodeGenCXX/cxx2a-compare.cpp
===
--- clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -9,8 +9,6 @@
 // Ensure we don't emit definitions for the global variables
 // since the builtins shouldn't ODR use them.
 // CHECK-NOT: constant %[[SO]]
-// CHECK-NOT: constant %[[SE]]
-// CHECK-NOT: constant %[[WE]]
 // CHECK-NOT: constant %[[PO]]
 
 // CHECK-LABEL: @_Z11test_signedii
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99839: [C++, test] Fix typo in NSS* vars

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre created this revision.
thopre added reviewers: respindola, pcc, EricWF.
thopre requested review of this revision.
Herald added a project: clang.

The NSS FileCheck variables at the end of the
CodeGenCXX/split-stacks.cpp clang testcase are off by 1, resulting in
the use of an undefined variable (NSS3). This commit fixes that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99839

Files:
  clang/test/CodeGenCXX/split-stacks.cpp


Index: clang/test/CodeGenCXX/split-stacks.cpp
===
--- clang/test/CodeGenCXX/split-stacks.cpp
+++ clang/test/CodeGenCXX/split-stacks.cpp
@@ -28,6 +28,6 @@
 // CHECK-NOSEGSTK: define dso_local i32 @_Z3foov() [[NSS0:#[0-9]+]] {
 // CHECK-NOSEGSTK: define dso_local i32 @_Z7nosplitv() [[NSS1:#[0-9]+]] {
 // CHECK-NOSEGSTK: define linkonce_odr dso_local i32 @_Z8tnosplitIiEiv() 
[[NSS2:#[0-9]+]] comdat {
+// CHECK-NOSEGSTK-NOT: [[NSS0]] = { {{.*}} "split-stack" {{.*}} }
 // CHECK-NOSEGSTK-NOT: [[NSS1]] = { {{.*}} "split-stack" {{.*}} }
 // CHECK-NOSEGSTK-NOT: [[NSS2]] = { {{.*}} "split-stack" {{.*}} }
-// CHECK-NOSEGSTK-NOT: [[NSS3]] = { {{.*}} "split-stack" {{.*}} }


Index: clang/test/CodeGenCXX/split-stacks.cpp
===
--- clang/test/CodeGenCXX/split-stacks.cpp
+++ clang/test/CodeGenCXX/split-stacks.cpp
@@ -28,6 +28,6 @@
 // CHECK-NOSEGSTK: define dso_local i32 @_Z3foov() [[NSS0:#[0-9]+]] {
 // CHECK-NOSEGSTK: define dso_local i32 @_Z7nosplitv() [[NSS1:#[0-9]+]] {
 // CHECK-NOSEGSTK: define linkonce_odr dso_local i32 @_Z8tnosplitIiEiv() [[NSS2:#[0-9]+]] comdat {
+// CHECK-NOSEGSTK-NOT: [[NSS0]] = { {{.*}} "split-stack" {{.*}} }
 // CHECK-NOSEGSTK-NOT: [[NSS1]] = { {{.*}} "split-stack" {{.*}} }
 // CHECK-NOSEGSTK-NOT: [[NSS2]] = { {{.*}} "split-stack" {{.*}} }
-// CHECK-NOSEGSTK-NOT: [[NSS3]] = { {{.*}} "split-stack" {{.*}} }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-04-03 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D69218#2654638 , @nick wrote:

> In D69218#2654614 , @steveire wrote:
>
>> @nick Sorry that getting these changes merged takes so long.
>>
>> @njames93 If you have an alternative way forward, please let us know what it 
>> is.
>>
>> Otherwise, this LGTM too and we should merge it soon unless there are 
>> objections which haven't been addressed.
>
> I had a PR in Boost that took 4 years to merge, so it is nothing new to me :D

Yes, I'm a few years trying to get 
https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/
 merged, but I've made some progress :D.

> Rebased, even though there was no conflicts.

I think `arc patch` might be a bit fickle, so it wasn't applying cleanly for me 
before.

This LGTM and there have been no further objections, and the objection from 
@njames93 seems to be addressed.

What name/email should I use for the commit?




Comment at: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp:301
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =

nick wrote:
> steveire wrote:
> > @nick Is this implemented in another MR? I don't see anything in your list 
> > of revisions. I think this is reasonable as is, but wondering if you intend 
> > to implement the top-level support too.
> The patch had some of the top-level matcher parts, but it cut them off when 
> rebased. I have no need in it myself and I am not sure there is any kind of 
> use for it.
If you look at the overloads of `MatchFinder::addMatcher` and compare to, say, 
the `using` declarations near the top of `ASTMatchers.h` or the types supported 
in `ASTNodeKind` or `DynTypedNode::BaseConverter`, this one is certainly 
"missing" as a supported top level node in `MatchFinder::addMatcher` (though 
it's not the only one in any case). 

That said I don't think merging this MR should depend on that feature.

Hopefully someone will submit it for review. I have the same difficulty with 
getting changes reviewed, but I can review them and get them towards approval 
and merge more easily than I can find a reviewer :).


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

https://reviews.llvm.org/D69218

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


[PATCH] D99797: [analyzer] Handle intersections and adjacency in RangeSet::Factory::add function

2021-04-03 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

@vsavchenko Many thanks for your feedback!
I will make a new separate function for checking intersections considering all 
your suggestions along with the old quick `add` versions. I'll be more 
optimized.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:142
+  ContainerType::size_type InsertIdx = 0;
+  for (auto it = Original.begin(), e = Original.end(); it != e; ++it) {
+const llvm::APSInt &RFrom = it->From();

vsavchenko wrote:
> Is there a reason not to use range-based loop in this case?
You are right. Working on restoring O(N) I been played with iterators, then 
found another solution, but forgot to return range-based loop back. Thnx.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:150
+if (!IsIntersected) {
+  auto One = APSIntType(From).getValue(1);
+  const bool isCurrentRangeOnTheLeft = RTo < From;

vsavchenko wrote:
> This should be done outside of the loop, we assume that all the ranges are of 
> the same type.
+1 I'll move it outside the loop.


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

https://reviews.llvm.org/D99797

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


Re: [clang] 2458aa0 - Add missing override to clang tblgen AttrEmitter

2021-04-03 Thread Aaron Ballman via cfe-commits
Thank you for this cleanup!

~Aaron

On Sat, Apr 3, 2021 at 12:00 AM David Blaikie via cfe-commits
 wrote:
>
>
> Author: David Blaikie
> Date: 2021-04-02T20:47:49-07:00
> New Revision: 2458aa0b9136e7616f529b027d1d478cf699340f
>
> URL: 
> https://github.com/llvm/llvm-project/commit/2458aa0b9136e7616f529b027d1d478cf699340f
> DIFF: 
> https://github.com/llvm/llvm-project/commit/2458aa0b9136e7616f529b027d1d478cf699340f.diff
>
> LOG: Add missing override to clang tblgen AttrEmitter
>
> Added:
>
>
> Modified:
> clang/utils/TableGen/ClangAttrEmitter.cpp
>
> Removed:
>
>
>
> 
> diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
> b/clang/utils/TableGen/ClangAttrEmitter.cpp
> index bddda1fe47f7..0d8439b697c8 100644
> --- a/clang/utils/TableGen/ClangAttrEmitter.cpp
> +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
> @@ -3677,7 +3677,7 @@ static void GenerateMutualExclusionsChecks(const Record 
> &Attr,
>if (!DeclAttrs.empty()) {
>  // Generate the ParsedAttrInfo subclass logic for declarations.
>  OS << "  bool diagMutualExclusion(Sema &S, const ParsedAttr &AL, "
> -   << "const Decl *D) const {\n";
> +   << "const Decl *D) const override {\n";
>  for (const std::string &A : DeclAttrs) {
>OS << "if (const auto *A = D->getAttr<" << A << ">()) {\n";
>OS << "  S.Diag(AL.getLoc(), 
> diag::err_attributes_are_not_compatible)"
> @@ -3714,7 +3714,7 @@ static void GenerateMutualExclusionsChecks(const Record 
> &Attr,
>if (!StmtAttrs.empty()) {
>  // Generate the ParsedAttrInfo subclass logic for statements.
>  OS << "  bool diagMutualExclusion(Sema &S, const ParsedAttr &AL, "
> -   << "const Stmt *St) const {\n";
> +   << "const Stmt *St) const override {\n";
>  OS << "if (const auto *AS = dyn_cast(St)) {\n";
>  OS << "  const ArrayRef &Attrs = AS->getAttrs();\n";
>  for (const std::string &A : StmtAttrs) {
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

2021-04-03 Thread Luna Kirkby via Phabricator via cfe-commits
lunasorcery created this revision.
lunasorcery added reviewers: djasper, klimek.
lunasorcery added a project: clang-format.
lunasorcery requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, with AllowShortEnumsOnASingleLine disabled, enums that would have 
otherwise fit on a single line would always put the opening brace on its own 
line.
This patch ensures that these enums will only put the brace on its own line if 
the existing attachment rules indicate that it should.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99840

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -343,8 +343,7 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var\n"
-   "{\n"
+  verifyFormat("public enum var {\n"
"none,\n"
"@string,\n"
"bool,\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1344,6 +1344,14 @@
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} ShortEnum1, ShortEnum2;",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
"{\n"
"  A,\n"
@@ -19454,8 +19462,7 @@
Style);
   // Enumerations are not records and should be unaffected.
   Style.AllowShortEnumsOnASingleLine = false;
-  verifyFormat("enum class E\n"
-   "{\n"
+  verifyFormat("enum class E {\n"
"  A,\n"
"  B\n"
"};\n",
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2455,6 +2455,8 @@
   if (FormatTok->Tok.is(tok::kw_enum))
 nextToken();
 
+  const FormatToken &InitialToken = *FormatTok;
+
   // In TypeScript, "enum" can also be used as property name, e.g. in interface
   // declarations. An "enum" keyword followed by a colon would be a syntax
   // error and thus assume it is just an identifier.
@@ -2501,7 +2503,8 @@
 return true;
   }
 
-  if (!Style.AllowShortEnumsOnASingleLine)
+  if (!Style.AllowShortEnumsOnASingleLine &&
+  ShouldBreakBeforeBrace(Style, InitialToken))
 addUnwrappedLine();
   // Parse enum body.
   nextToken();
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -515,8 +515,7 @@
   ///   enum { A, B } myEnum;
   ///
   ///   false:
-  ///   enum
-  ///   {
+  ///   enum {
   /// A,
   /// B
   ///   } myEnum;


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -343,8 +343,7 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
-  verifyFormat("public enum var\n"
-   "{\n"
+  verifyFormat("public enum var {\n"
"none,\n"
"@string,\n"
"bool,\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1344,6 +1344,14 @@
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} ShortEnum1, ShortEnum2;",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
"{\n"
"  A,\n"
@@ -19454,8 +19462,7 @@
Style);
   // Enumerations are not records and should be unaffected.
   Style.AllowShortEnumsOnASingleLine = false;
-  verifyFormat("enum class E\n"
-   "{\n"
+  verifyFormat("enum class E {\n"
"  A,\n"
"  B\n"
"};\n",
Index: clang/lib/Fo

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for the additional test case, this LGTM again.


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

https://reviews.llvm.org/D75844

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


[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:458
+same number of arguments as the caller. The types of the return value and all
+arguments must be similar, including the implicit "this" argument, if any.
+Any variables in scope, including all arguments to the function and the

It'd be nice if we could nail down "similar" somewhat. I don't know if `int` 
and `short` are similar (due to promotions) or `const int` and `int` are 
similar, etc.



Comment at: clang/include/clang/Basic/AttrDocs.td:461-462
+return value must be trivially destructible. The calling convention of the
+caller and callee must match, and they must not have old style K&R C function
+declarations.
+  }];

Not only is this not usable with K&R C declarations, but it's also not usable 
with `...` variadic functions either, right?



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

haberman wrote:
> aaron.ballman wrote:
> > This functionality belongs in SemaStmtAttr.cpp, I think.
> That is where I had originally put it, but that didn't work for templates. 
> The semantic checks can only be performed at instantiation time. 
> `ActOnAttributedStmt` seems to be the right hook point where I can evaluate 
> the semantic checks for both template and non-template functions (with 
> template functions getting checked at instantiation time).
I disagree that `ActOnAttributedStmt()` is the correct place for this checking 
-- template checking should occur when the template is instantiated, same as 
happens for declaration attributes. I'd like to see this functionality moved to 
SemaStmtAttr.cpp. Keeping the attribute logic together and following the same 
patterns is what allows us to tablegenerate more of the attribute logic. 
Statement attributes are just starting to get more such automation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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


[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-04-03 Thread Nikita Kniazev via Phabricator via cfe-commits
nick added a comment.

In D69218#2667523 , @steveire wrote:

> What name/email should I use for the commit?

Nikita Kniazev 


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

https://reviews.llvm.org/D69218

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


[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 335103.
haberman marked 3 inline comments as done.
haberman added a comment.

- Addressed comments and tried moving check to SemaStmtAttr.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'mus

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:588
+
+  const CallExpr *CE = dyn_cast(Ex->IgnoreUnlessSpelledInSource());
+

rsmith wrote:
> `IgnoreUnlessSpelledInSource` is a syntactic check that's only really 
> intended for tooling use cases; I think we want something a bit more semantic 
> here, so `IgnoreImplicitAsWritten` would be more appropriate.
> 
> I think it would be reasonable to also skip "parentheses" here (which we 
> treat as also including things like C's `_Generic`). Would 
> `Ex->IgnoreImplicitAsWritten()->IgnoreParens()` work?
> 
> If we're going to skip elidable copy construction of the result here (which I 
> think we should), should we also reflect that in the AST? Perhaps we should 
> strip the return value down to being just the call expression? I'm thinking 
> in particular of things like building in C++14 or before with 
> `-fno-elide-constructors`, where code generation for a by-value return of a 
> class object will synthesize a local temporary to hold the result, with a 
> final destination copy emitted after the call. (Testcase: `struct A { A(const 
> A&); }; A f(); A g() { [[clang::musttail]] return f(); }` with 
> `-fno-elide-constructors`.)
`IgnoreImplicitAsWritten()` doesn't skip `ExprWithCleanups`, and per your 
previous comment I was trying to find a `CallExpr` before doing the check 
prohibiting `ExprWithCleanups` with side effects.

I could write some custom ignore logic using `clang::IgnoreExprNodes()` 
directly.

> If we're going to skip elidable copy construction of the result here (which I 
> think we should)

To clarify, are you suggesting that we allow `musttail` through elidable copy 
constructors on the return value, even if `-fno-elide-constructors` is set? ie. 
we consider that `musttail` overrides the `-fno-elide-constructors` option on 
the command line?



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > That is where I had originally put it, but that didn't work for templates. 
> > The semantic checks can only be performed at instantiation time. 
> > `ActOnAttributedStmt` seems to be the right hook point where I can evaluate 
> > the semantic checks for both template and non-template functions (with 
> > template functions getting checked at instantiation time).
> I disagree that `ActOnAttributedStmt()` is the correct place for this 
> checking -- template checking should occur when the template is instantiated, 
> same as happens for declaration attributes. I'd like to see this 
> functionality moved to SemaStmtAttr.cpp. Keeping the attribute logic together 
> and following the same patterns is what allows us to tablegenerate more of 
> the attribute logic. Statement attributes are just starting to get more such 
> automation.
I tried commenting out this code and adding the following code into 
`handleMustTailAttr()` in `SemaStmtAttr.cpp`:

```
  if (!S.checkMustTailAttr(St, MTA))
return nullptr;
```

This caused my test cases related to templates to fail. It also seemed to break 
test cases related to `JumpDiagnostics`. My interpretation of this is that 
`handleMustTailAttr()` is called during parsing only, and cannot catch errors 
at template instantiation time or that require a more complete AST.

What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I put this 
check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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


[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 335106.
haberman added a comment.

- Added missing S.setFunctionHasMustTail().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'musttail' attribute requires that the return value, all parameters,

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

haberman wrote:
> aaron.ballman wrote:
> > haberman wrote:
> > > aaron.ballman wrote:
> > > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > > That is where I had originally put it, but that didn't work for 
> > > templates. The semantic checks can only be performed at instantiation 
> > > time. `ActOnAttributedStmt` seems to be the right hook point where I can 
> > > evaluate the semantic checks for both template and non-template functions 
> > > (with template functions getting checked at instantiation time).
> > I disagree that `ActOnAttributedStmt()` is the correct place for this 
> > checking -- template checking should occur when the template is 
> > instantiated, same as happens for declaration attributes. I'd like to see 
> > this functionality moved to SemaStmtAttr.cpp. Keeping the attribute logic 
> > together and following the same patterns is what allows us to tablegenerate 
> > more of the attribute logic. Statement attributes are just starting to get 
> > more such automation.
> I tried commenting out this code and adding the following code into 
> `handleMustTailAttr()` in `SemaStmtAttr.cpp`:
> 
> ```
>   if (!S.checkMustTailAttr(St, MTA))
> return nullptr;
> ```
> 
> This caused my test cases related to templates to fail. It also seemed to 
> break test cases related to `JumpDiagnostics`. My interpretation of this is 
> that `handleMustTailAttr()` is called during parsing only, and cannot catch 
> errors at template instantiation time or that require a more complete AST.
> 
> What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I put 
> this check?
Scratch the part about `JumpDiagnostics`, that was me failing to call 
`S.setFunctionHasMustTail()`. I added that and now the `JumpDiagnostics` tests 
pass.

But the template test cases still fail, and I can't find any hook point in 
`SemaStmtAttr.cpp` that will let me evaluate these checks at template 
instantiation time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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


[PATCH] D99848: [OPENMP51]Initial support for nocontext clause

2021-04-03 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 created this revision.
jyu2 added reviewers: mikerice, ABataev, jdoerfert.
Herald added subscribers: arphaman, guansong, yaxunl.
Herald added a reviewer: sscalpone.
jyu2 requested review of this revision.
Herald added a subscriber: sstefan1.
Herald added projects: clang, LLVM.

Added basic parsing/sema/serialization support for the 'nocontext' clause.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99848

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/dispatch_ast_print.cpp
  clang/test/OpenMP/dispatch_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -282,6 +282,10 @@
   let clangClass = "OMPNovariantsClause";
   let flangClass = "ScalarLogicalExpr";
 }
+def OMPC_Nocontext : Clause<"nocontext"> {
+  let clangClass = "OMPNocontextClause";
+  let flangClass = "ScalarLogicalExpr";
+}
 def OMPC_Detach : Clause<"detach"> {
   let clangClass = "OMPDetachClause";
 }
@@ -1667,7 +1671,8 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_Unknown : Directive<"unknown"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -730,6 +730,7 @@
 CHECK_SIMPLE_CLAUSE(Init, OMPC_init)
 CHECK_SIMPLE_CLAUSE(Use, OMPC_use)
 CHECK_SIMPLE_CLAUSE(Novariants, OMPC_novariants)
+CHECK_SIMPLE_CLAUSE(Nocontext, OMPC_nocontext)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Allocator, OMPC_allocator)
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2295,6 +2295,10 @@
   Visitor->AddStmt(C->getCondition());
 }
 
+void OMPClauseEnqueue::VisitOMPNocontextClause(const OMPNocontextClause *C) {
+  Visitor->AddStmt(C->getCondition());
+}
+
 void OMPClauseEnqueue::VisitOMPUnifiedAddressClause(
 const OMPUnifiedAddressClause *) {}
 
Index: clang/test/OpenMP/dispatch_messages.cpp
===
--- clang/test/OpenMP/dispatch_messages.cpp
+++ clang/test/OpenMP/dispatch_messages.cpp
@@ -46,6 +46,24 @@
   // expected-error@+1 {{use of undeclared identifier 'x'}}
   #pragma omp dispatch novariants(x)
   disp_call();
+  
+  // expected-error@+1 {{expected '(' after 'nocontext'}}
+  #pragma omp dispatch nocontext
+  disp_call();
+
+  // expected-error@+3 {{expected expression}}
+  // expected-error@+2 {{expected ')'}}
+  // expected-note@+1 {{to match this '('}}
+  #pragma omp dispatch nocontext (
+  disp_call();
+
+  // expected-error@+1 {{cannot contain more than one 'nocontext' clause}}
+  #pragma omp dispatch nocontext(dnum> 4) nocontext(3)
+  disp_call();
+
+  // expected-error@+1 {{use of undeclared identifier 'x'}}
+  #pragma omp dispatch nocontext(x)
+  disp_call();
 }
 
 void testit_two() {
Index: clang/test/OpenMP/dispatch_ast_print.cpp
===
--- clang/test/OpenMP/dispatch_ast_print.cpp
+++ clang/test/OpenMP/dispatch_ast_print.cpp
@@ -51,22 +51,22 @@
 void test_one()
 {
   int aaa, bbb, var;
-  //PRINT: #pragma omp dispatch depend(in : var) nowait novariants(aaa > 5)
+  //PRINT: #pragma omp dispatch depend(in : var) nowait novariants(aaa > 5) nocontext(bbb > 5)
   //DUMP: OMPDispatchDirective
   //DUMP: OMPDependClause
   //DUMP: OMPNowaitClause
   //DUMP: OMPNovariantsClause
-  #pragma omp dispatch depend(in:var) nowait novariants(aaa > 5)
+  #pragma omp dispatch depend(in:var) nowait novariants(aaa > 5) nocontext(bbb > 5)
   foo(aaa, &bbb);
 
   int *dp = get_device_ptr();
   int dev = get_device();
-  //PRINT: #pragma omp dispatch device(dev) is_device_ptr(dp) novariants(dev > 10)
+  //PRINT: #pragma omp dispatch device(dev) is_device_ptr(dp) novariants(dev > 10) nocontext(dev > 5)
   //DUMP: OMPDispatchDirective
   //DUMP: OMPDeviceClause
   //DUMP: OMPIs_device_ptrClause
   //DUMP: OMPNovariantsClause
-  #pragma omp dispatch device(dev) is_device_ptr(dp) novariants(dev > 10)
+  #pragma omp dispatch device(dev) is_device_ptr(dp) novariants(dev > 10) nocontext(dev > 5

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

2021-04-03 Thread Benson Chu via Phabricator via cfe-commits
pestctrl updated this revision to Diff 335113.
pestctrl edited the summary of this revision.
pestctrl added a comment.

Removed extra pass over decl chain for acting definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section 
"datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2193,14 +2193,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2193,14 +2193,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

2021-04-03 Thread Benson Chu via Phabricator via cfe-commits
pestctrl added inline comments.



Comment at: clang/lib/AST/Decl.cpp:2192
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;

rsmith wrote:
> Is there a good reason to switch from checking for a `Definition` in the loop 
> to checking it ahead of time? This change means we'll do two passes over the 
> redeclarations, and call `isThisDeclarationADefinition` twice for each, where 
> the old approach only performed one pass.
My thought was that separating the logic would make the loop easier to read, 
but I see that this causes 2 passes vs. 1. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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


[PATCH] D99811: [TextAPI] move source code files out of subdirectory, NFC

2021-04-03 Thread Jez Ng via Phabricator via cfe-commits
int3 accepted this revision.
int3 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/D99811/new/

https://reviews.llvm.org/D99811

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


[clang] 1cc9d94 - [C++20, test] Fix use of undef FileCheck variable

2021-04-03 Thread Thomas Preud'homme via cfe-commits

Author: Thomas Preud'homme
Date: 2021-04-04T00:05:48+01:00
New Revision: 1cc9d949a1233e8b17b3b345ccb67ca7296c1a6c

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

LOG: [C++20, test] Fix use of undef FileCheck variable

Commit f495de43bd5da50286da6020e508d106cfc60f57 forgot two lines when
removing checks for strong and weak equality, resulting in the use of an
undefined FileCheck variable.

Reviewed By: Quuxplusone

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

Added: 


Modified: 
clang/test/CodeGenCXX/cxx2a-compare.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/cxx2a-compare.cpp 
b/clang/test/CodeGenCXX/cxx2a-compare.cpp
index 262fead875797..48ce956d9d402 100644
--- a/clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ b/clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -9,8 +9,6 @@
 // Ensure we don't emit definitions for the global variables
 // since the builtins shouldn't ODR use them.
 // CHECK-NOT: constant %[[SO]]
-// CHECK-NOT: constant %[[SE]]
-// CHECK-NOT: constant %[[WE]]
 // CHECK-NOT: constant %[[PO]]
 
 // CHECK-LABEL: @_Z11test_signedii



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


[PATCH] D99838: [C++20, test] Fix use of undef FileCheck variable

2021-04-03 Thread Thomas Preud'homme via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1cc9d949a123: [C++20, test] Fix use of undef FileCheck 
variable (authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99838

Files:
  clang/test/CodeGenCXX/cxx2a-compare.cpp


Index: clang/test/CodeGenCXX/cxx2a-compare.cpp
===
--- clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -9,8 +9,6 @@
 // Ensure we don't emit definitions for the global variables
 // since the builtins shouldn't ODR use them.
 // CHECK-NOT: constant %[[SO]]
-// CHECK-NOT: constant %[[SE]]
-// CHECK-NOT: constant %[[WE]]
 // CHECK-NOT: constant %[[PO]]
 
 // CHECK-LABEL: @_Z11test_signedii


Index: clang/test/CodeGenCXX/cxx2a-compare.cpp
===
--- clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -9,8 +9,6 @@
 // Ensure we don't emit definitions for the global variables
 // since the builtins shouldn't ODR use them.
 // CHECK-NOT: constant %[[SO]]
-// CHECK-NOT: constant %[[SE]]
-// CHECK-NOT: constant %[[WE]]
 // CHECK-NOT: constant %[[PO]]
 
 // CHECK-LABEL: @_Z11test_signedii
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] 4a47da2 - [Sema] turns -Wfree-nonheap-object on by default

2021-04-03 Thread David Blaikie via cfe-commits
Looks like this has a false positive (that's firing on some mlir code,
committed a workaround in  499571ea835daf786626a0db1e12f890b6cd8f8d )
like this:

$ cat test.cpp
#include 
void f1(int & x) { free(&x); }
int main() { f1(*(int*)malloc(sizeof(int))); }

Could you fix that? (& then revert the workaround)

On Fri, Jan 15, 2021 at 1:44 PM Christopher Di Bella via cfe-commits
 wrote:
>
>
> Author: Christopher Di Bella
> Date: 2021-01-15T21:38:47Z
> New Revision: 4a47da2cf440c2f2006d9b04acfef4292de1e263
>
> URL: 
> https://github.com/llvm/llvm-project/commit/4a47da2cf440c2f2006d9b04acfef4292de1e263
> DIFF: 
> https://github.com/llvm/llvm-project/commit/4a47da2cf440c2f2006d9b04acfef4292de1e263.diff
>
> LOG: [Sema] turns -Wfree-nonheap-object on by default
>
> We'd discussed adding the warning to -Wall in D89988. This patch honours that.
>
> Added:
>
>
> Modified:
> clang/include/clang/Basic/DiagnosticGroups.td
> clang/include/clang/Basic/DiagnosticSemaKinds.td
> clang/test/Analysis/NewDelete-intersections.mm
> clang/test/Analysis/free.c
>
> Removed:
>
>
>
> 
> diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
> b/clang/include/clang/Basic/DiagnosticGroups.td
> index d500ab321058..04ba89aa457e 100644
> --- a/clang/include/clang/Basic/DiagnosticGroups.td
> +++ b/clang/include/clang/Basic/DiagnosticGroups.td
> @@ -110,6 +110,7 @@ def FloatConversion :
>   FloatZeroConversion]>;
>
>  def FrameAddress : DiagGroup<"frame-address">;
> +def FreeNonHeapObject : DiagGroup<"free-nonheap-object">;
>  def DoublePromotion : DiagGroup<"double-promotion">;
>  def EnumTooLarge : DiagGroup<"enum-too-large">;
>  def UnsupportedNan : DiagGroup<"unsupported-nan">;
>
> diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
> b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> index 7d36397a7993..e93657898f58 100644
> --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -7609,7 +7609,7 @@ def err_no_typeid_with_fno_rtti : Error<
>  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 "
> +  "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 "
> @@ -7625,8 +7625,7 @@ def warn_condition_is_assignment : Warning<"using the 
> result of an "
>InGroup;
>  def warn_free_nonheap_object
>: Warning<"attempt to call %0 on non-heap object %1">,
> -InGroup>,
> -DefaultIgnore; // FIXME: add to -Wall after sufficient testing
> +InGroup;
>
>  // Completely identical except off by default.
>  def warn_condition_is_idiomatic_assignment : Warning<"using the result "
>
> diff  --git a/clang/test/Analysis/NewDelete-intersections.mm 
> b/clang/test/Analysis/NewDelete-intersections.mm
> index f01d62f8d365..6f81034ee349 100644
> --- a/clang/test/Analysis/NewDelete-intersections.mm
> +++ b/clang/test/Analysis/NewDelete-intersections.mm
> @@ -24,9 +24,6 @@
>  extern "C" void free(void *);
>
>  void testMallocFreeNoWarn() {
> -  int i;
> -  free(&i); // no warn
> -
>int *p1 = (int *)malloc(sizeof(int));
>free(++p1); // no warn
>
> @@ -51,7 +48,7 @@ void testDeleteMalloced() {
>
>int *p2 = (int *)__builtin_alloca(sizeof(int));
>delete p2; // no warn
> -}
> +}
>
>  void testUseZeroAllocatedMalloced() {
>int *p1 = (int *)malloc(0);
> @@ -79,13 +76,13 @@ void testObjcFreeNewed() {
>  }
>
>  void testFreeAfterDelete() {
> -  int *p = new int;
> +  int *p = new int;
>delete p;
>free(p); // newdelete-warning{{Use of memory after it is freed}}
>  }
>
>  void testStandardPlacementNewAfterDelete() {
> -  int *p = new int;
> +  int *p = new int;
>delete p;
>p = new (p) int; // newdelete-warning{{Use of memory after it is freed}}
>  }
>
> diff  --git a/clang/test/Analysis/free.c b/clang/test/Analysis/free.c
> index 0d29bacf274c..b50145713924 100644
> --- a/clang/test/Analysis/free.c
> +++ b/clang/test/Analysis/free.c
> @@ -12,17 +12,23 @@ void *alloca(size_t);
>
>  void t1 () {
>int a[] = { 1 };
> -  free(a); // expected-warning {{Argument to free() is the address of the 
> local variable 'a', which is not memory allocated by malloc()}}
> +  free(a);
> +  // expected-warning@-1{{Argument to free() is the address of the local 
> variable 'a', which is not memory allocated by malloc()}}
> +  // expected-warning@-2{{attempt to call free on non-heap object 'a'}}
>  }
>
>  void t2 () {
>int a = 1;
> -  free(&a); // expected-warning {{Argument to free() is the address of the 
> local variable 'a', which is not memory allocated by malloc()}}
> +  free(&a);
> +  // expected-warning@-1{

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

2021-04-03 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/test/SemaCXX/conversion-function.cpp:160
+  return "Hello"; // cxx98_14-error {{calling a private constructor}}
 #if __cplusplus <= 199711L
   // expected-warning@-2 {{an accessible copy constructor}}

mizvekov wrote:
> Quuxplusone wrote:
> > When is this condition true?
> > (Honestly I'm surprised that the `expected-warning` parser understands 
> > `#if` in the first place.)
> Too bad you can't also define macros for these expected declarations :)
From my reading of the 
[[https://github.com/llvm/llvm-project/blob/1cc9d949a1233e8b17b3b345ccb67ca7296c1a6c/clang/lib/Frontend/InitPreprocessor.cpp#L399|code]],
 only when compiling this source as not C++ (__cplusplus undefined).

And you made me realize I made a mistake there.

So remember this is called from -cc1, and when you don't pass "-std=c++" option 
to it, it looks like it does not actually set __cplusplus.

So yeah I unintentionally removed some test coverage there, which I will 
restore, thanks for catching this!

I will also look for if I have made any similar mistakes in other parts of this 
patch.



Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:41-45
 struct MoveOnly {
-  MoveOnly() {};
+  MoveOnly() = default;
   MoveOnly(const MoveOnly&) = delete;
-  MoveOnly(MoveOnly&&) noexcept {};
-  ~MoveOnly() {};
+  MoveOnly(MoveOnly &&) = default;
+};

Quuxplusone wrote:
> This change makes `is_trivial_v` true, where before it was false.
> I don't know whether this matters to the coroutine machinery. Ideally, 
> @aaronpuchert would weigh in; I don't know, and am just raising the issue 
> because it struck my attention.
> 
> (If it //does// matter, then maybe we even want to add test coverage for 
> //both// the move-only-trivial case and the move-only-nontrivial case.)
So these are the changes I picked from D68845.

You were a reviewer, and you did not raise an objection there :)

Though that is perfectly fine, that was a couple of years back and our 
perceptions just change I guess.

Though once I have P2266 implementation rebased on top of this, I can see if 
this change still makes sense in our context, but I'd also like for 
@aaronpuchert to weight on this.



Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:144
+
+template_return_task param2template(MoveOnly value) {
+  co_return value; // We should deduce U = MoveOnly.

Quuxplusone wrote:
> Nit: `return_template_task` or `generic_task` would look less confusingly 
> like `template return_task...` at first glance. :)
Like previous, you did not raise an objection on the original D68845 :)

But I agree with this change. But I'd like for @aaronpuchert to have a final 
say on this since these are his tests that I am merely taking, just so they do 
not sit there unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

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


[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

2021-04-03 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 335121.
mizvekov added a comment.

- Addresses most of Arthur's points.
- Re-adds test coverage for `conversion-function.cpp` and `temp.mem/p5.cpp` 
with no C++ version specified.
- Adds a couple more tests on nrvo-tracking for blocks returning references.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99225

Files:
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-1y.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-1y.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
  clang/test/CXX/special/class.copy/p3-cxx11.cpp
  clang/test/CXX/special/class.copy/p33-0x.cpp
  clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
  clang/test/CodeGen/nrvo-tracking.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutines.cpp
  clang/test/SemaCXX/cxx1y-deduced-return-type.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/return-stack-addr.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++20 -verify=cxx20 %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -verify=expected %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17 -fcxx-exceptions -Wreturn-std-move %s
+
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 
 // definitions for std::move
 namespace std {
@@ -76,8 +78,8 @@
 Base test2() {
 Derived d2;
 return d2;  // e1
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
@@ -87,22 +89,22 @@
 ConstructFromBase test4() {
 Derived d4;
 return d4;  // e3
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }
 ConvertFromDerived test5() {
 Derived d5;
 return d5;  // e4
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
+// cxx11_17-warning@-1{{will be copied despite being returned by name}}
+// cxx11_17-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
 }
 ConvertFromBase test6() {
 Derived d6;
 return d6;  // e5
-// expected-warning@-1{{will be copied despite being returned by name}}
-// expected-note@-2{{to avoid 

[PATCH] D99696: [clang] WIP: NRVO: Improvements and handling of more cases.

2021-04-03 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 335122.
mizvekov added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/nrvo-tracking.cpp

Index: clang/test/CodeGen/nrvo-tracking.cpp
===
--- clang/test/CodeGen/nrvo-tracking.cpp
+++ clang/test/CodeGen/nrvo-tracking.cpp
@@ -29,8 +29,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(3, t, T);
@@ -45,8 +43,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(5, t, auto);
@@ -61,8 +57,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(7, t, decltype(auto));
@@ -113,8 +107,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(5, t, auto);
@@ -129,8 +121,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(7, t, decltype(auto));
@@ -152,7 +142,11 @@
   }; }()();\
 }
 
-//B(1, X); // Uncomment this line at your own peril ;)
+// CHECK-LABEL: define{{.*}} void @"___ZZ2b1v
+// CHECK:   call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call void @llvm.lifetime.end
+// CHECK-NEXT:  ret void
+B(1, X);
 
 // CHECK-LABEL: define{{.*}} void @_Z2b2v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
@@ -164,8 +158,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(3, T);
@@ -180,8 +172,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(5, );
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1052,9 +1052,18 @@
  StartingScope, InstantiatingVarTemplate);
 
   if (D->isNRVOVariable()) {
-QualType ReturnType = cast(DC)->getReturnType();
-if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
-  Var->setNRVOVariable(true);
+QualType ReturnType;
+if (auto *F = dyn_cast(DC))
+  ReturnType = F->getReturnType();
+else if (auto *F = dyn_cast(DC))
+  // FIXME: get the return type here somehow...
+  ReturnType = SemaRef.Context.getAutoDeductType();
+else
+  assert(false && "Unknown context type");
+
+Sema::NRVOResult Res = SemaRef.getNRVOResult(Var);
+SemaRef.updNRVOResultWithRetType(Res, ReturnType);
+Var->setNRVOVariable(Res.isCopyElidable());
   }
 
   Var->setImplicit(D->isImplicit());
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3026,99 +3026,157 @@
   return new (Context) BreakStmt(BreakLoc);
 }
 
-/// Determine whether the given expression is a candidate for
-/// copy elision in either a return statement or a throw expression.
+static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) {
+  Res.S = Res.S != Sema::NRVOResult::None && CanMove
+  ? Sema::NRVOResult::MoveEligible
+  : Sema::NRVOResult::None;
+}
+
+/// Determine whether the given NRVO candidate variable is move-eligible or
+/// copy-elidable, without considering function return type.
 ///
-/// \param ReturnType If we're determining the copy elision candidate for
-/// a return statement, this is the return type of the function. If we're
-/// determining the copy elision candidate for a t

[PATCH] D99696: [clang] WIP: NRVO: Improvements and handling of more cases.

2021-04-03 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 335124.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/nrvo-tracking.cpp

Index: clang/test/CodeGen/nrvo-tracking.cpp
===
--- clang/test/CodeGen/nrvo-tracking.cpp
+++ clang/test/CodeGen/nrvo-tracking.cpp
@@ -29,8 +29,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(3, t, T);
@@ -45,8 +43,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(5, t, auto);
@@ -61,8 +57,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2l7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(7, t, decltype(auto));
@@ -113,8 +107,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(5, t, auto);
@@ -129,8 +121,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2f7v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 F(7, t, decltype(auto));
@@ -152,7 +142,11 @@
   }; }()();\
 }
 
-//B(1, X); // Uncomment this line at your own peril ;)
+// CHECK-LABEL: define{{.*}} void @_Z2b1v
+// CHECK:   call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call void @llvm.lifetime.end
+// CHECK-NEXT:  ret void
+B(1, X);
 
 // CHECK-LABEL: define{{.*}} void @_Z2b2v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
@@ -164,8 +158,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b3v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(3, T);
@@ -180,8 +172,6 @@
 
 // CHECK-LABEL: define{{.*}} void @_Z2b5v
 // CHECK:   call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
-// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(5, );
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1052,9 +1052,18 @@
  StartingScope, InstantiatingVarTemplate);
 
   if (D->isNRVOVariable()) {
-QualType ReturnType = cast(DC)->getReturnType();
-if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
-  Var->setNRVOVariable(true);
+QualType ReturnType;
+if (auto *F = dyn_cast(DC))
+  ReturnType = F->getReturnType();
+else if (auto *F = dyn_cast(DC))
+  // FIXME: get the return type here somehow...
+  ReturnType = SemaRef.Context.getAutoDeductType();
+else
+  assert(false && "Unknown context type");
+
+Sema::NRVOResult Res = SemaRef.getNRVOResult(Var);
+SemaRef.updNRVOResultWithRetType(Res, ReturnType);
+Var->setNRVOVariable(Res.isCopyElidable());
   }
 
   Var->setImplicit(D->isImplicit());
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3026,99 +3026,157 @@
   return new (Context) BreakStmt(BreakLoc);
 }
 
-/// Determine whether the given expression is a candidate for
-/// copy elision in either a return statement or a throw expression.
+static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) {
+  Res.S = Res.S != Sema::NRVOResult::None && CanMove
+  ? Sema::NRVOResult::MoveEligible
+  : Sema::NRVOResult::None;
+}
+
+/// Determine whether the given NRVO candidate variable is move-eligible or
+/// copy-elidable, without considering function return type.
 ///
-/// \param ReturnType If we're determining the copy elision candidate for
-/// a return statement, this is the return type of the function. If we're
-/// determining the copy elision candidate for a throw expres

[PATCH] D99852: [Driver] Detect libstdc++ include paths for native gcc (-m32 and -m64) on Debian i386

2021-04-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added a reviewer: sylvestre.ledru.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Take gcc-8 on Debian i386 as an example. The target-specific libstdc++ search
path (`GPLUSPLUS_TOOL_INCLUDE_DIR`) uses the multiarch name `i386-linux-gnu`,
instead of the triple of the GCC installation `i686-linux-gnu` (the directory
under `usr/lib/gcc/`):

  /usr/include/c++/8
  /usr/include/i386-linux-gnu/c++/8
  /usr/include/c++/8/backward

Clang currently detects 
`/usr/lib/gcc/i686-linux-gnu/8/../../../include/i686-linux-gnu/c++/8`.
This patch changes the second i686-linux-gnu to i386-linux-gnu so that
`/usr/include/i386-linux-gnu/c++/8` can be found.

Fix PR49827 - this was somehow regressed by my previous libstdc++ include path
cleanups and fixes for gcc-cross, but it seems that the paths were never 
properly tested before.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99852

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/Inputs/debian_i386_tree/lib/i386-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_i386_tree/lib/x86_64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_i386_tree/lib64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/include/c++/10/backward/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/include/i386-linux-gnu/c++/10/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/include/i386-linux-gnu/c++/10/64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/include/x86_64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/lib/gcc/i686-linux-gnu/10/64/crtbegin.o
  
clang/test/Driver/Inputs/debian_i386_tree/usr/lib/gcc/i686-linux-gnu/10/crtbegin.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crt1.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crti.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crtn.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crt1.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crti.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crtn.o
  clang/test/Driver/linux-cross.cpp


Index: clang/test/Driver/linux-cross.cpp
===
--- clang/test/Driver/linux-cross.cpp
+++ clang/test/Driver/linux-cross.cpp
@@ -49,6 +49,55 @@
 // DEBIAN_X86_64_M32-SAME: {{^}} "-L[[SYSROOT]]/lib"
 // DEBIAN_X86_64_M32-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
 
+/// Test native GCC installation on Debian i386.
+// RUN: %clang -### %s --target=i686-linux-gnu 
--sysroot=%S/Inputs/debian_i386_tree \
+// RUN:   -resource-dir=%S/Inputs/resource_dir --stdlib=platform 
--rtlib=platform 2>&1 | FileCheck %s --check-prefix=DEBIAN_I686
+// DEBIAN_I686:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// DEBIAN_I686:  "-internal-isystem"
+// DEBIAN_I686-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10"
+/// Debian specific - the path component after 'include' is i386-linux-gnu even
+/// though the installation is i686-linux-gnu.
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/i386-linux-gnu/c++/10"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10/backward"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+/// This resolves to /usr/i686-linux-gnu/include. Because it does not exist,
+/// having it does no harm albeit not ideal.
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../i686-linux-gnu/include"
+// DEBIAN_I686:  "-internal-externc-isystem"
+// DEBIAN_I686-SAME: {{^}} "[[SYSROOT]]/usr/include/i386-linux-gnu"
+// DEBIAN_I686:  "-L
+// DEBIAN_I686-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-linux-gnu/10"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/lib/i386-linux-gnu"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/i386-linux-gnu"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+/// Test -m64 on Debian i386.
+// RUN: %clang -### %s --target=i686-linux-gnu 
--sysroot=%S/Inputs/debian_i386_tree -m64 \
+// RUN:   -resource-dir=%S/Inputs/resource_dir --stdlib=platform 
--rtlib=platform 2>&1 | FileCheck %s --check-prefix=DEBIAN_I686_M64
+// DEBIAN_I686_M64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// DEBIAN_I686_M64:  "-internal-isystem"
+// DEBIAN_I686_M64-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10"
+/// Debian specific - the path component after 'include' is i386-linux-gnu even
+/// though the installation is i686-linux-gnu.
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/

[PATCH] D99852: [Driver] Detect libstdc++ include paths for native gcc (-m32 and -m64) on Debian i386

2021-04-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 335127.
MaskRay added a comment.

`git add` two .keep files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99852

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/Inputs/debian_i386_tree/lib/i386-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_i386_tree/lib/x86_64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_i386_tree/lib64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/include/c++/10/backward/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/include/i386-linux-gnu/c++/10/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/include/i386-linux-gnu/c++/10/64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/include/x86_64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_i386_tree/usr/lib/gcc/i686-linux-gnu/10/64/crtbegin.o
  
clang/test/Driver/Inputs/debian_i386_tree/usr/lib/gcc/i686-linux-gnu/10/crtbegin.o
  
clang/test/Driver/Inputs/debian_i386_tree/usr/lib/gcc/i686-linux-gnu/10/crtend.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crt1.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crti.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/i386-linux-gnu/crtn.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib/x86_64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/.keep
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crt1.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crti.o
  clang/test/Driver/Inputs/debian_i386_tree/usr/lib64/crtn.o
  clang/test/Driver/linux-cross.cpp


Index: clang/test/Driver/linux-cross.cpp
===
--- clang/test/Driver/linux-cross.cpp
+++ clang/test/Driver/linux-cross.cpp
@@ -49,6 +49,55 @@
 // DEBIAN_X86_64_M32-SAME: {{^}} "-L[[SYSROOT]]/lib"
 // DEBIAN_X86_64_M32-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
 
+/// Test native GCC installation on Debian i386.
+// RUN: %clang -### %s --target=i686-linux-gnu 
--sysroot=%S/Inputs/debian_i386_tree \
+// RUN:   -resource-dir=%S/Inputs/resource_dir --stdlib=platform 
--rtlib=platform 2>&1 | FileCheck %s --check-prefix=DEBIAN_I686
+// DEBIAN_I686:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// DEBIAN_I686:  "-internal-isystem"
+// DEBIAN_I686-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10"
+/// Debian specific - the path component after 'include' is i386-linux-gnu even
+/// though the installation is i686-linux-gnu.
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/i386-linux-gnu/c++/10"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10/backward"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+/// This resolves to /usr/i686-linux-gnu/include. Because it does not exist,
+/// having it does no harm albeit not ideal.
+// DEBIAN_I686-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../i686-linux-gnu/include"
+// DEBIAN_I686:  "-internal-externc-isystem"
+// DEBIAN_I686-SAME: {{^}} "[[SYSROOT]]/usr/include/i386-linux-gnu"
+// DEBIAN_I686:  "-L
+// DEBIAN_I686-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-linux-gnu/10"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/lib/i386-linux-gnu"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/i386-linux-gnu"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// DEBIAN_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+/// Test -m64 on Debian i386.
+// RUN: %clang -### %s --target=i686-linux-gnu 
--sysroot=%S/Inputs/debian_i386_tree -m64 \
+// RUN:   -resource-dir=%S/Inputs/resource_dir --stdlib=platform 
--rtlib=platform 2>&1 | FileCheck %s --check-prefix=DEBIAN_I686_M64
+// DEBIAN_I686_M64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// DEBIAN_I686_M64:  "-internal-isystem"
+// DEBIAN_I686_M64-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10"
+/// Debian specific - the path component after 'include' is i386-linux-gnu even
+/// though the installation is i686-linux-gnu.
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/i386-linux-gnu/c++/10/64"
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../include/c++/10/backward"
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT]]/usr/local/include"
+// DEBIAN_I686_M64-SAME: {{^}} "-internal-isystem" 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-linux-gnu/10/../../../../i686-linux-gnu/include"
+// DEBIAN_I686_M64:  "-internal-externc-isystem"
+// DEBIAN_I686_M64-SAME: {{^}} "[[SYSROOT]]/usr/inc

[clang] 1b4800c - [clang][parser] Set source ranges for GNU-style attributes

2021-04-03 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2021-04-04T07:59:22+02:00
New Revision: 1b4800c2625912d16867d27e5eec3af27af31557

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

LOG: [clang][parser] Set source ranges for GNU-style attributes

Set the source ranges for parsed GNU-style attributes in
ParseGNUAttributes(), the same way that ParseCXX11Attributes() does it.

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

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/test/AST/sourceranges.cpp
clang/test/Parser/cxx0x-attributes.cpp
clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index b67e541836d5..6f6d4697e6d0 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1572,27 +1572,6 @@ class Parser : public CodeCompletionHandler {
 
   
//======//
   // C99 6.9: External Definitions.
-  struct ParsedAttributesWithRange : ParsedAttributes {
-ParsedAttributesWithRange(AttributeFactory &factory)
-  : ParsedAttributes(factory) {}
-
-void clear() {
-  ParsedAttributes::clear();
-  Range = SourceRange();
-}
-
-SourceRange Range;
-  };
-  struct ParsedAttributesViewWithRange : ParsedAttributesView {
-ParsedAttributesViewWithRange() : ParsedAttributesView() {}
-void clearListOnly() {
-  ParsedAttributesView::clearListOnly();
-  Range = SourceRange();
-}
-
-SourceRange Range;
-  };
-
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
   ParsingDeclSpec *DS = nullptr);
   bool isDeclarationAfterDeclarator();
@@ -2725,17 +2704,50 @@ class Parser : public CodeCompletionHandler {
   D.takeAttributes(attrs, endLoc);
 }
   }
-  bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
-   SourceLocation *endLoc = nullptr,
+
+  /// Parses GNU-style attributes and returns them without source range
+  /// information.
+  ///
+  /// This API is discouraged. Use the version that takes a
+  /// ParsedAttributesWithRange instead.
+  bool MaybeParseGNUAttributes(ParsedAttributes &Attrs,
+   SourceLocation *EndLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
-  ParseGNUAttributes(attrs, endLoc, LateAttrs);
+  ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  Attrs.takeAllFrom(AttrsWithRange);
   return true;
 }
 return false;
   }
-  void ParseGNUAttributes(ParsedAttributes &attrs,
-  SourceLocation *endLoc = nullptr,
+
+  bool MaybeParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+   SourceLocation *EndLoc = nullptr,
+   LateParsedAttrList *LateAttrs = nullptr) {
+if (Tok.is(tok::kw___attribute)) {
+  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  return true;
+}
+return false;
+  }
+
+  /// Parses GNU-style attributes and returns them without source range
+  /// information.
+  ///
+  /// This API is discouraged. Use the version that takes a
+  /// ParsedAttributesWithRange instead.
+  void ParseGNUAttributes(ParsedAttributes &Attrs,
+  SourceLocation *EndLoc = nullptr,
+  LateParsedAttrList *LateAttrs = nullptr,
+  Declarator *D = nullptr) {
+ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+ParseGNUAttributes(AttrsWithRange, EndLoc, LateAttrs, D);
+Attrs.takeAllFrom(AttrsWithRange);
+  }
+
+  void ParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+  SourceLocation *EndLoc = nullptr,
   LateParsedAttrList *LateAttrs = nullptr,
   Declarator *D = nullptr);
   void ParseGNUAttributeArgs(IdentifierInfo *AttrName,

diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 9cbee28d612a..471a1ecfac9b 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1048,6 +1048,27 @@ class ParsedAttributes : public ParsedAttributesView {
   mutable AttributePool pool;
 };
 
+struct ParsedAttributesWithRange : ParsedAttributes {
+  ParsedAttributesWithRange(AttributeFactory &factory)
+  : ParsedAttributes(factory) {}
+
+  void clear() {
+ParsedAttributes::clear();
+Range = SourceRange();
+  }
+