[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 211425.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Delete -Wmost from the hierarchy. It doesn't belong there


Repository:
  rC Clang

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

https://reviews.llvm.org/D65192

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags-enabled.c
  test/Parser/cxx2a-spaceship.cpp
  test/SemaCXX/parentheses.cpp


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: test/Misc/warning-flags-enabled.c
===
--- test/Misc/warning-flags-enabled.c
+++ test/Misc/warning-flags-enabled.c
@@ -36,7 +36,7 @@
 
 // Test if -Wshift-op-parentheses is a subgroup of -Wparentheses
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses 
-Wshift-op-parentheses %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES 
%s
-// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix 
CHECK-SHIFT-OP-PARENTHESES %s
+// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix 
CHECK-NO-SHIFT-OP-PARENTHESES %s
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses %s | FileCheck 
--check-prefix CHECK-NO-SHIFT-OP-PARENTHESES %s
 //
 // CHECK-SHIFT-OP-PARENTHESES: -Wshift-op-parentheses
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5639,21 +5639,21 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
   "comparison operator">,
-  InGroup;
+  InGroup, DefaultIgnore;
 def note_evaluate_comparison_first :Note<
   "place parentheses around comparison expression to evaluate it first">;
 
 def warn_addition_in_bitshift : Warning<
   "operator '%0' has lower precedence than '%1'; "
-  "'%1' will be evaluated first">, InGroup;
+  "'%1' will be evaluated first">, InGroup, DefaultIgnore;
 
 def warn_self_assignment_builtin : Warning<
   "explicitly assigning value of variable of type %0 to itself">,


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: test/Misc/warning-flags-enabled.c
===
--- test/Misc/warning-flags-enabled.c
+++ test/Misc/warning-flags-enabled.c
@@ -36,7 +36,7 @@
 
 // Test if -Wshift-op-parentheses is a subgroup of -Wparentheses
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses -Wshift-op-parentheses %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES %s
-// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES %s
+// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix CHECK-NO-SHIFT-OP-PARENTHESES %s
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses %s | FileCheck --check-prefix CHECK-NO-SHIFT-OP-PARENTHESES %s
 //
 // CHECK-SHIFT-OP-PARENTHESES: -Wshift-op-parentheses
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5639,21 +5639,21 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within 

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: akyrtzi, jyknight, rtrieu, rsmith.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

The -Wparentheses warnings are enabled by default in clang but they are under
-Wall in gcc (gcc/c-family/c.opt). Many of the operator precedence warnings are
oftentimes criticized as noise (clang: default; gcc: -Wall). If a warning is
very controversial, it is probably not a good idea to enable it by default.
This patch disables the rather annoying ones:

-Wbitwise-op-parentheses, e.g. i & i | i
-Wlogical-op-parentheses, e.g. i && i || i
-Wshift-op-parentheses (and -Woverloaded-shift-op-parentheses), e.g. i+i << i

After this change:

  * = enabled by default
  
  -Wall
-Wmost
  -Wparentheses
-Wlogical-op-parentheses
-Wlogical-not-parentheses*
-Wbitwise-op-parentheses
-Wshift-op-parentheses
-Woverloaded-shift-op-parentheses
-Wparentheses-equality*
-Wdangling-else*


Repository:
  rC Clang

https://reviews.llvm.org/D65192

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags-enabled.c
  test/Parser/cxx2a-spaceship.cpp
  test/SemaCXX/parentheses.cpp


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: test/Misc/warning-flags-enabled.c
===
--- test/Misc/warning-flags-enabled.c
+++ test/Misc/warning-flags-enabled.c
@@ -36,7 +36,7 @@
 
 // Test if -Wshift-op-parentheses is a subgroup of -Wparentheses
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses 
-Wshift-op-parentheses %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES 
%s
-// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix 
CHECK-SHIFT-OP-PARENTHESES %s
+// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix 
CHECK-NO-SHIFT-OP-PARENTHESES %s
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses %s | FileCheck 
--check-prefix CHECK-NO-SHIFT-OP-PARENTHESES %s
 //
 // CHECK-SHIFT-OP-PARENTHESES: -Wshift-op-parentheses
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5639,21 +5639,21 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
   "comparison operator">,
-  InGroup;
+  InGroup, DefaultIgnore;
 def note_evaluate_comparison_first :Note<
   "place parentheses around comparison expression to evaluate it first">;
 
 def warn_addition_in_bitshift : Warning<
   "operator '%0' has lower precedence than '%1'; "
-  "'%1' will be evaluated first">, InGroup;
+  "'%1' will be evaluated first">, InGroup, DefaultIgnore;
 
 def warn_self_assignment_builtin : Warning<
   "explicitly assigning value of variable of type %0 to itself">,


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: test/Misc/warning-flags-enabled.c
===
--- test/Misc/warning-flags-enabled.c
+++ test/Misc/warning-flags-enabled.c
@@ -36,7 +36,7 @@
 
 // Test if -Wshift-op-parentheses is a subgroup of -Wparentheses
 // RUN: diagtool show-enabled --no-levels -Wno-parentheses -Wshift-op-parentheses %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES %s
-// RUN: diagtool show-enabled --no-levels %s | FileCheck --check-prefix CHECK-SHIFT-OP-PARENTHESES %s
+// RUN: diagtool 

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

[Shaurya Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0ed5bea8812: [Clangd] Fixed ExtractVariable for certain 
types of Exprs (authored by SureYeaah).

Changed prior to commit:
  https://reviews.llvm.org/D64717?vs=210803=211423#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64717

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -291,18 +291,23 @@
   const char *Input = "struct ^X { int x; int y; }";
   EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 |   int x"));
 }
+
 TEST(TweakTest, ExtractVariable) {
   llvm::StringLiteral ID = "ExtractVariable";
   checkAvailable(ID, R"cpp(
-int xyz() {
+int xyz(int a = 1) {
+  struct T {
+int bar(int a = 1);
+int z;
+  } t;
   // return statement
-  return [[1]];
+  return t.b[[a]]r]](t.z)]];
 }
 void f() {
   int a = [[5 +]] [[4 * xyz]]();
   // multivariable initialization
   if(1)
-int x = [[1]], y = [[a]] + 1, a = [[1]], z = a + 1;
+int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
   // if without else
   if([[1]])
 a = [[1]];
@@ -316,13 +321,13 @@
 a = [[4]];
   else
 a = [[5]];
-  // for loop 
+  // for loop
   for(a = [[1]]; a > 3]] + [[4; a++)
 a = [[2]];
-  // while 
+  // while
   while(a < [[1]])
-[[a]]++;
-  // do while 
+[[a++]];
+  // do while
   do
 a = [[1]];
   while(a < [[3]]);
@@ -332,18 +337,19 @@
   checkNotAvailable(ID, R"cpp(
 template
 struct Test {
-Test(const T ) :val(^) {}
+Test(const T ) :val[[(^]]) {}
   T val;
 };
   )cpp");
   checkNotAvailable(ID, R"cpp(
 int xyz(int a = [[1]]) {
-  return 1;
-  class T {
-T(int a = [[1]]) {};
-int xyz = [[1]];
-  };
+  struct T {
+int bar(int a = [[1]]);
+int z = [[1]];
+  } t;
+  return [[t]].bar(t]].z]]);
 }
+void v() { return; }
 // function default argument
 void f(int b = [[1]]) {
   // empty selection
@@ -351,17 +357,26 @@
   // void expressions
   auto i = new int, j = new int;
   delete i]], delete j]];
+  [[v]]();
   // if
   if(1)
 int x = 1, y = a + 1, a = 1, z = [[a + 1]];
   if(int a = 1)
-if([[a]] == 4)
+if([[a + 1]] == 4)
   a = a]] +]] 1;
-  // for loop 
-  for(int a = 1, b = 2, c = 3; [[a]] > [[b + c]]; [[a]]++)
+  // for loop
+  for(int a = 1, b = 2, c = 3; a > [[b + c]]; [[a++]])
 a = [[a + 1]];
-  // lambda 
+  // lambda
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
+  // assigment
+  [[a = 5]];
+  // Variable DeclRefExpr
+  a = [[b]];
+  // label statement
+  goto label;
+  label:
+a = [[1]];
 }
   )cpp");
   // vector of pairs of input and output strings
@@ -397,6 +412,15 @@
  break;
}
  })cpp"},*/
+  // Macros
+  {R"cpp(#define PLUS(x) x++
+ void f(int a) {
+   PLUS([[1+a]]);
+ })cpp",
+   R"cpp(#define PLUS(x) x++
+ void f(int a) {
+   auto dummy = PLUS(1+a); dummy;
+ })cpp"},
   // ensure InsertionPoint isn't inside a macro
   {R"cpp(#define LOOP(x) while (1) {a = x;}
  void f(int a) {
@@ -425,25 +449,35 @@
auto dummy = 3; if(1)
 LOOP(5 + dummy)
  })cpp"},
-  // label and attribute testing
+  // attribute testing
   {R"cpp(void f(int a) {
-label: [ [gsl::suppress("type")] ] for (;;) a = [[1]];
+[ [gsl::suppress("type")] ] for (;;) a = [[1]];
  })cpp",
R"cpp(void f(int a) {
-auto dummy = 1; label: [ [gsl::suppress("type")] ] for (;;) a = dummy;
+auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = dummy;
  })cpp"},
-  // macro testing
-  {R"cpp(#define PLUS(x) x++
- void f(int a) {
-   PLUS([[a]]);
+  // MemberExpr
+  {R"cpp(class T {
+   T f() {
+ return [[T().f()]].f();
+   }
+ };)cpp",
+   R"cpp(class T {
+   T f() {
+ auto dummy = T().f(); return dummy.f();
+   

[clang-tools-extra] r366869 - [Clangd] Fixed ExtractVariable for certain types of Exprs

[Shaurya Gupta via cfe-commits]

Author: sureyeaah
Date: Tue Jul 23 22:42:55 2019
New Revision: 366869

URL: http://llvm.org/viewvc/llvm-project?rev=366869=rev
Log:
[Clangd] Fixed ExtractVariable for certain types of Exprs

Summary:

- Modified ExtractVariable for extraction of MemberExpr, DeclRefExpr and 
Assignment Expr
- Removed extraction from label statements.
- Fixed unittests

Reviewers: sammccall, kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp?rev=366869=366868=366869=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp Tue Jul 
23 22:42:55 2019
@@ -13,6 +13,7 @@
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
@@ -77,29 +78,15 @@ computeReferencedDecls(const clang::Expr
   return Visitor.ReferencedDecls;
 }
 
-// An expr is not extractable if it's null or an expression of type void
-// FIXME: Ignore assignment (a = 1) Expr since it is extracted as dummy = a =
-static bool isExtractableExpr(const clang::Expr *Expr) {
-  if (Expr) {
-const Type *ExprType = Expr->getType().getTypePtrOrNull();
-// FIXME: check if we need to cover any other types
-if (ExprType)
-  return !ExprType->isVoidType();
-  }
-  return false;
-}
-
 ExtractionContext::ExtractionContext(const SelectionTree::Node *Node,
  const SourceManager ,
  const ASTContext )
 : ExprNode(Node), SM(SM), Ctx(Ctx) {
   Expr = Node->ASTNode.get();
-  if (isExtractableExpr(Expr)) {
-ReferencedDecls = computeReferencedDecls(Expr);
-InsertionPoint = computeInsertionPoint();
-if (InsertionPoint)
-  Extractable = true;
-  }
+  ReferencedDecls = computeReferencedDecls(Expr);
+  InsertionPoint = computeInsertionPoint();
+  if (InsertionPoint)
+Extractable = true;
 }
 
 // checks whether extracting before InsertionPoint will take a
@@ -121,9 +108,9 @@ bool ExtractionContext::exprIsValidOutsi
 // the current Stmt. We ALWAYS insert before a Stmt whose parent is a
 // CompoundStmt
 //
-
-// FIXME: Extraction from switch and case statements
+// FIXME: Extraction from label, switch and case statements
 // FIXME: Doens't work for FoldExpr
+// FIXME: Ensure extraction from loops doesn't change semantics.
 const clang::Stmt *ExtractionContext::computeInsertionPoint() const {
   // returns true if we can extract before InsertionPoint
   auto CanExtractOutside =
@@ -141,8 +128,7 @@ const clang::Stmt *ExtractionContext::co
   return isa(Stmt) || isa(Stmt) ||
  isa(Stmt) || isa(Stmt) ||
  isa(Stmt) || isa(Stmt) || isa(Stmt) ||
- isa(Stmt) || isa(Stmt) ||
- isa(Stmt);
+ isa(Stmt) || isa(Stmt);
 }
 if (InsertionPoint->ASTNode.get())
   return true;
@@ -209,6 +195,9 @@ public:
 return "Extract subexpression to variable";
   }
   Intent intent() const override { return Refactor; }
+  // Compute the extraction context for the Selection
+  bool computeExtractionContext(const SelectionTree::Node *N,
+const SourceManager , const ASTContext 
);
 
 private:
   // the expression to extract
@@ -216,14 +205,13 @@ private:
 };
 REGISTER_TWEAK(ExtractVariable)
 bool ExtractVariable::prepare(const Selection ) {
+  // we don't trigger on empty selections for now
+  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
+return false;
   const ASTContext  = Inputs.AST.getASTContext();
   const SourceManager  = Inputs.AST.getSourceManager();
   const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
-  // we don't trigger on empty selections for now
-  if (!N || Inputs.SelectionBegin == Inputs.SelectionEnd)
-return false;
-  Target = llvm::make_unique(N, SM, Ctx);
-  return Target->isExtractable();
+  return computeExtractionContext(N, SM, Ctx);
 }
 
 Expected ExtractVariable::apply(const Selection ) {
@@ -239,6 +227,75 @@ Expected ExtractVariable:
   return Effect::applyEdit(Result);
 }
 
+// Find the CallExpr whose callee is an ancestor of the DeclRef
+const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
+  // we maintain a stack of all exprs encountered while traversing the
+  // selectiontree because the callee of the callexpr can be an ancestor of the
+  // 

[PATCH] D65176: [NFC][clang] Refactor getCompilationPhases()+Types.def step 2.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

This looks good to me generally.  I don't fully understand the reason for `u` 
being kept, is that something you intend to clean up in a subsequent patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65176



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

[PATCH] D65183: [Format] Make it easy to add new format::FormatStyle::LanguageStandard. NFCI

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

LGTM, but I don't actively work in this codebase so I really can't say. I'll 
wait to hear from some other more active clang-format reviewers.




Comment at: lib/Format/TokenAnnotator.cpp:2862
 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
-   (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
+   (Style.Standard == FormatStyle::LS_Cpp03 || Style.SpacesInAngles);
   }

Just a note: I don't know what the original intent of 
https://github.com/llvm/llvm-project/commit/dd978ae0e11 was, but in D65043 I 
modified this condition to be `Style.Standard == FormatStyle::LS_Cpp03 || 
Style.Standard == FormatStyle::LS_Auto`, because I believe that mirrors the 
current behavior exactly. With this change, a user that specified a standard of 
`FormatStyle::LS_Auto` will experience a change in behavior.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65183



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

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Also if we ever implement any fixits for path-sensitive reports that actually 
make sense, i suspect that it might as well make sense to attach fixits to 
individual path notes. For examples, if we report a double-free, we may attach 
a removal of the second free to the warning and a removal of the first free to 
the note that highlights the first free.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65182



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

[PATCH] D65169: [WIP] LLVM Optimization Remark for Attributes

[William Moses via Phabricator via cfe-commits]

wsmoses updated this revision to Diff 211382.
wsmoses removed subscribers: cfe-commits, llvm-commits.

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

https://reviews.llvm.org/D65169

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/LinkAllPasses.h
  llvm/include/llvm/Transforms/Utils.h
  llvm/include/llvm/Transforms/Utils/EmitAnnotations.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/EmitAnnotations.cpp
  llvm/lib/Transforms/Utils/Utils.cpp

Index: llvm/lib/Transforms/Utils/Utils.cpp
===
--- llvm/lib/Transforms/Utils/Utils.cpp
+++ llvm/lib/Transforms/Utils/Utils.cpp
@@ -26,6 +26,7 @@
   initializeAddDiscriminatorsLegacyPassPass(Registry);
   initializeBreakCriticalEdgesPass(Registry);
   initializeCanonicalizeAliasesLegacyPassPass(Registry);
+  initializeEmitAnnotationsLegacyPassPass(Registry);
   initializeInstNamerPass(Registry);
   initializeLCSSAWrapperPassPass(Registry);
   initializeLibCallsShrinkWrapLegacyPassPass(Registry);
