[PATCH] D73768: clang-format: [JS] document InsertTrailingCommas.

2020-02-02 Thread Martin Probst via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc04c54fc1f6: clang-format: [JS] document 
InsertTrailingCommas. (authored by mprobst).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73768

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -175,6 +175,11 @@
   }
 }
 
+- Option ``InsertTrailingCommas`` can be set to ``TCS_Wrapped`` to insert
+  trailing commas in container literals (arrays and objects) that wrap across
+  multiple lines. It is currently only available for JavaScript and disabled by
+  default (``TCS_None``).
+
 libclang
 
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -717,6 +717,26 @@
 aaa);
 }
 
+
+**InsertTrailingCommas** (``TrailingCommaStyle``) can be set to ``TCS_Wrapped``
+  to insert trailing commas in container literals (arrays and objects) that 
wrap
+  across multiple lines. It is currently only available for JavaScript and
+  disabled by default (``TCS_None``).
+
+  ``InsertTrailingCommas`` cannot be used together with ``BinPackArguments`` as
+  inserting the comma disables bin-packing.
+
+  .. code-block:: c++
+
+TSC_Wrapped:
+const someArray = [
+  aa,
+  aa,
+  aa,
+  //^ inserted
+]
+
+
 **BinPackParameters** (``bool``)
   If ``false``, a function declaration's or function definition's
   parameters will either all be on the same line or will have one line each.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -175,6 +175,11 @@
   }
 }
 
+- Option ``InsertTrailingCommas`` can be set to ``TCS_Wrapped`` to insert
+  trailing commas in container literals (arrays and objects) that wrap across
+  multiple lines. It is currently only available for JavaScript and disabled by
+  default (``TCS_None``).
+
 libclang
 
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -717,6 +717,26 @@
 aaa);
 }
 
+
+**InsertTrailingCommas** (``TrailingCommaStyle``) can be set to ``TCS_Wrapped``
+  to insert trailing commas in container literals (arrays and objects) that wrap
+  across multiple lines. It is currently only available for JavaScript and
+  disabled by default (``TCS_None``).
+
+  ``InsertTrailingCommas`` cannot be used together with ``BinPackArguments`` as
+  inserting the comma disables bin-packing.
+
+  .. code-block:: c++
+
+TSC_Wrapped:
+const someArray = [
+  aa,
+  aa,
+  aa,
+  //^ inserted
+]
+
+
 **BinPackParameters** (``bool``)
   If ``false``, a function declaration's or function definition's
   parameters will either all be on the same line or will have one line each.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] dc04c54 - clang-format: [JS] document InsertTrailingCommas.

2020-02-02 Thread Martin Probst via cfe-commits

Author: Martin Probst
Date: 2020-02-03T08:51:52+01:00
New Revision: dc04c54fc1f6660770040f9a17ea600ce95e4b60

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

LOG: clang-format: [JS] document InsertTrailingCommas.

Summary: In release notes and the regular docs.

Reviewers: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 981542451b29..0c515922e650 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -717,6 +717,26 @@ the configuration (without a prefix: ``Auto``).
 aaa);
 }
 
+
+**InsertTrailingCommas** (``TrailingCommaStyle``) can be set to ``TCS_Wrapped``
+  to insert trailing commas in container literals (arrays and objects) that 
wrap
+  across multiple lines. It is currently only available for JavaScript and
+  disabled by default (``TCS_None``).
+
+  ``InsertTrailingCommas`` cannot be used together with ``BinPackArguments`` as
+  inserting the comma disables bin-packing.
+
+  .. code-block:: c++
+
+TSC_Wrapped:
+const someArray = [
+  aa,
+  aa,
+  aa,
+  //^ inserted
+]
+
+
 **BinPackParameters** (``bool``)
   If ``false``, a function declaration's or function definition's
   parameters will either all be on the same line or will have one line each.

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index eae0dfb12151..cb8d73932bf6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -175,6 +175,11 @@ clang-format
   }
 }
 
+- Option ``InsertTrailingCommas`` can be set to ``TCS_Wrapped`` to insert
+  trailing commas in container literals (arrays and objects) that wrap across
+  multiple lines. It is currently only available for JavaScript and disabled by
+  default (``TCS_None``).
+
 libclang
 
 



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


