[PATCH] D70157: Align branches within 32-Byte boundary

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

In D70157#1771771 , @reames wrote:

> We uncovered another functional issue with this patch, or at least, the 
> interaction of this patch and other parts of LLVM.  In our support for 
> STATEPOINT, PATCHPOINT, and STACKMAP we use N-byte nop sequences for regions 
> of code which might be patched out.  It's important that these regions are 
> exactly N bytes as concurrent patching which doesn't replace an integral 
> number of instructions is ill-defined on X86-64.  This patch causes the 
> N-byte nop sequence to sometimes become (N+M) bytes which breaks the 
> patching.  I believe that the XRAY support may have a similar issue.
>
> More generally, I'm worried about the legality of arbitrarily prefixing 
> instructions from unknown sources.  In the particular example we saw, we had 
> something along the following:
>
> .Ltmp0:
>
>   .p2align3, 0x90
>   (16 byte nop sequence)
>
> .Ltmp3:
>
>   jmp *%rax
>
>
> In addition to the patching legality issue above, padding the nop sequence 
> does something else interesting in this example.  It changes the alignment of 
> Ltmp3.  Before, Ltmp3 was always 8 byte aligned, after prefixes are added, 
> it's not.  It's not clear to me exactly what the required semantics here are, 
> but we at least had been assuming the alignment of Ltmp3 was guaranteed in 
> this case.  (That's actually how we found the patching issue.)


I could not reproduce the phenomenon that N-byte nop becomes (N+M) bytes with 
your example. So according to my understanding, I slightly modified your case. 
(If my understand is wrong, I hope you can point it out :-). )

  .text
  nop
  .Ltmp0:
  .p2align 3, 0x90
  .rept 16
  nop
  .endr
  .Ltmp3:
  movl  %eax, -4(%rsp)
  .rept 2
  nop
  .endr
  jmp .Ltmp0

The instruction `jmp .Ltmp0` starts at byte 0x1e and ends at byte 0x20. If we 
align the jump with preifx, two prefixes will be added to the `.rept2 16 nop 
.endr`. After prefixes are added, the 16-byte nop becomes 18-byte nop, then the 
label '.Ltmp3' is not 8-byte aligned any more.

I doubt whether the assumption that '.Ltmp3' is 8-byte aligned is right, since 
the alignment is not explicitly required.  For example, I think we can not 
assuming the instruction `jmp .Ltmp0` always starts at byte 0x1e. And in fact, 
as long as the binary generated by the compiler does not have a one-to-one 
correspondence with the assembly code, at least one implict assumption of an 
instruction will be broken.


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

https://reviews.llvm.org/D70157



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

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

[Csaba Dabis via Phabricator via cfe-commits]

Charusso closed this revision.
Charusso added a comment.

Forgotten `amend` of commit message. Commited as:

https://github.com/llvm/llvm-project/commit/8d288a0668a574863d52784084ff565c89f7366e


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71179#1774487 , @jdoerfert wrote:

> In D71179#1774471 , @ABataev wrote:
>
> > They do this because they have several function definitions with the same 
> > name. In our case, we have several different functions with different names 
> > and for us no need to worry about overloading resolution, the compiler will 
> > do everything for us.
>
>
> I think we talk past each other again. This is the implementation of `omp 
> begin/end declare variant` as described in TR8. Bt definition, the new 
> variant mechanism will result in several different function definitions with 
> the same name. See the two tests for examples.


The intent of this feature is to allow us to include the device-function 
headers and the system headers simultaneously, giving preference to the device 
functions when compiling for the device, thus fixing a number of outstanding 
math.h OpenMP offloading problems. This definitely means that we'll have 
multiple functions with the same name and we need to pick the right ones during 
overload resolution.

@jdoerfert , how does the ".ompvariant" work with external functions? I see the 
part of the spec which says, "The symbol name of a function definition that 
appears between a begin declare variant...", but, if we append this name to, 
for example, the names of functions present in the device math library, won't 
we have a problem with linking?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

[Csaba Dabis via Phabricator via cfe-commits]

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

I think it is fine, but please let us wait with the final thoughts from 
@aaron.ballman.




Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:21
+static constexpr llvm::StringLiteral MutatingOperatorName = "MutatingOp";
+static constexpr llvm::StringLiteral MutatingCallName = "MutatingCall";
+

What about just "assignment" and "member-call"?



Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:63
+  anyOf(forEachDescendant(IsSourceMutatingAssignment),
+forEachDescendant(IsSourceMutatingMemberCall)));
+

I think `hasDescendant()` is more appropriate compared to the slower 
`forEachDescendant()`.


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

https://reviews.llvm.org/D70052



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

[clang] cafc741 - [c++20] Synthesis of defaulted comparison functions.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-08T23:21:52-08:00
New Revision: cafc7416baf7eecef8ecaf05802f2f7c0da725c0

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

LOG: [c++20] Synthesis of defaulted comparison functions.

Array members are not yet handled. In addition, defaulted comparisons
can't yet find comparison operators by unqualified lookup (only by
member lookup and ADL). These issues will be fixed in follow-on changes.

Added: 
clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
clang/test/CXX/class/class.compare/class.eq/p3.cpp
clang/test/CXX/class/class.compare/class.spaceship/p3.cpp

Modified: 
clang/include/clang/AST/Decl.h
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
clang/test/CXX/class/class.compare/class.eq/p2.cpp
clang/test/CXX/class/class.compare/class.rel/p2.cpp
clang/test/CXX/class/class.compare/class.spaceship/p1.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index f4913540bab4..2d1a30657cbd 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2115,6 +2115,16 @@ class FunctionDecl : public DeclaratorDecl,
 FunctionDeclBits.IsExplicitlyDefaulted = ED;
   }
 
+  /// True if this method is user-declared and was not
+  /// deleted or defaulted on its first declaration.
+  bool isUserProvided() const {
+auto *DeclAsWritten = this;
+if (FunctionDecl *Pattern = getTemplateInstantiationPattern())
+  DeclAsWritten = Pattern;
+return !(DeclAsWritten->isDeleted() ||
+ DeclAsWritten->getCanonicalDecl()->isDefaulted());
+  }
+
   /// Whether falling off this function implicitly returns null/zero.
   /// If a more specific implicit return value is required, front-ends
   /// should synthesize the appropriate return statements.

diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 0f2018fb9e8c..0043ce1a92dd 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1995,16 +1995,6 @@ class CXXMethodDecl : public FunctionDecl {
 return const_cast(this)->getMostRecentDecl();
   }
 
-  /// True if this method is user-declared and was not
-  /// deleted or defaulted on its first declaration.
-  bool isUserProvided() const {
-auto *DeclAsWritten = this;
-if (auto *Pattern = getTemplateInstantiationPattern())
-  DeclAsWritten = cast(Pattern);
-return !(DeclAsWritten->isDeleted() ||
- DeclAsWritten->getCanonicalDecl()->isDefaulted());
-  }
-
   void addOverriddenMethod(const CXXMethodDecl *MD);
 
   using method_iterator = const CXXMethodDecl *const *;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index dcfc8fb5de9c..939287014d9e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1732,7 +1732,10 @@ def note_ivar_decl : Note<"instance variable is declared 
here">;
 def note_bitfield_decl : Note<"bit-field is declared here">;
 def note_implicit_param_decl : Note<"%0 is an implicit parameter">;
 def note_member_synthesized_at : Note<