Index: llvm/lib/Transforms/Utils/EmitAnnotations.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Utils/EmitAnnotations.cpp
@@ -0,0 +1,96 @@
+//===-- UnrollLoop.cpp - Loop unrolling utilities -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements some loop unrolling utilities. It does not define any
+// actual pass or policy, but provides a single function to perform loop
+// unrolling.
+//
+// The process of unrolling can produce extraneous basic blocks linked with
+// unconditional branches.  This will be corrected in the future.
+//
+//===--===//
+
+#include 
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/Pass.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils.h"
+#include "llvm/Transforms/Utils/EmitAnnotations.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "emit-annotations"
+
+std::string fixQuotes(std::string str) {
+str.erase(std::remove(str.begin(), str.end(), '"'), str.end());
+return '"' + str + '"';
+}
+
+void EmitAnnotations(Function *F, OptimizationRemarkEmitter ) {
+OptimizationRemarkAnnotation annotations(DEBUG_TYPE, "annotation ", F);
+AttributeList list = F->getAttributes();
+for(auto a : list.getFnAttributes()) {
+annotations << "fn_attr(" << fixQuotes(a.getAsString(true)) << ") ";
+}
+for(auto a : list.getRetAttributes()) {
+annotations << "ret_attr(" << fixQuotes(a.getAsString(true)) << ") ";
+}
+for(unsigned i=0; igetFunctionType()->getNumParams(); i++)
+for(auto a : list.getParamAttributes(i)) {
+annotations << "arg_attr(" << std::to_string(i) << ", " << fixQuotes(a.getAsString(true)) << ") ";
+}
+annotations << "\n";
+  ORE.emit(annotations);
+}
+
+namespace llvm {
+
+struct EmitAnnotationsLegacyPass : public FunctionPass {
+  // Pass identification, replacement for typeid
+  static char ID;
+
+  EmitAnnotationsLegacyPass() : FunctionPass(ID) {
+initializeEmitAnnotationsLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
+
+  // runOnFunction - To run this pass, first we calculate the alloca
+  // instructions that are safe for promotion, then we promote each one.
+  bool runOnFunction(Function ) override {
+OptimizationRemarkEmitter  = getAnalysis().getORE();
+EmitAnnotations(, ORE);
+return false;
+  }
+
+  void getAnalysisUsage(AnalysisUsage ) const override {
+AU.addRequired();
+AU.setPreservesCFG();
+  }
+};
+
+} // end anonymous namespace
+
+char EmitAnnotationsLegacyPass::ID = 0;
+
+PreservedAnalyses EmitAnnotationsPass::run(Function ,
+   FunctionAnalysisManager ) {
+  OptimizationRemarkEmitter  = FM.getResult(F);
+  EmitAnnotations(, ORE);
+  return PreservedAnalyses::all();
+}
+
+INITIALIZE_PASS_BEGIN(EmitAnnotationsLegacyPass, 

[PATCH] D65183: [Format] Make it easy to add new format::FormatStyle::LanguageStandard. NFCI

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: modocache, MyDeveloperDay, klimek, rsmith, sammccall, 
Typz.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Preparatory change for D65043 .

We current use `!=LS_Cpp03` to enable language standards 11,14,17, and
2a. `>=LS_Cpp11` is better if we decide to add new LanguageStandard in
the future.


Repository:
  rC Clang

https://reviews.llvm.org/D65183

Files:
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2859,7 +2859,7 @@
 (Style.Language == FormatStyle::LK_Proto && Left.is(TT_DictLiteral)))
   return !Style.Cpp11BracedListStyle;
 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
-   (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
+   (Style.Standard == FormatStyle::LS_Cpp03 || Style.SpacesInAngles);
   }
   if (Right.isOneOf(tok::arrow, tok::arrowstar, tok::periodstar) ||
   Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2366,10 +2366,10 @@
 LangOptions getFormattingLangOpts(const FormatStyle ) {
   LangOptions LangOpts;
   LangOpts.CPlusPlus = 1;
-  LangOpts.CPlusPlus11 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus14 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus17 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus2a = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
+  LangOpts.CPlusPlus11 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus14 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus17 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus2a = Style.Standard >= FormatStyle::LS_Cpp11;
   LangOpts.LineComment = 1;
   bool AlternativeOperators = Style.isCpp();
   LangOpts.CXXOperatorNames = AlternativeOperators ? 1 : 0;


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2859,7 +2859,7 @@
 (Style.Language == FormatStyle::LK_Proto && Left.is(TT_DictLiteral)))
   return !Style.Cpp11BracedListStyle;
 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
-   (Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
+   (Style.Standard == FormatStyle::LS_Cpp03 || Style.SpacesInAngles);
   }
   if (Right.isOneOf(tok::arrow, tok::arrowstar, tok::periodstar) ||
   Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -2366,10 +2366,10 @@
 LangOptions getFormattingLangOpts(const FormatStyle ) {
   LangOptions LangOpts;
   LangOpts.CPlusPlus = 1;
-  LangOpts.CPlusPlus11 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus14 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus17 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus2a = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
+  LangOpts.CPlusPlus11 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus14 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus17 = Style.Standard >= FormatStyle::LS_Cpp11;
+  LangOpts.CPlusPlus2a = Style.Standard >= FormatStyle::LS_Cpp11;
   LangOpts.LineComment = 1;
   bool AlternativeOperators = Style.isCpp();
   LangOpts.CXXOperatorNames = AlternativeOperators ? 1 : 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, kristof.beyls, javed.absar.
Herald added a project: clang.

That's a pretty wonky experiment, even if a fairly easy one. I'm trying to add 
support for fix-it hints to the bug reporter. In the current shape the patch 
does the following:

- Allow attaching fixit hints to Static Analyzer `BugReport`s.
- Add support for fixits in text output (including the default text output that 
goes without notes, as the fixit "belongs" to the warning).
- Add support for fixits in the plist output mode (not sure if i'm fully 
supporting all kinds of fixits).
- Implement a fixit for the path-insensitive DeadStores checker (only one of 
the cases, and i'm still not sure it's a good fixit, but it was an obvious 
first thing to experiment with).
- Implement a fixit for the path-sensitive VirtualCall checker when the virtual 
method is not pure virtual (in this case the "fix" is to suppress the warning 
by qualifying the call).

More TODOs:

- We don't have a good way to test removal fixits (such as the one in the dead 
stores checker). Testing plist files is not a good way to test them (though we 
definitely need a few such tests). We can't test them via text output because 
clang itself generally doesn't display removal fixits in text output (it's 
kinda obvious if you look at how it prints them). In clang-tidy they use a more 
sophisticated facility for these tests, testing the fixed file instead, but 
it's deep within their custom testing scripts. We might need something similar.
- HTML output still doesn't support fixits. Not sure if they are useful in 
there because fixits are generally not very useful when there's no button to 
apply them. But there should be no harm in displaying them anyway, so i hope 
i'll have time to take a look.
- Need more tests with macros and such stuff.


Repository:
  rC Clang

https://reviews.llvm.org/D65182

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2190,6 +2190,26 @@
 86

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line86
+  col11
+  file0
+ 
+ 
+  line86
+  col40
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -403,6 +403,26 @@
 119

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line119
+  col7
+  file0
+ 
+ 
+  line119
+  col25
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
@@ -471,6 +491,26 @@
 139

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line139
+  col10
+  file0
+ 
+ 
+  line139
+  col53
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
@@ -539,6 +579,26 @@
 144

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line144
+  col10
+  file0
+ 
+ 
+  line144
+  col45
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
@@ -607,6 +667,26 @@
 145

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line145
+  col10
+  file0
+ 
+ 
+  line145
+  col44
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
@@ -675,6 +755,26 @@
 146

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line146
+  col10
+  file0
+ 
+ 
+  line146
+  col48
+  file0
+ 
+
+insert_string
+
+   
+  
   
   
path
@@ -1085,6 +1185,26 @@
 150

   
+  fixits
+  
+   
+remove_range
+
+ 
+  line150
+  col16
+  file0
+ 
+ 
+  line150
+  col64
+  file0
+ 
+
+

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 211399.
NoQ added a comment.

- Hmm, also i'd rather say "method" than "function".


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

https://reviews.llvm.org/D65180

Files:
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,7 +2,7 @@
   class Z {
   public:
 Z() {
-  foo(); // impure-warning {{Call to virtual function during construction}}
+  foo(); // impure-warning {{Call to virtual method 'Z::foo' during construction}}
 }
 virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,26 +1,33 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 #include "virtualcall.h"
 
+void clang_analyzer_warnIfReached();
+
 class A {
 public:
   A();
@@ -30,31 +37,32 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo(); // expected-warning{{Call to pure virtual function during construction}}
+foo(); // expected-warning{{Call to pure virtual method 'A::foo' during construction}}
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
-class B : public A {
+A::A() {
+  f();
+}
+
+class B {
 public:
   B() {
-foo(); // impure-warning {{Call to virtual function during construction}}
+foo(); // impure-warning {{Call to virtual method 'B::foo' during construction}}
   }
   ~B();
 
   virtual int foo();
   virtual void bar() {
-foo(); // impure-warning{{Call to virtual function during destruction}}
+foo(); // impure-warning {{Call to virtual method 'B::foo' during destruction}}
   }
 };
 
-A::A() {
-  f();
-}
-
 B::~B() {
   this->B::foo(); // no-warning
   this->B::bar();
-  this->foo(); // impure-warning {{Call to virtual function during destruction}}
+  this->foo(); // impure-warning {{Call to virtual method 'B::foo' during destruction}}
 }
 
 class C : public B {
@@ -67,7 +75,7 @@
 };
 
 C::C() {
-  f(foo()); // impure-warning {{Call to virtual function during construction}}
+  f(foo()); // impure-warning {{Call to virtual method 'C::foo' during construction}}
 }
 
 class D : public B {
@@ -121,23 +129,23 @@
 G g;
 g.foo();
 g.bar(); // no warning
-f(); // impure-warning {{Call to virtual function during construction}}
+f(); // impure-warning {{Call to virtual method 'H::f' during construction}}
 H  = *this;
-h.f(); // impure-warning {{Call to virtual function during construction}}
+h.f(); // impure-warning {{Call to virtual method 'H::f' during construction}}
   }
 };
 
 class X {
 public:
   X() {
-g(); // impure-warning {{Call to virtual function during construction}}
+g(); // impure-warning {{Call to virtual method 'X::g' during construction}}
   }
   X(int i) {
 if (i > 0) {
   X x(i - 1);
   x.g(); // no warning
 }
-g(); // impure-warning {{Call to virtual function during construction}}
+g(); // impure-warning {{Call to virtual method 'X::g' during construction}}
   }
   virtual void g();
 };
@@ -158,7 +166,7 @@
   virtual void foo();
 };
 void N::callFooOfM(M *m) {
-  m->foo(); // impure-warning {{Call to virtual function during construction}}
+  m->foo(); // impure-warning {{Call to virtual 

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 211397.
NoQ added a comment.

- Print only class name::method name instead of a fully qualified name, because 
fully qualified names may get pretty long.
- Remove more dead code.


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

https://reviews.llvm.org/D65180

Files:
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,7 +2,7 @@
   class Z {
   public:
 Z() {
-  foo(); // impure-warning {{Call to virtual function during construction}}
+  foo(); // impure-warning {{Call to virtual function 'Z::foo' during construction}}
 }
 virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,26 +1,33 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 #include "virtualcall.h"
 
+void clang_analyzer_warnIfReached();
+
 class A {
 public:
   A();
@@ -30,31 +37,32 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo(); // expected-warning{{Call to pure virtual function during construction}}
+foo(); // expected-warning{{Call to pure virtual function 'A::foo' during construction}}
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
-class B : public A {
+A::A() {
+  f();
+}
+
+class B {
 public:
   B() {
-foo(); // impure-warning {{Call to virtual function during construction}}
+foo(); // impure-warning {{Call to virtual function 'B::foo' during construction}}
   }
   ~B();
 
   virtual int foo();
   virtual void bar() {
-foo(); // impure-warning{{Call to virtual function during destruction}}
+foo(); // impure-warning {{Call to virtual function 'B::foo' during destruction}}
   }
 };
 
-A::A() {
-  f();
-}
-
 B::~B() {
   this->B::foo(); // no-warning
   this->B::bar();
-  this->foo(); // impure-warning {{Call to virtual function during destruction}}
+  this->foo(); // impure-warning {{Call to virtual function 'B::foo' during destruction}}
 }
 
 class C : public B {
@@ -67,7 +75,7 @@
 };
 
 C::C() {
-  f(foo()); // impure-warning {{Call to virtual function during construction}}
+  f(foo()); // impure-warning {{Call to virtual function 'C::foo' during construction}}
 }
 
 class D : public B {
@@ -121,23 +129,23 @@
 G g;
 g.foo();
 g.bar(); // no warning
-f(); // impure-warning {{Call to virtual function during construction}}
+f(); // impure-warning {{Call to virtual function 'H::f' during construction}}
 H  = *this;
-h.f(); // impure-warning {{Call to virtual function during construction}}
+h.f(); // impure-warning {{Call to virtual function 'H::f' during construction}}
   }
 };
 
 class X {
 public:
   X() {
-g(); // impure-warning {{Call to virtual function during construction}}
+g(); // impure-warning {{Call to virtual function 'X::g' during construction}}
   }
   X(int i) {
 if (i > 0) {
   X x(i - 1);
   x.g(); // no warning
 }
-g(); // impure-warning {{Call to virtual function during construction}}
+g(); // impure-warning {{Call to virtual function 'X::g' during construction}}
   }
   virtual void g();
 };
@@ -158,7 +166,7 @@
   virtual void foo();
 };
 void N::callFooOfM(M *m) {
-  m->foo(); 

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ added a parent revision: D64274: [analyzer] VirtualCallChecker overhaul..

- Mention the name of the virtual function. This should help when the warning 
appears in a complicated expression and it's unclear what part of the 
expression it is about.
- Calling a pure virtual function always sinks the analysis, even if the 
checker is not in pure-only mode.
- Refactor/modernize the code a bit.


Repository:
  rC Clang

https://reviews.llvm.org/D65180

Files:
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,7 +2,7 @@
   class Z {
   public:
 Z() {
-  foo(); // impure-warning {{Call to virtual function during construction}}
+  foo(); // impure-warning {{Call to virtual function 'header::Z::foo' during construction}}
 }
 virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,26 +1,33 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected,impure -std=c++11 %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
 // RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
 // RUN:-analyzer-config \
 // RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
 // RUN:-std=c++11 -verify=expected %s
 
 #include "virtualcall.h"
 
+void clang_analyzer_warnIfReached();
+
 class A {
 public:
   A();
@@ -30,31 +37,32 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo(); // expected-warning{{Call to pure virtual function during construction}}
+foo(); // expected-warning{{Call to pure virtual function 'A::foo' during construction}}
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
-class B : public A {
+A::A() {
+  f();
+}
+
+class B {
 public:
   B() {
-foo(); // impure-warning {{Call to virtual function during construction}}
+foo(); // impure-warning {{Call to virtual function 'B::foo' during construction}}
   }
   ~B();
 
   virtual int foo();
   virtual void bar() {
-foo(); // impure-warning{{Call to virtual function during destruction}}
+foo(); // impure-warning {{Call to virtual function 'B::foo' during destruction}}
   }
 };
 
-A::A() {
-  f();
-}
-
 B::~B() {
   this->B::foo(); // no-warning
   this->B::bar();
-  this->foo(); // impure-warning {{Call to virtual function during destruction}}
+  this->foo(); // impure-warning {{Call to virtual function 'B::foo' during destruction}}
 }
 
 class C : public B {
@@ -67,7 +75,7 @@
 };
 
 C::C() {
-  f(foo()); // impure-warning {{Call to virtual function during construction}}
+  f(foo()); // impure-warning {{Call to virtual function 'C::foo' during construction}}
 }
 
 class D : public B {
@@ -121,23 +129,23 @@
 G g;
 g.foo();
 g.bar(); // no warning
-f(); // impure-warning {{Call to virtual function during construction}}
+f(); // impure-warning {{Call to virtual function 'H::f' during construction}}
 H  = *this;
-h.f(); // impure-warning {{Call to virtual function during construction}}
+h.f(); // impure-warning {{Call to virtual function 'H::f' during construction}}
   }
 };
 
 class X {
 public:
   X() {
-g(); // impure-warning {{Call to virtual function during construction}}
+g(); // impure-warning {{Call to 

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[Connor Kuehl via Phabricator via cfe-commits]

connorkuehl added a comment.

In D59254#1429401 , @jfb wrote:

> I find it easier to understand the code by looking at the tests. When you add 
> tests, please make sure you test for:
>
> - Bit-fields
> - Zero-width bit-field


Hi JF,

Could you elaborate on what I should be testing for here regarding the 
zero-width bit-field? I'm not sure zero-width bit-fields are a concern since I 
think they're illegal. I just tried compiling a "hello world" program with a 
struct that has a zero width bit-field as one of its members and the compiler 
emitted an error pointing at it.

  error: named bit-field 'z' has zero width
  int z : 0;
  ^
  1 error generated.

For regular bit-fields the current implementation will keep adjacent bit-fields 
together and will preserve their original order but the overall group of 
adjacent bit-fields may relocate. My next revision will have a unit test 
describing and testing this behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

I will take a look next week when I get back!


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

https://reviews.llvm.org/D63907



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

[PATCH] D65176: [NFC][clang] Refactor getCompilationPhases()+Types.def step 2.

[Puyan Lotfi via Phabricator via cfe-commits]

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

- Removing a few of the entries in the Flags for the Types.def table.
- Removing redundant parts of getCompilationPhases().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65176

Files:
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -42,11 +42,19 @@
 }
 
 types::ID types::getPreprocessedType(ID Id) {
-  return getInfo(Id).PreprocessedType;
+  ID PPT = getInfo(Id).PreprocessedType;
+  assert((llvm::is_contained(getInfo(Id).Phases, phases::Preprocess) !=
+  (PPT == TY_INVALID)) &&
+ "Unexpected Preprocess Type.");
+  return PPT;
+}
+
+static bool isPrepeocessedModuleType(ID Id) {
+  return Id == TY_CXXModule || Id == TY_PP_CXXModule;
 }
 
 types::ID types::getPrecompiledType(ID Id) {
-  if (strchr(getInfo(Id).Flags, 'm'))
+  if (isPrepeocessedModuleType(Id))
 return TY_ModuleFile;
   if (onlyPrecompileType(Id))
 return TY_PCH;
@@ -71,11 +79,14 @@
 }
 
 bool types::onlyAssembleType(ID Id) {
-  return strchr(getInfo(Id).Flags, 'a');
+  return llvm::is_contained(getInfo(Id).Phases, phases::Assemble) &&
+ !llvm::is_contained(getInfo(Id).Phases, phases::Compile) &&
+ !llvm::is_contained(getInfo(Id).Phases, phases::Backend);
 }
 
 bool types::onlyPrecompileType(ID Id) {
-  return strchr(getInfo(Id).Flags, 'p');
+  return llvm::is_contained(getInfo(Id).Phases, phases::Precompile) &&
+ !isPrepeocessedModuleType(Id);
 }
 
 bool types::canTypeBeUserSpecified(ID Id) {
@@ -269,39 +280,7 @@
 // FIXME: The list is now in Types.def but for now this function will verify
 //the old behavior and a subsequent change will delete most of the body.
 void types::getCompilationPhases(ID Id, llvm::SmallVectorImpl ) {
-  if (Id != TY_Object) {
-if (getPreprocessedType(Id) != TY_INVALID) {
-  P.push_back(phases::Preprocess);
-}
-
-if (getPrecompiledType(Id) != TY_INVALID) {
-  P.push_back(phases::Precompile);
-}
-
-if (!onlyPrecompileType(Id)) {
-  if (!onlyAssembleType(Id)) {
-P.push_back(phases::Compile);
-P.push_back(phases::Backend);
-  }
-  P.push_back(phases::Assemble);
-}
-  }
-
-  if (!onlyPrecompileType(Id)) {
-P.push_back(phases::Link);
-  }
-
-  // Check that the static Phase list matches.
-  // TODO: These will be deleted.
-  const llvm::SmallVectorImpl  = getInfo(Id).Phases;
-  assert(Phases.size() == P.size() &&
- std::equal(Phases.begin(), Phases.end(), P.begin()) &&
- "Invalid phase or size");
-
-  // TODO: This function is still being used to assert that the phase list in
-  //   Types.def is correct. Everything above this comment will be removed
-  //   in a subsequent NFC commit.
-  P = Phases;
+  P = getInfo(Id).Phases;
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
 }
Index: clang/include/clang/Driver/Types.def
===
--- clang/include/clang/Driver/Types.def
+++ clang/include/clang/Driver/Types.def
@@ -30,15 +30,19 @@
 // of this type, or null if unspecified.
 
 // The fifth value is a string containing option flags. Valid values:
-//  a - The type should only be assembled.
-//  p - The type should only be precompiled.
 //  u - The type can be user specified (with -x).
-//  m - Precompiling this type produces a module file.
 //  A - The type's temporary suffix should be appended when generating
 //  outputs of this type.
 
 // The sixth value is a variadic list of phases for each type. Eventually the
 // options flag string will be replaced with this variadic list.
+// Some of the options in Flags have been removed, so far those are:
+//  a - The type should only be assembled: Now, check that Phases contains
+//  phases::Assemble but not phases::Compile or phases::Backend.
+//  p - The type should only be precompiled: Now, check that Phases contains
+//  phases::Precompile but that Flags does not contain 'm'.
+//  m - Precompiling this type produces a module file: Now, check that
+//  isPrepeocessedModuleType.
 
 // C family source language (with and without preprocessing).
 TYPE("cpp-output",   PP_C, INVALID, "i", "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
@@ -61,22 +65,22 @@
 TYPE("renderscript", RenderScript, PP_C,"rs","u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 
 // C family input files to precompile.
-TYPE("c-header-cpp-output",  PP_CHeader,   INVALID, "i", "p",  phases::Precompile)
-TYPE("c-header", 

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

[Artem Dergachev via Phabricator via cfe-commits]

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

In D64274#1586282 , @Szelethus wrote:

> //Ackchyually//,  it doesn't per se break anything, but will result in 
> CodeChecker no longer enabling `optin.cplusplus.VirtualCall` :^) Sorry, 
> oversight on my end. Observe the following monster of a clang invocation...


Mmm. Aha. Ok, i can reproduce your problem, but i don't think reversing the 
dependencies is gonna work. If we make the pure checker depend on the optin 
checker, we would always have the opt-in checker registered first, and 
therefore there's no way to figure out if we really wanted to opt in into the 
optin checker or we are simply loading it as a dependency. Which, in 
particular, makes it impossible to discriminate between `-analyzer-checker 
cplusplus.PureVirtualCall` and `-analyzer-checker optin.cplusplus.VirtualCall` 
when the option is unset: in both cases we'll first register the opt-in checker 
and then the pure checker.

I'm confused though; the way i was previously understanding your work, when 
disabling a dependency and then enabling a dependent checker *later* in the 
run-line, it should re-enable the dependency automatically. Did you consciously 
decide not to implement it that way?




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:569
   "false",
-  Released>
+  InAlpha>
   ]>,

Szelethus wrote:
> Lets hide it as well.
> 
> ```
> CmdLineOption   "PureOnly",
>   "Disables the checker. Keeps cplusplus.PureVirtualCall "
>   "enabled. This option is only provided for backwards "
>   "compatibility.",
>   "false",
>   InAlpha,
>   Hide>
> ```
Yup!


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

https://reviews.llvm.org/D64274



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

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 211385.
arphaman added a comment.

Fix a bug for empty minimized files where null terminator lookup by the lexer 
was an out of bounds read


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

https://reviews.llvm.org/D63907

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  
clang/test/ClangScanDeps/Inputs/frameworks/Framework.framework/Headers/Framework.h
  
clang/test/ClangScanDeps/Inputs/frameworks/Framework.framework/PrivateHeaders/PrivateHeader.h
  clang/test/ClangScanDeps/Inputs/header_stat_before_open_cdb.json
  clang/test/ClangScanDeps/Inputs/vfsoverlay.yaml
  clang/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json
  clang/test/ClangScanDeps/header_stat_before_open.m
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/test/ClangScanDeps/vfsoverlay.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/Support/InitLLVM.h"
@@ -45,9 +46,10 @@
   ///
   /// \param Compilations The reference to the compilation database that's
   /// used by the clang tool.
-  DependencyScanningTool(const tooling::CompilationDatabase ,
+  DependencyScanningTool(DependencyScanningService ,
+ const tooling::CompilationDatabase ,
  SharedStream , SharedStream )
-  : Compilations(Compilations), OS(OS), Errs(Errs) {}
+  : Worker(Service), Compilations(Compilations), OS(OS), Errs(Errs) {}
 
   /// Computes the dependencies for the given file and prints them out.
   ///
@@ -80,6 +82,20 @@
 
 llvm::cl::OptionCategory DependencyScannerCategory("Tool options");
 
+static llvm::cl::opt ScanMode(
+"mode",
+llvm::cl::desc("The preprocessing mode used to compute the dependencies"),
+llvm::cl::values(
+clEnumValN(ScanningMode::MinimizedSourcePreprocessing,
+   "preprocess-minimized-sources",
+   "The set of dependencies is computed by preprocessing the "
+   "source files that were minimized to only include the "
+   "contents that might affect the dependencies"),
+clEnumValN(ScanningMode::CanonicalPreprocessing, "preprocess",
+   "The set of dependencies is computed by preprocessing the "
+   "unmodified source files")),
+llvm::cl::init(ScanningMode::MinimizedSourcePreprocessing));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -136,12 +152,14 @@
   SharedStream Errs(llvm::errs());
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
+
+  DependencyScanningService Service(ScanMode);
   unsigned NumWorkers =
   NumThreads == 0 ? llvm::hardware_concurrency() : NumThreads;
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
 WorkerTools.push_back(llvm::make_unique(
-*AdjustingCompilations, DependencyOS, Errs));
+Service, *AdjustingCompilations, DependencyOS, Errs));
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
Index: clang/test/ClangScanDeps/vfsoverlay.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/vfsoverlay.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/vfsoverlay.cpp
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/vfsoverlay.yaml > %t.dir/vfsoverlay.yaml
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/vfsoverlay_cdb.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | \
+// RUN:   FileCheck %s
+
+#include "not_real.h"
+
+// CHECK: clang-scan-deps dependency
+// CHECK-NEXT: vfsoverlay.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Thanks, the update looks good.

One thing I just noticed: if I have a codebase with 
`-Wimplicit-float-conversion` it seems like updating clang will automatically 
turn the two new warnings on, correct? That seems good, but also difficult to 
roll out because I can't turns off just the two new warnings. In other words, 
if I want to adopt a new clang and I had `-Wimplicit-float-conversion` then the 
only options I have are: fix all the new warnings (which might take time given 
how much code I have), or turn off *all* of `-Wimplicit-float-conversion`. I 
think it makes sense to force fixing all warnings for the warning that's always 
a problem `warn_impcast_integer_float_precision_constant`, but for 
`warn_impcast_integer_float_precision` it would be nice if there were a 
`-Wno-*` which disables only `warn_impcast_integer_float_precision` and nothing 
else.
This would make it way easier to roll out a new clang.


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

https://reviews.llvm.org/D64666



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

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

ziangwan updated this revision to Diff 211381.
ziangwan added a comment.

Update diff.

1. Make "definitely lose" warning on by default.


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

https://reviews.llvm.org/D64666

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion.c
  clang/test/Sema/ext_vector_casts.c
  clang/test/Sema/implicit-float-conversion.c

Index: clang/test/Sema/implicit-float-conversion.c
===
--- /dev/null
+++ clang/test/Sema/implicit-float-conversion.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-float-conversion
+
+long testReturn(long a, float b) {
+  return a + b; // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+}
+
+void testAssignment() {
+  float f = 22;
+  double b = L;
+  
+  float ff = L; // expected-warning {{implicit conversion from 'long' to 'float' changes value from  to 1312}}
+  float  = UL; // expected-warning {{implicit conversion from 'unsigned long' to 'float' changes value from  to 1312}}
+
+  long l = L;
+  float fff = l; // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+}
+
+void testExpression() {
+  float a = 0.0f;
+  float b = L + a; // expected-warning {{implicit conversion from 'long' to 'float' changes value from  to 1312}}
+
+  float g =  + ;
+  float c =  + 2223; // expected-warning {{implicit conversion from 'int' to 'float' changes value from 4445 to }}
+
+  int i = 0;
+  float d = i + a; // expected-warning {{implicit conversion from 'int' to 'float' may lose precision}}
+  
+  double e = 0.0;
+  double f = i + e;
+}
Index: clang/test/Sema/ext_vector_casts.c
===
--- clang/test/Sema/ext_vector_casts.c
+++ clang/test/Sema/ext_vector_casts.c
@@ -115,12 +115,12 @@
   vl = vl + t; // expected-warning {{implicit conversion loses integer precision}}
   
   vf = 1 + vf;
-  vf = l + vf;
+  vf = l + vf; // expected-warning {{implicit conversion from 'long' to 'float2' (vector of 2 'float' values) may lose precision}}
   vf = 2.0 + vf;
   vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}}
-  vf = vf + 0x;
+  vf = vf + 0x; // expected-warning {{implicit conversion from 'unsigned int' to 'float2' (vector of 2 'float' values) changes value from 4294967295 to 4294967296}}
   vf = vf + 2.1; // expected-warning {{implicit conversion loses floating-point precision}}
   
-  vd = l + vd;
-  vd = vd + t;
+  vd = l + vd; // expected-warning {{implicit conversion from 'long' to 'double2' (vector of 2 'double' values) may lose precision}}
+  vd = vd + t; // expected-warning {{implicit conversion from '__uint128_t' (aka 'unsigned __int128') to 'double2' (vector of 2 'double' values) may lose precision}}
 }
Index: clang/test/Sema/conversion.c
===
--- clang/test/Sema/conversion.c
+++ clang/test/Sema/conversion.c
@@ -233,7 +233,7 @@
   takes_int(v);
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
+  takes_float(v); // expected-warning {{implicit conversion from 'int' to 'float' may lose precision}}
   takes_double(v);
   takes_longdouble(v);
 }
@@ -244,8 +244,8 @@
   takes_int(v); // expected-warning {{implicit conversion loses integer precision}}
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
-  takes_double(v);
+  takes_float(v); // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+  takes_double(v); // expected-warning {{implicit conversion from 'long' to 'double' may lose precision}}
   takes_longdouble(v);
 }
 
@@ -255,8 +255,8 @@
   takes_int(v); // expected-warning {{implicit conversion loses integer precision}}
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
-  takes_double(v);
+  takes_float(v); // expected-warning {{implicit conversion from 'long long' to 'float' may lose precision}}
+  takes_double(v); // expected-warning {{implicit conversion from 'long long' to 'double' may lose precision}}
   takes_longdouble(v);
 }
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11400,6 +11400,55 @@
 }
   }
 
+  // If we are casting an integer type to a floating point type, we might
+  // lose accuracy if the floating point type has a narrower significand
+  // than the integer type. Issue warnings for that accuracy loss. 
+  if (SourceBT && TargetBT && SourceBT->isIntegerType() &&
+  TargetBT->isFloatingType()) {
+// Determine the number of precision bits in the source integer 

[PATCH] D65169: [WIP] LLVM Optimization Remark for Attributes

[William Moses via Phabricator via cfe-commits]

wsmoses updated this revision to Diff 211371.
wsmoses edited the summary of this revision.

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

https://reviews.llvm.org/D65169

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/LinkAllPasses.h
  llvm/include/llvm/Transforms/Utils.h
  llvm/include/llvm/Transforms/Utils/EmitAnnotations.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/EmitAnnotations.cpp
  llvm/lib/Transforms/Utils/Utils.cpp

Index: llvm/lib/Transforms/Utils/Utils.cpp
===
--- llvm/lib/Transforms/Utils/Utils.cpp
+++ llvm/lib/Transforms/Utils/Utils.cpp
@@ -26,6 +26,7 @@
   initializeAddDiscriminatorsLegacyPassPass(Registry);
   initializeBreakCriticalEdgesPass(Registry);
   initializeCanonicalizeAliasesLegacyPassPass(Registry);
+  initializeEmitAnnotationsLegacyPassPass(Registry);
   initializeInstNamerPass(Registry);
   initializeLCSSAWrapperPassPass(Registry);
   initializeLibCallsShrinkWrapLegacyPassPass(Registry);
Index: llvm/lib/Transforms/Utils/EmitAnnotations.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Utils/EmitAnnotations.cpp
@@ -0,0 +1,96 @@
+//===-- UnrollLoop.cpp - Loop unrolling utilities -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements some loop unrolling utilities. It does not define any
+// actual pass or policy, but provides a single function to perform loop
+// unrolling.
+//
+// The process of unrolling can produce extraneous basic blocks linked with
+// unconditional branches.  This will be corrected in the future.
+//
+//===--===//
+
+#include 
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/Pass.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils.h"
+#include "llvm/Transforms/Utils/EmitAnnotations.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "emit-annotations"
+
+std::string fixQuotes(std::string str) {
+str.erase(std::remove(str.begin(), str.end(), '"'), str.end());
+return '"' + str + '"';
+}
+
+void EmitAnnotations(Function *F, OptimizationRemarkEmitter ) {
+OptimizationRemarkAnnotation annotations(DEBUG_TYPE, "annotation ", F);
+AttributeList list = F->getAttributes();
+for(auto a : list.getFnAttributes()) {
+annotations << "fn_attr(" << fixQuotes(a.getAsString(true)) << ") ";
+}
+for(auto a : list.getRetAttributes()) {
+annotations << "ret_attr(" << fixQuotes(a.getAsString(true)) << ") ";
+}
+for(unsigned i=0; igetFunctionType()->getNumParams(); i++)
+for(auto a : list.getParamAttributes(i)) {
+annotations << "arg_attr(" << std::to_string(i) << ", " << fixQuotes(a.getAsString(true)) << ") ";
+}
+annotations << "\n";
+  ORE.emit(annotations);
+}
+
+namespace llvm {
+
+struct EmitAnnotationsLegacyPass : public FunctionPass {
+  // Pass identification, replacement for typeid
+  static char ID;
+
+  EmitAnnotationsLegacyPass() : FunctionPass(ID) {
+initializeEmitAnnotationsLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
+
+  // runOnFunction - To run this pass, first we calculate the alloca
+  // instructions that are safe for promotion, then we promote each one.
+  bool runOnFunction(Function ) override {
+OptimizationRemarkEmitter  = getAnalysis().getORE();
+EmitAnnotations(, ORE);
+return false;
+  }
+
+  void getAnalysisUsage(AnalysisUsage ) const override {
+AU.addRequired();
+AU.setPreservesCFG();
+  }
+};
+
+} // end anonymous namespace
+
+char EmitAnnotationsLegacyPass::ID = 0;
+
+PreservedAnalyses EmitAnnotationsPass::run(Function ,
+   FunctionAnalysisManager ) {
+  OptimizationRemarkEmitter  = FM.getResult(F);
+  EmitAnnotations(, ORE);
+  return PreservedAnalyses::all();
+}
+
+INITIALIZE_PASS_BEGIN(EmitAnnotationsLegacyPass, 

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 211370.
mgehre marked 4 inline comments as done.
mgehre added a comment.

- Diagnose unions; improve other diagnostics; fix all comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -303,6 +303,8 @@
 std::string getIsOmitted() const override {
   if (type == "IdentifierInfo *")
 return "!get" + getUpperName().str() + "()";
+  if (type == "TypeSourceInfo *")
+return "!get" + getUpperName().str() + "Loc()";
   if (type == "ParamIdx")
 return "!get" + getUpperName().str() + "().isValid()";
   return "false";
@@ -336,6 +338,8 @@
<< "  OS << \" \" << SA->get" << getUpperName()
<< "()->getName();\n";
   } else if (type == "TypeSourceInfo *") {
+if (isOptional())
+  OS << "if (SA->get" << getUpperName() << "Loc())";
 OS << "OS << \" \" << SA->get" << getUpperName()
<< "().getAsString();\n";
   } else if (type == "bool") {
Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -ast-dump %s | \
+// RUN: FileCheck --implicit-check-not OwnerAttr --implicit-check-not PointerAttr %s
+
+class [[gsl::Owner]] OwnerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} OwnerMissingParameter
+// CHECK: OwnerAttr
+
+class [[gsl::Pointer]] PointerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} PointerMissingParameter
+// CHECK: PointerAttr
+
+class [[gsl::Owner()]] OwnerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} OwnerWithEmptyParameterList
+// CHECK: OwnerAttr {{.*}}
+
+class [[gsl::Pointer()]] PointerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} PointerWithEmptyParameterList
+// CHECK: PointerAttr {{.*}}
+
+class [[gsl::Owner(int)]] AnOwner{};
+// CHECK: CXXRecordDecl {{.*}} AnOwner
+// CHECK: OwnerAttr {{.*}} int
+
+struct S;
+class [[gsl::Pointer(S)]] APointer{};
+// CHECK: CXXRecordDecl {{.*}} APointer
+// CHECK: PointerAttr {{.*}} S
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner{};
+// CHECK: CXXRecordDecl {{.*}} DuplicateOwner
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer{};
+// CHECK: CXXRecordDecl {{.*}} DuplicatePointer
+// CHECK: PointerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater{};
+// CHECK: CXXRecordDecl {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater;
+// CHECK: CXXRecordDecl {{.*}} prev {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
Index: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+// expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer{};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+
+class [[gsl::Pointer(int)]] AddConflictLater{};
+class [[gsl::Owner(int)]] AddConflictLater;
+// expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
+// expected-note@-3 

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/AttrDocs.td:4167
+
+The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an
+object of type ``T``:

aaron.ballman wrote:
> Do either of these attributes make sense on a union type? If so, might be 
> worth mentioning unions here and below. If not, would it be worth diagnosing 
> on a union?
Good catch! I added diagnostics and a test.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2516
   "%0 and %1 attributes are not compatible">;
+def err_attribute_invalid_argument : Error<
+  "%0 is an invalid argument to attribute %1">;

aaron.ballman wrote:
> Can you combine this one with `err_attribute_argument_vec_type_hint`?
I'm not sure: vec_type_hint reads `"invalid attribute argument %0 - expecting a 
vector or vectorizable scalar type"` and here `""%0 is an invalid argument to 
attribute %1"`, i.e. one is positive ("expecting ...") and the other is 
negative ("%0 is an invalid argument").

I don't know how to describe "not void, not reference, not array type" in terms 
of "expecting ...", and I think that we should keep "expecting a vector or 
vectorizable scalar type" on the  VecTypeHint attribute diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954



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

[PATCH] D65169: [WIP] LLVM Optimization Remark for Attributes

[William Moses via Phabricator via cfe-commits]

wsmoses created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mgorny, 
mehdi_amini.
Herald added projects: clang, LLVM.

This is a *work in progress* patch illustrating how an optimization remark that 
prints out derived function/argument attributes could be implemented.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65169

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/LinkAllPasses.h
  llvm/include/llvm/Transforms/Utils.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/Utils.cpp

Index: llvm/lib/Transforms/Utils/Utils.cpp
===
--- llvm/lib/Transforms/Utils/Utils.cpp
+++ llvm/lib/Transforms/Utils/Utils.cpp
@@ -26,6 +26,7 @@
   initializeAddDiscriminatorsLegacyPassPass(Registry);
   initializeBreakCriticalEdgesPass(Registry);
   initializeCanonicalizeAliasesLegacyPassPass(Registry);
+  initializeEmitAnnotationsLegacyPassPass(Registry);
   initializeInstNamerPass(Registry);
   initializeLCSSAWrapperPassPass(Registry);
   initializeLibCallsShrinkWrapLegacyPassPass(Registry);
Index: llvm/lib/Transforms/Utils/CMakeLists.txt
===
--- llvm/lib/Transforms/Utils/CMakeLists.txt
+++ llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -12,6 +12,7 @@
   CodeExtractor.cpp
   CtorUtils.cpp
   DemoteRegToStack.cpp
+  EmitAnnotations.cpp
   EntryExitInstrumenter.cpp
   EscapeEnumerator.cpp
   Evaluator.cpp
Index: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
===
--- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -777,7 +777,7 @@
   MPM.add(createCFGSimplificationPass());
 
   addExtensionsToPM(EP_OptimizerLast, MPM);
-
+  MPM.add(createEmitAnnotationsPass());
   if (PrepareForLTO) {
 MPM.add(createCanonicalizeAliasesPass());
 // Rename anon globals to be able to handle them in the summary
Index: llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
===
--- llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
@@ -24,48 +24,6 @@
  "example -force-attribute=foo:noinline. This "
  "option can be specified multiple times."));
 
-static Attribute::AttrKind parseAttrKind(StringRef Kind) {
-  return StringSwitch(Kind)
-  .Case("alwaysinline", Attribute::AlwaysInline)
-  .Case("builtin", Attribute::Builtin)
-  .Case("cold", Attribute::Cold)
-  .Case("convergent", Attribute::Convergent)
-  .Case("inlinehint", Attribute::InlineHint)
-  .Case("jumptable", Attribute::JumpTable)
-  .Case("minsize", Attribute::MinSize)
-  .Case("naked", Attribute::Naked)
-  .Case("nobuiltin", Attribute::NoBuiltin)
-  .Case("noduplicate", Attribute::NoDuplicate)
-  .Case("noimplicitfloat", Attribute::NoImplicitFloat)
-  .Case("noinline", Attribute::NoInline)
-  .Case("nonlazybind", Attribute::NonLazyBind)
-  .Case("noredzone", Attribute::NoRedZone)
-  .Case("noreturn", Attribute::NoReturn)
-  .Case("nocf_check", Attribute::NoCfCheck)
-  .Case("norecurse", Attribute::NoRecurse)
-  .Case("nounwind", Attribute::NoUnwind)
-  .Case("optforfuzzing", Attribute::OptForFuzzing)
-  .Case("optnone", Attribute::OptimizeNone)
-  .Case("optsize", Attribute::OptimizeForSize)
-  .Case("readnone", Attribute::ReadNone)
-  .Case("readonly", Attribute::ReadOnly)
-  .Case("argmemonly", Attribute::ArgMemOnly)
-  .Case("returns_twice", Attribute::ReturnsTwice)
-  .Case("safestack", Attribute::SafeStack)
-  .Case("shadowcallstack", Attribute::ShadowCallStack)
-  .Case("sanitize_address", Attribute::SanitizeAddress)
-  .Case("sanitize_hwaddress", Attribute::SanitizeHWAddress)
-  .Case("sanitize_memory", Attribute::SanitizeMemory)
-  .Case("sanitize_thread", Attribute::SanitizeThread)
-  .Case("speculative_load_hardening", Attribute::SpeculativeLoadHardening)
-  .Case("ssp", Attribute::StackProtect)
-  .Case("sspreq", Attribute::StackProtectReq)
-  .Case("sspstrong", Attribute::StackProtectStrong)
-  .Case("strictfp", Attribute::StrictFP)
-  .Case("uwtable", Attribute::UWTable)
-  

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3279
+  "implicit conversion from %2 to %3 changes value from %0 to %1">,
+  InGroup, DefaultIgnore;
+

xbolva00 wrote:
> Drop ‘DefaultIgnore‘ here
I wish both warnings to be off by default and can be turned on by 
`-Wconversion` (`-Wimplicit-float-conversion`).


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

https://reviews.llvm.org/D64666



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

[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 211363.
gamesh411 added a comment.

Too much autoformat fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp

Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -188,8 +188,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance )
-: CI(CI), Context(CI.getASTContext()),
-  CTULoadThreshold(CI.getAnalyzerOpts()->CTUImportThreshold) {}
+: CI(CI), Context(CI.getASTContext()), ASTStorage(CI),
+  CTULoadGuard(CI.getAnalyzerOpts()->CTUImportThreshold) {}
 
 CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
 
@@ -237,8 +237,8 @@
   if (LookupName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
-  llvm::Expected ASTUnitOrError = loadExternalAST(
-  LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
+  llvm::Expected ASTUnitOrError =
+  loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -340,6 +340,144 @@
   }
 }
 
+CrossTranslationUnitContext::LoadGuard::LoadGuard(unsigned CTULoadThreshold)
+: CTULoadThreshold(CTULoadThreshold), NumASTLoaded(0u) {}
+
+CrossTranslationUnitContext::LoadPass
+CrossTranslationUnitContext::LoadGuard::beginLoad() {
+  bool CanBegin = NumASTLoaded < CTULoadThreshold;
+  return {NumASTLoaded, CanBegin};
+}
+
+CrossTranslationUnitContext::LoadPass::LoadPass(unsigned ,
+bool CanBegin)
+: NumASTLoaded(NumASTLoaded), CanBegin(CanBegin), WasSuccessful(false) {}
+
+CrossTranslationUnitContext::LoadPass::~LoadPass() {
+  if (WasSuccessful)
+++NumASTLoaded;
+}
+
+void CrossTranslationUnitContext::LoadPass::wasSuccessful() {
+  WasSuccessful = true;
+}
+
+CrossTranslationUnitContext::LoadPass::operator bool() const {
+  return CanBegin;
+}
+
+CrossTranslationUnitContext::ASTDumpLoader::ASTDumpLoader(
+const CompilerInstance )
+: CI(CI) {}
+
+std::unique_ptr
+CrossTranslationUnitContext::ASTDumpLoader::operator()(StringRef ASTFileName) {
+  // Load AST from ast-dump.
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr Diags(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  return ASTUnit::LoadFromASTFile(
+  ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts());
+}
+
+CrossTranslationUnitContext::ASTUnitStorage::ASTUnitStorage(
+const CompilerInstance )
+: FileAccessor(CI) {}
+
+llvm::Expected
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFile(StringRef FileName) {
+  // Try the cache first.
+  auto ASTCacheEntry = FileASTUnitMap.find(FileName);
+  if (ASTCacheEntry == FileASTUnitMap.end()) {
+// Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+std::unique_ptr LoadedUnit = FileAccessor(FileName);
+
+// Need the raw pointer and the unique_ptr as well.
+ASTUnit* Unit = LoadedUnit.get();
+
+// Update the cache.
+FileASTUnitMap[FileName] = std::move(LoadedUnit);
+return Unit;
+
+  } else {
+// Found in the cache.
+return ASTCacheEntry->second.get();
+  }
+}
+
+llvm::Expected
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFunction(
+StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
+  // Try the cache first.
+  auto ASTCacheEntry = NameASTUnitMap.find(FunctionName);
+  if (ASTCacheEntry == NameASTUnitMap.end()) {
+// Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+
+// Ensure that the Index is loaded, as we need to search in it.
+if (llvm::Error IndexLoadError =
+ensureCTUIndexLoaded(CrossTUDir, IndexName))
+  return std::move(IndexLoadError);
+
+// Check if there is and entry in the index for the function.
+if (!NameFileMap.count(FunctionName)) {
+  ++NumNotInOtherTU;
+  return llvm::make_error(index_error_code::missing_definition);
+}
+
+// Search in the index for the filename where the definition of FuncitonName
+// resides.
+if (llvm::Expected FoundForFile =
+getASTUnitForFile(NameFileMap[FunctionName])) {
+
+  // Update the cache.
+  NameASTUnitMap[FunctionName] = *FoundForFile;
+  return *FoundForFile;
+
+} else {
+  return FoundForFile.takeError();
+}
+  } else {
+// Found in the cache.
+return 

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for the patch and following up on the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64678



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

[PATCH] D64938: [clang-doc] Add option for user provided stylesheets

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 211362.
DiegoAstiazaran marked an inline comment as done.
DiegoAstiazaran added a comment.

Add a second CSS file to one of the tests in HTMLGeneratorTest.cpp


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

https://reviews.llvm.org/D64938

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -40,7 +40,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -94,7 +94,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -158,7 +158,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -206,7 +206,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -343,7 +343,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(---
Index: clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -38,7 +38,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(# namespace Namespace
 
@@ -101,7 +101,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(# class r
 
@@ -162,7 +162,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(### f
 
@@ -190,7 +190,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected = R"raw(| enum class e |
 
@@ -320,7 +320,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  auto Err = G->generateDocForInfo(, Actual, ClangDocContext());
   assert(!Err);
   std::string Expected =
   R"raw(### f
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -21,6 +21,16 @@
   return std::move(G.get());
 }
 
+ClangDocContext
+getClangDocContext(std::vector UserStylesheets = {}) {
+  ClangDocContext CDCtx;
+  CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+  CDCtx.UserStylesheets.insert(
+  CDCtx.UserStylesheets.begin(),
+  "../share/clang/clang-doc-default-stylesheet.css");
+  return CDCtx;
+}
+
 TEST(HTMLGeneratorTest, emitNamespaceHTML) {
   NamespaceInfo I;
   I.Name = "Namespace";
@@ -38,12 +48,14 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  ClangDocContext CDCtx = 

[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 211361.
gamesh411 added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Refactor functionality into local classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  llvm/test/CodeGen/X86/packss.ll

Index: llvm/test/CodeGen/X86/packss.ll
===
--- llvm/test/CodeGen/X86/packss.ll
+++ llvm/test/CodeGen/X86/packss.ll
@@ -266,7 +266,7 @@
 }
 
 define <16 x i8> @packsswb_icmp_128_zero(<8 x i16> %a0) {
-; SSE-LABEL: packsswb_128_zero:
+; SSE-LABEL: packsswb_icmp_128_zero:
 ; SSE:   # %bb.0:
 ; SSE-NEXT:pxor %xmm1, %xmm1
 ; SSE-NEXT:pcmpeqw %xmm0, %xmm1
@@ -274,7 +274,7 @@
 ; SSE-NEXT:movq {{.*#+}} xmm0 = xmm1[0],zero
 ; SSE-NEXT:ret{{[l|q]}}
 ;
-; AVX-LABEL: packsswb_128_zero:
+; AVX-LABEL: packsswb_icmp_128_zero:
 ; AVX:   # %bb.0:
 ; AVX-NEXT:vpxor %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:vpcmpeqw %xmm1, %xmm0, %xmm0
@@ -287,7 +287,7 @@
 }
 
 define <32 x i8> @packsswb_icmp_zero_256(<16 x i16> %a0) {
-; SSE-LABEL: packsswb_zero_256:
+; SSE-LABEL: packsswb_icmp_zero_256:
 ; SSE:   # %bb.0:
 ; SSE-NEXT:pxor %xmm2, %xmm2
 ; SSE-NEXT:pcmpeqw %xmm2, %xmm1
@@ -298,7 +298,7 @@
 ; SSE-NEXT:movaps %xmm2, %xmm1
 ; SSE-NEXT:ret{{[l|q]}}
 ;
-; AVX1-LABEL: packsswb_zero_256:
+; AVX1-LABEL: packsswb_icmp_zero_256:
 ; AVX1:   # %bb.0:
 ; AVX1-NEXT:vextractf128 $1, %ymm0, %xmm1
 ; AVX1-NEXT:vpxor %xmm2, %xmm2, %xmm2
@@ -311,7 +311,7 @@
 ; AVX1-NEXT:vblendps {{.*#+}} ymm0 = ymm1[0,1],ymm0[2,3],ymm1[4,5],ymm0[6,7]
 ; AVX1-NEXT:ret{{[l|q]}}
 ;
-; AVX2-LABEL: packsswb_zero_256:
+; AVX2-LABEL: packsswb_icmp_zero_256:
 ; AVX2:   # %bb.0:
 ; AVX2-NEXT:vpxor %xmm1, %xmm1, %xmm1
 ; AVX2-NEXT:vpcmpeqw %ymm1, %ymm0, %ymm0
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -18,8 +18,8 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Index/USRGeneration.h"
-#include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -65,8 +65,7 @@
   Rhs.getVendor() != Triple::UnknownVendor &&
   Lhs.getVendor() != Rhs.getVendor())
 return false;
-  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
-  Lhs.getOS() != Rhs.getOS())
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() && Lhs.getOS() != Rhs.getOS())
 return false;
   if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
   Rhs.getEnvironment() != Triple::UnknownEnvironment &&
@@ -188,8 +187,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance )
-: CI(CI), Context(CI.getASTContext()),
-  CTULoadThreshold(CI.getAnalyzerOpts()->CTUImportThreshold) {}
+: CI(CI), Context(CI.getASTContext()), ASTStorage(CI),
+  CTULoadGuard(CI.getAnalyzerOpts()->CTUImportThreshold) {}
 
 CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
 
@@ -237,8 +236,8 @@
   if (LookupName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
-  llvm::Expected ASTUnitOrError = loadExternalAST(
-  LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
+  llvm::Expected ASTUnitOrError =
+  loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -340,6 +339,144 @@
   }
 }
 
+CrossTranslationUnitContext::LoadGuard::LoadGuard(unsigned CTULoadThreshold)
+: CTULoadThreshold(CTULoadThreshold), NumASTLoaded(0u) {}
+
+CrossTranslationUnitContext::LoadPass
+CrossTranslationUnitContext::LoadGuard::beginLoad() {
+  bool CanBegin = NumASTLoaded < CTULoadThreshold;
+  return {NumASTLoaded, CanBegin};
+}
+
+CrossTranslationUnitContext::LoadPass::LoadPass(unsigned ,
+bool CanBegin)
+: NumASTLoaded(NumASTLoaded), CanBegin(CanBegin), WasSuccessful(false) {}
+
+CrossTranslationUnitContext::LoadPass::~LoadPass() {
+  if (WasSuccessful)
+++NumASTLoaded;
+}
+
+void CrossTranslationUnitContext::LoadPass::wasSuccessful() {
+  WasSuccessful = true;
+}
+
+CrossTranslationUnitContext::LoadPass::operator bool() const {
+  return CanBegin;
+}
+
+CrossTranslationUnitContext::ASTDumpLoader::ASTDumpLoader(
+const CompilerInstance )
+: CI(CI) {}
+
+std::unique_ptr
+CrossTranslationUnitContext::ASTDumpLoader::operator()(StringRef ASTFileName) {
+  // Load AST from ast-dump.
+  

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D65050#1596514 , @efriedma wrote:

> Is this the only place where a compound type can contain a PackExpansionType?


Yes AFAIK. No other place can contain a list or a pack expansion. The closest 
thing is dynamic exception specification (which isn't part of the type), and it 
seems to be handled properly.

> The code could probably use a brief comment explaining why we need to check 
> this explicitly, even though it isn't listed in [temp.dep.type].

Done.


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 211358.

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

https://reviews.llvm.org/D65050

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/alias-templates.cpp


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 
'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,10 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+// A pack expansion with a non-dependent pattern still affects the number 
of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4820 [temp.dep.type].
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,10 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+// A pack expansion with a non-dependent pattern still affects the number of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4820 [temp.dep.type].
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65110: [NewPM] Run avx*-builtins.c tests under the new pass manager only

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/test/CodeGen/avx512vlbw-builtins.c:904
+  // CHECK: [[TMP:%.*]] = bitcast [[SRCTY:<.*>]] [[SEL]] to [[DSTTY:<.*>]]
+  // CHECK: [[SEL:%.*]] = bitcast [[DSTTY]] [[TMP]] to [[SRCTY]]
   // CHECK: select <16 x i1> %{{.*}}, <16 x i8> [[SEL]], <16 x i8> %{{.*}}

Can you release the DSTTY/SRCTY patterns with the actual types to improve 
readability?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65110



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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry added a comment.

In D64454#1587102 , @aaron.ballman 
wrote:

> I think this looks reasonable to me, though I am still not certain if the 
> relative path in the python script will work with both the svn in-tree 
> directory layout as well as the git monorepo layout (which I'm far less 
> familiar with).


This works in the git monorepo layout as that's what I'm using. Not sure if it 
works with the svn in-tree directory layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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

[PATCH] D65107: [clang-doc] Fix html entities in rendered text

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65107



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

[PATCH] D64938: [clang-doc] Add option for user provided stylesheets

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:26
+  ClangDocContext CDCtx;
+  CDCtx.UserStylesheets = {"../share/clang/clang-doc-default-stylesheet.css"};
+  return CDCtx;

Can you write a test for the output of more than one stylesheet?


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

https://reviews.llvm.org/D64938



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett added a comment.

LGTM


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

https://reviews.llvm.org/D64539



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

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Apologies for the early ping! Bu I'm off next weeks, so it would be nice to get 
this in before that if there are no further comments.
Tomorrow, I will upload another diff that builds on top D64916 
, which enables code-generation with tail 
loops folded, thus also demonstrating an end to end test.


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

https://reviews.llvm.org/D64744



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

[PATCH] D64914: Implement P1771

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2835
+  D->getFunctionType()->getReturnType()->isVoidType() &&
+  !isa(D)) {
 S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0;

erichkeane wrote:
> aaron.ballman wrote:
> > Conversion functions?
> Conversion functions don't return 'void', so the 2nd condition should do it, 
> right?
true -- we already seem to diagnose a conversion function that returns void 
anyway. Sorry for the noise!


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

https://reviews.llvm.org/D64914



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

[PATCH] D64914: Implement P1771

[Erich Keane via Phabricator via cfe-commits]

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

Ugg... well conversion functions are an interesting bit, as well as Type{}; 
with no constructors.  It ends up being a function style cast to an 
init-list-expr, so its going to require a bit more work.  Stay tuned :)




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2835
+  D->getFunctionType()->getReturnType()->isVoidType() &&
+  !isa(D)) {
 S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0;

aaron.ballman wrote:
> Conversion functions?
Conversion functions don't return 'void', so the 2nd condition should do it, 
right?


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

https://reviews.llvm.org/D64914



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

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

The preprocessor is defined in `Lex`, so i guess i'm curious about a more 
precise description of what's impossible to do without `Frontend`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64638



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

[PATCH] D65154: [clangd] Reformat use of cl::opt: use unqualified name and don't bin-pack attributes. NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

That LGTM, but i didn't have any bad feelings regarding the previous state. 
Since we tend to read those options in isolation and don't care about how they 
look like in general, but that might be just me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65154



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:2110
+  /// Emit code for the user defined mapper construct.
+  void emitUserDefinedMapper(const OMPDeclareMapperDecl *D,
+ CodeGenFunction *CGF = nullptr) override;

lildmh wrote:
> ABataev wrote:
> > I think you can drop this function here if the original function is not 
> > virtual
> The function for simd only mode includes a `llvm_unreachable`, so I think 
> it's still needed as a virtual function above
It is better to reduce number of virtual functions, if possible.


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

https://reviews.llvm.org/D59474



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh marked 3 inline comments as done.
lildmh added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7116-7124
+  /// Get the offset of the OMP_MAP_MEMBER_OF field.
+  static unsigned getFlagMemberOffset() {
+unsigned Offset = 0;
+for (uint64_t Remain = OMP_MAP_MEMBER_OF; !(Remain & 1);
+ Remain = Remain >> 1)
+  Offset++;
+return Offset;

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > Maybe it is better to define a constant `constexpr uint64_t 
> > > OMP_MEMBER_OF_RANK = 48` and then deduce `OMP_MAP_MEMBER_OF` as 
> > > `~((1< > In libomptarget, the same way is used to define 
> > `OMP_TGT_MAPTYPE_MEMBER_OF`: `OMP_TGT_MAPTYPE_MEMBER_OF   = 
> > 0x`. So I think they should stay the same. Btw, the number 
> > 48 is directly used in libomptarget now, which may need to change in the 
> > future.
> > 
> > In your code, it assumes bits higher than 48 are all `OMP_MAP_MEMBER_OF`, 
> > which may not be true in the future. My code here is more universal, 
> > although it does not look great. What do you think?
> You can apply a mask to drop some of the most significant bits if required. 
> My code looks much cleaner it would be good to have the same code in 
> libomptarget too.
> But it is up to you what to do here.
> 
Maybe let's keep it this way in this patch, then we modify both places in a 
further patch?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8116
+MapFlagsArrayTy ) const {
+// FIXME: MSVC 2013 seems to require this-> to find member CurDir.
+assert(this->CurDir.is() &&

ABataev wrote:
> ABataev wrote:
> > AFAIK, LLVM has dropped support for msvc 2013, do we still need this?
> Ping.
I'm okay either way. I guess it doesn't hurt to keep it?



Comment at: lib/CodeGen/CGOpenMPRuntime.h:2110
+  /// Emit code for the user defined mapper construct.
+  void emitUserDefinedMapper(const OMPDeclareMapperDecl *D,
+ CodeGenFunction *CGF = nullptr) override;

ABataev wrote:
> I think you can drop this function here if the original function is not 
> virtual
The function for simd only mode includes a `llvm_unreachable`, so I think it's 
still needed as a virtual function above


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

https://reviews.llvm.org/D59474



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 211340.
Mordante added a comment.

Addresses @gribozavr comments.


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

https://reviews.llvm.org/D64696

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1044,6 +1044,48 @@
 template 
 void test_attach38::test_attach39(int, B);
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_bad(int);
+
+/// \a A
+int test_inline_no_argument_a_good(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_bad(int);
+
+/// @b A
+int test_inline_no_argument_b_good(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_bad(int);
+
+/// \c A
+int test_inline_no_argument_c_good(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_bad(int);
+
+/// \e A
+int test_inline_no_argument_e_good(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_bad(int);
+
+/// \em A
+int test_inline_no_argument_em_good(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_bad(int);
+
+/// \p A
+int test_inline_no_argument_p_good(int);
 
 // PR13411, reduced.  We used to crash on this.
 /**
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -422,6 +422,12 @@
 IC = S.actOnInlineCommand(CommandTok.getLocation(),
   CommandTok.getEndLocation(),
   CommandTok.getCommandID());
+
+Diag(CommandTok.getEndLocation().getLocWithOffset(1),
+ diag::warn_doc_inline_contents_no_argument)
+<< CommandTok.is(tok::at_command)
+<< Traits.getCommandInfo(CommandTok.getCommandID())->Name
+<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }
 
   Retokenizer.putBackLeftoverTokens();
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -153,6 +153,12 @@
 def note_add_deprecation_attr : Note<
   "add a deprecation attribute to the declaration to silence this warning">;
 
+// inline contents commands
+
+def warn_doc_inline_contents_no_argument : Warning<
+  "'%select{\\|@}0%1' command does not have an argument">,
+  InGroup, DefaultIgnore;
+
 // verbatim block commands
 
 def warn_verbatim_block_end_without_start : Warning<


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1044,6 +1044,48 @@
 template 
 void test_attach38::test_attach39(int, B);
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_bad(int);
+
+/// \a A
+int test_inline_no_argument_a_good(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_bad(int);
+
+/// @b A
+int test_inline_no_argument_b_good(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_bad(int);
+
+/// \c A
+int test_inline_no_argument_c_good(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_bad(int);
+
+/// \e A
+int test_inline_no_argument_e_good(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_bad(int);
+
+/// \em A
+int test_inline_no_argument_em_good(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_bad(int);
+
+/// \p A
+int test_inline_no_argument_p_good(int);
 
 // PR13411, reduced.  We used to crash on this.
 /**
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -422,6 +422,12 @@
 IC = S.actOnInlineCommand(CommandTok.getLocation(),
   CommandTok.getEndLocation(),
   CommandTok.getCommandID());
+
+

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

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

All inline commands defined in include/clang/AST/CommentCommands.td require an 
argument. The escape commands, like \&, are handled in the switch starting at 
lib/AST/CommentLexer.cpp:366. They are stored as Text and not as an 
InlineCommand.

If you want I can add an extra field in the Command class in 
include/clang/AST/CommentCommands.td. Something like `bit 
IsEmptyInlineCommandAllowed = 0;`.


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

https://reviews.llvm.org/D64696



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:2533
+ CodeGenFunction *CGF) {
+  if (!LangOpts.OpenMP || LangOpts.OpenMPSimd ||
+  (!LangOpts.EmitAllDecls && !D->isUsed()))

lildmh wrote:
> ABataev wrote:
> > Why do we need to emit it for simd only mode?
> This code is not emitting mapper for simd only mode.
Ah, yes, it is the condition for the early exit.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7116-7124
+  /// Get the offset of the OMP_MAP_MEMBER_OF field.
+  static unsigned getFlagMemberOffset() {
+unsigned Offset = 0;
+for (uint64_t Remain = OMP_MAP_MEMBER_OF; !(Remain & 1);
+ Remain = Remain >> 1)
+  Offset++;
+return Offset;

lildmh wrote:
> ABataev wrote:
> > Maybe it is better to define a constant `constexpr uint64_t 
> > OMP_MEMBER_OF_RANK = 48` and then deduce `OMP_MAP_MEMBER_OF` as 
> > `~((1< In libomptarget, the same way is used to define `OMP_TGT_MAPTYPE_MEMBER_OF`: 
> `OMP_TGT_MAPTYPE_MEMBER_OF   = 0x`. So I think they 
> should stay the same. Btw, the number 48 is directly used in libomptarget 
> now, which may need to change in the future.
> 
> In your code, it assumes bits higher than 48 are all `OMP_MAP_MEMBER_OF`, 
> which may not be true in the future. My code here is more universal, although 
> it does not look great. What do you think?
You can apply a mask to drop some of the most significant bits if required. My 
code looks much cleaner it would be good to have the same code in libomptarget 
too.
But it is up to you what to do here.




Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8116
+MapFlagsArrayTy ) const {
+// FIXME: MSVC 2013 seems to require this-> to find member CurDir.
+assert(this->CurDir.is() &&

ABataev wrote:
> AFAIK, LLVM has dropped support for msvc 2013, do we still need this?
Ping.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:810-815
+  virtual void emitUserDefinedMapper(const OMPDeclareMapperDecl *D,
+ CodeGenFunction *CGF = nullptr);
+
+  /// Emit the array initialization or deletion portion for user-defined mapper
+  /// code generation.
+  virtual void emitUDMapperArrayInitOrDel(

I don't think we need virtual functions here, non-virtual are good enough.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:2110
+  /// Emit code for the user defined mapper construct.
+  void emitUserDefinedMapper(const OMPDeclareMapperDecl *D,
+ CodeGenFunction *CGF = nullptr) override;

I think you can drop this function here if the original function is not virtual


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

https://reviews.llvm.org/D59474



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

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

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

xbolva00 added a comment.

>>   they parse the attributes first then attempt to parse a declaration; if 
>> that fails, they fall back to parsing a statement

Well, I don’t think this reparsing is ideal in terms of compile time either.

If we really care about attributes on implicit ints: I don’t think that parsing 
according to attribute name is so bad solution - if only “possible” issue is 
same attr. name for stmt and decl for some future attribute - let’s talk with 
GCC devs and make a deal about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[Rainer Orth via Phabricator via cfe-commits]

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

In D64793#1597550 , @jyknight wrote:

> Is this really necessary? Users don't typically pass -std= to the driver for 
> linking anyways (what do you even pass if you've compiled both C and C++ 
> code?) so this seems a rather odd way to control behavior.


I fear it is necessary: at least it matches documented behaviour of both the 
Sun/Oracle Studio compilers and gcc.

The -std= options usually get passed to the linking step because CFLAGS is 
added to the options as well.  In the mixed-language case, you have to link
with the C++ compiler, and the !isGNUMode test deals with both languages alike.

> How about instead just adding "values-xpg6.o" unconditionally, alongside the 
> current unconditional "values-Xa.o", and just forget about the xpg4 and Xc 
> modes?

If all else fails, that would have to be the last fallback.  I'd rather do 
things correctly, though.




Comment at: lib/Driver/ToolChains/Solaris.cpp:16
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/LangStandard.h"
 #include "llvm/Option/ArgList.h"

jyknight wrote:
> I'm not sure that's an acceptable dependency -- I think Driver probably is 
> not supposed to depend on Frontend. If so, I guess LangStandard should move 
> to clang/Basic/. Which also means moving InputKind from 
> clang/include/clang/Frontend/FrontendOptions.h.
> 
> (Maybe someone else can weigh in on this question?)
I wondered about this myself, including frontend code in the
driver might be considered a layering violation.  I certainly need
guidance here what is and isn't acceptable here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64793



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

[PATCH] D65125: clang-format: Fix namespace end comments for namespaces with attributes and macros

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366831: clang-format: Fix namespace end comments for 
namespaces with attributes and… (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65125?vs=211228=211334#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65125

Files:
  cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1873,8 +1873,13 @@
   if (InitialToken.is(TT_NamespaceMacro)) {
 parseParens();
   } else {
-while (FormatTok->isOneOf(tok::identifier, tok::coloncolon))
-  nextToken();
+while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
+  tok::l_square)) {
+  if (FormatTok->is(tok::l_square))
+parseSquare();
+  else
+nextToken();
+}
   }
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (ShouldBreakBeforeBrace(Style, InitialToken))
Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -36,7 +36,7 @@
   const FormatToken *Tok = NamespaceTok->getNextNonComment();
   if (NamespaceTok->is(TT_NamespaceMacro)) {
 // Collects all the non-comment tokens between opening parenthesis
-// and closing parenthesis or comma
+// and closing parenthesis or comma.
 assert(Tok && Tok->is(tok::l_paren) && "expected an opening parenthesis");
 Tok = Tok->getNextNonComment();
 while (Tok && !Tok->isOneOf(tok::r_paren, tok::comma)) {
@@ -44,9 +44,21 @@
   Tok = Tok->getNextNonComment();
 }
   } else {
-// Collects all the non-comment tokens between 'namespace' and '{'.
+// For `namespace [[foo]] A::B::inline C {` or
+// `namespace MACRO1 MACRO2 A::B::inline C {`, returns "A::B::inline C".
+// Peek for the first '::' (or '{') and then return all tokens from one
+// token before that up until the '{'.
+const FormatToken *FirstNSTok = Tok;
+while (Tok && !Tok->is(tok::l_brace) && !Tok->is(tok::coloncolon)) {
+  FirstNSTok = Tok;
+  Tok = Tok->getNextNonComment();
+}
+
+Tok = FirstNSTok;
 while (Tok && !Tok->is(tok::l_brace)) {
   name += Tok->TokenText;
+  if (Tok->is(tok::kw_inline))
+name += " ";
   Tok = Tok->getNextNonComment();
 }
   }
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1782,6 +1782,21 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("namespace N::inline D {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace N::inline D::E {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace [[deprecated(\"foo[bar\")]] some_namespace {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("/* something */ namespace some_namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -77,6 +77,44 @@
 "int i;\n"
 "int j;\n"
 "}"));
+
+  EXPECT_EQ("namespace [[deprecated(\"foo\")]] A::B {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::B",
+fixNamespaceEndComments("namespace [[deprecated(\"foo\")]] A::B {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+
+  EXPECT_EQ("namespace [[deprecated(\"foo\")]] A::inline B::inline C {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::inline B::inline C",
+fixNamespaceEndComments(
+

r366831 - clang-format: Fix namespace end comments for namespaces with attributes and macros.

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Jul 23 10:49:45 2019
New Revision: 366831

URL: http://llvm.org/viewvc/llvm-project?rev=366831=rev
Log:
clang-format: Fix namespace end comments for namespaces with attributes and 
macros.

Fixes PR39247.

While here, also make C++20 `namespace A::inline B::inline C` nested
inline namespaced definitions work.

Before:
#define DEPRECATE_WOOF [[deprecated("meow")]]

namespace DEPRECATE_WOOF woof {
void f() {}
} // namespace DEPRECATE_WOOFwoof

namespace [[deprecated("meow")]] woof {
  void f() {}
} // namespace [[deprecated("meow")]]woof

namespace woof::inline bark {
  void f() {}
} // namespace woof::inlinebark

Now:
#define DEPRECATE_WOOF [[deprecated("meow")]]

namespace DEPRECATE_WOOF woof {
void f() {}
} // namespace woof

namespace [[deprecated("meow")]] woof {
void f() {}
} // namespace woof

namespace woof::inline bark {
void f() {}
} // namespace woof::inline bark

(In addition to the fixed namespace end comments, also note the correct
indent of the namespace contents.)

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

Modified:
cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=366831=366830=366831=diff
==
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original)
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Tue Jul 23 10:49:45 2019
@@ -36,7 +36,7 @@ std::string computeName(const FormatToke
   const FormatToken *Tok = NamespaceTok->getNextNonComment();
   if (NamespaceTok->is(TT_NamespaceMacro)) {
 // Collects all the non-comment tokens between opening parenthesis
-// and closing parenthesis or comma
+// and closing parenthesis or comma.
 assert(Tok && Tok->is(tok::l_paren) && "expected an opening parenthesis");
 Tok = Tok->getNextNonComment();
 while (Tok && !Tok->isOneOf(tok::r_paren, tok::comma)) {
@@ -44,9 +44,21 @@ std::string computeName(const FormatToke
   Tok = Tok->getNextNonComment();
 }
   } else {
-// Collects all the non-comment tokens between 'namespace' and '{'.
+// For `namespace [[foo]] A::B::inline C {` or
+// `namespace MACRO1 MACRO2 A::B::inline C {`, returns "A::B::inline C".
+// Peek for the first '::' (or '{') and then return all tokens from one
+// token before that up until the '{'.
+const FormatToken *FirstNSTok = Tok;
+while (Tok && !Tok->is(tok::l_brace) && !Tok->is(tok::coloncolon)) {
+  FirstNSTok = Tok;
+  Tok = Tok->getNextNonComment();
+}
+
+Tok = FirstNSTok;
 while (Tok && !Tok->is(tok::l_brace)) {
   name += Tok->TokenText;
+  if (Tok->is(tok::kw_inline))
+name += " ";
   Tok = Tok->getNextNonComment();
 }
   }

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=366831=366830=366831=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Jul 23 10:49:45 2019
@@ -1873,8 +1873,13 @@ void UnwrappedLineParser::parseNamespace
   if (InitialToken.is(TT_NamespaceMacro)) {
 parseParens();
   } else {
-while (FormatTok->isOneOf(tok::identifier, tok::coloncolon))
-  nextToken();
+while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
+  tok::l_square)) {
+  if (FormatTok->is(tok::l_square))
+parseSquare();
+  else
+nextToken();
+}
   }
   if (FormatTok->Tok.is(tok::l_brace)) {
 if (ShouldBreakBeforeBrace(Style, InitialToken))

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=366831=366830=366831=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jul 23 10:49:45 2019
@@ -1782,6 +1782,21 @@ TEST_F(FormatTest, FormatsNamespaces) {
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("namespace N::inline D {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace N::inline D::E {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("namespace 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 211333.
lildmh marked 4 inline comments as done.

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

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,414 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:2533
+ CodeGenFunction *CGF) {
+  if (!LangOpts.OpenMP || LangOpts.OpenMPSimd ||
+  (!LangOpts.EmitAllDecls && !D->isUsed()))

ABataev wrote:
> Why do we need to emit it for simd only mode?
This code is not emitting mapper for simd only mode.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7116-7124
+  /// Get the offset of the OMP_MAP_MEMBER_OF field.
+  static unsigned getFlagMemberOffset() {
+unsigned Offset = 0;
+for (uint64_t Remain = OMP_MAP_MEMBER_OF; !(Remain & 1);
+ Remain = Remain >> 1)
+  Offset++;
+return Offset;

ABataev wrote:
> Maybe it is better to define a constant `constexpr uint64_t 
> OMP_MEMBER_OF_RANK = 48` and then deduce `OMP_MAP_MEMBER_OF` as 
> `~((1

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

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

xbolva00 added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3279
+  "implicit conversion from %2 to %3 changes value from %0 to %1">,
+  InGroup, DefaultIgnore;
+

Drop ‘DefaultIgnore‘ here


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

https://reviews.llvm.org/D64666



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

[PATCH] D65043: [Format] Add C++20 standard to style options

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Friendly ping! I'm wondering, from the clang-format maintainers' point of view, 
whether a diff like this and https://reviews.llvm.org/D65044 make sense to add? 
I do think that it is useful for users to specify whether they wish to use 
C++11 or C++20 constructs, but I'm not an expert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65043



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

[PATCH] D64998: Improve clang-format-diff help output

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366828: Improve clang-format-diff help output (authored by 
nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64998?vs=210837=211331#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64998

Files:
  cfe/trunk/tools/clang-format/clang-format-diff.py


Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -8,10 +8,7 @@
 #
 
#======#
 
-r"""
-ClangFormat Diff Reformatter
-
-
+"""
 This script reads input from a unified diff and reformats all the changed
 lines. This is useful to reformat all the lines touched by a specific patch.
 Example usage for git/svn users:
@@ -35,10 +32,9 @@
 
 
 def main():
-  parser = argparse.ArgumentParser(description=
-   'Reformat changed lines in diff. Without -i 
'
-   'option just output the diff that would be '
-   'introduced.')
+  parser = argparse.ArgumentParser(description=__doc__,
+   formatter_class=
+   
argparse.RawDescriptionHelpFormatter)
   parser.add_argument('-i', action='store_true', default=False,
   help='apply edits to files instead of displaying a diff')
   parser.add_argument('-p', metavar='NUM', default=0,


Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -8,10 +8,7 @@
 #
 #======#
 
-r"""
-ClangFormat Diff Reformatter
-
-
+"""
 This script reads input from a unified diff and reformats all the changed
 lines. This is useful to reformat all the lines touched by a specific patch.
 Example usage for git/svn users:
@@ -35,10 +32,9 @@
 
 
 def main():
-  parser = argparse.ArgumentParser(description=
-   'Reformat changed lines in diff. Without -i '
-   'option just output the diff that would be '
-   'introduced.')
+  parser = argparse.ArgumentParser(description=__doc__,
+   formatter_class=
+   argparse.RawDescriptionHelpFormatter)
   parser.add_argument('-i', action='store_true', default=False,
   help='apply edits to files instead of displaying a diff')
   parser.add_argument('-p', metavar='NUM', default=0,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r366828 - Improve clang-format-diff help output

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Jul 23 10:34:18 2019
New Revision: 366828

URL: http://llvm.org/viewvc/llvm-project?rev=366828=rev
Log:
Improve clang-format-diff help output

The description in clang-format-diff.py is more useful than the one
in `clang-format-diff -h`, so use the same description in both places.

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

Modified:
cfe/trunk/tools/clang-format/clang-format-diff.py

Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=366828=366827=366828=diff
==
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Tue Jul 23 10:34:18 2019
@@ -8,10 +8,7 @@
 #
 
#======#
 
-r"""
-ClangFormat Diff Reformatter
-
-
+"""
 This script reads input from a unified diff and reformats all the changed
 lines. This is useful to reformat all the lines touched by a specific patch.
 Example usage for git/svn users:
@@ -35,10 +32,9 @@ else:
 
 
 def main():
-  parser = argparse.ArgumentParser(description=
-   'Reformat changed lines in diff. Without -i 
'
-   'option just output the diff that would be '
-   'introduced.')
+  parser = argparse.ArgumentParser(description=__doc__,
+   formatter_class=
+   
argparse.RawDescriptionHelpFormatter)
   parser.add_argument('-i', action='store_true', default=False,
   help='apply edits to files instead of displaying a diff')
   parser.add_argument('-p', metavar='NUM', default=0,


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

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup>, DefaultIgnore;

aaron.ballman wrote:
> george.burgess.iv wrote:
> > aaron.ballman wrote:
> > > george.burgess.iv wrote:
> > > > aaron.ballman wrote:
> > > > > george.burgess.iv wrote:
> > > > > > nit: I'd just say "use of function '%0'" here; "builtin" doesn't 
> > > > > > really add much.
> > > > > > 
> > > > > > I also wonder if we should be saying anything more than "we found a 
> > > > > > use of this function." Looks like GCC doesn't 
> > > > > > (https://godbolt.org/z/sYs_8G), but since this warning is sort of 
> > > > > > opinionated in itself, might it be better to expand this to "use of 
> > > > > > '%0' is discouraged"?
> > > > > > 
> > > > > > WDYT, Aaron?
> > > > > What is the purpose to this diagnostic, aside from GCC compatibility? 
> > > > > What does it protect against?
> > > > > 
> > > > > If there's a reason users should not use alloc(), it would be better 
> > > > > for the diagnostic to spell it out.
> > > > > 
> > > > > Btw, I'm okay with this being default-off because the GCC warning is 
> > > > > as well. I'm mostly hoping we can do better with our diagnostic 
> > > > > wording.
> > > > > I'm mostly hoping we can do better with our diagnostic wording
> > > > 
> > > > +1
> > > > 
> > > > > What is the purpose to this diagnostic?
> > > > 
> > > > I think the intent boils down to that `alloca` is easily misused, and 
> > > > leads to e.g., 
> > > > https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt . Since 
> > > > its use often boils down to nothing but a tiny micro-optimization, some 
> > > > parties would like to discourage its use.
> > > > 
> > > > Both glibc and bionic recommend against the use of `alloca` in their 
> > > > documentation, though glibc's docs are less assertive than bionic's, 
> > > > and explicitly call out "[alloca's] use can improve efficiency compared 
> > > > to the use of malloc plus free."
> > > > 
> > > > Greping a codebase and investigating the first 15 results:
> > > > - all of them look like microoptimizations; many of them also sit close 
> > > > to other `malloc`/`new` ops, so allocating on these paths presumably 
> > > > isn't prohibitively expensive
> > > > - all but two of the uses of `alloca` have no logic to fall back to the 
> > > > heap `malloc` if the array they want to allocate passes a certain 
> > > > threshold. Some of the uses make it look *really* easy for the array to 
> > > > grow very large.
> > > > - one of the uses compares the result of `alloca` to `NULL`. Since 
> > > > `alloca`'s behavior is undefined if it fails, ...
> > > > 
> > > > I'm having trouble putting this into a concise and actionable 
> > > > diagnostic message, though. The best I can come up with at the moment 
> > > > is something along the lines of "use of function %0 is subtle; consider 
> > > > using heap allocation instead."
> > > Okay, that's along the lines of what I was thinking.
> > > 
> > > Part of me thinks that this should not diagnose calls to `alloca()` that 
> > > are given a constant value that we can test for sanity at compile time. 
> > > e.g., calling `alloca(10)` is highly unlikely to be a problem, but 
> > > calling `alloca(100)` certainly could be, while `alloca(x)` is a 
> > > different class of problem without good static analysis.
> > > 
> > > That said, perhaps we could get away with `use of function %0 is 
> > > discouraged; there is no way to check for failure but failure may still 
> > > occur, resulting in a possibly exploitable security vulnerability` or 
> > > something along those lines?
> > Yeah, GCC has a similar `-Walloca-larger-than=N` that does roughly what you 
> > described. The icky part is exactly what you said. GCC will warn about 
> > unknown values, but considers control flow when doing so: 
> > https://godbolt.org/z/J3pGiT
> > 
> > It looks like it's the same "we apply optimizations and then see what 
> > happens" behavior as similar diagnostics: https://godbolt.org/z/lLyteP
> > 
> > WRT the diag we emit here, maybe we could use notes to break it up and 
> > imply things? e.g.
> > 
> > warning: use of function %0 is discouraged, due to its security implications
> > note: 'malloc' or 'new' are suggested alternatives, since they have 
> > well-defined behavior on failure
> > 
> > ...not sold on the idea, but it's a thought.
> > 
> > If we don't want to break it to pieces, I'm fine with your suggestion
> I'm not certain the note adds value because it will always be printed on the 
> same line as the warning. A note would make sense if we had a secondary 
> location to annotate though.
SGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D64883



___
cfe-commits mailing list

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

ziangwan added a comment.

In D64666#1597627 , @jfb wrote:

> In D64666#1597193 , @aaron.ballman 
> wrote:
>
> > In D64666#1596660 , @xbolva00 
> > wrote:
> >
> > > I think we should warn in that case even if GCC does not warn.
> >
> >
> > Strong +1.
>
>
> Sorry if the phrasing was misleading: if we know for a fact that there's a 
> problem, we should warn unconditionally. If we don't know for a fact then the 
> warning should *not* be enabled by `-Wall` nor `-Wextra`. I don't really care 
> what GCC does by default, LLVM doesn't have to match every single thing. That 
> being said, if LLVM behaves differently then maybe the flag name should be 
> different.
>
> > In D64666#1596853 , @ziangwan 
> > wrote:
> > 
> >> Final review ping.
> > 
> > 
> > Please be sure to give reviewers enough time to respond to comments before 
> > pinging a review.
>
> Indeed. You haven't answered my first comment, I'd expect you to do so and 
> not "final ping" anything. I'm not saying you must do what I say, just that 
> you must answer comments, not ignore them.


Sorry to miss out your first comment, but I did answer it under @xbolva00's 
comment. I will make sure I put my response under the original one next time.

In D64666#1596655 , @ziangwan wrote:

> In D64666#1596512 , @xbolva00 wrote:
>
> > @jfb’s comment is not addressed yet.
> >
> > In D64666#1583629 , @jfb wrote:
> >
> > > I think you want to default-ignore the "may lose precision" warnings, but 
> > > not the ones that you know always lose precision.
> >
>
>
> In GCC, the int type -> float type conversion warning is disabled by default. 
> When the user uses `-Wconversion` or `-Wimplicit-float-conversion`, both 
> "definitely lose" warning and "may lose" warning are issued. The current 
> patch works the same way as GCC.


I think we definitely should issue the warning that says "may lose" precision. 
The reason is that a "may lose" warning encourages developers to examine their 
code and definitely helps them capture potential precision loss bugs. Also, in 
most case, we won't be able to know the exact value of the integer type, e.g. 
variables. If the developers are certain they are going to do an int->float 
conversion, they can always write explicit conversion, and the "may lose" 
warnings will go away.


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

https://reviews.llvm.org/D64666



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

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry added a comment.

In D64838#1593516 , @aaron.ballman 
wrote:

> In D64838#1592520 , 
> @Nathan-Huckleberry wrote:
>
> >   void foo() {
> > __attribute__((address_space(0))) *x;
> > *y;
> >   }
> >
> >
> > If the attributes are parsed then function body looks like this to the 
> > parser:
> >
> >   {
> > *x; //this one has attributes now
> > *y;
> >   {
> >
> >
> > The first line should be a valid declaration and the second like should be 
> > a dereference of an uninitialized variable. If the attributes token is 
> > discarded before parsing the rest of the line the only way to differentiate 
> > these is by looking at the attributes added to them.
> >
> > An alternative may be parse the attributes list and immediately try to 
> > parse as a declaration then if that parsing fails attempt to parse as 
> > something else. Although this approach also has the scary implication of 
> > things that are supposed to be declarations getting reparsed as something 
> > entirely different.
>
>
> The issue is that a leading GNU-style attribute is not sufficient information 
> to determine whether we're parsing a declaration or a statement; it shouldn't 
> always be treated as a decl-specifier. I spoke with a GCC dev about how they 
> handle this, and effectively, they parse the attributes first then attempt to 
> parse a declaration; if that fails, they fall back to parsing a statement. I 
> think the way forward for us that should be similar is to parse the 
> attributes first and then wait until we see a decl-specifier before 
> determining whether we want to parse a declaration or a statement, and attach 
> the attributes after we've figured out which production we have. @rsmith may 
> have even better approaches in mind, but we're definitely agreed that we 
> should not parse statement/decl based on attribute identity. I would hope we 
> could find a way to avoid lots of re-parsing work if we can (even to the 
> point of perhaps breaking the `address_space` case because implicit int is 
> pretty horrible to rely on in the first place; it depends on whether breaking 
> that will break a lot of code or not).


@xbolva00's patch https://reviews.llvm.org/D63260?id=204583 essentially does 
that already. I'm not sure how to continue on this patch, several solutions 
have been suggested, but I'm not sure which to implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added reviewers: aaron.ballman, erichkeane.
lebedev.ri added a comment.

Test looks good, adding a few more potential reviewers..


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

https://reviews.llvm.org/D64644



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

r366823 - [clang][NFCI] Fix random typos

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Jul 23 09:54:11 2019
New Revision: 366823

URL: http://llvm.org/viewvc/llvm-project?rev=366823=rev
Log:
[clang][NFCI] Fix random typos

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/lib/Index/IndexSymbol.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=366823=366822=366823=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Tue Jul 23 
09:54:11 2019
@@ -1315,7 +1315,7 @@ class ForEachMatcher : public WrapperMat
 ///
 /// Input matchers can have any type (including other polymorphic matcher
 /// types), and the actual Matcher is generated on demand with an implicit
-/// coversion operator.
+/// conversion operator.
 template  class VariadicOperatorMatcher {
 public:
   VariadicOperatorMatcher(DynTypedMatcher::VariadicOperator Op, Ps &&... 
Params)

Modified: cfe/trunk/lib/Index/IndexSymbol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexSymbol.cpp?rev=366823=366822=366823=diff
==
--- cfe/trunk/lib/Index/IndexSymbol.cpp (original)
+++ cfe/trunk/lib/Index/IndexSymbol.cpp Tue Jul 23 09:54:11 2019
@@ -513,7 +513,7 @@ StringRef index::getSymbolKindString(Sym
   case SymbolKind::StaticProperty: return "static-property";
   case SymbolKind::Constructor: return "constructor";
   case SymbolKind::Destructor: return "destructor";
-  case SymbolKind::ConversionFunction: return "coversion-func";
+  case SymbolKind::ConversionFunction: return "conversion-func";
   case SymbolKind::Parameter: return "param";
   case SymbolKind::Using: return "using";
   }

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=366823=366822=366823=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Jul 23 09:54:11 2019
@@ -7173,7 +7173,7 @@ ExprResult Sema::BuildCXXMemberCallExpr(
 
   if (Method->getParent()->isLambda() &&
   Method->getConversionType()->isBlockPointerType()) {
-// This is a lambda coversion to block pointer; check if the argument
+// This is a lambda conversion to block pointer; check if the argument
 // was a LambdaExpr.
 Expr *SubE = E;
 CastExpr *CE = dyn_cast(SubE);


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

[PATCH] D65102: [OpenCL] Rename lang mode flag for C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D65102#1597651 , @kpet wrote:

> Hmm, maybe we need to make sure that one of the tests is using a C++ feature 
> and building with `CLC++`. This would have caught the mistake.


That's right! The only test we had wasn't using any C++ features in it. :)


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

https://reviews.llvm.org/D65102



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

[PATCH] D65102: [OpenCL] Rename lang mode flag for C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 211320.
Anastasia added a comment.

Use -cl-std=CLC++ spelling for test/CodeGenOpenCLCXX/addrspace-with-class.cl


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

https://reviews.llvm.org/D65102

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/mangle-address-space.cpp
  test/CodeGenOpenCL/builtins.cl
  test/CodeGenOpenCL/images.cl
  test/CodeGenOpenCL/logical-ops.cl
  test/CodeGenOpenCL/pipe_builtin.cl
  test/CodeGenOpenCL/sampler.cl
  test/CodeGenOpenCL/spir_version.cl
  test/CodeGenOpenCL/to_addr_builtin.cl
  test/CodeGenOpenCLCXX/address-space-castoperators.cpp
  test/CodeGenOpenCLCXX/address-space-deduction.cl
  test/CodeGenOpenCLCXX/address-space-deduction2.cl
  test/CodeGenOpenCLCXX/addrspace-conversion.cl
  test/CodeGenOpenCLCXX/addrspace-derived-base.cl
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/addrspace-operators.cl
  test/CodeGenOpenCLCXX/addrspace-references.cl
  test/CodeGenOpenCLCXX/addrspace-with-class.cl
  test/CodeGenOpenCLCXX/atexit.cl
  test/CodeGenOpenCLCXX/global_init.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/CodeGenOpenCLCXX/template-address-spaces.cl
  test/Driver/autocomplete.c
  test/Driver/opencl.cl
  test/Frontend/opencl.cl
  test/Frontend/stdlang.c
  test/Headers/opencl-c-header.cl
  test/Parser/opencl-cxx-keywords.cl
  test/Parser/opencl-cxx-virtual.cl
  test/Preprocessor/predefined-macros.c
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl
  test/SemaOpenCL/builtin.cl
  test/SemaOpenCL/clk_event_t.cl
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/extensions.cl
  test/SemaOpenCL/invalid-image.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl
  test/SemaOpenCLCXX/address-space-deduction.cl
  test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
  test/SemaOpenCLCXX/address-space-of-this.cl
  test/SemaOpenCLCXX/address-space-references.cl
  test/SemaOpenCLCXX/address-space-templates.cl
  test/SemaOpenCLCXX/address_space_overloading.cl
  test/SemaOpenCLCXX/kernel_invalid.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/newdelete.cl
  test/SemaOpenCLCXX/restricted.cl

Index: test/SemaOpenCLCXX/restricted.cl
===
--- test/SemaOpenCLCXX/restricted.cl
+++ test/SemaOpenCLCXX/restricted.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 // This test checks that various C/C++/OpenCL C constructs are not available in
 // C++ for OpenCL.
Index: test/SemaOpenCLCXX/newdelete.cl
===
--- test/SemaOpenCLCXX/newdelete.cl
+++ test/SemaOpenCLCXX/newdelete.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 class A {
   public:
Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- test/SemaOpenCLCXX/method-overload-address-space.cl
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -1,4 +1,4 @@
-//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify
 
 struct C {
   void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
Index: test/SemaOpenCLCXX/kernel_invalid.cl
===
--- test/SemaOpenCLCXX/kernel_invalid.cl
+++ test/SemaOpenCLCXX/kernel_invalid.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 struct C {
   kernel void m(); //expected-error{{kernel functions cannot be class members}}
Index: test/SemaOpenCLCXX/address_space_overloading.cl
===
--- test/SemaOpenCLCXX/address_space_overloading.cl
+++ test/SemaOpenCLCXX/address_space_overloading.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++
 // expected-no-diagnostics
 
 struct RetGlob {
Index: test/SemaOpenCLCXX/address-space-templates.cl
===
--- test/SemaOpenCLCXX/address-space-templates.cl
+++ test/SemaOpenCLCXX/address-space-templates.cl
@@ -1,4 +1,4 @@
-//RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+//RUN: %clang_cc1 %s 

Re: r366694 - [NFC] Relaxed regression tests for PR42665

[Anastasia Stulova via cfe-commits]

Great! Thanks!


From: Hans Wennborg 
Sent: 23 July 2019 15:58
To: Anastasia Stulova 
Cc: Marco Antognini ; Clang Commits 
; nd 
Subject: Re: r366694 - [NFC] Relaxed regression tests for PR42665

Merged them both in r366814.

Thanks,
Hans

On Tue, Jul 23, 2019 at 7:20 AM Anastasia Stulova
 wrote:
>
>
> + cfe-commits
>
> 
> From: Anastasia Stulova
> Sent: 23 July 2019 15:16
> To: Hans Wennborg 
> Cc: Marco Antognini 
> Subject: Re: r366694 - [NFC] Relaxed regression tests for PR42665
>
>
> @Hans, would it be possible to merge this commit along with r366670 to the 
> release 9.0?
>
>
> FY, I have also added this to the blocker bug: 
> https://bugs.llvm.org/show_bug.cgi?id=41727
>
> Thanks!
> Anastasia
> 
> From: cfe-commits  on behalf of Marco 
> Antognini via cfe-commits 
> Sent: 22 July 2019 15:47
> To: cfe-commits@lists.llvm.org 
> Subject: r366694 - [NFC] Relaxed regression tests for PR42665
>
> Author: mantognini
> Date: Mon Jul 22 07:47:36 2019
> New Revision: 366694
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366694=rev
> Log:
> [NFC] Relaxed regression tests for PR42665
>
> Following up on the buildbot failures, this commits relaxes some tests:
> instead of checking for specific IR output, it now ensures that the
> underlying issue (the crash), and only that, doesn't happen.
>
> Modified:
> cfe/trunk/test/CodeGenCXX/PR42665.cpp
>
> Modified: cfe/trunk/test/CodeGenCXX/PR42665.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR42665.cpp?rev=366694=366693=366694=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/PR42665.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/PR42665.cpp Mon Jul 22 07:47:36 2019
> @@ -1,7 +1,8 @@
> -// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -std=c++17 -O0 %s 
> -o - | FileCheck %s
> -// RUN: %clang_cc1 -triple %ms_abi_triple -emit-llvm -std=c++17 -O0 %s -o - 
> | FileCheck %s
> +// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
> %itanium_abi_triple
> +// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
> %ms_abi_triple
>
>  // Minimal reproducer for PR42665.
> +// expected-no-diagnostics
>
>  struct Foo {
>Foo() = default;
> @@ -31,31 +32,3 @@ void foobar() {
>d(f); // Invoke virtual destructor of Foo through d.
>  } // p's destructor is invoked.
>
> -// Regexes are used to handle both kind of mangling.
> -//
> -// CHECK-LABEL: define linkonce_odr{{( dso_local)?}} void 
> @{{.*deleter.*Foo.*}}(%struct.Foo* dereferenceable({{[0-9]+}})
> -// CHECK-SAME: [[T:%.*]])
> -// CHECK: [[T_ADDR:%.*]] = alloca %struct.Foo*
> -// CHECK: store %struct.Foo* [[T]], %struct.Foo** [[T_ADDR]]
> -// CHECK: [[R0:%.*]] = load %struct.Foo*, %struct.Foo** [[T_ADDR]]
> -// CHECK: [[R1:%.*]] = bitcast %struct.Foo* [[R0]] to 
> [[TYPE:.*struct\.Foo.*]]***
> -// CHECK: [[VTABLE:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[R1]]
> -// CHECK: [[VFUN:%.*]] = getelementptr inbounds [[TYPE]]*, [[TYPE]]** 
> [[VTABLE]], i64 0
> -// CHECK: [[DTOR:%.*]] = load [[TYPE]]*, [[TYPE]]** [[VFUN]]
> -// CHECK: call {{(void|i8\*)}} [[DTOR]](%struct.Foo* [[R0]]
> -//
> -// CHECK-LABEL: define{{( dso_local)?}} void @{{.*foobar.*}}()
> -// CHECK: [[P:%.*]] = alloca %struct.Pair
> -// CHECK: [[F:%.*]] = alloca %struct.Foo*
> -// CHECK: [[D:%.*]] = alloca [[TYPE:void \(%struct.Foo\*\)]]**
> -// CHECK: call void @{{.*make_pair.*}}(%struct.Pair* sret [[P]])
> -// CHECK: [[FIRST:%.*]] = getelementptr inbounds %struct.Pair, %struct.Pair* 
> [[P]], i32 0, i32 0
> -// CHECK: store %struct.Foo* [[FIRST]], %struct.Foo** [[F]]
> -// CHECK: [[SECOND:%.*]] = getelementptr inbounds %struct.Pair, 
> %struct.Pair* [[P]], i32 0, i32 1
> -// CHECK: store void (%struct.Foo*)** [[SECOND]], [[TYPE]]*** [[D]]
> -// CHECK: [[R0:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[D]]
> -// CHECK: [[R1:%.*]] = load [[TYPE]]*, [[TYPE]]** [[R0]]
> -// CHECK: [[R2:%.*]] = load %struct.Foo*, %struct.Foo** [[F]]
> -// CHECK: call void [[R1]](%struct.Foo* dereferenceable({{[0-9]+}}) [[R2]])
> -// CHECK: call void @{{.*Pair.*Foo.*}}(%struct.Pair* [[P]])
> -
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:2533
+ CodeGenFunction *CGF) {
+  if (!LangOpts.OpenMP || LangOpts.OpenMPSimd ||
+  (!LangOpts.EmitAllDecls && !D->isUsed()))

Why do we need to emit it for simd only mode?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1695
+  if (FunctionUDMMap.count(CGF.CurFn) > 0) {
+for(auto *D : FunctionUDMMap[CGF.CurFn])
+  UDMMap.erase(D);

You're looking for `CGF.CurFn` twice here, used `find` member function instead 
and work with iterator.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7116-7124
+  /// Get the offset of the OMP_MAP_MEMBER_OF field.
+  static unsigned getFlagMemberOffset() {
+unsigned Offset = 0;
+for (uint64_t Remain = OMP_MAP_MEMBER_OF; !(Remain & 1);
+ Remain = Remain >> 1)
+  Offset++;
+return Offset;

Maybe it is better to define a constant `constexpr uint64_t OMP_MEMBER_OF_RANK 
= 48` and then deduce `OMP_MAP_MEMBER_OF` as `~((1< to find member CurDir.
+assert(this->CurDir.is() &&

AFAIK, LLVM has dropped support for msvc 2013, do we still need this?



Comment at: lib/CodeGen/CGOpenMPRuntime.h:352
+  /// Map of functions and their local user-defined mappers.
+  typedef llvm::DenseMap>

Use `using` instead of `typedef`.


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

https://reviews.llvm.org/D59474



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

[PATCH] D65102: [OpenCL] Rename lang mode flag for C++ mode

[Kévin Petit via Phabricator via cfe-commits]

kpet added a comment.

Hmm, maybe we need to make sure that one of the tests is using a C++ feature 
and building with `CLC++`. This would have caught the mistake.


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

https://reviews.llvm.org/D65102



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

[PATCH] D64487: [clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

[Rainer Orth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2da6eea07cde: [clang, test] Fix Clang :: Headers/max_align.c 
on 64-bit SPARC (authored by ro).

Changed prior to commit:
  https://reviews.llvm.org/D64487?vs=208946=211316#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64487

Files:
  clang/lib/Basic/Targets/Sparc.h
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -9599,6 +9599,7 @@
 // X86-64-DECLSPEC: #define __declspec{{.*}}
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=sparc64-none-none < /dev/null 
| FileCheck -match-full-lines -check-prefix SPARCV9 %s
+// SPARCV9:#define __BIGGEST_ALIGNMENT__ 16
 // SPARCV9:#define __INT64_TYPE__ long int
 // SPARCV9:#define __INTMAX_C_SUFFIX__ L
 // SPARCV9:#define __INTMAX_TYPE__ long int
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -208,6 +208,7 @@
 // aligned. The SPARCv9 SCD 2.4.1 says 16-byte aligned.
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
+SuitableAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();
 MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
   }


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -9599,6 +9599,7 @@
 // X86-64-DECLSPEC: #define __declspec{{.*}}
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=sparc64-none-none < /dev/null | FileCheck -match-full-lines -check-prefix SPARCV9 %s
+// SPARCV9:#define __BIGGEST_ALIGNMENT__ 16
 // SPARCV9:#define __INT64_TYPE__ long int
 // SPARCV9:#define __INTMAX_C_SUFFIX__ L
 // SPARCV9:#define __INTMAX_TYPE__ long int
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -208,6 +208,7 @@
 // aligned. The SPARCv9 SCD 2.4.1 says 16-byte aligned.
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
+SuitableAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();
 MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65102: [OpenCL] Rename lang mode flag for C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: include/clang/Frontend/LangStandards.def:177
 LANGSTANDARD_ALIAS_DEPR(opencl20, "CL2.0")
+LANGSTANDARD_ALIAS_DEPR(opencl20, "CLC++")
 

kpet wrote:
> Shouldn't this be `openclcpp`?
Definitely! Good spot! Thanks!


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

https://reviews.llvm.org/D65102



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

[PATCH] D65102: [OpenCL] Rename lang mode flag for C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 211315.
Anastasia added a comment.

- Fixed `LANGSTANDARD_ALIAS_DEPR`


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

https://reviews.llvm.org/D65102

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/mangle-address-space.cpp
  test/CodeGenOpenCL/builtins.cl
  test/CodeGenOpenCL/images.cl
  test/CodeGenOpenCL/logical-ops.cl
  test/CodeGenOpenCL/pipe_builtin.cl
  test/CodeGenOpenCL/sampler.cl
  test/CodeGenOpenCL/spir_version.cl
  test/CodeGenOpenCL/to_addr_builtin.cl
  test/CodeGenOpenCLCXX/address-space-castoperators.cpp
  test/CodeGenOpenCLCXX/address-space-deduction.cl
  test/CodeGenOpenCLCXX/address-space-deduction2.cl
  test/CodeGenOpenCLCXX/addrspace-conversion.cl
  test/CodeGenOpenCLCXX/addrspace-derived-base.cl
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/addrspace-operators.cl
  test/CodeGenOpenCLCXX/addrspace-references.cl
  test/CodeGenOpenCLCXX/addrspace-with-class.cl
  test/CodeGenOpenCLCXX/atexit.cl
  test/CodeGenOpenCLCXX/global_init.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/CodeGenOpenCLCXX/template-address-spaces.cl
  test/Driver/autocomplete.c
  test/Driver/opencl.cl
  test/Frontend/opencl.cl
  test/Frontend/stdlang.c
  test/Headers/opencl-c-header.cl
  test/Parser/opencl-cxx-keywords.cl
  test/Parser/opencl-cxx-virtual.cl
  test/Preprocessor/predefined-macros.c
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl
  test/SemaOpenCL/builtin.cl
  test/SemaOpenCL/clk_event_t.cl
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/extensions.cl
  test/SemaOpenCL/invalid-image.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl
  test/SemaOpenCLCXX/address-space-deduction.cl
  test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
  test/SemaOpenCLCXX/address-space-of-this.cl
  test/SemaOpenCLCXX/address-space-references.cl
  test/SemaOpenCLCXX/address-space-templates.cl
  test/SemaOpenCLCXX/address_space_overloading.cl
  test/SemaOpenCLCXX/kernel_invalid.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/newdelete.cl
  test/SemaOpenCLCXX/restricted.cl

Index: test/SemaOpenCLCXX/restricted.cl
===
--- test/SemaOpenCLCXX/restricted.cl
+++ test/SemaOpenCLCXX/restricted.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 // This test checks that various C/C++/OpenCL C constructs are not available in
 // C++ for OpenCL.
Index: test/SemaOpenCLCXX/newdelete.cl
===
--- test/SemaOpenCLCXX/newdelete.cl
+++ test/SemaOpenCLCXX/newdelete.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 class A {
   public:
Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- test/SemaOpenCLCXX/method-overload-address-space.cl
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -1,4 +1,4 @@
-//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify
 
 struct C {
   void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
Index: test/SemaOpenCLCXX/kernel_invalid.cl
===
--- test/SemaOpenCLCXX/kernel_invalid.cl
+++ test/SemaOpenCLCXX/kernel_invalid.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 struct C {
   kernel void m(); //expected-error{{kernel functions cannot be class members}}
Index: test/SemaOpenCLCXX/address_space_overloading.cl
===
--- test/SemaOpenCLCXX/address_space_overloading.cl
+++ test/SemaOpenCLCXX/address_space_overloading.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++
 // expected-no-diagnostics
 
 struct RetGlob {
Index: test/SemaOpenCLCXX/address-space-templates.cl
===
--- test/SemaOpenCLCXX/address-space-templates.cl
+++ test/SemaOpenCLCXX/address-space-templates.cl
@@ -1,4 +1,4 @@
-//RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 

r366820 - [clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

[Rainer Orth via cfe-commits]

Author: ro
Date: Tue Jul 23 09:24:00 2019
New Revision: 366820

URL: http://llvm.org/viewvc/llvm-project?rev=366820=rev
Log:
[clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

Clang :: Headers/max_align.c currently FAILs on 64-bit SPARC:

  error: 'error' diagnostics seen but not expected: 
File /vol/llvm/src/clang/dist/test/Headers/max_align.c Line 12: 
static_assert failed due to requirement '8 == _Alignof(max_align_t)' ""
  1 error generated.

This happens because SuitableAlign isn't defined for SPARCv9 unlike SPARCv8
(which uses the default of 64 bits).  gcc's sparc/sparc.h has

  #define BIGGEST_ALIGNMENT (TARGET_ARCH64 ? 128 : 64)

This patch sets SuitableAlign to match and updates the corresponding testcase.

Tested on sparcv9-sun-solaris2.11.

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

Modified:
cfe/trunk/lib/Basic/Targets/Sparc.h
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Basic/Targets/Sparc.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Sparc.h?rev=366820=366819=366820=diff
==
--- cfe/trunk/lib/Basic/Targets/Sparc.h (original)
+++ cfe/trunk/lib/Basic/Targets/Sparc.h Tue Jul 23 09:24:00 2019
@@ -208,6 +208,7 @@ public:
 // aligned. The SPARCv9 SCD 2.4.1 says 16-byte aligned.
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
+SuitableAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();
 MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
   }

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=366820=366819=366820=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Tue Jul 23 09:24:00 2019
@@ -9599,6 +9599,7 @@
 // X86-64-DECLSPEC: #define __declspec{{.*}}
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=sparc64-none-none < /dev/null 
| FileCheck -match-full-lines -check-prefix SPARCV9 %s
+// SPARCV9:#define __BIGGEST_ALIGNMENT__ 16
 // SPARCV9:#define __INT64_TYPE__ long int
 // SPARCV9:#define __INTMAX_C_SUFFIX__ L
 // SPARCV9:#define __INTMAX_TYPE__ long int


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

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[JF Bastien via Phabricator via cfe-commits]

jfb requested changes to this revision.
jfb added a comment.
Herald added a subscriber: dexonsmith.

In D64666#1597193 , @aaron.ballman 
wrote:

> In D64666#1596660 , @xbolva00 wrote:
>
> > I think we should warn in that case even if GCC does not warn.
>
>
> Strong +1.


Sorry if the phrasing was misleading: if we know for a fact that there's a 
problem, we should warn unconditionally. If we don't know for a fact then the 
warning should *not* be enabled by `-Wall` nor `-Wextra`. I don't really care 
what GCC does by default, LLVM doesn't have to match every single thing. That 
being said, if LLVM behaves differently then maybe the flag name should be 
different.

> In D64666#1596853 , @ziangwan wrote:
> 
>> Final review ping.
> 
> 
> Please be sure to give reviewers enough time to respond to comments before 
> pinging a review.

Indeed. You haven't answered my first comment, I'd expect you to do so and not 
"final ping" anything. I'm not saying you must do what I say, just that you 
must answer comments, not ignore them.


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

https://reviews.llvm.org/D64666



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5244
+}
+
+

a_sidorin wrote:
> A redundant newline?
Yes, I deleted that. And moved the test case closer the the last `ImportDecl` 
test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44100



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 211314.
martong marked 5 inline comments as done.
martong added a comment.

- Address Alexei's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44100

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1472,7 +1472,7 @@
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase,
-   DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
+   CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   // The original recursive algorithm of ASTImporter first imports 'c' then
@@ -2795,6 +2795,16 @@
   "main.c", enumDecl(), VerificationMatcher);
 }
 
+TEST_P(ImportDecl, ImportFieldOrder) {
+  MatchVerifier Verifier;
+  testImport("struct declToImport {"
+ "  int b = a + 2;"
+ "  int a = 5;"
+ "};",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ recordDecl(hasFieldOrder({"b", "a"})));
+}
+
 const internal::VariadicDynCastAllOfMatcher
 dependentScopeDeclRefExpr;
 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1645,7 +1645,6 @@
   bool AccumulateChildErrors = isa(FromDC);
 
   Error ChildErrors = Error::success();
-  llvm::SmallVector ImportedDecls;
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
 if (!ImportedOrErr) {
@@ -1657,6 +1656,59 @@
 }
   }
 
+  // We reorder declarations in RecordDecls because they may have another order
+  // in the "to" context than they have in the "from" context. This may happen
+  // e.g when we import a class like this:
+  //struct declToImport {
+  //int a = c + b;
+  //int b = 1;
+  //int c = 2;
+  //};
+  // During the import of `a` we import first the dependencies in sequence,
+  // thus the order would be `c`, `b`, `a`. We will get the normal order by
+  // first removing the already imported members and then adding them in the
+  // order as they apper in the "from" context.
+  //
+  // Keeping field order is vital because it determines structure layout.
+  //
+  // Here and below, we cannot call field_begin() method and its callers on
+  // ToDC if it has an external storage. Calling field_begin() will
+  // automatically load all the fields by calling
+  // LoadFieldsFromExternalStorage(). LoadFieldsFromExternalStorage() would
+  // call ASTImporter::Import(). This is because the ExternalASTSource
+  // interface in LLDB is implemented by the means of the ASTImporter. However,
+  // calling an import at this point would result in an uncontrolled import, we
+  // must avoid that.
+  const auto *FromRD = dyn_cast(FromDC);
+  if (!FromRD)
+return ChildErrors;
+
+  auto ToDCOrErr = Importer.ImportContext(FromDC);
+  if (!ToDCOrErr) {
+consumeError(std::move(ChildErrors));
+return ToDCOrErr.takeError();
+  }
+
+  DeclContext *ToDC = *ToDCOrErr;
+  // Remove all declarations, which may be in wrong order in the
+  // lexical DeclContext and then add them in the proper order.
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  assert(D && "DC contains a null decl");
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  // Remove only the decls which we successfully imported.
+  if (ToD) {
+assert(ToDC == ToD->getLexicalDeclContext() && ToDC->containsDecl(ToD));
+// Remove the decl from its wrong place in the linked list.
+ToDC->removeDecl(ToD);
+// Add the decl to the end of the linked list.
+// This time it will be at the proper place because the enclosing for
+// loop iterates in the original (good) order of the decls.
+ToDC->addDeclInternal(ToD);
+  }
+}
+  }
+
   return ChildErrors;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64914: Implement P1771

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D64914#1595660 , @erichkeane wrote:

> Rebased and did all the comments (including the www_status).  @aaron.ballman 
> : Wasn't positive what you meant about the conversion functions, but I think 
> I got one?


I was talking about [dcl.attr.nodiscard]p2.2: "an explicit type conversion 
(7.6.1.8 [expr.static.cast], 7.6.3 [expr.cast], 7.6.1.3 [expr.type.conv]) that 
constructs an object through a constructor declared nodiscard, or that 
initializes an object of a nodiscard type." and specifically the part about 
type conversion operators that are marked `nodiscard` directly.




Comment at: clang/include/clang/Basic/AttrDocs.td:1518
+marked_ctor(); // diagnoses.
+marked_ctor(3); // Does not diagnose, int constructor isn't marked 
nodiscard.
+  }

I think we may want to add a case like:
```
struct S {
  operator marked_type() const;
  [[nodiscard]] operator int() const;
};

void func() {
  S s;
  static_cast(s); // diagnoses
  (int)s; // diagnoses
}
```



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2835
+  D->getFunctionType()->getReturnType()->isVoidType() &&
+  !isa(D)) {
 S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0;

Conversion functions?



Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:100
+S s;
+(ConvertTo)s; // expected-warning {{expression result unused}}
+  }

If you specify a message on the `nodiscard` attribute used here, does the 
message get printed?


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

https://reviews.llvm.org/D64914



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 211312.
martong added a comment.

- Rebase to master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1879,7 +1879,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2389,6 +2389,64 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5207,6 +5265,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
 DefaultTestValuesForRunOptions, );
 
Index: clang/test/Analysis/Inputs/ctu-other.c
===
--- clang/test/Analysis/Inputs/ctu-other.c
+++ clang/test/Analysis/Inputs/ctu-other.c
@@ -12,11 +12,11 @@
 }
 
 // Test enums.
-enum B { x = 42,
- l,
- s };
+enum B { x2 = 42,
+ y2,
+ z2 };
 int enumCheck(void) {
-  return x;
+  return x2;
 }
 
 // Test reporting an error in macro definition
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -80,6 +80,7 @@
   using ExpectedExpr = llvm::Expected;
   using ExpectedDecl = llvm::Expected;
   using ExpectedSLoc = llvm::Expected;
+  using ExpectedName = llvm::Expected;
 
   std::string ImportError::toString() const {
 // FIXME: Improve error texts.
@@ -2188,11 +2189,11 @@
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
-  if (!Name)
-return make_error(ImportError::NameConflict);
+  ExpectedName NameOrErr = Importer.HandleNameConflict(
+  Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(),
+  ConflictingDecls.size());
+  if (!NameOrErr)
+return NameOrErr.takeError();
 }
   }
 
@@ -2296,21 +2297,19 @@
   // already have a complete underlying type then return with that.
   if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
 return Importer.MapImported(D, FoundTypedef);
+  // FIXME Handle redecl chain. When you do that make consistent changes
+  // in ASTImporterLookupTable too.
+} else {
+  ConflictingDecls.push_back(FoundDecl);
 }
-// FIXME 

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366818: [ASTImporter] Fix inequivalence of 
ClassTemplateInstantiations (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64241?vs=210581=211311#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64241

Files:
  cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
  cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

Index: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
===
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
@@ -235,12 +235,21 @@
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  const TemplateName ,
  const TemplateName ) {
-  if (N1.getKind() != N2.getKind())
+  TemplateDecl *TemplateDeclN1 = N1.getAsTemplateDecl();
+  TemplateDecl *TemplateDeclN2 = N2.getAsTemplateDecl();
+  if (TemplateDeclN1 && TemplateDeclN2) {
+if (!IsStructurallyEquivalent(Context, TemplateDeclN1, TemplateDeclN2))
+  return false;
+// If the kind is different we compare only the template decl.
+if (N1.getKind() != N2.getKind())
+  return true;
+  } else if (TemplateDeclN1 || TemplateDeclN2)
 return false;
+  else if (N1.getKind() != N2.getKind())
+return false;
+
+  // Check for special case incompatibilities.
   switch (N1.getKind()) {
-  case TemplateName::Template:
-return IsStructurallyEquivalent(Context, N1.getAsTemplateDecl(),
-N2.getAsTemplateDecl());
 
   case TemplateName::OverloadedTemplate: {
 OverloadedTemplateStorage *OS1 = N1.getAsOverloadedTemplate(),
@@ -259,14 +268,6 @@
 return TN1->getDeclName() == TN2->getDeclName();
   }
 
-  case TemplateName::QualifiedTemplate: {
-QualifiedTemplateName *QN1 = N1.getAsQualifiedTemplateName(),
-  *QN2 = N2.getAsQualifiedTemplateName();
-return IsStructurallyEquivalent(Context, QN1->getDecl(), QN2->getDecl()) &&
-   IsStructurallyEquivalent(Context, QN1->getQualifier(),
-QN2->getQualifier());
-  }
-
   case TemplateName::DependentTemplate: {
 DependentTemplateName *DN1 = N1.getAsDependentTemplateName(),
   *DN2 = N2.getAsDependentTemplateName();
@@ -281,15 +282,6 @@
 return false;
   }
 
-  case TemplateName::SubstTemplateTemplateParm: {
-SubstTemplateTemplateParmStorage *TS1 = N1.getAsSubstTemplateTemplateParm(),
- *TS2 = N2.getAsSubstTemplateTemplateParm();
-return IsStructurallyEquivalent(Context, TS1->getParameter(),
-TS2->getParameter()) &&
-   IsStructurallyEquivalent(Context, TS1->getReplacement(),
-TS2->getReplacement());
-  }
-
   case TemplateName::SubstTemplateTemplateParmPack: {
 SubstTemplateTemplateParmPackStorage
 *P1 = N1.getAsSubstTemplateTemplateParmPack(),
@@ -299,8 +291,16 @@
IsStructurallyEquivalent(Context, P1->getParameterPack(),
 P2->getParameterPack());
   }
+
+   case TemplateName::Template:
+   case TemplateName::QualifiedTemplate:
+   case TemplateName::SubstTemplateTemplateParm:
+ // It is sufficient to check value of getAsTemplateDecl.
+ break;
+
   }
-  return false;
+
+  return true;
 }
 
 /// Determine whether two template arguments are equivalent.
Index: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
===
--- cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
+++ cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
@@ -944,6 +944,67 @@
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgNotEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make 

r366818 - [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via cfe-commits]

Author: martong
Date: Tue Jul 23 08:46:38 2019
New Revision: 366818

URL: http://llvm.org/viewvc/llvm-project?rev=366818=rev
Log:
[ASTImporter] Fix inequivalence of ClassTemplateInstantiations

Summary:
We falsely state inequivalence if the template parameter is a
qualified/nonquialified template in the first/second instantiation.
Also, different kinds of TemplateName should be equal if the template
decl (if available) is equal (even if the name kind is different).

Reviewers: a_sidorin, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=366818=366817=366818=diff
==
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original)
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Tue Jul 23 08:46:38 2019
@@ -235,12 +235,21 @@ static bool IsStructurallyEquivalent(Str
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  const TemplateName ,
  const TemplateName ) {
-  if (N1.getKind() != N2.getKind())
+  TemplateDecl *TemplateDeclN1 = N1.getAsTemplateDecl();
+  TemplateDecl *TemplateDeclN2 = N2.getAsTemplateDecl();
+  if (TemplateDeclN1 && TemplateDeclN2) {
+if (!IsStructurallyEquivalent(Context, TemplateDeclN1, TemplateDeclN2))
+  return false;
+// If the kind is different we compare only the template decl.
+if (N1.getKind() != N2.getKind())
+  return true;
+  } else if (TemplateDeclN1 || TemplateDeclN2)
+return false;
+  else if (N1.getKind() != N2.getKind())
 return false;
+
+  // Check for special case incompatibilities.
   switch (N1.getKind()) {
-  case TemplateName::Template:
-return IsStructurallyEquivalent(Context, N1.getAsTemplateDecl(),
-N2.getAsTemplateDecl());
 
   case TemplateName::OverloadedTemplate: {
 OverloadedTemplateStorage *OS1 = N1.getAsOverloadedTemplate(),
@@ -259,14 +268,6 @@ static bool IsStructurallyEquivalent(Str
 return TN1->getDeclName() == TN2->getDeclName();
   }
 
-  case TemplateName::QualifiedTemplate: {
-QualifiedTemplateName *QN1 = N1.getAsQualifiedTemplateName(),
-  *QN2 = N2.getAsQualifiedTemplateName();
-return IsStructurallyEquivalent(Context, QN1->getDecl(), QN2->getDecl()) &&
-   IsStructurallyEquivalent(Context, QN1->getQualifier(),
-QN2->getQualifier());
-  }
-
   case TemplateName::DependentTemplate: {
 DependentTemplateName *DN1 = N1.getAsDependentTemplateName(),
   *DN2 = N2.getAsDependentTemplateName();
@@ -281,15 +282,6 @@ static bool IsStructurallyEquivalent(Str
 return false;
   }
 
-  case TemplateName::SubstTemplateTemplateParm: {
-SubstTemplateTemplateParmStorage *TS1 = 
N1.getAsSubstTemplateTemplateParm(),
- *TS2 = 
N2.getAsSubstTemplateTemplateParm();
-return IsStructurallyEquivalent(Context, TS1->getParameter(),
-TS2->getParameter()) &&
-   IsStructurallyEquivalent(Context, TS1->getReplacement(),
-TS2->getReplacement());
-  }
-
   case TemplateName::SubstTemplateTemplateParmPack: {
 SubstTemplateTemplateParmPackStorage
 *P1 = N1.getAsSubstTemplateTemplateParmPack(),
@@ -299,8 +291,16 @@ static bool IsStructurallyEquivalent(Str
IsStructurallyEquivalent(Context, P1->getParameterPack(),
 P2->getParameterPack());
   }
+
+   case TemplateName::Template:
+   case TemplateName::QualifiedTemplate:
+   case TemplateName::SubstTemplateTemplateParm:
+ // It is sufficient to check value of getAsTemplateDecl.
+ break;
+
   }
-  return false;
+
+  return true;
 }
 
 /// Determine whether two template arguments are equivalent.

Modified: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp?rev=366818=366817=366818=diff
==
--- cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp (original)
+++ cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp Tue Jul 23 08:46:38 
2019
@@ -944,6 +944,67 @@ TEST_F(StructuralEquivalenceTemplateTest
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make 

[PATCH] D65154: [clangd] Reformat use of cl::opt: use unqualified name and don't bin-pack attributes. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65154

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -36,128 +36,161 @@
 namespace clang {
 namespace clangd {
 
-static llvm::cl::opt CompileCommandsDir(
+using llvm::cl::cat;
+using llvm::cl::CommaSeparated;
+using llvm::cl::desc;
+using llvm::cl::Hidden;
+using llvm::cl::init;
+using llvm::cl::list;
+using llvm::cl::opt;
+using llvm::cl::values;
+
+static opt CompileCommandsDir{
 "compile-commands-dir",
-llvm::cl::desc("Specify a path to look for compile_commands.json. If path "
-   "is invalid, clangd will look in the current directory and "
-   "parent paths of each source file"));
+desc("Specify a path to look for compile_commands.json. If path "
+ "is invalid, clangd will look in the current directory and "
+ "parent paths of each source file"),
+};
 
-static llvm::cl::opt
-WorkerThreadsCount("j",
-   llvm::cl::desc("Number of async workers used by clangd"),
-   llvm::cl::init(getDefaultAsyncThreadsCount()));
+static opt WorkerThreadsCount{
+"j",
+desc("Number of async workers used by clangd"),
+init(getDefaultAsyncThreadsCount()),
+};
 
 // FIXME: also support "plain" style where signatures are always omitted.
 enum CompletionStyleFlag { Detailed, Bundled };
-static llvm::cl::opt CompletionStyle(
+static opt CompletionStyle{
 "completion-style",
-llvm::cl::desc("Granularity of code completion suggestions"),
-llvm::cl::values(
-clEnumValN(Detailed, "detailed",
-   "One completion item for each semantically distinct "
-   "completion, with full type information"),
-clEnumValN(Bundled, "bundled",
-   "Similar completion items (e.g. function overloads) are "
-   "combined. Type information shown where possible")));
+desc("Granularity of code completion suggestions"),
+values(clEnumValN(Detailed, "detailed",
+  "One completion item for each semantically distinct "
+  "completion, with full type information"),
+   clEnumValN(Bundled, "bundled",
+  "Similar completion items (e.g. function overloads) are "
+  "combined. Type information shown where possible")),
+};
 
 // FIXME: Flags are the wrong mechanism for user preferences.
 // We should probably read a dotfile or similar.
-static llvm::cl::opt IncludeIneligibleResults(
+static opt IncludeIneligibleResults{
 "include-ineligible-results",
-llvm::cl::desc(
-"Include ineligible completion results (e.g. private members)"),
-llvm::cl::init(CodeCompleteOptions().IncludeIneligibleResults),
-llvm::cl::Hidden);
-
-static llvm::cl::opt InputStyle(
-"input-style", llvm::cl::desc("Input JSON stream encoding"),
-llvm::cl::values(
+desc("Include ineligible completion results (e.g. private members)"),
+init(CodeCompleteOptions().IncludeIneligibleResults),
+Hidden,
+};
+
+static opt InputStyle{
+"input-style",
+desc("Input JSON stream encoding"),
+values(
 clEnumValN(JSONStreamStyle::Standard, "standard", "usual LSP protocol"),
 clEnumValN(JSONStreamStyle::Delimited, "delimited",
"messages delimited by --- lines, with # comment support")),
-llvm::cl::init(JSONStreamStyle::Standard), llvm::cl::Hidden);
-
-static llvm::cl::opt
-PrettyPrint("pretty", llvm::cl::desc("Pretty-print JSON output"),
-llvm::cl::init(false));
-
-static llvm::cl::opt LogLevel(
-"log", llvm::cl::desc("Verbosity of log messages written to stderr"),
-llvm::cl::values(clEnumValN(Logger::Error, "error", "Error messages only"),
- clEnumValN(Logger::Info, "info",
-"High level execution tracing"),
- clEnumValN(Logger::Debug, "verbose", "Low level details")),
-llvm::cl::init(Logger::Info));
-
-static llvm::cl::opt
-Test("lit-test",
- llvm::cl::desc("Abbreviation for -input-style=delimited -pretty -sync "
-"-enable-test-scheme -log=verbose."
-"Intended to simplify lit tests"),
- llvm::cl::init(false), llvm::cl::Hidden);
-
-static llvm::cl::opt EnableTestScheme(
+init(JSONStreamStyle::Standard),
+Hidden,
+};
+
+static opt PrettyPrint{
+"pretty",
+desc("Pretty-print JSON output"),
+init(false),
+};
+
+static opt LogLevel{
+"log",
+desc("Verbosity 

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Is this really necessary? Users don't typically pass -std= to the driver for 
linking anyways (what do you even pass if you've compiled both C and C++ code?) 
so this seems a rather odd way to control behavior.

How about instead just adding "values-xpg6.o" unconditionally, alongside the 
current unconditional "values-Xa.o", and just forget about the xpg4 and Xc 
modes?




Comment at: lib/Driver/ToolChains/Solaris.cpp:16
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/LangStandard.h"
 #include "llvm/Option/ArgList.h"

I'm not sure that's an acceptable dependency -- I think Driver probably is not 
supposed to depend on Frontend. If so, I guess LangStandard should move to 
clang/Basic/. Which also means moving InputKind from 
clang/include/clang/Frontend/FrontendOptions.h.

(Maybe someone else can weigh in on this question?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64793



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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst:9
+
+This can pose problems in certain multithreaded contexts.

Eugene.Zelenko wrote:
> Will be good idea to provide example.
Agreed, I'd like to see an example in the docs.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:33-35
+  // This may work fine when optimization is enabled because bar() can
+  // be turned into a constant 7.  But without optimization, it can
+  // cause problems. Therefore, we must err on the side of conservatism.

What problems can be caused here? Typically, dynamic init is only problematic 
when it happens before main() is executed (because of initialization order 
dependencies), but that doesn't apply to local statics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62829



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

[PATCH] D63954: Add lifetime categories attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4167
+
+The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an
+object of type ``T``:

Do either of these attributes make sense on a union type? If so, might be worth 
mentioning unions here and below. If not, would it be worth diagnosing on a 
union?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2516
   "%0 and %1 attributes are not compatible">;
+def err_attribute_invalid_argument : Error<
+  "%0 is an invalid argument to attribute %1">;

Can you combine this one with `err_attribute_argument_vec_type_hint`?



Comment at: clang/lib/Parse/ParseDecl.cpp:385
+  TheParsedType = T.get();
+break; // Multiple type arguments are not implemented.
+  } else if (Tok.is(tok::identifier) &&

Can you add a FIXME prefix to the comment?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4550
+  S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument)
+  << "A reference type" << AL;
+  return;

This should say `a reference type`, but I prefer seeing this done in a 
`%select{}` within the diagnostic rather than manually adding string literals. 
This will also simplify the logic down to:
```
unsigned SelectIdx = ~0U;
if (ParamType->isVoidType())
  SelectIdx = 0;
else if (ParamType->isReferenceType())
  SelectIdx = 1;
else if (ParamType->isArrayType())
  SelectIdx = 2;

if (SelectIdx != ~0U) {
  S.Diag(...) << SelectIdx << AL;
  return;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954



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

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup>, DefaultIgnore;

george.burgess.iv wrote:
> aaron.ballman wrote:
> > george.burgess.iv wrote:
> > > aaron.ballman wrote:
> > > > george.burgess.iv wrote:
> > > > > nit: I'd just say "use of function '%0'" here; "builtin" doesn't 
> > > > > really add much.
> > > > > 
> > > > > I also wonder if we should be saying anything more than "we found a 
> > > > > use of this function." Looks like GCC doesn't 
> > > > > (https://godbolt.org/z/sYs_8G), but since this warning is sort of 
> > > > > opinionated in itself, might it be better to expand this to "use of 
> > > > > '%0' is discouraged"?
> > > > > 
> > > > > WDYT, Aaron?
> > > > What is the purpose to this diagnostic, aside from GCC compatibility? 
> > > > What does it protect against?
> > > > 
> > > > If there's a reason users should not use alloc(), it would be better 
> > > > for the diagnostic to spell it out.
> > > > 
> > > > Btw, I'm okay with this being default-off because the GCC warning is as 
> > > > well. I'm mostly hoping we can do better with our diagnostic wording.
> > > > I'm mostly hoping we can do better with our diagnostic wording
> > > 
> > > +1
> > > 
> > > > What is the purpose to this diagnostic?
> > > 
> > > I think the intent boils down to that `alloca` is easily misused, and 
> > > leads to e.g., 
> > > https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt . Since its 
> > > use often boils down to nothing but a tiny micro-optimization, some 
> > > parties would like to discourage its use.
> > > 
> > > Both glibc and bionic recommend against the use of `alloca` in their 
> > > documentation, though glibc's docs are less assertive than bionic's, and 
> > > explicitly call out "[alloca's] use can improve efficiency compared to 
> > > the use of malloc plus free."
> > > 
> > > Greping a codebase and investigating the first 15 results:
> > > - all of them look like microoptimizations; many of them also sit close 
> > > to other `malloc`/`new` ops, so allocating on these paths presumably 
> > > isn't prohibitively expensive
> > > - all but two of the uses of `alloca` have no logic to fall back to the 
> > > heap `malloc` if the array they want to allocate passes a certain 
> > > threshold. Some of the uses make it look *really* easy for the array to 
> > > grow very large.
> > > - one of the uses compares the result of `alloca` to `NULL`. Since 
> > > `alloca`'s behavior is undefined if it fails, ...
> > > 
> > > I'm having trouble putting this into a concise and actionable diagnostic 
> > > message, though. The best I can come up with at the moment is something 
> > > along the lines of "use of function %0 is subtle; consider using heap 
> > > allocation instead."
> > Okay, that's along the lines of what I was thinking.
> > 
> > Part of me thinks that this should not diagnose calls to `alloca()` that 
> > are given a constant value that we can test for sanity at compile time. 
> > e.g., calling `alloca(10)` is highly unlikely to be a problem, but calling 
> > `alloca(100)` certainly could be, while `alloca(x)` is a different 
> > class of problem without good static analysis.
> > 
> > That said, perhaps we could get away with `use of function %0 is 
> > discouraged; there is no way to check for failure but failure may still 
> > occur, resulting in a possibly exploitable security vulnerability` or 
> > something along those lines?
> Yeah, GCC has a similar `-Walloca-larger-than=N` that does roughly what you 
> described. The icky part is exactly what you said. GCC will warn about 
> unknown values, but considers control flow when doing so: 
> https://godbolt.org/z/J3pGiT
> 
> It looks like it's the same "we apply optimizations and then see what 
> happens" behavior as similar diagnostics: https://godbolt.org/z/lLyteP
> 
> WRT the diag we emit here, maybe we could use notes to break it up and imply 
> things? e.g.
> 
> warning: use of function %0 is discouraged, due to its security implications
> note: 'malloc' or 'new' are suggested alternatives, since they have 
> well-defined behavior on failure
> 
> ...not sold on the idea, but it's a thought.
> 
> If we don't want to break it to pieces, I'm fine with your suggestion
I'm not certain the note adds value because it will always be printed on the 
same line as the warning. A note would make sense if we had a secondary 
location to annotate though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64883



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

[PATCH] D65153: [clangd] Also accept flags from CLANGD_FLAGS variable.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This simplifies various workflows, particularly in debugging/development.
e.g. editors will tend to propagate flags, so you can run
`env CLANGD_FLAGS=-input-mirror-file=/tmp/mirror vim foo.cc` rather than change
the configuration in a persistent way.
(This also gives us a generic lever when we don't know how to customize the
flags in some particular LSP client).

While here, add a test for this and other startup logging, and fix a couple
of direct writes to errs() that should have been logs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65153

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/log.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
@@ -342,14 +343,18 @@
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
   });
-  llvm::cl::ParseCommandLineOptions(
-  argc, argv,
-  "clangd is a language server that provides IDE-like features to editors. 
"
-  "\n\nIt should be used via an editor plugin rather than invoked "
-  "directly. "
-  "For more information, see:"
-  "\n\thttps://clang.llvm.org/extra/clangd.html;
-  "\n\thttps://microsoft.github.io/language-server-protocol/;);
+  const char *FlagsEnvVar = "CLANGD_FLAGS";
+  const char *Overview =
+  R"(clangd is a language server that provides IDE-like features to 
editors.
+
+It should be used via an editor plugin rather than invoked directly. For more 
information, see:
+   https://clang.llvm.org/extra/clangd.html
+   https://microsoft.github.io/language-server-protocol/
+
+clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment 
variable.
+)";
+  llvm::cl::ParseCommandLineOptions(argc, argv, Overview,
+/*Errs=*/nullptr, FlagsEnvVar);
   if (Test) {
 Sync = true;
 InputStyle = JSONStreamStyle::Delimited;
@@ -434,6 +439,8 @@
   }
   for (int I = 0; I < argc; ++I)
 log("argv[{0}]: {1}", I, argv[I]);
+  if (auto EnvFlags = llvm::sys::Process::GetEnv(FlagsEnvVar))
+log("{0}: {1}", FlagsEnvVar, *EnvFlags);
 
   // If --compile-commands-dir arg was invoked, check value and override 
default
   // path.
Index: clang-tools-extra/clangd/test/log.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/log.test
@@ -0,0 +1,9 @@
+# RUN: not env CLANGD_FLAGS=-index-file=no-such-index clangd -lit-test 
&1 >/dev/null | FileCheck %s
+CHECK: I[{{.*}}] clangd version {{.*}}
+CHECK: Working directory: {{.*}}
+CHECK: argv[0]: clangd
+CHECK: argv[1]: -lit-test
+CHECK: CLANGD_FLAGS: -index-file=no-such-index
+CHECK: E[{{.*}}] Can't open no-such-index
+CHECK: Starting LSP over stdin/stdout
+
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -690,7 +690,7 @@
   trace::Span OverallTracer("LoadIndex");
   auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
   if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
+elog("Can't open {0}", SymbolFilename);
 return nullptr;
   }
 
@@ -707,7 +707,7 @@
   if (I->Relations)
 Relations = std::move(*I->Relations);
 } else {
-  llvm::errs() << "Bad Index: " << llvm::toString(I.takeError()) << "\n";
+  elog("Bad Index: {0}", I.takeError());
   return nullptr;
 }
   }


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
@@ -342,14 +343,18 @@
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
   });
-  llvm::cl::ParseCommandLineOptions(
-  argc, argv,
-  "clangd is a language server that provides IDE-like features to 

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Maybe CrossTU should not use the Frontend library? (Now the StaticAnalyzerCore 
is using CrossTU and CrossTU is using Frontend, in this way StaticAnalyzerCore 
could use Frontend too.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64638



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

Re: r366694 - [NFC] Relaxed regression tests for PR42665

[Hans Wennborg via cfe-commits]

Merged them both in r366814.

Thanks,
Hans

On Tue, Jul 23, 2019 at 7:20 AM Anastasia Stulova
 wrote:
>
>
> + cfe-commits
>
> 
> From: Anastasia Stulova
> Sent: 23 July 2019 15:16
> To: Hans Wennborg 
> Cc: Marco Antognini 
> Subject: Re: r366694 - [NFC] Relaxed regression tests for PR42665
>
>
> @Hans, would it be possible to merge this commit along with r366670 to the 
> release 9.0?
>
>
> FY, I have also added this to the blocker bug: 
> https://bugs.llvm.org/show_bug.cgi?id=41727
>
> Thanks!
> Anastasia
> 
> From: cfe-commits  on behalf of Marco 
> Antognini via cfe-commits 
> Sent: 22 July 2019 15:47
> To: cfe-commits@lists.llvm.org 
> Subject: r366694 - [NFC] Relaxed regression tests for PR42665
>
> Author: mantognini
> Date: Mon Jul 22 07:47:36 2019
> New Revision: 366694
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366694=rev
> Log:
> [NFC] Relaxed regression tests for PR42665
>
> Following up on the buildbot failures, this commits relaxes some tests:
> instead of checking for specific IR output, it now ensures that the
> underlying issue (the crash), and only that, doesn't happen.
>
> Modified:
> cfe/trunk/test/CodeGenCXX/PR42665.cpp
>
> Modified: cfe/trunk/test/CodeGenCXX/PR42665.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR42665.cpp?rev=366694=366693=366694=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/PR42665.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/PR42665.cpp Mon Jul 22 07:47:36 2019
> @@ -1,7 +1,8 @@
> -// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -std=c++17 -O0 %s 
> -o - | FileCheck %s
> -// RUN: %clang_cc1 -triple %ms_abi_triple -emit-llvm -std=c++17 -O0 %s -o - 
> | FileCheck %s
> +// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
> %itanium_abi_triple
> +// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
> %ms_abi_triple
>
>  // Minimal reproducer for PR42665.
> +// expected-no-diagnostics
>
>  struct Foo {
>Foo() = default;
> @@ -31,31 +32,3 @@ void foobar() {
>d(f); // Invoke virtual destructor of Foo through d.
>  } // p's destructor is invoked.
>
> -// Regexes are used to handle both kind of mangling.
> -//
> -// CHECK-LABEL: define linkonce_odr{{( dso_local)?}} void 
> @{{.*deleter.*Foo.*}}(%struct.Foo* dereferenceable({{[0-9]+}})
> -// CHECK-SAME: [[T:%.*]])
> -// CHECK: [[T_ADDR:%.*]] = alloca %struct.Foo*
> -// CHECK: store %struct.Foo* [[T]], %struct.Foo** [[T_ADDR]]
> -// CHECK: [[R0:%.*]] = load %struct.Foo*, %struct.Foo** [[T_ADDR]]
> -// CHECK: [[R1:%.*]] = bitcast %struct.Foo* [[R0]] to 
> [[TYPE:.*struct\.Foo.*]]***
> -// CHECK: [[VTABLE:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[R1]]
> -// CHECK: [[VFUN:%.*]] = getelementptr inbounds [[TYPE]]*, [[TYPE]]** 
> [[VTABLE]], i64 0
> -// CHECK: [[DTOR:%.*]] = load [[TYPE]]*, [[TYPE]]** [[VFUN]]
> -// CHECK: call {{(void|i8\*)}} [[DTOR]](%struct.Foo* [[R0]]
> -//
> -// CHECK-LABEL: define{{( dso_local)?}} void @{{.*foobar.*}}()
> -// CHECK: [[P:%.*]] = alloca %struct.Pair
> -// CHECK: [[F:%.*]] = alloca %struct.Foo*
> -// CHECK: [[D:%.*]] = alloca [[TYPE:void \(%struct.Foo\*\)]]**
> -// CHECK: call void @{{.*make_pair.*}}(%struct.Pair* sret [[P]])
> -// CHECK: [[FIRST:%.*]] = getelementptr inbounds %struct.Pair, %struct.Pair* 
> [[P]], i32 0, i32 0
> -// CHECK: store %struct.Foo* [[FIRST]], %struct.Foo** [[F]]
> -// CHECK: [[SECOND:%.*]] = getelementptr inbounds %struct.Pair, 
> %struct.Pair* [[P]], i32 0, i32 1
> -// CHECK: store void (%struct.Foo*)** [[SECOND]], [[TYPE]]*** [[D]]
> -// CHECK: [[R0:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[D]]
> -// CHECK: [[R1:%.*]] = load [[TYPE]]*, [[TYPE]]** [[R0]]
> -// CHECK: [[R2:%.*]] = load %struct.Foo*, %struct.Foo** [[F]]
> -// CHECK: call void [[R1]](%struct.Foo* dereferenceable({{[0-9]+}}) [[R2]])
> -// CHECK: call void @{{.*Pair.*Foo.*}}(%struct.Pair* [[P]])
> -
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63126: [clangd] Implement "prepareRename"

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 211302.
hokein marked 4 inline comments as done.
hokein added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63126

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/test/rename.test

Index: clang-tools-extra/clangd/test/rename.test
===
--- clang-tools-extra/clangd/test/rename.test
+++ clang-tools-extra/clangd/test/rename.test
@@ -1,12 +1,45 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"rename": {"dynamicRegistration": true, "prepareSupport": true}}},"trace":"off"}}
+# CHECK:  "renameProvider": {
+# CHECK-NEXT:"prepareProvider": true
+# CHECK-NEXT: },
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
 ---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5}}}
 #  CHECK:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "there is no symbol at the given location"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+---
+{"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "there is no symbol at the given location"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 4,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+---
+{"jsonrpc":"2.0","id":3,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+#  CHECK:  "id": 3,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
 # CHECK-NEXT:"changes": {
 # CHECK-NEXT:  "file://{{.*}}/foo.cpp": [
 # CHECK-NEXT:{
@@ -26,14 +59,6 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
-#  CHECK:  "error": {
-# CHECK-NEXT:"code": -32001,
-# CHECK-NEXT:"message": "there is no symbol at the given location"
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0"

-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html;,
 "engines": {
-"vscode": "^1.30.0"
+"vscode": "^1.36.0"
 },
 "categories": [
 "Programming Languages",
@@ -35,8 +35,8 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^5.2.0",
-"vscode-languageserver": "^5.2.0"
+"vscode-languageclient": "^5.3.0-next.6",
+"vscode-languageserver": "^5.3.0-next.6"
 },
 "devDependencies": {
 "@types/mocha": "^2.2.32",
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -422,6 +422,10 @@
   /// The content format that should be used 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:27
 
+static cl::opt TimeTraceGranularity(
+"time-trace-granularity",

I know this is late, but... this shows up in the help for any tool that links 
in libSupport, many of which don't support the time profiler. Can you mark this 
as hidden or (preferably) move this to cc1_main?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60663



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

[PATCH] D65146: [clangd] Log version, cwd, args, and transport on startup. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366811: [clangd] Log version, cwd, args, and transport on 
startup. NFC (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65146?vs=211295=211301#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65146

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -423,6 +423,17 @@
   llvm::errs().SetBuffered();
   StreamLogger Logger(llvm::errs(), LogLevel);
   LoggingSession LoggingSession(Logger);
+  // Write some initial logs before we start doing any real work.
+  log("{0}", clang::getClangToolFullVersion("clangd"));
+  {
+SmallString<128> CWD;
+if (auto Err = llvm::sys::fs::current_path(CWD))
+  log("Working directory unknown: {0}", Err.message());
+else
+  log("Working directory: {0}", CWD);
+  }
+  for (int I = 0; I < argc; ++I)
+log("argv[{0}]: {1}", I, argv[I]);
 
   // If --compile-commands-dir arg was invoked, check value and override 
default
   // path.
@@ -501,12 +512,14 @@
   std::unique_ptr TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {
 #if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
 return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
+log("Starting LSP over stdin/stdout");
 TransportLayer = newJSONTransport(
 stdin, llvm::outs(),
 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -423,6 +423,17 @@
   llvm::errs().SetBuffered();
   StreamLogger Logger(llvm::errs(), LogLevel);
   LoggingSession LoggingSession(Logger);
+  // Write some initial logs before we start doing any real work.
+  log("{0}", clang::getClangToolFullVersion("clangd"));
+  {
+SmallString<128> CWD;
+if (auto Err = llvm::sys::fs::current_path(CWD))
+  log("Working directory unknown: {0}", Err.message());
+else
+  log("Working directory: {0}", CWD);
+  }
+  for (int I = 0; I < argc; ++I)
+log("argv[{0}]: {1}", I, argv[I]);
 
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
@@ -501,12 +512,14 @@
   std::unique_ptr TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {
 #if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
 return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
+log("Starting LSP over stdin/stdout");
 TransportLayer = newJSONTransport(
 stdin, llvm::outs(),
 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r366811 - [clangd] Log version, cwd, args, and transport on startup. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jul 23 07:30:28 2019
New Revision: 366811

URL: http://llvm.org/viewvc/llvm-project?rev=366811=rev
Log:
[clangd] Log version, cwd, args, and transport on startup. NFC

Reviewers: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=366811=366810=366811=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Jul 23 07:30:28 2019
@@ -423,6 +423,17 @@ int main(int argc, char *argv[]) {
   llvm::errs().SetBuffered();
   StreamLogger Logger(llvm::errs(), LogLevel);
   LoggingSession LoggingSession(Logger);
+  // Write some initial logs before we start doing any real work.
+  log("{0}", clang::getClangToolFullVersion("clangd"));
+  {
+SmallString<128> CWD;
+if (auto Err = llvm::sys::fs::current_path(CWD))
+  log("Working directory unknown: {0}", Err.message());
+else
+  log("Working directory: {0}", CWD);
+  }
+  for (int I = 0; I < argc; ++I)
+log("argv[{0}]: {1}", I, argv[I]);
 
   // If --compile-commands-dir arg was invoked, check value and override 
default
   // path.
@@ -501,12 +512,14 @@ int main(int argc, char *argv[]) {
   std::unique_ptr TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {
 #if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
 return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
+log("Starting LSP over stdin/stdout");
 TransportLayer = newJSONTransport(
 stdin, llvm::outs(),
 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,


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

[PATCH] D65064: [CrossTU] Add a function to retrieve original source location.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:175
+  /// returned.
+  llvm::Optional>
+  getImportedFromSourceLocation(const clang::SourceLocation ) const;

It would be better to use `std::tuple` here. (Or * if & does not work.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65064



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

[PATCH] D65149: [Format] Add test demonstrating PR42722

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: ank, klimek, acoomans.
Herald added a project: clang.

https://bugs.llvm.org/show_bug.cgi?id=42722 describes what I believe to
be a bug in lambda formatting. If it is indeed a bug, I'd like to commit
this test that reliably reproduces it. I'll try in the coming days to
then fix the behavior and update this test to demonstrate the correct
behavior (but if anyone fixes it before I do, this test will help them
too, I think).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65149

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5173,6 +5173,17 @@
"   \"\");\n"
"}",
getLLVMStyleWithColumns(30));
+  verifyFormat("testPR42722() {\n"
+   "  int c = 19;\n"
+   "  int d = 32;\n"
+   "  Executor ex;\n"
+   "  ex.addFuture([]() {\n"
+   "  throw std::runtime_error(\"oops\");\n"
+   "}).then([](Result &) {\n"
+   "c++;\n"
+   "d++\n"
+   "  });\n"
+   "}");
 }
 
 TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5173,6 +5173,17 @@
"   \"\");\n"
"}",
getLLVMStyleWithColumns(30));
+  verifyFormat("testPR42722() {\n"
+   "  int c = 19;\n"
+   "  int d = 32;\n"
+   "  Executor ex;\n"
+   "  ex.addFuture([]() {\n"
+   "  throw std::runtime_error(\"oops\");\n"
+   "}).then([](Result &) {\n"
+   "c++;\n"
+   "d++\n"
+   "  });\n"
+   "}");
 }
 
 TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63126: [clangd] Implement "prepareRename"

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:393
 ;
+  if (Params.capabilities.RenamePrepareSupport)
+Result["renameProvider"] = llvm::json::Object{{"prepareProvider", true}};

hokein wrote:
> sammccall wrote:
> > why set this only if the client advertised support?
> The LSP says
> ``` 
>  /**
>* The server provides rename support. RenameOptions may only be
>* specified if the client states that it supports
>* `prepareSupport` in its initial `initialize` request.
>*/
>   renameProvider?: boolean | RenameOptions;
> ```
> 
> so we only send `RenameOptions` when the client declares it supports 
> prepareRename.
Ah, OK. this should be documented.

Could we put this logic above instead, for less data structure wrangling?
```
llvm::json::Value RenameProvider = llvm::json::Object{{"prepareProvider", 
true}};
if (!Params.capabilities.RenamePrepareSupport) // Only boolean allowed per LSP
  RenameProvider = true;
...
{"renameProvider", std::move(RenameProvider)}
```




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:581
+  Params.textDocument.uri.file(), Params.position,
+  Bind(
+  [](decltype(Reply) Reply, llvm::Expected> R) {

this appears to be the same as just passing std::move(Reply) to preparerename



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:298
+if (!Changes) {
+  // LSP clients may emit a hard-coded error message (e.g. "can't be
+  // renamed" in VSCode) if we returns null, we use the interl error 
message

There's some context missing here - you're explaining *why* we're going against 
LSP, but you haven't explained that we're doing so!

"LSP says to return null on failure, but that will result in a generic failure 
message. If we send an LSP error response, clients can surface the message to 
users (VSCode does)"

("will" not "may", because there's no way it can result in a detailed message)



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:296
+// Return null if the "rename" is not valid at the position.
+auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index);
+if (!Changes) {

sammccall wrote:
> High level comment indicating why we use rename to implement preparerename?
> (i.e. why isn't it too expensive)
can you say something like "Performing the rename isn't substantially more 
expensive than an AST-based check, so we just rename and throw away the 
results. We may have to revisit this when we support cross-file rename"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63126



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

[PATCH] D65146: [clangd] Log version, cwd, args, and transport on startup. NFC

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:436
+  {
+std::string LoggableArgs;
+for (int I = 0; I < argc; ++I)

this variable is not used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65146



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

Re: r366694 - [NFC] Relaxed regression tests for PR42665

[Anastasia Stulova via cfe-commits]

+ cfe-commits


From: Anastasia Stulova
Sent: 23 July 2019 15:16
To: Hans Wennborg 
Cc: Marco Antognini 
Subject: Re: r366694 - [NFC] Relaxed regression tests for PR42665


@Hans, would it be possible to merge this commit along with r366670 to the 
release 9.0?


FY, I have also added this to the blocker bug: 
https://bugs.llvm.org/show_bug.cgi?id=41727

Thanks!
Anastasia

From: cfe-commits  on behalf of Marco 
Antognini via cfe-commits 
Sent: 22 July 2019 15:47
To: cfe-commits@lists.llvm.org 
Subject: r366694 - [NFC] Relaxed regression tests for PR42665

Author: mantognini
Date: Mon Jul 22 07:47:36 2019
New Revision: 366694

URL: http://llvm.org/viewvc/llvm-project?rev=366694=rev
Log:
[NFC] Relaxed regression tests for PR42665

Following up on the buildbot failures, this commits relaxes some tests:
instead of checking for specific IR output, it now ensures that the
underlying issue (the crash), and only that, doesn't happen.

Modified:
cfe/trunk/test/CodeGenCXX/PR42665.cpp

Modified: cfe/trunk/test/CodeGenCXX/PR42665.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR42665.cpp?rev=366694=366693=366694=diff
==
--- cfe/trunk/test/CodeGenCXX/PR42665.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/PR42665.cpp Mon Jul 22 07:47:36 2019
@@ -1,7 +1,8 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -std=c++17 -O0 %s -o 
- | FileCheck %s
-// RUN: %clang_cc1 -triple %ms_abi_triple -emit-llvm -std=c++17 -O0 %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
%itanium_abi_triple
+// RUN: %clang_cc1 -std=c++17 -O0 %s -emit-llvm -o /dev/null -verify -triple 
%ms_abi_triple

 // Minimal reproducer for PR42665.
+// expected-no-diagnostics

 struct Foo {
   Foo() = default;
@@ -31,31 +32,3 @@ void foobar() {
   d(f); // Invoke virtual destructor of Foo through d.
 } // p's destructor is invoked.

-// Regexes are used to handle both kind of mangling.
-//
-// CHECK-LABEL: define linkonce_odr{{( dso_local)?}} void 
@{{.*deleter.*Foo.*}}(%struct.Foo* dereferenceable({{[0-9]+}})
-// CHECK-SAME: [[T:%.*]])
-// CHECK: [[T_ADDR:%.*]] = alloca %struct.Foo*
-// CHECK: store %struct.Foo* [[T]], %struct.Foo** [[T_ADDR]]
-// CHECK: [[R0:%.*]] = load %struct.Foo*, %struct.Foo** [[T_ADDR]]
-// CHECK: [[R1:%.*]] = bitcast %struct.Foo* [[R0]] to 
[[TYPE:.*struct\.Foo.*]]***
-// CHECK: [[VTABLE:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[R1]]
-// CHECK: [[VFUN:%.*]] = getelementptr inbounds [[TYPE]]*, [[TYPE]]** 
[[VTABLE]], i64 0
-// CHECK: [[DTOR:%.*]] = load [[TYPE]]*, [[TYPE]]** [[VFUN]]
-// CHECK: call {{(void|i8\*)}} [[DTOR]](%struct.Foo* [[R0]]
-//
-// CHECK-LABEL: define{{( dso_local)?}} void @{{.*foobar.*}}()
-// CHECK: [[P:%.*]] = alloca %struct.Pair
-// CHECK: [[F:%.*]] = alloca %struct.Foo*
-// CHECK: [[D:%.*]] = alloca [[TYPE:void \(%struct.Foo\*\)]]**
-// CHECK: call void @{{.*make_pair.*}}(%struct.Pair* sret [[P]])
-// CHECK: [[FIRST:%.*]] = getelementptr inbounds %struct.Pair, %struct.Pair* 
[[P]], i32 0, i32 0
-// CHECK: store %struct.Foo* [[FIRST]], %struct.Foo** [[F]]
-// CHECK: [[SECOND:%.*]] = getelementptr inbounds %struct.Pair, %struct.Pair* 
[[P]], i32 0, i32 1
-// CHECK: store void (%struct.Foo*)** [[SECOND]], [[TYPE]]*** [[D]]
-// CHECK: [[R0:%.*]] = load [[TYPE]]**, [[TYPE]]*** [[D]]
-// CHECK: [[R1:%.*]] = load [[TYPE]]*, [[TYPE]]** [[R0]]
-// CHECK: [[R2:%.*]] = load %struct.Foo*, %struct.Foo** [[F]]
-// CHECK: call void [[R1]](%struct.Foo* dereferenceable({{[0-9]+}}) [[R2]])
-// CHECK: call void @{{.*Pair.*Foo.*}}(%struct.Pair* [[P]])
-


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

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

It is possible to use the `Preprocessor` instead of `ASTUnit` in 
`getImportedFromSourceLocation`. But this will contain less useful (even if 
currently not used) information. The test extension in D65064 
 is not possible to do if not `ASTUnit` is 
used. (Or a tuple must be used that contains `Preprocessor` and 
`TranslationUnitDecl` and maybe other objects that are stored in the `ASTUnit`.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64638



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

[PATCH] D65146: [clangd] Log version, cwd, args, and transport on startup. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65146

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -423,6 +423,20 @@
   llvm::errs().SetBuffered();
   StreamLogger Logger(llvm::errs(), LogLevel);
   LoggingSession LoggingSession(Logger);
+  // Write some initial logs before we start doing any real work.
+  log("{0}", clang::getClangToolFullVersion("clangd"));
+  {
+SmallString<128> CWD;
+if (auto Err = llvm::sys::fs::current_path(CWD))
+  log("Working directory unknown: {0}", Err.message());
+else
+  log("Working directory: {0}", CWD);
+  }
+  {
+std::string LoggableArgs;
+for (int I = 0; I < argc; ++I)
+  log("argv[{0}]: {1}", I, argv[I]);
+  }
 
   // If --compile-commands-dir arg was invoked, check value and override 
default
   // path.
@@ -501,12 +515,14 @@
   std::unique_ptr TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {
 #if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
 return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
+log("Starting LSP over stdin/stdout");
 TransportLayer = newJSONTransport(
 stdin, llvm::outs(),
 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -423,6 +423,20 @@
   llvm::errs().SetBuffered();
   StreamLogger Logger(llvm::errs(), LogLevel);
   LoggingSession LoggingSession(Logger);
+  // Write some initial logs before we start doing any real work.
+  log("{0}", clang::getClangToolFullVersion("clangd"));
+  {
+SmallString<128> CWD;
+if (auto Err = llvm::sys::fs::current_path(CWD))
+  log("Working directory unknown: {0}", Err.message());
+else
+  log("Working directory: {0}", CWD);
+  }
+  {
+std::string LoggableArgs;
+for (int I = 0; I < argc; ++I)
+  log("argv[{0}]: {1}", I, argv[I]);
+  }
 
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
@@ -501,12 +515,14 @@
   std::unique_ptr TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {
 #if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
 TransportLayer = newXPCTransport();
 #else
 llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
 return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
+log("Starting LSP over stdin/stdout");
 TransportLayer = newJSONTransport(
 stdin, llvm::outs(),
 InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Ping.


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

https://reviews.llvm.org/D61466



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

[PATCH] D63126: [clangd] Implement "prepareRename"

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:393
 ;
+  if (Params.capabilities.RenamePrepareSupport)
+Result["renameProvider"] = llvm::json::Object{{"prepareProvider", true}};

sammccall wrote:
> why set this only if the client advertised support?
The LSP says
``` 
 /**
 * The server provides rename support. RenameOptions may only be
 * specified if the client states that it supports
 * `prepareSupport` in its initial `initialize` request.
 */
renameProvider?: boolean | RenameOptions;
```

so we only send `RenameOptions` when the client declares it supports 
prepareRename.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:298
+if (!Changes) {
+  llvm::consumeError(Changes.takeError());
+  return CB(llvm::None);

sammccall wrote:
> I thought the point of supporting this was to get errors to the user sooner. 
> That can't happen if we throw away the error message...
use our own error message instead, luckily, VSCode prompt this error message to 
users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63126



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

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

[Gabor Marton via Phabricator via cfe-commits]

martong added a subscriber: NoQ.
martong added a comment.

In D64638#1593382 , @ilya-biryukov 
wrote:

> `StaticAnalyzer/Core` does not depend on `clangFrontend` now, you can see 
> this by looking at `lib/StaticAnalyzer/Core/CMakeLists.txt`:
>
>   add_clang_library(clangStaticAnalyzerCore
>   ...
> LINK_LIBS
> clangAST
> clangASTMatchers
> clangAnalysis
> clangBasic
> clangCrossTU
> clangLex
> clangRewrite
> )
>
>
> Not a `StaticAnalyzer` expert, so I don't know whether it's acceptable to add 
> this dependency to `clangStaticAnalyzerCore`, you'll have to find someone who 
> owns the code to know whether this dependency is justified.
>  (My wild guess from looking at the names of the libraries would be that this 
> dependency is not ok and the code should go into 
> `clangStaticAnalyzerFrontend` instead. But again, not an expert here, just a 
> guess).
>
> But please add a dependency into `LINK_LIBS` inside `CMakeLists.txt` if you 
> start depending on `clangFrontend`.
>  Most of these violations are found if you build in a `cmake 
> -DBUILD_SHARED_LIBS=On` configuration.


@Szelethus @Noq Could StaticAnalyzer/Core depend on clangFrontend? I am not 
sure if we can get the Preprocessor somewhere else ...


Repository:
  rC Clang

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

https://reviews.llvm.org/D64638



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

[PATCH] D63126: [clangd] Implement "prepareRename"

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 211288.
hokein marked 6 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63126

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/test/rename.test

Index: clang-tools-extra/clangd/test/rename.test
===
--- clang-tools-extra/clangd/test/rename.test
+++ clang-tools-extra/clangd/test/rename.test
@@ -1,12 +1,45 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"rename": {"dynamicRegistration": true, "prepareSupport": true}}},"trace":"off"}}
+# CHECK:  "renameProvider": {
+# CHECK-NEXT:"prepareProvider": true
+# CHECK-NEXT: },
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
 ---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5}}}
 #  CHECK:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "there is no symbol at the given location"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+---
+{"jsonrpc":"2.0","id":4,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "there is no symbol at the given location"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 4,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+---
+{"jsonrpc":"2.0","id":3,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+#  CHECK:  "id": 3,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
 # CHECK-NEXT:"changes": {
 # CHECK-NEXT:  "file://{{.*}}/foo.cpp": [
 # CHECK-NEXT:{
@@ -26,14 +59,6 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
-#  CHECK:  "error": {
-# CHECK-NEXT:"code": -32001,
-# CHECK-NEXT:"message": "there is no symbol at the given location"
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0"

-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html;,
 "engines": {
-"vscode": "^1.30.0"
+"vscode": "^1.36.0"
 },
 "categories": [
 "Programming Languages",
@@ -35,8 +35,8 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^5.2.0",
-"vscode-languageserver": "^5.2.0"
+"vscode-languageclient": "^5.3.0-next.6",
+"vscode-languageserver": "^5.3.0-next.6"
 },
 "devDependencies": {
 "@types/mocha": "^2.2.32",
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -422,6 +422,10 @@
   /// The content format that should be