date:20200307

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

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

tambre added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2579
+  if (Version == CudaVersion::UNKNOWN) {
+C.getDriver().Diag(clang::diag::err_drv_cuda_unknown_version)
+<< VersionString;

This error is hit when simply running `clang++ -v`, because a `CudaToolchain` 
isn't created. The `CudaInstallation` is instead created in `Generic_GCC`. My 
current approach of propagating the current CUDA version to here seems even 
worse now. Ignoring an unknown version doesn't seem like a good idea either.
Ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75811



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

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

2020-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rG29e1a16be821: [NFC] Let mangler accept GlobalDecl (authored 
by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75700

Files:
  clang/include/clang/AST/GlobalDecl.h
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3922,7 +3922,7 @@
   // Calculate the mangled name.
   SmallString<256> ThunkName;
   llvm::raw_svector_ostream Out(ThunkName);
-  getMangleContext().mangleCXXCtor(CD, CT, Out);
+  getMangleContext().mangleName(GlobalDecl(CD, CT), Out);
 
   // If the thunk has been generated previously, just return it.
   if (llvm::GlobalValue *GV = CGM.getModule().getNamedValue(ThunkName))
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1023,22 +1023,15 @@
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
   MangleContext  = CGM.getCXXABI().getMangleContext();
-  if (MC.shouldMangleDeclName(ND)) {
-llvm::raw_svector_ostream Out(Buffer);
-if (const auto *D = dyn_cast(ND))
-  MC.mangleCXXCtor(D, GD.getCtorType(), Out);
-else if (const auto *D = dyn_cast(ND))
-  MC.mangleCXXDtor(D, GD.getDtorType(), Out);
-else
-  MC.mangleName(ND, Out);
-  } else {
+  if (MC.shouldMangleDeclName(ND))
+MC.mangleName(GD.getWithDecl(ND), Out);
+  else {
 IdentifierInfo *II = ND->getIdentifier();
 assert(II && "Attempt to mangle unnamed decl.");
 const auto *FD = dyn_cast(ND);
 
 if (FD &&
 FD->getType()->castAs()->getCallConv() == CC_X86RegCall) {
-  llvm::raw_svector_ostream Out(Buffer);
   Out << "__regcall3__" << II->getName();
 } else {
   Out << II->getName();
@@ -4485,7 +4478,7 @@
 
   maybeSetTrivialComdat(*D, *Fn);
 
-  CodeGenFunction(*this).GenerateCode(D, Fn, FI);
+  CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
 
   setNonAliasAttributes(GD, Fn);
   SetLLVMFunctionAttributesForDefinition(D, Fn);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2413,13 +2413,14 @@
 }
 
 static llvm::Constant *EmitFunctionDeclPointer(CodeGenModule ,
-   const FunctionDecl *FD) {
+   GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
   if (FD->hasAttr()) {
 ConstantAddress aliasee = CGM.GetWeakRefReference(FD);
 return aliasee.getPointer();
   }
 
-  llvm::Constant *V = CGM.GetAddrOfFunction(FD);
+  llvm::Constant *V = CGM.GetAddrOfFunction(GD);
   if (!FD->hasPrototype()) {
 if (const FunctionProtoType *Proto =
 FD->getType()->getAs()) {
@@ -2436,9 +2437,10 @@
   return V;
 }
 
-static LValue EmitFunctionDeclLValue(CodeGenFunction ,
- const Expr *E, const FunctionDecl *FD) {
-  llvm::Value *V = EmitFunctionDeclPointer(CGF.CGM, FD);
+static LValue EmitFunctionDeclLValue(CodeGenFunction , const Expr *E,
+ GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
+  llvm::Value *V = EmitFunctionDeclPointer(CGF.CGM, GD);
   CharUnits Alignment = CGF.getContext().getDeclAlign(FD);
   return CGF.MakeAddrLValue(V, E->getType(), Alignment,
 AlignmentSource::Decl);
@@ -4638,7 +4640,8 @@
   return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue);
 }
 
-static CGCallee EmitDirectCallee(CodeGenFunction , const FunctionDecl *FD) {
+static CGCallee EmitDirectCallee(CodeGenFunction , GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
 
   if (auto builtinID = FD->getBuiltinID()) {
 // Replaceable builtin provide their own implementation of a builtin. Unless
@@ -4650,8 +4653,8 @@
   return CGCallee::forBuiltin(builtinID, FD);
   }
 
-  llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
-  return CGCallee::forDirect(calleePtr, GlobalDecl(FD));
+  llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
+  return CGCallee::forDirect(calleePtr, GD);
 }
 
 CGCallee CodeGenFunction::EmitCallee(const Expr *E) {
Index: clang/lib/AST/MicrosoftMangle.cpp
===
---

[clang] 29e1a16 - [NFC] Let mangler accept GlobalDecl

2020-03-07 Thread Yaxun Liu via cfe-commits


Author: Yaxun (Sam) Liu
Date: 2020-03-07T23:51:41-05:00
New Revision: 29e1a16be8216066d1ed733a763a749aed13ff47

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

LOG: [NFC] Let mangler accept GlobalDecl

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

Added: 


Modified: 
clang/include/clang/AST/GlobalDecl.h
clang/include/clang/AST/Mangle.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/Mangle.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp

Removed: 




diff  --git a/clang/include/clang/AST/GlobalDecl.h 
b/clang/include/clang/AST/GlobalDecl.h
index 145e961a23a3..0945ebb56a48 100644
--- a/clang/include/clang/AST/GlobalDecl.h
+++ b/clang/include/clang/AST/GlobalDecl.h
@@ -35,10 +35,18 @@ enum class DynamicInitKind : unsigned {
 
 /// GlobalDecl - represents a global declaration. This can either be a
 /// CXXConstructorDecl and the constructor type (Base, Complete).
-/// a CXXDestructorDecl and the destructor type (Base, Complete) or
+/// a CXXDestructorDecl and the destructor type (Base, Complete),
+/// a FunctionDecl and the kernel reference type (Kernel, Stub), or
 /// a VarDecl, a FunctionDecl or a BlockDecl.
+///
+/// When a new type of GlobalDecl is added, the following places should
+/// be updated to convert a Decl* to a GlobalDecl:
+/// PredefinedExpr::ComputeName() in lib/AST/Expr.cpp.
+/// getParentOfLocalEntity() in lib/AST/ItaniumMangle.cpp
+/// ASTNameGenerator::Implementation::writeFuncOrVarName in lib/AST/Mangle.cpp
+///
 class GlobalDecl {
-  llvm::PointerIntPair Value;
+  llvm::PointerIntPair Value;
   unsigned MultiVersionIndex = 0;
 
   void Init(const Decl *D) {
@@ -55,6 +63,7 @@ class GlobalDecl {
   : MultiVersionIndex(MVIndex) {
 Init(D);
   }
+  GlobalDecl(const NamedDecl *D) { Init(D); }
   GlobalDecl(const BlockDecl *D) { Init(D); }
   GlobalDecl(const CapturedDecl *D) { Init(D); }
   GlobalDecl(const ObjCMethodDecl *D) { Init(D); }
@@ -108,6 +117,8 @@ class GlobalDecl {
 
   void *getAsOpaquePtr() const { return Value.getOpaqueValue(); }
 
+  explicit operator bool() const { return getAsOpaquePtr(); }
+
   static GlobalDecl getFromOpaquePtr(void *P) {
 GlobalDecl GD;
 GD.Value.setFromOpaqueValue(P);

diff  --git a/clang/include/clang/AST/Mangle.h 
b/clang/include/clang/AST/Mangle.h
index 5db5c5b977da..39e4f2335deb 100644
--- a/clang/include/clang/AST/Mangle.h
+++ b/clang/include/clang/AST/Mangle.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_AST_MANGLE_H
 
 #include "clang/AST/Decl.h"
+#include "clang/AST/GlobalDecl.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/ABI.h"
 #include "llvm/ADT/DenseMap.h"
@@ -96,8 +97,8 @@ class MangleContext {
   virtual bool shouldMangleStringLiteral(const StringLiteral *SL) = 0;
 
   // FIXME: consider replacing raw_ostream & with something like SmallString &.
-  void mangleName(const NamedDecl *D, raw_ostream &);
-  virtual void mangleCXXName(const NamedDecl *D, raw_ostream &) = 0;
+  void mangleName(GlobalDecl GD, raw_ostream &);
+  virtual void mangleCXXName(GlobalDecl GD, raw_ostream &) = 0;
   virtual void mangleThunk(const CXXMethodDecl *MD,
   const ThunkInfo ,
   raw_ostream &) = 0;
@@ -109,10 +110,6 @@ class MangleContext {
 raw_ostream &) = 0;
   virtual void mangleCXXRTTI(QualType T, raw_ostream &) = 0;
   virtual void mangleCXXRTTIName(QualType T, raw_ostream &) = 0;
-  virtual void mangleCXXCtor(const CXXConstructorDecl *D, CXXCtorType Type,
- raw_ostream &) = 0;
-  virtual void mangleCXXDtor(const CXXDestructorDecl *D, CXXDtorType Type,
- raw_ostream &) = 0;
   virtual void mangleStringLiteral(const StringLiteral *SL, raw_ostream &) = 0;
 
   void mangleGlobalBlock(const BlockDecl *BD,

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 79f9f42224d0..3377afccb5e6 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -650,12 +650,14 @@ std::string PredefinedExpr::ComputeName(IdentKind IK, 
const Decl *CurrentDecl) {
   if (MC->shouldMangleDeclName(ND)) {
 SmallString<256> Buffer;
 llvm::raw_svector_ostream Out(Buffer);
+GlobalDecl GD;
 if (const CXXConstructorDecl *CD = dyn_cast(ND))
-  MC->mangleCXXCtor(CD, Ctor_Base, Out);
+  GD = GlobalDecl(CD, Ctor_Base);
 else if (const CXXDestructorDecl *DD = dyn_cast(ND))
-  MC->mangleCXXDtor(DD, Dtor_Base, Out);
+  GD = GlobalDecl(DD, Dtor_Base);
 else
-  MC->mangleName(ND, Out);
+  GD = GlobalDecl(ND);
+

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-07 Thread Marcus Johnson via Phabricator via cfe-commits

MarcusJohnson91 updated this revision to Diff 248975.
MarcusJohnson91 added a comment.

Updated the release notes


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

https://reviews.llvm.org/D75791

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -200,6 +200,23 @@
 
 
 
+- Option ``IndentExternBlock`` has been added to optionally apply indenting 
inside extern "C" blocks.
+  
+  The ``BraceWrapping.AfterExternBlock`` option has been modified so it no 
longer indents when set to true, now it just wraps the braces around extern 
blocks.
+
+  .. code-block:: c++
+
+true: false:
+  #ifdef __cplusplus  #ifdef __cplusplus
+  extern "C" {extern "C" {
+  #endif  #endif
+  
+  void f(void);   void f(void);
+  
+  #ifdef __cplusplus  #ifdef __cplusplus
+  }   }
+  #endif  #endif
+
 - Option ``IndentCaseBlocks`` has been added to support treating the block
   following a switch case label as a scope block which gets indented itself.
   It helps avoid having the closing bracket align with the switch statement's


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -200,6 +200,23 @@
 
 
 
+- Option ``IndentExternBlock`` has been added to optionally apply indenting inside extern "C" blocks.
+  
+  The ``BraceWrapping.AfterExternBlock`` option has been modified so it no longer indents when set to true, now it just wraps the braces around extern blocks.
+
+  .. code-block:: c++
+
+true: false:
+  #ifdef __cplusplus  #ifdef __cplusplus
+  extern "C" {extern "C" {
+  #endif  #endif
+  
+  void f(void);   void f(void);
+  
+  #ifdef __cplusplus  #ifdef __cplusplus
+  }   }
+  #endif  #endif
+
 - Option ``IndentCaseBlocks`` has been added to support treating the block
   following a switch case label as a scope block which gets indented itself.
   It helps avoid having the closing bracket align with the switch statement's
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-07 Thread Marcus Johnson via Phabricator via cfe-commits

MarcusJohnson91 updated this revision to Diff 248974.
MarcusJohnson91 added a comment.

Removed the debugging comments I added to FormatTest.cpp


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

https://reviews.llvm.org/D75791

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2436,14 +2436,14 @@
 }
 
 TEST_F(FormatTest, FormatsExternC) {
-  verifyFormat("extern \"C\" {\nint a;");
+  verifyFormat("extern \"C\" {\nint a; /*2.1*/");
   verifyFormat("extern \"C\" {}");
   verifyFormat("extern \"C\" {\n"
-   "int foo();\n"
+   "int FormatsExternC_1();\n"
"}");
-  verifyFormat("extern \"C\" int foo() {}");
-  verifyFormat("extern \"C\" int foo();");
-  verifyFormat("extern \"C\" int foo() {\n"
+  verifyFormat("extern \"C\" int FormatsExternC_2() {}");
+  verifyFormat("extern \"C\" int FormatsExternC_3();");
+  verifyFormat("extern \"C\" int FormatsExternC_4() {\n"
"  int i = 42;\n"
"  return i;\n"
"}");
@@ -2451,9 +2451,9 @@
   FormatStyle Style = getLLVMStyle();
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  verifyFormat("extern \"C\" int foo() {}", Style);
-  verifyFormat("extern \"C\" int foo();", Style);
-  verifyFormat("extern \"C\" int foo()\n"
+  verifyFormat("extern \"C\" int FormatsExternC_5() {}", Style);
+  verifyFormat("extern \"C\" int FormatsExternC_6();", Style);
+  verifyFormat("extern \"C\" int FormatsExternC_7()\n"
"{\n"
"  int i = 42;\n"
"  return i;\n"
@@ -2462,20 +2462,41 @@
 
   Style.BraceWrapping.AfterExternBlock = true;
   Style.BraceWrapping.SplitEmptyRecord = false;
-  verifyFormat("extern \"C\" {}", Style);
-  verifyFormat("extern \"C\" {\n"
-   "  int foo();\n"
-   "}",
-   Style);
+  verifyFormat("extern \"C\"\n{}", Style);
+  verifyFormat("extern \"C\"\n{\nint FormatsExternC_8();\n}", Style);
 
   Style.BraceWrapping.AfterExternBlock = false;
   verifyFormat("extern \"C\" {}", Style);
   verifyFormat("extern \"C\" {\n"
-   "int foo();\n"
+   "int FormatsExternC_9();\n"
"}",
Style);
 }
 
+TEST_F(FormatTest, FormatsExternBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentWidth = 2;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = true;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n  int FormatsExternBlock_1();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  Style.IndentExternBlock = true;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n  int FormatsExternBlock_2();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\nint FormatsExternBlock_3();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  Style.IndentExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\nint FormatsExternBlock_4();\n}", Style);
+}
+
 TEST_F(FormatTest, FormatsInlineASM) {
   verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));");
   verifyFormat("asm(\"nop\" ::: \"memory\");");
@@ -12640,6 +12661,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(IndentExternBlock);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1085,10 +1085,21 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterExternBlock) {
-  parseBlock(/*MustBeDeclaration=*/true);
-} else {
+if (Style.BraceWrapping.AfterExternBlock == true &&
+Style.IndentExternBlock == true) {
+  addUnwrappedLine();
+  parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true);
+} else if (Style.BraceWrapping.AfterExternBlock == false &&
+   Style.IndentExternBlock == false) {
   parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
+} else if (Style.BraceWrapping.AfterExternBlock == false &&
+   Style.IndentExternBlock == true) {
+

[PATCH] D72635: Allow arbitrary capability name in Thread Safety Analysis

2020-03-07 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

@aaron.ballman, I've just noted that one of the `-Wthread-safety-attributes` 
warnings says 


> //A// attribute can only be applied in a context annotated with 
> ‘capability(“mutex”)’ attribute

This should be changed then, right? We never enforced the name anyway.


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

https://reviews.llvm.org/D72635



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

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a41b31fcdfc: [Sema] Add -Wpointer-to-enum-cast and 
-Wvoid-pointer-to-enum-cast (authored by nathanchance, committed by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D75758?vs=248824=248972#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/cast.c


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@
 void *intToPointerCast3() {
   return (void*)(1 + 3);
 }
+
+void voidPointerToEnumCast(VoidPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'VoidPtr' (aka 'void *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // no-warning
+#pragma clang diagnostic pop
+}
+
+void pointerToEnumCast(CharPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+#pragma clang diagnostic pop
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2777,11 +2777,16 @@
   // If the result cannot be represented in the integer type, the behavior
   // is undefined. The result need not be in the range of values of any
   // integer type.
-  unsigned Diag = Self.getLangOpts().MicrosoftExt
-  ? diag::ext_ms_pointer_to_int_cast
-  : SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
+  unsigned Diag;
+  if (Self.getLangOpts().MicrosoftExt)
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+Diag = DestType->isEnumeralType() ? 
diag::warn_void_pointer_to_enum_cast
+  : 
diag::warn_void_pointer_to_int_cast;
+  else if (DestType->isEnumeralType())
+Diag = diag::warn_pointer_to_enum_cast;
+  else
+Diag = diag::warn_pointer_to_int_cast;
   Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
 }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3668,9 +3668,15 @@
 def warn_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_pointer_to_enum_cast : Warning<
+  warn_pointer_to_int_cast.Text>,
+  InGroup;
 def warn_void_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_void_pointer_to_enum_cast : Warning<
+  warn_void_pointer_to_int_cast.Text>,
+  InGroup;
 def ext_ms_pointer_to_int_cast : ExtWarn<
   "cast to smaller integer type %1 from %0 is a Microsoft extension">,
 InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -838,9 +838,13 @@
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
-def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def VoidPointerToEnumCast : DiagGroup<"void-pointer-to-enum-cast">;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast",
+ [VoidPointerToEnumCast]>;
+def PointerToEnumCast : DiagGroup<"pointer-to-enum-cast",
+  [VoidPointerToEnumCast]>;
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
- [VoidPointerToIntCast]>;
+ [PointerToEnumCast, VoidPointerToIntCast]>;
 
 def Move : DiagGroup<"move", [
 PessimizingMove,


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

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

MaskRay added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:2783
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+if (DestType->isEnumeralType())

A ternary operator is better here. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758



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

[clang] 2a41b31 - [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

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


Author: Nathan Chancellor
Date: 2020-03-07T16:43:39-08:00
New Revision: 2a41b31fcdfcb67ab7038fc2ffb606fd50b83a84

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

LOG: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

GCC does not warn on casts from pointers to enumerators, while clang
currently does: https://godbolt.org/z/3DFDVG

This causes a bunch of extra warnings in the Linux kernel, where
certain structs contain a void pointer to avoid using a gigantic
union for all of the various types of driver data, such as
versions.

Add a diagnostic that allows certain projects like the kernel to
disable the warning just for enums, which allows those projects to
keep full compatibility with GCC but keeps the intention of treating
casts to integers and enumerators the same by default so that other
projects have the opportunity to catch issues not noticed before (or
follow suite and disable the warning).

Link: https://github.com/ClangBuiltLinux/linux/issues/887

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/test/Sema/cast.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 6c79ea591734..ae3f882dd910 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -838,9 +838,13 @@ def IncompatibleExceptionSpec : 
DiagGroup<"incompatible-exception-spec">;
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
-def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def VoidPointerToEnumCast : DiagGroup<"void-pointer-to-enum-cast">;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast",
+ [VoidPointerToEnumCast]>;
+def PointerToEnumCast : DiagGroup<"pointer-to-enum-cast",
+  [VoidPointerToEnumCast]>;
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
- [VoidPointerToIntCast]>;
+ [PointerToEnumCast, VoidPointerToIntCast]>;
 
 def Move : DiagGroup<"move", [
 PessimizingMove,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 181341fed780..b0338c44cca9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3668,9 +3668,15 @@ def warn_int_to_void_pointer_cast : Warning<
 def warn_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_pointer_to_enum_cast : Warning<
+  warn_pointer_to_int_cast.Text>,
+  InGroup;
 def warn_void_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_void_pointer_to_enum_cast : Warning<
+  warn_void_pointer_to_int_cast.Text>,
+  InGroup;
 def ext_ms_pointer_to_int_cast : ExtWarn<
   "cast to smaller integer type %1 from %0 is a Microsoft extension">,
 InGroup;

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 17d07c57a412..8edff2439e0d 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2777,11 +2777,16 @@ void CastOperation::CheckCStyleCast() {
   // If the result cannot be represented in the integer type, the behavior
   // is undefined. The result need not be in the range of values of any
   // integer type.
-  unsigned Diag = Self.getLangOpts().MicrosoftExt
-  ? diag::ext_ms_pointer_to_int_cast
-  : SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
+  unsigned Diag;
+  if (Self.getLangOpts().MicrosoftExt)
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+Diag = DestType->isEnumeralType() ? 
diag::warn_void_pointer_to_enum_cast
+  : 
diag::warn_void_pointer_to_int_cast;
+  else if (DestType->isEnumeralType())
+Diag = diag::warn_pointer_to_enum_cast;
+  else
+Diag = diag::warn_pointer_to_int_cast;
   Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
 }
   }

diff  --git a/clang/test/Sema/cast.c b/clang/test/Sema/cast.c
index 0c4fc7d129fc..2335f2198071 100644
--- a/clang/test/Sema/cast.c
+++ b/clang/test/Sema/cast.c
@@ -186,3

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

2020-03-07 Thread Nathan Lanza via Phabricator via cfe-commits

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

> Adding some more knowledgeable reviewers for comments on your RFC. I pointed 
> out a few minor nits, but I'll hold off on a technical review until the 
> ObjC-specific details are worked out and there is buy-in on the feature.

Thanks!

> This should be implemented via a custom LangOpt in Attr.td

This was done to more-or-less mirror the behavior from `objc_direct`. Though if 
people think the implementations should differ I'm cool with that.

In D75574#1911368 , @rjmccall wrote:

> I'm fine with people developing a proposal for this openly, but it needs to 
> be said that language changes cannot just be made in open-source; they have 
> to go through the official language review process, which for Objective-C is 
> an internal committee within Apple.


Yea sure, whatever the correct process is I'm more than happy to cooperate. I 
was going to post something on the mailing list some time this week, but this 
diff got attention early! What would be the best way to start that conversation?

> The summary calls this `objc_direct_protocol`, but it's 
> `objc_static_protocol` in the patch.  I agree that "direct" isn't a great 
> name for this.  `static` is complicated, because while we use "static" vs. 
> "dynamic" this way when we're talking *about* languages, I'm not sure any of 
> the umpteen language uses of `static` ever use it in exactly this way, and 
> it's possibly quite confusing to add one.  Throwing  out other names here: 
> `objc_non_runtime_protocol`? `objc_compiler_only_protocol`?

Yea, the naming I'm not too happy about and changed it around a few times. Your 
proposed naming is certainly more clear.

> The technical content of the patch seems fine.






Comment at: clang/include/clang/AST/DeclObjC.h:2197
 
+  /// True if the protocol is tagged as objc_static_protocol
+  bool isStaticProtocol() const;

aaron.ballman wrote:
> Comments should be properly punctuated (missing a full stop at the end of the 
> sentence), here and elsewhere.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

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

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGExprAgg.cpp:822
+Dest.setExternallyDestructed();
+  }
+

I don't think `setExternallyDestructed` can be used to communicate outwards 
like this; the code isn't set up to just do modifications on a single 
`AggValueSlot` that's passed around by reference.  Instead, the flags are used 
to communicate downwards to the callees, and the expectation needs to be that 
callees will push a destructor when they're done initializing unless 
`isExternallyDestructed` is set on the dest slot they receive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66094



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

[clang] 2b17438 - [Index/USRGeneration] Make sure that ObjC properties in categories also get namescoped properly for USR generation

2020-03-07 Thread Argyrios Kyrtzidis via cfe-commits


Author: Argyrios Kyrtzidis
Date: 2020-03-07T15:07:37-08:00
New Revision: 2b17438a92ea1ea178d9e14219a8e6ba01d4f04d

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

LOG: [Index/USRGeneration] Make sure that ObjC properties in categories also 
get namescoped properly for USR generation

If the property is in a category that has module names from 
external_declaration property, make sure they are included in the USR.

rdar://59897320

Added: 


Modified: 
clang/lib/Index/USRGeneration.cpp
clang/test/Index/Core/external-source-symbol-attr.m

Removed: 




diff  --git a/clang/lib/Index/USRGeneration.cpp 
b/clang/lib/Index/USRGeneration.cpp
index 394daf94c4b2..f3eb653f10fa 100644
--- a/clang/lib/Index/USRGeneration.cpp
+++ b/clang/lib/Index/USRGeneration.cpp
@@ -382,6 +382,14 @@ void USRGenerator::VisitNamespaceAliasDecl(const 
NamespaceAliasDecl *D) {
 Out << "@NA@" << D->getName();
 }
 
+static const ObjCCategoryDecl *getCategoryContext(const NamedDecl *D) {
+  if (auto *CD = dyn_cast(D->getDeclContext()))
+return CD;
+  if (auto *ICD = dyn_cast(D->getDeclContext()))
+return ICD->getCategoryDecl();
+  return nullptr;
+};
+
 void USRGenerator::VisitObjCMethodDecl(const ObjCMethodDecl *D) {
   const DeclContext *container = D->getDeclContext();
   if (const ObjCProtocolDecl *pd = dyn_cast(container)) {
@@ -395,14 +403,6 @@ void USRGenerator::VisitObjCMethodDecl(const 
ObjCMethodDecl *D) {
   IgnoreResults = true;
   return;
 }
-auto getCategoryContext = [](const ObjCMethodDecl *D) ->
-const ObjCCategoryDecl * {
-  if (auto *CD = dyn_cast(D->getDeclContext()))
-return CD;
-  if (auto *ICD = dyn_cast(D->getDeclContext()))
-return ICD->getCategoryDecl();
-  return nullptr;
-};
 auto *CD = getCategoryContext(D);
 VisitObjCContainerDecl(ID, CD);
   }
@@ -475,7 +475,7 @@ void USRGenerator::VisitObjCPropertyDecl(const 
ObjCPropertyDecl *D) {
   // The USR for a property declared in a class extension or category is based
   // on the ObjCInterfaceDecl, not the ObjCCategoryDecl.
   if (const ObjCInterfaceDecl *ID = Context->getObjContainingInterface(D))
-Visit(ID);
+VisitObjCContainerDecl(ID, getCategoryContext(D));
   else
 Visit(cast(D->getDeclContext()));
   GenObjCProperty(D->getName(), D->isClassProperty());

diff  --git a/clang/test/Index/Core/external-source-symbol-attr.m 
b/clang/test/Index/Core/external-source-symbol-attr.m
index 41bb7a264ab3..d2cef35ffab2 100644
--- a/clang/test/Index/Core/external-source-symbol-attr.m
+++ b/clang/test/Index/Core/external-source-symbol-attr.m
@@ -21,6 +21,10 @@ @interface I2
 // CHECK: [[@LINE-1]]:12 | class/Swift | I2 | c:@M@some_module@objc(cs)I2 | 
{{.*}} | Decl | rel: 0
 -(void)method;
 // CHECK: [[@LINE-1]]:8 | instance-method/Swift | method | 
c:@M@some_module@objc(cs)I2(im)method | -[I2 method] | Decl,Dyn,RelChild | rel: 
1
+@property int prop;
+// CHECK: [[@LINE-1]]:15 | instance-method/acc-get/Swift | prop | 
c:@M@some_module@objc(cs)I2(im)prop |
+// CHECK: [[@LINE-2]]:15 | instance-method/acc-set/Swift | setProp: | 
c:@M@some_module@objc(cs)I2(im)setProp: |
+// CHECK: [[@LINE-3]]:15 | instance-property/Swift | prop | 
c:@M@some_module@objc(cs)I2(py)prop |
 @end
 
 void test1(I1 *o) {
@@ -45,6 +49,10 @@ @interface I1(cat2)
 // CHECK: [[@LINE-1]]:15 | extension/Swift | cat2 | 
c:@CM@cat_module@some_module@objc(cy)I1@cat2 |
 -(void)cat_method2;
 // CHECK: [[@LINE-1]]:8 | instance-method/Swift | cat_method2 | 
c:@CM@cat_module@some_module@objc(cs)I1(im)cat_method2
+@property int cat_prop2;
+// CHECK: [[@LINE-1]]:15 | instance-method/acc-get/Swift | cat_prop2 | 
c:@CM@cat_module@some_module@objc(cs)I1(im)cat_prop2 |
+// CHECK: [[@LINE-2]]:15 | instance-method/acc-set/Swift | setCat_prop2: | 
c:@CM@cat_module@some_module@objc(cs)I1(im)setCat_prop2: |
+// CHECK: [[@LINE-3]]:15 | instance-property/Swift | cat_prop2 | 
c:@CM@cat_module@some_module@objc(cs)I1(py)cat_prop2 |
 @end
 
 #define NS_ENUM(_name, _type) enum _name:_type _name; enum _name : _type



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

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

2020-03-07 Thread Nathan Chancellor via Phabricator via cfe-commits

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

Thanks for the review and accepting. I do not have commit rights, would you 
mind doing that on my behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758



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

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

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

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBlocks.cpp:869
+  if (auto *BD = C.dyn_cast())
+enterBlockScope(*this, BD);
   }

I wonder if we could just switch blocks to the same thing.



Comment at: clang/lib/Sema/SemaExpr.cpp:6271
+getCurFunction()->setHasBranchProtectedScope();
+  }
+

This should all be conditional on C++ (but you should leave a comment 
explaining why).



Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:1729
+  Record.push_back(serialization::COK_CompoundLiteral);
+  Record.AddStmt(CLE);
+}

Will this just serialize a second copy of the compound literal expression?



Comment at: lib/CodeGen/CGExpr.cpp:4100
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType());
+

ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > ahatanak wrote:
> > > > > rjmccall wrote:
> > > > > > rjmccall wrote:
> > > > > > > Unfortunately, the lifetime of compound literals in C is not this 
> > > > > > > simple; they're like blocks in that they're destroyed at the end 
> > > > > > > of the enclosing scope rather than at the end of the current 
> > > > > > > statement. (The cleanup here will be popped at the end of the 
> > > > > > > full-expression if we've entered an `ExprWithCleanups`.) And the 
> > > > > > > l-value case is exactly the case where this matters.
> > > > > > > 
> > > > > > > I think you need to do something like what we do with blocks, 
> > > > > > > where we record all the blocks in the full-expression on the 
> > > > > > > `ExprWithCleanups` so that we can push an inactive cleanup for 
> > > > > > > them and then activate it when we emit the block.
> > > > > > Can we make the check here something like (1) this is a block-scope 
> > > > > > compound literal and (2) it has a non-trivially-destructed type (of 
> > > > > > any kind)?  That way we're not conflating two potentially unrelated 
> > > > > > elements, the lifetime of the object and the kinds of types that 
> > > > > > can be constructed by the literal.
> > > > > > 
> > > > > > Oh, actually, there's a concrete reason to do this: C99 compound 
> > > > > > literals are not required to have struct type; they can have any 
> > > > > > object type, including arrays but also scalars.  So we could, even 
> > > > > > without non-trivial C structs, have a block-scope compound of type 
> > > > > > `__strong id[]`; I guess we've always just gotten this wrong.  
> > > > > > Please add tests for this case. :)
> > > > > There is a check `E->isFileScope()` above this. Is that sufficient to 
> > > > > check for block-scoped compound literals?
> > > > That plus the C/C++ difference; compound literals in C++ are just 
> > > > temporaries.
> > > I haven't been able to come up with a piece of C++ code that executes 
> > > `EmitCompoundLiteralLValue`. The following code gets rejected because you 
> > > can't take the address of a temporary object in C++:
> > > 
> > > ```
> > > StrongSmall *p = &(StrongSmall){ 1, 0 };
> > > ```
> > > 
> > > If a bind a reference to it, `AggExprEmitter::VisitCompoundLiteralExpr` 
> > > is called.
> > That makes sense; they're not gl-values in C++.  It would be reasonable to 
> > assert that.  But the C++ point does apply elsewhere.
> It turns out this function is called in C++ when the compound literal is a 
> vector type, so I've just added a check for C++ instead of an assert.
Really?  Is the expression actually an l-value in this case somehow?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464



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

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

2020-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/AST/Expr.cpp:693
 else
-  MC->mangleName(ND, Out);
+  GD = GlobalDecl(ND);
+MC->mangleName(GD, Out);

rjmccall wrote:
> This will need an extension for your case, right?  Maybe there should be 
> comments in `GlobalDecl` pointing out all the places where we need to go from 
> an arbitrary `Decl*` to a `GlobalDecl` so that someone adding a new kind of 
> discriminated declaration will know to update them all.
Added extension for kernel reference here and other places in the other patch. 
Also added comments to GlobalDecl.



Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+GD = GlobalDecl(dyn_cast(DC));
+  return GD;

rjmccall wrote:
> `cast`?  But I'm not sure this is true, local entities can be in non-function 
> declarations: blocks, ObjC methods, and captured statements.  You can just 
> `cast(DC)`.
Original code is `cast(DC)` since `ObjCMethodDecl` and 
`BlockDecl` are handled separately. Since we dot have GlobalDecl ctor for 
Decl*, I will cast to FunctionDecl.



Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+GD = GlobalDecl(dyn_cast(DC));
+  return GD;

rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > `cast`?  But I'm not sure this is true, local entities can be in 
> > > non-function declarations: blocks, ObjC methods, and captured statements. 
> > >  You can just `cast(DC)`.
> > Original code is `cast(DC)` since `ObjCMethodDecl` and 
> > `BlockDecl` are handled separately. Since we dot have GlobalDecl ctor for 
> > Decl*, I will cast to FunctionDecl.
> I guess we just never enter this for local names within ObjC methods or 
> blocks?
Yes.


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

https://reviews.llvm.org/D75700



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

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

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

LGTM aside from the issue with keeping the comment up to date from Erich (but 
please only land this once D75779  is 
approved, as that's what carries the testing load for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

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

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

Thank you for working on this, this LGTM! If you wanted a follow-up patch 
beyond adding semantic support for the `inline` keyword, I think it might make 
sense to investigate divorcing the qualifier parsing from the `DeclSpec` 
interface. These are ASM statements, not declarations, so the fact that we're 
using a DeclSpec to smuggle the qualifiers around is a bit unexpected. However, 
I don't think that work needs to hold up this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D75701: Initialize IsSurrogate

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5704f92b835: [Sema] Initialize IsSurrogate (authored by 
espindola, committed by MaskRay).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75701

Files:
  clang/include/clang/Sema/Overload.h


Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -908,7 +908,7 @@
   private:
 friend class OverloadCandidateSet;
 OverloadCandidate()
-: IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), 
RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++


Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -908,7 +908,7 @@
   private:
 friend class OverloadCandidateSet;
 OverloadCandidate()
-: IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75701: Initialize IsSurrogate

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

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

Confirmed that the patch fixes `valgrind clang -cc1 -S -std=c++17 -o 
native-stack.s native-stack.ii` for a -O0 -g Debug build.

`-DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=on`  does not have the 
`Conditional jump or move depends on uninitialised value` issue.


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

https://reviews.llvm.org/D75701



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

[PATCH] D75803: [clang-tidy] [NFC] Remove unnecessary matchers

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

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75803



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

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

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

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75800



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

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

rjmccall added a comment.

I'm fine with people developing a proposal for this openly, but it needs to be 
said that language changes cannot just be made in open-source; they have to go 
through the official language review process, which for Objective-C is an 
internal committee within Apple.

The summary calls this `objc_direct_protocol`, but it's `objc_static_protocol` 
in the patch.  I agree that "direct" isn't a great name for this.  `static` is 
complicated, because while we use "static" vs. "dynamic" this way when we're 
talking *about* languages, I'm not sure any of the umpteen language uses of 
`static` ever use it in exactly this way, and it's possibly quite confusing to 
add one.  Throwing  out other names here: `objc_non_runtime_protocol`? 
`objc_compiler_only_protocol`?

The technical content of the patch seems fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

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

rjmccall added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:1470
+/// diagnostics should be emitted.
+SmallVector DeclsToCheckForDeferredDiags;
+

This needs to be saved and restored in modules / PCH.



Comment at: clang/lib/Sema/Sema.cpp:1472
   if (HasWarningOrError && ShowCallStack)
-emitCallStackNotes(S, FD);
+emitCallStackNotes(*this, FD);
+}

Hmm.  I know this is existing code, but I just realized something.  I think 
it's okay to not emit the notes on every diagnostic, but you might want to emit 
them on the first diagnostic from a function instead of after the last.  If the 
real bug is that the program is using something it's not supposed to use, and 
there are enough errors in that function to reach the error limit, then the 
diagnostics emitter will associate these notes with a diagnostic it's 
suppressing and so presumably suppress them as well, leaving the user with no 
way to find this information.



Comment at: clang/lib/Sema/Sema.cpp:1494
+  visitUsedDecl(E->getLocation(), FD);
+}
+  }

This needs to trigger if you use a variable with delayed diagnostics, too, 
right?

When you add these methods to `UsedDeclVisitor`, you'll be able to remove them 
here.



Comment at: clang/lib/Sema/Sema.cpp:1512
+Inherited::VisitCapturedStmt(Node);
+  }
+

Should this also go in the base `UsedDeclVisitor`?  I'm less sure about that 
because the captured statement is really always a part of the enclosing 
function, right?  Should the delay mechanism just be looking through local 
sub-contexts instead?



Comment at: clang/lib/Sema/UsedDeclVisitor.h:21
+template 
+class UsedDeclVisitor : public EvaluatedExprVisitor {
+protected:

Could you add this in a separate patch?



Comment at: clang/lib/Sema/UsedDeclVisitor.h:30
+
+  Derived () { return *static_cast(this); }
+

There should definitely be cases in here for every expression that uses a 
declaration, including both `DeclRefExpr` and `MemberExpr`.  Those might be 
overridden in subclasses, but that's their business; the default behavior 
should be to visit every used decl.


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

https://reviews.llvm.org/D70172



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

[clang] a5704f9 - [Sema] Initialize IsSurrogate

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


Author: Rafael Ávila de Espíndola
Date: 2020-03-07T12:24:35-08:00
New Revision: a5704f92b835d1810d83a223f70dfe6c92a34c03

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

LOG: [Sema] Initialize IsSurrogate

This fixes https://bugs.llvm.org/show_bug.cgi?id=45096

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

Added: 


Modified: 
clang/include/clang/Sema/Overload.h

Removed: 




diff  --git a/clang/include/clang/Sema/Overload.h 
b/clang/include/clang/Sema/Overload.h
index f1a8b98e5efd..6944b0b5756e 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -908,7 +908,7 @@ class Sema;
   private:
 friend class OverloadCandidateSet;
 OverloadCandidate()
-: IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), 
RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++



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

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

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

rjmccall added a comment.

In D72841#1908084 , @mibintc wrote:

> @rjmccall suggested that I needed to remove FPOptions from the Stmt class 
> since the sizeof assertion failed. I moved that information into the relevant 
> expression nodes and fixed a few small things that I didn't like in the 
> previous version.


You only need to do this for the expression nodes where it causes an overflow 
in the size of the bit-field; I don't think you're overflowing the capacity of 
UnaryOperatorBitfields, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

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

tambre updated this revision to Diff 248949.
tambre added a comment.

Add missing error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75811

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/Cuda.h

Index: clang/lib/Driver/ToolChains/Cuda.h
===
--- clang/lib/Driver/ToolChains/Cuda.h
+++ clang/lib/Driver/ToolChains/Cuda.h
@@ -188,6 +188,7 @@
  const llvm::opt::ArgList ) const override;
 
   unsigned GetDefaultDwarfVersion() const override { return 2; }
+  CudaVersion getCudaVersion() const;
 
   const ToolChain 
   CudaInstallationDetector CudaInstallation;
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -884,3 +884,7 @@
const ArgList ) const {
   return HostTC.computeMSVCVersion(D, Args);
 }
+
+CudaVersion CudaToolChain::getCudaVersion() const {
+  return CudaInstallation.version();
+}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -634,12 +634,17 @@
 llvm::Triple CudaTriple(DeviceTripleStr);
 // Use the CUDA and host triples as the key into the ToolChains map,
 // because the device toolchain we create depends on both.
-auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
-if (!CudaTC) {
-  CudaTC = std::make_unique(
-  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
+if (!Toolchain) {
+  std::unique_ptr CudaTC =
+  std::make_unique(
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+  C.getArgs().AddJoinedArg(
+  nullptr, getOpts().getOption(options::OPT_cuda_version_EQ),
+  CudaVersionToString(CudaTC->getCudaVersion()));
+  Toolchain = std::move(CudaTC);
 }
-C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
+C.addOffloadDeviceToolChain(Toolchain.get(), OFK);
   } else if (IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
@@ -2566,7 +2571,16 @@
 CudaActionBuilder(Compilation , DerivedArgList ,
   const Driver::InputList )
 : CudaActionBuilderBase(C, Args, Inputs, Action::OFK_Cuda) {
-  DefaultCudaArch = CudaArch::SM_20;
+  StringRef VersionString =
+  Args.getLastArgValue(options::OPT_cuda_version_EQ);
+  CudaVersion Version = CudaStringToVersion(VersionString);
+
+  if (Version == CudaVersion::UNKNOWN) {
+C.getDriver().Diag(clang::diag::err_drv_cuda_unknown_version)
+<< VersionString;
+  }
+
+  DefaultCudaArch = MinArchForCudaVersion(Version);
 }
 
 ActionBuilderReturnCode
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -362,6 +362,14 @@
   }
 }
 
+CudaArch MinArchForCudaVersion(CudaVersion V) {
+  if (V >= CudaVersion::CUDA_90) {
+return CudaArch::SM_30;
+  } else {
+return CudaArch::SM_20;
+  }
+}
+
 static CudaVersion ToCudaVersion(llvm::VersionTuple Version) {
   int IVer =
   Version.getMajor() * 10 + Version.getMinor().getValueOr(0);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -563,6 +563,9 @@
 def cuda_compile_host_device : Flag<["--"], "cuda-compile-host-device">,
   HelpText<"Compile CUDA code for both host and device (default).  Has no "
"effect on non-CUDA compilations.">;
+def cuda_version_EQ : Joined<["--"], "cuda-version=">, Flags<[DriverOption]>,
+  MetaVarName<"">, Values<".">,
+  HelpText<"Used to choose default architecture if no other options are given.">;
 def cuda_include_ptx_EQ : Joined<["--"], "cuda-include-ptx=">, Flags<[DriverOption]>,
   HelpText<"Include PTX for the following GPU architecture (e.g. sm_35) or 'all'. May be specified more than once.">;
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, Flags<[DriverOption]>,
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -64,6 +64,7 @@
   "Unknown CUDA version %0. Assuming the latest supported version

[PATCH] D58164: Block+lambda: allow reference capture

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

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

Okay, I think this is a reasonable fix.   If @rsmith has thoughts about this, I 
guess they'll have to happen post-review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58164



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

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

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

rjmccall accepted this revision.
rjmccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/AST/ItaniumMangle.cpp:1563
+  else
+GD = GlobalDecl(dyn_cast(DC));
+  return GD;

rjmccall wrote:
> `cast`?  But I'm not sure this is true, local entities can be in non-function 
> declarations: blocks, ObjC methods, and captured statements.  You can just 
> `cast(DC)`.
I guess we just never enter this for local names within ObjC methods or blocks?


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

https://reviews.llvm.org/D75700



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

[clang] 118b057 - [SYCL] Driver option to select SYCL version

2020-03-07 Thread Alexey Bader via cfe-commits


Author: Ruyman
Date: 2020-03-07T18:28:54+03:00
New Revision: 118b057f1268d1789e40ffceb214e73772df04f4

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

LOG: [SYCL] Driver option to select SYCL version

Summary:
User can select the version of SYCL the compiler will
use via the flag -sycl-std, similar to -cl-std.

The flag defines the LangOpts.SYCLVersion option to the
version of SYCL. The default value is undefined.
If driver is building SYCL code, flag is set to the default SYCL
version (1.2.1)

The preprocessor uses this variable to define CL_SYCL_LANGUAGE_VERSION macro,
which should be defined according to SYCL 1.2.1 standard.

Only valid value at this point for the flag is 1.2.1.

Co-Authored-By: David Wood 
Signed-off-by: Ruyman Reyes 

Subscribers: ebevhan, Anastasia, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Frontend/InitPreprocessor.cpp
clang/test/Driver/sycl.c
clang/test/Frontend/sycl-aux-triple.cpp
clang/test/Preprocessor/sycl-macro.cpp
clang/test/SemaSYCL/kernel-attribute.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 3cc7c384ac10..53b87b737568 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -230,7 +230,9 @@ LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate 
relocatable device code")
 LANGOPT(GPUAllowDeviceInit, 1, 0, "allowing device side global init functions 
for HIP")
 LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
kernel launch bounds for HIP")
 
+LANGOPT(SYCL  , 1, 0, "SYCL")
 LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
+LANGOPT(SYCLVersion   , 32, 0, "Version of the SYCL standard used")
 
 LANGOPT(HIPUseNewLaunchAPI, 1, 0, "Use new kernel launching API for HIP")
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 2aaf85434214..0d5cba8d682a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3422,10 +3422,12 @@ defm underscoring : BooleanFFlag<"underscoring">, 
Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
 // C++ SYCL options
-def fsycl : Flag<["-"], "fsycl">, Group,
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[CC1Option, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
-def fno_sycl : Flag<["-"], "fno-sycl">, Group,
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, Flags<[CoreOption]>,
   HelpText<"Disable SYCL kernels compilation for device">;
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, 
Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"2017, 121, 
1.2.1, sycl-1.2.1">;
 
 include "CC1Options.td"
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 3ca034e69b3b..99faae396be3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4048,9 +4048,18 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
-  if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false))
+  if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false)) {
+CmdArgs.push_back("-fsycl");
 CmdArgs.push_back("-fsycl-is-device");
 
+if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
+  A->render(Args, CmdArgs);
+} else {
+  // Ensure the default version in SYCL mode is 1.2.1 (aka 2017)
+  CmdArgs.push_back("-sycl-std=2017");
+}
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP 
device.
 std::string NormalizedTriple =

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 48c65aded817..9f3522a3e654 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,24 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
   LangStd = OpenCLLangStd;
   }
 
+  Opts.SYCL = Args.hasArg(options::OPT_fsycl);
+  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL) {
+// -sycl-std applies to any SYCL source, not only those containing kernels,
+// but also those using the SYCL API
+if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+  Opts.SYCLVersion =

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

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

jdoerfert added a comment.

There will be an update later today or tomorrow, I am making the actual target 
offloading CUDA interoperability "work" right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75779



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

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

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

tambre added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:642
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+  C.getArgs().AddJoinedArg(
+  nullptr, getOpts().getOption(options::OPT_cuda_version_EQ),

This isn't very pretty. Any better ideas for how to pass the current CUDA 
version or default arch to `CudaActionBuilder`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75811



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

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

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

tambre created this revision.
tambre added reviewers: tra, jlebar.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
tambre added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:642
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+  C.getArgs().AddJoinedArg(
+  nullptr, getOpts().getOption(options::OPT_cuda_version_EQ),

This isn't very pretty. Any better ideas for how to pass the current CUDA 
version or default arch to `CudaActionBuilder`?


Currently always defaults to sm_20.
However, CUDA >=9.0 doesn't support the sm_20 architecture.
Choose the minimum architecture the CUDA installation supports as the default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75811

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/Cuda.h

Index: clang/lib/Driver/ToolChains/Cuda.h
===
--- clang/lib/Driver/ToolChains/Cuda.h
+++ clang/lib/Driver/ToolChains/Cuda.h
@@ -188,6 +188,7 @@
  const llvm::opt::ArgList ) const override;
 
   unsigned GetDefaultDwarfVersion() const override { return 2; }
+  CudaVersion getCudaVersion() const;
 
   const ToolChain 
   CudaInstallationDetector CudaInstallation;
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -884,3 +884,7 @@
const ArgList ) const {
   return HostTC.computeMSVCVersion(D, Args);
 }
+
+CudaVersion CudaToolChain::getCudaVersion() const {
+  return CudaInstallation.version();
+}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -634,12 +634,17 @@
 llvm::Triple CudaTriple(DeviceTripleStr);
 // Use the CUDA and host triples as the key into the ToolChains map,
 // because the device toolchain we create depends on both.
-auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
-if (!CudaTC) {
-  CudaTC = std::make_unique(
-  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
+if (!Toolchain) {
+  std::unique_ptr CudaTC =
+  std::make_unique(
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+  C.getArgs().AddJoinedArg(
+  nullptr, getOpts().getOption(options::OPT_cuda_version_EQ),
+  CudaVersionToString(CudaTC->getCudaVersion()));
+  Toolchain = std::move(CudaTC);
 }
-C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
+C.addOffloadDeviceToolChain(Toolchain.get(), OFK);
   } else if (IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
@@ -2566,7 +2571,16 @@
 CudaActionBuilder(Compilation , DerivedArgList ,
   const Driver::InputList )
 : CudaActionBuilderBase(C, Args, Inputs, Action::OFK_Cuda) {
-  DefaultCudaArch = CudaArch::SM_20;
+  StringRef VersionString =
+  Args.getLastArgValue(options::OPT_cuda_version_EQ);
+  CudaVersion Version = CudaStringToVersion(VersionString);
+
+  if (Version == CudaVersion::UNKNOWN) {
+C.getDriver().Diag(clang::diag::err_drv_cuda_unknown_version)
+<< VersionString;
+  }
+
+  DefaultCudaArch = MinArchForCudaVersion(Version);
 }
 
 ActionBuilderReturnCode
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -362,6 +362,14 @@
   }
 }
 
+CudaArch MinArchForCudaVersion(CudaVersion V) {
+  if (V >= CudaVersion::CUDA_90) {
+return CudaArch::SM_30;
+  } else {
+return CudaArch::SM_20;
+  }
+}
+
 static CudaVersion ToCudaVersion(llvm::VersionTuple Version) {
   int IVer =
   Version.getMajor() * 10 + Version.getMinor().getValueOr(0);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -563,6 +563,9 @@
 def cuda_compile_host_device : Flag<["--"], "cuda-compile-host-device">,
   HelpText<"Compile CUDA code for both host and device (default).  Has no "
"effect on non-CUDA compilations.">;
+def cuda_version_EQ : Joined<["--"], "cuda-version=">, Flags<[DriverOption]>,
+  MetaVarName<"">, Values<".">,
+  HelpText<"Used to choose default architecture if no other options are given.">;
 def cuda_include_ptx_EQ : Joined<["--"], "cuda-include-ptx=">, Flags<[DriverOption]>,
   HelpText<"Include PTX for

[clang] a4e71f0 - Assume ieee behavior without denormal-fp-math attribute

2020-03-07 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2020-03-07T12:10:56-05:00
New Revision: a4e71f01c08fbaeaccfe3e11cc08790432cc7e45

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

LOG: Assume ieee behavior without denormal-fp-math attribute

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/denormalfpmode.c
clang/test/CodeGenCUDA/flush-denormals.cu
clang/test/CodeGenCUDA/propagate-metadata.cu
clang/test/Driver/default-denormal-fp-math.c
clang/test/Driver/denormal-fp-math.c
llvm/lib/CodeGen/MachineFunction.cpp
llvm/test/CodeGen/X86/pow.ll
llvm/test/CodeGen/X86/sqrt-fastmath-mir.ll
llvm/test/CodeGen/X86/sqrt-fastmath.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 42d5467c63dc..1188ea39ba2c 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1748,11 +1748,10 @@ void 
CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
 if (CodeGenOpts.NullPointerIsValid)
   FuncAttrs.addAttribute("null-pointer-is-valid", "true");
 
-// TODO: Omit attribute when the default is IEEE.
-if (CodeGenOpts.FPDenormalMode.isValid())
+if (CodeGenOpts.FPDenormalMode != llvm::DenormalMode::getIEEE())
   FuncAttrs.addAttribute("denormal-fp-math",
  CodeGenOpts.FPDenormalMode.str());
-if (CodeGenOpts.FP32DenormalMode.isValid()) {
+if (CodeGenOpts.FP32DenormalMode != CodeGenOpts.FPDenormalMode) {
   FuncAttrs.addAttribute(
   "denormal-fp-math-f32",
   CodeGenOpts.FP32DenormalMode.str());

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a998561a218b..3ca034e69b3b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2824,8 +2824,8 @@ static void RenderFloatingPointOptions(const ToolChain 
, const Driver ,
   } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");
 
-  // TODO: Omit flag for the default IEEE instead
-  if (DenormalFPMath.isValid()) {
+  // The default is IEEE.
+  if (DenormalFPMath != llvm::DenormalMode::getIEEE()) {
 llvm::SmallString<64> DenormFlag;
 llvm::raw_svector_ostream ArgStr(DenormFlag);
 ArgStr << "-fdenormal-fp-math=" << DenormalFPMath;

diff  --git a/clang/test/CodeGen/denormalfpmode.c 
b/clang/test/CodeGen/denormalfpmode.c
index 3b9ad0d7273b..7fd2b0b42e9d 100644
--- a/clang/test/CodeGen/denormalfpmode.c
+++ b/clang/test/CodeGen/denormalfpmode.c
@@ -3,7 +3,9 @@
 // RUN: %clang_cc1 -S -fdenormal-fp-math=positive-zero %s -emit-llvm -o - | 
FileCheck %s --check-prefix=CHECK-PZ
 
 // CHECK-LABEL: main
-// CHECK-IEEE: attributes #0 = {{.*}}"denormal-fp-math"="ieee,ieee"{{.*}}
+
+// The ieee,ieee is the default, so omit the attribute
+// CHECK-IEEE-NOT:"denormal-fp-math"
 // CHECK-PS: attributes #0 = 
{{.*}}"denormal-fp-math"="preserve-sign,preserve-sign"{{.*}}
 // CHECK-PZ: attributes #0 = 
{{.*}}"denormal-fp-math"="positive-zero,positive-zero"{{.*}}
 

diff  --git a/clang/test/CodeGenCUDA/flush-denormals.cu 
b/clang/test/CodeGenCUDA/flush-denormals.cu
index 275338635bc6..8577fca92866 100644
--- a/clang/test/CodeGenCUDA/flush-denormals.cu
+++ b/clang/test/CodeGenCUDA/flush-denormals.cu
@@ -39,7 +39,7 @@
 extern "C" __device__ void foo() {}
 
 // FTZ: attributes #0 = {{.*}} 
"denormal-fp-math-f32"="preserve-sign,preserve-sign"
-// NOFTZ: attributes #0 = {{.*}} "denormal-fp-math-f32"="ieee,ieee"
+// NOFTZ-NOT: "denormal-fp-math-f32"
 
 // AMDNOFTZ: attributes #0 = {{.*}}+fp32-denormals{{.*}}+fp64-fp16-denormals
 // AMDFTZ: attributes #0 = {{.*}}+fp64-fp16-denormals{{.*}}-fp32-denormals

diff  --git a/clang/test/CodeGenCUDA/propagate-metadata.cu 
b/clang/test/CodeGenCUDA/propagate-metadata.cu
index 2514eeb55d20..a8cabe6b6ced 100644
--- a/clang/test/CodeGenCUDA/propagate-metadata.cu
+++ b/clang/test/CodeGenCUDA/propagate-metadata.cu
@@ -60,9 +60,9 @@ __global__ void kernel() { lib_fn(); }
 // CHECK-SAME: convergent
 // CHECK-SAME: norecurse
 
-// FTZ: "denormal-fp-math"="ieee,ieee"
+// FTZ-NOT: "denormal-fp-math"
 // FTZ-SAME: "denormal-fp-math-f32"="preserve-sign,preserve-sign"
-// NOFTZ-SAME: "denormal-fp-math-f32"="ieee,ieee"
+// NOFTZ-NOT: "denormal-fp-math-f32"
 
 // CHECK-SAME: "no-trapping-math"="true"
 
@@ -75,11 +75,11 @@ __global__ void kernel() { lib_fn(); }
 // CHECK-SAME: convergent
 // CHECK-NOT: norecurse
 
-// FTZ-SAME: "denormal-fp-math"="ieee,ieee"
-// NOFTZ-SAME: "denormal-fp-math"="ieee,ieee"
+// FTZ-NOT: "denormal-fp-math"
+// NOFTZ-NOT: "denormal-fp-math"
 
 // FTZ-SAME: "denormal-fp-math-f32"="preserve-sign,preserve-sign"
-// NOFTZ-SAME: "denormal-fp-math-f32"="ieee,ieee"
+// NOFTZ-NOT:

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

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

jdoerfert added a comment.

In D75209#1910047 , @ABataev wrote:

> In D75209#1909976 , @jdoerfert wrote:
>
> > I am unsure we need this with the proper math support. Sema patch for that 
> > is going for review today. I'll try this out soon.
>
>
> It has nothing to do with the math functions support. These functions are 
> required for the definition of __muldc3/__divdc3/__mulsc2/__divsc3 functions 
> emitted for the complex types. CUDA does absolutely the same.


The fact that CUDA does the same is the important part here. Why should we copy 
it then? See https://reviews.llvm.org/D75788#change-0Ax9LSfw7OAk for working 
support of complex math functions in target regions without copying what cuda 
does but by just using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75209



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

[PATCH] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

2020-03-07 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:511
+  //
+  // Also, we cannot model the parameters. CXXInheritedCtorInitExpr doesn't
+  // take arguments and doesn't model parameter initialization because there is

I'd rather put this Richard's comment somewhere near the respective `CallEvent` 
definition. We clearly don't need to analyze these functions, so it doesn't 
really matter for anybody who reads this code that there are temporary 
technical difficulties with analyzing them. On the other hand, it does matter a 
lot for people who try to understand how to implement the call event correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75678



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

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

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

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

Add complex test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_libdevice_declares.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/new
  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/test/OpenMP/target_nvptx_math_complex.c
  clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
  clang/test/OpenMP/target_nvptx_math_sin.c

Index: clang/test/OpenMP/target_nvptx_math_sin.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_sin.c
@@ -0,0 +1,22 @@
+// 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 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+double math(short s, int i, float f, double d, double ld) {
+  double r = 0;
+  r += sin(s);
+  r += sin(i);
+  r += sin(f);
+  r += sin(d);
+  return r;
+}
+
+long double foo(short s, int i, float f, double d, long double ld) {
+  double r = sin(ld);
+  r += math(s, i, f, d, ld);
+#pragma omp target map(r)
+  { r += math(s, i, f, d, ld); }
+  return r;
+}
Index: clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_fp_macro.cpp
@@ -0,0 +1,10 @@
+// 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 
+// TODO: How to include a "mock systme cmath" here for testing?
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/target_nvptx_math_complex.c
===
--- /dev/null
+++ clang/test/OpenMP/target_nvptx_math_complex.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+
+// CHECK-DAG: call { float, float } @__divsc3(
+// CHECK-DAG: call { float, float } @__mulsc3(
+void test_scmplx(float _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
+
+
+// CHECK-DAG: call { double, double } @__divdc3(
+// CHECK-DAG: call { double, double } @__muldc3(
+void test_dcmplx(double _Complex a) {
+#pragma omp target
+  {
+(void)(a * (a / a));
+  }
+}
Index: clang/lib/Headers/openmp_wrappers/math.h
===
--- clang/lib/Headers/openmp_wrappers/math.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*===- math.h - Alternative math.h header --===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
-
Index: clang/lib/Headers/openmp_wrappers/cmath
===
--- clang/lib/Headers/openmp_wrappers/cmath
+++ /dev/null
@@ -1,16 +0,0 @@
-/*===-- cmath - Alternative cmath header ---===
- *
- * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- * See https://llvm.org/LICENSE.txt for license information.
- * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
- *
- *===---===
- */
-
-#include <__clang_openmp_math.h>
-
-#ifndef __CLANG_NO_HOST_MATH__
-#include_next 
-#else
-#undef __CLANG_NO_HOST_MATH__
-#endif
Index: clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
===
--- clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
+++ /dev/null
@@ -1,33 +0,0

[PATCH] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

2020-03-07 Thread Artem Dergachev via Phabricator via cfe-commits

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

Looks great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75678



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-07 Thread Marcus Johnson via Phabricator via cfe-commits

MarcusJohnson91 added a comment.

In D75791#1911133 , @MyDeveloperDay 
wrote:

> you need documentation and release note changes too


The comments were only for testing, I'll remove them.

The tests had to change because the behavior has changed slightly.

In practice it should be the same because LLVMStyle.IndentExternBlock default 
is set to false, but previously the BraceWrapping.AfterExternBlock = true code 
would indent as well, and now the behavior of BraceWrapping.AfterExternBlock 
only effects the brace wrapping.

As for the release notes, which file should I edit for that, and also which 
version will this even end up in? probably 11 right, because 10 is in RC status 
right now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75791



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

Re: [clang] d052a57 - [c++2a] Allow comparison functions to be explicitly defaulted.

2020-03-07 Thread Hubert Tong via cfe-commits

Following this commit, the error recovery for invalid cases that explicitly
define (out-of-line) a member function template as deleted and attempts to
instantiate said function appears broken.

:4:35: error: deleted definition must be first declaration
template  void A::f() = delete;
  ^
:2:35: note: previous declaration is here
  template  static void f();
  ^
clang:
/src_d052a578de58cbbb638cbe2dba05242d1ff443b9/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4288:
void clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation,
clang::FunctionDecl *, bool, bool, bool): Assertion `(Pattern ||
PatternDecl->isDefaulted() || PatternDecl->hasSkippedBody()) && "unexpected
kind of function template definition"' failed.
Stack dump:
0.  Program arguments:
/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/clang -cc1 -std=c++11
-xc++ -
1.  :5:26: current parser token ';'
 #0 0x3fff7fe6a024 PrintStackTraceSignalHandler(void*)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/libLLVMSupport.so.10svn+0x1ea024)
 #1 0x3fff7fe670c8 llvm::sys::RunSignalHandlers()
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/libLLVMSupport.so.10svn+0x1e70c8)
 #2 0x3fff7fe6a49c SignalHandler(int)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/libLLVMSupport.so.10svn+0x1ea49c)
 #3 0x3fff82030478  0x478 abort
 #4 0x3fff82030478
 #5 0x3fff82030478 __assert_fail_base (+0x478)
 #6 0x3fff7e0a1f94 __assert_fail (/lib64/libc.so.6+0x41f94)
 #7 0x3fff7e0955d4
clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation,
clang::FunctionDecl*, bool, bool, bool) (/lib64/libc.so.6+0x355d4)
 #8 0x3fff7e0956c4
clang::Sema::ActOnExplicitInstantiation(clang::Scope*,
clang::SourceLocation, clang::SourceLocation, clang::Declarator&)
(/lib64/libc.so.6+0x356c4)
 #9 0x3fff7c28d604
clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&,
clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangSema.so.10svn+0x8ad604)
#10 0x3fff7c15c2b0
clang::Parser::ParseDeclarationAfterDeclarator(clang::Declarator&,
clang::Parser::ParsedTemplateInfo const&)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangSema.so.10svn+0x77c2b0)
#11 0x3fff7c4cc8f8
clang::Parser::ParseSingleDeclarationAfterTemplate(clang::DeclaratorContext,
clang::Parser::ParsedTemplateInfo const&, clang::ParsingDeclRAIIObject&,
clang::SourceLocation&, clang::ParsedAttributes&, clang::AccessSpecifier)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x4c8f8)
#12 0x3fff7c4cdf48
clang::Parser::ParseExplicitInstantiation(clang::DeclaratorContext,
clang::SourceLocation, clang::SourceLocation, clang::SourceLocation&,
clang::ParsedAttributes&, clang::AccessSpecifier)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x4df48)
#13 0x3fff7c57c1f0
clang::Parser::ParseDeclarationStartingWithTemplate(clang::DeclaratorContext,
clang::SourceLocation&, clang::ParsedAttributes&, clang::AccessSpecifier)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0xfc1f0)
#14 0x3fff7c57a6c0
clang::Parser::ParseDeclaration(clang::DeclaratorContext,
clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&,
clang::SourceLocation*)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0xfa6c0)
#15 0x3fff7c57a4f8
clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec*)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0xfa4f8)
#16 0x3fff7c4c5db0
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&,
bool)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x45db0)
#17 0x3fff7c58fffc clang::ParseAST(clang::Sema&, bool, bool)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x10fffc)
#18 0x3fff7c58def4 clang::ASTFrontendAction::ExecuteAction()
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x10def4)
#19 0x3fff7c4b01e0 clang::FrontendAction::Execute()
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/../lib/libclangParse.so.10svn+0x301e0)
#20 0x3fff7e93d57c
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/libclangFrontend.so.10svn+0x10d57c)
#21 0x3fff7e93cbf0
clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
(/build_d052a578de58cbbb638cbe2dba05242d1ff443b9/bin/../lib/libclangFrontend.so.10svn+0x10cbf0)
#22 0x3fff7e8d5bd4 cc1_main(llvm::ArrayRef, char const*,
void*)

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added a comment.

Anyway, I still don't see the point of this patch. It seems like you're just 
duplicating the work of `performance-move-const-arg`. People who want to be 
shown notes about moves-of-const-args should just enable that check instead.


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

https://reviews.llvm.org/D74692



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured 
const here
 };
   }

zinovy.nis wrote:
> aaron.ballman wrote:
> > One more test case to try out (it might be a FIXME because I imagine this 
> > requires flow control to get right):
> > ```
> > A a;
> > std::move(a);
> > 
> > auto lambda = [=] {
> >   a.foo(); // Use of 'a' after it was moved
> > }
> > ```
> `a` in lambda is `const`, but it's not moved inside lambda, so my warning is 
> not expected to be shown.
The "use" of `a` that Aaron was talking about is actually

auto lambda = [=] { a.foo(); };
   ^ here

where the moved-from object is captured-by-copy into the lambda. Making a copy 
of a moved-from object should warn. (Your patch shouldn't have affected this 
AFAICT; I think he's just asking for a test to verify+document the currently 
expected behavior.)


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

https://reviews.llvm.org/D74692



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-07 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

you need documentation and release note changes too




Comment at: clang/unittests/Format/FormatTest.cpp:31
+class FormatTest
+: public ::testing::Test { // FormatTest is a Fixture, data is reused
 protected:

is this comment necessary?



Comment at: clang/unittests/Format/FormatTest.cpp:2440
 TEST_F(FormatTest, FormatsExternC) {
-  verifyFormat("extern \"C\" {\nint a;");
-  verifyFormat("extern \"C\" {}");
+  verifyFormat("extern \"C\" {\nint a; /*2.1*/");
+  verifyFormat("extern \"C\" { /*2.2*/\n}");

why are you changing tests? where is the test that shows this works when a 
comment isn't present?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75791



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-07 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1088
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterExternBlock) {
-  parseBlock(/*MustBeDeclaration=*/true);
-} else {
+if (Style.BraceWrapping.AfterExternBlock == true &&
+Style.IndentExternBlock == true) {

something here looks abit odd? there is too much repetition around you option, 
I think you doing something at the wrong level.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75791



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

[PATCH] D75806: [CUDA] Add CUDA 10.2 detection

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

tambre created this revision.
tambre added reviewers: tra, jlebar.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75806

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -634,23 +634,24 @@
   // back-end.
   const char *PtxFeature = nullptr;
   switch(CudaInstallation.version()) {
-case CudaVersion::CUDA_101:
-  PtxFeature = "+ptx64";
-  break;
-case CudaVersion::CUDA_100:
-  PtxFeature = "+ptx63";
-  break;
-case CudaVersion::CUDA_92:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_91:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_90:
-  PtxFeature = "+ptx60";
-  break;
-default:
-  PtxFeature = "+ptx42";
+  case CudaVersion::CUDA_102:
+  case CudaVersion::CUDA_101:
+PtxFeature = "+ptx64";
+break;
+  case CudaVersion::CUDA_100:
+PtxFeature = "+ptx63";
+break;
+  case CudaVersion::CUDA_92:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_91:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_90:
+PtxFeature = "+ptx60";
+break;
+  default:
+PtxFeature = "+ptx42";
   }
   CC1Args.append({"-target-feature", PtxFeature});
   if (DriverArgs.hasFlag(options::OPT_fcuda_short_ptr,
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -28,6 +28,8 @@
 return "10.0";
   case CudaVersion::CUDA_101:
 return "10.1";
+  case CudaVersion::CUDA_102:
+return "10.2";
   }
   llvm_unreachable("invalid enum");
 }
@@ -42,6 +44,7 @@
   .Case("9.2", CudaVersion::CUDA_92)
   .Case("10.0", CudaVersion::CUDA_100)
   .Case("10.1", CudaVersion::CUDA_101)
+  .Case("10.2", CudaVersion::CUDA_102)
   .Default(CudaVersion::UNKNOWN);
 }
 
@@ -382,6 +385,8 @@
 return CudaVersion::CUDA_100;
   case 101:
 return CudaVersion::CUDA_101;
+  case 102:
+return CudaVersion::CUDA_102;
   default:
 return CudaVersion::UNKNOWN;
   }
Index: clang/include/clang/Basic/Cuda.h
===
--- clang/include/clang/Basic/Cuda.h
+++ clang/include/clang/Basic/Cuda.h
@@ -27,7 +27,8 @@
   CUDA_92,
   CUDA_100,
   CUDA_101,
-  LATEST = CUDA_101,
+  CUDA_102,
+  LATEST = CUDA_102,
 };
 const char *CudaVersionToString(CudaVersion V);
 // Input is "Major.Minor"


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -634,23 +634,24 @@
   // back-end.
   const char *PtxFeature = nullptr;
   switch(CudaInstallation.version()) {
-case CudaVersion::CUDA_101:
-  PtxFeature = "+ptx64";
-  break;
-case CudaVersion::CUDA_100:
-  PtxFeature = "+ptx63";
-  break;
-case CudaVersion::CUDA_92:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_91:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_90:
-  PtxFeature = "+ptx60";
-  break;
-default:
-  PtxFeature = "+ptx42";
+  case CudaVersion::CUDA_102:
+  case CudaVersion::CUDA_101:
+PtxFeature = "+ptx64";
+break;
+  case CudaVersion::CUDA_100:
+PtxFeature = "+ptx63";
+break;
+  case CudaVersion::CUDA_92:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_91:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_90:
+PtxFeature = "+ptx60";
+break;
+  default:
+PtxFeature = "+ptx42";
   }
   CC1Args.append({"-target-feature", PtxFeature});
   if (DriverArgs.hasFlag(options::OPT_fcuda_short_ptr,
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -28,6 +28,8 @@
 return "10.0";
   case CudaVersion::CUDA_101:
 return "10.1";
+  case CudaVersion::CUDA_102:
+return "10.2";
   }
   llvm_unreachable("invalid enum");
 }
@@ -42,6 +44,7 @@
   .Case("9.2", CudaVersion::CUDA_92)
   .Case("10.0", CudaVersion::CUDA_100)
   .Case("10.1", CudaVersion::CUDA_101)
+  .Case("10.2", CudaVersion::CUDA_102)
   .Default(CudaVersion::UNKNOWN);
 }
 
@@ -382,6 +385,8 @@
 return CudaVersion::CUDA_100;
   case 101:
 return CudaVersion::CUDA_101;
+  case 102:
+return CudaVersion::CUDA_102;
   default:
 return CudaVersion::UNKNOWN;
   }
Index: clang/include/clang/Basic/Cuda.h
===
--- clang/include/clang/Basic/Cuda.h
+++ clang/include/clang/Basic/Cuda.h
@@ -27,7 +27,8

[PATCH] D68578: [HIP] Fix device stub name

2020-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 248927.
yaxunl added a comment.

update patch


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

https://reviews.llvm.org/D68578

Files:
  clang/include/clang/AST/GlobalDecl.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
  clang/test/CodeGenCUDA/kernel-stub-name.cu
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- clang/test/CodeGenCUDA/unnamed-types.cu
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -36,4 +36,4 @@
   }(p);
 }
 // HOST: @__hip_register_globals
-// HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
+// HOST: __hipRegisterFunction{{.*}}@_Z17__device_stub__k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/test/CodeGenCUDA/kernel-stub-name.cu
===
--- clang/test/CodeGenCUDA/kernel-stub-name.cu
+++ clang/test/CodeGenCUDA/kernel-stub-name.cu
@@ -6,15 +6,50 @@
 
 #include "Inputs/cuda.h"
 
+extern "C" __global__ void ckernel() {}
+
+namespace ns {
+__global__ void nskernel() {}
+} // namespace ns
+
 template
 __global__ void kernelfunc() {}
 
+__global__ void kernel_decl();
+
+// Device side kernel names
+
+// CHECK: @[[CKERN:[0-9]*]] = {{.*}} c"ckernel\00"
+// CHECK: @[[NSKERN:[0-9]*]] = {{.*}} c"_ZN2ns8nskernelEv\00"
+// CHECK: @[[TKERN:[0-9]*]] = {{.*}} c"_Z10kernelfuncIiEvv\00"
+
+// Non-template kernel stub functions
+
+// CHECK: define{{.*}}@[[CSTUB:__device_stub__ckernel]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[CSTUB]]
+// CHECK: define{{.*}}@[[NSSTUB:_ZN2ns23__device_stub__nskernelEv]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[NSSTUB]]
+
 // CHECK-LABEL: define{{.*}}@_Z8hostfuncv()
-// CHECK: call void @[[STUB:_Z10kernelfuncIiEvv.stub]]()
-void hostfunc(void) { kernelfunc<<<1, 1>>>(); }
+// CHECK: call void @[[CSTUB]]()
+// CHECK: call void @[[NSSTUB]]()
+// CHECK: call void @[[TSTUB:_Z25__device_stub__kernelfuncIiEvv]]()
+// CHECK: call void @[[DSTUB:_Z26__device_stub__kernel_declv]]()
+void hostfunc(void) {
+  ckernel<<<1, 1>>>();
+  ns::nskernel<<<1, 1>>>();
+  kernelfunc<<<1, 1>>>();
+  kernel_decl<<<1, 1>>>();
+}
+
+// Template kernel stub functions
+
+// CHECK: define{{.*}}@[[TSTUB]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[TSTUB]]
 
-// CHECK: define{{.*}}@[[STUB]]
-// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[STUB]]
+// CHECK: declare{{.*}}@[[DSTUB]]
 
 // CHECK-LABEL: define{{.*}}@__hip_register_globals
-// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[STUB]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[CSTUB]]{{.*}}@[[CKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[NSSTUB]]{{.*}}@[[NSKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[TSTUB]]{{.*}}@[[TKERN]]
Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -13,19 +13,19 @@
 // HOST-NOT: %struct.T.coerce
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
-// HOST: define void @_Z7kernel1Pi.stub(i32* %x)
+// HOST: define void @_Z22__device_stub__kernel1Pi(i32* %x)
 __global__ void kernel1(int *x) {
   x[0]++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
-// HOST: define void @_Z7kernel2Ri.stub(i32* dereferenceable(4) %x)
+// HOST: define void @_Z22__device_stub__kernel2Ri(i32* dereferenceable(4) %x)
 __global__ void kernel2(int ) {
   x++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
-// HOST: define void @_Z7kernel3PU3AS2iPU3AS1i.stub(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+// HOST: define void @_Z22__device_stub__kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
 __global__ void kernel3(__attribute__((address_space(2))) int *x,
 __attribute__((address_space(1))) int *y) {
   y[0] = x[0];
@@ -43,7 +43,7 @@
 // `by-val` struct will be coerced into a similar struct with all generic
 // pointers lowerd into global ones.
 // CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
-// HOST: define void @_Z7kernel41S.stub(i32* %s.coerce0, float* %s.coerce1)
+// HOST: define void @_Z22__device_stub__kernel41S(i32* %s.coerce0, float* %s.coerce1)
 __global__ void kernel4(struct S s) {
   s.x[0]++;
   s.y[0] += 1.f;
@@ -51,7 +51,7 @@
 
 // If a pointer to struct is passed, only the pointer itself is coerced

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

2020-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 248926.
yaxunl marked 6 inline comments as done.
yaxunl added a comment.

revised by John's comments.


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

https://reviews.llvm.org/D75700

Files:
  clang/include/clang/AST/GlobalDecl.h
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3922,7 +3922,7 @@
   // Calculate the mangled name.
   SmallString<256> ThunkName;
   llvm::raw_svector_ostream Out(ThunkName);
-  getMangleContext().mangleCXXCtor(CD, CT, Out);
+  getMangleContext().mangleName(GlobalDecl(CD, CT), Out);
 
   // If the thunk has been generated previously, just return it.
   if (llvm::GlobalValue *GV = CGM.getModule().getNamedValue(ThunkName))
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1019,22 +1019,15 @@
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
   MangleContext  = CGM.getCXXABI().getMangleContext();
-  if (MC.shouldMangleDeclName(ND)) {
-llvm::raw_svector_ostream Out(Buffer);
-if (const auto *D = dyn_cast(ND))
-  MC.mangleCXXCtor(D, GD.getCtorType(), Out);
-else if (const auto *D = dyn_cast(ND))
-  MC.mangleCXXDtor(D, GD.getDtorType(), Out);
-else
-  MC.mangleName(ND, Out);
-  } else {
+  if (MC.shouldMangleDeclName(ND))
+MC.mangleName(GD.getWithDecl(ND), Out);
+  else {
 IdentifierInfo *II = ND->getIdentifier();
 assert(II && "Attempt to mangle unnamed decl.");
 const auto *FD = dyn_cast(ND);
 
 if (FD &&
 FD->getType()->castAs()->getCallConv() == CC_X86RegCall) {
-  llvm::raw_svector_ostream Out(Buffer);
   Out << "__regcall3__" << II->getName();
 } else {
   Out << II->getName();
@@ -4473,7 +4466,7 @@
 
   maybeSetTrivialComdat(*D, *Fn);
 
-  CodeGenFunction(*this).GenerateCode(D, Fn, FI);
+  CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
 
   setNonAliasAttributes(GD, Fn);
   SetLLVMFunctionAttributesForDefinition(D, Fn);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2412,13 +2412,14 @@
 }
 
 static llvm::Constant *EmitFunctionDeclPointer(CodeGenModule ,
-   const FunctionDecl *FD) {
+   GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
   if (FD->hasAttr()) {
 ConstantAddress aliasee = CGM.GetWeakRefReference(FD);
 return aliasee.getPointer();
   }
 
-  llvm::Constant *V = CGM.GetAddrOfFunction(FD);
+  llvm::Constant *V = CGM.GetAddrOfFunction(GD);
   if (!FD->hasPrototype()) {
 if (const FunctionProtoType *Proto =
 FD->getType()->getAs()) {
@@ -2435,9 +2436,10 @@
   return V;
 }
 
-static LValue EmitFunctionDeclLValue(CodeGenFunction ,
- const Expr *E, const FunctionDecl *FD) {
-  llvm::Value *V = EmitFunctionDeclPointer(CGF.CGM, FD);
+static LValue EmitFunctionDeclLValue(CodeGenFunction , const Expr *E,
+ GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
+  llvm::Value *V = EmitFunctionDeclPointer(CGF.CGM, GD);
   CharUnits Alignment = CGF.getContext().getDeclAlign(FD);
   return CGF.MakeAddrLValue(V, E->getType(), Alignment,
 AlignmentSource::Decl);
@@ -4637,7 +4639,8 @@
   return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue);
 }
 
-static CGCallee EmitDirectCallee(CodeGenFunction , const FunctionDecl *FD) {
+static CGCallee EmitDirectCallee(CodeGenFunction , GlobalDecl GD) {
+  const FunctionDecl *FD = cast(GD.getDecl());
 
   if (auto builtinID = FD->getBuiltinID()) {
 // Replaceable builtin provide their own implementation of a builtin. Unless
@@ -4649,8 +4652,8 @@
   return CGCallee::forBuiltin(builtinID, FD);
   }
 
-  llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
-  return CGCallee::forDirect(calleePtr, GlobalDecl(FD));
+  llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
+  return CGCallee::forDirect(calleePtr, GD);
 }
 
 CGCallee CodeGenFunction::EmitCallee(const Expr *E) {
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -135,7 +135,7 @@
   MicrosoftMangleContextImpl(ASTContext , DiagnosticsEngine );
   bool

[PATCH] D75805: Make malign-double effective only for x86

2020-03-07 Thread kamlesh kumar via Phabricator via cfe-commits

kamleshbhalui created this revision.
kamleshbhalui added a reviewer: rnk.
kamleshbhalui added a project: clang.
Herald added a subscriber: cfe-commits.

Making -malign-double effective only for x86.

Fixes.
https://bugs.llvm.org/show_bug.cgi?id=45136


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75805

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2916,7 +2916,7 @@
   Opts.EmitAllDecls = Args.hasArg(OPT_femit_all_decls);
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, 
Diags);
-  Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.AlignDouble = T.getArch() == llvm::Triple::x86 && 
Args.hasArg(OPT_malign_double);
   Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
 ? 128
 : Args.hasArg(OPT_mlong_double_64) ? 64 : 0;


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2916,7 +2916,7 @@
   Opts.EmitAllDecls = Args.hasArg(OPT_femit_all_decls);
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
-  Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.AlignDouble = T.getArch() == llvm::Triple::x86 && Args.hasArg(OPT_malign_double);
   Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
 ? 128
 : Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

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

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



Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:570
   const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
-  isComparisonOperator(), expr().bind(Id),
+  matchers::isComparisonOperator(), expr().bind(Id),
   anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),

This will be reverted in a follow up, but it was breaking the build, same goes 
below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75800



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

[PATCH] D75803: [clang-tidy] [NFC] Remove unnecessary matchers

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

njames93 created this revision.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.
njames93 added a parent revision: D75800: [ASTMatchers] adds 
isComparisonOperator to BinaryOperator and CXXOperatorCallExpr.
Herald added a subscriber: wuzish.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
njames93 added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75803

Files:
  clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h

Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -16,20 +16,12 @@
 namespace tidy {
 namespace matchers {
 
-AST_MATCHER(BinaryOperator, isAssignmentOperator) {
-  return Node.isAssignmentOp();
-}
-
 AST_MATCHER(BinaryOperator, isRelationalOperator) {
   return Node.isRelationalOp();
 }
 
 AST_MATCHER(BinaryOperator, isEqualityOperator) { return Node.isEqualityOp(); }
 
-AST_MATCHER(BinaryOperator, isComparisonOperator) {
-  return Node.isComparisonOp();
-}
-
 AST_MATCHER(QualType, isExpensiveToCopy) {
   llvm::Optional IsExpensive =
   utils::type_traits::isExpensiveToCopy(Node, Finder->getASTContext());
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -41,7 +41,7 @@
 
   const auto WrongUse = anyOf(
   hasParent(binaryOperator(
-matchers::isComparisonOperator(),
+isComparisonOperator(),
 hasEitherOperand(ignoringImpCasts(anyOf(
 integerLiteral(equals(1)), integerLiteral(equals(0))
 .bind("SizeBinaryOp")),
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -567,7 +567,7 @@
   std::string OverloadId = (Id + "-overload").str();
 
   const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
-  matchers::isComparisonOperator(), expr().bind(Id),
+  isComparisonOperator(), expr().bind(Id),
   anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
   hasRHS(matchIntegerConstantExpr(Id))),
 allOf(hasLHS(matchIntegerConstantExpr(Id)),
@@ -836,9 +836,8 @@
   binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
hasOperatorName("%"), hasOperatorName("|"),
hasOperatorName("&"), hasOperatorName("^"),
-   matchers::isComparisonOperator(),
-   hasOperatorName("&&"), hasOperatorName("||"),
-   hasOperatorName("=")),
+   isComparisonOperator(), hasOperatorName("&&"),
+   hasOperatorName("||"), hasOperatorName("=")),
  operandsAreEquivalent(),
  // Filter noisy false positives.
  unless(isInTemplateInstantiation()),
@@ -943,7 +942,7 @@
   const auto SymRight = matchSymbolicExpr("rhs");
 
   // Match expressions like: x  0xFF == 0xF00.
-  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(isComparisonOperator(),
 hasEitherOperand(BinOpCstLeft),
 hasEitherOperand(CstRight))
  .bind("binop-const-compare-to-const"),
@@ -951,14 +950,14 @@
 
   // Match expressions like: x  0xFF == x.
   Finder->addMatcher(
-  binaryOperator(matchers::isComparisonOperator(),
+  binaryOperator(isComparisonOperator(),
  anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft
   .bind("binop-const-compare-to-sym"),
   this);
 
   // Match expressions like: x  10 == x  12.
-  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(isComparisonOperator(),
 hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
 //

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 248917.
ztamas added a comment.

Update code based on reviewer comments.

- Update docs: comparison -> equality/inequality comparison.
- Use hasAnyOperatorName
- Plus fix a typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  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
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharIneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,39 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it 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
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

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

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97
+  const auto CompareOperator =
+  expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+  anyOf(allOf(hasLHS(SignedCharCastExpr),

ztamas wrote:
> njames93 wrote:
> > `binaryOperator(hasAnyOperatorName("==", "!=") ...`
> > Also whats the reason for not checking `<`, `>`, `<=`, `>=` or `<=>`?
> I think these are different cases when the programmer checks the equality of 
> two characters or checking some kind of order of characters. In case of 
> equality, there seems to be a clear assumption, that if the two character 
> variables represent the same character (semantically the same), then the 
> equality should return true. This assumption fails with mixed signed and 
> unsigned char variables.
> However, when the programmer uses a less or a greater operator it's a bit 
> different. I guess a programmer in the case starts to wonder what is the 
> actual order of the characters, checks the ASCII table if it's not obvious 
> for a set of characters. Also, it's not clear what might be the false 
> assumption here. Is an ASCII character smaller or greater than a non-ASCII 
> character? So I don't see a probable false assumption here what we can point 
> out. At least, in general.
> Furthermore, I optimized the check for equality/inequality, with ignoring the 
> ASCII characters for both the signed and the unsigned operand. This makes 
> sense for equality/inequality but does not make sense for the other 
> comparison operators.
> All-in-all I'm now focusing on the equality / unequality operators. Usage of 
> these operands clearly something we can catch. Checking other comparison 
> operators is something that needs consideration, so I don't bother with that 
> in this patch.
Fair enough, in that case can you specify in the docs that you only check for 
`==` and `!=` comparison. Currently they only say it looks for comparison 
rather than maybe equality comparison. 
Also point about `hasAnyOperatorName` still stands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

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

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75800

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2689,6 +2689,20 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(IsComparisonOperator, Basic) {
+  StatementMatcher BinCompOperator = binaryOperator(isComparisonOperator());
+  StatementMatcher CXXCompOperator =
+  cxxOperatorCallExpr(isComparisonOperator());
+
+  EXPECT_TRUE(matches("void x() { int a; a == 1; }", BinCompOperator));
+  EXPECT_TRUE(matches("void x() { int a; a > 2; }", BinCompOperator));
+  EXPECT_TRUE(matches("struct S { bool operator==(const S&); };"
+  "void x() { S s1, s2; bool b1 = s1 == s2; }",
+  CXXCompOperator));
+  EXPECT_TRUE(
+  notMatches("void x() { int a; if(a = 0) return; }", BinCompOperator));
+}
+
 TEST(HasInit, Basic) {
   EXPECT_TRUE(
 matches("int x{0};",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -358,6 +358,7 @@
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isClassMessage);
   REGISTER_MATCHER(isClassMethod);
+  REGISTER_MATCHER(isComparisonOperator);
   REGISTER_MATCHER(isConst);
   REGISTER_MATCHER(isConstQualified);
   REGISTER_MATCHER(isConstexpr);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4783,7 +4783,7 @@
 ///(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
 /// \code
 ///   struct S { S& operator=(const S&); };
-///   void x() { S s1, s2; s1 = s2; })
+///   void x() { S s1, s2; s1 = s2; }
 /// \endcode
 AST_POLYMORPHIC_MATCHER(isAssignmentOperator,
 AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
@@ -4791,6 +4791,26 @@
   return Node.isAssignmentOp();
 }
 
+/// Matches comparison operators.
+///
+/// Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+/// \code
+///   if (a == b)
+/// a += b;
+/// \endcode
+///
+/// Example 2: matches s1 < s2
+///(matcher = cxxOperatorCallExpr(isComparisonOperator()))
+/// \code
+///   struct S { bool operator<(const S& other); };
+///   void x(S s1, S s2) { bool b1 = s1 < s2; }
+/// \endcode
+AST_POLYMORPHIC_MATCHER(isComparisonOperator,
+AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
+CXXOperatorCallExpr)) {
+  return Node.isComparisonOp();
+}
+
 /// Matches the left hand side of binary operator expressions.
 ///
 /// Example matches a (matcher = binaryOperator(hasLHS()))
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -118,6 +118,22 @@
   }
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
+  static bool isComparisonOp(OverloadedOperatorKind Opc) {
+switch (Opc) {
+case OO_EqualEqual:
+case OO_ExclaimEqual:
+case OO_Greater:
+case OO_GreaterEqual:
+case OO_Less:
+case OO_LessEqual:
+case OO_Spaceship:
+  return true;
+default:
+  return false;
+}
+  }
+  bool isComparisonOp() const { return isComparisonOp(getOperator()); }
+
   /// Is this written as an infix binary operator?
   bool isInfixBinaryOp() const;
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -2157,7 +2157,21 @@
 Example 2: matches s1 = s2
(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S operator=(const S); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+
+
+
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html;>BinaryOperatorisComparisonOperator
+Matches comparison operators.
+
+Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+  if (a == b)
+a += b;
+
+Example 2: matches s1  s2
+

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 248912.
ztamas added a comment.

Use quotes in warning message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  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
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharUneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,38 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it 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
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value from [0..255]), however, the actual value is in
-[-128..127] interval. This also applies to the plain ``char``

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97
+  const auto CompareOperator =
+  expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+  anyOf(allOf(hasLHS(SignedCharCastExpr),

njames93 wrote:
> `binaryOperator(hasAnyOperatorName("==", "!=") ...`
> Also whats the reason for not checking `<`, `>`, `<=`, `>=` or `<=>`?
I think these are different cases when the programmer checks the equality of 
two characters or checking some kind of order of characters. In case of 
equality, there seems to be a clear assumption, that if the two character 
variables represent the same character (semantically the same), then the 
equality should return true. This assumption fails with mixed signed and 
unsigned char variables.
However, when the programmer uses a less or a greater operator it's a bit 
different. I guess a programmer in the case starts to wonder what is the actual 
order of the characters, checks the ASCII table if it's not obvious for a set 
of characters. Also, it's not clear what might be the false assumption here. Is 
an ASCII character smaller or greater than a non-ASCII character? So I don't 
see a probable false assumption here what we can point out. At least, in 
general.
Furthermore, I optimized the check for equality/inequality, with ignoring the 
ASCII characters for both the signed and the unsigned operand. This makes sense 
for equality/inequality but does not make sense for the other comparison 
operators.
All-in-all I'm now focusing on the equality / unequality operators. Usage of 
these operands clearly something we can catch. Checking other comparison 
operators is something that needs consideration, so I don't bother with that in 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 248909.
ztamas added a comment.

One more unneeded allOf and clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  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
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharUneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,38 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it 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
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value from [0..255]), however, the actual value is in
-[-128..127] interval. This also applies to the plain ``char`` type on
-those

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 248907.
ztamas added a comment.

Fix pre-merge check error

Plus remove unneeded "no warning" comments
and add a missing quote mark in docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  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
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharUneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,38 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it 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
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value from [0..255]), however, the actual value is

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 248908.
ztamas added a comment.

Changed code based on reviewer comments.

- Removed unneeded allOf()
- Remove unneeded ';'
- Added new line at the end of the  file
- Moved variable declarations inside if()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  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
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharUneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,38 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it 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
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value from

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

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

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:74
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IntegerType = qualType(allOf(isInteger(), 
unless(isAnyCharacter()),
+  unless(booleanType(

All node matchers are implicitly `allOf` matchers so you can safely drop the 
`allOf` matcher.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97
+  const auto CompareOperator =
+  expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+  anyOf(allOf(hasLHS(SignedCharCastExpr),

`binaryOperator(hasAnyOperatorName("==", "!=") ...`
Also whats the reason for not checking `<`, `>`, `<=`, `>=` or `<=>`?



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:103
+  .bind("comparison");
+  ;
+

Any reason for this trailing `;`?



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:125
 
-  diag(CastExpression->getBeginLoc(),
-   "'signed char' to %0 conversion; "
-   "consider casting to 'unsigned char' first.")
-  << *IntegerType;
+  if (Comparison) {
+const auto *UnSignedCastExpression =

This should probably be a condition variable and bring the init from the start 
of `check` inside.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:141-143
+  } else {
+const auto *IntegerType = Result.Nodes.getNodeAs("integerType");
+assert(IntegerType);

```} else if (const auto *IntegerType = 
Result.Nodes.getNodeAs("integerType")) {
  ...
} else 
  llvm_unreachable("Unexpected match");
```



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


New line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-07 Thread Zinovy Nis via Phabricator via cfe-commits

zinovy.nis added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured 
const here
 };
   }

aaron.ballman wrote:
> One more test case to try out (it might be a FIXME because I imagine this 
> requires flow control to get right):
> ```
> A a;
> std::move(a);
> 
> auto lambda = [=] {
>   a.foo(); // Use of 'a' after it was moved
> }
> ```
`a` in lambda is `const`, but it's not moved inside lambda, so my warning is 
not expected to be shown.


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

https://reviews.llvm.org/D74692



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

[PATCH] D75739: [clangd][vscode] Enable dot-to-arrow fixes in clangd completion.

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

njames93 added a comment.

How does this handle cases where an object overloads the `->` operator or worse 
still overloads the operator and has conflicting names when accessed with `. ` 
or `->`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75739



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-07 Thread Zinovy Nis via Phabricator via cfe-commits

zinovy.nis updated this revision to Diff 248905.
zinovy.nis marked 4 inline comments as done.
zinovy.nis added a comment.

Cosmetic and style fixes.


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

https://reviews.llvm.org/D74692

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,16 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+void simpleConst() {
+  const A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved; move of a 'const' argument has no effect
+  // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
+  // CHECK-NOTES: [[@LINE-6]]:11: note: variable 'a' declared const here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +324,21 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved; move of a 'const' argument has no effect
   // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' captured const here
+};
+  }
+  // Use-after-moves inside a lambda should be detected.
+  {
+A a;
+// Implicit capture
+auto lambda = [=] {
+  std::move(a);
+  a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved; move of a 'const' argument has no effect
+  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured const here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +744,16 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved; move of a 'const' argument has no effect
 // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+// CHECK-NOTES: [[@LINE-5]]:13: note: variable 'a' declared const here
   }
   const A a;
   std::move(a);
   passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved; move of a 'const' argument has no effect
   // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:11: note: variable 'a' declared const here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -375,14 +375,36 @@
 }
 
 static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg,
-   const UseAfterMove , ClangTidyCheck *Check,
+   const UseAfterMove ,
+   const LambdaExpr *Lambda, ClangTidyCheck *Check,
ASTContext *Context) {
   SourceLocation UseLoc = Use.DeclRef->getExprLoc();
   SourceLocation MoveLoc = MovingCall->getExprLoc();
+  const int MoveArgIsConst = MoveArg->getType().isConstQualified();
 
-  Check->diag(UseLoc, "'%0' used after it was moved")
-  << MoveArg->getDecl()->getName();
+  Check->diag(UseLoc, "'%0' used after it was moved%select{|; move of a "
+  "'const' argument has no effect}1")
+  << MoveArg->getDecl()->getName() << MoveArgIsConst;
   Check->diag(MoveLoc, "move occurred here", DiagnosticIDs::Note);
+  if (MoveArgIsConst) {
+if (Lambda) {
+  for (const auto  : Lambda->captures()) {
+if (MoveArg->getDecl() == Capture.getCapturedVar()) {
+  const int IsExplicitCapture = Capture.isExplicit();
+  Check->diag(IsExplicitCapture ? Capture.getLocation()
+: Lambda->getCaptureDefaultLoc(),
+  "variable %0 %select{implicitly|}1 captured const here",
+  DiagnosticIDs::Note)
+  << Capture.getCapturedVar() << IsExplicitCapture;
+  break;
+}
+  }
+} else {
+  Check->diag(MoveArg->getDecl()->getLocation(),
+  "variable '%0' declared const here", DiagnosticIDs::Note);
+}
+  }
+
   if (Use.EvaluationOrderUndefined) {
 Check->diag(UseLoc,

61 matches

Mail list logo