-  "in implicit %sub{select_special_member_kind}0 for %1 "
+  "in %select{implicit|defaulted}0 %sub{select_special_member_kind}1 for %2 "
+  "first required here">;
+def note_comparison_synthesized_at : Note<
+  "in defaulted %sub{select_defaulted_comparison_kind}0 for %1 "
   "first required here">;
 def err_missing_default_ctor : Error<
   "%select{constructor for %1 must explicitly initialize the|"

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ed1f1370b332..2b0db07e6bac 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3319,7 +3319,12 @@ class Sema final {
const UnresolvedSetImpl ,
Expr *LHS, Expr *RHS,
bool RequiresADL = true,
-   bool AllowRewrittenCandidates = true);
+   bool AllowRewrittenCandidates = true,
+   FunctionDecl *DefaultedFn = nullptr);
+  ExprResult BuildSynthesizedThreeWayComparison(SourceLocation OpLoc,
+const UnresolvedSetImpl ,
+Expr *LHS, Expr *RHS,

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h:17
 
 #if defined(__NVPTX__) && defined(_OPENMP)
 

Should we use a more-specific selector and then get rid of this `__NVPTX__` 
check?



Comment at: clang/lib/Parse/ParseOpenMP.cpp:1489
+++Nesting;
+} while (Nesting);
+

Will this just inf-loop if the file ends?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[clang] 27f5d35 - Fix for build bot failure. For more details see:

[Zahira Ammarguellat via cfe-commits]

Author: Zahira Ammarguellat
Date: 2019-12-09T00:50:30-05:00
New Revision: 27f5d35137cb45e60d9988a9b55875883c55023c

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

LOG: Fix for build bot failure. For more details see:
https://reviews.llvm.org/D70691
Upated LIT test.

Added: 


Modified: 
clang/test/CodeGen/opt-record-1.c

Removed: 




diff  --git a/clang/test/CodeGen/opt-record-1.c 
b/clang/test/CodeGen/opt-record-1.c
index 3f37e32d5390..00a753d93dff 100644
--- a/clang/test/CodeGen/opt-record-1.c
+++ b/clang/test/CodeGen/opt-record-1.c
@@ -1,12 +1,12 @@
-// RUN: %clang_cc1 %s -opt-record-file=t1.opt -fopenmp -emit-llvm-bc -o %t.bc
-// RUN: %clang_cc1 -x ir %t.bc -opt-record-file %t.opt -fopenmp -emit-obj
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-cpu x86-64  %s -O3 
-opt-record-file=t1.opt -fopenmp -emit-llvm-bc -o %t.bc
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-cpu x86-64 -O3 -x 
ir %t.bc -opt-record-file %t.opt -fopenmp -emit-obj
 // RUN: cat %t.opt |  FileCheck -check-prefix=CHECK  %s
 
 void foo(int *a, int *b, int *c) {
 #pragma omp parallel for
- for (int i = 0; i < 100; i++) {
- a[i] = b[i] + c[i];
- }
+  for (int i = 0; i < 100; i++) {
+a[i] = b[i] + c[i];
+  }
 }
 
 // CHECK: --- !Missed



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

[PATCH] D70615: Add an -fno-temp-file flag for compilation

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

In our build system every file access in an "output directory" needs to be 
accounted for.  Until this patch, the random temporary file name has forced us 
to rely on workarounds that hurt build throughput.

I've uploaded example ProcMon dumps of clang-cl.exe file accesses to help 
illustrate what the patch is trying to achieve.  In the RTM log, clang-cl 
creates a file ConsoleApplication1-dc91d1ce.obj.tmp directly in the output 
directory, writes the contents and ends by calling SetRenameInformationFile to 
"create" the final ConsoleApplication1.obj.  In the patched log the file 
ConsoleApplication1.obj is created and written to directly.

F11003490: clang9.0-RTM.CSV 
F11003493: clang9.0-patched.CSV 


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

https://reviews.llvm.org/D70615



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71179#1774471 , @ABataev wrote:

> They do this because they have several function definitions with the same 
> name. In our case, we have several different functions with different names 
> and for us no need to worry about overloading resolution, the compiler will 
> do everything for us.


I think we talk past each other again. This is the implementation of `omp 
begin/end declare variant` as described in TR8. Bt definition, the new variant 
mechanism will result in several different function definitions with the same 
name. See the two tests for examples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1774470 , @jdoerfert wrote:

> In D71179#1774469 , @ABataev wrote:
>
> > In D71179#1774448 , @jdoerfert 
> > wrote:
> >
> > > In D71179#177 , @ABataev 
> > > wrote:
> > >
> > > > I read the spec and don't think that we need all this complex stuff for 
> > > > the implementation. Yiu need judt to check at the codegen phase if the 
> > > > function must be emitted or not. We don't even need to move context 
> > > > checksnfrom codegen, because with the current semantics all the 
> > > > checkscan be safely performed at the codegen phase.
> > >
> > >
> > > For better or worse we need this and it is actually a natural reuse of 
> > > the multi-versioning code. We need this because:
> > >
> > > 1. For the begin/end version we cannot even parse anything in a context 
> > > that does not match at encounter time, e.g. the `kind(fpga)` context in 
> > > `clang/test/AST/ast-dump-openmp-begin-declare-variant.c`.
> >
> >
> > Ok, here we can check the context and just skip everything between 
> > begin/end pragmas just like if something like #ifdef...#endif is seen.
>
>
> Agreed.
>
> >> 2. For the 5.0 version we cannot use the `replaceAllUses` approach 
> >> currently implemented in `tryEmitDeclareVariant` as soon as we have the 
> >> construct context selector trait. That means we will have to resolve the 
> >> call target earlier anyway.
> > 
> > I thought about this. Here we need to use a little bit different method, 
> > but again everything can be reolved at the codegen phase, no need to 
> > resolve it at parsing/sema.
>
> I doubt we can, yet alone want to do (basically) overload resolution during 
> codegen.
>  Depending on what function we resolve to, we get different instantiations 
> which require everything from semantic analysis to run on that code again, 
> right?
>  Could you elaborate why we should not do all the overload resolution at the 
> same time and with the same mechanism that is already present? I mean, 
> SemaOverload deals with multi-versioning already.


They do this because they have several function definitions with the same name. 
In our case, we have several different functions with different names and for 
us no need to worry about overloading resolution, the compiler will do 
everything for us.

> 
> 
>> Plus, this is completely different problem and must be solved in the 
>> different patch.
> 
> As I noted in the very beginning of the commit message, this is not supposed 
> to be a commited like this but split into multiple patches. Let's not mix 
> discussions here.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71179#1774469 , @ABataev wrote:

> In D71179#1774448 , @jdoerfert wrote:
>
> > In D71179#177 , @ABataev wrote:
> >
> > > I read the spec and don't think that we need all this complex stuff for 
> > > the implementation. Yiu need judt to check at the codegen phase if the 
> > > function must be emitted or not. We don't even need to move context 
> > > checksnfrom codegen, because with the current semantics all the checkscan 
> > > be safely performed at the codegen phase.
> >
> >
> > For better or worse we need this and it is actually a natural reuse of the 
> > multi-versioning code. We need this because:
> >
> > 1. For the begin/end version we cannot even parse anything in a context 
> > that does not match at encounter time, e.g. the `kind(fpga)` context in 
> > `clang/test/AST/ast-dump-openmp-begin-declare-variant.c`.
>
>
> Ok, here we can check the context and just skip everything between begin/end 
> pragmas just like if something like #ifdef...#endif is seen.


Agreed.

>> 2. For the 5.0 version we cannot use the `replaceAllUses` approach currently 
>> implemented in `tryEmitDeclareVariant` as soon as we have the construct 
>> context selector trait. That means we will have to resolve the call target 
>> earlier anyway.
> 
> I thought about this. Here we need to use a little bit different method, but 
> again everything can be reolved at the codegen phase, no need to resolve it 
> at parsing/sema.

I doubt we can, yet alone want to do (basically) overload resolution during 
codegen.
Depending on what function we resolve to, we get different instantiations which 
require everything from semantic analysis to run on that code again, right?
Could you elaborate why we should not do all the overload resolution at the 
same time and with the same mechanism that is already present? I mean, 
SemaOverload deals with multi-versioning already.

