date:20191012

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

2019-10-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 224767.
nridge added a comment.

Address final comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,6 +51,9 @@
   {HighlightingKind::StaticField, "StaticField"},
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
+  {HighlightingKind::Typedef, "Typedef"},
+  {HighlightingKind::DependentType, "DependentType"},
+  {HighlightingKind::DependentName, "DependentName"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
   {HighlightingKind::Primitive, "Primitive"},
   {HighlightingKind::Macro, "Macro"}};
@@ -175,7 +178,7 @@
   }
   template
   struct $Class[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
-typename $TemplateParameter[[T]]::A* $Field[[D]];
+typename $TemplateParameter[[T]]::$DependentType[[A]]* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Variable[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Class[[AAA]];
@@ -523,6 +526,58 @@
   $Typedef[[Pointer]], $Typedef[[LVReference]], $Typedef[[RVReference]],
   $Typedef[[Array]], $Typedef[[MemberPointer]]);
   };
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[phase1]]($TemplateParameter[[T]]);
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[T]] $Parameter[[P]]) {
+$Function[[phase1]]($Parameter[[P]]);
+$DependentName[[phase2]]($Parameter[[P]]);
+  }
+)cpp",
+  R"cpp(
+  class $Class[[A]] {
+template 
+$Primitive[[void]] $Method[[bar]]($TemplateParameter[[T]]);
+  };
+
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[U]] $Parameter[[P]]) {
+$Class[[A]]().$Method[[bar]]($Parameter[[P]]);
+  }
+)cpp",
+  R"cpp(
+  struct $Class[[A]] {
+template 
+static $Primitive[[void]] $StaticMethod[[foo]]($TemplateParameter[[T]]);
+  };
+
+  template 
+  struct $Class[[B]] {
+$Primitive[[void]] $Method[[bar]]() {
+  $Class[[A]]::$StaticMethod[[foo]]($TemplateParameter[[T]]());
+}
+  };
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[foo]](typename $TemplateParameter[[T]]::$DependentType[[Type]]
+= $TemplateParameter[[T]]::$DependentName[[val]]);
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[T]] $Parameter[[P]]) {
+$Parameter[[P]].$DependentName[[Field]];
+  }
+)cpp",
+  R"cpp(
+  template 
+  class $Class[[A]] {
+$Primitive[[int]] $Method[[foo]]() {
+  return $TemplateParameter[[T]]::$DependentName[[Field]];
+}
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -41,6 +41,12 @@
 # CHECK-NEXT:"entity.name.type.typedef.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.type.dependent.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.other.dependent.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
@@ -61,7 +67,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -76,11 +82,11 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAE

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

2019-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith updated this revision to Diff 224765.
rsmith added a comment.

Add comments for various NC_ classifications. Remove the unused and broken
NC_NestedNameSpecifier classification and the code in ClassifyName that
tries to classify names as nested name specifiers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68896

Files:
  include/clang/Basic/TokenKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  test/CodeGenCXX/odr-use-lookahead.cpp
  test/SemaCXX/lambda-invalid-capture.cpp

Index: test/SemaCXX/lambda-invalid-capture.cpp
===
--- test/SemaCXX/lambda-invalid-capture.cpp
+++ test/SemaCXX/lambda-invalid-capture.cpp
@@ -16,3 +16,10 @@
   auto q = [child]{};
   const int n = sizeof(q);
 }