[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-02-02 Thread Alexey Romanov via Phabricator via cfe-commits
alexeyr added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:363
+  ASTContext ) {
+  const auto OpKind = getOp(TheExpr);
+  // if there are no nested operators of the same kind, it's handled by

Eugene.Zelenko wrote:
> Please don't use auto when type is not spelled explicitly or iterator.
In this case the type will depend on `TExpr`: either `BinaryOperator::Opcode` 
or `OverloadedOperatorKind`. I could make it a template parameter (but it won't 
be deducible) or convert `OverloadedOperatorKind` to `Opcode`. Any preference?



Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:375
+  for (ast_type_traits::DynTypedNode Parent : Parents) {
+if (checkOpKind(Parent.get(), OpKind)) {
+  return false;

Eugene.Zelenko wrote:
> Please elide braces.
Should braces be elided from `for` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73775



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


[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

2020-02-02 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment.

Sorry for my english. Sometimes it's more difficult than bug fixing




Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:74
   if (ReplaceRange.getBegin().isMacroID() ||
-  ReplaceRange.getBegin() >= LastReplacementEnd) {
+  (Result.SourceManager->getFileID(ReplaceRange.getBegin()) != 
Result.SourceManager->getFileID(LastReplacementEnd)) ||
+  (ReplaceRange.getBegin() >= LastReplacementEnd)) {

aaron.ballman wrote:
> Be sure to run clang-format over the patch; this looks well beyond the usual 
> 80-col limit. You can also drop the unnecessary parens around the sub 
> expressions.
> 
> Also, a comment explaining why this code is needed would help future folks as 
> would a test case showing what this corrects.
I left the brackets because they seem to increase readability. Or not? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73090



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


[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

2020-02-02 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 241971.
f00kat marked an inline comment as done.
f00kat added a comment.

Clean up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73090

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I %S/Inputs/modernize-use-using/
 
 typedef int Type;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
@@ -267,3 +267,14 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:30: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: using R_t = struct { int a; };
 // CHECK-FIXES-NEXT: using R_p = R_t*;
+
+typedef enum { ea1, eb1 } EnumT1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using EnumT1 = enum { ea1, eb1 };
+
+#include "modernize-use-using.h"
+
+typedef enum { ea2, eb2 } EnumT2_CheckTypedefImpactFromAnotherFile;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using EnumT2_CheckTypedefImpactFromAnotherFile = enum { ea2, eb2 };
+
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
@@ -0,0 +1,6 @@
+#ifndef MODERNIZE_USE_USING_H
+#define MODERNIZE_USE_USING_H
+
+typedef int mytype;
+
+#endif
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,7 @@
 
   const bool IgnoreMacros;
   SourceLocation LastReplacementEnd;
-  SourceRange LastCxxDeclRange;
+  SourceRange LastTagDeclRange;
   std::string FirstTypedefType;
   std::string FirstTypedefName;
 
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -25,18 +25,17 @@
 return;
   Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
  this);
-  // This matcher used to find structs defined in source code within typedefs.
+  // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("struct"), this);
+  Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
-  const auto *MatchedCxxRecordDecl =
-  Result.Nodes.getNodeAs("struct");
-  if (MatchedCxxRecordDecl) {
-LastCxxDeclRange = MatchedCxxRecordDecl->getSourceRange();
+  const auto *MatchedTagDecl = Result.Nodes.getNodeAs("tagdecl");
+  if (MatchedTagDecl) {
+LastTagDeclRange = MatchedTagDecl->getSourceRange();
 return;
   }
 
@@ -70,9 +69,13 @@
   // consecutive TypedefDecl nodes whose SourceRanges overlap. Each range starts
   // at the "typedef" and then continues *across* previous definitions through
   // the end of the current TypedefDecl definition.
+  // But also we need to check that the ranges belong to the same file because
+  // different files may contain overlapping ranges.
   std::string Using = "using ";
   if (ReplaceRange.getBegin().isMacroID() ||
-  ReplaceRange.getBegin() >= LastReplacementEnd) {
+  (Result.SourceManager->getFileID(ReplaceRange.getBegin()) !=
+   Result.SourceManager->getFileID(LastReplacementEnd)) ||
+  (ReplaceRange.getBegin() >= LastReplacementEnd)) {
 // This is the first (and possibly the only) TypedefDecl in a typedef. Save
 // Type and Name in case we find subsequent TypedefDecl's in this typedef.
 FirstTypedefType = Type;
@@ -95,11 +98,12 @@
 
   auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
 
-  // If typedef contains a full struct/class declaration, extract its full text.
-  if 

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

2020-02-02 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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


[clang] 3ecba39 - [Driver][test] Change %itanium_abi_triple to generic ELF

2020-02-02 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-02-02T21:46:48-08:00
New Revision: 3ecba396e91ccfb6e066afcfd18bdc24597fe4dd

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

LOG: [Driver][test] Change %itanium_abi_triple to generic ELF

x86_64-windows and darwin default to PIC. They don't use PIE.

Added: 


Modified: 
clang/test/Driver/fsemantic-interposition.c

Removed: 




diff  --git a/clang/test/Driver/fsemantic-interposition.c 
b/clang/test/Driver/fsemantic-interposition.c
index 252f5fce6944..20bc2c6f7270 100644
--- a/clang/test/Driver/fsemantic-interposition.c
+++ b/clang/test/Driver/fsemantic-interposition.c
@@ -1,9 +1,9 @@
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fpic 
-fsemantic-interposition -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fPIC 
-fsemantic-interposition -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64 %s -Werror -fpic -fsemantic-interposition -c 
-### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64 %s -Werror -fPIC -fsemantic-interposition -c 
-### 2>&1 | FileCheck %s
 // CHECK: "-fsemantic-interposition"
 
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fPIC 
-fsemantic-interposition -fno-semantic-interposition -c -### 2>&1 | FileCheck 
--check-prefix=NO %s
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fno-PIC 
-fsemantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fPIC -c -### 2>&1 | 
FileCheck --check-prefix=NO %s
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fPIE 
-fsemantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
+// RUN: %clang -target x86_64 %s -Werror -fPIC -fsemantic-interposition 
-fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
+// RUN: %clang -target x86_64 %s -Werror -fsemantic-interposition -c -### 2>&1 
| FileCheck --check-prefix=NO %s
+// RUN: %clang -target x86_64 %s -Werror -fPIC -c -### 2>&1 | FileCheck 
--check-prefix=NO %s
+// RUN: %clang -target x86_64 %s -Werror -fPIE -fsemantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"



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


[clang] 7eeb901 - [Driver] Fix fsemantic-interposition.c for Windows and Darwin

2020-02-02 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-02-02T20:55:24-08:00
New Revision: 7eeb90152859f254fe4e9182c749ab065d33c89d

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

LOG: [Driver] Fix fsemantic-interposition.c for Windows and Darwin

Added: 


Modified: 
clang/test/Driver/fsemantic-interposition.c

Removed: 




diff  --git a/clang/test/Driver/fsemantic-interposition.c 
b/clang/test/Driver/fsemantic-interposition.c
index 484be679c232..252f5fce6944 100644
--- a/clang/test/Driver/fsemantic-interposition.c
+++ b/clang/test/Driver/fsemantic-interposition.c
@@ -3,7 +3,7 @@
 // CHECK: "-fsemantic-interposition"
 
 // RUN: %clang -target %itanium_abi_triple %s -Werror -fPIC 
-fsemantic-interposition -fno-semantic-interposition -c -### 2>&1 | FileCheck 
--check-prefix=NO %s
-// RUN: %clang -target %itanium_abi_triple %s -Werror -fsemantic-interposition 
-c -### 2>&1 | FileCheck --check-prefix=NO %s
+// RUN: %clang -target %itanium_abi_triple %s -Werror -fno-PIC 
-fsemantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target %itanium_abi_triple %s -Werror -fPIC -c -### 2>&1 | 
FileCheck --check-prefix=NO %s
 // RUN: %clang -target %itanium_abi_triple %s -Werror -fPIE 
-fsemantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"



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


[clang] aed488e - [Driver] Move -fsemantic-interposition decision from cc1 to driver

2020-02-02 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-02-02T20:45:29-08:00
New Revision: aed488e3a4dfdf15696521e13ac5e6ac2f9e7af7

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

LOG: [Driver] Move -fsemantic-interposition decision from cc1 to driver

And add test/Driver/fsemantic-interposition.c

Added: 
clang/test/Driver/fsemantic-interposition.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/semantic-interposition.c
clang/test/Driver/clang_f_opts.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c8195f0f4ccf..433fdded5b7a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4384,6 +4384,11 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 CmdArgs.push_back(A->getValue());
   }
 
+  if (Args.hasFlag(options::OPT_fsemantic_interposition,
+   options::OPT_fno_semantic_interposition, false) &&
+  RelocationModel != llvm::Reloc::Static && !IsPIE)
+CmdArgs.push_back("-fsemantic-interposition");
+
   CmdArgs.push_back("-mthread-model");
   if (Arg *A = Args.getLastArg(options::OPT_mthread_model)) {
 if (!TC.isThreadModelSupported(A->getValue()))
@@ -5036,10 +5041,6 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   options::OPT_fno_emulated_tls);
   Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
 
-  if (Args.hasFlag(options::OPT_fsemantic_interposition,
-   options::OPT_fno_semantic_interposition, false))
-CmdArgs.push_back("-fsemantic-interposition");
-
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa(JA) || Output.getType() != types::TY_PP_Asm)
 Args.AddLastArg(CmdArgs, options::OPT_fzvector);

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f3015d0f7085..a71145202052 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3031,9 +3031,7 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
   Opts.setDefaultCallingConv(DefaultCC);
   }
 
-  // -fsemantic-interposition
-  Opts.SemanticInterposition =
-  Args.hasArg(OPT_fsemantic_interposition) && Opts.PICLevel && !Opts.PIE;
+  Opts.SemanticInterposition = Args.hasArg(OPT_fsemantic_interposition);
 
   // -mrtd option
   if (Arg *A = Args.getLastArg(OPT_mrtd)) {

diff  --git a/clang/test/CodeGen/semantic-interposition.c 
b/clang/test/CodeGen/semantic-interposition.c
index 77df61566e07..43656e36021f 100644
--- a/clang/test/CodeGen/semantic-interposition.c
+++ b/clang/test/CodeGen/semantic-interposition.c
@@ -1,14 +1,5 @@
-// Semantic Interposition is active if
-//  -fsemantic-interposition is set,
-// - pic-level > 0
-// - pic-is-pie is not set
+// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition %s -o - | FileCheck 
--check-prefix=INTERPOSITION %s
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck --check-prefix=NO %s
 
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 0 %s -o - | 
FileCheck %s -check-prefix=CHECK-NO-INTERPOSITION
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 1 %s -o - | 
FileCheck %s -check-prefix=CHECK-INTERPOSITION
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 2 %s -o - | 
FileCheck %s -check-prefix=CHECK-INTERPOSITION
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 0 %s -o - | 
FileCheck %s -check-prefix=CHECK-NO-INTERPOSITION
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 1 
-pic-is-pie %s -o - | FileCheck %s -check-prefix=CHECK-NO-INTERPOSITION
-// RUN: %clang_cc1 -emit-llvm -fsemantic-interposition -pic-level 2 
-pic-is-pie %s -o - | FileCheck %s -check-prefix=CHECK-NO-INTERPOSITION
-
-// CHECK-NO-INTERPOSITION-NOT: "SemanticInterposition"
-// CHECK-INTERPOSITION: !{{[0-9]+}} = !{i32 1, !"SemanticInterposition", i32 1}
+// INTERPOSITION: !{{[0-9]+}} = !{i32 1, !"SemanticInterposition", i32 1}
+// NO-NOT: "SemanticInterposition"

diff  --git a/clang/test/Driver/clang_f_opts.c 
b/clang/test/Driver/clang_f_opts.c
index e3fe373d7198..970b4e934e78 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -251,8 +251,6 @@
 // RUN: -fexec-charset=UTF-8 \
 // RUN: -fivopts -fno-ivopts  \
 // RUN: -fnon-call-exceptions -fno-non-call-exceptions\
-// RUN: -fno-semantic-interposition   \
-// RUN: -fsemantic-interposition  \
 // RUN: -fpermissive 

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

2020-02-02 Thread Caleb Helbling via Phabricator via cfe-commits
calebh added a comment.

What about unary operations? I don't see those supported in the current version 
of libclang Python either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10833



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


[PATCH] D73856: [docu] Improve docu of misc-misplaced-const

2020-02-02 Thread Alexander Lanin via Phabricator via cfe-commits
AlexanderLanin created this revision.
AlexanderLanin added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

East const makes the problem more obvious. With west const people were telling 
me the const is on the wrong side.

Also it's time to use using instead of typedef.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73856

Files:
  clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst


Index: clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
@@ -13,10 +13,9 @@
 
 .. code-block:: c++
 
-  typedef int *int_ptr;
-  void f(const int_ptr ptr) {
+  using int_ptr = int*;
+  void f(int_ptr const ptr) {
 *ptr = 0; // potentially quite unexpectedly the int can be modified here
-ptr = 0; // does not compile
   }
 
 The check does not diagnose when the underlying ``typedef``/``using`` type is a


Index: clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst
@@ -13,10 +13,9 @@
 
 .. code-block:: c++
 
-  typedef int *int_ptr;
-  void f(const int_ptr ptr) {
+  using int_ptr = int*;
+  void f(int_ptr const ptr) {
 *ptr = 0; // potentially quite unexpectedly the int can be modified here
-ptr = 0; // does not compile
   }
 
 The check does not diagnose when the underlying ``typedef``/``using`` type is a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] d24d8af - Fixed typo in CTE release notes failing build

2020-02-02 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-02-02T21:51:34Z
New Revision: d24d8af320fd8977f085ee5ceafadaad98f76bf0

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

LOG: Fixed typo in CTE release notes failing build

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 4e0f79dfd45c..2c61591bdc34 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -105,7 +105,7 @@ Changes in existing checks
 ^^
 
 - Improved :doc:`readability-qualified-auto
-  ` check now supports a 
+  ` check now supports a 
   `AddConstToQualified` to enable adding ``const`` qualifiers to variables
   typed with ``auto *`` and ``auto &``.
 



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


[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! I'm happy to commit on your behalf once the minor nits are fixed up.




Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:73
+  // But also we need to check that ranges belongs the same file because
+  // different files may contain overlap ranges.
   std::string Using = "using ";

that ranges -> that the ranges
belongs -> belong to
overlap -> overlapping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73090



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


[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262
+  << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
+  << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const ");
+}

njames93 wrote:
> aaron.ballman wrote:
> > Not specific to this patch, so feel free to address in another one -- I 
> > think you should be passing `Var` instead of `Var->getName()` -- the 
> > diagnostics engine does magic to format the name properly for any 
> > `NamedDecl`, including proper quoting. (Similar applies elsewhere.)
> I couldn't do that as it actually messes the quotes up: `'auto 
> &'TdNakedCRef''`
Ah, good point. I was thinking about variable template specializations, but I'm 
not certain there's an actual issue with them -- it seems like we really want 
just the base identifier here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548



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


[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
njames93 marked an inline comment as done.
Closed by commit rG8a68c40a1bf2: [clang-tidy] Added option for disabling const 
qualifiers in readability… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+  // CHECK-FIXES: {{^}}  auto *NakedPtr = getIntPtr();
+  auto NakedConstPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+  // CHECK-FIXES: {{^}}  const auto *NakedConstPtr = getCIntPtr();
+  auto *Ptr = getIntPtr();
+  auto *ConstPtr = getCIntPtr();
+  auto  = *getIntPtr();
+  auto  = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
 readability-qualified-auto
 ==
 
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to 
+pointers.
 
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or 
-constant reference. This check will transform ``auto`` to ``auto *`` when the 
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This 
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
 
 .. code-block:: c++
 
-  for (auto  : MutatableContainer) {
-change(Data);
-  }
-  for (auto  : ConstantContainer) {
-observe(Data);
-  }
   for (auto Data : MutatablePtrContainer) {
 change(*Data);
   }
@@ -31,12 +24,6 @@
 
 .. code-block:: c++
 
-  for (auto  : MutatableContainer) {
-change(Data);
-  }
-  for (const auto  : ConstantContainer) {
-observe(Data);
-  }
   for (auto *Data : MutatablePtrContainer) {
 change(*Data);
   }
@@ -44,21 +31,54 @@
 observe(*Data);
   }
 
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and 
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
 
 .. code-block:: c++
 
-  const auto Foo = cast(Baz1);
-  const auto Bar = cast(Baz2);
-  volatile auto FooBar = cast(Baz3);
+   const auto Foo = cast(Baz1);
+   const auto Bar = cast(Baz2);
+   volatile auto FooBar = cast(Baz3);
+   auto BarFoo = cast(Baz4);
 
 Would be transformed into:
 
 .. code-block:: c++
 
-  auto *const Foo = cast(Baz1);
-  const auto *const Bar = cast(Baz2);
-  auto *volatile FooBar = cast(Baz3);
+   auto *const Foo = cast(Baz1);
+   const auto *const Bar = cast(Baz2);
+   auto *volatile FooBar = cast(Baz3);
+   auto *BarFoo = cast(Baz4);
+
+Options
+---
+
+.. option:: AddConstToQualified
+   
+   When set to `1` the check will add const qualifiers variables defined as
+   ``auto *`` or ``auto &`` when applicable.
+   Default value is '1'.
+
+.. code-block:: c++
+
+   auto Foo1 = cast(Bar1);
+   auto *Foo2 = cast(Bar2);
+   auto  = cast(Bar3);
+
+   If AddConstToQualified is set to `0`,  it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast(Bar1);
+   auto *Foo2 = cast(Bar2);
+   auto  = cast(Bar3);
+
+   Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast(Bar1);
+   const auto *Foo2 

[clang-tools-extra] 8a68c40 - [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-02-02T21:27:25Z
New Revision: 8a68c40a1bf256523993ee97b39f79001eaade91

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

LOG: [clang-tidy] Added option for disabling const qualifiers in 
readability-qualified-auto

Summary: Adds an option called `AddConstToQualified` to 
readability-qualified-auto to toggle adding const to the auto typed pointers 
and references. By default its enabled but in the LLVM module its disabled.

Reviewers: aaron.ballman, alexfh, JonasToth, hokein, sammccall

Reviewed By: aaron.ballman

Subscribers: Quuxplusone, merge_guards_bot, lebedev.ri, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp

Modified: 
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp 
b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
index 5ae927c2cf5a..2aaf07639267 100644
--- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -36,6 +36,12 @@ class LLVMModule : public ClangTidyModule {
 "llvm-qualified-auto");
 CheckFactories.registerCheck("llvm-twine-local");
   }
+
+  ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+Options.CheckOptions["llvm-qualified-auto.AddConstToQualified"] = "0";
+return Options;
+  }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.

diff  --git a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
index 1d0b85b9c4ce..79885dbe4b43 100644
--- a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
@@ -102,6 +102,10 @@ bool isAutoPointerConst(QualType QType) {
 
 } // namespace
 
+void QualifiedAutoCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "AddConstToQualified", AddConstToQualified);
+}
+
 void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
 return; // Auto deduction not used in 'C or C++03 and earlier', so don't
@@ -142,6 +146,8 @@ void QualifiedAutoCheck::registerMatchers(MatchFinder 
*Finder) {
   hasAnyTemplateArgument(IsBoundToType),
   "auto"),
   this);
+  if (!AddConstToQualified)
+return;
   Finder->addMatcher(ExplicitSingleVarDecl(
  hasType(pointerType(pointee(autoType(, 
"auto_ptr"),
  this);
@@ -177,11 +183,9 @@ void QualifiedAutoCheck::check(const 
MatchFinder::MatchResult ) {
 bool IsLocalVolatile = Var->getType().isLocalVolatileQualified();
 bool IsLocalRestrict = Var->getType().isLocalRestrictQualified();
 
-if (CheckQualifier(IsLocalConst, Qualifier::Const))
-  return;
-if (CheckQualifier(IsLocalVolatile, Qualifier::Volatile))
-  return;
-if (CheckQualifier(IsLocalRestrict, Qualifier::Restrict))
+if (CheckQualifier(IsLocalConst, Qualifier::Const) ||
+CheckQualifier(IsLocalVolatile, Qualifier::Volatile) ||
+CheckQualifier(IsLocalRestrict, Qualifier::Restrict))
   return;
 
 // Check for bridging the gap between the asterisk and name.
@@ -214,8 +218,7 @@ void QualifiedAutoCheck::check(const 
MatchFinder::MatchResult ) {
 << (IsLocalRestrict ? "__restrict " : "") << Var->getName() << ReplStr;
 
 for (SourceRange  : RemoveQualifiersRange) {
-  Diag << FixItHint::CreateRemoval(
-  CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()));
+  Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range));
 }
 
 Diag << FixItHint::CreateReplacement(FixItRange, ReplStr);
@@ -247,22 +250,17 @@ void QualifiedAutoCheck::check(const 
MatchFinder::MatchResult ) {
 return;
 }
 
-CharSourceRange FixItRange;
 if (llvm::Optional TypeSpec =
 getTypeSpecifierLocation(Var, Result)) {
-  FixItRange = CharSourceRange::getCharRange(*TypeSpec);
-  if (FixItRange.isInvalid())
+  if (TypeSpec->isInvalid() || TypeSpec->getBegin().isMacroID() ||
+  TypeSpec->getEnd().isMacroID())
 return;
-} else
-  return;
-
-DiagnosticBuilder Diag =
-diag(FixItRange.getBegin(),
- "'auto *%0%1%2' can be 

[PATCH] D73843: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

2020-02-02 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGefcd09cea9a5: [clang-tidy] Fix false positive for 
cppcoreguidelines-init-variables (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73843

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -78,3 +78,9 @@
   int parens(42);
   int braces{42};
 }
+
+template 
+void f(RANGE r) {
+  for (char c : r) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -29,11 +29,15 @@
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(varDecl(unless(hasInitializer(anything())),
- unless(isInstantiated()), isLocalVarDecl(),
- unless(isStaticLocal()), isDefinition())
- .bind("vardecl"),
- this);
+  std::string BadDecl = "badDecl";
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
+  isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  optionally(hasParent(declStmt(hasParent(
+  
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))
+  .bind("vardecl"),
+  this);
 }
 
 void InitVariablesCheck::registerPPCallbacks(const SourceManager ,


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -78,3 +78,9 @@
   int parens(42);
   int braces{42};
 }
+
+template 
+void f(RANGE r) {
+  for (char c : r) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -29,11 +29,15 @@
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(varDecl(unless(hasInitializer(anything())),
- unless(isInstantiated()), isLocalVarDecl(),
- unless(isStaticLocal()), isDefinition())
- .bind("vardecl"),
- this);
+  std::string BadDecl = "badDecl";
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
+  isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  optionally(hasParent(declStmt(hasParent(
+  cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))
+  .bind("vardecl"),
+  this);
 }
 
 void InitVariablesCheck::registerPPCallbacks(const SourceManager ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] efcd09c - [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

2020-02-02 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-02-02T21:26:29Z
New Revision: efcd09cea9a51c522954aa24e4b5513266daf6c3

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

LOG: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

Summary: Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=44746 | False positive 
for cppcoreguidelines-init-variables in range based for loop in template 
function ]]

Reviewers: aaron.ballman, alexfh, hokein, JonasToth, gribozavr2

Reviewed By: aaron.ballman

Subscribers: merge_guards_bot, xazax.hun, wuzish, nemanjai, kbarton, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
index 8a628317a302..a5d9275afaf7 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -29,11 +29,15 @@ InitVariablesCheck::InitVariablesCheck(StringRef Name,
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(varDecl(unless(hasInitializer(anything())),
- unless(isInstantiated()), isLocalVarDecl(),
- unless(isStaticLocal()), isDefinition())
- .bind("vardecl"),
- this);
+  std::string BadDecl = "badDecl";
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
+  isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  optionally(hasParent(declStmt(hasParent(
+  
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
+  unless(equalsBoundNode(BadDecl)))
+  .bind("vardecl"),
+  this);
 }
 
 void InitVariablesCheck::registerPPCallbacks(const SourceManager ,

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
index d43e44808a49..4c92e1976f91 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -78,3 +78,9 @@ void init_unit_tests() {
   int parens(42);
   int braces{42};
 }
+
+template 
+void f(RANGE r) {
+  for (char c : r) {
+  }
+}



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


[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done.
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262
+  << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
+  << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const ");
+}

aaron.ballman wrote:
> Not specific to this patch, so feel free to address in another one -- I think 
> you should be passing `Var` instead of `Var->getName()` -- the diagnostics 
> engine does magic to format the name properly for any `NamedDecl`, including 
> proper quoting. (Similar applies elsewhere.)
I couldn't do that as it actually messes the quotes up: `'auto &'TdNakedCRef''`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548



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


[PATCH] D73843: [clang-tidy] Fix false positive for cppcoreguidelines-init-variables

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73843



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


[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

2020-02-02 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 241931.
f00kat added a comment.

1. clang-format
2. Add comment for additional "files IDs equals" check
3. Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73090

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I %S/Inputs/modernize-use-using/
 
 typedef int Type;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
@@ -267,3 +267,14 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:30: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: using R_t = struct { int a; };
 // CHECK-FIXES-NEXT: using R_p = R_t*;
+
+typedef enum { ea1, eb1 } EnumT1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using EnumT1 = enum { ea1, eb1 };
+
+#include "modernize-use-using.h"
+
+typedef enum { ea2, eb2 } EnumT2_CheckTypedefImpactFromAnotherFile;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using EnumT2_CheckTypedefImpactFromAnotherFile = enum { ea2, eb2 };
+
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-use-using/modernize-use-using.h
@@ -0,0 +1,6 @@
+#ifndef MODERNIZE_USE_USING_H
+#define MODERNIZE_USE_USING_H
+
+typedef int mytype;
+
+#endif
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,7 @@
 
   const bool IgnoreMacros;
   SourceLocation LastReplacementEnd;
-  SourceRange LastCxxDeclRange;
+  SourceRange LastTagDeclRange;
   std::string FirstTypedefType;
   std::string FirstTypedefName;
 
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -25,18 +25,17 @@
 return;
   Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
  this);
-  // This matcher used to find structs defined in source code within typedefs.
+  // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("struct"), this);
+  Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
-  const auto *MatchedCxxRecordDecl =
-  Result.Nodes.getNodeAs("struct");
-  if (MatchedCxxRecordDecl) {
-LastCxxDeclRange = MatchedCxxRecordDecl->getSourceRange();
+  const auto *MatchedTagDecl = Result.Nodes.getNodeAs("tagdecl");
+  if (MatchedTagDecl) {
+LastTagDeclRange = MatchedTagDecl->getSourceRange();
 return;
   }
 
@@ -70,9 +69,13 @@
   // consecutive TypedefDecl nodes whose SourceRanges overlap. Each range starts
   // at the "typedef" and then continues *across* previous definitions through
   // the end of the current TypedefDecl definition.
+  // But also we need to check that ranges belongs the same file because
+  // different files may contain overlap ranges.
   std::string Using = "using ";
   if (ReplaceRange.getBegin().isMacroID() ||
-  ReplaceRange.getBegin() >= LastReplacementEnd) {
+  (Result.SourceManager->getFileID(ReplaceRange.getBegin()) !=
+   Result.SourceManager->getFileID(LastReplacementEnd)) ||
+  (ReplaceRange.getBegin() >= LastReplacementEnd)) {
 // This is the first (and possibly the only) TypedefDecl in a typedef. Save
 // Type and Name in case we find subsequent TypedefDecl's in this typedef.
 FirstTypedefType = Type;
@@ -95,11 +98,12 @@
 
   auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
 
-  // If typedef contains a full struct/class declaration, extract its 

[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262
+  << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
+  << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const ");
+}

Not specific to this patch, so feel free to address in another one -- I think 
you should be passing `Var` instead of `Var->getName()` -- the diagnostics 
engine does magic to format the name properly for any `NamedDecl`, including 
proper quoting. (Similar applies elsewhere.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548



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


[PATCH] D31343: Add an attribute plugin example

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/examples/Attribute/Attribute.cpp:25
+struct ExampleAttrInfo : public ParsedAttrInfo {
+  ExampleAttrInfo() {
+// Set the kind to NoSemaHandlerAttribute to make sure clang doesn't assume

It would be really handy for the example to show how to list subjects for the 
declaration attribute, as that's a very common need.



Comment at: clang/examples/Attribute/Attribute.cpp:29
+AttrKind = ParsedAttr::NoSemaHandlerAttribute;
+// Can take an optional string argument.
+OptArgs = 1;

Nothing here suggests the argument must be a string. It would be ideal to show 
users how to list out arguments and their types.



Comment at: clang/examples/Attribute/Attribute.cpp:32-33
+// GNU-style __attribute__(("example")) and C++-style [[example]] 
supported.
+Spellings.push_back({ParsedAttr::AS_GNU, "example"});
+Spellings.push_back({ParsedAttr::AS_CXX11, "::example"});
+  }

Showing how to have a vendor namespace would also be especially helpful.

As a point of design, I wonder if we should be documenting (or diagnosing?) 
attributes from plugins that are added to the clang vendor namespace, as that 
does not seem like something we want to encourage?



Comment at: clang/examples/Attribute/Attribute.cpp:35
+  }
+  virtual bool handleDeclAttribute(Sema , Decl *D,
+   const ParsedAttr ) const {

It is unclear to me how a user would add an attribute that needs to follow the 
merge pattern we use in `mergeDeclAttribute()`.



Comment at: clang/examples/Attribute/Attribute.cpp:53
+  } else {
+S.Diag(ArgExpr->getExprLoc(), diag::err_attribute_argument_type)
+<< Attr.getAttrName() << AANT_ArgumentString;

Do we have a way for plugin authors to introduce their own diagnostics, or are 
they limited to just what diagnostics we already provide?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D31343



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


[PATCH] D31337: Use virtual functions in ParsedAttrInfo instead of function pointers

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/ParsedAttr.cpp:141
 
+static ParsedAttrInfo DefaultParsedAttrInfo;
 static const ParsedAttrInfo (const ParsedAttr ) {

Might as well lower this variable into the function -- no real need for it to 
have file scope.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3400
 
-static std::string GenerateAppertainsTo(const Record , raw_ostream ) {
+static void GenerateAppertainsTo(const Record , raw_ostream ,
+ raw_ostream ) {

erichkeane wrote:
> john.brawn wrote:
> > erichkeane wrote:
> > > Why does this take SS AND OS.  What is the difference here?  Does this 
> > > actually use OS anymore?
> > It's because of GenerateCustomAppertainsTo.  That generates a function that 
> > expects to be at file scope (because emitAttributeMatchRules re-uses the 
> > same function), but at the time GenerateAppertainsTo is called SS is in the 
> > middle of the ParsedAttrInfo. Previously both the function that 
> > GenerateCustomAppertainsTo generates and that this file generates would be 
> > at file scope, so both were written to OS.
> Thanks!
I think the stream parameter names should be a bit more descriptive here (and 
perhaps some comments on the function would be a good idea as well). Perhaps 
`FileScopeStream` and `LexicalScopeStream`?


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

https://reviews.llvm.org/D31337



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


[PATCH] D31342: Add ParsedAttrInfo::handleDeclAttribute

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:74
+
+  class ExampleAttrInfo : public ParsedAttrInfo {
+  public:

We should be documenting the fields of `ParsedAttrInfo` that the user would be 
interacting with. We should also consider whether we are introducing a new 
stability guarantee with that interface and document accordingly. I think we 
should not promise that this structure will be stable from release to release, 
but it will be stable within a given release (e.g., the type can change between 
Clang 11 -> Clang 12, but the type will not change between Clang 11 -> Clang 
11.0.1).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6701-6702
 
+  if (AL.getInfo().handleDeclAttribute(S, D, AL))
+return;
+

I think this might make more logical sense as part of the `default` label in 
the `switch` statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D31342



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


[PATCH] D31338: Move ParsedAttrInfos into a registry and point to one in ParsedAttr

2020-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/ParsedAttr.h:45
+struct ParsedAttrInfo {
+  /// Corresponds to the Kind enum
+  unsigned AttrKind : 16;

Please add a full stop to the end of all the comments (here and elsewhere).



Comment at: clang/include/clang/Sema/ParsedAttr.h:63
+  unsigned IsSupportedByPragmaAttribute : 1;
+  // The syntaxes supported by this attribute and how they're spelt
+  struct Spelling {

spelt -> spelled



Comment at: clang/include/clang/Sema/ParsedAttr.h:615
   AttributeCommonInfo::Kind getKind() const { return getParsedKind(); }
+  ParsedAttrInfo () const { return *Info; }
 };

I think this should return a `const ParsedAttrInfo&` so that callers don't try 
to mutate it.



Comment at: clang/lib/Basic/Attributes.cpp:101-103
+  for (ParsedAttrInfoRegistry::iterator it = ParsedAttrInfoRegistry::begin(),
+ie = ParsedAttrInfoRegistry::end();
+   it != ie; ++it) {

Range-based for loop? Also, `it` and `ie` don't meet the usual naming 
conventions.

One thing I'm not keen on with this is the performance hit. We spent a decent 
amount of effort making this lookup fast and it's now a linear search through 
all of the attributes and requires memory allocations and deallocations to 
perform the search.



Comment at: clang/lib/Basic/Attributes.cpp:113
+  // If we failed to find a match then the attribute is unknown
+  return std::make_unique();
+}

This seems surprising because it makes it harder to determine whether you have 
a valid result from this function or not. I think this should return a null 
`unique_ptr`.



Comment at: clang/lib/Basic/Attributes.cpp:117
+std::unique_ptr
+ParsedAttrInfo::get(AttributeCommonInfo::Kind K) {
+  // Search for a ParsedAttrInfo whose kind matches

Same concerns in this function as above.


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

https://reviews.llvm.org/D31338



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


[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

2020-02-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

Another side effect is that rename_check.py messes list.rst by removing "Offer 
fixes". Probably proper solution would be moving update_checks_list from 
add_new_check.py to shared module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73580



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


[PATCH] D73786: [ARM,MVE] Fix vreinterpretq in big-endian mode.

2020-02-02 Thread Dave Green via Phabricator via cfe-commits
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Big endian making things fun again.

VECTOR_REG_CAST looks useful for lowering too. LGTM




Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:3958
 
 // Occasionally we need to cast between a i32 and a boolean vector, for
 // example when moving between rGPR and VPR.P0 as part of predicate vector

Please add a comment, similar to this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73786



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


[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

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

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

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

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


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548



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


[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

2020-02-02 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 241924.
njames93 added a comment.

- Small reformat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73548

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+  // CHECK-FIXES: {{^}}  auto *NakedPtr = getIntPtr();
+  auto NakedConstPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+  // CHECK-FIXES: {{^}}  const auto *NakedConstPtr = getCIntPtr();
+  auto *Ptr = getIntPtr();
+  auto *ConstPtr = getCIntPtr();
+  auto  = *getIntPtr();
+  auto  = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
 readability-qualified-auto
 ==
 
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to 
+pointers.
 
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or 
-constant reference. This check will transform ``auto`` to ``auto *`` when the 
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This 
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
 
 .. code-block:: c++
 
-  for (auto  : MutatableContainer) {
-change(Data);
-  }
-  for (auto  : ConstantContainer) {
-observe(Data);
-  }
   for (auto Data : MutatablePtrContainer) {
 change(*Data);
   }
@@ -31,12 +24,6 @@
 
 .. code-block:: c++
 
-  for (auto  : MutatableContainer) {
-change(Data);
-  }
-  for (const auto  : ConstantContainer) {
-observe(Data);
-  }
   for (auto *Data : MutatablePtrContainer) {
 change(*Data);
   }
@@ -44,21 +31,54 @@
 observe(*Data);
   }
 
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and 
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
 
 .. code-block:: c++
 
-  const auto Foo = cast(Baz1);
-  const auto Bar = cast(Baz2);
-  volatile auto FooBar = cast(Baz3);
+   const auto Foo = cast(Baz1);
+   const auto Bar = cast(Baz2);
+   volatile auto FooBar = cast(Baz3);
+   auto BarFoo = cast(Baz4);
 
 Would be transformed into:
 
 .. code-block:: c++
 
-  auto *const Foo = cast(Baz1);
-  const auto *const Bar = cast(Baz2);
-  auto *volatile FooBar = cast(Baz3);
+   auto *const Foo = cast(Baz1);
+   const auto *const Bar = cast(Baz2);
+   auto *volatile FooBar = cast(Baz3);
+   auto *BarFoo = cast(Baz4);
+
+Options
+---
+
+.. option:: AddConstToQualified
+   
+   When set to `1` the check will add const qualifiers variables defined as
+   ``auto *`` or ``auto &`` when applicable.
+   Default value is '1'.
+
+.. code-block:: c++
+
+   auto Foo1 = cast(Bar1);
+   auto *Foo2 = cast(Bar2);
+   auto  = cast(Bar3);
+
+   If AddConstToQualified is set to `0`,  it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast(Bar1);
+   auto *Foo2 = cast(Bar2);
+   auto  = cast(Bar3);
+
+   Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast(Bar1);
+   const auto *Foo2 = cast(Bar2);
+   const auto  = cast(Bar3);
 
-This check helps to enforce this `LLVM Coding Standards recommendation

[clang] a9ab01a - Remove superfluous space from -Wrange-loop-construct message

2020-02-02 Thread Aaron Puchert via cfe-commits

Author: Aaron Puchert
Date: 2020-02-02T16:22:58+01:00
New Revision: a9ab01a330f4c7c316fa87c8446888c73dae5536

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

LOG: Remove superfluous space from -Wrange-loop-construct message

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8d2aacce2eb8..d893289022d2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2383,7 +2383,7 @@ def note_for_range_begin_end : Note<
 def warn_for_range_const_reference_copy : Warning<
   "loop variable %0 "
   "%
diff {has type $ but is initialized with type $"
-  "| is initialized with a value of a 
diff erent type}1,2 resulting in a copy">,
+  "|is initialized with a value of a 
diff erent type}1,2 resulting in a copy">,
   InGroup, DefaultIgnore;
 def note_use_type_or_non_reference : Note<
   "use non-reference type %0 to keep the copy or type %1 to prevent copying">;



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


[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

2020-02-02 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

I've been thinking about the warning messages, which seem to a bit inaccurate. 
Sorry for piggy-backing this onto the change, I hope you don't mind.




Comment at: clang/lib/Sema/SemaStmt.cpp:2758-2759
 // the correct time to bind a const reference.
 SemaRef.Diag(VD->getLocation(), diag::warn_for_range_const_reference_copy)
 << VD << VariableType << E->getType();
 QualType NonReferenceType = VariableType.getNonReferenceType();

The message here says: "loop variable A has type B but is initialized with type 
C resulting in a copy".

That's not necessarily true, we just know (I think) that a constructor is 
called. The constructor might copy, but it might also not. However, the 
constructed object is bound to a reference, which is potentially misleading. 
One writes `const X& x : range` and `x` doesn't bind to the return value of 
`*begin(range)`, but to a temporary constructed from that.

Perhaps a more accurate message would be: "loop variable A of type B binds to a 
temporary constructed from type C" with the appropriate types.



Comment at: clang/lib/Sema/SemaStmt.cpp:2772-2773
 // semantic of the code.
 SemaRef.Diag(VD->getLocation(), diag::warn_for_range_variable_always_copy)
 << VD << RangeInitType;
 QualType NonReferenceType = VariableType.getNonReferenceType();

The message here is: "loop variable A is always a **copy** because the range of 
type B does not return a reference". This is somewhat misleading, some ranges 
create objects on-the-fly, in which case these aren't copies. (Assuming RVO the 
copy constructor might have never been called.)

I would suggest to rephrase this as something like "loop variable A binds to a 
temporary value produced by a range of type B".



Comment at: clang/lib/Sema/SemaStmt.cpp:2823-2824
   // if doing so will prevent a copy.
   SemaRef.Diag(VD->getLocation(), diag::warn_for_range_copy)
   << VD << VariableType << InitExpr->getType();
   SemaRef.Diag(VD->getBeginLoc(), diag::note_use_reference_type)

Given that `VariableType == VD->getType()` and  `InitExpr->getType() == 
VD->getInit()->getType()`, can these two types ever be different other than 
cv-qualifiers? We only call this function if `VariableType` is not a reference 
type.

Which begs the question: which value provides C in "loop variable A of type B 
creates a copy from type C"? I would suggest to omit the second type and write: 
"loop variable A creates a copy of type B", perhaps with stripping const from B.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007



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


[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

2020-02-02 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd591bdce6d62: [clang-tidy] Fixed crash 44745 in 
readability-else-after-return (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73841

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' 
[readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;


Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' [readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] d591bdc - [clang-tidy] Fixed crash 44745 in readability-else-after-return

2020-02-02 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-02-02T14:04:46Z
New Revision: d591bdce6d623208d4aeb335a762d839f0f8f0f7

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

LOG: [clang-tidy] Fixed crash 44745 in readability-else-after-return

Summary: Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=44745 | 
readability-else-after-return crashes ]]

Reviewers: aaron.ballman, alexfh, hokein, JonasToth, gribozavr2

Reviewed By: alexfh

Subscribers: merge_guards_bot, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index 93b197a52a52..b3b4e0de2bf0 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -27,6 +27,8 @@ static const char WarningMessage[] = "do not use 'else' after 
'%0'";
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 
 const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (DeclRef->getDecl()->getID() == DeclIdentifier) {
   return DeclRef;
@@ -44,6 +46,8 @@ const DeclRefExpr *findUsage(const Stmt *Node, int64_t 
DeclIdentifier) {
 const DeclRefExpr *
 findUsageRange(const Stmt *Node,
const llvm::iterator_range ) {
+  if (!Node)
+return nullptr;
   if (const auto *DeclRef = dyn_cast(Node)) {
 if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
   return DeclRef;

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
index 69ac9eb29571..1e3b4cf5755a 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -213,3 +213,16 @@ int lifeTimeExtensionTests(int a) {
 return b;
   }
 }
+
+void test_B44745() {
+  // This is the actual minimum test case for the crash in bug 44745. We aren't
+  // too worried about the warning or fix here, more we don't want a crash.
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: do not use 'else' after 'return' 
[readability-else-after-return]
+  if (auto X = false) {
+return;
+  } else {
+for (;;) {
+}
+  }
+  return;
+}



___
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

2020-02-02 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

In D68923#1853303 , @aaron.ballman 
wrote:

> In D68923#1853302 , @aaronpuchert 
> wrote:
>
> > Thanks for the reviews! Do you think it makes sense to bring this to Clang 
> > 10?
>
>
> I think it's a simple enough fix that it may be worth it, but it isn't fixing 
> a regression in behavior so it's not critical.


Exactly, it would just be a bug fix.

> If it helps you out to move it onto the 10 branch, I think it's fine.

I might be porting this back anyways on downstream, but I don't want to be 
selfish.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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


[PATCH] D73845: [Gnu toolchain] Look at standard GCC multilib/multiarch paths by default

2020-02-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

1. Please clang-format your changes
2. Hurd stuff is not a NFC refactoring, but adding some new feature - tests 
missing, i think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D73845



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


[PATCH] D73453: Preserve -nostdinc and --sysroot when calling query driver

2020-02-02 Thread Tobias Pisani via Phabricator via cfe-commits
topisani updated this revision to Diff 241914.

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

https://reviews.llvm.org/D73453

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test

Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,8 +5,11 @@
 
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.
-# RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh
+# RUN: echo '#!/bin/sh' >> %t.dir/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/my_driver.sh" ] || exit' >> %t.dir/my_driver.sh
+# RUN: echo '[ -z "${args##"-nostdinc"}" ]  || exit' >> %t.dir/my_driver.sh
+# RUN: echo '[ -z "${args##"-isysroot=/isysroot"}" ] || exit' >> %t.dir/my_driver.sh
+# RUN: echo 'echo " $* " | grep " --sysroot /my/sysroot/path " || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
@@ -22,7 +25,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/my_driver.sh the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -85,9 +85,10 @@
   return SystemIncludes;
 }
 
-std::vector extractSystemIncludes(PathRef Driver,
-   llvm::StringRef Lang,
-   llvm::Regex ) {
+std::vector
+extractSystemIncludes(PathRef Driver, llvm::StringRef Lang,
+  llvm::ArrayRef CommandLine,
+  llvm::Regex ) {
   trace::Span Tracer("Extract system includes");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
@@ -120,14 +121,43 @@
   llvm::Optional Redirects[] = {
   {""}, {""}, llvm::StringRef(StdErrPath)};
 
-  // Should we also preserve flags like "-sysroot", "-nostdinc" ?
-  const llvm::StringRef Args[] = {Driver, "-E", "-x", Lang, "-", "-v"};
+  llvm::SmallVector Args = {Driver, "-E", "-x",
+ Lang,   "-",  "-v"};
+
+  // These flags will be preserved
+  const llvm::StringRef FlagsToPreserve[] = {
+  "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
+  // Preserves these flags and their values, either as separate args or with an
+  // equalsbetween them
+  const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
+
+  for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
+llvm::StringRef Arg = CommandLine[I];
+if (llvm::any_of(FlagsToPreserve,
+ [](llvm::StringRef S) { return S == Arg; })) {
+  Args.push_back(Arg);
+} else {
+  const auto *Found =
+  llvm::find_if(ArgsToPreserve, [](llvm::StringRef S) {
+return Arg.startswith(S);
+  });
+  if (Found != std::end(ArgsToPreserve)) {
+Arg.consume_front(*Found);
+if (Arg.empty() && I + 1 < E) {
+  Args.push_back(CommandLine[I]);
+  Args.push_back(CommandLine[I + 1]);
+} else if (Arg.startswith("=")) {
+  Args.push_back(CommandLine[I]);
+}
+  }
+}
+  }
 
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
  Redirects)) {
 elog("System include extraction: driver execution failed with return code: "
- "{0}",
- llvm::to_string(RC));
+ "{0}. Args: ['{1}']",
+ llvm::to_string(RC), llvm::join(Args, "', '"));
 return {};
   }
 
@@ -247,8 +277,8 @@
   if (It != DriverToIncludesCache.end())
 SystemIncludes = It->second;
   else
-DriverToIncludesCache[Key] = SystemIncludes =
-extractSystemIncludes(Key.first, Key.second, QueryDriverRegex);
+DriverToIncludesCache[Key] = SystemIncludes = extractSystemIncludes(
+Key.first, Key.second, Cmd->CommandLine, QueryDriverRegex);
 }
 
 return addSystemIncludes(*Cmd, SystemIncludes);
@@ 

[PATCH] D73845: [Gnu toolchain] Look at standard GCC multilib/multiarch paths by default

2020-02-02 Thread Samuel Thibault via Phabricator via cfe-commits
sthibaul added a comment.

There is no behavior change, I checked for no regression on GNU/Linux with 
ninja check-all.


Repository:
  rC Clang

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

https://reviews.llvm.org/D73845



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


[PATCH] D73845: [Gnu toolchain] Look at standard GCC multilib/multiarch paths by default

2020-02-02 Thread Samuel Thibault via Phabricator via cfe-commits
sthibaul created this revision.
sthibaul added reviewers: kristina, sammccall, lebedev.ri.
Herald added subscribers: cfe-commits, krytarowski, arichardson, emaste.
Herald added a project: clang.

The current code for GNU/Linux is actually completely generic, and can be moved 
to Gnu, so it can benefit GNU/Hurd and GNU/kFreeBSD


Repository:
  rC Clang

https://reviews.llvm.org/D73845

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Linux.cpp

Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -208,15 +208,6 @@
   return Triple.isArch32Bit() ? "lib" : "lib64";
 }
 
-static void addMultilibsFilePaths(const Driver , const MultilibSet ,
-  const Multilib ,
-  StringRef InstallPath,
-  ToolChain::path_list ) {
-  if (const auto  = Multilibs.filePathsCallback())
-for (const auto  : PathsCallback(Multilib))
-  addPathIfExists(D, InstallPath + Path, Paths);
-}
-
 Linux::Linux(const Driver , const llvm::Triple , const ArgList )
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
@@ -224,21 +215,9 @@
   SelectedMultilib = GCCInstallation.getMultilib();
   llvm::Triple::ArchType Arch = Triple.getArch();
   std::string SysRoot = computeSysRoot();
-
-  // Cross-compiling binutils and GCC installations (vanilla and openSUSE at
-  // least) put various tools in a triple-prefixed directory off of the parent
-  // of the GCC installation. We use the GCC triple here to ensure that we end
-  // up with tools that support the same amount of cross compiling as the
-  // detected GCC installation. For example, if we find a GCC installation
-  // targeting x86_64, but it is a bi-arch GCC installation, it can also be
-  // used to target i386.
-  // FIXME: This seems unlikely to be Linux-specific.
   ToolChain::path_list  = getProgramPaths();
-  if (GCCInstallation.isValid()) {
-PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
-   GCCInstallation.getTriple().str() + "/bin")
- .str());
-  }
+
+  Generic_GCC::PushGCCPPaths(PPaths);
 
   Distro Distro(D.getVFS(), Triple);
 
@@ -317,58 +296,7 @@
   const std::string OSLibDir = std::string(getOSLibDir(Triple, Args));
   const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
 
-  // Add the multilib suffixed paths where they are available.
-  if (GCCInstallation.isValid()) {
-const llvm::Triple  = GCCInstallation.getTriple();
-const std::string  =
-std::string(GCCInstallation.getParentLibPath());
-
-// Add toolchain / multilib specific file paths.
-addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
-  GCCInstallation.getInstallPath(), Paths);
-
-// Sourcery CodeBench MIPS toolchain holds some libraries under
-// a biarch-like suffix of the GCC installation.
-addPathIfExists(
-D, GCCInstallation.getInstallPath() + SelectedMultilib.gccSuffix(),
-Paths);
-
-// GCC cross compiling toolchains will install target libraries which ship
-// as part of the toolchain under // rather than as
-// any part of the GCC installation in
-// //gcc//. This decision is somewhat
-// debatable, but is the reality today. We need to search this tree even
-// when we have a sysroot somewhere else. It is the responsibility of
-// whomever is doing the cross build targeting a sysroot using a GCC
-// installation that is *not* within the system root to ensure two things:
-//
-//  1) Any DSOs that are linked in from this tree or from the install path
-// above must be present on the system root and found via an
-// appropriate rpath.
-//  2) There must not be libraries installed into
-// // unless they should be preferred over
-// those within the system root.
-//
-// Note that this matches the GCC behavior. See the below comment for where
-// Clang diverges from GCC's behavior.
-addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
-   OSLibDir + SelectedMultilib.osSuffix(),
-Paths);
-
-// If the GCC installation we found is inside of the sysroot, we want to
-// prefer libraries installed in the parent prefix of the GCC installation.
-// It is important to *not* use these paths when the GCC installation is
-// outside of the system root as that can pick up unintended libraries.
-// This usually happens when there is an external cross compiler on the
-// host system, and a more minimal sysroot available that is the target of
-// the cross. Note that GCC does include some of these directories in