> Plus, this is completely different problem and must be solved in the 
> different patch.

As I noted in the very beginning of the commit message, this is not supposed to 
be a commited like this but split into multiple patches. Let's not mix 
discussions here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1774448 , @jdoerfert wrote:

> In D71179#177 , @ABataev wrote:
>
> > I read the spec and don't think that we need all this complex stuff for the 
> > implementation. Yiu need judt to check at the codegen phase if the function 
> > must be emitted or not. We don't even need to move context checksnfrom 
> > codegen, because with the current semantics all the checkscan be safely 
> > performed at the codegen phase.
>
>
> For better or worse we need this and it is actually a natural reuse of the 
> multi-versioning code. We need this because:
>
> 1. For the begin/end version we cannot even parse anything in a context that 
> does not match at encounter time, e.g. the `kind(fpga)` context in 
> `clang/test/AST/ast-dump-openmp-begin-declare-variant.c`.


Ok, here we can check the context and just skip everything between begin/end 
pragmas just like if something like #ifdef...#endif is seen.

> 2. For the 5.0 version we cannot use the `replaceAllUses` approach currently 
> implemented in `tryEmitDeclareVariant` as soon as we have the construct 
> context selector trait. That means we will have to resolve the call target 
> earlier anyway.

I thought about this. Here we need to use a little bit different method, but 
again everything can be reolved at the codegen phase, no need to resolve it at 
parsing/sema. Plus, this is completely different problem and must be solved in 
the different patch.

> (FWIW, I wrote this part of the SPEC.)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D64034: [c++] Implement categorizing pointer-to-bool as narrowing conversions

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 232752.
lichray added a comment.

Regenerate the diff in Git, update description, link to paper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64034

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-cxx11-nowarn.cpp

Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-cxx11-nowarn.cpp
===
--- clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-cxx11-nowarn.cpp
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-cxx11-nowarn.cpp
@@ -20,6 +20,7 @@
   int ii = {2.0};  // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
   float f1 { x };  // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
   float f2 { 7 };  // OK: 7 can be exactly represented as a float
+  bool b = {"meow"};  // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
   int f(int);
   int a[] =
 { 2, f(2), f(2.0) };  // OK: the double-to-int conversion is not at the top level
@@ -131,7 +132,7 @@
 //   cannot represent all the values of the original type, except where the
 //   source is a constant expression and the actual value after conversion will
 //   fit into the target type and will produce the original value when converted
-//   back to the original type.
+//   back to the original type, or
 void shrink_int() {
   // Not a constant expression.
   short s = 1;
@@ -163,14 +164,24 @@
   Agg b2 = {1};  // OK
   Agg b3 = {-1};  // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
 
-  // Conversions from pointers to booleans aren't narrowing conversions.
-  Agg* ptr = 
-  Agg b = {ptr};  // OK
-
   Agg ce1 = { Convert(10) }; // expected-warning {{constant expression evaluates to 10 which cannot be narrowed to type 'short'}} expected-note {{silence}} expected-warning {{changes value from 10 to -31072}}
   Agg ce2 = { ConvertVar() }; // expected-warning {{non-constant-expression cannot be narrowed from type 'short' to 'char'}} expected-note {{silence}}
 }
 
+// * from a pointer type or a pointer-to-member type to bool.
+void pointer_to_bool() {
+  Agg obj;
+  Agg *p1 = 
+  constexpr void *p2 = nullptr;
+
+  Agg b1 = {p1};   // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
+  Agg b2 = {p2};   // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
+  Agg b3 = {::t}; // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
+
+  Agg b4 = {ConvertVar()};   // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
+  Agg b5 = {ConvertVar::*>()}; // expected-warning {{ cannot be narrowed }} expected-note {{silence}}
+}
+
 // Be sure that type- and value-dependent expressions in templates get the warning
 // too.
 
Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -19,6 +19,7 @@
   int ii = {2.0};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
   float f1 { x };  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
   float f2 { 7 };  // OK: 7 can be exactly represented as a float
+  bool b = {"meow"};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
   int f(int);
   int a[] =
 { 2, f(2), f(2.0) };  // OK: the double-to-int conversion is not at the top level
@@ -143,7 +144,7 @@
 //   cannot represent all the values of the original type, except where the
 //   source is a constant expression and the actual value after conversion will
 //   fit into the target type and will produce the original value when converted
-//   back to the original type.
+//   back to the original type, or
 void shrink_int() {
   // Not a constant expression.
   short s = 1;
@@ -180,10 +181,6 @@
   Agg b2 = {1};  // OK
   Agg b3 = {-1};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
 
-  // Conversions from pointers to booleans aren't narrowing conversions.
-  Agg* ptr = 
-  Agg b = {ptr};  // OK
-
   Agg ce1 = { Convert(10) }; // expected-error {{constant expression evaluates to 10 which cannot be narrowed to type 'short'}} expected-note {{silence}} expected-warning {{changes value from 10 to -31072}}
   Agg ce2 = { ConvertVar() }; // expected-error {{non-constant-expression cannot be narrowed from type 'short' to 'char'}} expected-note {{silence}}
 
@@ -202,6 +199,20 @@
   unsigned short usc3 = { (signed char)-1 }; // expected-error {{ -1 which cannot be narrowed}} expected-note {{silence}}
 }
 
+// * from a pointer type or a pointer-to-member type to bool.
+void 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71179#177 , @ABataev wrote:

> I read the spec and don't think that we need all this complex stuff for the 
> implementation. Yiu need judt to check at the codegen phase if the function 
> must be emitted or not. We don't even need to move context checksnfrom 
> codegen, because with the current semantics all the checkscan be safely 
> performed at the codegen phase.


For better or worse we need this and it is actually a natural reuse of the 
multi-versioning code. We need this because:

1. For the begin/end version we cannot even parse anything in a context that 
does not match at encounter time, e.g. the `kind(fpga)` context in 
`clang/test/AST/ast-dump-openmp-begin-declare-variant.c`.
2. For the 5.0 version we cannot use the `replaceAllUses` approach currently 
implemented in `tryEmitDeclareVariant` as soon as we have the construct context 
selector trait. That means we will have to resolve the call target earlier 
anyway.

(FWIW, I wrote this part of the SPEC.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I read the spec and don't think that we need all this complex stuff for the 
implementation. Yiu need judt to check at the codegen phase if the function 
must be emitted or not. We don't even need to move context checksnfrom codegen, 
because with the current semantics all the checkscan be safely performed at the 
codegen phase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - 
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/StmtOpenMP.cpp:2243
 }
+
+// TODO: We have various representations for the same data, it might help to

This code was basically only moved, not written for this patch. It needs to 
life somewhere accessible from Parser to CodeGen, see the TODOs below.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:38
 #endif
 
-// For C++ 17 we need to include noexcept attribute to be compatible

NOTE: It might be cleaner to revert the patches that put the OpenMP handling 
code here first.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:70
 __DEVICE__ float fmod(float __x, float __y) { return ::fmodf(__x, __y); }
-// TODO: remove when variant is supported
-#ifndef _OPENMP

As far as I can tell, `fpclassify` is not available in CUDA so it is unclear if 
we want to have it here or not. I removed it due to the TODO above. 
Consequently I also had to remove other `fpclassify` occurrences. If it turns 
out the host version is not usable on the device and we need the builtins, we 
add them back but under the opposite guard, that is `#ifdef _OPENMP`.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:1048
 }
 
-/// Parse clauses for '#pragma omp declare variant ( variant-func-id ) clause'.

The diff is confusing here. I actually extracted some code into a helper 
function (`ParseOMPDeclareVariantMatchClause` on the right) which I can reuse 
in the begin/end handling. The code "deleted" here is below 
`ParseOMPDeclareVariantMatchClause` on the right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 232750.
jdoerfert added a comment.

