date:20200112

[clang] 6d6a459 - [DWARF5][clang]: Added support for DebugInfo generation for auto return type for C++ member functions.

2020-01-12 Thread Sourabh Singh Tomar via cfe-commits


Author: Awanish Pandey
Date: 2020-01-13T12:40:18+05:30
New Revision: 6d6a4590c5d4c7fc7445d72fe685f966b0a8cafb

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

LOG: [DWARF5][clang]: Added support for DebugInfo generation for auto return 
type for C++ member functions.

Summary:
This patch will provide support for auto return type for the C++ member
functions.

This patch includes clang side implementation of this feature.

Patch by: Awanish Pandey 

Reviewers: dblaikie, aprantl, shafik, alok, SouraVX, jini.susan.george
Reviewed by: dblaikie

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

Added: 
clang/test/CodeGenCXX/debug-info-auto-return.cpp

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 675df309e3f0..6d2123a7b0e1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -810,6 +810,10 @@ llvm::DIType *CGDebugInfo::CreateType(const BuiltinType 
*BT) {
   return DBuilder.createBasicType(BTName, Size, Encoding);
 }
 
+llvm::DIType *CGDebugInfo::CreateType(const AutoType *Ty) {
+  return DBuilder.createUnspecifiedType("auto");
+}
+
 llvm::DIType *CGDebugInfo::CreateType(const ComplexType *Ty) {
   // Bit size and offset of the type.
   llvm::dwarf::TypeKind Encoding = llvm::dwarf::DW_ATE_complex_float;
@@ -1457,16 +1461,18 @@ void CGDebugInfo::CollectRecordFields(
 
 llvm::DISubroutineType *
 CGDebugInfo::getOrCreateMethodType(const CXXMethodDecl *Method,
-   llvm::DIFile *Unit) {
+   llvm::DIFile *Unit, bool decl) {
   const FunctionProtoType *Func = 
Method->getType()->getAs();
   if (Method->isStatic())
 return cast_or_null(
 getOrCreateType(QualType(Func, 0), Unit));
-  return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit);
+  return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit, 
decl);
 }
 
-llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
-QualType ThisPtr, const FunctionProtoType *Func, llvm::DIFile *Unit) {
+llvm::DISubroutineType *
+CGDebugInfo::getOrCreateInstanceMethodType(QualType ThisPtr,
+   const FunctionProtoType *Func,
+   llvm::DIFile *Unit, bool decl) {
   // Add "this" pointer.
   llvm::DITypeRefArray Args(
   cast(getOrCreateType(QualType(Func, 0), Unit))
@@ -1474,9 +1480,12 @@ llvm::DISubroutineType 
*CGDebugInfo::getOrCreateInstanceMethodType(
   assert(Args.size() && "Invalid number of arguments!");
 
   SmallVector Elts;
-
   // First element is always return type. For 'void' functions it is NULL.
-  Elts.push_back(Args[0]);
+  QualType temp = Func->getReturnType();
+  if (temp->getTypeClass() == Type::Auto && decl)
+Elts.push_back(CreateType(cast(temp)));
+  else
+Elts.push_back(Args[0]);
 
   // "this" pointer is always first argument.
   const CXXRecordDecl *RD = ThisPtr->getPointeeCXXRecordDecl();
@@ -1535,7 +1544,7 @@ llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
   isa(Method) || isa(Method);
 
   StringRef MethodName = getFunctionName(Method);
-  llvm::DISubroutineType *MethodTy = getOrCreateMethodType(Method, Unit);
+  llvm::DISubroutineType *MethodTy = getOrCreateMethodType(Method, Unit, true);
 
   // Since a single ctor/dtor corresponds to multiple functions, it doesn't
   // make sense to give a single ctor/dtor a linkage name.
@@ -2754,7 +2763,7 @@ llvm::DIType *CGDebugInfo::CreateType(const 
MemberPointerType *Ty,
   return DBuilder.createMemberPointerType(
   getOrCreateInstanceMethodType(
   CXXMethodDecl::getThisType(FPT, Ty->getMostRecentCXXRecordDecl()),
-  FPT, U),
+  FPT, U, false),
   ClassType, Size, /*Align=*/0, Flags);
 }
 
@@ -3529,7 +3538,7 @@ llvm::DISubroutineType 
*CGDebugInfo::getOrCreateFunctionType(const Decl *D,
 return DBuilder.createSubroutineType(DBuilder.getOrCreateTypeArray(None));
 
   if (const auto *Method = dyn_cast(D))
-return getOrCreateMethodType(Method, F);
+return getOrCreateMethodType(Method, F, false);
 
   const auto *FTy = FnType->getAs();
   CallingConv CC = FTy ? FTy->getCallConv() : CallingConv::CC_C;

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 90e9a61ebe96..d9c6b4d79097 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -165,6 +165,7 @@ class CGDebugInfo {
   /// ivars and property accessors.
   llvm::DIType *CreateType(const BuiltinType *Ty);
   llvm::DIType *CreateType(const ComplexType *Ty);
+  llvm::DIType *CreateType(const AutoType *Ty);
   llvm::DIType

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2020-01-12 Thread Jaydeep Chauhan via Phabricator via cfe-commits

Jac1494 added a comment.

ping


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

https://reviews.llvm.org/D71451



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

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

2020-01-12 Thread Sourabh Singh Tomar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc958639098a8: [DWARF5][DebugInfo]: Added support for 
DebugInfo generation for auto return… (authored by awpandey, committed by 
SouraVX).

Changed prior to commit:
  https://reviews.llvm.org/D70524?vs=237264=237578#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70524

Files:
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/test/DebugInfo/X86/debug-info-auto-return.ll


Index: llvm/test/DebugInfo/X86/debug-info-auto-return.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-auto-return.ll
@@ -0,0 +1,70 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; CHECK: .debug_info contents:
+
+; CHECK: DW_TAG_subprogram 
+; CHECK-NEXT: DW_AT_linkage_name [DW_FORM_strx1](indexed {{.*}} string = 
"_ZN7myClass7findMaxEv")
+; CHECK: DW_AT_type [DW_FORM_ref4] (cu + {{.*}} "auto")
+; CHECK-NEXT: DW_AT_declaration [DW_FORM_flag_present]  (true)
+
+; CHECK: DW_TAG_subprogram 
+; CHECK: DW_AT_type [DW_FORM_ref4]   (cu + {{.*}} "double")
+; CHECK: DW_AT_specification [DW_FORM_ref4]  (cu + {{.*}} 
"_ZN7myClass7findMaxEv")
+
+; C++ source to regenerate:
+; struct myClass {
+;auto findMax();
+; };
+;
+; auto myClass::findMax() {
+;return 0.0;
+; }
+
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.myClass = type { i8 }
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local double @_ZN7myClass7findMaxEv(%struct.myClass* %this) #0 
align 2 !dbg !7 {
+entry:
+  %this.addr = alloca %struct.myClass*, align 8
+  store %struct.myClass* %this, %struct.myClass** %this.addr, align 8
+  call void @llvm.dbg.declare(metadata %struct.myClass** %this.addr, metadata 
!17, metadata !DIExpression()), !dbg !19
+  %this1 = load %struct.myClass*, %struct.myClass** %this.addr, align 8
+  ret double 0.00e+00, !dbg !20
+}
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, 
emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: 
None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/dir/", checksumkind: 
CSK_MD5, checksum: "4bed8955bd441e3129c12f557ed53962")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0"}
+!7 = distinct !DISubprogram(name: "findMax", linkageName: 
"_ZN7myClass7findMaxEv", scope: !8, file: !1, line: 20, type: !9, scopeLine: 
20, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, 
declaration: !13, retainedNodes: !2)
+!8 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "myClass", 
file: !1, line: 16, size: 8, flags: DIFlagTypePassByValue, elements: !2, 
identifier: "_ZTS7myClass")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !12}
+!11 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 64, flags: 
DIFlagArtificial | DIFlagObjectPointer)
+!13 = !DISubprogram(name: "findMax", linkageName: "_ZN7myClass7findMaxEv", 
scope: !8, file: !1, line: 17, type: !14, scopeLine: 17, flags: 
DIFlagPrototyped, spFlags: 0)
+!14 = !DISubroutineType(types: !15)
+!15 = !{!16, !12}
+!16 = !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")
+!17 = !DILocalVariable(name: "this", arg: 1, scope: !7, type: !18, flags: 
DIFlagArtificial | DIFlagObjectPointer)
+!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 64)
+!19 = !DILocation(line: 0, scope: !7)
+!20 = !DILocation(line: 21, column: 3, scope: !7)
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1165,6 +1165,14 @@
   DIE *DeclDie = nullptr;
   StringRef DeclLinkageName;
   if (auto *SPDecl = SP->getDeclaration()) {
+DITypeRefArray DeclArgs, DefinationArgs;
+DeclArgs = SPDecl->getType()->getTypeArray();
+DefinationArgs = SP->getType()->getTypeArray();
+
+if (DeclArgs.size() && DefinationArgs.size())
+  if (DeclArgs[0] != DefinationArgs[0])
+addType(SPDie, DefinationArgs[0]);
+

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61745 tests passed, 0 failed 
and 780 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2020-01-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov updated this revision to Diff 237577.
ilya-biryukov added a comment.

- Add compound assignment operations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920

Files:
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang/include/clang/AST/DependencyFlags.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp

Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -511,10 +511,23 @@
 void ASTStmtReader::VisitExpr(Expr *E) {
   VisitStmt(E);
   E->setType(Record.readType());
-  E->setTypeDependent(Record.readInt());
-  E->setValueDependent(Record.readInt());
-  E->setInstantiationDependent(Record.readInt());
-  E->ExprBits.ContainsUnexpandedParameterPack = Record.readInt();
+
+  // FIXME: write and read all DependentFlags with a single call.
+  bool TypeDependent = Record.readInt();
+  bool ValueDependent = Record.readInt();
+  bool InstantiationDependent = Record.readInt();
+  bool ContainsUnexpandedTemplateParameters = Record.readInt();
+  DependencyFlags Deps = DependencyFlags::None;
+  if (TypeDependent)
+Deps |= DependencyFlags::Type;
+  if (ValueDependent)
+Deps |= DependencyFlags::Value;
+  if (InstantiationDependent)
+Deps |= DependencyFlags::Instantiation;
+  if (ContainsUnexpandedTemplateParameters)
+Deps |= DependencyFlags::UnexpandedPack;
+  E->setDependencies(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12695,9 +12695,7 @@
   // base classes.
   CallExpr *CE = CallExpr::Create(Context, Fn, Args, Context.DependentTy,
   VK_RValue, RParenLoc);
-  CE->setTypeDependent(true);
-  CE->setValueDependent(true);
-  CE->setInstantiationDependent(true);
+  CE->addDependencies(DependencyFlags::TypeValueInstantiation);
   *Result = CE;
   return true;
 }
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -111,84 +111,65 @@
   return TemplateArgument(Args.copy(Context));
 }
 
-bool TemplateArgument::isDependent() const {
+DependencyFlags TemplateArgument::getDependencies() const {
+  DependencyFlags Deps = DependencyFlags::None;
   switch (getKind()) {
   case Null:
 llvm_unreachable("Should not have a NULL template argument");
 
   case Type:
-return getAsType()->isDependentType() ||
-   isa(getAsType());
+Deps = getAsType()->getDependencies();
+if (isa(getAsType()))
+  Deps |= DependencyFlags::Type;
+return Deps;
 
   case Template:
-return getAsTemplate().isDependent();
+// FIXME: add TemplateName::getDependencies.
+if (getAsTemplate().isDependent())
+  Deps |= DependencyFlags::TypeValue;
+if (getAsTemplate().isInstantiationDependent())
+  Deps |= DependencyFlags::Instantiation;
+if (getAsTemplate().containsUnexpandedParameterPack())
+  Deps |= DependencyFlags::UnexpandedPack;
+return Deps;
 
   case TemplateExpansion:
-return true;
+return DependencyFlags::TypeValueInstantiation;
 
-  case Declaration:
-if (DeclContext *DC = dyn_cast(getAsDecl()))
-  return DC->isDependentContext();
-return getAsDecl()->getDeclContext()->isDependentContext();
+  case Declaration: {
+auto *DC = dyn_cast(getAsDecl());
+if (!DC)
+  DC = getAsDecl()->getDeclContext();
+if (DC->isDependentContext())
+  Deps = DependencyFlags::TypeValueInstantiation;
+return Deps;
+  }
 
   case NullPtr:
-return false;
-
   case Integral:
-// Never dependent
-return false;
+return DependencyFlags::None;
 
   case Expression:
-return (getAsExpr()->isTypeDependent() || getAsExpr()->isValueDependent() ||
-isa(getAsExpr()));
+Deps = getAsExpr()->getDependencies();
+if (isa(getAsExpr()))
+  Deps |= DependencyFlags::TypeValueInstantiation;
+return Deps;
 
   case Pack:
 for (const auto  : pack_elements())
-  if (P.isDependent())
-return true;
-return false;
+  Deps |= P.getDependencies();
+return Deps;
   }
+  llvm_unreachable("unhandled ArgKind");
+}
 
-  llvm_unreachable("Invalid

[PATCH] D72581: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61746 tests passed, 0 failed 
and 780 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72581



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

[PATCH] D72581: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

2020-01-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a project: clang.

Same restrictions apply as in the other direction: macro arguments are
not supported yet, only full macro expansions can be mapped.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72581

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -57,6 +57,7 @@
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
@@ -178,10 +179,14 @@
   template 
   llvm::ArrayRef findSubrange(llvm::ArrayRef Subrange,
  llvm::ArrayRef Range, Eq F) {
-for (auto Begin = Range.begin(); Begin < Range.end(); ++Begin) {
+assert(Subrange.size() >= 1);
+if (Range.size() < Subrange.size())
+  return llvm::makeArrayRef(Range.end(), Range.end());
+for (auto Begin = Range.begin(), Last = Range.end() - Subrange.size();
+ Begin <= Last; ++Begin) {
   auto It = Begin;
-  for (auto ItSub = Subrange.begin();
-   ItSub != Subrange.end() && It != Range.end(); ++ItSub, ++It) {
+  for (auto ItSub = Subrange.begin(); ItSub != Subrange.end();
+   ++ItSub, ++It) {
 if (!F(*ItSub, *It))
   goto continue_outer;
   }
@@ -664,6 +669,101 @@
   ValueIs(SameRange(findSpelled("not_mapped";
 }
 
+TEST_F(TokenBufferTest, ExpandedBySpelled) {
+  recordTokens(R"cpp(
+a1 a2 a3 b1 b2
+  )cpp");
+  // Sanity check: expanded and spelled tokens are stored separately.
+  EXPECT_THAT(findExpanded("a1 a2"), Not(SameRange(findSpelled("a1 a2";
+  // Searching for subranges of expanded tokens should give the corresponding
+  // spelled ones.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1 a2 a3 b1 b2")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1 a2 a3")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("b1 b2")),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+
+  // Test search on simple macro expansions.
+  recordTokens(R"cpp(
+#define A a1 a2 a3
+#define B b1 b2
+
+A split B
+  )cpp");
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("A split B")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 split b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("A split").drop_back()),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("split B").drop_front()),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+
+  // Recursive macro invocations.
+  recordTokens(R"cpp(
+#define ID(x) x
+#define B b1 b2
+
+ID(ID(ID(a1) a2 a3)) split ID(B)
+  )cpp");
+
+  EXPECT_THAT(
+  Buffer.expandedForSpelled(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) )")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( B )")),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(
+  findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) ) split ID ( B )")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 split b1 b2";
+  // FIXME: these should succeed, but we do not support macro arguments yet.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1")), IsEmpty());
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( a1 ) a2")),
+  IsEmpty());
+
+  // Empty macro expansions.
+  recordTokens(R"cpp(
+#define EMPTY
+#define ID(X) X
+
+EMPTY EMPTY ID(1 2 3) EMPTY EMPTY split1
+EMPTY EMPTY ID(4 5 6) split2
+ID(7 8 9) EMPTY EMPTY
+  )cpp");
+  // Covered by empty expansions on one of both of the sides.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( 1 2 3 )")),
+  ElementsAre(SameRange(findExpanded("1 2 3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( 4 5 6 )")),
+  ElementsAre(SameRange(findExpanded("4 5 6";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( 7 8 9 )")),
+  ElementsAre(SameRange(findExpanded("7 8 9";
+  // Including the empty macro expansions on the side.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("EMPTY ID ( 1 2 3 )")),
+  ElementsAre(SameRange(findExpanded("1 2 3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( 1 2 3 ) EMPTY")),
+  ElementsAre(SameRange(findExpanded("1 2 3";
+

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

2020-01-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D72498#1815500 , @lh123 wrote:

> - hover over the `front` , you'll see "instance-method `front` → 
> `std::vector >::reference`".
> - hover over the `push_back`, you'll see "`std::vector std::allocator >::value_type && __x`".

These look terrible and are the great examples where showing canonical types 
results in better output than canonical types.
I wonder why we add `std::vector>::` in the 
first place, I believe the standard library uses `value_type` in the 
declaration. Showing `value_type` is not great, but at least that doesn't 
uglify what was written in the code in the first place.
FWIW, I think the perfect output in those cases would be `int (aka value_type)`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72498

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

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

2020-01-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D72498#1814366 , @sammccall wrote:

> @ilya-biryukov @kadircet what do you think about unwrapping decltype only 
> when it's a return value (optional: of a function whose leading return type 
> is auto) to narrowly catch this idiom?

If we feel it's useful in the function return type, it's probably also useful 
in other template contexts:
E.g.

  template 
  struct X {
typedef decltype(T() + T()) add_result_type;
  };

  X::^add_result_type y;

And I don't think it's used in practice in more contexts.

Moreover, I believe usages in function returns will become more rare as 
projects move to C++14 and beyond (`auto` return type gives the same results in 
most interesting cases without code duplication).

Therefore, I'd not special-case for function return types.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72498

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

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61774 tests passed, 0 failed 
and 780 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533



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

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61771 tests passed, 0 failed 
and 780 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355



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

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

2020-01-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 237572.
nridge marked an inline comment as done.
nridge added a comment.

Address nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -409,17 +409,21 @@
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
-  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
-  // template names (e.g. "S").
+  // The parent is reported as "S" because "S<0>" is an invalid instantiation.
+  // We then iterate once more and find "S" again before detecting the
+  // recursion.
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+Parents(
+AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")),
+  Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +453,12 @@
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<2>"), WithKind(SymbolKind::Struct),
+Parents(AllOf(
+WithName("S<1>"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents(AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+  Parents()));
   Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0,
 TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
@@ -462,6 +469,83 @@
   SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
+TEST(TypeHierarchy, DeriveFromImplicitSpec) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result,
+  AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+Children(AllOf(WithName("Child"),
+   WithKind(SymbolKind::Struct), Children();
+}
+
+TEST(TypeHierarchy, DeriveFromPartialSpec) {
+  Annotations Source(R"cpp(
+  template  struct Parent {};
+  template  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result, AllOf(WithName("Parent"),
+ WithKind(SymbolKind::Struct), Children()));
+}
+
+TEST(TypeHierarchy, DeriveFromTemplate) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  template 
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  // FIXME: We'd like this to return the implicit specialization Child,
+  //but currently libIndex does not expose relationships between
+  //implicit specializations.
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

2020-01-12 Thread Nathan Ridge via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG79a09d8bf4d5: [clangd] Show template arguments in type 
hierarchy when possible (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -409,17 +409,21 @@
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
-  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
-  // template names (e.g. "S").
+  // The parent is reported as "S" because "S<0>" is an invalid instantiation.
+  // We then iterate once more and find "S" again before detecting the
+  // recursion.
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+Parents(
+AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")),
+  Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +453,12 @@
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<2>"), WithKind(SymbolKind::Struct),
+Parents(AllOf(
+WithName("S<1>"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents(AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+  Parents()));
   Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0,
 TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
@@ -462,6 +469,83 @@
   SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
+TEST(TypeHierarchy, DeriveFromImplicitSpec) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result,
+  AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+Children(AllOf(WithName("Child"),
+   WithKind(SymbolKind::Struct), Children();
+}
+
+TEST(TypeHierarchy, DeriveFromPartialSpec) {
+  Annotations Source(R"cpp(
+  template  struct Parent {};
+  template  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result, AllOf(WithName("Parent"),
+ WithKind(SymbolKind::Struct), Children()));
+}
+
+TEST(TypeHierarchy, DeriveFromTemplate) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  template 
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  // FIXME: We'd like this to return the implicit specialization Child,
+  //but currently libIndex does not expose relationships between
+  //implicit specializations.
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2,

[clang-tools-extra] 79a09d8 - [clangd] Show template arguments in type hierarchy when possible

2020-01-12 Thread Nathan Ridge via cfe-commits


Author: Nathan Ridge
Date: 2020-01-12T22:31:40-05:00
New Revision: 79a09d8bf4d508b0ae6a1e3c90907488092678c5

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

LOG: [clangd] Show template arguments in type hierarchy when possible

Summary: Fixes https://github.com/clangd/clangd/issues/31

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index fecc3668f98e..fa33c796ce29 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -672,9 +672,18 @@ const CXXRecordDecl *findRecordTypeAt(ParsedAST , 
Position Pos) {
   const SourceManager  = AST.getSourceManager();
   SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
-  DeclRelationSet Relations =
-  DeclRelation::TemplatePattern | DeclRelation::Underlying;
-  auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
+  unsigned Offset =
+  
AST.getSourceManager().getDecomposedSpellingLoc(SourceLocationBeg).second;
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  const SelectionTree::Node *N = Selection.commonAncestor();
+  if (!N)
+return nullptr;
+
+  // Note: explicitReferenceTargets() will search for both template
+  // instantiations and template patterns, and prefer the former if available
+  // (generally, one will be available for non-dependent specializations of a
+  // class template).
+  auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Underlying);
   if (Decls.empty())
 return nullptr;
 
@@ -700,6 +709,13 @@ const CXXRecordDecl *findRecordTypeAt(ParsedAST , 
Position Pos) {
 std::vector typeParents(const CXXRecordDecl *CXXRD) {
   std::vector Result;
 
+  // If this is an invalid instantiation, instantiation of the bases
+  // may not have succeeded, so fall back to the template pattern.
+  if (auto *CTSD = dyn_cast(CXXRD)) {
+if (CTSD->isInvalidDecl())
+  CXXRD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
+  }
+
   for (auto Base : CXXRD->bases()) {
 const CXXRecordDecl *ParentDecl = nullptr;
 
@@ -754,6 +770,11 @@ getTypeHierarchy(ParsedAST , Position Pos, int 
ResolveLevels,
 Result->children.emplace();
 
 if (Index) {
+  // The index does not store relationships between implicit
+  // specializations, so if we have one, use the template pattern instead.
+  if (auto *CTSD = dyn_cast(CXXRD))
+CXXRD = CTSD->getTemplateInstantiationPattern();
+
   if (Optional ID = getSymbolID(CXXRD))
 fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath);
 }

diff  --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp 
b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
index 76137850b91f..f365a141e208 100644
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -409,17 +409,21 @@ TEST(TypeHierarchy, RecursiveHierarchyUnbounded) {
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
-  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
-  // template names (e.g. "S").
+  // The parent is reported as "S" because "S<0>" is an invalid instantiation.
+  // We then iterate once more and find "S" again before detecting the
+  // recursion.
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), 
Parents();
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+Parents(
+AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")),
+  Parents(AllOf(WithName("S"), 
WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +453,12 @@ TEST(TypeHierarchy, RecursiveHierarchyBounded) {

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

2020-01-12 Thread Nathan Ridge via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ad1308b69b8: [clangd] Assert that the testcases in 
FindExplicitReferencesTest.All have no… (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1ad1308 - [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

2020-01-12 Thread Nathan Ridge via cfe-commits


Author: Nathan Ridge
Date: 2020-01-12T22:18:21-05:00
New Revision: 1ad1308b69b89cc87533c16957189a84e1dd9754

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

LOG: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have 
no diagnostics

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 263d0154c9be..408ebe24e773 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@ class FindExplicitReferencesTest : public ::testing::Test {
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@ TEST_F(FindExplicitReferencesTest, All) {
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@ TEST_F(FindExplicitReferencesTest, All) {
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -924,7 +928,7 @@ TEST_F(FindExplicitReferencesTest, All) {
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 



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

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

2020-01-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:898
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
  $12^Foo(): $13^Foo(111) {}

ilya-biryukov wrote:
> NIT: is this change redundant now? This was probably a warning, not an error.
You're right, reverted this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355



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

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

2020-01-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 237570.
nridge marked 2 inline comments as done.
nridge added a comment.

Address nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61771 tests passed, 0 failed 
and 780 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72579



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

[PATCH] D72565: adding __has_feature support for left c++ type_traits

2020-01-12 Thread kamlesh kumar via Phabricator via cfe-commits

kamleshbhalui abandoned this revision.
kamleshbhalui added a comment.

not an issue since __has_builtin does the work.
i.e. __has_builtin(__is_same)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72565



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

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

2020-01-12 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits

khchen added a subscriber: efriedma.
khchen added a comment.

Hi @efriedma 
Could you please guide me and review this PoC?
or take a look at this [[ take a look at | maillist  ]] thread?
Thank you!


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

https://reviews.llvm.org/D72245



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

[PATCH] D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

2020-01-12 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay created this revision.
MaskRay added reviewers: adalava, dim, nemanjai, jfb, rsmith.
Herald added subscribers: cfe-commits, steven.zhang, dexonsmith, krytarowski, 
arichardson, emaste.
Herald added a project: clang.

MaxAtomicPromoteWidth is defined as "the maximum width lock-free atomic
operation which will ever be supported for the given target", so an
oversized __c11_atomic_is_lock_free() or __atomic_is_lock_free() can be
evaluated to 0. This is advantageous in some scenarios (e.g. FreeBSD
powerpc, see D71600 ) to avoid the dependency 
on libatomic.

The behavior of __atomic_is_lock_free() will diverge from GCC as GCC
never evaluates it to 0 (`gcc/builtins.c:fold_builtin_atomic_always_lock_free`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72579

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/atomic-ops.c
  clang/test/CodeGen/big-atomic-ops.c
  clang/test/Sema/atomic-ops.c


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -36,23 +36,23 @@
 _Static_assert(__c11_atomic_is_lock_free(3), ""); // expected-error {{not an 
integral constant expression}}
 _Static_assert(__c11_atomic_is_lock_free(4), "");
 _Static_assert(__c11_atomic_is_lock_free(8), "");
-_Static_assert(__c11_atomic_is_lock_free(16), ""); // expected-error {{not an 
integral constant expression}}
-_Static_assert(__c11_atomic_is_lock_free(17), ""); // expected-error {{not an 
integral constant expression}}
+_Static_assert(!__c11_atomic_is_lock_free(16), "");
+_Static_assert(!__c11_atomic_is_lock_free(17), "");
 
 _Static_assert(__atomic_is_lock_free(1, 0), "");
 _Static_assert(__atomic_is_lock_free(2, 0), "");
 _Static_assert(__atomic_is_lock_free(3, 0), ""); // expected-error {{not an 
integral constant expression}}
 _Static_assert(__atomic_is_lock_free(4, 0), "");
 _Static_assert(__atomic_is_lock_free(8, 0), "");
-_Static_assert(__atomic_is_lock_free(16, 0), ""); // expected-error {{not an 
integral constant expression}}
-_Static_assert(__atomic_is_lock_free(17, 0), ""); // expected-error {{not an 
integral constant expression}}
+_Static_assert(!__atomic_is_lock_free(16, 0), "");
+_Static_assert(!__atomic_is_lock_free(17, 0), "");
 
 _Static_assert(atomic_is_lock_free((atomic_char*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_short*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_int*)0), "");
 _Static_assert(atomic_is_lock_free((atomic_long*)0), "");
 // expected-error@+1 {{__int128 is not supported on this target}}
-_Static_assert(atomic_is_lock_free((_Atomic(__int128)*)0), ""); // 
expected-error {{not an integral constant expression}}
+_Static_assert(!atomic_is_lock_free((_Atomic(__int128)*)0), "");
 _Static_assert(atomic_is_lock_free(0 + (atomic_char*)0), "");
 
 char i8;
Index: clang/test/CodeGen/big-atomic-ops.c
===
--- clang/test/CodeGen/big-atomic-ops.c
+++ clang/test/CodeGen/big-atomic-ops.c
@@ -204,9 +204,6 @@
   // CHECK: call i32 @__atomic_is_lock_free(i64 16, i8* {{.*}}@sixteen{{.*}})
   __atomic_is_lock_free(16, );
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 17, i8* {{.*}}@seventeen{{.*}})
-  __atomic_is_lock_free(17, );
-
   // CHECK: call i32 @__atomic_is_lock_free(i64 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
 
@@ -215,6 +212,8 @@
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
+  __atomic_is_lock_free(17, );
+  __atomic_is_lock_free(32, 0);
   __atomic_always_lock_free(3, 0);
   __atomic_always_lock_free(16, 0);
   __atomic_always_lock_free(17, 0);
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -346,11 +346,8 @@
   // CHECK: call i32 @__atomic_is_lock_free(i32 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 16, i8* {{.*}}@sixteen{{.*}})
-  __atomic_is_lock_free(16, );
-
-  // CHECK: call i32 @__atomic_is_lock_free(i32 17, i8* {{.*}}@seventeen{{.*}})
-  __atomic_is_lock_free(17, );
+  // CHECK: call i32 @__atomic_is_lock_free(i32 8, i8* {{.*}}@seventeen{{.*}})
+  __atomic_is_lock_free(8, );
 
   // CHECK: call i32 @__atomic_is_lock_free(i32 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
@@ -360,6 +357,8 @@
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
+  __atomic_is_lock_free(9, );
+  __atomic_is_lock_free(16, 0);
   __atomic_always_lock_free(3, 0);
   __atomic_always_lock_free(16, 0);
   __atomic_always_lock_free(17, 0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11152,6 +11152,9 @@
 
 // Check power-of-two.
 CharUnits Size = CharUnits::fromQuantity(SizeVal.getZExtValue());
+

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

2020-01-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

@aaron.ballman We could make the warning even stricter, but that should be a 
separate discussion. Is this change Ok for now?




Comment at: clang/test/Sema/warn-strict-prototypes.c:60-62
 // K function definition with previous prototype declared is not diagnosed.
 void foo11(int p, int p2);
 void foo11(p, p2) int p; int p2; {}

We might want to warn here as well, but that should be a separate change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



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

[PATCH] D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time

2020-01-12 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I am still confused why you need the special rule for `__atomic_is_lock_free` 
(GCC/clang) and `__c11_atomic_is_lock_free` (clang).

https://github.com/gcc-mirror/gcc/blob/master/gcc/builtins.c#L7300-L7314 GCC 
`__atomic_is_lock_free` expands to either 1 or a library call to 
`__atomic_is_lock_free`, never 0. We can probably extend the semantics of 
`__c11_atomic_is_lock_free` and use 
`clang::TargetInfo::getMaxAtomicPromoteWidth`, but I'll make more research in 
this area.

Can you be elaborate how do `__atomic_is_lock_free`/`__c11_atomic_is_lock_free` 
pull in libatomic.{a,so} dependency on FreeBSD powerpc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71600



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

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

2020-01-12 Thread Francois JEAN via Phabricator via cfe-commits

Wawha updated this revision to Diff 237563.
Wawha added a comment.

Here a new version of the patch with the last version of the source.
It fixes problem with inline lambda. I add more more UnitTest to test.
I also make few small changes to follow remarks.

Do not hesitate to make remarks, I hope that it will be integrate in the coming 
version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12582,6 +12582,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeLambdaBody);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
@@ -13865,6 +13866,245 @@
"function([]() { return b; }, a)", MergeInline);
   verifyFormat("function(a, []() { return b; })",
"function(a, []() { return b; })", MergeInline);
+
+  // Check option "BraceWrapping.BeforeLambdaBody" and different state of
+  // AllowShortLambdasOnASingleLine
+  FormatStyle LLVMWithBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBeforeLambdaBody.BreakBeforeBraces = FormatStyle::BS_Custom;
+  LLVMWithBeforeLambdaBody.BraceWrapping.BeforeLambdaBody = true;
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_None = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_None(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("void Fct()\n"
+   "{\n"
+   "  return {[]()\n"
+   "  {\n"
+   "return 17;\n"
+   "  }};\n"
+   "}",
+   LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_Empty;
+  verifyFormat("FctWithOneNestedLambdaInline_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmpty_SLS_Empty([]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithOneNestedLambdaEmptyInsideAVeryVeryVeryVeryVeryVeryVeryL"
+   "ongFunctionName_SLS_Empty(\n"
+   "[]() {});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("FctWithMultipleParams_SLS_Empty(A, B,\n"
+   "[]()\n"
+   "{\n"
+   "  return 17;\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("auto fct_SLS_Empty = []()\n"
+   "{\n"
+   "  return 17;\n"
+   "};",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call([]() {});\n"
+   "});",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas_SLS_Empty(A,\n"
+   "   []()\n"
+   "   {\n"
+   " return Call([]() {});\n"
+   "   });",
+   LLVMWithBeforeLambdaBody);
+  verifyFormat(
+  "FctWithLongLineInLambda_SLS_Empty(\n"
+  "[]()\n"
+  "{\n"
+  "  return

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

2020-01-12 Thread Fady Ghanim via Phabricator via cfe-commits

fghanim marked 4 inline comments as done.
fghanim added a comment.

I am working on a patch. In the mean time ;)




Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3020
+  if (AllocaIP.isSet())
+AllocaInsertPt = &*AllocaIP.getPoint();
+  auto OldReturnBlock = ReturnBlock;

jdoerfert wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > Why the `isSet` check? If we need it, then it's missing in other places 
> > > as well (I think).
> > While I was writing the response I realized that I should've made it
> > ` assert((!AllocaIP.isSet() || AllocaInsertPt == AllocaIP.getpoint()) && 
> > "Inlined directives allocas should be emitted to entry block of the inner 
> > most containing outlined function"; ` 
> > or something less verbose.
> > 
> > Inlined directives should emit their `alloca`s to the entry block of the 
> > containing outined function, right? For that reason, and since we don't 
> > store an `alloca` insertion point in the OMPBuilder, I pass an empty 
> > insertion point, and back up the current point, without changing it.
> > I was planning, in the future, to keep a copy of the most recent alloca 
> > insertion point which we set when we outline, and pass that for `BodyCB`s 
> > within inlined directives.
> I try not to make assumptions like "Inlined directives should emit their 
> allocas to the entry block of the containing outined function, right" if we 
> don't need to. If we do, the assert is probably a better way to go. I would 
> actually say we want a helper that sets/resets all these things for *all* 
> body functions. If the OMPBuilder doesn't want to set a new allocaip, it 
> should provide an "invalid" location to indicate that. Now I finally see what 
> you do here and why. Go with the assert for now, and make it `!isSet()` if 
> there is no reason to complicate it further.
I think most (if not all) llvm passes that are concerned with allocas, expect 
them to be in the entry block, this is why I made this assumption. The only 
time that I can think of, where this will not be true is in case of a delayed 
outlining. In that case, the insertion block is going to be the "future" entry 
block of the "future" outlined function, and needs to be passed. Then I think 
it is also necessary to keep track of the insertion block somewhere inside the 
OMPBuilder.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:800
+  ExitCall->removeFromParent();
+  Builder.Insert(ExitCall);
+

jdoerfert wrote:
> `ExitCall->moveBefore(IP)`, potentially with some IP = ... above.
> We should specify what the builder insert position is after this call, maybe 
> "unspecified"?
before this point, the builder is always set to insert before the terminator of 
the finalization block. when `emitInlinedFunction` is done, it sets it to 
`ExitBB.end()`



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:824
+  Out << Name;
+  StringRef RuntimeName = Out.str();
+  auto  = *InternalVars.try_emplace(RuntimeName, nullptr).first;

jdoerfert wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > This seems to be too much trouble to get a StringRef from a Twine. Maybe 
> > > start with a StringRef and avoid all of it?
> > Same as prev.
> Here, the argument that people recognize it doesn't work at all, it's a type. 
> So please pass a StringRef if there is no other reason not to.
You are right. *That* argument doesn't work. However the argument that I found 
it this way in clang kinda does. Also the argument that it makes things much 
easier to move. ;)
I assume whoever did it this way had his/her good reason. I understand you may 
not think it is, but given that it's used a lot in clang for getting and 
creating internal OMP variables, you can't really judge without going over 
every case to make sure that what you want can be done in every case, as such I 
am against the change *NOW*. However, for the time being, I included a todo 
basically saying that.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:836
+ /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal,
+ AddressSpace);
+}

jdoerfert wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > Do we really want common linkage for "internal variables"? Internal or 
> > > private would have been my first guess. Maybe the "internal" part is 
> > > different than I expect it.
> > > 
> > > ---
> > > 
> > > FWIW, the usual pattern is
> > > ```
> > > if (!Elem.second)
> > >   Elem.second = ...
> > > assert(...)
> > > return Elem.second;
> > > ```
> > same as prev.
> The code pattern is unrelated from "same as prev", I hope.
> 
> For the linkage, add a TODO to investigate if private linkage would work as 
> well.
The assert is only needed if `Elem.second` already exist. It's quite useless if 
it's not and we are creating it, right?
I'll change it to an `if..else`, and return it once.

critical name

[PATCH] D72578: [compiler-rt] [builtins] Fix clear_cache_test to work with MPROTECT

2020-01-12 Thread Michał Górny via Phabricator via cfe-commits

mgorny created this revision.
mgorny added reviewers: ro, delcypher, thakis, saugustine, rnk, compnerd.
Herald added subscribers: llvm-commits, kristof.beyls, krytarowski, dberris.
Herald added a project: LLVM.

Fix clear_cache_test to work on NetBSD with PaX MPROTECT enabled, that
is when creating W+X mmaps is prohibited.  Use the recommended solution:
create two mappings for the same memory area, make one of them RW, while
the other RX.  Copy the function into the RW area but run it from the RX
area.

In order to implement this, I've split the pointer variables to
'write_buffer' and 'execution_buffer'.  Both are separate pointers
on NetBSD, while they have the same value on other systems.

I've also split the memcpy_f() into two: new memcpy_f() that only takes
care of copying memory and discards the (known) result of memcpy(),
and realign_f() that applies ARM realignment to the given pointer.
Again, there should be no difference on non-NetBSD systems but on NetBSD
copying is done on write_buffer, while realignment on pointer
to the execution_buffer.

I have tested this change on NetBSD and Linux.


https://reviews.llvm.org/D72578

Files:
  compiler-rt/test/builtins/Unit/clear_cache_test.c


Index: compiler-rt/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/test/builtins/Unit/clear_cache_test.c
@@ -1,6 +1,6 @@
 // REQUIRES: native-run
 // UNSUPPORTED: arm, aarch64
-// RUN: %clang_builtins %s %librt -o %t && %run_nomprotect %t
+// RUN: %clang_builtins %s %librt -o %t && %run %t
 // REQUIRES: librt_has_clear_cache
 //===-- clear_cache_test.c - Test clear_cache 
-===//
 //
@@ -30,28 +30,56 @@
 static int func1() { return 1; }
 static int func2() { return 2; }
 
-void *__attribute__((noinline))
+void __attribute__((noinline))
 memcpy_f(void *dst, const void *src, size_t n) {
 // ARM and MIPS nartually align functions, but use the LSB for ISA selection
 // (THUMB, MIPS16/uMIPS respectively).  Ensure that the ISA bit is ignored in
 // the memcpy
 #if defined(__arm__) || defined(__mips__)
-  return (void *)((uintptr_t)memcpy(dst, (void *)((uintptr_t)src & ~1), n) |
-  ((uintptr_t)src & 1));
+  memcpy(dst, (void *)((uintptr_t)src & ~1), n);
+#else
+  memcpy(dst, (void *)((uintptr_t)src), n);
+#endif
+}
+
+// Realign the 'dst' pointer as if it has been returned by memcpy() above.
+// We need to split it because we're using two mappings for the same area.
+void *__attribute__((noinline))
+realign_f(void *dst, const void *src, size_t n) {
+#if defined(__arm__) || defined(__mips__)
+  return (void *)((uintptr_t)dst | ((uintptr_t)src & 1));
 #else
-  return memcpy(dst, (void *)((uintptr_t)src), n);
+  return dst;
 #endif
 }
 
 int main()
 {
 const int kSize = 128;
-#if !defined(_WIN32)
+#if defined(__NetBSD__)
+// we need to create separate RW and RX mappings to satisfy MPROTECT
+uint8_t *write_buffer = mmap(0, kSize,
+ PROT_MPROTECT(PROT_READ | PROT_WRITE |
+   PROT_EXEC),
+ MAP_ANON | MAP_PRIVATE, -1, 0);
+if (write_buffer == MAP_FAILED)
+  return 1;
+uint8_t *execution_buffer = mremap(write_buffer, kSize, NULL, kSize,
+   MAP_REMAPDUP);
+if (execution_buffer == MAP_FAILED)
+  return 1;
+
+if (mprotect(write_buffer, kSize, PROT_READ | PROT_WRITE) == -1)
+  return 1;
+if (mprotect(execution_buffer, kSize, PROT_READ | PROT_EXEC) == -1)
+  return 1;
+#elif !defined(_WIN32)
 uint8_t *execution_buffer = mmap(0, kSize,
  PROT_READ | PROT_WRITE | PROT_EXEC,
  MAP_ANON | MAP_PRIVATE, -1, 0);
 if (execution_buffer == MAP_FAILED)
   return 1;
+uint8_t *write_buffer = execution_buffer;
 #else
 HANDLE mapping = CreateFileMapping(INVALID_HANDLE_VALUE, NULL,
PAGE_EXECUTE_READWRITE, 0, kSize, NULL);
@@ -62,16 +90,19 @@
 mapping, FILE_MAP_ALL_ACCESS | FILE_MAP_EXECUTE, 0, 0, 0);
 if (execution_buffer == NULL)
 return 1;
+uint8_t *write_buffer = execution_buffer;
 #endif
 
 // verify you can copy and execute a function
-pfunc f1 = (pfunc)memcpy_f(execution_buffer, func1, kSize);
+memcpy_f(write_buffer, func1, kSize);
+pfunc f1 = (pfunc)realign_f(execution_buffer, func1, kSize);
 __clear_cache(execution_buffer, execution_buffer + kSize);
 if ((*f1)() != 1)
 return 1;
 
 // verify you can overwrite a function with another
-pfunc f2 = (pfunc)memcpy_f(execution_buffer, func2, kSize);
+memcpy_f(write_buffer, func2, kSize);
+pfunc f2 = (pfunc)realign_f(execution_buffer, func2, kSize);
 __clear_cache(execution_buffer, execution_buffer + kSize);
 if ((*f2)() != 2)

[clang] 54b2914 - Fix "pointer is null" static analyzer warnings. NFCI.

2020-01-12 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-01-12T22:08:56Z
New Revision: 54b2914accb4f5c9b58305fd6da405d20a47c452

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

LOG: Fix "pointer is null" static analyzer warnings. NFCI.

Use castAs<> instead of getAs<> since the pointers are dereferenced immediately 
and castAs will perform the null assertion for us.

Added: 


Modified: 
clang/lib/AST/VTableBuilder.cpp

Removed: 




diff  --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index 5688042dadd9..2b5b74be5961 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -270,8 +270,8 @@ static BaseOffset
 ComputeReturnAdjustmentBaseOffset(ASTContext ,
   const CXXMethodDecl *DerivedMD,
   const CXXMethodDecl *BaseMD) {
-  const FunctionType *BaseFT = BaseMD->getType()->getAs();
-  const FunctionType *DerivedFT = DerivedMD->getType()->getAs();
+  const auto *BaseFT = BaseMD->getType()->castAs();
+  const auto *DerivedFT = DerivedMD->getType()->castAs();
 
   // Canonicalize the return types.
   CanQualType CanDerivedReturnType =



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

[clang] ada22c8 - Fix "pointer is null" static analyzer warning. NFCI.

2020-01-12 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-01-12T21:48:00Z
New Revision: ada22c804cd956f3ee7cc9dc82e6d54ead8a4ffe

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

LOG: Fix "pointer is null" static analyzer warning. NFCI.

Added: 


Modified: 
clang/include/clang/Basic/SourceManager.h

Removed: 




diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index ec1b0bcf9897..d87e9ac810fa 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -831,6 +831,7 @@ class SourceManager : public RefCountedBase {
   FileID createFileID(const FileEntry *SourceFile, SourceLocation IncludePos,
   SrcMgr::CharacteristicKind FileCharacter,
   int LoadedID = 0, unsigned LoadedOffset = 0) {
+assert(SourceFile && "Null source file!");
 const SrcMgr::ContentCache *IR =
 getOrCreateContentCache(SourceFile, isSystem(FileCharacter));
 assert(IR && "getOrCreateContentCache() cannot return NULL");



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

[PATCH] D72373: [clang-tidy] extend misc-misplaced-const to detect using besides typedef

2020-01-12 Thread Alexander Lanin via Phabricator via cfe-commits

AlexanderLanin updated this revision to Diff 237558.
AlexanderLanin marked an inline comment as done.
AlexanderLanin added a comment.

Updated revision with llvm_unreachable


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

https://reviews.llvm.org/D72373

Files:
  clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp
@@ -1,20 +1,40 @@
-// RUN: %check_clang_tidy %s misc-misplaced-const %t
-
-typedef int plain_i;
-typedef int *ip;
-typedef const int *cip;
-
-void func() {
-  if (const int *i = 0)
-;
-  if (const plain_i *i = 0)
-;
-  if (const cip i = 0)
-;
-
-  // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: 'i' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *'
-  if (const ip i = 0)
-;
+// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -DUSING
+// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -DTYPEDEF
+
+#ifdef TYPEDEF
+typedef int int_;
+typedef int *ptr_to_int;
+typedef const int *ptr_to_const_int;
+#endif
+#ifdef USING
+using int_ = int;
+using ptr_to_int = int *;
+using ptr_to_const_int = const int *;
+#endif
+
+void const_pointers() {
+  if (const int *i = 0) {
+i = 0;
+// *i = 0;
+  }
+
+  if (const int_ *i = 0) {
+i = 0;
+// *i = 0;
+  }
+
+  if (const ptr_to_const_int i = 0) {
+// i = 0;
+// *i = 0;
+  }
+
+  // Potentially quite unexpectedly the int can be modified here
+  // CHECK-MESSAGES: :[[@LINE+1]]:23: warning: 'i' declared with a const-qualified {{.*}}; results in the type being 'int *const' instead of 'const int *'
+  if (const ptr_to_int i = 0) {
+//i = 0;
+
+*i = 0;
+  }
 }
 
 template 
@@ -24,8 +44,8 @@
 };
 
 template struct S;
-template struct S; // ok
-template struct S;
+template struct S; // ok
+template struct S;
 
 template 
 struct U {
Index: clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
@@ -3,10 +3,10 @@
 misc-misplaced-const
 
 
-This check diagnoses when a ``const`` qualifier is applied to a ``typedef`` to a
-pointer type rather than to the pointee, because such constructs are often
-misleading to developers because the ``const`` applies to the pointer rather
-than the pointee.
+This check diagnoses when a ``const`` qualifier is applied to a ``typedef``/
+``using`` to a pointer type rather than to the pointee, because such constructs
+are often misleading to developers because the ``const`` applies to the pointer
+rather than the pointee.
 
 For instance, in the following code, the resulting type is ``int *`` ``const``
 rather than ``const int *``:
@@ -14,9 +14,12 @@
 .. code-block:: c++
 
   typedef int *int_ptr;
-  void f(const int_ptr ptr);
+  void f(const int_ptr ptr) {
+*ptr = 0; // potentially quite unexpectedly the int can be modified here
+ptr = 0; // does not compile
+  }
 
-The check does not diagnose when the underlying ``typedef`` type is a pointer to
-a ``const`` type or a function pointer type. This is because the ``const``
-qualifier is less likely to be mistaken because it would be redundant (or
-disallowed) on the underlying pointee type.
+The check does not diagnose when the underlying ``typedef``/``using`` type is a
+pointer to a ``const`` type or a function pointer type. This is because the
+``const`` qualifier is less likely to be mistaken because it would be redundant
+(or disallowed) on the underlying pointee type.
Index: clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
@@ -17,13 +17,16 @@
 namespace misc {
 
 void MisplacedConstCheck::registerMatchers(MatchFinder *Finder) {
+  auto NonConstAndNonFunctionPointerType = hasType(pointerType(unless(
+  pointee(anyOf(isConstQualified(), ignoringParens(functionType()));
+
   Finder->addMatcher(
-  valueDecl(hasType(isConstQualified()),
-hasType(typedefType(hasDeclaration(
-typedefDecl(hasType(pointerType(unless(pointee(
-anyOf(isConstQualified(),
-  ignoringParens(functionType(
-.bind("typedef")
+  valueDecl(
+

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

I think the fact that this is a fourth (?) different incarnation of a patch
trying to solve the same *major* *ugly* problem, it may be an evidence that
perhaps this problem should not be approached from the 'let's hack it together'
approach, but with a concrete plan sent as an RFC to cfe-dev :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Another potential solution is leave is as is by default. Then emit a warning if 
a check is ran twice. The warning could be suppressed by passing a option to 
say what behaviour you want?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

2020-01-12 Thread Alexandre Ganea via Phabricator via cfe-commits

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



Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:269
+  const_cast(CRCI)->HandleCrash(
+  (int)ExceptionInfo->ExceptionRecord->ExceptionCode, ExceptionInfo);
 

mstorsjo wrote:
> This implicitly converts a pointer to `uintptr_t`, which is an error by 
> default in mingw mode; a cast is needed.
Fixed by rGde797ccdd74f46d5f637ccf66c78da9905a46f42


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70568



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

[PATCH] D72560: Add -Wrange-loop-analysis changes to ReleaseNotes

2020-01-12 Thread Mark de Wever via Phabricator via cfe-commits

Mordante closed this revision.
Mordante added a comment.

Committed as https://reviews.llvm.org/rGdc422e968e73790178e500f506e8fb7cfa1e62ea

I accidentally forgot to add the revision id.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72560



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

[PATCH] D72565: adding __has_feature support for left c++ type_traits

2020-01-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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



Comment at: clang/include/clang/Basic/Features.def:191-192
 // Type traits
 // N.B. Additional type traits should not be added to the following list.
 // Instead, they should be detected by has_builtin.
 FEATURE(has_nothrow_assign, LangOpts.CPlusPlus)

Please see this comment. `__has_builtin` should be used to detect these type 
traits, per the documentation 
(https://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives and 
https://clang.llvm.org/docs/LanguageExtensions.html#has-builtin), and already 
works for this purpose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72565



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

[PATCH] D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time

2020-01-12 Thread Nemanja Ivanovic via Phabricator via cfe-commits

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

LGTM.




Comment at: clang/lib/AST/ExprConstant.cpp:11028
+// The lock free possibilities on this platform are covered by the lines 
+// above and we know in advance other cases require lock
+if (Info.Ctx.getTargetInfo().getTriple().getArch() == llvm::Triple::ppc) {

Please add to this comment that this may need to be restricted to specific 
operating systems in the future as some (perhaps AIX) might provide libatomic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71600



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

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

2020-01-12 Thread Florin Iucha via Phabricator via cfe-commits

0x8000- added a comment.

In D54943#1815916 , @Eugene.Zelenko 
wrote:

> In D54943#1815912 , @0x8000- 
> wrote:
>
> > As an aside, once this is merged in, I dream of a "fix-it" for old style C 
> > code:
> >
> >   int foo;
> >  
> >   ...
> >   /* page of code which does not use either foo or bar */
> >   ...
> >   foo = 5;
> >
> >
> > Moves the foo declaration to the line where it is actually first 
> > initialized. Then run the constify tool on top.
>
>
> You are not first :-)  See PR21983.


There's https://reviews.llvm.org/D64671 which solves half the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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

[PATCH] D72565: adding __has_feature support for left c++ type_traits

2020-01-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

Probably needs test coverage (perhaps check the commits that added other 
feature tests - there's probably somewhere you can plugin tests in a similar 
way to plugging in the feature tests themselves)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72565



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

How does this sound? 
I'll put in a config option in there to disable the new alias behaviour.
Personally I'd say default to the new behaviour. Then if an organisation
relies on the old behaviour then can revert back with a command line option
or .clang-tidy file edit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D72566#1815913 , @njames93 wrote:

> In D72566#1815904 , @lebedev.ri 
> wrote:
>
> > I agree that the current alias situation is not ideal, though i'm not sure
> >  how much we can fix it since it crosses user interface boundary
> >  (i.e. what fixes won't lead to some regressions from the previous expected 
> > behavior)
>
>
>   If you're expecting a test to run twice usually something has gone wrong

I'm not certain I agree. Many of the aliases have different sets of default 
checking options from the primary check. Running the "same" check twice in 
these cases produces different output. How do you intend to handle situations 
like that? If the answer is "disable the aliases", then this has the potential 
to lose valuable coverage for some folks who are asking for all checks to be 
enabled.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566

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

[clang] ad20169 - Fix "pointer is null" static analyzer warnings. NFCI.

2020-01-12 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-01-12T14:36:59Z
New Revision: ad201691d5cc0f15f6f885f3847dcc6440ee3de5

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

LOG: Fix "pointer is null" static analyzer warnings. NFCI.

Use cast<> instead of dyn_cast<> and move into its users where its dereferenced 
immediately.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 0cf7056a0783..21c4bbc60264 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1006,12 +1006,9 @@ ProgramStateRef 
CStringChecker::InvalidateBuffer(CheckerContext ,
 
 bool CStringChecker::SummarizeRegion(raw_ostream , ASTContext ,
  const MemRegion *MR) {
-  const TypedValueRegion *TVR = dyn_cast(MR);
-
   switch (MR->getKind()) {
   case MemRegion::FunctionCodeRegionKind: {
-const NamedDecl *FD = cast(MR)->getDecl();
-if (FD)
+if (const auto *FD = cast(MR)->getDecl())
   os << "the address of the function '" << *FD << '\'';
 else
   os << "the address of a function";
@@ -1025,16 +1022,20 @@ bool CStringChecker::SummarizeRegion(raw_ostream , 
ASTContext ,
 return true;
   case MemRegion::CXXThisRegionKind:
   case MemRegion::CXXTempObjectRegionKind:
-os << "a C++ temp object of type " << TVR->getValueType().getAsString();
+os << "a C++ temp object of type "
+   << cast(MR)->getValueType().getAsString();
 return true;
   case MemRegion::VarRegionKind:
-os << "a variable of type" << TVR->getValueType().getAsString();
+os << "a variable of type"
+   << cast(MR)->getValueType().getAsString();
 return true;
   case MemRegion::FieldRegionKind:
-os << "a field of type " << TVR->getValueType().getAsString();
+os << "a field of type "
+   << cast(MR)->getValueType().getAsString();
 return true;
   case MemRegion::ObjCIvarRegionKind:
-os << "an instance variable of type " << TVR->getValueType().getAsString();
+os << "an instance variable of type "
+   << cast(MR)->getValueType().getAsString();
 return true;
   default:
 return false;



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

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

2020-01-12 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

In D54943#1815912 , @0x8000- wrote:

> As an aside, once this is merged in, I dream of a "fix-it" for old style C 
> code:
>
>   int foo;
>  
>   ...
>   /* page of code which does not use either foo or bar */
>   ...
>   foo = 5;
>
>
> Moves the foo declaration to the line where it is actually first initialized. 
> Then run the constify tool on top.


You are not first :-)  See PR21983.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D72566#1815913 , @njames93 wrote:

> In D72566#1815904 , @lebedev.ri 
> wrote:
>
> > I agree that the current alias situation is not ideal, though i'm not sure
> >  how much we can fix it since it crosses user interface boundary
> >  (i.e. what fixes won't lead to some regressions from the previous expected 
> > behavior)
>
>
>   If you're expecting a test to run twice usually something has gone wrong


That is not what i'm talking about.
I'm mainly worried about checks with custom config options:
It is likely that said options were specified only for one check,
not all of it's aliases too. So now the fact whether or not
these options will be used will depend on the registering order of checks.

>> To be noted, this will make adding of new aliases in non-up-stream modules 
>> more involved,
>>  since now the aliases will no longer be confined to the new module, but to 
>> the original modules.
> 
> That's how it currently is for adding new alias definitions. 
>  When you create an alias definition now you need the implementation files 
> available.
>  However this approach doesn't need to modify the original module and appears 
> mostly transparent to it.
>  Its also made in such a way that if you set a check as aliasing to a now 
> existent name, it would just give you another way to invoke said check
> 
>> If we proceed as-is, i think we also need one more config switch 
>> "CheckAliasesAreEnabled = 1".
>>  And we should completely ignore aliases unless it's enabled.
> 
> That does sound like something I can get behind
> 
> There is one major nit I can think of and that is if you create an alias to 
> the wrong check it would overwrite said check. I may look into adjusting that




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

In D72566#1815904 , @lebedev.ri wrote:

> I agree that the current alias situation is not ideal, though i'm not sure
>  how much we can fix it since it crosses user interface boundary
>  (i.e. what fixes won't lead to some regressions from the previous expected 
> behavior)

If you're expecting a test to run twice usually something has gone wrong

> To be noted, this will make adding of new aliases in non-up-stream modules 
> more involved,
>  since now the aliases will no longer be confined to the new module, but to 
> the original modules.

That's how it currently is for adding new alias definitions. 
When you create an alias definition now you need the implementation files 
available.
However this approach doesn't need to modify the original module and appears 
mostly transparent to it.
Its also made in such a way that if you set a check as aliasing to a now 
existent name, it would just give you another way to invoke said check

> If we proceed as-is, i think we also need one more config switch 
> "CheckAliasesAreEnabled = 1".
>  And we should completely ignore aliases unless it's enabled.

That does sound like something I can get behind

There is one major nit I can think of and that is if you create an alias to the 
wrong check it would overwrite said check. I may look into adjusting that

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566

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

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

2020-01-12 Thread Florin Iucha via Phabricator via cfe-commits

0x8000- added a comment.

As an aside, once this is merged in, I dream of a "fix-it" for old style C code:

  int foo;
  
  ...
  /* page of code which does not use either foo or bar */
  ...
  foo = 5;

Moves the foo declaration to the line where it is actually first initialized. 
Then run the constify tool on top.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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

[PATCH] D72568: [clang-tidy] Alias defintions for checks

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61774 tests passed, 0 failed 
and 780 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72568



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

I agree that the current alias situation is not ideal, though i'm not sure
how much we can fix it since it crosses user interface boundary
(i.e. what fixes won't lead to some regressions from the previous expected 
behavior)

To be noted, this will make adding of new aliases in non-up-stream modules more 
involved,
since now the aliases will no longer be confined to the new module, but to the 
original modules.

Also, there was previously some attempts at this:
https://lists.llvm.org/pipermail/cfe-dev/2017-September/055589.html
D25659 
D38171 
???

If we proceed as-is, i think we also need one more config switch 
"CheckAliasesAreEnabled = 1".
And we should completely ignore aliases unless it's enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72568: [clang-tidy] Alias defintions for checks

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, JonasToth.
njames93 added projects: clang, clang-tools-extra.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
njames93 added a parent revision: D72566: [clang-tidy] Clang tidy is now alias 
aware.
Herald added a subscriber: wuzish.

Child of https://reviews.llvm.org/D72566 - just implementing the checks and a 
test case


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72568

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
@@ -1,15 +1,30 @@
-// RUN: %check_clang_tidy %s cert-err09-cpp,cert-err61-cpp %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s "cert-err09-cpp,cert-err61-cpp, \
+// RUN: google-readability-braces-around-statements, \
+// RUN: readability-braces-around-statements, \
+// RUN: hicpp=braces-around-statements" %t -- -- -fexceptions
 
 void alwaysThrows() {
   int ex = 42;
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp]
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err61-cpp]
   throw ex;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead
+  // ^^ Dont specify which check as could be either.
+  // Original name is misc-throw-by-value-catch-by-reference.
 }
 
 void doTheJob() {
   try {
 alwaysThrows();
-  } catch (int&) {
+  } catch (int &) {
   }
 }
+
+void bracesStatements() {
+  for (;;)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: statement should be inside braces [readability-braces-around-statements]
+  // Specify check name as readability-braces-around-statements is the non
+  // aliased name of the check
+  // CHECK-FIXES:  {{^}}  for (;;) {
+  // CHECK-FIXES-NEXT: {{^}};
+  // CHECK-FIXES-NEXT: {{^}}}
+}
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -102,7 +102,7 @@
 "readability-static-definition-in-anonymous-namespace");
 CheckFactories.registerCheck(
 "readability-string-compare");
-CheckFactories.registerCheck(
+CheckFactories.registerCheck(
 "readability-named-parameter");
 CheckFactories.registerCheck(
 "readability-non-const-parameter");
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -26,7 +26,7 @@
 CheckFactories.registerCheck("llvm-header-guard");
 CheckFactories.registerCheck("llvm-include-order");
 CheckFactories.registerCheck(
-"llvm-namespace-comment");
+"llvm-namespace-comment", "readability-namespace-comment");
 CheckFactories.registerCheck(
 "llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -49,64 +49,67 @@
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
-"hicpp-avoid-c-arrays");
+"hicpp-avoid-c-arrays", "modernize-avoid-c-arrays");
 CheckFactories.registerCheck(
-"hicpp-avoid-goto");
+"hicpp-avoid-goto", "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
-"hicpp-braces-around-statements");
+"hicpp-braces-around-statements",
+"readability-braces-around-statements");
 CheckFactories.registerCheck(
-"hicpp-deprecated-headers");
+"hicpp-deprecated-headers", "modernize-deprecated-headers");
 CheckFactories.registerCheck(
 "hicpp-exception-baseclass");
 CheckFactories.registerCheck(

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

should warning be printed with all the enabled checks, or just the primary name?

  warning: statement should be inside braces 
[readability-braces-around-statements]

vs

  warning: statement should be inside braces 
[readability-braces-around-statements, hicpp-braces-around-statements]
  warning: statement should be inside braces 
[readability-braces-around-statements], [hicpp-braces-around-statements]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61774 tests passed, 0 failed 
and 780 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72566



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

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, JonasToth, hokein.
njames93 added projects: clang-tools-extra, clang.
Herald added a subscriber: xazax.hun.

Clang-tidy has had an on growing issue with blindly adding all checks from a 
module using the `-checks=somemodule-* ` option. If somemodule alias' another 
enabled check. the check will get ran twice giving duplicated warning and fix 
options. This can lead to fixes overlapping preventing them from being applied 
or worse still applying twice resulting in malformed code. There are other 
issues with ConfigOptions being different in the 2 different instances so one 
instance of the check may be running a different set of options to the other.

This patch solves these issue by making clang-tidy only run 1 instance of an 
aliased check even if it has been turned on with multiple names. Warning 
messages are appended with the original check name if it has been enabled(via 
cmd line, or config file). If the original check hasn't been enabled then the 
name will be the first enabled alias that was registered.

Config options are also handled in a similar fashion, see example

  [{ key: MyAliasedName.Option1, value: 0 }.
  { key: OriginalCheckName.Option1, value: 1 }]

If the check `original-check-name` was turned on, then `Option1` will be parsed 
as being `1`, otherwise it ignores `OriginalCheckName.Option1` key and falls 
back to the first enabled alias check that declares `Option1`

To declare a check as an alias when you register then check, just add the check 
this alias' to as a second parameter

  CheckFactories.registerCheck(
  "hicpp-braces-around-statements");
  // Update to
  CheckFactories.registerCheck(
  "hicpp-braces-around-statements", "readability-braces-around-statements");

I haven't added in all the alias declarations in just yet (If you want me to 
just ask), nor have I added test cases. However once all the alias declarations 
are added in, the tests fall nicely in line with the current test 
infrastructure.
Adding this line to the readability-braces-around-statements checks causes no 
errors, no multiple fix attempts or warning messages.

  // RUN: %check_clang_tidy %s 
readability-braces-around-statements,hicpp-readability-braces-around-statements 
%t

Likewise changing it to

  // RUN: %check_clang_tidy %s hicpp-readability-braces-around-statements %t

Also runs without a hitch (obviously warning messages are appended with 
`[hicpp-readability-braces-around-statements]` instead of 
`[readability-braces-around-statements]` as above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72566

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyModule.cpp
  clang-tools-extra/clang-tidy/ClangTidyModule.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
 Improvements to clang-tidy
 --
 
+- Clang tidy is now aware of alias checks and will only run one instance of
+  an alias check that's enabled by different names.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyModule.h
===
--- clang-tools-extra/clang-tidy/ClangTidyModule.h
+++ clang-tools-extra/clang-tidy/ClangTidyModule.h
@@ -11,6 +11,7 @@
 
 #include "ClangTidy.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
 #include 
 #include 
 #include 
@@ -34,6 +35,13 @@
   /// For all checks that have default constructors, use \c registerCheck.
   void registerCheckFactory(StringRef Name, CheckFactory Factory);
 
+  /// Registers check \p Factory with name \p Name that is an alias to
+  /// another check called \p AliasToName.
+  ///
+  /// For all checks that have default constructors, use \c registerCheck.
+  void registerCheckFactory(StringRef Name, StringRef AliasToName,
+CheckFactory Factory);
+
   /// Registers the \c CheckType with the name \p Name.
   ///
   /// This method should be used for all \c ClangTidyChecks that don't require
@@ -62,6 +70,16 @@
  });
   }
 
+  /// Registers the \c CheckType with the name \p Name that is an alias to
+  /// another check called \p AliasToName.
+  template 
+  void registerCheck(StringRef CheckName, StringRef AliasToName) {
+registerCheckFactory(CheckName, AliasToName,
+ [](StringRef Name, ClangTidyContext *Context) {
+   return std::make_unique(Name, Context);
+ });
+  }
+
   /// Create instances of checks that are enabled.
   std::vector>
   createChecks(ClangTidyContext *Context);
@@

[PATCH] D72565: adding __has_feature support for left c++ type_traits

2020-01-12 Thread kamlesh kumar via Phabricator via cfe-commits

kamleshbhalui created this revision.
kamleshbhalui added reviewers: rsmith, aaron.ballman, dblaikie, ldionne.
kamleshbhalui added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

adding __has_feature support for left c++ type_traits.

fixes this
https://bugs.llvm.org/show_bug.cgi?id=44517


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72565

Files:
  clang/include/clang/Basic/Features.def


Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -216,6 +216,10 @@
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
+FEATURE(is_same, LangOpts.CPlusPlus)
+FEATURE(is_function, LangOpts.CPlusPlus)
+FEATURE(is_assignable, LangOpts.CPlusPlus)
+FEATURE(is_nothrow_constructible, LangOpts.CPlusPlus)
 FEATURE(modules, LangOpts.Modules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
 FEATURE(shadow_call_stack,


Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -216,6 +216,10 @@
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
+FEATURE(is_same, LangOpts.CPlusPlus)
+FEATURE(is_function, LangOpts.CPlusPlus)
+FEATURE(is_assignable, LangOpts.CPlusPlus)
+FEATURE(is_nothrow_constructible, LangOpts.CPlusPlus)
 FEATURE(modules, LangOpts.Modules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
 FEATURE(shadow_call_stack,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-01-12 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas added a comment.

In D71174#1815830 , @lebedev.ri wrote:

> A little bit late to the party, but still.
>  I'm honestly wondering if this should be a proper clang static analyzer 
> data-flow aware check.
>  This is basically diagnosing every `signed char` -> `signed int` promotion, 
> regardless of whether 
>  the `char` is used as an ASCII char or not. It really seems like a wrong, 
> noise-filled approach..

I've considered creating a static analyzer check instead before I implemented 
this clang-tidy check
and it seemed to me path analysis would not help too much. At least in those 
cases, what I found
in LibreOffice codebase, there was no additional knowledge in the code about 
the character's value range.
(Actually, in one use case the method name contained "ASCII", but this 
information can't be used by the
static analyzer I guess.)
For example, when the code reads a file it's not known whether the file will 
contain only ASCII characters
or not. Or if you have a function working with characters (without having any 
explicit check in it about
character value range), then the static analyzer won't help there anyway.
I'm not sure how many test cases can be filtered out with a static analyzer 
check in practice, but I feel like
it's not much more effective than a clang-tidy check in this case. As I recall 
path analysis stops working if it
reaches a loop, right? It loses the information from before the loop or 
something like that, which makes a static
analyzer check even less useful, because characters are rare to go alone.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174

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

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

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

A little bit late to the party, but still.
I'm honestly wondering if this should be a proper clang static analyzer 
data-flow aware check.
This is basically diagnosing every `signed char` -> `signed int` promotion, 
regardless of whether 
the `char` is used as an ASCII char or not. It really seems like a wrong, 
noise-filled approach..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

2020-01-12 Thread Gabor Bencze via Phabricator via cfe-commits

gbencze added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:82-83
+
+static bool hasPadding(const ASTContext , const RecordDecl *RD,
+   uint64_t ComparedBits) {
+  assert(RD && RD->isCompleteDefinition() &&

aaron.ballman wrote:
> I am surprised that we have to do this much manual work, I would have 
> expected this to be something that `RecordLayout` would handle for us. I am a 
> bit worried about this manual work being split in a few places because we're 
> likely to get it wrong in at least one of them. For instance, this doesn't 
> seem to be taking into account things like `[[no_unique_address]]` or 
> alignment when considering the layout of fields.
> 
> I sort of wonder if we should try using the `RecordLayout` class to do this 
> work instead, even if that requires larger changes.
Thanks, I didn't realize the [[no_unique_address]] attribute existed but I 
fixed it (in this check for now) and also added some tests to cover it as well 
as alignas. 

If you think this information should be provided by RecordLayout I'm willing to 
work on that as well (though I guess those changes should be in a different 
patch). Do you have any ideas as to how it should expose the necessary 
information? 


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

https://reviews.llvm.org/D71973



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

2020-01-12 Thread Gabor Bencze via Phabricator via cfe-commits

gbencze updated this revision to Diff 237529.
gbencze added a comment.

Address (most of the) comments by @aaron.ballman

- remove top-level `const` on locals
- move declaration into `if`
- pass `TagDecl` to diag
- added test for `operator void *`
- fixed `[[no_unique_address]]`
  - remove assertion that checks for overlapping fields
  - in `hasPaddingBetweenFields`: only do recursive call and add field size to 
`TotalSize` if not `isZeroSize`
  - + added tests


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

https://reviews.llvm.org/D71973

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,228 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+} // namespace std
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+  int i;
+
+public:
+  virtual void f();
+};
+
+void f(C , C ) {
+  if (!std::memcmp(, , sizeof(C))) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+// of non-standard-layout type sei_cert_example_oop57_cpp::C; consider
+// using a comparison operator instead
+  }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace inner_padding_64bit_only {
+struct S {
+  int x;
+  int *y;
+};
+
+void test() {
+  S a, b;
+  std::memcmp(, , sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // inner_padding_64bit_only::S; consider comparing the fields manually
+}
+} // namespace inner_padding_64bit_only
+
+namespace padding_in_base {
+class Base {
+  char c;
+  int i;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+  Derived a, b;
+  std::memcmp(, , sizeof(char));
+  std::memcmp(, , sizeof(Base));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived; consider comparing the fields manually
+  std::memcmp(, , sizeof(Derived));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived; consider comparing the fields manually
+}
+
+void testDerived2() {
+  Derived2 a, b;
+  std::memcmp(, , sizeof(char));
+  std::memcmp(, , sizeof(Base));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived2; consider comparing the fields manually
+  std::memcmp(, , sizeof(Derived2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived2; consider comparing the fields manually
+}
+
+} // namespace padding_in_base
+
+namespace no_padding_in_base {
+class Base {
+  int a, b;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+  Derived a, b;
+  std::memcmp(, , sizeof(Base));
+  std::memcmp(, , sizeof(Derived));
+}
+
+void testDerived2() {
+  Derived2 a, b;
+  std::memcmp(, , sizeof(char));
+  std::memcmp(, , sizeof(Base));
+  std::memcmp(, , sizeof(Derived2));
+}
+} // namespace no_padding_in_base
+
+namespace non_standard_layout {
+class C {
+private:
+  int x;
+
+public:
+  int y;
+};
+
+void test() {
+  C a, b;
+  std::memcmp(, , sizeof(C));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation
+  // of non-standard-layout type non_standard_layout::C; consider using a
+  // comparison operator instead
+}
+
+} // namespace non_standard_layout
+
+namespace static_ignored {
+struct S {
+  static char c;
+  int i;
+};
+
+void test() {
+  S a, b;
+  std::memcmp(, , sizeof(S));
+}
+} // namespace static_ignored
+
+namespace operator_void_ptr {
+struct S {
+  operator void *() const;
+};
+
+void test() {
+  S s;
+  std::memcmp(s, s, sizeof(s));
+}
+} // namespace operator_void_ptr
+
+namespace empty_struct {
+struct S {};
+
+void test() {
+  S a, b;
+

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

2020-01-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D65591#1791876 , @rsmith wrote:

> @ilya-biryukov Did I forget anything?


SG, I don't think anything is missing. Thanks for the write-up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

57 matches

Mail list logo