+
+int pr43080(int i) { // expected-note {{declared here}}
+  return [] { // expected-note {{begins here}}
+return sizeof i <
+  i; // expected-error {{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
+  }();
+}
Index: test/CodeGenCXX/odr-use-lookahead.cpp
===
--- /dev/null
+++ test/CodeGenCXX/odr-use-lookahead.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+namespace PR43080 {
+  int f(int i) { return sizeof igetName());
 if (Index.first) {
-  InsertOCLBuiltinDeclarationsFromTable(S, R, II, Index.first - 1,
+  InsertOCLBuiltinDeclarationsFromTable(*this, R, II, Index.first - 1,
 Index.second);
   return true;
 }
@@ -860,14 +860,14 @@
   if (unsigned BuiltinID = II->getBuiltinID()) {
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
 // library functions like 'malloc'. Instead, we'll just error.
-if ((S.getLangOpts().CPlusPlus || S.getLangOpts().OpenCL) &&
-S.Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
+if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
+Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
   return false;
 
-if (NamedDecl *D = S.LazilyCreateBuiltin((IdentifierInfo *)II,
- BuiltinID, S.TUScope,
- R.isForRedeclaration(),
- R.getNameLoc())) {
+if (NamedDecl *D = LazilyCreateBuiltin((IdentifierInfo *)II,
+   BuiltinID, TUScope,
+   R.isForRedeclaration(),
+   R.getNameLoc())) {
   R.addDecl(D);
   return true;
 }
@@ -1013,7 +1013,7 @@
 }
   }
 
-  if (!Found && DC->isTranslationUnit() && LookupBuiltin(S, R))
+  if (!Found && DC->isTranslationUnit() && S.LookupBuiltin(R))
 return true;
 
   if (R.getLookupName().getNameKind()
@@ -2011,7 +2011,7 @@
   // If we didn't find a use of this identifier, and if the identifier
   // corresponds to a compiler builtin, create the decl object for the builtin
   // now, injecting it into translation unit scope, and return it.
-  if (AllowBuiltinCreation && LookupBuiltin(*this, R))
+  if (AllowBuiltinCreation && LookupBuiltin(R))
 return true;
 
   // If we didn't find a use of this identifier, the ExternalSource
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2482,23 +2482,20 @@
   return BuildDeclarationNameExpr(SS, R, /* ADL */ false);
 }
 
-/// LookupInObjCMethod - The parser has read a name in, and Sema has
-/// detected that we're currently inside an ObjC method.  Perform some
-/// additional lookup.
+/// The parser has read a name in, and Sema has detected that we're currently
+/// inside an ObjC method. Perform some additional checks and determine if we
+/// should form a reference to an ivar.
 ///
 /// Ideally, most of this would be done by lookup, but there's
 /// actually quite a lot of extra work involved.
-///
-/// Returns a null sentinel to indicate trivial success.
-ExprResult
-Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S,
- IdentifierInfo *II, bool AllowBuiltinCreation) {
+DeclResult Sema::LookupIvarInObjCMethod(LookupResult &Lookup, Scope *S,
+IdentifierInfo *II) {
   SourceLocation Loc = Lookup.getNameLoc();
   ObjCMethodDecl *CurMethod = getCurMethodDecl();
 
   // Check for error condition which is already reported.
   if (!CurMethod)
-return Exp

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/include/clang/Basic/SourceManager.h:232
+// nullptr
+static const char *getInvalidBOM(StringRef BufStr);
   };

Can you make the parameter `const`?



Comment at: clang/lib/Basic/SourceManager.cpp:98
 
+const char *ContentCache::getInvalidBOM(StringRef BufStr) {
+  // If the buffer is valid, check to see if it has a UTF Byte Order Mark

Add `const` to the parameter to be more precise?


Repository:
  rC Clang

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

https://reviews.llvm.org/D68914



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

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

Thank you for taking care of this! When I fixed a bug in this part of the code 
last time, I had to do it in both files.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68914



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

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits

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

Just like templates, they are excepted from the ODR rule.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68923

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/warn-missing-variable-declarations.cpp


Index: clang/test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- clang/test/SemaCXX/warn-missing-variable-declarations.cpp
+++ clang/test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -70,6 +70,8 @@
 template constexpr int const_var_template = 0;
 template static int static_var_template = 0;
 
+template int var_template;
+
 template int var_template;
 int use_var_template() { return var_template; }
 template int var_template;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12345,6 +12345,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
   var->getLocation())) {


Index: clang/test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- clang/test/SemaCXX/warn-missing-variable-declarations.cpp
+++ clang/test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -70,6 +70,8 @@
 template constexpr int const_var_template = 0;
 template static int static_var_template = 0;
 
+template int var_template;
+
 template int var_template;
 int use_var_template() { return var_template; }
 template int var_template;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12345,6 +12345,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
   var->getLocation())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

@GorNishanov Do I read the standard correctly that there are no constraints on 
what `return_value` is? That is, could it also be a function pointer or 
function object?

  struct promise_function_pointer {
// ...
void (*return_value)(T&& value);
  };
  struct promise_function_object {
// ...
struct { void operator()(T&& value) {}; } return_value;
  };

In both cases, the expression `.return_value()` 
would be well-formed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68845



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

Re: [PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloper Day via cfe-commits

Thank you for handling that

I will look into your suggestions

MyDeveloperDay

On Sat, 12 Oct 2019 at 23:55, Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> This fails on macOS:
>
>   : 'RUN: at line 2';   grep -E "*code should be clang-formatted*"
> /Users/thakis/src/llvm-project/out/gn/obj/clang/test/Format/Output/dry-run.cpp.tmp.stderr
>   --
>   Exit Code: 2
>
>   Command Output (stderr):
>   --
>   grep: repetition-operator operand invalid
>
> grep -E means extended regex, where you want `.*` to match anything. But
> this probably should use FileCheck, or maybe even -verify since you're
> adding a dep on Frontend anyways.
>
> I've reverted this for now to green up the bots.
>
> For the reland, did y'all consider the impact of this on clang-format
> build time (it now depends on Frontend and hence on Driver and Sema, and
> Sema is slow to build) and binary size (it's now basically impossible to
> ever get the diagnostics tables in clang-format gc'd by the linker)?
>
> It might make sense to instead use LLVM's diag stuff (instead of clang's)
> since all the actual diags will be disjoint.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68554/new/
>
> https://reviews.llvm.org/D68554
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r374688 - Revert r374663 "[clang-format] Proposal for clang-format to give compiler style warnings"

2019-10-12 Thread Nico Weber via cfe-commits

Author: nico
Date: Sat Oct 12 15:58:34 2019
New Revision: 374688

URL: http://llvm.org/viewvc/llvm-project?rev=374688&view=rev
Log:
Revert r374663 "[clang-format] Proposal for clang-format to give compiler style 
warnings"

The test fails on macOS and looks a bit wrong, see comments on the review.

Also revert follow-up r374686.

Removed:
cfe/trunk/test/Format/dry-run-alias.cpp
cfe/trunk/test/Format/dry-run.cpp
Modified:
cfe/trunk/tools/clang-format/CMakeLists.txt
cfe/trunk/tools/clang-format/ClangFormat.cpp

Removed: cfe/trunk/test/Format/dry-run-alias.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run-alias.cpp?rev=374687&view=auto
==
--- cfe/trunk/test/Format/dry-run-alias.cpp (original)
+++ cfe/trunk/test/Format/dry-run-alias.cpp (removed)
@@ -1,4 +0,0 @@
-// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr
-// RUN: grep -E "*code should be clang-formatted*" %t.stderr
-
-int a ;

Removed: cfe/trunk/test/Format/dry-run.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run.cpp?rev=374687&view=auto
==
--- cfe/trunk/test/Format/dry-run.cpp (original)
+++ cfe/trunk/test/Format/dry-run.cpp (removed)
@@ -1,4 +0,0 @@
-// RUN: clang-format -style=LLVM -i --dry-run %s 2> %t.stderr
-// RUN: grep -E "*code should be clang-formatted*" %t.stderr
-
-int a ;

Modified: cfe/trunk/tools/clang-format/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/CMakeLists.txt?rev=374688&r1=374687&r2=374688&view=diff
==
--- cfe/trunk/tools/clang-format/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-format/CMakeLists.txt Sat Oct 12 15:58:34 2019
@@ -7,7 +7,6 @@ add_clang_tool(clang-format
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=374688&r1=374687&r2=374688&view=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Sat Oct 12 15:58:34 2019
@@ -18,7 +18,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -109,54 +108,6 @@ static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
-// Use --dry-run to match other LLVM tools when you mean do it but don't
-// actually do it
-static cl::opt
-DryRun("dry-run",
-   cl::desc("If set, do not actually make the formatting changes"),
-   cl::cat(ClangFormatCategory));
-
-// Use -n as a common command as an alias for --dry-run. (git and make use -n)
-static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
- cl::cat(ClangFormatCategory), 
cl::aliasopt(DryRun),
- cl::NotHidden);
-
-// Emulate being able to turn on/off the warning.
-static cl::opt
-WarnFormat("Wclang-format-violations",
-   cl::desc("Warnings about individual formatting changes needed. "
-"Used only with --dry-run or -n"),
-   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
-
-static cl::opt
-NoWarnFormat("Wno-clang-format-violations",
- cl::desc("Do not warn about individual formatting changes "
-  "needed. Used only with --dry-run or -n"),
- cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
-
-static cl::opt ErrorLimit(
-"ferror-limit",
-cl::desc("Set the maximum number of clang-format errors to emit before "
- "stopping (0 = no limit). Used only with --dry-run or -n"),
-cl::init(0), cl::cat(ClangFormatCategory));
-
-static cl::opt
-WarningsAsErrors("Werror",
- cl::desc("If set, changes formatting warnings to errors"),
- cl::cat(ClangFormatCategory));
-
-static cl::opt
-ShowColors("fcolor-diagnostics",
-   cl::desc("If set, and on a color-capable terminal controls "
-"whether or not to print diagnostics in color"),
-   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
-
-static cl::opt
-NoShowColors("fno-color-diagnostics",
- cl::desc("If set, and on a color-capable terminal controls "
-  "whether or not to print diagnostics in color"),
- cl::

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

This fails on macOS:

  : 'RUN: at line 2';   grep -E "*code should be clang-formatted*" 
/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Format/Output/dry-run.cpp.tmp.stderr
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  grep: repetition-operator operand invalid

grep -E means extended regex, where you want `.*` to match anything. But this 
probably should use FileCheck, or maybe even -verify since you're adding a dep 
on Frontend anyways.

I've reverted this for now to green up the bots.

For the reland, did y'all consider the impact of this on clang-format build 
time (it now depends on Frontend and hence on Driver and Sema, and Sema is slow 
to build) and binary size (it's now basically impossible to ever get the 
diagnostics tables in clang-format gc'd by the linker)?

It might make sense to instead use LLVM's diag stuff (instead of clang's) since 
all the actual diags will be disjoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay abandoned this revision.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.

Review comments regarding movign getInvalidBOM are addressed in D68914: 
[clang-format] Remove duplciate code from Invalid BOM detection 



Repository:
  rC Clang

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

https://reviews.llvm.org/D68767



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

[PATCH] D68915: [clang][IFS] Escape mangled name in-order to not break llvm-ifs with names mangled using MS ABI

2019-10-12 Thread Puyan Lotfi via Phabricator via cfe-commits

plotfi created this revision.
plotfi added reviewers: compnerd, cishida.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
plotfi updated this revision to Diff 224756.

Microsoft's ABI mangles names differently than Itanium and this breaks the LLVM 
yaml parser unless the name is escaped in quotes.


Repository:
  rC Clang

https://reviews.llvm.org/D68915

Files:
  lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  test/InterfaceStubs/inline.c
  test/InterfaceStubs/object.c
  test/InterfaceStubs/windows.cpp


Index: test/InterfaceStubs/windows.cpp
===
--- test/InterfaceStubs/windows.cpp
+++ test/InterfaceStubs/windows.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -target x86_64-windows-msvc -o - %s \
+// RUN: -emit-interface-stubs -emit-merged-ifs | FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT: ?helloWindowsMsvc@@YAHXZ
+int helloWindowsMsvc();
Index: test/InterfaceStubs/object.c
===
--- test/InterfaceStubs/object.c
+++ test/InterfaceStubs/object.c
@@ -2,6 +2,6 @@
 // RUN: %clang -fvisibility=default -c -o - -emit-interface-stubs %s | 
FileCheck -check-prefix=CHECK-SYMBOLS %s
 // RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck 
-check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI: data: { Type: Object, Size: 4 }
+// CHECK-TAPI: "data" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data
 int data = 42;
Index: test/InterfaceStubs/inline.c
===
--- test/InterfaceStubs/inline.c
+++ test/InterfaceStubs/inline.c
@@ -56,8 +56,8 @@
 // RUN: -c -std=gnu89 -xc %s | llvm-nm - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI-DAG: foo: { Type: Func }
-// CHECK-TAPI-DAG: foo.var: { Type: Object, Size: 4 }
+// CHECK-TAPI-DAG: "foo" : { Type: Func }
+// CHECK-TAPI-DAG: "foo.var" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS-DAG: foo
 // CHECK-SYMBOLS-DAG: foo.var
 #include "inline.h"
Index: lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -263,11 +263,11 @@
   for (const auto &E : Symbols) {
 const MangledSymbol &Symbol = E.second;
 for (auto Name : Symbol.Names) {
-  OS << "  "
+  OS << "  \""
  << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
  ? ""
  : (Symbol.ParentName + "."))
- << Name << ": { Type: ";
+ << Name << "\" : { Type: ";
   switch (Symbol.Type) {
   default:
 llvm_unreachable(


Index: test/InterfaceStubs/windows.cpp
===
--- test/InterfaceStubs/windows.cpp
+++ test/InterfaceStubs/windows.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -target x86_64-windows-msvc -o - %s \
+// RUN: -emit-interface-stubs -emit-merged-ifs | FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT: ?helloWindowsMsvc@@YAHXZ
+int helloWindowsMsvc();
Index: test/InterfaceStubs/object.c
===
--- test/InterfaceStubs/object.c
+++ test/InterfaceStubs/object.c
@@ -2,6 +2,6 @@
 // RUN: %clang -fvisibility=default -c -o - -emit-interface-stubs %s | FileCheck -check-prefix=CHECK-SYMBOLS %s
 // RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI: data: { Type: Object, Size: 4 }
+// CHECK-TAPI: "data" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data
 int data = 42;
Index: test/InterfaceStubs/inline.c
===
--- test/InterfaceStubs/inline.c
+++ test/InterfaceStubs/inline.c
@@ -56,8 +56,8 @@
 // RUN: -c -std=gnu89 -xc %s | llvm-nm - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI-DAG: foo: { Type: Func }
-// CHECK-TAPI-DAG: foo.var: { Type: Object, Size: 4 }
+// CHECK-TAPI-DAG: "foo" : { Type: Func }
+// CHECK-TAPI-DAG: "foo.var" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS-DAG: foo
 // CHECK-SYMBOLS-DAG: foo.var
 #include "inline.h"
Index: lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -263,11 +263,11 @@
   for (const auto &E : Symbols) {
 const MangledSymbol &Symbol = E.second;
 for (auto Name : Symbol.Names) {
-  OS << "  "
+  OS << "  \""
  << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
  ? ""
  : (Symbol.ParentName + "."))
- << Name << ":

[PATCH] D68915: [clang][IFS] Escape mangled name in-order to not break llvm-ifs with names mangled using MS ABI

2019-10-12 Thread Puyan Lotfi via Phabricator via cfe-commits

plotfi updated this revision to Diff 224756.

Repository:
  rC Clang

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

https://reviews.llvm.org/D68915

Files:
  lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  test/InterfaceStubs/inline.c
  test/InterfaceStubs/object.c
  test/InterfaceStubs/windows.cpp


Index: test/InterfaceStubs/windows.cpp
===
--- test/InterfaceStubs/windows.cpp
+++ test/InterfaceStubs/windows.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -target x86_64-windows-msvc -o - %s \
+// RUN: -emit-interface-stubs -emit-merged-ifs | FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT: ?helloWindowsMsvc@@YAHXZ
+int helloWindowsMsvc();
Index: test/InterfaceStubs/object.c
===
--- test/InterfaceStubs/object.c
+++ test/InterfaceStubs/object.c
@@ -2,6 +2,6 @@
 // RUN: %clang -fvisibility=default -c -o - -emit-interface-stubs %s | 
FileCheck -check-prefix=CHECK-SYMBOLS %s
 // RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck 
-check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI: data: { Type: Object, Size: 4 }
+// CHECK-TAPI: "data" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data
 int data = 42;
Index: test/InterfaceStubs/inline.c
===
--- test/InterfaceStubs/inline.c
+++ test/InterfaceStubs/inline.c
@@ -56,8 +56,8 @@
 // RUN: -c -std=gnu89 -xc %s | llvm-nm - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI-DAG: foo: { Type: Func }
-// CHECK-TAPI-DAG: foo.var: { Type: Object, Size: 4 }
+// CHECK-TAPI-DAG: "foo" : { Type: Func }
+// CHECK-TAPI-DAG: "foo.var" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS-DAG: foo
 // CHECK-SYMBOLS-DAG: foo.var
 #include "inline.h"
Index: lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -263,11 +263,11 @@
   for (const auto &E : Symbols) {
 const MangledSymbol &Symbol = E.second;
 for (auto Name : Symbol.Names) {
-  OS << "  "
+  OS << "  \""
  << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
  ? ""
  : (Symbol.ParentName + "."))
- << Name << ": { Type: ";
+ << Name << "\" : { Type: ";
   switch (Symbol.Type) {
   default:
 llvm_unreachable(


Index: test/InterfaceStubs/windows.cpp
===
--- test/InterfaceStubs/windows.cpp
+++ test/InterfaceStubs/windows.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -target x86_64-windows-msvc -o - %s \
+// RUN: -emit-interface-stubs -emit-merged-ifs | FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT: ?helloWindowsMsvc@@YAHXZ
+int helloWindowsMsvc();
Index: test/InterfaceStubs/object.c
===
--- test/InterfaceStubs/object.c
+++ test/InterfaceStubs/object.c
@@ -2,6 +2,6 @@
 // RUN: %clang -fvisibility=default -c -o - -emit-interface-stubs %s | FileCheck -check-prefix=CHECK-SYMBOLS %s
 // RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI: data: { Type: Object, Size: 4 }
+// CHECK-TAPI: "data" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data
 int data = 42;
Index: test/InterfaceStubs/inline.c
===
--- test/InterfaceStubs/inline.c
+++ test/InterfaceStubs/inline.c
@@ -56,8 +56,8 @@
 // RUN: -c -std=gnu89 -xc %s | llvm-nm - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
-// CHECK-TAPI-DAG: foo: { Type: Func }
-// CHECK-TAPI-DAG: foo.var: { Type: Object, Size: 4 }
+// CHECK-TAPI-DAG: "foo" : { Type: Func }
+// CHECK-TAPI-DAG: "foo.var" : { Type: Object, Size: 4 }
 // CHECK-SYMBOLS-DAG: foo
 // CHECK-SYMBOLS-DAG: foo.var
 #include "inline.h"
Index: lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -263,11 +263,11 @@
   for (const auto &E : Symbols) {
 const MangledSymbol &Symbol = E.second;
 for (auto Name : Symbol.Names) {
-  OS << "  "
+  OS << "  \""
  << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
  ? ""
  : (Symbol.ParentName + "."))
- << Name << ": { Type: ";
+ << Name << "\" : { Type: ";
   switch (Symbol.Type) {
   default:
 llvm_unreachable(
___
cfe-commits mailing list

[PATCH] D64874: [Sema] Improve handling of function pointer conversions

2019-10-12 Thread Mark de Wever via Phabricator via cfe-commits

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

Removed the language version test.

I also added `clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp` to show 
this is not enough to allow `noreturn` to all language versions, so this file 
should not be committed.

Do you want me to look at a way to allow the `noreturn` for C++98/C++11? If yes 
I prefer to make that a separate commit.

AFAIK the `[[noreturn]]` conversion is a clang extension, should I file a DR 
for the standard? AFAIK there isn't one yet.

If you still like the patch could you commit it (except 
`clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp`) since I don't have 
commit access?


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

https://reviews.llvm.org/D64874

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
  clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp

Index: clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+typedef int (*fp)();
+typedef int (*fpnr)() __attribute__((noreturn));
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template 
+struct FP {};
+
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template 
+struct FPNR {};
+
+int f();
+int fnr() __attribute__((noreturn));
+
+FP<&f> fp_valid;
+FPNR<&fnr> fpnr_valid;
+
+#if __cplusplus < 201703L
+// expected-error@+2 {{non-type template argument of type 'int (*)() __attribute__((noreturn))' cannot be converted to a value of type 'fp' (aka 'int (*)()')}}
+#endif
+FP<&fnr> fp_invalid_before_cxx17;
+
+#if __cplusplus < 201703L
+// expected-error@+4 {{non-type template argument of type 'int (*)()' cannot be converted to a value of type 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#else
+// expected-error@+2 {{value of type 'int (*)()' is not implicitly convertible to 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#endif
+FPNR<&f> fpnr_invalid;
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for PR40024.
+
+#if __cplusplus < 201703L
+// expected-no-diagnostics
+#endif
+
+namespace valid_conversion {
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace valid_conversion
+
+namespace invalid_conversion {
+struct S {
+  int f(void) { return 110; }
+} s;
+
+#if __cplusplus >= 201703L
+// expected-note@+3 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'a'}}
+#endif
+template 
+int f10(void) { return (s.*a)(); }
+
+#if __cplusplus >= 201703L
+// expected-error@+3 {{no matching function for call to 'f10'}}
+#endif
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace invalid_conversion
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,13 @@
 ObjCLifetimeConversion))
 RefExpr = ImpCastExprToType(RefExpr.get(), ParamType.getUnqualifiedType(), CK_NoOp);
 
+  // Starting with C++17 the noexcept is part of the function signature but
+  // a noexcept function can be converted to a noexcept(false) function.
+  QualType ResultTy;
+  if (IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+   ParamType.getUnqualifiedType(), ResultTy))
+RefExpr = ImpCastExprToType(RefExpr.get(), ResultTy, CK_NoOp);
+
   assert(!RefExpr.isInvalid() &&
  Context.hasSameType(((Expr*) RefExpr.get())->getType(),
  ParamType.getUnqualifiedType()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-12 Thread Daniel via Phabricator via cfe-commits

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



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:868
+  if (CheckNewIdentifier != Idents.end() &&
+  CheckNewIdentifier->second->isKeyword(getLangOpts())) {
+Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;

Daniel599 wrote:
> aaron.ballman wrote:
> > What if changing it would switch to using a macro instead of a keyword? 
> > e.g.,
> > ```
> > #define foo 12
> > 
> > void func(int Foo); // Changing Foo to foo would be bad, no?
> > ```
> That's another type of bug, just like the one I found 
> https://bugs.llvm.org/show_bug.cgi?id=43306
> I don't aim on solving all of them in one patch, my patch just fixes keywords.
> Also, I don't think my patch makes the above situation worse.
Regarding your comment about macro name, I can fix it using 
`IdentifierInfo::hasMacroDefinition`.
Should I fix it in this patch? I`ll add another value to `ShouldFixStatus` and 
another error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539



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

[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-12 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:1
+//===--- AIX.cpp - AIX ToolChain Implementations *- C++ -*-===//
+//

See Jason's comment about the line length.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:21
+using namespace clang;
+using namespace llvm::opt;
+

The name lookup rules should be sufficient to avoid needing so many using 
directives.

Try:
```
namespace aix = clang::driver::tools::aix;
using AIX = clang::driver::toolchains::AIX;

using namespace llvm::opt;
```




Comment at: clang/lib/Driver/ToolChains/AIX.cpp:24
+void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+   const InputInfo &Output,
+   const InputInfoList &Inputs,

See the comment regarding `clang-format`.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:34
+  const bool IsArch64Bit = ToolChain.getTriple().isArch64Bit();
+  // Only support 32 and 64 bit
+  if (!IsArch32Bit && !IsArch64Bit)

Periods at the end of sentences in comments please. Please update the comments 
in this patch.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:36
+  if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value");
+

Add a period to the end of the sentence.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:38
+
+  if (!Args.hasArg(options::OPT_nostdlib)) {
+CmdArgs.push_back("-e");

Xiangling_L wrote:
> Test with Clangtana on terran, when no '-nostdlib' specified, since '-e' & 
> '__start' are the default behavior for AIX system linker, so there are no 
> explicitly '-e' & '__start' found on linker input commanline, so I am 
> wondering do we need to explicitly add them to 'CmdArgs'?
Agreed. It does not seem that the compiler (GCC or XL) passes `-e __start` 
explicitly. This block should be removed.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:57
+
+  // Set linking mode (i.e. 32/64-bit) and the address of
+  // text and data sections based on arch bit width

Add a comma after "i.e.".



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:74
+if (Args.hasArg(options::OPT_pg))
+  crt0 = IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o";
+// Enable profiling when "-p" is specified

For 32-bit mode, there is a "reentrant" variant for when `-pthread` or 
`-pthreads` is specified.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:81
+
+if (crt0)
+  CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crt0)));

This is always true. Maybe use a `getCrt0Basename` lambda?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:105
+
+/// AIX - AIX tool chain which can call as(1) and ld(1) directly.
+AIX::AIX(const Driver &D, const llvm::Triple &Triple,

See comment regarding a TODO.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:107
+AIX::AIX(const Driver &D, const llvm::Triple &Triple,
+ const ArgList &Args)
+: ToolChain(D, Triple, Args) {

See comment regarding `clang-format`.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:114
+
+Tool *AIX::buildLinker() const { 
+  return new tools::aix::Linker(*this); 

Use a trailing return type to make use of name lookup rules to find `Tool`:
```
auto AIX::buildLinker() const -> Tool * {
```



Comment at: clang/lib/Driver/ToolChains/AIX.h:19
+
+/// aix -- Directly call system default assembler and linker
+namespace aix {

This patch does not cover the assembler as the comment claims. Please add a 
TODO.



Comment at: clang/lib/Driver/ToolChains/AIX.h:35
+} // end namespace aix
+} // end namespace tools
+

I would suggest adding a blank line before closing `tools` and also closing the 
`driver` and `clang` namespaces here. Reopen the latter two in a block with the 
opening of `toolchains` below.



Comment at: clang/lib/Driver/ToolChains/AIX.h:42
+  AIX(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args);
+  ~AIX() override;

I believe `clang-format` would indent this so that the first character comes 
right after the position of the left parenthesis that opened the parameter list.



Comment at: clang/test/Driver/aix-ld.c:1
+// General tests that ld invocations on AIX targets sane. Note that we use
+// sysroot to make these tests independent of the host system.

Add "are" before "sane".



Comment at: clang/test/Driver/aix-ld.c:3
+// sysroot to make these tests independent of the host system.
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
-

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: bruno, arphaman, klimek, owenpan, 
mitchell-stellar, dexonsmith.
MyDeveloperDay added projects: clang, clang-format, clang-tools-extra.

Review comments on D68767: [clang-format] NFC - Move functionality into 
functions to help code structure  asked that 
this duplicated code in clang-format was moved to one central location that 
being SourceManager (where it had originally be copied from I assume)

Moved function into static function  ContentCache::getInvalidBOM(...)  - 
(closest class to where it was defined before)
Updated clang-format to call this static function

Added unit tests for said new function in BasicTests

Sorry not my normal code area so may have the wrong reviewers. (but your names 
were on the recent history)


Repository:
  rC Clang

https://reviews.llvm.org/D68914

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Basic/SourceManagerTest.cpp

Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -200,6 +200,47 @@
 "");
 }
 
+TEST_F(SourceManagerTest, getInvalidBOM) {
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM(""), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\x00\x00\x00"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\xFF\xFF\xFF"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("#include "),
+nullptr);
+
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFE\xFF#include ")),
+"UTF-16 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFF\xFE#include ")),
+"UTF-16 (LE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x2B\x2F\x76#include ")),
+"UTF-7");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xF7\x64\x4C#include ")),
+"UTF-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xDD\x73\x66\x73#include ")),
+"UTF-EBCDIC");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x0E\xFE\xFF#include ")),
+"SCSU");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFB\xEE\x28#include ")),
+"BOCU-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x84\x31\x95\x33#include ")),
+"GB-18030");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\x00\x00\xFE\xFF#include "))),
+"UTF-32 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\xFF\xFE\x00\x00#include "))),
+"UTF-32 (LE)");
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -290,31 +290,6 @@
   }
 }
 
-// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
-// nullptr.
-static const char *getInValidBOM(StringRef BufStr) {
-  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
-  // We only support UTF-8 with and without a BOM right now.  See
-  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
-  // for more information.
-  const char *InvalidBOM =
-  llvm::StringSwitch(BufStr)
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
-  "UTF-32 (BE)")
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
-  "UTF-32 (LE)")
-  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
-  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
-  .StartsWith("\x2B\x2F\x76", "UTF-7")
-  .StartsWith("\xF7\x64\x4C", "UTF-1")
-  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-  .StartsWith("\x0E\xFE\xFF", "SCSU")
-  .StartsWith("\xFB\xEE\x28", "BOCU-1")
-  .StartsWith("\x84\x31\x95\x33", "GB-18030")
-  .Default(nullptr);
-  return InvalidBOM;
-}
-
 static bool
 emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
 const std::unique_ptr &Code) {
@@ -400,7 +375,7 @@
 
   StringRef BufStr = Code->getBuffer();
 
-  const char *InvalidBOM = getInValidBOM(BufStr);
+  const char *InvalidBOM = SrcMgr::ContentCache::getInvalidBOM(BufStr);
 
   if (InvalidBOM) {
 errs() << "error: encoding with unsupported byte order

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2019-10-12 Thread Mark de Wever via Phabricator via cfe-commits

Mordante created this revision.
Mordante added reviewers: rsmith, rtrieu, xbolva00.
Mordante added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68913

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template 
 struct Iterator {
@@ -67,14 +68,17 @@
   for (const int &x : int_non_ref_container) {}
   // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
   // expected-note@-2 {{use non-reference type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
   // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
   // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
   // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
 
 void test1() {
@@ -83,6 +87,7 @@
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (int &x : A) {}
@@ -93,6 +98,7 @@
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (double &x : A) {}
@@ -103,6 +109,7 @@
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : A) {}
@@ -126,6 +133,7 @@
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   //for (double &x : B) {}
   // Binding error
@@ -135,6 +143,7 @@
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   //for (Bar &x : B) {}
   // Binding error
@@ -148,6 +157,7 @@
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : C) {}
@@ -158,6 +168,7 @@
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
   //for (int &x : C) {}
@@ -174,6 +185,7 @@
   for (const Bar x : D) {}
   // expected-warning@-1 {{creates a copy}}
   // expected-note@-2 {{'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
   for (Bar &x : D) {}
   // No warning
   for (Bar x : D) {}
@@ -182,6 +194,7 @@
   for (const int &x : D) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : D) {}
   // No warning
   //for (int &x : D) {}
@@ -196,6 +209,7 @@
   for (const Bar &x : E) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : E) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : E) {}
@@ -210,6 +224,7 @@
   for (const Bar &x : F) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-12 Thread Mark de Wever via Phabricator via cfe-commits

Mordante created this revision.
Mordante added reviewers: rsmith, rtrieu, xbolva00.
Mordante added a project: clang.

This makes the range loop warnings part of -Wall.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68912

Files:
  clang/include/clang/Basic/DiagnosticGroups.td


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f20bc17d005: [clang-format] Proposal for clang-format to 
give compiler style warnings (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554

Files:
  clang/test/Format/dry-run-alias.cpp
  clang/test/Format/dry-run.cpp
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -108,6 +109,54 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+// Use --dry-run to match other LLVM tools when you mean do it but don't
+// actually do it
+static cl::opt
+DryRun("dry-run",
+   cl::desc("If set, do not actually make the formatting changes"),
+   cl::cat(ClangFormatCategory));
+
+// Use -n as a common command as an alias for --dry-run. (git and make use -n)
+static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
+ cl::cat(ClangFormatCategory), cl::aliasopt(DryRun),
+ cl::NotHidden);
+
+// Emulate being able to turn on/off the warning.
+static cl::opt
+WarnFormat("Wclang-format-violations",
+   cl::desc("Warnings about individual formatting changes needed. "
+"Used only with --dry-run or -n"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoWarnFormat("Wno-clang-format-violations",
+ cl::desc("Do not warn about individual formatting changes "
+  "needed. Used only with --dry-run or -n"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt ErrorLimit(
+"ferror-limit",
+cl::desc("Set the maximum number of clang-format errors to emit before "
+ "stopping (0 = no limit). Used only with --dry-run or -n"),
+cl::init(0), cl::cat(ClangFormatCategory));
+
+static cl::opt
+WarningsAsErrors("Werror",
+ cl::desc("If set, changes formatting warnings to errors"),
+ cl::cat(ClangFormatCategory));
+
+static cl::opt
+ShowColors("fcolor-diagnostics",
+   cl::desc("If set, and on a color-capable terminal controls "
+"whether or not to print diagnostics in color"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoShowColors("fno-color-diagnostics",
+ cl::desc("If set, and on a color-capable terminal controls "
+  "whether or not to print diagnostics in color"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
 static cl::list FileNames(cl::Positional, cl::desc("[ ...]"),
cl::cat(ClangFormatCategory));
 
@@ -241,6 +290,95 @@
   }
 }
 
+// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
+// nullptr.
+static const char *getInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+  llvm::StringSwitch(BufStr)
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
+  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
+  .StartsWith("\x2B\x2F\x76", "UTF-7")
+  .StartsWith("\xF7\x64\x4C", "UTF-1")
+  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+  .StartsWith("\x0E\xFE\xFF", "SCSU")
+  .StartsWith("\xFB\xEE\x28", "BOCU-1")
+  .StartsWith("\x84\x31\x95\x33", "GB-18030")
+  .Default(nullptr);
+  return InvalidBOM;
+}
+
+static bool
+emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
+const std::unique_ptr &Code) {
+  if (Replaces.empty()) {
+return false;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  DiagOpts->ShowColors = (ShowColors && !NoShowColors);
+
+  TextDiagnosticPrinter *DiagsBuffer =
+

r374663 - [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread Paul Hoad via cfe-commits

Author: paulhoad
Date: Sat Oct 12 08:36:05 2019
New Revision: 374663

URL: http://llvm.org/viewvc/llvm-project?rev=374663&view=rev
Log:
[clang-format] Proposal for clang-format to give compiler style warnings

Summary:
Related somewhat to {D29039}

On seeing a quote on twitter by @invalidop

> If it's not formatted with clang-format it's a build error.

This made me want to change the way I use clang-format into a tool that could 
optionally show me where my source code violates clang-format syle.

When I'm making a change to clang-format itself, one thing I like to do to test 
the change is to ensure I didn't cause a huge wave of changes, what I want to 
do is simply run this on a known formatted directory and see if any new 
differences arrive in a manner I'm used to.

This started me thinking that we should allow build systems to run clang-format 
on a whole tree and emit compiler style warnings about files that fail 
clang-format in a form that would make them as a warning in most build systems 
and because those build systems range in their construction I don't think its 
unreasonable to NOT expect them to have to do the directory searching or 
parsing the output replacements themselves, but simply transform that into an 
error code when there are changes required.

I am starting this by suggesing adding a -n or -dry-run command line argument 
which would emit a warning/error of the form

Support for various common compiler command line argumuments like '-Werror' and 
'-ferror-limit' could make this very flexible to be integrated into build 
systems and CI systems.

```
> $ /usr/bin/clang-format --dry-run ClangFormat.cpp -ferror-limit=3 
> -fcolor-diagnostics
> ClangFormat.cpp:54:29: warning: code should be clang-formatted 
> [-Wclang-format-violations]
> static cl::list
> ^
> ClangFormat.cpp:55:20: warning: code should be clang-formatted 
> [-Wclang-format-violations]
> LineRanges("lines", cl::desc(": - format a range of\n"
>^
> ClangFormat.cpp:55:77: warning: code should be clang-formatted 
> [-Wclang-format-violations]
> LineRanges("lines", cl::desc(": - format a range of\n"
> ^
```

Reviewers: mitchell-stellar, klimek, owenpan

Reviewed By: klimek

Subscribers: mgorny, cfe-commits

Tags: #clang-format, #clang-tools-extra, #clang

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

Added:
cfe/trunk/test/Format/dry-run-alias.cpp
cfe/trunk/test/Format/dry-run.cpp
Modified:
cfe/trunk/tools/clang-format/CMakeLists.txt
cfe/trunk/tools/clang-format/ClangFormat.cpp

Added: cfe/trunk/test/Format/dry-run-alias.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run-alias.cpp?rev=374663&view=auto
==
--- cfe/trunk/test/Format/dry-run-alias.cpp (added)
+++ cfe/trunk/test/Format/dry-run-alias.cpp Sat Oct 12 08:36:05 2019
@@ -0,0 +1,4 @@
+// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr
+// RUN: grep -E "*code should be clang-formatted*" %t.stderr
+
+int a ;

Added: cfe/trunk/test/Format/dry-run.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run.cpp?rev=374663&view=auto
==
--- cfe/trunk/test/Format/dry-run.cpp (added)
+++ cfe/trunk/test/Format/dry-run.cpp Sat Oct 12 08:36:05 2019
@@ -0,0 +1,4 @@
+// RUN: clang-format -style=LLVM -i --dry-run %s 2> %t.stderr
+// RUN: grep -E "*code should be clang-formatted*" %t.stderr
+
+int a ;

Modified: cfe/trunk/tools/clang-format/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/CMakeLists.txt?rev=374663&r1=374662&r2=374663&view=diff
==
--- cfe/trunk/tools/clang-format/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-format/CMakeLists.txt Sat Oct 12 08:36:05 2019
@@ -7,6 +7,7 @@ add_clang_tool(clang-format
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
+  clangFrontend
   clangRewrite
   clangToolingCore
   )

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=374663&r1=374662&r2=374663&view=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Sat Oct 12 08:36:05 2019
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -108,6 +109,54 @@ static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),

r374659 - remove an useless allocation found by scan-build - the new Dead nested assignment check

2019-10-12 Thread Sylvestre Ledru via cfe-commits

Author: sylvestre
Date: Sat Oct 12 08:24:00 2019
New Revision: 374659

URL: http://llvm.org/viewvc/llvm-project?rev=374659&view=rev
Log:
remove an useless allocation found by scan-build - the new Dead nested 
assignment check

Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=374659&r1=374658&r2=374659&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Sat Oct 12 08:24:00 2019
@@ -4413,8 +4413,8 @@ Value *ScalarExprEmitter::VisitAsTypeExp
 return Src;
   }
 
-  return Src = createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(),
-Src, DstTy, "astype");
+  return createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(),
+  Src, DstTy, "astype");
 }
 
 Value *ScalarExprEmitter::VisitAtomicExpr(AtomicExpr *E) {


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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 224743.
MyDeveloperDay added a comment.

Incorporate feedback from @owenpan on D68767: [clang-format] NFC - Move 
functionality into functions to help code structure 
 which this revision is reliant upon.


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

https://reviews.llvm.org/D68554

Files:
  clang/test/Format/dry-run-alias.cpp
  clang/test/Format/dry-run.cpp
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -108,6 +109,54 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+// Use --dry-run to match other LLVM tools when you mean do it but don't
+// actually do it
+static cl::opt
+DryRun("dry-run",
+   cl::desc("If set, do not actually make the formatting changes"),
+   cl::cat(ClangFormatCategory));
+
+// Use -n as a common command as an alias for --dry-run. (git and make use -n)
+static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
+ cl::cat(ClangFormatCategory), cl::aliasopt(DryRun),
+ cl::NotHidden);
+
+// Emulate being able to turn on/off the warning.
+static cl::opt
+WarnFormat("Wclang-format-violations",
+   cl::desc("Warnings about individual formatting changes needed. "
+"Used only with --dry-run or -n"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoWarnFormat("Wno-clang-format-violations",
+ cl::desc("Do not warn about individual formatting changes "
+  "needed. Used only with --dry-run or -n"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt ErrorLimit(
+"ferror-limit",
+cl::desc("Set the maximum number of clang-format errors to emit before "
+ "stopping (0 = no limit). Used only with --dry-run or -n"),
+cl::init(0), cl::cat(ClangFormatCategory));
+
+static cl::opt
+WarningsAsErrors("Werror",
+ cl::desc("If set, changes formatting warnings to errors"),
+ cl::cat(ClangFormatCategory));
+
+static cl::opt
+ShowColors("fcolor-diagnostics",
+   cl::desc("If set, and on a color-capable terminal controls "
+"whether or not to print diagnostics in color"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoShowColors("fno-color-diagnostics",
+ cl::desc("If set, and on a color-capable terminal controls "
+  "whether or not to print diagnostics in color"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
 static cl::list FileNames(cl::Positional, cl::desc("[ ...]"),
cl::cat(ClangFormatCategory));
 
@@ -241,6 +290,95 @@
   }
 }
 
+// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
+// nullptr.
+static const char *getInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+  llvm::StringSwitch(BufStr)
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
+  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
+  .StartsWith("\x2B\x2F\x76", "UTF-7")
+  .StartsWith("\xF7\x64\x4C", "UTF-1")
+  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+  .StartsWith("\x0E\xFE\xFF", "SCSU")
+  .StartsWith("\xFB\xEE\x28", "BOCU-1")
+  .StartsWith("\x84\x31\x95\x33", "GB-18030")
+  .Default(nullptr);
+  return InvalidBOM;
+}
+
+static bool
+emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
+const std::unique_ptr &Code) {
+  if (Replaces.empty()) {
+return false;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  DiagOpts->ShowColors = (ShowColors && !NoShowColors);
+
+  TextDiag

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added a comment.

I'll likely abandon this review when D68554: [clang-format] Proposal for 
clang-format to give compiler style warnings  
lands




Comment at: clang/tools/clang-format/ClangFormat.cpp:245
+// Returns an invalid BOM
+static const char *hasInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).

owenpan wrote:
> owenpan wrote:
> > This code was copied from `clang/lib/Basic/SourceManager.cpp`. I suggest 
> > that you move this function to there and export it through 
> > `clang/include/clang/Basic/SourceManager.h`.
> May I suggest `getInvalidBOM(const StringRef)` for the function name and 
> parameter type?
@klimek  already got me on that one over in {D68554} 



Comment at: clang/tools/clang-format/ClangFormat.cpp:245-266
+static const char *hasInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+  llvm::StringSwitch(BufStr)

MyDeveloperDay wrote:
> owenpan wrote:
> > owenpan wrote:
> > > This code was copied from `clang/lib/Basic/SourceManager.cpp`. I suggest 
> > > that you move this function to there and export it through 
> > > `clang/include/clang/Basic/SourceManager.h`.
> > May I suggest `getInvalidBOM(const StringRef)` for the function name and 
> > parameter type?
> @klimek  already got me on that one over in {D68554} 
That sounds good, do you mind if I do that separately? (as this review is a 
little entwined with 
{D68554}, which is where I'll actually update that revision with your 
suggestions.



Comment at: clang/tools/clang-format/ClangFormat.cpp:381
+// Dump the configuration.
+static unsigned dumpConfig(StringRef AssumeFileName) {
+  StringRef FileName;

owenpan wrote:
> `AssumeFileName` is a file-scope global, so the file-scope function doesn't 
> need to make it a parameter? Also, a `bool` or `int` may be a better return 
> type than `unsigned`.
I think it should be `int` as it ends up being the return from main, I'll do 
that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68767



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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 224742.
MyDeveloperDay added a comment.

I'm going to commit this as it was approved, but I've added some simple lit 
tests, which exposed a double free, I fixed that up, and for the lit tests not 
to fail with an exit code I now return WarningAsError from the emitReplacements 
function.


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

https://reviews.llvm.org/D68554

Files:
  clang/test/Format/dry-run-alias.cpp
  clang/test/Format/dry-run.cpp
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -108,6 +109,54 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+// Use --dry-run to match other LLVM tools when you mean do it but don't
+// actually do it
+static cl::opt
+DryRun("dry-run",
+   cl::desc("If set, do not actually make the formatting changes"),
+   cl::cat(ClangFormatCategory));
+
+// Use -n as a common command as an alias for --dry-run. (git and make use -n)
+static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
+ cl::cat(ClangFormatCategory), cl::aliasopt(DryRun),
+ cl::NotHidden);
+
+// Emulate being able to turn on/off the warning.
+static cl::opt
+WarnFormat("Wclang-format-violations",
+   cl::desc("Warnings about individual formatting changes needed. "
+"Used only with --dry-run or -n"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoWarnFormat("Wno-clang-format-violations",
+ cl::desc("Do not warn about individual formatting changes "
+  "needed. Used only with --dry-run or -n"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt ErrorLimit(
+"ferror-limit",
+cl::desc("Set the maximum number of clang-format errors to emit before "
+ "stopping (0 = no limit). Used only with --dry-run or -n"),
+cl::init(0), cl::cat(ClangFormatCategory));
+
+static cl::opt
+WarningsAsErrors("Werror",
+ cl::desc("If set, changes formatting warnings to errors"),
+ cl::cat(ClangFormatCategory));
+
+static cl::opt
+ShowColors("fcolor-diagnostics",
+   cl::desc("If set, and on a color-capable terminal controls "
+"whether or not to print diagnostics in color"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoShowColors("fno-color-diagnostics",
+ cl::desc("If set, and on a color-capable terminal controls "
+  "whether or not to print diagnostics in color"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
 static cl::list FileNames(cl::Positional, cl::desc("[ ...]"),
cl::cat(ClangFormatCategory));
 
@@ -241,6 +290,95 @@
   }
 }
 
+// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
+// nullptr.
+static const char *getInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+  llvm::StringSwitch(BufStr)
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
+  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
+  .StartsWith("\x2B\x2F\x76", "UTF-7")
+  .StartsWith("\xF7\x64\x4C", "UTF-1")
+  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+  .StartsWith("\x0E\xFE\xFF", "SCSU")
+  .StartsWith("\xFB\xEE\x28", "BOCU-1")
+  .StartsWith("\x84\x31\x95\x33", "GB-18030")
+  .Default(nullptr);
+  return InvalidBOM;
+}
+
+static bool
+emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
+const std::unique_ptr &Code) {
+  if (Replaces.empty()) {
+return false;
+  }
+
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  DiagOpts->ShowCo

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

I'll add your test case, but I'll probably reuse the existing data structures.

In D68845#1705430 , @Quuxplusone wrote:

> Oh, and can you please make sure there are test cases for all the various 
> cases covered in P1155 
> ? 
> Specifically, I would expect all three of the following test cases to compile 
> successfully. It looks like they compile successfully in trunk right now 
> (Godbolt ), so we're just testing that 
> they don't get broken in the future.
>
>   struct Widget { Widget(); Widget(const Widget&) = delete; Widget(Widget&&); 
> };
>   struct To { operator Widget() &&; };
>   task nine() { To t; co_return t; }
>  
>   struct Fowl { Fowl(Widget); };
>   task eleven() { Widget w; co_return w; }
>  
>   struct Base { Base(); Base(const Base&) = delete; Base(Base&&); };
>   struct Derived : Base {};
>   task thirteen() { Derived result; co_return result; }
>


These seem to work automatically, because in the end we're just building a 
function call, which does the right implicit conversions if needed.

In D68845#1706193 , @Quuxplusone wrote:

> One more test to add:
>
>   struct Widget {
>   task foo() && {
>   co_return *this;  // IIUC this should call return_value(Widget&), 
> not return_value(Widget&&)
>   }
>   };
>


We're currently not considering `*this` as implicitly movable, because it's not 
a DeclRefExpr.




Comment at: clang/lib/Sema/SemaCoroutine.cpp:869
   if (E) {
-auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, 
CES_AsIfByStdMove);
-if (NRVOCandidate) {
-  InitializedEntity Entity =
-  InitializedEntity::InitializeResult(Loc, E->getType(), 
NRVOCandidate);
-  ExprResult MoveResult = this->PerformMoveOrCopyInitialization(
-  Entity, NRVOCandidate, E->getType(), E);
-  if (MoveResult.get())
-E = MoveResult.get();
-}
+VarDecl *NRVOCandidate =
+getCopyElisionCandidate(E->getType(), E, CES_Default);

Quuxplusone wrote:
> aaronpuchert wrote:
> > Quuxplusone wrote:
> > > aaronpuchert wrote:
> > > > Should be renamed to `RVOCandidate`, I don't think NRVO can happen here.
> > > (Btw, I have no comment on the actual code change in this patch. I'm here 
> > > in my capacity as 
> > > [RVO-explainer](https://www.youtube.com/watch?v=hA1WNtNyNbo)-and-[P1155](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1155r2.html)-author,
> > >  not code-understander. ;))
> > > 
> > > What's happening here is never technically "RVO" at all, because there is 
> > > no "RV". However, the "N" is accurate. (See [my acronym 
> > > glossary](https://quuxplusone.github.io/blog/2019/08/02/the-tough-guide-to-cpp-acronyms/#rvo-nrvo-urvo)
> > >  for details.)
> > > The important thing here is that when you say `co_return x;` the `x` is 
> > > //named//, and it //would be// a candidate for NRVO if we were in a 
> > > situation where NRVO was possible at all.
> > > 
> > > The actual optimization that is happening here is "implicit move."
> > > 
> > > I would strongly prefer to keep the name `NRVOCandidate` here, because 
> > > that is the name that is used for the exact same purpose — computing 
> > > "implicit move" candidates — in `BuildReturnStmt`. Ideally this code 
> > > would be factored out so that it appeared in only one place; but until 
> > > then, gratuitous differences between the two sites should be minimized 
> > > IMO.
> > Hmm, you're completely right. What do you think about 
> > `ImplicitMoveCandidate`? Otherwise I'll stick with the current name, as you 
> > suggested.
> > 
> > > Ideally this code would be factored out so that it appeared in only one 
> > > place; but until then, gratuitous differences between the two sites 
> > > should be minimized IMO.
> > 
> > Isn't it already factored out? I let `getCopyElisionCandidate` do all the 
> > heavy lifting. (Except filtering out lvalue references...)
> > What do you think about `ImplicitMoveCandidate`?
> 
> I think that would be more correct in this case, but it wouldn't be parallel 
> with the one in `BuildReturnStmt`, and I'm not sure whether it would be 
> correct to change it over there too.
> 
> > Isn't it already factored out?
> 
> I see some parallelism in the two other places that call 
> `getCopyElisionCandidate` followed by `PerformMoveOrCopyInitialization`. 
> Maybe this is as factored-out as it can get, but it still looks remarkably 
> parallel. (And I wish `NRVOVariable` was named `NRVOCandidate` in the latter 
> case.)
> 
> In `BuildReturnStmt`:
> 
> if (RetValExp)
>   NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, 
> CES_Strict);
> if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
>   // we have a non-void function with an expression, continue

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

Re: [PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

r374688 - Revert r374663 "[clang-format] Proposal for clang-format to give compiler style warnings"

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

[PATCH] D68915: [clang][IFS] Escape mangled name in-order to not break llvm-ifs with names mangled using MS ABI

[PATCH] D68915: [clang][IFS] Escape mangled name in-order to not break llvm-ifs with names mangled using MS ABI

[PATCH] D64874: [Sema] Improve handling of function pointer conversions

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[PATCH] D68340: Add AIX toolchain and basic linker functionality

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

r374663 - [clang-format] Proposal for clang-format to give compiler style warnings

r374659 - remove an useless allocation found by scan-build - the new Dead nested assignment check

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

25 matches

Site Navigation

Mail list logo

Footer information