Add (missing) include. (Worked locally just fine).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant.c
  clang/test/OpenMP/begin_declare_variant_codegen.cpp
  clang/test/OpenMP/math_fp_macro.cpp

Index: clang/test/OpenMP/math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_fp_macro.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/begin_declare_variant_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_codegen.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+int bar(void) {
+  return 0;
+}
+
+template 
+T baz(void) { return 0; }
+
+#pragma omp begin declare variant match(device={kind(cpu)})
+int foo(void) {
+  return 1;
+}
+int bar(void) {
+  return 1;
+}
+template 
+T baz(void) { return 1; }
+
+template 
+T biz(void) { return 1; }
+
+template 
+T buz(void) { return 3; }
+
+template <>
+char buz(void) { return 1; }
+
+template 
+T bez(void) { return 3; }
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device={kind(gpu)})
+int foo(void) {
+  return 2;
+}
+int bar(void) {
+  return 2;
+}
+#pragma omp end declare variant
+
+
+#pragma omp begin declare variant match(device={kind(fpga)})
+
+This text is never parsed!
+
+#pragma omp end declare variant
+
+int foo(void) {
+  return 0;
+}
+
+template 
+T biz(void) { return 0; }
+
+template <>
+char buz(void) { return 0; }
+
+template <>
+long bez(void) { return 0; }
+
+#pragma omp begin declare variant match(device = {kind(cpu)})
+template <>
+long bez(void) { return 1; }
+#pragma omp end declare variant
+
+int test() {
+  return foo() + bar() + baz() + biz() + buz() + bez();
+}
+
+// Make sure all ompvariant functions return 1 and all others return 0.
+
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3barv()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 0
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3foov.ompvariant()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 1
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3barv.ompvariant()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 1
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define signext i8 @_Z3buzIcET_v.ompvariant()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i8 1
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3foov()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 0
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define signext i8 @_Z3buzIcET_v()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i8 0
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i64 @_Z3bezIlET_v()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i64 0
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i64 @_Z3bezIlET_v.ompvariant()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i64 1
+// CHECK-NEXT:  }
+
+// Make sure we call only ompvariant functions
+
+// CHECK:  define i32 @_Z4testv()
+// CHECK:%call = call i32 @_Z3foov.ompvariant()
+// CHECK:%call1 = call i32 @_Z3barv.ompvariant()
+// CHECK:%call2 = call i32 @_Z3bazIiET_v.ompvariant()
+// CHECK:%call4 = call signext i16 @_Z3bizIsET_v.ompvariant()
+// CHECK:%call6 = call signext i8 @_Z3buzIcET_v.ompvariant()
+// 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - 
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim.
Herald added subscribers: s.egerton, guansong, bollu, simoncook, fedor.sergeev, 
aheejin, rampitec.
Herald added a project: clang.

NOTE: This is a WIP patch to foster a discussion. Please do consider
  that when browsing the code. Details will be discussed in
  individual commits once we agreed on the overall model. This is
  also the reason why test coverage, documentation, TODOs, etc. is
  lacking.

This patch provides initial support for `omp begin/end declare variant`,
as defined in OpenMP technical report 8 (TR8).

A major purpose of this patch is to provide proper math.h/cmath support
for OpenMP target offloading. See PR42061, PR42798, PR42799.
The three tests included in this patch show that these bugs (should be)
fixed in this scheme.

In contrast to the declare variant handling we already have, this patch
makes use of the multi-version handling in clang. This is especially
useful as the variants have the same name as the base functions. We
should try to port all OpenMP variant handling to this scheme, see the
TODO in CodeGenModule for a proposed way towards this goal. Other than
that, we tried to reuse the existing multi-version and OpenMP variant
handling as much as possible.

NOTE: There are various TODOs that need fixing and switches that need
  additional cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71179

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant.c
  clang/test/OpenMP/begin_declare_variant_codegen.cpp
  clang/test/OpenMP/math_fp_macro.cpp

Index: clang/test/OpenMP/math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_fp_macro.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/begin_declare_variant_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_codegen.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+int bar(void) {
+  return 0;
+}
+
+template 
+T baz(void) { return 0; }
+
+#pragma omp begin declare variant match(device={kind(cpu)})
+int foo(void) {
+  return 1;
+}
+int bar(void) {
+  return 1;
+}
+template 
+T baz(void) { return 1; }
+
+template 
+T biz(void) { return 1; }
+
+template 
+T buz(void) { return 3; }
+
+template <>
+char buz(void) { return 1; }
+
+template 
+T bez(void) { return 3; }
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device={kind(gpu)})
+int foo(void) {
+  return 2;
+}
+int bar(void) {
+  return 2;
+}
+#pragma omp end declare variant
+
+
+#pragma omp begin declare variant match(device={kind(fpga)})
+
+This text is never parsed!
+
+#pragma omp end declare variant
+
+int foo(void) {
+  return 0;
+}
+
+template 
+T biz(void) { return 0; }
+
+template <>
+char buz(void) { return 0; }
+
+template <>
+long bez(void) { return 0; }
+
+#pragma omp begin declare variant match(device = {kind(cpu)})
+template <>
+long bez(void) { return 1; }
+#pragma omp end declare variant
+
+int test() {
+  return foo() + bar() + baz() + biz() + buz() + bez();
+}
+
+// Make sure all ompvariant functions return 1 and all others return 0.
+
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3barv()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 0
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:
+// CHECK-NEXT:  define i32 @_Z3foov.ompvariant()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 1
+// CHECK-NEXT:  }
+// CHECK:   ; Function Attrs:

[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60605 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594



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

[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 232744.
hokein added a comment.

some minor fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -14,6 +14,7 @@
 #include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -853,6 +854,306 @@
 expectedResult(Code, expectedResult(T, "abc")));
 }
 
+TEST(CrossFileRenameTests, adjustRenameRanges) {
+  // Ranges in IndexedCode indicate the indexed occurrences;
+  // ranges in DraftCode indicate the expected mapped result, empty indicates
+  // we expect no matched result found.
+  struct {
+llvm::StringRef IndexedCode;
+llvm::StringRef DraftCode;
+  } Tests[] = {
+{
+  // both line and column are changed, not a near miss.
+  R"cpp(
+int [[x]] = 0;
+  )cpp",
+  R"cpp(
+
+ int x = 0;
+  )cpp",
+},
+{
+  // subset.
+  R"cpp(
+int [[x]] = 0;
+  )cpp",
+  R"cpp(
+int [[x]] = 0;
+{int x = 0; }
+  )cpp",
+},
+{
+  // shift columns.
+  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
+  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
+},
+{
+  // insert a line.
+  R"cpp(
+int [[x]] = 0;
+void foo(int x);
+  )cpp",
+  R"cpp(
+//
+int [[x]] = 0;
+void foo(int x);
+  )cpp",
+},
+  };
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus = true;
+  for (const auto  : Tests) {
+Annotations Draft(T.DraftCode);
+auto ActualRanges = adjustRenameRanges(
+Draft.code(), "x", Annotations(T.IndexedCode).ranges(), LangOpts);
+if (!Draft.ranges().empty()) {
+  EXPECT_TRUE((bool)ActualRanges) << "patchRange returned an error: "
+ << llvm::toString(ActualRanges.takeError());
+  EXPECT_THAT(Draft.ranges(),
+  testing::UnorderedElementsAreArray(*ActualRanges))
+  << T.DraftCode;
+} else {
+  EXPECT_FALSE(ActualRanges)
+  << "expected an error: " << T.DraftCode;
+  llvm::consumeError(ActualRanges.takeError());
+}
+  }
+}
+
+TEST(RangePatchingHeuristic, GetMappedRanges) {
+  // ^ in LexedCode marks the ranges we expect to be mapped; no ^ indicates
+  // there are no mapped ranges.
+  struct {
+llvm::StringRef IndexedCode;
+llvm::StringRef LexedCode;
+  } Tests[] = {
+{
+  // no lexed ranges.
+  "[[]]",
+  "",
+},
+{
+  // both line and column are changed, not a near miss.
+  R"([[]])",
+  R"(
+[[]]
+  )",
+},
+{
+  // subset.
+  "[[]]",
+  "^[[]]  [[]]"
+},
+{
+  // shift columns.
+  "[[]]   [[]]",
+  "  ^[[]]   ^[[]]  [[]]"
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+^[[]]
+
+^[[]] ^[[]]
+  )",
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+^[[]]
+  ^[[]]  ^[[]] // column is shifted.
+  )",
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+[[]]
+
+  [[]]  [[]] // not mapped (both line and column are changed).
+  )",
+},
+{
+  R"(
+[[]]
+[[]]
+
+   [[]]
+  [[]]
+
+}
+  )",
+  R"(
+// insert a new line
+^[[]]
+^[[]]
+ [[]] // additional range
+   ^[[]]
+  ^[[]]
+[[]] // additional range
+  )",
+},
+{
+  // non-distinct result (two best results), not a near miss
+  R"(
+[[]]
+[[]]
+[[]]
+  )",
+  R"(
+[[]]
+[[]]
+[[]]
+[[]]
+  )",
+}
+  };
+  for (const auto  : Tests) {
+auto Lexed = Annotations(T.LexedCode);
+auto LexedRanges = Lexed.ranges();
+std::vector ExpectedMatches;
+for (auto P : Lexed.points()) {
+  auto Match = llvm::find_if(LexedRanges, [](const Range& R) {
+return R.start == P;
+  });
+  ASSERT_NE(Match, LexedRanges.end());
+  ExpectedMatches.push_back(*Match);
+}
+
+auto Mapped =
+getMappedRanges(Annotations(T.IndexedCode).ranges(), LexedRanges);
+if (!ExpectedMatches.empty()) {
+   

[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:452
+  for (int Pos : MatchedIndex)
+Mapped.push_back(Lexed[Pos]);
+  return MatchedCB(std::move(Mapped));

sammccall wrote:
> if we're actually evaluating all ranges, can we pass the index array (by 
> reference), use it to evaluate scores, and only copy ranges for the winner?
we could use the index array to evaluate the scores, but it would make the cost 
API signature a bit weird, like `size_t 
renameRangeAdjustmentCost(ArrayRef Indexed, ArrayRef Lexed, 
ArrayRef MatchedLexedIndex);`



Comment at: clang-tools-extra/clangd/refactor/Rename.h:95
+  /// \p getBest, exposing for testing only.
+  static MatchType match(llvm::ArrayRef LHS, llvm::ArrayRef RHS);
+

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > Oops, forgot this...
> > > I think the public API isn't quite right here - exposing parts for 
> > > testing is fine, but the classification itself isn't fine grained enough 
> > > I think. (Too easy to write a test that "passes" but the actual mapping 
> > > found isn't the right one).
> > > 
> > > And the class structure wrapping a LangOpts ref seems like a detail that 
> > > can be hidden.
> > > 
> > > I'd like to see:
> > >  - a function that returns the lexed ranges from a StringRef/LangOpts
> > >  - a function that constructs the mapping given two sequences of ranges 
> > > (like `getMappedRanges(ArrayRef, ArrayRef) -> vector`
> > >  - a function that ties these together to the data structures we care 
> > > about (i.e. taking Code + identifier + LangOpts + ArrayRef or so)
> > > 
> > > then you can unit test the first and second and smoke test the third.
> > > 
> > > Tests like
> > > 
> > > ```
> > > Indexed = "int [[x]] = 0; void foo(int x);";
> > > Draft = "double [[x]] = 0; void foo(double x);";
> > > verifyRenameMatches(Indexed, Draft);
> > > ```
> > > a function that returns the lexed ranges from a StringRef/LangOpts
> > 
> > There is an existing function `collectIdentifierRanges` in SourceCode.cpp, 
> > and it has been unittested.
> > 
> > 
> > > a function that constructs the mapping given two sequences of ranges 
> > > (like getMappedRanges(ArrayRef, ArrayRef) -> vector
> > > a function that ties these together to the data structures we care about 
> > > (i.e. taking Code + identifier + LangOpts + ArrayRef or so)
> > 
> > sure, I think it is sufficient to test the second one, since the second one 
> > is a simple wrapper of the `getMappedRanges`.
> > sure, I think it is sufficient to test the second one, since the second one 
> > is a simple wrapper of the `getMappedRanges`.
> 
> Did you mean "sufficient to test the first one"?
> Testing the second one is certainly sufficient, but tests more than it needs 
> to (particularly the lexing bits again).
I got the point here, exposed `getMappedRanges` and added unit tests for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594



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

[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60605 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594



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

[PATCH] D70594: [clangd] Implement range patching heuristics for cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 232743.
hokein marked 21 inline comments as done.
hokein added a comment.

address reveiw comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70594

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -14,6 +14,7 @@
 #include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -853,6 +854,310 @@
 expectedResult(Code, expectedResult(T, "abc")));
 }
 
+TEST(CrossFileRenameTests, adjustRenameRanges) {
+  // Ranges in IndexedCode indicate the indexed occurrences;
+  // ranges in DraftCode indicate the expected mapped result, empty indicates
+  // we expect no matched result found.
+  struct {
+llvm::StringRef IndexedCode;
+llvm::StringRef DraftCode;
+  } Tests[] = {
+{
+  // both line and column are changed, not a near miss.
+  R"cpp(
+int [[x]] = 0;
+  )cpp",
+  R"cpp(
+
+ int x = 0;
+  )cpp",
+},
+{
+  // subset.
+  R"cpp(
+int [[x]] = 0;
+  )cpp",
+  R"cpp(
+int [[x]] = 0;
+{int x = 0; }
+  )cpp",
+},
+{
+  // column shift
+  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
+  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
+},
+{
+  // insert a line
+  R"cpp(
+int [[x]] = 0;
+void foo(int x);
+  )cpp",
+  R"cpp(
+// insert a line.
+int [[x]] = 0;
+void foo(int x);
+  )cpp",
+},
+  };
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus = true;
+  for (const auto  : Tests) {
+auto IndexedOccurrences = Annotations(T.IndexedCode).ranges();
+Annotations Draft(T.DraftCode);
+auto ActualRanges =
+adjustRenameRanges(Draft.code(), "x", IndexedOccurrences, LangOpts);
+if (!Draft.ranges().empty()) {
+  EXPECT_TRUE((bool)ActualRanges) << "patchRange returned an error: "
+ << llvm::toString(ActualRanges.takeError());
+  EXPECT_THAT(Draft.ranges(),
+  testing::UnorderedElementsAreArray(*ActualRanges))
+  << T.DraftCode;
+} else {
+  EXPECT_FALSE(ActualRanges)
+  << "expected an error: " << T.DraftCode;
+  llvm::consumeError(ActualRanges.takeError());
+}
+  }
+}
+
+TEST(RangePatchingHeuristic, GetMappedRanges) {
+  // ^ in LexedCode marks the ranges we expect to be mapped; no ^ indicates
+  // there are no mapped ranges.
+  struct {
+llvm::StringRef IndexedCode;
+llvm::StringRef LexedCode;
+  } Tests[] = {
+{
+  // no lexed ranges.
+  "[[]]",
+  "",
+},
+{
+  // both line and column are changed, not a near miss.
+  R"([[]])",
+  R"(
+[[]]
+  )",
+},
+{
+  // subset.
+  "[[]]",
+  "^[[]]  [[]]"
+},
+{
+  // column shift
+  "[[]]   [[]]",
+  "  ^[[]]   ^[[]]  [[]]"
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+^[[]]
+
+^[[]] ^[[]]
+  )",
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+^[[]]
+  ^[[]]  ^[[]] // column is changed.
+  )",
+},
+{
+  R"(
+[[]]
+
+[[]] [[]]
+  )",
+  R"(
+// insert a line
+[[]]
+
+  [[]]  [[]] // not mapped (both line and column are changed).
+  )",
+},
+{
+  R"(
+[[]]
+[[]]
+
+   [[]]
+  [[]]
+
+}
+  )",
+  R"(
+// insert a new line
+^[[]]
+^[[]]
+ [[]] // additional range
+   ^[[]]
+  ^[[]]
+[[]] // additional range
+  )",
+},
+{
+ // non-distinct result (have two best results), not a near miss
+  R"(
+[[]]
+[[]]
+[[]]
+  )",
+  R"(
+[[]]
+[[]]
+[[]]
+[[]]
+  )",
+}
+  };
+  for (const auto  : Tests) {
+auto IndexedRanges = Annotations(T.IndexedCode).ranges();
+auto Lexed = Annotations(T.LexedCode);
+auto LexedRanges = Lexed.ranges();
+std::vector ExpectedMatches;
+for (auto P : Lexed.points()) {
+  auto Match = llvm::find_if(LexedRanges, [](const Range& R) {
+return R.start == P;
+  });
+  ASSERT_TRUE(Match != LexedRanges.end());
+  

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG74e6ce2529fa: [Frontend] Allow OpenMP offloading to aarch64 
(authored by bryanpkc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/openmp_offload_registration.cpp


Index: clang/test/OpenMP/openmp_offload_registration.cpp
===
--- clang/test/OpenMP/openmp_offload_registration.cpp
+++ clang/test/OpenMP/openmp_offload_registration.cpp
@@ -1,5 +1,6 @@
-// Test for offload registration code for two targets
+// Test offload registration for two targets, and test offload target 
validation.
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o 
- | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=aarch64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // expected-no-diagnostics
 
 void foo() {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3070,7 +3070,8 @@
   llvm::Triple TT(A->getValue(i));
 
   if (TT.getArch() == llvm::Triple::UnknownArch ||
-  !(TT.getArch() == llvm::Triple::ppc ||
+  !(TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::ppc ||
 TT.getArch() == llvm::Triple::ppc64 ||
 TT.getArch() == llvm::Triple::ppc64le ||
 TT.getArch() == llvm::Triple::nvptx ||


Index: clang/test/OpenMP/openmp_offload_registration.cpp
===
--- clang/test/OpenMP/openmp_offload_registration.cpp
+++ clang/test/OpenMP/openmp_offload_registration.cpp
@@ -1,5 +1,6 @@
-// Test for offload registration code for two targets
+// Test offload registration for two targets, and test offload target validation.
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu -fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu -fopenmp-targets=aarch64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // expected-no-diagnostics
 
 void foo() {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3070,7 +3070,8 @@
   llvm::Triple TT(A->getValue(i));
 
   if (TT.getArch() == llvm::Triple::UnknownArch ||
-  !(TT.getArch() == llvm::Triple::ppc ||
+  !(TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::ppc ||
 TT.getArch() == llvm::Triple::ppc64 ||
 TT.getArch() == llvm::Triple::ppc64le ||
 TT.getArch() == llvm::Triple::nvptx ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 74e6ce2 - [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via cfe-commits]

Author: Bryan Chan
Date: 2019-12-08T14:45:16-05:00
New Revision: 74e6ce2529fae2c3318731c6f4f77bfa92eb6eb7

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

LOG: [Frontend] Allow OpenMP offloading to aarch64

Summary:
D30644 added OpenMP offloading to AArch64 targets, then D32035 changed the
frontend to throw an error when offloading is requested for an unsupported
target architecture. However the latter did not include AArch64 in the list
of supported architectures, causing the following unit tests to fail:

libomptarget :: api/omp_get_num_devices.c
libomptarget :: mapping/pr38704.c
libomptarget :: offloading/offloading_success.c
libomptarget :: offloading/offloading_success.cpp

Reviewers: pawosm01, gtbercea, jdoerfert, ABataev

Subscribers: kristof.beyls, guansong, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/OpenMP/openmp_offload_registration.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 198ae69b7655..1af2bdab61e4 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3070,7 +3070,8 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
   llvm::Triple TT(A->getValue(i));
 
   if (TT.getArch() == llvm::Triple::UnknownArch ||
-  !(TT.getArch() == llvm::Triple::ppc ||
+  !(TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::ppc ||
 TT.getArch() == llvm::Triple::ppc64 ||
 TT.getArch() == llvm::Triple::ppc64le ||
 TT.getArch() == llvm::Triple::nvptx ||

diff  --git a/clang/test/OpenMP/openmp_offload_registration.cpp 
b/clang/test/OpenMP/openmp_offload_registration.cpp
index b49af4d0e380..1aa2067ab8e8 100644
--- a/clang/test/OpenMP/openmp_offload_registration.cpp
+++ b/clang/test/OpenMP/openmp_offload_registration.cpp
@@ -1,5 +1,6 @@
-// Test for offload registration code for two targets
+// Test offload registration for two targets, and test offload target 
validation.
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o 
- | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=aarch64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // expected-no-diagnostics
 
 void foo() {



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

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@EricWF ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052



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

[PATCH] D71142: [Sema] Validate large bitfields

[Mark de Wever via Phabricator via cfe-commits]

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

In D71142#1774270 , @xbolva00 wrote:

> Maybe a bit offtopic, but it would be good to diagnose this case too:
>
>   enum A {a, b, c = 8};
>  
>   struct T {
>   enum A field : 2;
>   };
>
>
> GCC catches this bug.


I created https://bugs.llvm.org/show_bug.cgi?id=44251 and will look at adding 
this warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7424
+ ParsedAttr ) {
+  if (CurType->isFunctionType()) {
+State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type)

aaron.ballman wrote:
> aaron.ballman wrote:
> > Is this correct, or should it be `if (!CurType->isFunctionType())`?
> I see now that this is only incorrect for the acquire attribute, but not the 
> other two.
I think we might consider it incorrect for the acquire as well. Because if we 
let the user put acquire on the function, it is ambiguous where to put the 
annotation. If we do not let them put on the function they are always forced to 
put on the return type to express this. But in case this kind of ambiguity is a 
feature and not a bug I can change the behavior.


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

https://reviews.llvm.org/D70469



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

[PATCH] D67265: [clang-tidy] Magic number checker shouldn't warn on user defined string literals

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.
Herald added a subscriber: mgehre.

@aaron.ballman  - can you please review this patch and if you find it 
acceptable please integrate it?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67265



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7424
+ ParsedAttr ) {
+  if (CurType->isFunctionType()) {
+State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type)

aaron.ballman wrote:
> Is this correct, or should it be `if (!CurType->isFunctionType())`?
I see now that this is only incorrect for the acquire attribute, but not the 
other two.



Comment at: clang/test/Sema/attr-handles.cpp:20-21
+void ht(int __attribute__((acquire_handle(1))) *a); // expected-error 
{{'acquire_handle' attribute takes no arguments}}
+int it() __attribute__((release_handle)); // expected-warning 
{{'release_handle' only applies to non-function types; type here is 'int ()'}}
+int jt() __attribute__((use_handle)); // expected-warning {{'use_handle' only 
applies to non-function types; type here is 'int ()'}}
+int __attribute__((acquire_handle)) at; // expected-warning {{'acquire_handle' 
attribute only applies to functions, function pointers, and parameters}}

xazax.hun wrote:
> aaron.ballman wrote:
> > These diagnostics look incorrect to me -- I thought we wanted the attribute 
> > to apply to function types?
> > 
> > You should also add some test cases where the attribute is applied to a 
> > function pointer type.
> I think this one can be a bit confusing. We can apply this to the types of 
> the parameter or the return type. I thought restricting to users to put it 
> either on the parameter or the return type would make it more obvious what is 
> the target of this attribute.  In case this is not idiomatic, I can let the 
> user to attach it to the function type. 
Ah, I see -- acquire can be on a function type, but use and release are on the 
parameter. You are missing some test cases:
```
int correct() [[clang::acquire_handle]]; // Should work, applies to the 
function type
int (*fp [[clang::acquire_handle]])(); // Should work, applies to the function 
type
```



Comment at: clang/test/Sema/attr-handles.cpp:3
+
+// Decl annotations.
+void f(int *a __attribute__((acquire_handle)));

Can you add some test cases showing that these work on a typedef declaration?


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

https://reviews.llvm.org/D70469



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:22
+namespace {
+Matcher hasAnyListedName(const std::string ) {
+  const std::vector NameList =

Please use static for functions. See LLVM Coding Guidelines.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100
+
+  Finds ``signed char`` -> integer conversions which might indicate a 
programming
+  error. The basic problem with the ``signed char``, that it might store the

One sentence is enough.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:73
+  an issue.
\ No newline at end of file


Please add newline.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp:75
+}
\ No newline at end of file


Please add newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[clang] a05d7c2 - Fix typo in the AST Matcher Reference doc Closes: #54

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2019-12-08T16:14:31+01:00
New Revision: a05d7c278ee2a29aec73dbe5316e5cf2a2d190f8

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

LOG: Fix typo in the AST Matcher Reference doc Closes: #54

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index d9e867c9519d..97c75277c275 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -5212,7 +5212,7 @@ AST Traversal Matchers
 
 Given:
   MyClass *p1 = new MyClass[10];
-cxxNewExpr(hasArraySize(intgerLiteral(equals(10
+cxxNewExpr(hasArraySize(integerLiteral(equals(10
   matches the expression 'new MyClass[10]'.
 
 

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 608454631556..9c0aae2886fc 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6566,7 +6566,7 @@ AST_MATCHER(CXXNewExpr, isArray) {
 /// \code
 ///   MyClass *p1 = new MyClass[10];
 /// \endcode
-/// cxxNewExpr(hasArraySize(intgerLiteral(equals(10
+/// cxxNewExpr(hasArraySize(integerLiteral(equals(10
 ///   matches the expression 'new MyClass[10]'.
 AST_MATCHER_P(CXXNewExpr, hasArraySize, internal::Matcher, InnerMatcher) 
{
   return Node.isArray() && *Node.getArraySize() &&



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

[PATCH] D71142: [Sema] Validate large bitfields

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Maybe a bit offtopic, but it would be good to diagnose this case too:

  enum A {a, b, c = 8};
  
  struct T {
  enum A field : 2;
  };

GCC catches this bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D71142: [Sema] Validate large bitfields

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Further testing revealed the Codegen already has decided the limit.
`clang/lib/CodeGen/CGRecordLayout.h:64`:

  struct CGBitFieldInfo {
/// The offset within a contiguous run of bitfields that are represented as
/// a single "field" within the LLVM struct type. This offset is in bits.
unsigned Offset : 16;
  
/// The total size of the bit-field, in bits.
unsigned Size : 15;

So I'll look at using that limit. I also want to look at improving the 
diagnostics a bit more.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5186-5189
+def warn_bitfield_width_exceeds_maximum_width: Error<
+  "width of bit-field %0 doesn't fit in a 64 bit unsigned integer">;
+def warn_anon_bitfield_width_exceeds_maximum_width : Error<
+  "width of anonymous bit-field doesn't fit in a 64 bit unsigned integer">;

aaron.ballman wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > I feel like this situation should be an error rather than a warning -- 
> > > what could the code possibly have meant?
> > The name of the diagnostic and the kind of diagnostic should agree. 
> > Currently we have `warn_` vs `Error<`.
> Ooof, good catch! These diagnostics should be renamed to start with `err_` 
> instead.
Hmm odd I'm sure I left a comment here yesterday, but it seems I didn't commit 
it properly.
It is a copy paste bug and I forgot to change the `warn_` prefix to `err_`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D71154: Driver: Don't look for libc++ headers in the install directory on Android.

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

Sorry, I had to revert this for breaking things. Please ping me when you have a 
new patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71154



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

[clang] b324902 - Revert "Driver: Don't look for libc++ headers in the install directory on Android."

[David Zarzycki via cfe-commits]

Author: David Zarzycki
Date: 2019-12-08T16:41:46+02:00
New Revision: b32490270b786d2c5ba154e613ee2d5e36ed4197

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

LOG: Revert "Driver: Don't look for libc++ headers in the install directory on 
Android."

This reverts commit 198fbcb817492ff45946e3f7517de15e8cdf0607.

This breaks Fedora 31.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Linux.cpp
clang/test/Driver/stdlibxx-isystem.cpp

Removed: 
clang/test/Driver/android-no-installed-libcxx.cpp



diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index adacd705d831..736a2d435ca5 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -888,25 +888,20 @@ static std::string 
DetectLibcxxIncludePath(llvm::vfs::FileSystem ,
 void Linux::addLibCxxIncludePaths(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
   const std::string& SysRoot = computeSysRoot();
-  auto AddIncludePath = [&](std::string Path) {
-std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
+  const std::string LibCXXIncludePathCandidates[] = {
+  DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
+  // If this is a development, non-installed, clang, libcxx will
+  // not be found at ../include/c++ but it likely to be found at
+  // one of the following two locations:
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
+  DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
+  for (const auto  : LibCXXIncludePathCandidates) {
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
-  return false;
+  continue;
+// Use the first candidate that exists.
 addSystemInclude(DriverArgs, CC1Args, IncludePath);
-return true;
-  };
-  // Android never uses the libc++ headers installed alongside the toolchain,
-  // which are generally incompatible with the NDK libraries anyway.
-  if (!getTriple().isAndroid())
-if (AddIncludePath(getDriver().Dir + "/../include/c++"))
-  return;
-  // If this is a development, non-installed, clang, libcxx will
-  // not be found at ../include/c++ but it likely to be found at
-  // one of the following two locations:
-  if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
-return;
-  if (AddIncludePath(SysRoot + "/usr/include/c++"))
 return;
+  }
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,

diff  --git a/clang/test/Driver/android-no-installed-libcxx.cpp 
b/clang/test/Driver/android-no-installed-libcxx.cpp
deleted file mode 100644
index b6f4227c7dc1..
--- a/clang/test/Driver/android-no-installed-libcxx.cpp
+++ /dev/null
@@ -1,8 +0,0 @@
-// Check that we don't find the libc++ in the installation directory when
-// targeting Android.
-
-// RUN: mkdir -p %t/bin
-// RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
-// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | FileCheck %s
-// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

diff  --git a/clang/test/Driver/stdlibxx-isystem.cpp 
b/clang/test/Driver/stdlibxx-isystem.cpp
index 827cdf9a7c71..cf7535ff423e 100644
--- a/clang/test/Driver/stdlibxx-isystem.cpp
+++ b/clang/test/Driver/stdlibxx-isystem.cpp
@@ -6,7 +6,7 @@
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -16,7 +16,7 @@
 // LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // Passing -stdlib++-isystem should suppress the default search.
-// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
@@ -26,7 +26,7 @@
 // NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // And we should add it as an -internal-isystem.
-// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH 

[PATCH] D71141: [Wdocumentation] Use C2x/C++14 deprecated attribute

[Mark de Wever via Phabricator via cfe-commits]

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

Thanks for the review! I'll wait with committing until @gribozavr2 had time to 
review this patch and D71140 .




Comment at: clang/lib/AST/CommentSema.cpp:693
+StringRef AttributeSpelling =
+CPlusPlus14 ? "[[deprecated]]" : "__attribute__((deprecated))";
 if (PP) {

aaron.ballman wrote:
> Mordante wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Mordante wrote:
> > > > > aaron.ballman wrote:
> > > > > > This attribute also exists with this spelling in C2x, FWIW.
> > > > > True, but unless I'm mistaken `CPlusPlus17` and `CPlusPlus2a` also 
> > > > > include `CPlusPlus14`. Do you prefer a different name for the Boolean?
> > > > I'm talking about C2x, not C++2a. The name for the variable is fine, 
> > > > but we should prefer `[[deprecated]]` in C2x mode to 
> > > > `__attribute__((deprecated))`.
> > > > 
> > > > I think the correct predicate is: 
> > > > `getLangOpts().DoubleSquareBracketAttributes` -- if the user says they 
> > > > want to use double-square bracket attributes, we should probably prefer 
> > > > them to GNU-style attributes.
> > > Ah sorry I misread. I'll have a look at C2x. Thanks for the information.
> > `getLangOpts().DoubleSquareBracketAttributes` will not work since it 
> > includes C++11, which doesn't support `[[deprecated]]`, so I will just test 
> > for C++14 and C2x.
> > (I had a look at the proper syntax in C2x and found N2334 ;-))
> > getLangOpts().DoubleSquareBracketAttributes will not work since it includes 
> > C++11, which doesn't support [[deprecated]], so I will just test for C++14 
> > and C2x.
> 
> We support `[[deprecated]]` in C++11 mode as an extension, but I suppose the 
> concern there is pedantic warnings?
> 
> I think what you're doing now is fine, but it suggests that we need a better 
> interface to `clang::hasAttribute()` so that you can test for support through 
> the same machinery as `__has_cpp_attribute`/`__has_c_attribute`. It wouldn't 
> be suitable to use here because you want to know something more specific than 
> it can tell you (whether the attribute can be used without triggering an 
> extension diagnostic), but this seems like a reasonable utility for us to 
> have someday.
My reason not to use `[[deprecated]]` in C++11 is standard compliance. The 
original bug reporter was using clang-tidy to improve the code. We don't know 
whether users of clang-tidy only use the clang compiler or also other compilers 
which don't offer `[[deprecated]]` as C++11 extension.


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

https://reviews.llvm.org/D71141



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I run the new check on LLVM codebase with the option CharTypdefsToIgnore = 
"int8_t".
The check produced 12 findings.

All findings seem valid use cases, where a char -> integer conversion happens.
I had a closer look at some of the catches.

Somewhere I see only type mismatch. Char is used instead of an integer type.
For example, in the finding above, trailingBytesForUTF8 is defined as char 
array, but it is used
everywhere as integer:

  /home/zolnai/clang/llvm-project/llvm/lib/Support/ConvertUTF.cpp:623:43: 
warning: singed char -> integer ('unsigned short') conversion; consider to cast 
to unsigned char first. [bugprone-signed-char-misuse]
  unsigned short extraBytesToRead = trailingBytesForUTF8[*source];

Another use case seems suspicious to me. Here we have some parsing code, if I'm 
right, which uses the EOF from cstdio header, which is -1,
so might be a similar use case which is mentioned by the CERT specification.
/home/zolnai/clang/llvm-project/llvm/lib/TableGen/TGLexer.cpp:596:20: warning: 
singed char -> integer ('int') conversion; consider to cast to unsigned char 
first. [bugprone-signed-char-misuse]

  int NextChar = *CurPtr;

At the location of the third use case, there is already a comment about signed 
chars ("/* xxx what about signed chars here... */"),
so I interpret it as a validation of the check.
/home/zolnai/clang/llvm-project/llvm/lib/Support/regcomp.c:929:12: warning: 
singed char -> integer ('int') conversion; consider to cast to unsigned char 
first. [bugprone-signed-char-misuse]

  for (i = start; i <= finish; i++)

The whole output can be found here:
https://gist.github.com/tzolnai/b1cbe3f021b53a7ca1a6ecbffc1a9bf6


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore 
= "sal_Int8".
The check produced 32 findings.

I see 3 false positives which are similar to the DereferenceWithTypdef test 
case where the code
uses sal_Int8 typedef, but the check still catches the cast, because the 
typedef information is
somehow lost by the dereferencing. Other use cases seem to be valid catches.

I had a closer look at some of the catches. Two of them were doing the same 
conversion what
an unsigned char conversion does. So there I replaced the old solution with a 
cast:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=85b5253492cd1c87cebc98d4c453812562a2f9ef

In other cases, the code is suspicious and surely would fail on non-ASCII 
characters, but
because of the context that does not happen. For example, a code which works 
with
font names never could get any special character, since font names are ASCII 
strings.

The output can be found here:
https://gist.github.com/tzolnai/c7e70d9bb51b78dd8a4036d209b19682


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

This check searches for signed char -> integer conversions which might
indicate programming error, because of the misinterpretation of char
values. A signed char might store the non-ASCII characters as negative
values. The human programmer probably expects that after an integer
conversion the converted value matches with the character code
(a value from [0..255]), however, the actual value is in
[-128..127] interval.

See also:
STR34-C. Cast characters to unsigned char before converting to larger integer 
sizes
https://wiki.sei.cmu.edu/confluence/display/c/STR34-C.+Cast+characters+to+unsigned+char+before+converting+to+larger+integer+sizes

By now this check is limited to assignment / variable declarations.
If we would catch all signed char -> integer conversion, then it would
produce a lot of findings and also false positives. So I added only
this use case now, but this check can be extended with additional
use cases later.
The CERT documentation mentions another use case when the char is
used for array subscript. Next to that a third use case can be
the signed char - unsigned char comparison, which also a use case
where things happen unexpectedly because of conversion to integer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71174

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t
+
+///
+/// Test cases correctly caught by the check.
+
+int SimpleVarDeclaration() {
+  signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CStyleCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = (int)CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int StaticCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = static_cast(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:33: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int FunctionalCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = int(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int NegativeConstValue() {
+  const signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CharPointer(signed char *CCharacter) {
+  int NCharacter = *CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///
+/// Test cases correctly ignored by the check.
+
+int UnsignedCharCast() {
+  unsigned char CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+int PositiveConstValue() {
+  const signed char CCharacter = 5;
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of declaration expression.
+int DescendantCast() {
+  signed char CCharacter = 'a';
+  int NCharacter = 10 + 

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60602 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71172



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

[PATCH] D71172: [clang][Tooling] Fix potential UB in ExpandResponseFilesCompilationDatabase

[liu hui via Phabricator via cfe-commits]

lh123 created this revision.
lh123 added reviewers: kadircet, sammccall, hokein.
Herald added subscribers: cfe-commits, usaxena95, ilya-biryukov.
Herald added a project: clang.

`vector::assign` will cause UB at here.

fixes: https://github.com/clangd/clangd/issues/223


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71172

Files:
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,8 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }


Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,7 +61,8 @@
   llvm::StringSaver Saver(Alloc);
   llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
 llvm::StringRef(Cmd.Directory));
-  Cmd.CommandLine.assign(Argv.begin(), Argv.end());
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  Cmd.CommandLine = std::move(ExpandedArgv);
 }
 return Cmds;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71111: [Sema] Improve diagnostic about addr spaces for overload candidates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Hmm.  How about:

  // For the `this` argument
  candidate function not viable: 'this' object is in '__private' address space, 
but method expects object in '__global' address space
  
  // For pointer arguments
  candidate function not viable: cannot pass pointer to '__private' address 
space as a pointer to '__global' address space in 1st argument
  
  // For reference arguments
  candidate function not viable: cannot bind reference in '__global' address 
space to object in '__private' address space in 1st argument

This would require you to render a string describing the address space 
yourself.  I would suggest "generic address space" for the default AS outside 
of OpenCL, "'foo' address space" for a language-specific AS (including implicit 
__private in OpenCL), or "address space N" for a numbered address space.

What do you think?


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

https://reviews.llvm.org/D7



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