[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added reviewers: dschuff, tlively.
Herald added subscribers: wingo, ecnelises, sunfish, hiraditya, 
jgravelle-google, sbc100, mgorny.
aheejin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Background:

Wasm EH, while using Windows EH (catchpad/cleanuppad based) IR, uses
Itanium-based libraries and ABIs with some modifications.

`__clang_call_terminate` is a wrapper generated in Clang's Itanium C++
ABI implementation. It contains this code, in C-style pseudocode:

  void __clang_call_terminate(void *exn) {
__cxa_begin_catch(exn);
std::terminate();
  }

So this function is a wrapper to call `__cxa_begin_catch` on the
exeption pointer before termination.

In Itanium ABI, this function is called when another exception is thrown
while processing an exception. The pointer for this second, violating
exception is passed as the argument of this `__clang_call_terminate`,
which calls `__cxa_begin_catch` with that pointer and calls
`std::terminate` to terminate the program.

The spec (https://libcxxabi.llvm.org/spec.html) says,

  When the personality routine encounters a termination condition, it
  will call __cxa_begin_catch() to mark the exception as handled and then
  call terminate(), which shall not return to its caller.

In wasm EH's Clang implementation, this function is called from
cleanuppads that terminates the program, which we also call terminate
pads. Cleanuppads normally don't access the thrown exception and the
wasm backend converts them to `catch_all` blocks. But because we need
the exception pointer in this cleanuppad, we generate
`wasm.get.exception` intrinsic (which will eventually be lowered to
`catch` instruction) as we do in the catchpads. But because terminate
pads are cleanup pads and should run even when a foreign exception is
thrown, so what we have been doing is:

1. In `WebAssemblyLateEHPrepare::ensureSingleBBTermPads()`, we make sure

terminate pads are in this simple shape:

  %exn = catch
  call @__clang_call_terminate(%exn)
  unreachable

2. In `WebAssemblyHandleEHTerminatePads` pass at the end of the

pipeline, we attach a `catch_all` to terminate pads, so they will be in
this form:

  %exn = catch
  call @__clang_call_terminate(%exn)
  unreachable
  catch_all
  call @std::terminate()
  unreachable

In `catch_all` part, we don't have the exception pointer, so we call
`std::terminate()` directly. The reason we ran HandleEHTerminatePads at
the end of the pipeline, separate from LateEHPrepare, was it was
convenient to assume there was only a single `catch` part per `try`
during CFGSort and CFGStackify.

---

Problem:

While it thinks terminate pads could have been possibly split or calls
to `__clang_call_terminate` could have been duplicated,
`WebAssemblyLateEHPrepare::ensureSingleBBTermPads()` assumes terminate
pads contain no more than calls to `__clang_call_terminate` and
`unreachable` instruction. I assumed that because in LLVM very limited
forms of transformations are done to catchpads and cleanuppads to
maintain the scoping structure. But it turned out to be incorrect;
passes can merge multiple cleanuppads into one, including terminate
pads, as long as the new code has a correct scoping structure. One pass
that does this I observed was SimplifyCFG, but there can be more. After
this transformation, a single cleanuppad can contain any number of other
instructions with the call to `__clang_call_terminate` and can span many
BBs. It wouldn't be practical to duplicate all these BBs within the
cleanuppad to generate the equivalent `catch_all` blocks, only with
calls to `__clang_call_terminate` replaced by calls to `std::terminate`.

Unless we do more complicated transformation to split those calls to
`__clang_call_terminate` into a separate cleanuppad, it is tricky to
solve.

---

Solution (?):

This CL just disables the generation and use of `__clang_call_terminate`
and calls `std::terminate()` directly in its place.

The possible downside of this approach can be, because the Itanium ABI
intended to "mark" the violating exception handled, we don't do that
anymore. What `__cxa_begin_catch` actually does is increment the
exception's handler count and decrement the uncaught exception count,
which in my opinion do not matter much given that we are about to
terminate the program anyway. Also it does not affect info like stack
traces that can be possibly shown to developers.

And while we use a variant of Itanium EH ABI, we can make some
deviations if we choose to; we are already different in that in the
current version of the EH spec we don't support two-phase unwinding. We
can possibly consider a more complicated transformation later to
reenable this, but I don't think that has high priority.

Changes in this CL contains:

- In Clang, we don't generate a call to `wasm.get.exception()` intrinsic and 
`__clang_call_terminate` function in terminate pads anymore; we 

[PATCH] D96347: Include function return type in JSON AST dumps.

[Rokas Kupstys via Phabricator via cfe-commits]

rokups added a comment.

Hey @Temtaime,
Until we can rely on this i opted in for poor man's workaround - splitting 
type/qualType field at the first ( and using first part. I realize it is not 
reliable and will break when function pointers are used, but since they arent 
in my project that is an acceptable short term workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96347

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

[PATCH] D97832: [X86] Refine "Support -march=alderlake"

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:865
 
+// Alderlake
+  list ADLAdditionalFeatures = [FeatureSERIALIZE,

Extra spaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97832

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

[PATCH] D97832: [X86] Refine "Support -march=alderlake"

[Pengfei Wang via Phabricator via cfe-commits]

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

LGTM. But let's wait one day or two to see if others object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97832

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

[PATCH] D97801: [clangd] Add `limit` extension on completion and workspace-symbols

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:784
   Server->workspaceSymbols(
-  Params.query, Opts.CodeComplete.Limit,
+  Params.query, Params.limit.getValueOr(Opts.CodeComplete.Limit),
   [Reply = std::move(Reply),

should we check for non-negativity in here as well ? (or maybe just change 
deserialization logic to set it to none for negative ones and store a size_t 
instead)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97801

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

[PATCH] D97832: [X86] Refine "Support -march=alderlake"

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe created this revision.
Herald added subscribers: pengfei, hiraditya.
FreddyYe requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97832

Files:
  clang/test/Preprocessor/predefined-arch-macros.c
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td

Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -784,17 +784,6 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
-  // Alderlake
-  list ADLAdditionalFeatures = [FeatureAVXVNNI,
-  FeatureCLDEMOTE,
-  FeatureHRESET,
-  FeaturePTWRITE,
-  FeatureSERIALIZE,
-  FeatureWAITPKG];
-  list ADLTuning = SKLTuning;
-  list ADLFeatures =
-!listconcat(SKLFeatures, ADLAdditionalFeatures);
-
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCMPXCHG8B,
@@ -873,6 +862,31 @@
   list TRMFeatures =
 !listconcat(GLPFeatures, TRMAdditionalFeatures);
 
+// Alderlake
+  list ADLAdditionalFeatures = [FeatureSERIALIZE,
+  FeaturePCONFIG,
+  FeatureSHSTK,
+  FeatureWIDEKL,
+  FeatureINVPCID,
+  FeatureADX,
+  FeatureFMA,
+  FeatureVAES,
+  FeatureVPCLMULQDQ,
+  FeatureF16C,
+  FeatureBMI,
+  FeatureBMI2,
+  FeatureLZCNT,
+  FeatureAVXVNNI,
+  FeaturePKU,
+  FeatureHRESET,
+  FeatureCLDEMOTE,
+  FeatureMOVDIRI,
+  FeatureMOVDIR64B,
+  FeatureWAITPKG];
+  list ADLTuning = SKLTuning;
+  list ADLFeatures =
+!listconcat(TRMFeatures, ADLAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCMPXCHG8B,
Index: llvm/lib/Support/X86TargetParser.cpp
===
--- llvm/lib/Support/X86TargetParser.cpp
+++ llvm/lib/Support/X86TargetParser.cpp
@@ -205,9 +205,6 @@
 FeatureENQCMD | FeatureMOVDIR64B | FeatureMOVDIRI | FeaturePTWRITE |
 FeatureSERIALIZE | FeatureSHSTK | FeatureTSXLDTRK | FeatureUINTR |
 FeatureWAITPKG | FeatureAVXVNNI;
-constexpr FeatureBitset FeaturesAlderlake =
-FeaturesSkylakeClient | FeatureCLDEMOTE | FeatureHRESET | FeaturePTWRITE |
-FeatureSERIALIZE | FeatureWAITPKG | FeatureAVXVNNI;
 
 // Intel Atom processors.
 // Bonnell has feature parity with Core2 and adds MOVBE.
@@ -223,6 +220,12 @@
 FeaturesGoldmont | FeaturePTWRITE | FeatureRDPID | FeatureSGX;
 constexpr FeatureBitset FeaturesTremont =
 FeaturesGoldmontPlus | FeatureCLWB | FeatureGFNI;
+constexpr FeatureBitset FeaturesAlderlake =
+FeaturesTremont | FeatureADX | FeatureBMI | FeatureBMI2 | FeatureF16C |
+FeatureFMA | FeatureINVPCID | FeatureLZCNT | FeaturePCONFIG | FeaturePKU |
+FeatureSERIALIZE | FeatureSHSTK | FeatureVAES | FeatureVPCLMULQDQ |
+FeatureCLDEMOTE | FeatureMOVDIR64B | FeatureMOVDIRI | FeatureWAITPKG |
+FeatureAVXVNNI | FeatureHRESET | FeatureWIDEKL;
 
 // Geode Processor.
 constexpr FeatureBitset FeaturesGeode =
Index: clang/test/Preprocessor/predefined-arch-macros.c
===
--- clang/test/Preprocessor/predefined-arch-macros.c
+++ clang/test/Preprocessor/predefined-arch-macros.c
@@ -1791,32 +1791,53 @@
 // CHECK_ADL_M32: #define __BMI__ 1
 // CHECK_ADL_M32: #define __CLDEMOTE__ 1
 // CHECK_ADL_M32: #define __CLFLUSHOPT__ 1
+// CHECK_ADL_M32: #define __CLWB__ 1
 // CHECK_ADL_M32: #define __F16C__ 1
 // CHECK_ADL_M32: #define __FMA__ 1
+// CHECK_ADL_M32: #define __FSGSBASE__ 1
+// CHECK_ADL_M32: #define __FXSR__ 1
+// CHECK_ADL_M32: #define __GFNI__ 1
 // CHECK_ADL_M32: #define __HRESET__ 1
 // CHECK_ADL_M32: #define __INVPCID__ 1
+// CHECK_ADL_M32: #define __KL__ 1
 // 

[PATCH] D97573: [OpenMP] Handle non-function context before checking for diagnostic emission

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader 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/D97573/new/

https://reviews.llvm.org/D97573

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

[PATCH] D97825: [RISCV] Use RISCVV_BUILTIN for vector intrinsic checking.

[Hsiangkai Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7e675b3da1d: [RISCV] Use RISCVV_BUILTIN for vector 
intrinsic checking. (authored by HsiangKai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97825

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3395,7 +3395,7 @@
   switch (BuiltinID) {
   default:
 break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
 #include "clang/Basic/BuiltinsRISCV.def"
 if (!TI.hasFeature("experimental-v"))
   return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3395,7 +3395,7 @@
   switch (BuiltinID) {
   default:
 break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
 #include "clang/Basic/BuiltinsRISCV.def"
 if (!TI.hasFeature("experimental-v"))
   return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f7e675b - [RISCV] Use RISCVV_BUILTIN for vector intrinsic checking.

[Hsiangkai Wang via cfe-commits]

Author: Hsiangkai Wang
Date: 2021-03-03T13:42:54+08:00
New Revision: f7e675b3da1d7afd43af51c5a5e9140c87ef945d

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

LOG: [RISCV] Use RISCVV_BUILTIN for vector intrinsic checking.

There may be other BUILTINs for other extensions. Use RISCVV_BUILTIN for
vector builtin checking.

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ccde2f3164a1..779bc23a6fcd 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3395,7 +3395,7 @@ bool Sema::CheckRISCVBuiltinFunctionCall(const TargetInfo 
,
   switch (BuiltinID) {
   default:
 break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
 #include "clang/Basic/BuiltinsRISCV.def"
 if (!TI.hasFeature("experimental-v"))
   return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)



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

[PATCH] D97831: [Clang][Sema] Implement GCC -Wcast-function-type

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: aaron.ballman, rsmith.
ychen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Warn when a function pointer is cast to an incompatible function
  pointer. In a cast involving function types with a variable argument
  list only the types of initial arguments that are provided are
  considered. Any parameter of pointer-type matches any other
  pointer-type. Any benign differences in integral types are ignored, like
  int vs. long on ILP32 targets. Likewise type qualifiers are ignored. The
  function type void (*) (void) is special and matches everything, which
  can be used to suppress this warning. In a cast involving pointer to
  member types this warning warns whenever the type cast is changing the
  pointer to member type. This warning is enabled by -Wextra.

It is not part of -Wextra for now. I could add it if it is preferred.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97831

Files:
  clang/docs/DiagnosticsReference.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/warn-cast-function-type.c
  clang/test/Sema/warn-cast-function-type.cpp

Index: clang/test/Sema/warn-cast-function-type.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-cast-function-type.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -x c++ %s -fsyntax-only -Wno-unused-value -Wcast-function-type -triple x86_64-- -verify
+
+int x(long);
+
+typedef int (f1)(long);
+typedef int (f2)(void*);
+typedef int (f3)(...);
+typedef void (f4)(...);
+typedef void (f5)(void);
+typedef int (f6)(long, int);
+typedef int (f7)(long,...);
+
+f1 *a;
+f2 *b;
+f3 *c;
+f4 *d;
+f5 *e;
+f6 *f;
+f7 *g;
+
+struct S
+{
+  void foo (int*);
+  void bar (int);
+};
+
+typedef void (S::*mf)(int);
+
+void foo() {
+  a = (f1 *)x;
+  b = (f2 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)')}} */
+  b = reinterpret_cast(x); /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)')}} */
+  c = (f3 *)x;
+  d = (f4 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f4 *' (aka 'void (*)(...)')}} */
+  e = (f5 *)x;
+  f = (f6 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)')}} */
+  g = (f7 *)x;
+
+  mf p1 = (mf)::foo; /* expected-warning {{cast between incompatible function types from 'void (S::*)(int *)' to 'mf' (aka 'void (S::*)(int)')}} */
+}
Index: clang/test/Sema/warn-cast-function-type.c
===
--- /dev/null
+++ clang/test/Sema/warn-cast-function-type.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -x c %s -fsyntax-only -Wno-unused-value -Wcast-function-type -triple x86_64-- -verify
+
+int x(long);
+
+typedef int (f1)(long);
+typedef int (f2)(void*);
+typedef int (f3)();
+typedef void (f4)();
+typedef void (f5)(void);
+typedef int (f6)(long, int);
+typedef int (f7)(long,...);
+
+f1 *a;
+f2 *b;
+f3 *c;
+f4 *d;
+f5 *e;
+f6 *f;
+f7 *g;
+
+void foo(void) {
+  a = (f1 *)x;
+  b = (f2 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)')}} */
+  c = (f3 *)x;
+  d = (f4 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f4 *' (aka 'void (*)()')}} */
+  e = (f5 *)x;
+  f = (f6 *)x; /* expected-warning {{cast between incompatible function types from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)')}} */
+  g = (f7 *)x;
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -13,8 +13,8 @@
 //
 //===--===//
 
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
@@ -23,6 +23,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
+#include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
 using namespace clang;
@@ -1035,6 +1036,105 @@
 << FixItHint::CreateReplacement(BeginLoc, "static_cast");
 }
 
+static bool argTypeIsABIEquivalent(QualType SrcType, QualType DestType,
+   ASTContext ) {
+  if (SrcType->isPointerType() && DestType->isPointerType())
+return true;
+
+  // Allow integral type mismatch if their size are equal.
+  if (SrcType->isIntegralType(Context) && DestType->isIntegralType(Context))
+if 

[PATCH] D97101: [Coverage] Emit zero count gap region between statements if first statements contains return, throw, goto, co_return

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: clang/test/CoverageMapping/deferred-region.cpp:43
   if (true)
-return; // CHECK: Gap,File 0, [[@LINE]]:11
+return;
   else

vsk wrote:
> I'm confused by this change. Do we lose the gap here? I assumed it was needed 
> to prevent the condition count from `if (true)` from kicking in after the 
> `return`?
Fixed.



Comment at: clang/test/CoverageMapping/switch.cpp:62
   default:  // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #4
 break;  // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> 
[[@LINE-1]]:10 = #4, (#0 - #4)
+  }

vsk wrote:
> The blank space after `default: break;` and before the closing '}' should 
> have the same count as the switch condition, I thought (this goes for all of 
> the unreachable code within a switch body)? The idea is to prevent 
> 'not-executed' regions from appearing after the `break` stmt.
Yes, they are unreachable. So, shouldn't they always have zero count instead of 
same count as switch condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97101

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

[PATCH] D97101: [Coverage] Emit zero count gap region between statements if first statements contains return, throw, goto, co_return

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 327661.
zequanwu marked an inline comment as done.
zequanwu added a comment.

- Deprecate deferred region. Use the notion of gap region inserted in VisitStmt 
to replace deferred region.
- Emit gap region with OutCounter from last statement if exists.
- Move test cases from deferred-region.cpp to terminate-statements.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97101

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/terminate-statements.cpp
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c

Index: compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
===
--- compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
+++ compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
@@ -21,9 +21,9 @@
 
 // CHECK-COVERAGE: Filename RegionsMissed Regions Cover   Functions  Missed Functions  Executed   Lines  Missed Lines Cover
 // CHECK-COVERAGE-NEXT: ---
-// CHECK-COVERAGE-NEXT: runtime-counter-relocation.c  4 175.00%   1 0   100.00%   5 260.00%
+// CHECK-COVERAGE-NEXT: runtime-counter-relocation.c  4 175.00%   1 0   100.00%   5 180.00%
 // CHECK-COVERAGE-NEXT: ---
-// CHECK-COVERAGE-NEXT: TOTAL 4 175.00%   1 0   100.00%   5 260.00%
+// CHECK-COVERAGE-NEXT: TOTAL 4 175.00%   1 0   100.00%   5 180.00%
 
 extern int __llvm_profile_is_continuous_mode_enabled(void);
 
Index: clang/test/CoverageMapping/unreachable-macro.c
===
--- clang/test/CoverageMapping/unreachable-macro.c
+++ clang/test/CoverageMapping/unreachable-macro.c
@@ -6,6 +6,7 @@
 void counters_in_macro_following_unreachable() {
   // CHECK-NEXT: File 0, [[@LINE-1]]:48 -> {{[0-9]+}}:2 = #0
   return;
+  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:10 -> [[@LINE+3]]:3 = 0
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:3 -> [[@LINE+2]]:8 = 0
   // CHECK-NEXT: File 0, [[@LINE+1]]:8 -> [[@LINE+2]]:2 = 0
   WHILE
Index: clang/test/CoverageMapping/trycatch.cpp
===
--- clang/test/CoverageMapping/trycatch.cpp
+++ clang/test/CoverageMapping/trycatch.cpp
@@ -13,7 +13,7 @@
 void func(int i) {// CHECK-NEXT: File 0, [[@LINE]]:18 -> {{[0-9]+}}:2 = #0
   // CHECK-NEXT: File 0, [[@LINE+1]]:6 -> [[@LINE+1]]:11 = #0
   if(i % 2) { // CHECK: File 0, [[@LINE]]:13 -> [[@LINE+4]]:4 = #1
-throw Error();
+throw Error();// CHECK-NEXT: Gap,File 0, [[@LINE]]:19 -> [[@LINE+1]]:5 = 0
 int j = 0;// CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:4 = 0
   // CHECK: File 0, [[@LINE+1]]:10 -> [[@LINE+2]]:27 = (#0 - #1)
   } else if(i == 8)   // CHECK-NEXT: File 0, [[@LINE]]:13 -> [[@LINE]]:19 = (#0 - #1)
Index: clang/test/CoverageMapping/terminate-statements.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/terminate-statements.cpp
@@ -0,0 +1,343 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fexceptions -fcxx-exceptions -emit-llvm-only -main-file-name deferred-region.cpp -I %S/Inputs %s | FileCheck %s
+
+int f1() {
+  return 0;
+  return 0; // CHECK: Gap,File 0, [[@LINE-1]]:12 -> [[@LINE]]:3 = 0
+}
+
+int f2(int i) {
+  if (i)
+return 0;
+  else
+;   // CHECK: Gap,File 0, [[@LINE]]:6 -> [[@LINE+1]]:3 = (#0 - #1)
+  return 1; // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:11 = (#0 - #1)
+}
+
+int f3() {
+  for (int a = 1; a < 9; a--)
+return a; // CHECK: Gap,File 0, [[@LINE]]:14 -> [[@LINE+1]]:3 = (#0 - #1)
+  return 0;   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:11 = (#0 - #1)
+}
+
+int f4(int i) {
+  while (i > 0) {
+i++;
+return i;
+  } // CHECK: File 

[PATCH] D97823: Update default arch on AIX

[Jinsong Ji via Phabricator via cfe-commits]

jsji added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:405
+  T.getOSVersion(major, minor, micro);
+  TargetCPUName = major == 7 && minor < 2 ? "pwr4" : "pwr7";
+} else if (T.getArch() == llvm::Triple::ppc64le)

jsji wrote:
> Please add comments here about AIX 7.2's minimal arch is P7.
`major == 7 ` should this be `major <= 7 `? Or else, we may get `pwr7` for AIX 
5.1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97823

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

[PATCH] D97823: Update default arch on AIX

[Jinsong Ji via Phabricator via cfe-commits]

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

LGTM. Thanks.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:405
+  T.getOSVersion(major, minor, micro);
+  TargetCPUName = major == 7 && minor < 2 ? "pwr4" : "pwr7";
+} else if (T.getArch() == llvm::Triple::ppc64le)

Please add comments here about AIX 7.2's minimal arch is P7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97823

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

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

[Bing Yu via Phabricator via cfe-commits]

yubing added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:311
+  Value *ResElt = B.CreateAdd(EltC, SubVecR);
+  Value *NewVecC = B.CreateInsertElement(VecCPhi, ResElt, IdxC);
+  Value *NewVecD = B.CreateInsertElement(VecDPhi, ResElt, IdxC);

LuoYuanke wrote:
> yubing wrote:
> > pengfei wrote:
> > > yubing wrote:
> > > > pengfei wrote:
> > > > > Is it necessary to insert the ResElt to VecC?
> > > > Yes, it is necessary since you should use updated eltC(aka, Cij) when 
> > > > you are doing matrix dotproduct:
> > > > Cij =Cij+Ai1.*B1j
> > > > Cij =Cij+Ai2.*B2j
> > > > 
> > > > Cij =Cij+AiK.*BKj
> > > But you don't need to update both C and D. Something like the psudo code 
> > > should enough:
> > > ```
> > > for (k : K)
> > >   Dij += Aik * Bkj;
> > > Dij += Cij
> > > ```
> > I change code into the following style, and it can also reduce inner loop's 
> > size:
> > ```
> > for (k : K)
> >   Cij += Aik * Bkj;
> > Dij = Cij
> > ```
> > Besides, I hoist the procedure of calculating (i,j)'s linear index above 
> > inner loops.
> It seems keeping vector C unchanged is simpler. We can eliminate the phi, 
> extract and insert instruction for vector C.
But your solution  still need to update D so D's phi will be kept in the inner 
loops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 327655.
0x8000- added a comment.

Add an option "IgnoredVariableNames" that allows filtering out acceptable 
variable names (similar to the loop counters); also implemented via regex.

Added tests for the IgnoredVariableNames option.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-variable-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-variable-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept
+{
+   int val;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: variable name 'z' is too short, expected at least 3 characters [readability-variable-length]
+{
+   int i = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-variable-length]
+
+   int jj = z;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-variable-length]
+
+   for (int m = 0; m < 5; ++ m)
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& x)
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+}
+
+void longEnoughVariableNames(int y) // argument 'y' ignored by configuration
+{
+   int var = 5;
+
+   for (int i = 0; i < 42; ++ i) // 'i' is default allowed, for historical reasons
+   {
+  doIt();
+   }
+
+   for (int kk = 0; kk < 42; ++ kk)
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& ex)
+   {
+  doIt();
+   }
+
+   int x = 5;  // ignored by configuration
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
@@ -0,0 +1,76 @@
+.. title:: clang-tidy - readability-variable-length
+
+readability-variable-length
+===
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumExceptionNameLength`
+
+.. option:: MinimumLoopCounterNameLength
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is `2`).
+
+.. code-block:: c++
+
+  // This warns that 'q' is too short.
+  for (int q = 0; q < size; ++ q) {
+ // ...
+  }
+
+.. option:: IgnoredLoopCounterNames
+
+Specifies a regular expression for counter names that are to be ignored.
+The default value is `^[ijk_]$`; the first three symbols for historical
+reasons and the last one since it is frequently used as a "dont't care"
+value, specifically in tools such as Google Benchmark.
+
+.. code-block:: c++
+
+  // This does not warn by default, for historical reasons.
+  for (int i = 0; i < size; ++ i) {
+  // ...
+  }
+
+.. option:: MinimumExceptionNameLength
+
+Exception clause variables are expected to have a length of at least
+`MinimumExceptionNameLength` (default is `2`).
+
+.. code-block:: c++
+
+  try {
+  // ...
+  }
+  // This warns that 'e' is too short.
+  catch (const std::exception& e) {
+  // ...
+  }
+
+.. option:: MinimumVariableNameLength
+
+All other variables are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`).
+
+.. code-block:: c++
+
+  int i = 42;// warns 

[PATCH] D97826: [RISCV] Make use of the required features in BuiltinInfo to store that V extension builtins require 'experimental-v'.

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97826

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

[PATCH] D96709: Add Windows ehcont section support (/guard:ehcont).

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks for the review.




Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:499
   CmdArgs.push_back("-guard:cf-");
+} else if (GuardArgs.equals_lower("ehcont")) {
+  CmdArgs.push_back("/guard:ehcont");

rnk wrote:
> This is gone now, btw
Sorry, I don't understand by the mean "gone"? The check for cf is still there 
after I rebased, I think we should do for ehcont too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96709

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

[PATCH] D96709: Add Windows ehcont section support (/guard:ehcont).

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 327654.
pengfei added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96709

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-options.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -51,6 +51,9 @@
 
Makes programs 10x faster by doing Special New Thing.
 
+* Windows Control-flow Enforcement Technology: the ``-ehcontguard`` option now
+  emits valid unwind entrypoints which are validated when the context is being
+  set during exception handling.
 
 Changes to the LLVM IR
 --
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -614,6 +614,13 @@
 // RUN: %clang_cl /guard:nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDNOCHECKSINVALID %s
 // CFGUARDNOCHECKSINVALID: invalid value 'nochecks' in '/guard:'
 
+// RUN: %clang_cl  -### -- %s 2>&1 | FileCheck -check-prefix=NOEHCONTGUARD %s
+// RUN: %clang_cl /guard:ehcont- -### -- %s 2>&1 | FileCheck -check-prefix=NOEHCONTGUARD %s
+// NOEHCONTGUARD-NOT: -ehcontguard
+
+// RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck -check-prefix=EHCONTGUARD %s
+// EHCONTGUARD: -ehcontguard
+
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 
Index: clang/test/CodeGen/cfguardtable.c
===
--- clang/test/CodeGen/cfguardtable.c
+++ clang/test/CodeGen/cfguardtable.c
@@ -1,8 +1,10 @@
-// RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARDNOCHECKS
-// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARD
-
-void f() {}
-
-// Check that the cfguard metadata flag gets correctly set on the module.
-// CFGUARDNOCHECKS: !"cfguard", i32 1}
-// CFGUARD: !"cfguard", i32 2}
+// RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARDNOCHECKS
+// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARD
+// RUN: %clang_cc1 -ehcontguard -emit-llvm %s -o - | FileCheck %s -check-prefix=EHCONTGUARD
+
+void f() {}
+
+// Check that the cfguard metadata flag gets correctly set on the module.
+// CFGUARDNOCHECKS: !"cfguard", i32 1}
+// CFGUARD: !"cfguard", i32 2}
+// EHCONTGUARD: !"ehcontguard", i32 1}
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -509,6 +509,10 @@
   CmdArgs.push_back("-guard:cf");
 } else if (GuardArgs.equals_lower("cf-")) {
   CmdArgs.push_back("-guard:cf-");
+} else if (GuardArgs.equals_lower("ehcont")) {
+  CmdArgs.push_back("-guard:ehcont");
+} else if (GuardArgs.equals_lower("ehcont-")) {
+  CmdArgs.push_back("-guard:ehcont-");
 }
   }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7076,14 +7076,19 @@
 
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
-// The only valid options are "cf", "cf,nochecks", and "cf-".
+// The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
+// "ehcont-".
 if (GuardArgs.equals_lower("cf")) {
   // Emit CFG instrumentation and the table of address-taken functions.
   CmdArgs.push_back("-cfguard");
 } else if (GuardArgs.equals_lower("cf,nochecks")) {
   // Emit only the table of address-taken functions.
   CmdArgs.push_back("-cfguard-no-checks");
-} else if (GuardArgs.equals_lower("cf-")) {
+} else if (GuardArgs.equals_lower("ehcont")) {
+  // Emit EH continuation table.
+  CmdArgs.push_back("-ehcontguard");
+} else if (GuardArgs.equals_lower("cf-") ||
+   GuardArgs.equals_lower("ehcont-")) {
   // Do nothing, but we might want to emit a security warning in future.
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -552,6 +552,10 @@
 // Function ID tables for Control Flow 

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst:21
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is `2`).

It'll be good idea to use 2 symbols indention, like in code. Same in other 
places.


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

https://reviews.llvm.org/D97753

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 327653.
0x8000- marked an inline comment as done.
0x8000- added a comment.

Use regular expression for filtering out loop counter variables to ignore.

Fully document the options.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s readability-variable-length %t
+
+struct myexcept
+{
+   int val;
+};
+
+void doIt();
+
+void tooShortVariableNames()
+{
+   int i = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-variable-length]
+
+   int jj = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-variable-length]
+
+   for (int m = 0; m < 5; ++ m)
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& x)
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+}
+
+void longEnoughVariableNames()
+{
+   int var = 5;
+
+   for (int i = 0; i < 42; ++ i) // 'i' is default allowed, for historical reasons
+   {
+  doIt();
+   }
+
+   for (int kk = 0; kk < 42; ++ kk)
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& ex)
+   {
+  doIt();
+   }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
@@ -0,0 +1,70 @@
+.. title:: clang-tidy - readability-variable-length
+
+readability-variable-length
+===
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumVariableNameLength`, :option:`MinimumExceptionNameLength`
+
+.. option:: MinimumLoopCounterNameLength
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is `2`).
+
+.. code-block:: c++
+
+// This warns that 'q' is too short.
+for (int q = 0; q < size; ++ q) {
+// ...
+}
+
+.. option:: IgnoredLoopCounterNames
+
+Specifies a regular expression for counter names that are to be ignored.
+The default value is `^[ijk_]$`; the first three symbols for historical
+reasons and the last one since it is frequently used as a "dont't care"
+value, specifically in tools such as Google Benchmark.
+
+.. code-block:: c++
+
+// This does not warn by default, for historical reasons.
+for (int i = 0; i < size; ++ i) {
+// ...
+}
+
+.. option:: MinimumExceptionNameLength
+
+Exception clause variables are expected to have a length of at least
+`MinimumExceptionNameLength` (default is `2`).
+
+.. code-block:: c++
+
+try {
+// ...
+}
+// This warns that 'e' is too short.
+catch (const std::exception& e) {
+// ...
+}
+
+.. option:: MinimumVariableNameLength
+
+All other variables are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`).
+
+.. code-block:: c++
+
+int i = 42;// warns that 'i' is too short
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -312,6 +312,7 @@

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- marked an inline comment as done.
0x8000- added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst:9
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is 2). Additionally, `i`,

Eugene.Zelenko wrote:
> Eugene.Zelenko wrote:
> > See other checks documentation as example of options descriptions.
> You still need to add `Options` section and mark-up for options. See other 
> checks documentation as example.
Oh, sorry, now I see what you mean. I have sampled randomly a handful of 
documentation snippets, and didn't find anything. But after your comment, I 
looked at readability-identifier-naming.rst which had "options" in all their 
glory.


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

https://reviews.llvm.org/D97753

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst:9
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is 2). Additionally, `i`,

Eugene.Zelenko wrote:
> See other checks documentation as example of options descriptions.
You still need to add `Options` section and mark-up for options. See other 
checks documentation as example.


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

https://reviews.llvm.org/D97753

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

[PATCH] D97553: [clang][NFC] pack StaticDiagInfoRec

[Nathan James via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG335375ef2c66: [clang][NFC] pack StaticDiagInfoRec (authored 
by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97553

Files:
  clang/lib/Basic/DiagnosticIDs.cpp


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -109,15 +109,15 @@
 
 struct StaticDiagInfoRec {
   uint16_t DiagID;
-  unsigned DefaultSeverity : 3;
-  unsigned Class : 3;
-  unsigned SFINAE : 2;
-  unsigned WarnNoWerror : 1;
-  unsigned WarnShowInSystemHeader : 1;
-  unsigned Deferrable : 1;
-  unsigned Category : 6;
+  uint8_t DefaultSeverity : 3;
+  uint8_t Class : 3;
+  uint8_t SFINAE : 2;
+  uint8_t Category : 6;
+  uint8_t WarnNoWerror : 1;
+  uint8_t WarnShowInSystemHeader : 1;
 
-  uint16_t OptionGroupIndex;
+  uint16_t OptionGroupIndex : 15;
+  uint16_t Deferrable : 1;
 
   uint16_t DescriptionLen;
 
@@ -168,20 +168,20 @@
 #undef STRINGIFY_NAME
 
 const StaticDiagInfoRec StaticDiagInfo[] = {
+// clang-format off
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, 
\
- SHOWINSYSHEADER, DEFERRABLE, CATEGORY) \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
   {
\
   diag::ENUM,  
\
   DEFAULT_SEVERITY,
\
   CLASS,   
\
   DiagnosticIDs::SFINAE,   
\
+  CATEGORY,
\
   NOWERROR,
\
   SHOWINSYSHEADER, 
\
- DEFERRABLE,  \
-  CATEGORY,
\
   GROUP,   
\
+   DEFERRABLE, 
 \
   STR_SIZE(DESC, uint16_t)},
-// clang-format off
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
@@ -194,7 +194,7 @@
 #include "clang/Basic/DiagnosticSemaKinds.inc"
 #include "clang/Basic/DiagnosticAnalysisKinds.inc"
 #include "clang/Basic/DiagnosticRefactoringKinds.inc"
-  // clang-format on
+// clang-format on
 #undef DIAG
 };
 


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -109,15 +109,15 @@
 
 struct StaticDiagInfoRec {
   uint16_t DiagID;
-  unsigned DefaultSeverity : 3;
-  unsigned Class : 3;
-  unsigned SFINAE : 2;
-  unsigned WarnNoWerror : 1;
-  unsigned WarnShowInSystemHeader : 1;
-  unsigned Deferrable : 1;
-  unsigned Category : 6;
+  uint8_t DefaultSeverity : 3;
+  uint8_t Class : 3;
+  uint8_t SFINAE : 2;
+  uint8_t Category : 6;
+  uint8_t WarnNoWerror : 1;
+  uint8_t WarnShowInSystemHeader : 1;
 
-  uint16_t OptionGroupIndex;
+  uint16_t OptionGroupIndex : 15;
+  uint16_t Deferrable : 1;
 
   uint16_t DescriptionLen;
 
@@ -168,20 +168,20 @@
 #undef STRINGIFY_NAME
 
 const StaticDiagInfoRec StaticDiagInfo[] = {
+// clang-format off
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
- SHOWINSYSHEADER, DEFERRABLE, CATEGORY) \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)\
   {\
   diag::ENUM,  \
   DEFAULT_SEVERITY,\
   CLASS,   \
   DiagnosticIDs::SFINAE,   \
+  CATEGORY,\
   NOWERROR,\
   SHOWINSYSHEADER, \
-	  DEFERRABLE,  \
-  CATEGORY,\
   GROUP,   \
+	DEFERRABLE,   

[clang] 335375e - [clang][NFC] pack StaticDiagInfoRec

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-03-03T02:53:10Z
New Revision: 335375ef2c66408da751223d13758d14da248cd3

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

LOG: [clang][NFC] pack StaticDiagInfoRec

Exchanging types, reordering fields and borrowing a bit from OptionGroupIndex 
shrinks this from 12 bytes to 8.
This knocks ~20k from the binary size.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Basic/DiagnosticIDs.cpp

Removed: 




diff  --git a/clang/lib/Basic/DiagnosticIDs.cpp 
b/clang/lib/Basic/DiagnosticIDs.cpp
index 06a8e2ed5ebd..c333076d2efc 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -109,15 +109,15 @@ enum {
 
 struct StaticDiagInfoRec {
   uint16_t DiagID;
-  unsigned DefaultSeverity : 3;
-  unsigned Class : 3;
-  unsigned SFINAE : 2;
-  unsigned WarnNoWerror : 1;
-  unsigned WarnShowInSystemHeader : 1;
-  unsigned Deferrable : 1;
-  unsigned Category : 6;
+  uint8_t DefaultSeverity : 3;
+  uint8_t Class : 3;
+  uint8_t SFINAE : 2;
+  uint8_t Category : 6;
+  uint8_t WarnNoWerror : 1;
+  uint8_t WarnShowInSystemHeader : 1;
 
-  uint16_t OptionGroupIndex;
+  uint16_t OptionGroupIndex : 15;
+  uint16_t Deferrable : 1;
 
   uint16_t DescriptionLen;
 
@@ -168,20 +168,20 @@ VALIDATE_DIAG_SIZE(REFACTORING)
 #undef STRINGIFY_NAME
 
 const StaticDiagInfoRec StaticDiagInfo[] = {
+// clang-format off
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, 
\
- SHOWINSYSHEADER, DEFERRABLE, CATEGORY) \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
   {
\
   diag::ENUM,  
\
   DEFAULT_SEVERITY,
\
   CLASS,   
\
   DiagnosticIDs::SFINAE,   
\
+  CATEGORY,
\
   NOWERROR,
\
   SHOWINSYSHEADER, 
\
- DEFERRABLE,  \
-  CATEGORY,
\
   GROUP,   
\
+   DEFERRABLE, 
 \
   STR_SIZE(DESC, uint16_t)},
-// clang-format off
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
@@ -194,7 +194,7 @@ const StaticDiagInfoRec StaticDiagInfo[] = {
 #include "clang/Basic/DiagnosticSemaKinds.inc"
 #include "clang/Basic/DiagnosticAnalysisKinds.inc"
 #include "clang/Basic/DiagnosticRefactoringKinds.inc"
-  // clang-format on
+// clang-format on
 #undef DIAG
 };
 



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

[PATCH] D97639: [clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a91b8232a5d: [clang-tidy][NFC] Use equalsBoundNode matchers 
to simplify LoopConvertCheck (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97639

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -61,21 +61,16 @@
 static const char LoopNameReverseIterator[] = "forLoopReverseIterator";
 static const char LoopNamePseudoArray[] = "forLoopPseudoArray";
 static const char ConditionBoundName[] = "conditionBound";
-static const char ConditionVarName[] = "conditionVar";
-static const char IncrementVarName[] = "incrementVar";
 static const char InitVarName[] = "initVar";
 static const char BeginCallName[] = "beginCall";
 static const char EndCallName[] = "endCall";
-static const char ConditionEndVarName[] = "conditionEndVar";
 static const char EndVarName[] = "endVar";
 static const char DerefByValueResultName[] = "derefByValueResult";
 static const char DerefByRefResultName[] = "derefByRefResult";
-// shared matchers
-static const TypeMatcher anyType() { return anything(); }
 
 static const StatementMatcher integerComparisonMatcher() {
   return expr(ignoringParenImpCasts(
-  declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName);
+  declRefExpr(to(varDecl(equalsBoundNode(InitVarName));
 }
 
 static const DeclarationMatcher initToZeroMatcher() {
@@ -85,25 +80,19 @@
 }
 
 static const StatementMatcher incrementVarMatcher() {
-  return declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName)));
+  return declRefExpr(to(varDecl(equalsBoundNode(InitVarName;
 }
 
 /// The matcher for loops over arrays.
-///
-/// In this general example, assuming 'j' and 'k' are of integral type:
 /// \code
-///   for (int i = 0; j < 3 + 2; ++k) { ... }
+///   for (int i = 0; i < 3 + 2; ++i) { ... }
 /// \endcode
 /// The following string identifiers are bound to these parts of the AST:
-///   ConditionVarName: 'j' (as a VarDecl)
 ///   ConditionBoundName: '3 + 2' (as an Expr)
 ///   InitVarName: 'i' (as a VarDecl)
-///   IncrementVarName: 'k' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///
 /// Client code will need to make sure that:
-///   - The three index variables identified by the matcher are the same
-/// VarDecl.
 ///   - The index variable is only used as an array index.
 ///   - All arrays indexed by the loop are the same.
 StatementMatcher makeArrayLoopMatcher() {
@@ -131,27 +120,22 @@
 /// catch loops of the following textual forms (regardless of whether the
 /// iterator type is actually a pointer type or a class type):
 ///
-/// Assuming f, g, and h are of type containerType::iterator,
 /// \code
 ///   for (containerType::iterator it = container.begin(),
-///e = createIterator(); f != g; ++h) { ... }
+///e = createIterator(); it != e; ++it) { ... }
 ///   for (containerType::iterator it = container.begin();
-///f != anotherContainer.end(); ++h) { ... }
+///it != anotherContainer.end(); ++it) { ... }
 /// \endcode
 /// The following string identifiers are bound to the parts of the AST:
 ///   InitVarName: 'it' (as a VarDecl)
-///   ConditionVarName: 'f' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///   In the first example only:
 /// EndVarName: 'e' (as a VarDecl)
-/// ConditionEndVarName: 'g' (as a VarDecl)
 ///   In the second example only:
 /// EndCallName: 'container.end()' (as a CXXMemberCallExpr)
 ///
 /// Client code will need to make sure that:
-///   - The iterator variables 'it', 'f', and 'h' are the same.
 ///   - The two containers on which 'begin' and 'end' are called are the same.
-///   - If the end iterator variable 'g' is defined, it is the same as 'f'.
 StatementMatcher makeIteratorLoopMatcher(bool IsReverse) {
 
   auto BeginNameMatcher = IsReverse ? hasAnyName("rbegin", "crbegin")
@@ -180,13 +164,13 @@
 
   StatementMatcher IteratorBoundMatcher =
   expr(anyOf(ignoringParenImpCasts(
- declRefExpr(to(varDecl().bind(ConditionEndVarName,
+ declRefExpr(to(varDecl(equalsBoundNode(EndVarName),
  ignoringParenImpCasts(expr(EndCallMatcher).bind(EndCallName)),
  materializeTemporaryExpr(ignoringParenImpCasts(
  expr(EndCallMatcher).bind(EndCallName);
 
-  StatementMatcher IteratorComparisonMatcher = expr(
-  ignoringParenImpCasts(declRefExpr(to(varDecl().bind(ConditionVarName);
+  StatementMatcher IteratorComparisonMatcher = expr(ignoringParenImpCasts(
+  

[clang-tools-extra] 1a91b82 - [clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-03-03T02:51:34Z
New Revision: 1a91b8232a5d31ef3a4f8eb52f5a7aaf79525b9b

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

LOG: [clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck

Make use of the `equalsBoundNode` matcher to ensure Init, Conditon and 
Increment variables all refer to the same variable during matching.

Reviewed By: steveire

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index ec75be9c5f53..9df343f81d1c 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -61,21 +61,16 @@ static const char LoopNameIterator[] = "forLoopIterator";
 static const char LoopNameReverseIterator[] = "forLoopReverseIterator";
 static const char LoopNamePseudoArray[] = "forLoopPseudoArray";
 static const char ConditionBoundName[] = "conditionBound";
-static const char ConditionVarName[] = "conditionVar";
-static const char IncrementVarName[] = "incrementVar";
 static const char InitVarName[] = "initVar";
 static const char BeginCallName[] = "beginCall";
 static const char EndCallName[] = "endCall";
-static const char ConditionEndVarName[] = "conditionEndVar";
 static const char EndVarName[] = "endVar";
 static const char DerefByValueResultName[] = "derefByValueResult";
 static const char DerefByRefResultName[] = "derefByRefResult";
-// shared matchers
-static const TypeMatcher anyType() { return anything(); }
 
 static const StatementMatcher integerComparisonMatcher() {
   return expr(ignoringParenImpCasts(
-  declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName);
+  declRefExpr(to(varDecl(equalsBoundNode(InitVarName));
 }
 
 static const DeclarationMatcher initToZeroMatcher() {
@@ -85,25 +80,19 @@ static const DeclarationMatcher initToZeroMatcher() {
 }
 
 static const StatementMatcher incrementVarMatcher() {
-  return declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName)));
+  return declRefExpr(to(varDecl(equalsBoundNode(InitVarName;
 }
 
 /// The matcher for loops over arrays.
-///
-/// In this general example, assuming 'j' and 'k' are of integral type:
 /// \code
-///   for (int i = 0; j < 3 + 2; ++k) { ... }
+///   for (int i = 0; i < 3 + 2; ++i) { ... }
 /// \endcode
 /// The following string identifiers are bound to these parts of the AST:
-///   ConditionVarName: 'j' (as a VarDecl)
 ///   ConditionBoundName: '3 + 2' (as an Expr)
 ///   InitVarName: 'i' (as a VarDecl)
-///   IncrementVarName: 'k' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///
 /// Client code will need to make sure that:
-///   - The three index variables identified by the matcher are the same
-/// VarDecl.
 ///   - The index variable is only used as an array index.
 ///   - All arrays indexed by the loop are the same.
 StatementMatcher makeArrayLoopMatcher() {
@@ -131,27 +120,22 @@ StatementMatcher makeArrayLoopMatcher() {
 /// catch loops of the following textual forms (regardless of whether the
 /// iterator type is actually a pointer type or a class type):
 ///
-/// Assuming f, g, and h are of type containerType::iterator,
 /// \code
 ///   for (containerType::iterator it = container.begin(),
-///e = createIterator(); f != g; ++h) { ... }
+///e = createIterator(); it != e; ++it) { ... }
 ///   for (containerType::iterator it = container.begin();
-///f != anotherContainer.end(); ++h) { ... }
+///it != anotherContainer.end(); ++it) { ... }
 /// \endcode
 /// The following string identifiers are bound to the parts of the AST:
 ///   InitVarName: 'it' (as a VarDecl)
-///   ConditionVarName: 'f' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///   In the first example only:
 /// EndVarName: 'e' (as a VarDecl)
-/// ConditionEndVarName: 'g' (as a VarDecl)
 ///   In the second example only:
 /// EndCallName: 'container.end()' (as a CXXMemberCallExpr)
 ///
 /// Client code will need to make sure that:
-///   - The iterator variables 'it', 'f', and 'h' are the same.
 ///   - The two containers on which 'begin' and 'end' are called are the same.
-///   - If the end iterator variable 'g' is defined, it is the same as 'f'.
 StatementMatcher makeIteratorLoopMatcher(bool IsReverse) {
 
   auto BeginNameMatcher = IsReverse ? hasAnyName("rbegin", "crbegin")
@@ -180,13 +164,13 @@ StatementMatcher makeIteratorLoopMatcher(bool IsReverse) {
 
   StatementMatcher IteratorBoundMatcher =
   

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 327649.
0x8000- added a comment.

Addressed most of the review comments with the exception of switching to regex 
for ignoring loop counters; this change is in the follow-up review.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s readability-variable-length %t
+
+struct myexcept
+{
+   int val;
+};
+
+void doIt();
+
+void tooShortVariableNames()
+{
+   int i = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-variable-length]
+
+   int jj = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-variable-length]
+
+   for (int m = 0; m < 5; ++ m)
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& x)
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+}
+
+void longEnoughVariableNames()
+{
+   int var = 5;
+
+   for (int i = 0; i < 42; ++ i) // 'i' is default allowed, for historical reasons
+   {
+  doIt();
+   }
+
+   for (int kk = 0; kk < 42; ++ kk)
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& ex)
+   {
+  doIt();
+   }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - readability-variable-length
+
+readability-variable-length
+===
+
+This check finds variables and function parameters whose length are too short.
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is `2`). Additionally, `i`,
+`j`, `k` and `_` are accepted as legacy values (this is the default value for
+`IgnoredLoopCounterNames`).
+
+.. code-block:: c++
+
+   // This warns that 'q' is too short.
+   for (int q = 0; q < size; ++ q) {
+  /* */
+   }
+
+   // This does not warn, for historical reasons.
+   for (int i = 0; i < size; ++ i) {
+  /* */
+   }
+
+Exception clause variables are expected to have a length of at least
+`MinimumExceptionNameLength` (default is `2`).
+
+.. code-block:: c++
+
+   try {
+  /* */
+   }
+   // This warns that 'e' is too short.
+   catch (const std::exception& e) {
+   }
+
+All other variables are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`).
+
+.. code-block:: c++
+
+   int i = 42;// warns that 'i' is too short
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -312,6 +312,7 @@
`readability-uniqueptr-delete-release `_, "Yes"
`readability-uppercase-literal-suffix `_, "Yes"
`readability-use-anyofallof `_,
+   `readability-variable-length `_,
`zircon-temporary-objects `_,
 
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
+- New :doc:`readability-variable-length
+  ` check.
+
+  Finds variables and function arguments whose names are too short.
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h
===
--- /dev/null
+++ 

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- marked 10 inline comments as done.
0x8000- added a comment.

@Eugene.Zelenko and @njames93

Thank you both for the kind and thoughtful feedback. I am incorporating all 
recommended changes.

First step are all the smaller scope comments, with regex support in a 
follow-up commit.

I will also generate the diffs in the future using the -U9 option as 
suggested.




Comment at: 
clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:58-62
+  Finder->addMatcher(
+  varDecl(unless(anyOf(hasParent(declStmt(hasParent(forStmt(,
+   hasParent(cxxCatchStmt()
+  .bind("standaloneVar"),
+  this);

njames93 wrote:
> This will miss the match for `k` in the same example:
> ```lang=c
> for (int i = 4, j = 5;;)
>   int k = 5;
> ```
> Gotta scratch my head for this one though. Though like the case before its 
> rather unlikely to show up in code so not the end of the world.
> 
> Also varDecl will match parameters, is this intended can you add tests and 
> explain in documentation.
> If its unintended putting `unless(parmVarDecl())` in the matcher disable that.
Matching of function parameter names is intentional. I will make sure it is 
emphasized in the documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97753

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

[PATCH] D94554: [clangd] Add a Filesystem that overlays Dirty files.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/DraftStore.h:37
 
+  DraftStore(const ThreadsafeFS );
+

sammccall wrote:
> having DraftStore sit on top of TFS seems a bit inside-out, giving it bigger 
> scope than necessary.
> 
> What about giving DraftStore an asVFS() method that returns an in-memory 
> filesystem?
> 
> then separately we can have a separate DirtyFS : TFS, that has a 
> `DraftStore&` and a `TFS ` and implements viewImpl() on top of their 
> public interfaces. Having the dependency in that direction seems more natural 
> to me.
How I had it was a symptom of using the underlying TFS for getting the UniqueID 
of files. But yes now that's gone this approach works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94554

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

[PATCH] D94554: [clangd] Add a Filesystem that overlays Dirty files.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 327646.
njames93 added a comment.

Getting there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94554

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/DraftStore.cpp
  clang-tools-extra/clangd/DraftStore.h
  clang-tools-extra/clangd/unittests/DraftStoreTests.cpp

Index: clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
===
--- clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -24,20 +24,20 @@
 
   EXPECT_EQ("25", DS.addDraft(File, "25", ""));
   EXPECT_EQ("25", DS.getDraft(File)->Version);
-  EXPECT_EQ("", DS.getDraft(File)->Contents);
+  EXPECT_EQ("", *DS.getDraft(File)->Contents);
 
   EXPECT_EQ("26", DS.addDraft(File, "", "x"));
   EXPECT_EQ("26", DS.getDraft(File)->Version);
-  EXPECT_EQ("x", DS.getDraft(File)->Contents);
+  EXPECT_EQ("x", *DS.getDraft(File)->Contents);
 
   EXPECT_EQ("27", DS.addDraft(File, "", "x")) << "no-op change";
   EXPECT_EQ("27", DS.getDraft(File)->Version);
-  EXPECT_EQ("x", DS.getDraft(File)->Contents);
+  EXPECT_EQ("x", *DS.getDraft(File)->Contents);
 
   // We allow versions to go backwards.
   EXPECT_EQ("7", DS.addDraft(File, "7", "y"));
   EXPECT_EQ("7", DS.getDraft(File)->Version);
-  EXPECT_EQ("y", DS.getDraft(File)->Contents);
+  EXPECT_EQ("y", *DS.getDraft(File)->Contents);
 }
 
 } // namespace
Index: clang-tools-extra/clangd/DraftStore.h
===
--- clang-tools-extra/clangd/DraftStore.h
+++ clang-tools-extra/clangd/DraftStore.h
@@ -13,6 +13,7 @@
 #include "support/Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include 
 #include 
 #include 
@@ -27,7 +28,7 @@
 class DraftStore {
 public:
   struct Draft {
-std::string Contents;
+std::shared_ptr Contents;
 std::string Version;
   };
 
@@ -47,9 +48,15 @@
   /// Remove the draft from the store.
   void removeDraft(PathRef File);
 
+  llvm::IntrusiveRefCntPtr asVFS() const;
+
 private:
+  struct DraftAndTime {
+Draft Draft;
+time_t MTime;
+  };
   mutable std::mutex Mutex;
-  llvm::StringMap Drafts;
+  llvm::StringMap Drafts;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/DraftStore.cpp
===
--- clang-tools-extra/clangd/DraftStore.cpp
+++ clang-tools-extra/clangd/DraftStore.cpp
@@ -11,6 +11,8 @@
 #include "support/Logger.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -22,7 +24,7 @@
   if (It == Drafts.end())
 return None;
 
-  return It->second;
+  return It->second.Draft;
 }
 
 std::vector DraftStore::getActiveFiles() const {
@@ -75,10 +77,12 @@
  llvm::StringRef Contents) {
   std::lock_guard Lock(Mutex);
 
-  Draft  = Drafts[File];
-  updateVersion(D, Version);
-  D.Contents = Contents.str();
-  return D.Version;
+  auto Res = Drafts.try_emplace(File);
+  auto  = Res.first->getValue();
+  updateVersion(D.Draft, Version);
+  time();
+  D.Draft.Contents = std::make_shared(Contents);
+  return D.Draft.Version;
 }
 
 void DraftStore::removeDraft(PathRef File) {
@@ -87,5 +91,38 @@
   Drafts.erase(File);
 }
 
+namespace {
+
+/// A read only MemoryBuffer shares ownership of a ref counted string. The
+/// shared string object must not be modified while an owned by this buffer.
+class SharedStringBuffer : public llvm::MemoryBuffer {
+  const std::shared_ptr BufferContents;
+  const std::string Name;
+
+public:
+  BufferKind getBufferKind() const override {
+return MemoryBuffer::MemoryBuffer_Malloc;
+  }
+
+  StringRef getBufferIdentifier() const override { return Name; }
+
+  SharedStringBuffer(std::shared_ptr Data, StringRef Name)
+  : BufferContents(std::move(Data)), Name(Name) {
+assert(BufferContents && "Can't create from empty shared_ptr");
+MemoryBuffer::init(BufferContents->c_str(),
+   BufferContents->c_str() + BufferContents->size(), true);
+  }
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr DraftStore::asVFS() const {
+  auto MemFS = llvm::makeIntrusiveRefCnt();
+  std::lock_guard Guard(Mutex);
+  for (const auto  : Drafts)
+MemFS->addFile(Draft.getKey(), Draft.getValue().MTime,
+   std::make_unique(
+   Draft.getValue().Draft.Contents, Draft.getKey()));
+  return MemFS;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ 

[PATCH] D97826: [RISCV] Make use of the required features in BuiltinInfo to store that V extension builtins require 'experimental-v'.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: evandro, HsiangKai, rogfer01, khchen, arcbbb.
Herald added subscribers: StephenFan, vkmr, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
craig.topper requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

Use that to print the diagnostic in SemaChecking instead of
listing all of the builtins in a switch.

With this is the required features, IR generation will also be
able to error on this. Checking this here allows us to have a RISCV
focused error message.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97826

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3392,16 +3392,13 @@
 bool Sema::CheckRISCVBuiltinFunctionCall(const TargetInfo ,
  unsigned BuiltinID,
  CallExpr *TheCall) {
-  switch (BuiltinID) {
-  default:
-break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
-#include "clang/Basic/BuiltinsRISCV.def"
-if (!TI.hasFeature("experimental-v"))
-  return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
- << TheCall->getSourceRange();
-break;
-  }
+  // CodeGenFunction can also detect this, but this gives a better error
+  // message.
+  StringRef Features = Context.BuiltinInfo.getRequiredFeatures(BuiltinID);
+  if (Features.find("experimental-v") != StringRef::npos &&
+  !TI.hasFeature("experimental-v"))
+return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
+   << TheCall->getSourceRange();
 
   return false;
 }
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -201,6 +201,8 @@
 const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)   
\
   {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
+#define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)   
\
+{#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE},
 #include "clang/Basic/BuiltinsRISCV.def"
 };
 
Index: clang/include/clang/Basic/BuiltinsRISCV.def
===
--- clang/include/clang/Basic/BuiltinsRISCV.def
+++ clang/include/clang/Basic/BuiltinsRISCV.def
@@ -11,8 +11,12 @@
 //
 
//===--===//
 
-#if defined(BUILTIN) && !defined(RISCVV_BUILTIN)
-#define RISCVV_BUILTIN(ID, TYPE, ATTRS) BUILTIN(ID, TYPE, ATTRS)
+#if defined(BUILTIN) && !defined(TARGET_BUILTIN)
+#   define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
+#endif
+
+#if defined(TARGET_BUILTIN) && !defined(RISCVV_BUILTIN)
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) TARGET_BUILTIN(ID, TYPE, ATTRS, 
"experimental-v")
 #endif
 
 RISCVV_BUILTIN(__builtin_rvv_vadd_vv_i8m1_vl, "q8Scq8Scq8Scz", "n")


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3392,16 +3392,13 @@
 bool Sema::CheckRISCVBuiltinFunctionCall(const TargetInfo ,
  unsigned BuiltinID,
  CallExpr *TheCall) {
-  switch (BuiltinID) {
-  default:
-break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
-#include "clang/Basic/BuiltinsRISCV.def"
-if (!TI.hasFeature("experimental-v"))
-  return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
- << TheCall->getSourceRange();
-break;
-  }
+  // CodeGenFunction can also detect this, but this gives a better error
+  // message.
+  StringRef Features = Context.BuiltinInfo.getRequiredFeatures(BuiltinID);
+  if (Features.find("experimental-v") != StringRef::npos &&
+  !TI.hasFeature("experimental-v"))
+return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
+   << TheCall->getSourceRange();
 
   return false;
 }
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -201,6 +201,8 @@
 const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)   \
   {#ID, TYPE, ATTRS, nullptr, 

[PATCH] D97816: [clang] Note that -debug-pass only works with legacy PM when using new PM

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

Sorry, I forgot that codegen is still legacy PM, so `-debug-pass=Structure` 
does give the correct output. Then users need to specify both 
-debug-pass-manager and `-debug-pass=x` to get the pass information. Hmm, not a 
big deal but seems hassles. Maybe we should bridge  `-debug-pass=Structure` to 
`-debug-pass-manager` ? Or actually, this could be a feature where users can 
specify either pipeline they want the pass information for. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97816

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

[PATCH] D97825: [RISCV] Use RISCVV_BUILTIN for vector intrinsic checking.

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D97825/new/

https://reviews.llvm.org/D97825

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

[PATCH] D97824: [ObjC][ARC] Don't add operand bundle `clang.arc.attachedcall` to a call if the call already has the operand bundle

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

We could emit a call to `@llvm.objc.retain` instead of the call to 
`@llvm.objc.retainAutoreleasedReturnValue`, but this doesn't seem to happen 
often and ARC optimizer can remove or replace the call anyway, so I'm just 
letting the front-end emit `@llvm.objc.retainAutoreleasedReturnValue` for 
simplicity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97824

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

[PATCH] D97825: [RISCV] Use RISCVV_BUILTIN for vector intrinsic checking.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai created this revision.
HsiangKai added a reviewer: craig.topper.
Herald added subscribers: StephenFan, vkmr, evandro, luismarques, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, PkmX, rogfer01, shiva0217, 
kito-cheng, simoncook.
HsiangKai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There may be other BUILTINs for other extensions. Use RISCVV_BUILTIN for
vector builtin checking.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97825

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3395,7 +3395,7 @@
   switch (BuiltinID) {
   default:
 break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
 #include "clang/Basic/BuiltinsRISCV.def"
 if (!TI.hasFeature("experimental-v"))
   return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3395,7 +3395,7 @@
   switch (BuiltinID) {
   default:
 break;
-#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#define RISCVV_BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
 #include "clang/Basic/BuiltinsRISCV.def"
 if (!TI.hasFeature("experimental-v"))
   return Diag(TheCall->getBeginLoc(), diag::err_riscvv_builtin_requires_v)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97824: [ObjC][ARC] Don't add operand bundle `clang.arc.attachedcall` to a call if the call already has the operand bundle

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added a subscriber: jkorous.
ahatanak requested review of this revision.

This bug was causing the call to `replaceAllUsesWith` to crash because the old 
call instruction and the new call instruction were the same.

rdar://74957948


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97824

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/test/CodeGenObjCXX/arc-rv-attr.mm


Index: clang/test/CodeGenObjCXX/arc-rv-attr.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/arc-rv-attr.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc 
-std=c++11 -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s 
-check-prefix=CHECK
+
+id foo(void);
+
+// CHECK-LABEL: define{{.*}} void @_Z14test_list_initv(
+// CHECK: %[[CALL1:.*]] = call i8* @_Z3foov() [ "clang.arc.attachedcall"(i64 
0) ]
+// CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL1]])
+
+void test_list_init() {
+  auto t = id{foo()};
+}
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -2338,14 +2338,16 @@
   // optimization level isn't -O0 since global-isel, which is currently run at
   // -O0, doesn't know about the operand bundle.
 
+  auto *oldCall = cast(value);
+
   // FIXME: Do this when the target isn't aarch64.
   if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
-  CGF.CGM.getTarget().getTriple().isAArch64()) {
+  CGF.CGM.getTarget().getTriple().isAArch64() &&
+  !llvm::objcarc::hasAttachedCallOpBundle(oldCall)) {
 llvm::Value *bundleArgs[] = {llvm::ConstantInt::get(
 CGF.Int64Ty,
 llvm::objcarc::getAttachedCallOperandBundleEnum(IsRetainRV))};
 llvm::OperandBundleDef OB("clang.arc.attachedcall", bundleArgs);
-auto *oldCall = cast(value);
 llvm::CallBase *newCall = llvm::CallBase::addOperandBundle(
 oldCall, llvm::LLVMContext::OB_clang_arc_attachedcall, OB, oldCall);
 newCall->copyMetadata(*oldCall);


Index: clang/test/CodeGenObjCXX/arc-rv-attr.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/arc-rv-attr.mm
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -std=c++11 -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK
+
+id foo(void);
+
+// CHECK-LABEL: define{{.*}} void @_Z14test_list_initv(
+// CHECK: %[[CALL1:.*]] = call i8* @_Z3foov() [ "clang.arc.attachedcall"(i64 0) ]
+// CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL1]])
+
+void test_list_init() {
+  auto t = id{foo()};
+}
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -2338,14 +2338,16 @@
   // optimization level isn't -O0 since global-isel, which is currently run at
   // -O0, doesn't know about the operand bundle.
 
+  auto *oldCall = cast(value);
+
   // FIXME: Do this when the target isn't aarch64.
   if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
-  CGF.CGM.getTarget().getTriple().isAArch64()) {
+  CGF.CGM.getTarget().getTriple().isAArch64() &&
+  !llvm::objcarc::hasAttachedCallOpBundle(oldCall)) {
 llvm::Value *bundleArgs[] = {llvm::ConstantInt::get(
 CGF.Int64Ty,
 llvm::objcarc::getAttachedCallOperandBundleEnum(IsRetainRV))};
 llvm::OperandBundleDef OB("clang.arc.attachedcall", bundleArgs);
-auto *oldCall = cast(value);
 llvm::CallBase *newCall = llvm::CallBase::addOperandBundle(
 oldCall, llvm::LLVMContext::OB_clang_arc_attachedcall, OB, oldCall);
 newCall->copyMetadata(*oldCall);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97823: Update default arch on AIX

[Steven Wan via Phabricator via cfe-commits]

stevewan created this revision.
stevewan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On AIX, the default arch level should match the minimum supported arch level of 
the OS version


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97823

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-mcpu-default.c


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -1,10 +1,18 @@
-// Check that the target cpu defaults to power4 on AIX.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
-// RUN:-target powerpc-ibm-aix \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT %s
-// CHECK-MCPU-DEFAULT-NOT: warning:
-// CHECK-MCPU-DEFAULT: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT: "-target-cpu" "pwr4"
+// RUN:-target powerpc-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
+
+// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc-ibm-aix7.1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
+// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -399,9 +399,11 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX())
-  TargetCPUName = "pwr4";
-else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX()) {
+  unsigned major, minor, micro;
+  T.getOSVersion(major, minor, micro);
+  TargetCPUName = major == 7 && minor < 2 ? "pwr4" : "pwr7";
+} else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -1,10 +1,18 @@
-// Check that the target cpu defaults to power4 on AIX.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
-// RUN:-target powerpc-ibm-aix \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT %s
-// CHECK-MCPU-DEFAULT-NOT: warning:
-// CHECK-MCPU-DEFAULT: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT: "-target-cpu" "pwr4"
+// RUN:-target powerpc-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
+
+// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc-ibm-aix7.1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
+// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -399,9 +399,11 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX())
-  TargetCPUName = "pwr4";
-else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX()) {
+  unsigned major, minor, micro;
+  T.getOSVersion(major, minor, micro);
+  TargetCPUName = major == 7 && minor < 2 ? "pwr4" : "pwr7";
+} else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D92808#2593331 , @ahatanak wrote:

> In D92808#2593204 , @thakis wrote:
>
>> FYI, we're seeing test failures caused by this patch in iOS/arm64 builds 
>> when arc is used (simulator is fine though): 
>> https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c11
>>
>> We'll try to investigate a bit more on our side, but I wanted to give an 
>> early(ish) heads-up in case others see this or whatnot.
>>
>> Not sure if this landed before or after the 12.0 branch.
>
> Thank you. Let me know when you have more information.
>
> This isn't in release/12.x. The older version of the patch, which used 
> attributes instead of operand bundles, was in release/12.x, but got reverted 
> later.

Just to confirm, the revert is also in release/12.x, so I don't think there's 
any branch concerns here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

I think they're all singular types, like __hash_node 
.
 Actually, `__tree_node` might not be problematic.

I could look more at the types if it's helpful; I think the other issue is that 
there are a lot of libc++ tests and most are not really useful, and I'm kind of 
building them all and putting all the diffs together. It also seems like the 
`__hash_node` definitions are all present in the ctor homing dwarfdumps; it's 
just `__hash_value_type` and `__value_type` that are missing. Not sure why; I 
would have expected stuff like `__hash_node` to also be missing.

(I saw some other missing types while diffing (like `_Storage` and 
`__shared_mutex_base`) but looking at the code, it seems like both of those 
types get constructed, so not sure what's going on there..)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

[Felix Berger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa189b3b9e8bb: [clang-tidy] performance-for-range-copy: 
Dont trigger on implicit type… (authored by flx).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97577

Files:
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -60,13 +60,13 @@
 
 void negativeUserDefinedConversion() {
   Convertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 
 void negativeImplicitConstructorConversion() {
   ConstructorConvertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 
Index: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -45,10 +45,14 @@
   hasOverloadedOperatorName("*"),
   callee(
   cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()));
+  auto NotConstructedByCopy = cxxConstructExpr(
+  hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor();
+  auto ConstructedByConversion = 
cxxMemberCallExpr(callee(cxxConversionDecl()));
   auto LoopVar =
   varDecl(HasReferenceOrPointerTypeOrIsAllowed,
-  unless(hasInitializer(expr(hasDescendant(expr(anyOf(
-  materializeTemporaryExpr(), IteratorReturnsValueType)));
+  unless(hasInitializer(expr(hasDescendant(expr(
+  anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+NotConstructedByCopy, ConstructedByConversion)));
   Finder->addMatcher(
   traverse(TK_AsIs,
cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))


Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -60,13 +60,13 @@
 
 void negativeUserDefinedConversion() {
   Convertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 
 void negativeImplicitConstructorConversion() {
   ConstructorConvertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 
Index: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -45,10 +45,14 @@
   hasOverloadedOperatorName("*"),
   callee(
   cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()));
+  auto NotConstructedByCopy = cxxConstructExpr(
+  hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor();
+  auto ConstructedByConversion = cxxMemberCallExpr(callee(cxxConversionDecl()));
   auto LoopVar =
   varDecl(HasReferenceOrPointerTypeOrIsAllowed,
-  unless(hasInitializer(expr(hasDescendant(expr(anyOf(
-  materializeTemporaryExpr(), IteratorReturnsValueType)));
+  unless(hasInitializer(expr(hasDescendant(expr(
+  anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+NotConstructedByCopy, ConstructedByConversion)));
   Finder->addMatcher(
   traverse(TK_AsIs,
cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] a189b3b - [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

[Felix Berger via cfe-commits]

Author: Felix Berger
Date: 2021-03-02T20:02:48-05:00
New Revision: a189b3b9e8bb398d9fe8770956f8ad1d58c2a214

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

LOG: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type 
conversions.

This disables the check for false positive cases where implicit type conversion
through either an implicit single argument constructor or a member conversion
operator is triggered when constructing the loop variable.

Fix the test cases that meant to cover these cases.

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

Reviewed-by: hokein

Added: 


Modified: 
clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp 
b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
index 4b7a34f6c40f..8046301e3ce7 100644
--- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -45,10 +45,14 @@ void ForRangeCopyCheck::registerMatchers(MatchFinder 
*Finder) {
   hasOverloadedOperatorName("*"),
   callee(
   cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()));
+  auto NotConstructedByCopy = cxxConstructExpr(
+  hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor();
+  auto ConstructedByConversion = 
cxxMemberCallExpr(callee(cxxConversionDecl()));
   auto LoopVar =
   varDecl(HasReferenceOrPointerTypeOrIsAllowed,
-  unless(hasInitializer(expr(hasDescendant(expr(anyOf(
-  materializeTemporaryExpr(), IteratorReturnsValueType)));
+  unless(hasInitializer(expr(hasDescendant(expr(
+  anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+NotConstructedByCopy, ConstructedByConversion)));
   Finder->addMatcher(
   traverse(TK_AsIs,
cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
index e22650e10198..07b5116dfb14 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -60,13 +60,13 @@ void negativeConstReference() {
 
 void negativeUserDefinedConversion() {
   Convertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 
 void negativeImplicitConstructorConversion() {
   ConstructorConvertible C[0];
-  for (const S  : C) {
+  for (const S S1 : C) {
   }
 }
 



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

[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

[Felix Berger via Phabricator via cfe-commits]

flx added a comment.

Thank you for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97577

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

[PATCH] D97819: [CMake] Enable Polly for Fuchsia toolchain build

[Petr Hosek via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d1983f2d0a0: [CMake] Enable Polly for Fuchsia toolchain 
build (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97819

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING 
"")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING 
"")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING "")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1d1983f - [CMake] Enable Polly for Fuchsia toolchain build

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2021-03-02T16:51:16-08:00
New Revision: 1d1983f2d0a04f2516881aae258d7264624a609e

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

LOG: [CMake] Enable Polly for Fuchsia toolchain build

We want to enable the use of Polly in Fuchsia.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 7f84f74d348f..667e33c700d5 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,7 +4,7 @@ set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64;RISCV CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING 
"")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING 
"")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")



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

[PATCH] D97796: [test] Fix apparent typo in clang/test/Driver/std.c

[Nico Weber via Phabricator via cfe-commits]

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

I have no memories of this file, but I agree with your analysis :) Thanks for 
the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97796

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

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Looks like `test/FixIt/fixit-static-assert.cpp` is failing in Phabricator's 
pre-merge checks: B91556 . Please take a look 
at that before landing this; I think there's a decent chance that it's 
indicative of a real problem.


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

https://reviews.llvm.org/D95396

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

[PATCH] D97819: [CMake] Enable Polly for Fuchsia toolchain build

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: leonardchan, haowei.
Herald added a subscriber: mgorny.
Herald added a reviewer: bollu.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We want to enable the use of Polly in Fuchsia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97819

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING 
"")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING 
"")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm" CACHE STRING "")
+set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld;llvm;polly" CACHE STRING "")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97817: [CMake] Rename check-clang-tools to check-clang-tools-extra

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:81
 
-add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
+add_lit_testsuite(check-clang-tools-extra "Running the Clang tools extra' 
regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}

Nit: You need an explicit "'s" now (as in "Clang tools extra's regression 
tests"), since "extra" doesn't end with an s.



Comment at: clang-tools-extra/test/CMakeLists.txt:87
 
-set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+set_target_properties(check-clang-tools-extra PROPERTIES FOLDER "Clang tools 
extra' tests")
 

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97817

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

[PATCH] D97817: [CMake] Rename check-clang-tools to check-clang-tools-extra

[Shoaib Meenai via Phabricator via cfe-commits]

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

LGTM.

If you care about eventually getting rid of the old `check-clang-tools` alias, 
you'd want to have a mailing list announcement/deprecation notices/etc. (I 
don't think leaving it around does any harm though.)

Might want to add a release notes entry?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97817

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

[PATCH] D97817: [CMake] Rename check-clang-tools to check-clang-tools-extra

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: smeenai.
Herald added a subscriber: mgorny.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Match the top-level project name (that is the name developers would use
in LLVM_ENABLE_PROJECTS) with the name of the target, and provide alias
for backwards compatibility.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97817

Files:
  clang-tools-extra/test/CMakeLists.txt


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -78,14 +78,18 @@
   endif()
 endforeach()
 
-add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression 
tests"
+add_lit_testsuite(check-clang-tools-extra "Running the Clang tools extra' 
regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
-set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' 
tests")
+set_target_properties(check-clang-tools-extra PROPERTIES FOLDER "Clang tools 
extra' tests")
 
-add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
+add_lit_testsuites(CLANG-TOOLS-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+# Provide `check-clang-tools` alias for backwards compatibility.
+add_custom_target(check-clang-tools)
+add_dependencies(check-clang-tools check-clang-tools-extra)


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -78,14 +78,18 @@
   endif()
 endforeach()
 
-add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
+add_lit_testsuite(check-clang-tools-extra "Running the Clang tools extra' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   ARGS ${CLANG_TOOLS_TEST_EXTRA_ARGS}
   )
 
-set_target_properties(check-clang-tools PROPERTIES FOLDER "Clang extra tools' tests")
+set_target_properties(check-clang-tools-extra PROPERTIES FOLDER "Clang tools extra' tests")
 
-add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
+add_lit_testsuites(CLANG-TOOLS-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+# Provide `check-clang-tools` alias for backwards compatibility.
+add_custom_target(check-clang-tools)
+add_dependencies(check-clang-tools check-clang-tools-extra)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97101: [Coverage] Emit zero count gap region between statements if first statements contains return, throw, goto, co_return

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Thanks for the patient explanation.




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1391
+// Clear DeferredRegion because we don't need to complete it after switch.
+DeferredRegion = None;
+

Is the reason why we don't need to complete it because the logic in VisitStmt 
handles filling in the gap region? What happens if this isn't cleared, or if 
there are nested switches?



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:942
 pushRegion(Counter::getZero());
-auto  = getRegion();
-ZeroRegion.setDeferred(true);
-LastTerminatedRegion = {EndLoc, RegionStack.size()};
+if (!HasTerminateStmt) {
+  auto  = getRegion();

zequanwu wrote:
> vsk wrote:
> > zequanwu wrote:
> > > vsk wrote:
> > > > What's supposed to be the difference between the zero region pushed 
> > > > after a `return;` vs. after a `break;`?
> > > What I think is `DeferredRegion` is only used for `break;` and 
> > > `continue`. Other terminate statements like `return;`, `throw` etc will 
> > > use the logic in `VisitStmt` to emit gap region. So, I added this 
> > > condition to separate the two cases.
> > What do you think of the notion of using the gaps inserted in VisitStmt to 
> > replace the whole deferred region system? Is it something that might be 
> > feasible (if perhaps out of scope for this patch), or do you see a 
> > fundamental reason it can't/shouldn't be done?
> I think it is feasible. For break and continue, they only affect the 
> statements after them in the same block. For other terminate statements, they 
> affect all the statements after them even outside the block, see example 
> below. Also instead of emitting a zero gap region in VisitStmt, we need emit 
> gap region with (previous counter - current statement counter).
> 
> ```
> while (cond) {
> ...
> break; // This affects statements' count until the end of the while body.
> ...
> }
> 
> while (cond) {
> ...
> return; // This affects statements' count until the end of the function body.
> ...
> }
> ```
> 
Thanks, that's really helpful. It's clicking now that the main difference is 
that the deferred region sticks around after we're done visiting a block.



Comment at: clang/test/CoverageMapping/classtemplate.cpp:83
 std::map Test4() { // CHECK-INIT-LIST: File 0, [[@LINE]]:28 -> 
[[@LINE+3]]:2 = #0
-  abort();
+  abort();   // CHECK-INIT-LIST-NEXT: Gap,File 0, 
[[@LINE]]:11 -> [[@LINE+1]]:3 = 0
   return std::map{{0, 0}}; // CHECK-INIT-LIST-NEXT: [[@LINE]]:3 -> 
[[@LINE]]:36 = 0

Nice, this is from the special handling of noreturn calls.



Comment at: clang/test/CoverageMapping/deferred-region.cpp:19
 return;
-  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:3 = (#0 - #1)
+  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:3 = 0
 

Nice, this should amount to the same end result, it's a smaller encoding.



Comment at: clang/test/CoverageMapping/deferred-region.cpp:43
   if (true)
-return; // CHECK: Gap,File 0, [[@LINE]]:11
+return;
   else

I'm confused by this change. Do we lose the gap here? I assumed it was needed 
to prevent the condition count from `if (true)` from kicking in after the 
`return`?



Comment at: clang/test/CoverageMapping/switch.cpp:62
   default:  // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #4
 break;  // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> 
[[@LINE-1]]:10 = #4, (#0 - #4)
+  }

The blank space after `default: break;` and before the closing '}' should have 
the same count as the switch condition, I thought (this goes for all of the 
unreachable code within a switch body)? The idea is to prevent 'not-executed' 
regions from appearing after the `break` stmt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97101

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

[PATCH] D97816: [clang] Note that -debug-pass only works with legacy PM when using new PM

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added a reviewer: ychen.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Follow-up to https://reviews.llvm.org/D97810.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97816

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Misc/debug-pass-npm.c


Index: clang/test/Misc/debug-pass-npm.c
===
--- /dev/null
+++ clang/test/Misc/debug-pass-npm.c
@@ -0,0 +1,8 @@
+// RUN: %clang -fexperimental-new-pass-manager %s -c -o /dev/null -mllvm 
-debug-pass=Structure 2>&1 | FileCheck %s
+// RUN: %clang -fexperimental-new-pass-manager %s -c -o /dev/null 
-fdebug-pass-arguments 2>&1 | FileCheck %s
+// RUN: %clang -flegacy-pass-manager %s -c -o /dev/null -fdebug-pass-arguments 
2>&1 | FileCheck %s --check-prefix=LPM
+
+void foo() {}
+
+// CHECK: does not work
+// LPM-NOT: does not work
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1179,6 +1179,14 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
+  if (llvm::legacy::debugPassSpecified()) {
+llvm::errs()
+<< "-debug-pass/-fdebug-pass-* does not work with the new pass manager 
"
+   "for the middle-end optimization pipeline, either use "
+   "-fdebug-pass-manager, or use the legacy PM "
+   "(-flegacy-pass-manager)\n";
+  }
+
   bool RequiresCodeGen = (Action != Backend_EmitNothing &&
   Action != Backend_EmitBC &&
   Action != Backend_EmitLL);


Index: clang/test/Misc/debug-pass-npm.c
===
--- /dev/null
+++ clang/test/Misc/debug-pass-npm.c
@@ -0,0 +1,8 @@
+// RUN: %clang -fexperimental-new-pass-manager %s -c -o /dev/null -mllvm -debug-pass=Structure 2>&1 | FileCheck %s
+// RUN: %clang -fexperimental-new-pass-manager %s -c -o /dev/null -fdebug-pass-arguments 2>&1 | FileCheck %s
+// RUN: %clang -flegacy-pass-manager %s -c -o /dev/null -fdebug-pass-arguments 2>&1 | FileCheck %s --check-prefix=LPM
+
+void foo() {}
+
+// CHECK: does not work
+// LPM-NOT: does not work
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1179,6 +1179,14 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
+  if (llvm::legacy::debugPassSpecified()) {
+llvm::errs()
+<< "-debug-pass/-fdebug-pass-* does not work with the new pass manager "
+   "for the middle-end optimization pipeline, either use "
+   "-fdebug-pass-manager, or use the legacy PM "
+   "(-flegacy-pass-manager)\n";
+  }
+
   bool RequiresCodeGen = (Action != Backend_EmitNothing &&
   Action != Backend_EmitBC &&
   Action != Backend_EmitLL);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In some cases `BugReport.isInteresting(InnerPointerVal.getAsSymbol())` would 
yield us exactly what we want. But if there's no symbol we have no choice but 
to interact with the `trackExpressionValue` facility and this may turn out to 
be pretty challenging.

We could, for instance, teach it to mark //exploded nodes// as interesting when 
it changes tracking mode. That'd be a completely new form of interestingness 
for us to track. Or maybe pairs (exploded node, expression) so that to be more 
specific. Then we could query it from our note tag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D97340: [HIP] Support Spack packages

[Artem Belevich via Phabricator via cfe-commits]

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

LGTM modulo couple of nits.




Comment at: clang/include/clang/Driver/Options.td:3535-3536
   HelpText<"Print the registered targets">;
+def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
+  HelpText<"Print the paths used for finding ROCm installation">;
 def private__bundle : Flag<["-"], "private_bundle">;

yaxunl wrote:
> tra wrote:
> > Perhaps the existing `print-search-dirs` option would do the job?
> The concern is similar to https://reviews.llvm.org/D79210:
> 
> There may be tools parsing print-search-dirs output, changing it may break 
> those tools.
> 
OK. New option it is then,



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:163
 
 // For candidate specified by --rocm-path we do not do strict check.
+const SmallVectorImpl &

I'm not quite sure which part of the code is related to the `do not do strict 
check`.  Perhaps move the comment closer to the code it describes and add the 
details what we'd do otherwise, so one can understand what's missing.

Also, adding a comment describing what the function does (returns candidate 
list, populating it once, if necessary) would be great.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:361-372
   auto Path = CandidatePath;
+  // Device library built by SPACK is installed to
+  // /rocm-device-libs-- directory.
+  if (Candidate.isSPACK()) {
+auto SPACKPath = findSPACKPackage(D, Path, "rocm-device-libs",
+  Candidate.SPACKReleaseStr);
+if (!SPACKPath.empty())

This could be simplified to
```
 auto MaybeSpackPath = Candidate.isSPACK() ? findSPACKPackage() : "";
 auto Path = MaybeSpackPath.empty() ? CandidatePath : MaybeSpackPath;

```

We could further simplify it if we move  the `isSpack()` check into 
findSPACKPackage and make it return an empty string if it's not. 



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:180
   if (!RocmPathArg.empty()) {
-Candidates.emplace_back(RocmPathArg.str());
-return Candidates;
+ROCmSearchDirs.emplace_back(RocmPathArg.str());
+DoPrintROCmSearchDirs();

yaxunl wrote:
> tra wrote:
> > We call getInstallationPathCandidates() more more than one place, so we may 
> > end up with the same path added multiple times.
> > 
> > I think population of the candidate list should be separated from reading 
> > it back, and should be done only once.
> > E.g.
> > ```
> >   if (!isCandidateListReady) 
> >  initCandidateList(); // populates ROCmSearchDirs, sets 
> > isCandidateListReady
> >   return ROCmSearchDirs;
> > ```
> > 
> > 
> We check whether ROCmSearchDirs is populated on line 166 and only populate it 
> if not.
Agreed, the code works. That said, the fact that I didn't notice that is a 
point in case -- it's not obvious.
It's much easier to keep track of what's going on when one function does one 
things without changing its behavior from one call to another.
I'd at least add a comment around line 166 saying that just return the list if 
we've already populated it.




Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:207
+// installation candidate for SPACK.
+if (ParentName.startswith("llvm-amdgpu-")) {
+  auto SPACKReleaseStr =

yaxunl wrote:
> tra wrote:
> > What would be a consequence of mistakingly detecting a non-SPACK 
> > installation as SPACK?
> > 
> > I'm still not quite comfortable with using a path name, which can be 
> > arbitrarily named by a user, as a way to detect particular installation 
> > method.
> > Do SPACK packages have something *inside* the package directory that we 
> > could use to tell that it is indeed an SPACK package? Some sort of metadata 
> > or version file, perhaps?
> > 
> > It's probably OK to rely on the path naming scheme, but there are still 
> > corner cases it can't handle.
> > I could have a setup when the path-name based detection would still fail 
> > despite considering both the reaolved and unresolved symlink names.
> > E.g. 
> > ```
> > /pkg/llvm-amdgpu-hash -> /pkg/some-other-real-dir-name
> > /usr/local/bin/clang -> /pkg/llvm-amdgpu-hash/bin/clang
> > ```
> > If I compile with `/usr/local/bin/clang` neither the realpath nor the 
> > unresolved symlink will have 'llvm-amdgpu'. 
> > 
> > Granted, it's a somewhat contrived setup. Using the name as the hint would 
> > work for the standard setups, so we can live with that, provided that we 
> > can always tell compiler where to find the files it needs, if the user has 
> > an unusual install.
> > 
> llvm-amdgpu Spack installation does not add extra files. It uses normal llvm 
> cmake files to build and differs from a non-Spack llvm build only by name. 
> However, its name follows a very unusual pattern 
> llvm-amdgpu--, where  is a string 

[PATCH] D95691: Implement P2173 for attributes on lambdas

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269
+def CXXPre2BCompatPedantic :
+  DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic", [CXXPre2BCompat]>;
 

aaron.ballman wrote:
> rsmith wrote:
> > rjmccall wrote:
> > > rsmith wrote:
> > > > rjmccall wrote:
> > > > > Quuxplusone wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > Uh, I think we're a couple standard releases past the point at 
> > > > > > > > which we should have reconsidered this schema.  I guess the 
> > > > > > > > problem is that we can't say `-Wpre-c++23-compat` without 
> > > > > > > > jumping the gun.  Is there a problem with `-Wc++20-compat` and 
> > > > > > > > then having the earlier warning groups imply the later ones?  
> > > > > > > > That seems to be what we do with `-Wc++98-compat`; did we 
> > > > > > > > abandon that approach intentionally?
> > > > > > > @rsmith may have more background here. I was following the 
> > > > > > > pattern already in the file, but I tend to agree that this 
> > > > > > > pattern is not leading us somewhere good. FWIW, I ran into a 
> > > > > > > similar situation with this on the C side of things in D95396, so 
> > > > > > > we should probably be consistent there too.
> > > > > > My understanding is that the //command-line user// is expected to 
> > > > > > pass
> > > > > > - `clang++ -std=c++20 -Wc++11-compat` to indicate "I want 
> > > > > > //actually// to compile in C++20 mode, but give me warnings about 
> > > > > > anything that would prevent compiling in C++11 mode"
> > > > > > - `clang++ -std=c++17 -Wc++14-compat` to indicate "I want 
> > > > > > //actually// to compile in C++17 mode, but give me warnings about 
> > > > > > anything that would prevent compiling in C++14 mode"
> > > > > > - `clang++ -std=c++14 -Wc++20-compat` to indicate "I want 
> > > > > > //actually// to compile in C++14 mode, but give me warnings about 
> > > > > > anything that would prevent compiling in C++20 mode" — EXCEPT that 
> > > > > > I think this is not supported. My impression is that 
> > > > > > forward-compatibility warnings are generally just rolled into 
> > > > > > `-Wall` and not handled separately beyond that?
> > > > > > 
> > > > > > I don't think any human user is expected to pass 
> > > > > > `-Wc++98-c++11-c++14-c++17-c++20-compat` by hand; it's just an 
> > > > > > internal name for a particular subset of `-Wc++98-compat`.
> > > > > > 
> > > > > > IOW, we could choose a new naming scheme for it, but that would be 
> > > > > > a purely internal change that won't affect how command-line users 
> > > > > > interact with Clang at all (for better and for worse).
> > > > > Diagnostic groups can both directly contain diagnostics and imply 
> > > > > other diagnostic groups, so I don't think there's any reason to make 
> > > > > a dedicated group just to contain the new diagnostics in e.g. 
> > > > > `-Wc++14-compat` except to allow someone turn on those warnings 
> > > > > separately.  And it does show up to users as the warning group under 
> > > > > `-fdiagnostics-show-option` (which is the default).
> > > > @Quuxplusone's comment describes the intent. `-std=c++20 
> > > > -Wc++14-compat` should give a more or less complete list of reasons why 
> > > > the code would not compile in C++14 (at least on the language side; we 
> > > > don't check for stdlib compatibility). The other direction -- 
> > > > `-std=c++11 -Wc++14-compat` -- is more of a best-effort check for 
> > > > things that we've seen cause problems in practice and can easily 
> > > > detect. (As a consequence, I don't think there's any subset/superset 
> > > > relation between `-Wc++X-compat` and `-Wc++Y-compat`.)
> > > > 
> > > > I'd be happy to see these groups renamed to `-Wpre-c++20-compat` or 
> > > > similar. Warning group synonyms are relatively cheap, so I wouldn't be 
> > > > worried about adding a `-Wpre-c++2b-compat` now and renaming it to 
> > > > `-Wpre-c++23-compat` flag later.
> > > > 
> > > > (As an aside, it'd be handy if there were some way to mark a 
> > > > `DiagGroup` as existing only for grouping purposes, so that we could 
> > > > avoid exposing a `-W` flag for cases where groups are added for 
> > > > internal reasons.)
> > > Okay.  It looks like `-Wc++X-compat` is consistently (1) all the 
> > > this-feature-used-not-to-exist diagnostics from C++X and later plus (2) 
> > > warnings about deprecation and semantic changes introduced by exactly 
> > > version X.  This seems like an unfortunate pairing, basically caused by 
> > > the option names not being very clear about what kind of compatibility 
> > > they mean.  If we want @Quuxplusone's interpretation, which I agree is a 
> > > natural human interpretation of those command lines, we'll need special 
> > > support for it in diagnostic-option handling, so that we 

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D97411#2598625 , @akhuang wrote:

> In D97411#2595155 , @dblaikie wrote:
>
>> In D97411#2595049 , @akhuang wrote:
>>
>>> In D97411#2594345 , @ldionne wrote:
>>>
 I don't have an opinion about the attribute itself. I do have an opinion 
 about using that attribute in libc++ instead of fixing the underlying 
 issue (I think we shouldn't do it). Can you confirm what the problematic 
 types are? In another patch I saw `__hash_node`, `__hash_value_type`, 
 `__tree_node` and `__value_type`. Is that it?
>>>
>>> Not entirely sure - those were pointed out as types with missing debug 
>>> info, but there might be more. I tried looking for types in libc++ that 
>>> have a `value_type` member, since those seem to follow a similar pattern. 
>>> Possibly `__forward_list_node`?
>>
>> Since we'll need to identify this list of types either way (to attribute or 
>> to fix) might be worth making the list - I'd guess building all the libc++ 
>> tests with/without ctor homing and seeing which types go missing as a result 
>> might be informative?
>
> I started looking into some diffs of debug info in libc++ tests, but it's 
> pretty hard to tell what's different - as far as I can see, there are just a 
> bunch of `__hash_value_type`s and `__value_type`s.

Happy to help if there's anything I can do there (did a lot of comparisons like 
this when adding other type homing strategies and diffing GCC and Clang output 
in the past) dumping the fully qualified names (or mangled names) of all the 
types (llvm-dwarfdump has a mode where it specifically dumps the names of types 
in type units that I've used for this purpose in the past, for instance) then 
diffing, stripping off uninteresting template parameters to deduplicate, etc, 
possibly.

> If I look in Visual Studio's debug info (on a file where I just construct all 
> the libc++ types) I get errors for `__list_node`, `__tree_node`, and 
> `__hash_node`. That's probably about it, and if there end up being more, it 
> should be pretty straightforward to fix those later?

Yeah - though having at least a rough list to get a sense of the scale of the 
work's probably helpful. Maybe that list is these 3-4 names. (are they each 
singular types - or are there several distinct uses of "__tree_node" as a name 
in different contexts that would need to be fixed separately, for instance?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[libunwind] 5c3fc50 - [libunwind] [risc-v] This patch is for fixing

[Kamlesh Kumar via cfe-commits]

Author: Kamlesh Kumar
Date: 2021-03-03T04:32:47+05:30
New Revision: 5c3fc5093aaf7d9422459d295eb4eb769abcbaa4

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

LOG: [libunwind] [risc-v] This patch is for fixing
immediate build failure when Cross Unwinding enabled.
Follow up patch will cleanup some Macros handling.

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

Added: 


Modified: 
libunwind/src/Registers.hpp

Removed: 




diff  --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index 1d23f97aedfb..aea84cc22721 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -3731,26 +3731,35 @@ inline const char 
*Registers_hexagon::getRegisterName(int regNum) {
 /// Registers_riscv holds the register state of a thread in a RISC-V
 /// process.
 
-# if __riscv_xlen == 32
+// This check makes it safe when LIBUNWIND_ENABLE_CROSS_UNWINDING enabled.
+# ifdef __riscv
+#  if __riscv_xlen == 32
 typedef uint32_t reg_t;
-# elif __riscv_xlen == 64
+#  elif __riscv_xlen == 64
 typedef uint64_t reg_t;
-# else
-#  error "Unsupported __riscv_xlen"
-# endif
+#  else
+#   error "Unsupported __riscv_xlen"
+#  endif
 
-# if defined(__riscv_flen)
-#  if __riscv_flen == 64
+#  if defined(__riscv_flen)
+#   if __riscv_flen == 64
 typedef double fp_t;
-#  elif __riscv_flen == 32
+#   elif __riscv_flen == 32
 typedef float fp_t;
+#   else
+#error "Unsupported __riscv_flen"
+#   endif
 #  else
-#   error "Unsupported __riscv_flen"
+// This is just for supressing undeclared error of fp_t.
+typedef double fp_t;
 #  endif
 # else
-// This is just for supressing undeclared error of fp_t.
+// Use Max possible width when cross unwinding
+typedef uint64_t reg_t;
 typedef double fp_t;
-# endif
+# define __riscv_xlen 64
+# define __riscv_flen 64
+#endif
 
 /// Registers_riscv holds the register state of a thread.
 class _LIBUNWIND_HIDDEN Registers_riscv {



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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/www/cxx_status.html:1206-1210
+
+  More implicit moves
+  https://wg21.link/p1825r0;>P1825R0 (DR)
+  Clang 13
+

We have, so far, listed DR papers only in the section of `cxx_status` for the 
earliest language they affect (that is: the section for C++XY lists the papers 
that we implement in C++XY but not in earlier versions). I'm not tied to that 
approach, but if we want to switch away from that (and list DR papers multiple 
times) we should do so consistently throughout this page -- also, if we go that 
way, maybe the earlier reference to the paper should contain a link to the 
later one?

The preceding paper, P0593R6, is a bit of a special case because while -- per 
our "DRs apply retroactively as far back as they make sense" policy -- it 
should apply retroactively all the way back to C++98, it only actually affects 
Clang's behavior in C++20 mode, as part of P0784R7, which allowed 
pseudo-destruction in constant expressions. Maybe there's a clearer way to 
present that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D97340: [HIP] Support Spack packages

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 327596.
yaxunl marked 5 inline comments as done.
yaxunl added a comment.

revised by Artem's comments


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

https://reviews.llvm.org/D97340

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/ROCm.h
  
clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/bin/.hipVersion
  
clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include/hip/hip_runtime.h
  
clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/.keep
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/asanrtl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ockl.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_daz_opt_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_finite_only_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1010.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1011.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_1012.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_803.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_900.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_isa_version_908.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_unsafe_math_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_unsafe_math_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_wavefrontsize64_off.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/oclc_wavefrontsize64_on.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/ocml.bc
  
clang/test/Driver/Inputs/rocm-spack/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/opencl.bc
  clang/test/Driver/rocm-detect.hip

Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -16,14 +16,75 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,GFX902-DEFAULTLIBS %s
 
-
 // RUN: %clang -### -v -target x86_64-linux-gnu --cuda-gpu-arch=gfx902 -nogpulib \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test ROCm installation built by SPACK by invoke clang at %T/rocm-spack/llvm-amdgpu-*
+// directory through a soft link.
+
+// RUN: rm -rf %T/rocm-spack
+// RUN: cp -r %S/Inputs/rocm-spack %T
+// RUN: ln -fs %clang %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=SPACK %s
+
+// Test SPACK installation with multiple hip and rocm-device-libs packages of the same
+// ROCm release. Clang cannot determine which one to use and emit diagnostics. --hip-path
+// and --rocm-device-lib-path can be used to specify them.
+
+// RUN: cp -r %T/rocm-spack/hip-* %T/rocm-spack/hip-4.0.0-abcd
+// RUN: cp -r %T/rocm-spack/rocm-device-libs-* %T/rocm-spack/rocm-device-libs-4.0.0-efgh
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang 

[PATCH] D97340: [HIP] Support Spack packages

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:3535-3536
   HelpText<"Print the registered targets">;
+def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
+  HelpText<"Print the paths used for finding ROCm installation">;
 def private__bundle : Flag<["-"], "private_bundle">;

tra wrote:
> Perhaps the existing `print-search-dirs` option would do the job?
The concern is similar to https://reviews.llvm.org/D79210:

There may be tools parsing print-search-dirs output, changing it may break 
those tools.




Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:180
   if (!RocmPathArg.empty()) {
-Candidates.emplace_back(RocmPathArg.str());
-return Candidates;
+ROCmSearchDirs.emplace_back(RocmPathArg.str());
+DoPrintROCmSearchDirs();

tra wrote:
> We call getInstallationPathCandidates() more more than one place, so we may 
> end up with the same path added multiple times.
> 
> I think population of the candidate list should be separated from reading it 
> back, and should be done only once.
> E.g.
> ```
>   if (!isCandidateListReady) 
>  initCandidateList(); // populates ROCmSearchDirs, sets 
> isCandidateListReady
>   return ROCmSearchDirs;
> ```
> 
> 
We check whether ROCmSearchDirs is populated on line 166 and only populate it 
if not.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:207
+// installation candidate for SPACK.
+if (ParentName.startswith("llvm-amdgpu-")) {
+  auto SPACKReleaseStr =

tra wrote:
> What would be a consequence of mistakingly detecting a non-SPACK installation 
> as SPACK?
> 
> I'm still not quite comfortable with using a path name, which can be 
> arbitrarily named by a user, as a way to detect particular installation 
> method.
> Do SPACK packages have something *inside* the package directory that we could 
> use to tell that it is indeed an SPACK package? Some sort of metadata or 
> version file, perhaps?
> 
> It's probably OK to rely on the path naming scheme, but there are still 
> corner cases it can't handle.
> I could have a setup when the path-name based detection would still fail 
> despite considering both the reaolved and unresolved symlink names.
> E.g. 
> ```
> /pkg/llvm-amdgpu-hash -> /pkg/some-other-real-dir-name
> /usr/local/bin/clang -> /pkg/llvm-amdgpu-hash/bin/clang
> ```
> If I compile with `/usr/local/bin/clang` neither the realpath nor the 
> unresolved symlink will have 'llvm-amdgpu'. 
> 
> Granted, it's a somewhat contrived setup. Using the name as the hint would 
> work for the standard setups, so we can live with that, provided that we can 
> always tell compiler where to find the files it needs, if the user has an 
> unusual install.
> 
llvm-amdgpu Spack installation does not add extra files. It uses normal llvm 
cmake files to build and differs from a non-Spack llvm build only by name. 
However, its name follows a very unusual pattern llvm-amdgpu--, 
where  is a string of 32 alphanumerical characters. I think we could 
detect it by checking the length of .



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:354-372
+  CandidatesInfo CanInfo;
+  if (!HIPPathArg.empty())
+CanInfo.Candidates.emplace_back(HIPPathArg.str(), /*StrictChecking=*/true);
+  else
+CanInfo = getInstallationPathCandidates();
   auto  = D.getVFS();
 

tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > So, normally `--hip-path` would point to the HIP package installation. 
> > > However, is clang installation path contains `llvm-amdgpu` (and thus 
> > > CanInfo.SPACKReleaseStr is set to whatever version may follow),
> > > then `--hip-path` (and other candidate paths) directories will also be 
> > > treated as an SPACK package directory and, if the `hip` package is found 
> > > there, then *that* package directory will be used as the actual path to 
> > > be used.
> > > 
> > > I see several potential issues with that.
> > > 
> > > - if clang path contains llvm-amdgpu, but it is *not* installed as an 
> > > SPACK package We have no control over user's choice of the name for the 
> > > installation path.
> > > - what if clang is installed as an SPCAK, but is invoked via a symlink 
> > > with the name that does not contain `llvm-amdgpu`
> > > - the fact that `--hip-path`/`--rocm-path` behavior may change based on 
> > > clang's executable path is rather peculiar. User-supplied parameters 
> > > should not change their meaning depending on what you call the 
> > > executable. 
> > > 
> > > I'd be OK to derive spack-based paths if user didn't provide the paths 
> > > explicitly, but I don't think we should add any magic to explicit 
> > > overrides.
> > > 
> > > 
> > Will make SPACKReleaseStr a member of Candidate and allow mixed Spack 
> > candidates and non-Spack candidates. If clang path contains llvm-amdgpu but 
> > is not a 

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

In D97411#2595155 , @dblaikie wrote:

> In D97411#2595049 , @akhuang wrote:
>
>> In D97411#2594345 , @ldionne wrote:
>>
>>> I don't have an opinion about the attribute itself. I do have an opinion 
>>> about using that attribute in libc++ instead of fixing the underlying issue 
>>> (I think we shouldn't do it). Can you confirm what the problematic types 
>>> are? In another patch I saw `__hash_node`, `__hash_value_type`, 
>>> `__tree_node` and `__value_type`. Is that it?
>>
>> Not entirely sure - those were pointed out as types with missing debug info, 
>> but there might be more. I tried looking for types in libc++ that have a 
>> `value_type` member, since those seem to follow a similar pattern. Possibly 
>> `__forward_list_node`?
>
> Since we'll need to identify this list of types either way (to attribute or 
> to fix) might be worth making the list - I'd guess building all the libc++ 
> tests with/without ctor homing and seeing which types go missing as a result 
> might be informative?

I started looking into some diffs of debug info in libc++ tests, but it's 
pretty hard to tell what's different - as far as I can see, there are just a 
bunch of `__hash_value_type`s and `__value_type`s.

If I look in Visual Studio's debug info (on a file where I just construct all 
the libc++ types) I get errors for `__list_node`, `__tree_node`, and 
`__hash_node`. That's probably about it, and if there end up being more, it 
should be pretty straightforward to fix those later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D96347: Include function return type in JSON AST dumps.

[Tem Taime via Phabricator via cfe-commits]

Temtaime added a comment.

Merge it, please. I badly need this field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96347

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

[PATCH] D97417: [clangd] use a compatible preamble for the first AST built

[Quentin Chateau via Phabricator via cfe-commits]

qchateau added a comment.

I am much more afraid of providing bad results than I am afraid of degrading 
performance. I mean, eventually the "real" preamble is built, and the results 
are just as correct as before, but it may yield incorrect results until we 
invalidate the AST.
That is especially the case as I'm not very experienced in compiler design and 
I don't have a deep understanding of what happens behind the curtains when 
using a PCH.
There are probably cases where we degrade perf, but overall my experience with 
this patch is overwhelmingly positive, especially for "long coding sessions".

> I am afraid this can happen more than twice

I'm not sure I follow, I see these different situations (in TUScheduler):

- runWithPreamble
  - PreambleConsistency::Stale -> will wait for a "real" preamble, no change in 
behavior
  - PreambleConsistency::StaleOrAbsent
- a preamble is available, we'll use it, no change in behavior
- the preamble is not yet built, we'll use a compatible preamble instead -> 
this means instant partial completion !
- runWithAST
  - a preamble is available, we'll use it, no change in behavior
  - the preamble is not yet built, we'll use a compatible preamble to start 
building the AST straight away -> here there is a risk of double work, if the 
compatible preamble is useless, we'll start building the AST *while* we're also 
building the preamble

But I don't see how we can do worse than building the AST and the preamble at 
the same time ?

> could you give a bit more information on average preamble build latency in 
> the files you are testing this with

Extracts from VSCode logs, between the file being opened and the semantic token 
request completion.
Also worth noting that we have partial completion with 0 delay (with the 
compatible preamble)

Without the patch:

  I[22:40:57.317] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.h version 1 
with command inferred from 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.cpp
  I[22:41:07.893] --> reply:textDocument/semanticTokens/full(1) 9921 ms
  
  I[22:41:16.526] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.cpp version 1 
with command
  I[22:41:38.401] --> reply:textDocument/semanticTokens/full(12) 21877 ms
  
  I[22:41:53.861] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.h version 
1 with command inferred from 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.cpp
  I[22:42:06.006] --> reply:textDocument/semanticTokens/full(23) 12144 ms
  
  I[22:42:08.619] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.cpp 
version 1 with command
  I[22:42:21.920] --> reply:textDocument/semanticTokens/full(28) 13278 ms

With the patch

  I[22:36:32.828] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.h version 1 
with command inferred from 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.cpp
  I[22:36:43.211] --> reply:textDocument/semanticTokens/full(3) 9675 ms
  
  I[22:36:52.331] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/ParsedAST.cpp version 1 
with command
  I[22:36:58.206] --> reply:textDocument/semanticTokens/full(9) 5881 ms
  
  I[22:37:13.474] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.h version 
1 with command inferred from 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.cpp
  I[22:37:15.840] --> reply:textDocument/semanticTokens/full(15) 2351 ms
  
  I[22:37:23.939] ASTWorker building file 
/home/quentin/dev/llvm-project/clang-tools-extra/clangd/IncludeFixer.cpp 
version 1 with command
  I[22:37:25.030] --> reply:textDocument/semanticTokens/full(20) 1089 ms


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97417

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

[PATCH] D97156: [ASTMatchers] Make Param functors variadic

[Nathan James via Phabricator via cfe-commits]

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

LGTM, assuming none of the later changes have effected the documentation script.

In D97156#2598398 , @steveire wrote:

> In D97156#2593209 , @njames93 wrote:
>
>> Is it not nicer to name the class `PolymorphicMatcher` and do away with 
>> `WithParamN`, It may require partial specialization for the empty parameters 
>> case.
>> But I think it would be worth it.
>
> I'm not sure any partial specialization is needed?

Oh neat, I wasn't sure if the empty tuple would cause some issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

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

[PATCH] D97806: [clang-query] Fix help text after D91918

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: aaron.ballman.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

After rG5e1801813d93210acae84ff3c68a01512c2df9bc 
 The help 
command still lists IgnoreImplicitCastsAndParentheses as a valid option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97806

Files:
  clang-tools-extra/clang-query/Query.cpp


Index: clang-tools-extra/clang-query/Query.cpp
===
--- clang-tools-extra/clang-query/Query.cpp
+++ clang-tools-extra/clang-query/Query.cpp
@@ -48,8 +48,6 @@
 "AsIs"
 "Print and match the AST as clang sees it.  This mode is the "
 "default.\n"
-"IgnoreImplicitCastsAndParentheses  "
-"Omit implicit casts and parens in matching and dumping.\n"
 "IgnoreUnlessSpelledInSource "
 "Omit AST nodes unless spelled in the source.\n"
 "  set output   "


Index: clang-tools-extra/clang-query/Query.cpp
===
--- clang-tools-extra/clang-query/Query.cpp
+++ clang-tools-extra/clang-query/Query.cpp
@@ -48,8 +48,6 @@
 "AsIs"
 "Print and match the AST as clang sees it.  This mode is the "
 "default.\n"
-"IgnoreImplicitCastsAndParentheses  "
-"Omit implicit casts and parens in matching and dumping.\n"
 "IgnoreUnlessSpelledInSource "
 "Omit AST nodes unless spelled in the source.\n"
 "  set output   "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbca3e24139cc: [clangd] Move DraftStore from ClangdLSPServer 
into ClangdServer. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97738

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/DraftStore.cpp
  clang-tools-extra/clangd/DraftStore.h
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/test/crash-non-added-files.test
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -50,7 +50,7 @@
  const clangd::RenameOptions );
 
 llvm::Expected
-runFormatFile(ClangdServer , PathRef File, StringRef Code);
+runFormatFile(ClangdServer , PathRef File, llvm::Optional);
 
 SymbolSlab runFuzzyFind(const SymbolIndex , StringRef Query);
 SymbolSlab runFuzzyFind(const SymbolIndex , const FuzzyFindRequest );
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -115,9 +115,9 @@
 }
 
 llvm::Expected
-runFormatFile(ClangdServer , PathRef File, StringRef Code) {
+runFormatFile(ClangdServer , PathRef File, llvm::Optional Rng) {
   llvm::Optional> Result;
-  Server.formatFile(File, Code, capture(Result));
+  Server.formatFile(File, Rng, capture(Result));
   return std::move(*Result);
 }
 
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -27,6 +27,7 @@
 namespace {
 
 using llvm::Failed;
+using llvm::FailedWithMessage;
 using llvm::HasValue;
 
 MATCHER_P2(Pos, Line, Col, "") {
@@ -802,6 +803,226 @@
   EXPECT_FALSE(isKeyword("override", LangOpts));
 }
 
+struct IncrementalTestStep {
+  llvm::StringRef Src;
+  llvm::StringRef Contents;
+};
+
+int rangeLength(llvm::StringRef Code, const Range ) {
+  llvm::Expected Start = positionToOffset(Code, Rng.start);
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  assert(Start);
+  assert(End);
+  return *End - *Start;
+}
+
+/// Send the changes one by one to updateDraft, verify the intermediate results.
+void stepByStep(llvm::ArrayRef Steps) {
+  std::string Code = Annotations(Steps.front().Src).code().str();
+
+  for (size_t I = 1; I < Steps.size(); I++) {
+Annotations SrcBefore(Steps[I - 1].Src);
+Annotations SrcAfter(Steps[I].Src);
+llvm::StringRef Contents = Steps[I - 1].Contents;
+TextDocumentContentChangeEvent Event{
+SrcBefore.range(),
+rangeLength(SrcBefore.code(), SrcBefore.range()),
+Contents.str(),
+};
+
+EXPECT_THAT_ERROR(applyChange(Code, Event), llvm::Succeeded());
+EXPECT_EQ(Code, SrcAfter.code());
+  }
+}
+
+TEST(ApplyEditsTest, Simple) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+"Universe"
+  },
+  // Delete a range
+  {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+"Monde"
+  },
+  {
+R"cpp(static int
+helloMonde()
+{})cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+}
+
+TEST(ApplyEditsTest, MultiLine) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+  },
+  // Delete a range
+  {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static char[[]]()
+{})cpp",
+R"cpp(
+cookies)cpp"
+  },
+  // Replace the whole file
+  {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+R"cpp(#include 
+)cpp"
+  },
+  // Delete the whole file
+  {
+R"cpp([[#include 
+]])cpp",
+"",
+  },
+  // Add something to an empty file
+  {
+"[[]]",
+R"cpp(int main() {
+)cpp",
+  },
+  {
+R"cpp(int main() {
+)cpp",
+   

[clang-tools-extra] bca3e24 - [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-03-02T22:58:50+01:00
New Revision: bca3e24139cc301d7deae56a020057cf657035b6

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

LOG: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

ClangdServer already gets notified of every change, so it makes sense for it to
be the source of truth.
This is a step towards having ClangdServer expose a FS that includes dirty
buffers: D94554

Related changes:
 - version is now optional for ClangdServer, to preserve our existing fuzziness
   in this area (missing version ==> autoincrement)
 - ClangdServer::format{File,Range} are now more regular ClangdServer functions
   that don't need the code passed in. While here, combine into one function.
 - incremental content update logic is moved from DraftStore to
   ClangdLSPServer, with most of the implementation in SourceCode.cpp.
   DraftStore is now fairly trivial, and will probably ultimately be
   *replaced* by the dirty FS stuff.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/DraftStore.cpp
clang-tools-extra/clangd/DraftStore.h
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/test/crash-non-added-files.test
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/SyncAPI.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index f2ec8189b92c..0efc7124477a 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -59,8 +59,8 @@ constexpr trace::Metric LSPLatency("lsp_latency", 
trace::Metric::Distribution,
 
 // LSP defines file versions as numbers that increase.
 // ClangdServer treats them as opaque and therefore uses strings instead.
-std::string encodeVersion(int64_t LSPVersion) {
-  return llvm::to_string(LSPVersion);
+std::string encodeVersion(llvm::Optional LSPVersion) {
+  return LSPVersion ? llvm::to_string(*LSPVersion) : "";
 }
 llvm::Optional decodeVersion(llvm::StringRef Encoded) {
   int64_t Result;
@@ -124,15 +124,15 @@ CompletionItemKindBitset defaultCompletionItemKinds() {
 // Makes sure edits in \p FE are applicable to latest file contents reported by
 // editor. If not generates an error message containing information about files
 // that needs to be saved.
-llvm::Error validateEdits(const DraftStore , const FileEdits ) {
+llvm::Error validateEdits(const ClangdServer , const FileEdits ) {
   size_t InvalidFileCount = 0;
   llvm::StringRef LastInvalidFile;
   for (const auto  : FE) {
-if (auto Draft = DraftMgr.getDraft(It.first())) {
+if (auto Draft = Server.getDraft(It.first())) {
   // If the file is open in user's editor, make sure the version we
   // saw and current version are compatible as this is the text that
   // will be replaced by editors.
-  if (!It.second.canApplyTo(Draft->Contents)) {
+  if (!It.second.canApplyTo(*Draft)) {
 ++InvalidFileCount;
 LastInvalidFile = It.first();
   }
@@ -648,8 +648,8 @@ void ClangdLSPServer::onDocumentDidOpen(
 
   const std::string  = Params.textDocument.text;
 
-  auto Version = DraftMgr.addDraft(File, Params.textDocument.version, 
Contents);
-  Server->addDocument(File, Contents, encodeVersion(Version),
+  Server->addDocument(File, Contents,
+  encodeVersion(Params.textDocument.version),
   WantDiagnostics::Yes);
 }
 
@@ -661,25 +661,28 @@ void ClangdLSPServer::onDocumentDidChange(
   : WantDiagnostics::No;
 
   PathRef File = Params.textDocument.uri.file();
-  llvm::Expected Draft = DraftMgr.updateDraft(
-  File, Params.textDocument.version, Params.contentChanges);
-  if (!Draft) {
-// If this fails, we are most likely going to be not in sync anymore with
-// the client.  It is better to remove the draft and let further operations
-// fail rather than giving wrong results.
-DraftMgr.removeDraft(File);
-Server->removeDocument(File);
-elog("Failed to update {0}: {1}", File, Draft.takeError());
+  auto Code = Server->getDraft(File);
+  if (!Code) {
+log("Trying to incrementally change non-added document: {0}", File);
 return;
   }
-
-  Server->addDocument(File, Draft->Contents, 

[clang-tools-extra] 00c7d66 - [cte][NFC] Remove all references to stdlib stream headers.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-03-02T21:57:16Z
New Revision: 00c7d6699a3937cd1fe34921f1fbc0c4fc09cebd

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

LOG: [cte][NFC] Remove all references to stdlib stream headers.

Inclusion of iostream is frobidden and using other stream classes from standard 
library is discouraged as per 
https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clang-query/tool/ClangQuery.cpp
clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp

clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/modularize/Modularize.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/tool/ClangQuery.cpp 
b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
index 45a355073945..2bdb36192f35 100644
--- a/clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -33,10 +33,10 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
-#include 
 #include 
 
 using namespace clang;
@@ -73,16 +73,15 @@ static cl::opt PreloadFile(
 
 bool runCommandsInFile(const char *ExeName, std::string const ,
QuerySession ) {
-  std::ifstream Input(FileName.c_str());
-  if (!Input.is_open()) {
-llvm::errs() << ExeName << ": cannot open " << FileName << "\n";
-return 1;
+  auto Buffer = llvm::MemoryBuffer::getFile(FileName);
+  if (!Buffer) {
+llvm::errs() << ExeName << ": cannot open " << FileName << ": "
+ << Buffer.getError().message() << "\n";
+return true;
   }
 
-  std::string FileContent((std::istreambuf_iterator(Input)),
-  std::istreambuf_iterator());
+  StringRef FileContentRef(Buffer.get()->getBuffer());
 
-  StringRef FileContentRef(FileContent);
   while (!FileContentRef.empty()) {
 QueryRef Q = QueryParser::parse(FileContentRef, QS);
 if (!Q->run(llvm::outs(), QS))

diff  --git a/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp 
b/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
index 9f28a22a9d03..a2178befa9df 100644
--- a/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
@@ -11,7 +11,6 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include 
-#include 
 
 using namespace clang::ast_matchers;
 
@@ -109,15 +108,13 @@ void StructPackAlignCheck::check(const 
MatchFinder::MatchResult ) {
   AlignedAttr *Attribute = Struct->getAttr();
   std::string NewAlignQuantity = std::to_string((int)NewAlign.getQuantity());
   if (Attribute) {
-std::ostringstream FixItString;
-FixItString << "aligned(" << NewAlignQuantity << ")";
-FixIt =
-FixItHint::CreateReplacement(Attribute->getRange(), FixItString.str());
+FixIt = FixItHint::CreateReplacement(
+Attribute->getRange(),
+(Twine("aligned(") + NewAlignQuantity + ")").str());
   } else {
-std::ostringstream FixItString;
-FixItString << " __attribute__((aligned(" << NewAlignQuantity << ")))";
-FixIt = FixItHint::CreateInsertion(Struct->getEndLoc().getLocWithOffset(1),
-   FixItString.str());
+FixIt = FixItHint::CreateInsertion(
+Struct->getEndLoc().getLocWithOffset(1),
+(Twine(" __attribute__((aligned(") + NewAlignQuantity + ")))").str());
   }
 
   // And suggest the minimum power-of-two alignment for the struct as a whole

diff  --git 
a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
 
b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
index 43d2f6a69cd1..c28424b11f27 100644
--- 
a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -12,7 +12,6 @@
 
 #include 
 #include 
-#include 
 
 using namespace clang::ast_matchers;
 

diff  --git a/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp 
b/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
index 237655dcc9bc..deeb07b4cb64 100644
--- a/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ 

[PATCH] D97771: [cte][NFC] Remove all references to stdlib stream headers.

[Nathan James via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
njames93 marked an inline comment as done.
Closed by commit rG00c7d6699a39: [cte][NFC] Remove all references to stdlib 
stream headers. (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D97771?vs=327448=327581#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97771

Files:
  clang-tools-extra/clang-query/tool/ClangQuery.cpp
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/modularize/Modularize.cpp

Index: clang-tools-extra/modularize/Modularize.cpp
===
--- clang-tools-extra/modularize/Modularize.cpp
+++ clang-tools-extra/modularize/Modularize.cpp
@@ -247,7 +247,6 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include 
-#include 
 #include 
 #include 
 #include 
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -39,7 +39,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp
===
--- clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp
+++ clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp
@@ -16,7 +16,6 @@
 #include "ClangdServer.h"
 #include "support/ThreadsafeFS.h"
 #include 
-#include 
 
 using namespace clang::clangd;
 
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -13,8 +13,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
-#include 
-#include 
 #include 
 
 const char *IndexFilename;
@@ -34,9 +32,15 @@
 
 // Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
 std::vector extractQueriesFromLogs() {
-  std::ifstream InputStream(RequestsFilename);
-  std::string Log((std::istreambuf_iterator(InputStream)),
-  std::istreambuf_iterator());
+
+  auto Buffer = llvm::MemoryBuffer::getFile(RequestsFilename);
+  if (!Buffer) {
+llvm::errs() << "Error cannot open JSON request file:" << RequestsFilename
+ << ": " << Buffer.getError().message() << "\n";
+exit(1);
+  }
+
+  StringRef Log = Buffer.get()->getBuffer();
 
   std::vector Requests;
   auto JSONArray = llvm::json::parse(Log);
Index: clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -12,7 +12,6 @@
 
 #include 
 #include 
-#include 
 
 using namespace clang::ast_matchers;
 
Index: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
===
--- clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
@@ -11,7 +11,6 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include 
-#include 
 
 using namespace clang::ast_matchers;
 
@@ -109,15 +108,13 @@
   AlignedAttr *Attribute = Struct->getAttr();
   std::string NewAlignQuantity = std::to_string((int)NewAlign.getQuantity());
   if (Attribute) {
-std::ostringstream FixItString;
-FixItString << "aligned(" << NewAlignQuantity << ")";
-FixIt =
-FixItHint::CreateReplacement(Attribute->getRange(), FixItString.str());
+FixIt = FixItHint::CreateReplacement(
+Attribute->getRange(),
+(Twine("aligned(") + NewAlignQuantity + ")").str());
   } else {
-std::ostringstream FixItString;
-FixItString << " __attribute__((aligned(" << NewAlignQuantity << ")))";
-FixIt = FixItHint::CreateInsertion(Struct->getEndLoc().getLocWithOffset(1),
-   FixItString.str());
+FixIt = FixItHint::CreateInsertion(
+Struct->getEndLoc().getLocWithOffset(1),
+(Twine(" __attribute__((aligned(") + NewAlignQuantity + ")))").str());
   }
 
   // And suggest the minimum power-of-two alignment for the struct as a whole
Index: 

[PATCH] D97805: [clang-query] Add option to enable only displaying main file matches

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, steveire.
njames93 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add option `set match-scope (include-headers|main-file-only)` to control only 
printing matches from the main file.
This can be set once and then avoids the need to pollute all your match 
expressions with `isExpansionInMainFile()`
By default it is enabled to only show match results from the main file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97805

Files:
  clang-tools-extra/clang-query/Query.cpp
  clang-tools-extra/clang-query/QueryParser.cpp
  clang-tools-extra/clang-query/QueryParser.h
  clang-tools-extra/clang-query/QuerySession.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-query/Inputs/foo.h
  clang-tools-extra/test/clang-query/headers.cpp

Index: clang-tools-extra/test/clang-query/headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-query/headers.cpp
@@ -0,0 +1,37 @@
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl(hasName('foo'))" %s -- | FileCheck %s --check-prefix=CHECK-ALL-FOO
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl(hasName('foo'))" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-FOO
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl(hasName('bar'))" %s -- | FileCheck %s --check-prefix=CHECK-ALL-BAR
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl(hasName('bar'))" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-BAR
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-ALL-ANY
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-ANY
+
+// Test to ensure default behavious only includes main file matches.
+// RUN: clang-query -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-ANY
+
+#include "Inputs/foo.h"
+
+void foo() {}
+
+// CHECK-ALL-FOO: /Inputs/foo.h:4:1: note: "root" binds here
+// CHECK-ALL-FOO: /headers.cpp:13:1: note: "root" binds here
+// CHECK-ALL-FOO: 2 matches.
+
+// CHECK-MAIN-FOO: /headers.cpp:13:1: note: "root" binds here
+// CHECK-MAIN-FOO: 1 match.
+// CHECK-MAIN-FOO-Next: Skipped 1 match from header files.
+// CHECK-MAIN-FOO-Next: Use 'set match-mode include-headers' to display them.
+
+// CHECK-ALL-BAR: /Inputs/foo.h:5:1: note: "root" binds here
+// CHECK-ALL-BAR: 1 match.
+
+// CHECK-MAIN-BAR: 0 matches.
+// CHECK-MAIN-BAR-Next: Skipped 1 match from header files.
+
+// CHECK-ALL-ANY: /Inputs/foo.h:4:1: note: "root" binds here
+// CHECK-ALL-ANY: /Inputs/foo.h:5:1: note: "root" binds here
+// CHECK-ALL-ANY: /headers.cpp:13:1: note: "root" binds here
+// CHECK-ALL-ANY: 3 matches.
+
+// CHECK-MAIN-ANY: /headers.cpp:13:1: note: "root" binds here
+// CHECK-MAIN-ANY: 1 match.
+// CHECK-MAIN-BAR-Next: Skipped 2 matches from header files.
Index: clang-tools-extra/test/clang-query/Inputs/foo.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-query/Inputs/foo.h
@@ -0,0 +1,7 @@
+#ifndef INPUTS_FOO_HH
+#define INPUTS_FOO_HH
+
+void foo();
+void bar();
+
+#endif // INPUTS_FOO_HH
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -57,7 +57,8 @@
 Improvements to clang-query
 ---
 
-The improvements are...
+ - Added a option `set match-scope` to control displaying matches that occur 
+   in header files.
 
 Improvements to clang-rename
 
Index: clang-tools-extra/clang-query/QuerySession.h
===
--- clang-tools-extra/clang-query/QuerySession.h
+++ clang-tools-extra/clang-query/QuerySession.h
@@ -26,7 +26,7 @@
   QuerySession(llvm::ArrayRef> ASTs)
   : ASTs(ASTs), PrintOutput(false), DiagOutput(true),
 DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-Terminate(false), TK(TK_AsIs) {}
+OnlyMainFileMatches(true), Terminate(false), TK(TK_AsIs) {}
 
   llvm::ArrayRef> ASTs;
 
@@ -36,6 +36,7 @@
 
   bool BindRoot;
   bool PrintMatcher;
+  bool OnlyMainFileMatches;
   bool Terminate;
 
   TraversalKind TK;
Index: clang-tools-extra/clang-query/QueryParser.h
===
--- clang-tools-extra/clang-query/QueryParser.h
+++ clang-tools-extra/clang-query/QueryParser.h
@@ -44,6 +44,7 @@
 
   QueryRef parseSetBool(bool QuerySession::*Var);
   QueryRef parseSetTraversalKind(TraversalKind QuerySession::*Var);
+  QueryRef parseMatchScope(bool QuerySession::*Var);
   

[PATCH] D97784: [ASTMatchers] Always create a "binding" for the node being matched on.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: klimek, aaron.ballman, steveire.
njames93 published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Most often when using matchers, there is a need to get the root node that the 
matcher matched on.
Currently the only way to do this is to manually create a binding the the root 
matcher, however this has some drawbacks.
It can be prone to typos and there is no universal agreed name to give the root 
node, so an API that accepts a DynTypedMatcher wont be able to figure out what 
the root actually is.

By altering the infrastructure to always provide access to the node being 
matched on, that isn't dependent on names, it can simplify workflow a lot.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97784

Files:
  clang/include/clang/ASTMatchers/ASTMatchFinder.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp

Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -98,11 +98,12 @@
 /*LookupInDependent =*/true);
 }
 
-void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
+void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor,
+ const DynTypedNode ) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
   for (BoundNodesMap  : Bindings) {
-ResultVisitor->visitMatch(BoundNodes(Binding));
+ResultVisitor->visitMatch(BoundNodes(Binding, RootNode));
   }
 }
 
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -815,7 +815,7 @@
   BoundNodesTreeBuilder Builder;
   if (MP.first.matches(Node, this, )) {
 MatchVisitor Visitor(ActiveASTContext, MP.second);
-Builder.visitMatches();
+Builder.visitMatches(, DynTypedNode::create(Node));
   }
 }
   }
@@ -847,7 +847,7 @@
 
   if (MP.first.matches(DynNode, this, )) {
 MatchVisitor Visitor(ActiveASTContext, MP.second);
-Builder.visitMatches();
+Builder.visitMatches(, DynNode);
   }
 }
   }
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -248,7 +248,7 @@
   /// Visits all matches that this BoundNodesTree represents.
   ///
   /// The ownership of 'ResultVisitor' remains at the caller.
-  void visitMatches(Visitor* ResultVisitor);
+  void visitMatches(Visitor *ResultVisitor, const DynTypedNode );
 
   template 
   bool removeBindings(const ExcludePredicate ) {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -125,14 +125,21 @@
 return MyBoundNodes.getMap();
   }
 
+  template  const T *getRootAs() const { return RootNode.get(); }
+
+  const DynTypedNode () const { return RootNode; }
+
 private:
   friend class internal::BoundNodesTreeBuilder;
 
   /// Create BoundNodes from a pre-filled map of bindings.
-  BoundNodes(internal::BoundNodesMap )
-  : MyBoundNodes(MyBoundNodes) {}
+  BoundNodes(internal::BoundNodesMap ,
+ const DynTypedNode )
+  : MyBoundNodes(MyBoundNodes), RootNode(RootNode) {}
 
   internal::BoundNodesMap MyBoundNodes;
+
+  DynTypedNode RootNode;
 };
 
 /// Types of matchers for the top-level classes in the AST class
Index: clang/include/clang/ASTMatchers/ASTMatchFinder.h
===
--- clang/include/clang/ASTMatchers/ASTMatchFinder.h
+++ clang/include/clang/ASTMatchers/ASTMatchFinder.h
@@ -280,6 +280,24 @@
   return nullptr;
 }
 
+/// Returns the first result of type \c NodeT bound to the root match result.
+///
+/// Returns \c NULL if there is no match, or if the matching node cannot be
+/// casted to \c NodeT.
+///
+/// This is useful in combanation with \c match():
+/// \code
+///   const Decl *D = selectFirst(match(DeclMatcher, Node, Context));
+/// \endcode
+template 
+const NodeT *selectFirst(const SmallVectorImpl ) {
+  for (const BoundNodes  : Results) {
+if (const NodeT *Node = N.getRootAs())
+  return Node;
+  }
+  return nullptr;
+}
+
 namespace internal {
 class CollectMatchesCallback : public MatchFinder::MatchCallback {
 public:
___
cfe-commits mailing list

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 327572.
arnamoy10 added a comment.

Rebased on main


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

https://reviews.llvm.org/D96344

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fdefault.f90
  flang/test/Flang-Driver/flarge_sizes.f90
  flang/test/Flang-Driver/pipeline.f90

Index: flang/test/Flang-Driver/pipeline.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/pipeline.f90
@@ -0,0 +1,17 @@
+! This file tests that flang-new forwards 
+! all Flang frontend options to flang-new -fc1
+! as expected.
+!
+! RUN: %flang-new -fsyntax-only -### %s -o %t 2>&1 \
+! RUN: -fdefault-double-8 \
+! RUN: -fdefault-integer-8 \
+! RUN: -fdefault-real-8 \
+! RUN: -flarge-sizes \
+! RUN:   | FileCheck %s
+!
+!
+! CHECK: "-fdefault-double-8"
+! CHECK: "-fdefault-integer-8"
+! CHECK: "-fdefault-real-8"
+! CHECK: "-flarge-sizes"
+
Index: flang/test/Flang-Driver/flarge_sizes.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/flarge_sizes.f90
@@ -0,0 +1,36 @@
+! Ensure argument -flarge-sizes works as expected.
+! TODO: Add checks when actual codegen is possible.
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOLARGE
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -flarge-sizes -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=LARGE
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang_fc1 -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOLARGE
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang_fc1 -fsyntax-only -flarge-sizes -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=LARGE
+
+!-
+! EXPECTED OUTPUT WITHOUT -flarge-sizes
+!-
+! NOLARGE: real(4)::z(1_8:10_8)
+! NOLARGE-NEXT: integer(4),parameter::size_kind=4_4
+
+!-
+! EXPECTED OUTPUT FOR -flarge-sizes
+!-
+! LARGE: real(4)::z(1_8:10_8)
+! LARGE-NEXT: integer(4),parameter::size_kind=8_4
+
+module m
+  implicit none
+  real :: z(10)
+  integer, parameter :: size_kind = kind(ubound(z, 1)) !-flarge-sizes
+end
Index: flang/test/Flang-Driver/fdefault.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fdefault.f90
@@ -0,0 +1,61 @@
+! Ensure argument -fdefault* work as expected.
+! TODO: Add checks when actual codegen is possible for this family
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOOPTION
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -fdefault-real-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=REAL8
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -fdefault-real-8 -fdefault-double-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=DOUBLE8
+! RUN: not %flang -fsyntax-only -fdefault-double-8 %s  2>&1 | FileCheck %s --check-prefix=ERROR
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang_fc1 -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOOPTION
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang_fc1 -fsyntax-only -fdefault-real-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=REAL8
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang_fc1 -fsyntax-only -fdefault-real-8 -fdefault-double-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s 

[PATCH] D97803: [clangd] Overload bundles are only deprecated if each overloads is.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 327574.
sammccall added a comment.

simplify


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97803

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -281,6 +281,7 @@
   : ASTCtx(ASTCtx),
 EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
 IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
+Completion.Deprecated = true; // cleared by any non-deprecated overload.
 add(C, SemaCCS);
 if (C.SemaResult) {
   assert(ASTCtx);
@@ -309,8 +310,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,7 +331,6 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
 }
 if (C.IdentifierResult) {
   Completion.Origin |= SymbolOrigin::Identifier;
@@ -408,6 +406,12 @@
  /*CommentsFromHeader=*/false));
   }
 }
+if (Completion.Deprecated) {
+  Completion.Deprecated =
+  (C.SemaResult &&
+   C.SemaResult->Availability == CXAvailability_Deprecated) ||
+  (C.IndexResult && C.IndexResult->Flags & Symbol::Deprecated);
+}
   }
 
   CodeCompletion build() {


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -281,6 +281,7 @@
   : ASTCtx(ASTCtx),
 EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
 IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
+Completion.Deprecated = true; // cleared by any non-deprecated overload.
 add(C, SemaCCS);
 if (C.SemaResult) {
   assert(ASTCtx);
@@ -309,8 +310,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,7 +331,6 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
 }
 if (C.IdentifierResult) {
   Completion.Origin |= SymbolOrigin::Identifier;
@@ -408,6 +406,12 @@
  /*CommentsFromHeader=*/false));
   }
 }
+if (Completion.Deprecated) {
+  

[PATCH] D97142: [clang-tidy] Simplify unused RAII check

[Stephen Kelly via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b6fc9a1055a: [clang-tidy] Simplify unused RAII check 
(authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D97142?vs=327004=327567#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97142

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-unused-raii %t
+// RUN: %check_clang_tidy %s bugprone-unused-raii %t -- -- -fno-delayed-template-parsing
 
 struct Foo {
   Foo();
@@ -46,6 +46,42 @@
   T();
 }
 
+struct CtorDefaultArg {
+  CtorDefaultArg(int i = 0);
+  ~CtorDefaultArg();
+};
+
+template 
+struct TCtorDefaultArg {
+  TCtorDefaultArg(T i = 0);
+  ~TCtorDefaultArg();
+};
+
+struct CtorTwoDefaultArg {
+  CtorTwoDefaultArg(int i = 0, bool b = false);
+  ~CtorTwoDefaultArg();
+};
+
+template 
+void templatetest() {
+  TCtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+  TCtorDefaultArg{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
+  TCtorDefaultArg(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name(T{});
+  TCtorDefaultArg{T{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name{T{}};
+
+  int i = 0;
+  (void)i;
+}
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
@@ -71,6 +107,22 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
   // CHECK-FIXES: FooBar give_me_a_name;
 
+  CtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: CtorDefaultArg give_me_a_name;
+
+  CtorTwoDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: CtorTwoDefaultArg give_me_a_name;
+
+  TCtorDefaultArg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
+  TCtorDefaultArg{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TCtorDefaultArg give_me_a_name;
+
   templ();
   templ();
 
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
@@ -28,6 +28,9 @@
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,25 +25,37 @@
 
 void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
   // Look for temporaries that are constructed in-place and immediately
-  // destroyed. Look for temporaries created by a functional cast but not for
-  // those returned from a call.
-  auto BindTemp = cxxBindTemporaryExpr(
-  unless(has(ignoringParenImpCasts(callExpr(,
-  unless(has(ignoringParenImpCasts(objcMessageExpr()
-  .bind("temp");
+  // destroyed.
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   exprWithCleanups(
-   unless(isInTemplateInstantiation()),
-   

[clang-tools-extra] 7b6fc9a - [clang-tidy] Simplify unused RAII check

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2021-03-02T21:33:34Z
New Revision: 7b6fc9a1055a7eae07645fb7d3085dc89a8d54ab

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

LOG: [clang-tidy] Simplify unused RAII check

Fix handling of default construction where the constructor has a default arg.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
index 3a52180cf04d..b87bb9e8ca95 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,25 +25,37 @@ AST_MATCHER(CXXRecordDecl, hasNonTrivialDestructor) {
 
 void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
   // Look for temporaries that are constructed in-place and immediately
-  // destroyed. Look for temporaries created by a functional cast but not for
-  // those returned from a call.
-  auto BindTemp = cxxBindTemporaryExpr(
-  unless(has(ignoringParenImpCasts(callExpr(,
-  unless(has(ignoringParenImpCasts(objcMessageExpr()
-  .bind("temp");
+  // destroyed.
   Finder->addMatcher(
-  traverse(TK_AsIs,
-   exprWithCleanups(
-   unless(isInTemplateInstantiation()),
-   hasParent(compoundStmt().bind("compound")),
-   hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-   anyOf(has(ignoringParenImpCasts(BindTemp)),
- has(ignoringParenImpCasts(cxxFunctionalCastExpr(
- has(ignoringParenImpCasts(BindTemp)))
-   .bind("expr")),
+  mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
+  .with(hasParent(compoundStmt().bind("compound")),
+anyOf(hasType(cxxRecordDecl(hasNonTrivialDestructor())),
+  hasType(templateSpecializationType(
+  hasDeclaration(classTemplateDecl(has(
+  cxxRecordDecl(hasNonTrivialDestructor()
+  .bind("expr"),
   this);
 }
 
+template 
+void reportDiagnostic(DiagnosticBuilder D, const T *Node, SourceRange SR,
+  bool DefaultConstruction) {
+  const char *Replacement = " give_me_a_name";
+
+  // If this is a default ctor we have to remove the parens or we'll introduce 
a
+  // most vexing parse.
+  if (DefaultConstruction) {
+D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(SR),
+  Replacement);
+return;
+  }
+
+  // Otherwise just suggest adding a name. To find the place to insert the name
+  // find the first TypeLoc in the children of E, which always points to the
+  // written type.
+  D << FixItHint::CreateInsertion(SR.getBegin(), Replacement);
+}
+
 void UnusedRaiiCheck::check(const MatchFinder::MatchResult ) {
   const auto *E = Result.Nodes.getNodeAs("expr");
 
@@ -55,35 +67,32 @@ void UnusedRaiiCheck::check(const MatchFinder::MatchResult 
) {
   // Don't emit a warning for the last statement in the surrounding compound
   // statement.
   const auto *CS = Result.Nodes.getNodeAs("compound");
-  if (E == CS->body_back())
+  const auto *LastExpr = dyn_cast(CS->body_back());
+
+  if (LastExpr && E == LastExpr->IgnoreUnlessSpelledInSource())
 return;
 
   // Emit a warning.
   auto D = diag(E->getBeginLoc(), "object destroyed immediately after "
   "creation; did you mean to name the 
object?");
-  const char *Replacement = " give_me_a_name";
 
-  // If this is a default ctor we have to remove the parens or we'll introduce 
a
-  // most vexing parse.
-  const auto *BTE = Result.Nodes.getNodeAs("temp");
-  if (const auto *TOE = dyn_cast(BTE->getSubExpr()))
-if (TOE->getNumArgs() == 0) {
-  D << FixItHint::CreateReplacement(
-  CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()),
-  Replacement);
-  return;
+  if (const auto *Node = dyn_cast(E))
+reportDiagnostic(D, Node, Node->getParenOrBraceRange(),
+ Node->getNumArgs() == 0 ||
+ isa(Node->getArg(0)));
+  if (const auto *Node = dyn_cast(E)) {
+auto SR = SourceRange(Node->getLParenLoc(), Node->getRParenLoc());
+auto DefaultConstruction = Node->getNumArgs() == 0;
+if (!DefaultConstruction) {
+  auto FirstArg = Node->getArg(0);
+  DefaultConstruction = isa(FirstArg);
+  if 

[PATCH] D97803: [clangd] Overload bundles are only deprecated if each overloads is.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 327565.
sammccall edited the summary of this revision.
sammccall added a comment.

fix bug link


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97803

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -249,6 +249,12 @@
 return RankedIncludeHeaders[0];
   }
 
+  bool isDeprecated() const {
+return (SemaResult &&
+SemaResult->Availability == CXAvailability_Deprecated) ||
+   (IndexResult && IndexResult->Flags & Symbol::Deprecated);
+  }
+
   using Bundle = llvm::SmallVector;
 };
 using ScoredBundle =
@@ -309,8 +315,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,13 +336,13 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
 }
 if (C.IdentifierResult) {
   Completion.Origin |= SymbolOrigin::Identifier;
   Completion.Kind = CompletionItemKind::Text;
   Completion.Name = std::string(C.IdentifierResult->Name);
 }
+Completion.Deprecated |= C.isDeprecated();
 
 // Turn absolute path into a literal string that can be #included.
 auto Inserted = [&](llvm::StringRef Header)
@@ -408,6 +412,7 @@
  /*CommentsFromHeader=*/false));
   }
 }
+Completion.Deprecated = Completion.Deprecated && C.isDeprecated();
   }
 
   CodeCompletion build() {


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -249,6 +249,12 @@
 return RankedIncludeHeaders[0];
   }
 
+  bool isDeprecated() const {
+return (SemaResult &&
+SemaResult->Availability == CXAvailability_Deprecated) ||
+   (IndexResult && IndexResult->Flags & Symbol::Deprecated);
+  }
+
   using Bundle = llvm::SmallVector;
 };
 using ScoredBundle =
@@ -309,8 +315,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,13 +336,13 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
 }
 if (C.IdentifierResult) {
   Completion.Origin |= 

[PATCH] D97803: [clangd] Overload bundles are only deprecated if each overloads is.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Fixes https://github.com/clangd/clangd/issues/707


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97803

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -249,6 +249,12 @@
 return RankedIncludeHeaders[0];
   }
 
+  bool isDeprecated() const {
+return (SemaResult &&
+SemaResult->Availability == CXAvailability_Deprecated) ||
+   (IndexResult && IndexResult->Flags & Symbol::Deprecated);
+  }
+
   using Bundle = llvm::SmallVector;
 };
 using ScoredBundle =
@@ -309,8 +315,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,13 +336,13 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
 }
 if (C.IdentifierResult) {
   Completion.Origin |= SymbolOrigin::Identifier;
   Completion.Kind = CompletionItemKind::Text;
   Completion.Name = std::string(C.IdentifierResult->Name);
 }
+Completion.Deprecated |= C.isDeprecated();
 
 // Turn absolute path into a literal string that can be #included.
 auto Inserted = [&](llvm::StringRef Header)
@@ -408,6 +412,7 @@
  /*CommentsFromHeader=*/false));
   }
 }
+Completion.Deprecated = Completion.Deprecated && C.isDeprecated();
   }
 
   CodeCompletion build() {


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1644,7 +1644,7 @@
   std::string Context = R"cpp(
 struct X {
   // Overload with int
-  int a(int);
+  int a(int) __attribute__((deprecated("", "")));
   // Overload with bool
   int a(bool);
   int b(float);
@@ -1682,6 +1682,7 @@
   EXPECT_EQ(A.ReturnType, "int"); // All overloads return int.
   // For now we just return one of the doc strings arbitrarily.
   ASSERT_TRUE(A.Documentation);
+  ASSERT_FALSE(A.Deprecated); // Not all overloads deprecated.
   EXPECT_THAT(
   A.Documentation->asPlainText(),
   AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool")));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -249,6 +249,12 @@
 return RankedIncludeHeaders[0];
   }
 
+  bool isDeprecated() const {
+return (SemaResult &&
+SemaResult->Availability == CXAvailability_Deprecated) ||
+   (IndexResult && IndexResult->Flags & Symbol::Deprecated);
+  }
+
   using Bundle = llvm::SmallVector;
 };
 using ScoredBundle =
@@ -309,8 +315,6 @@
 return std::tie(X.range.start.line, X.range.start.character) <
std::tie(Y.range.start.line, Y.range.start.character);
   });
-  Completion.Deprecated |=
-  (C.SemaResult->Availability == CXAvailability_Deprecated);
 }
 if (C.IndexResult) {
   Completion.Origin |= C.IndexResult->Origin;
@@ -332,13 +336,13 @@
 }
 Completion.RequiredQualifier = std::string(ShortestQualifier);
   }
-  Completion.Deprecated |= 

[PATCH] D97156: [ASTMatchers] Make Param functors variadic

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D97156#2593209 , @njames93 wrote:

> Is it not nicer to name the class `PolymorphicMatcher` and do away with 
> `WithParamN`, It may require partial specialization for the empty parameters 
> case.
> But I think it would be worth it.

I'm not sure any partial specialization is needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

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

[PATCH] D97156: [ASTMatchers] Make Param functors variadic

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 327559.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97156

Files:
  clang/docs/tools/dump_ast_matchers.py
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -239,10 +239,10 @@
  *Builder) const override; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
   internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>\
   DefineMatcher() {\
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam0<  \
+return ::clang::ast_matchers::internal::PolymorphicMatcher<\
 internal::matcher_##DefineMatcher##Matcher, ReturnTypesF>();   \
   }\
   template  \
@@ -284,18 +284,17 @@
 ParamType const Param; \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType>   \
   DefineMatcher(ParamType const ) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType, \
-ReturnTypesF>(Param);  \
+return ::clang::ast_matchers::internal::PolymorphicMatcher<\
+internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,  \
+ParamType>(Param); \
   }\
-  typedef ::clang::ast_matchers::internal::PolymorphicMatcherWithParam1<   \
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType,   \
-  ReturnTypesF>(##_Type##OverloadId)(\
-  ParamType const ); \
+  typedef ::clang::ast_matchers::internal::PolymorphicMatcher< \
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType> (##_Type##OverloadId)(ParamType const );  \
   template \
   bool internal::  \
   matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -338,17 +337,17 @@
 ParamType2 const Param2;   \
   };   \
   }\
-  inline ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<\
-  internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,  \
-  ParamType2, ReturnTypesF>\
+  inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
+  internal::matcher_##DefineMatcher##OverloadId##Matcher, ReturnTypesF,\
+  ParamType1, ParamType2>  \
   DefineMatcher(ParamType1 const , ParamType2 const ) {  \
-return ::clang::ast_matchers::internal::PolymorphicMatcherWithParam2<  \
-internal::matcher_##DefineMatcher##OverloadId##Matcher, ParamType1,\
-ParamType2, ReturnTypesF>(Param1, Param2); \
+return ::clang::ast_matchers::internal::PolymorphicMatcher<\
+

[PATCH] D97801: [clangd] Add `limit` extension on completion and workspace-symbols

[Sam McCall via Phabricator via cfe-commits]

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

This overrides the --limit-results command-line flag, and is not constrained
by it.
See https://github.com/clangd/clangd/issues/707


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97801

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h

Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1047,8 +1047,13 @@
 
 /// The parameters of a Workspace Symbol Request.
 struct WorkspaceSymbolParams {
-  /// A non-empty query string
+  /// A query string to filter symbols by.
+  /// Clients may send an empty string here to request all the symbols.
   std::string query;
+
+  /// Max results to return, overriding global default. 0 means no limit.
+  /// Clangd extension.
+  llvm::Optional limit;
 };
 bool fromJSON(const llvm::json::Value &, WorkspaceSymbolParams &,
   llvm::json::Path);
@@ -1097,6 +1102,10 @@
 
 struct CompletionParams : TextDocumentPositionParams {
   CompletionContext context;
+
+  /// Max results to return, overriding global default. 0 means no limit.
+  /// Clangd extension.
+  llvm::Optional limit;
 };
 bool fromJSON(const llvm::json::Value &, CompletionParams &, llvm::json::Path);
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -740,7 +740,8 @@
 bool fromJSON(const llvm::json::Value , WorkspaceSymbolParams ,
   llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
-  return O && O.map("query", R.query);
+  return O && O.map("query", R.query) &&
+ mapOptOrNull(Params, "limit", R.limit, P);
 }
 
 llvm::json::Value toJSON(const Command ) {
@@ -841,7 +842,8 @@
 
 bool fromJSON(const llvm::json::Value , CompletionParams ,
   llvm::json::Path P) {
-  if (!fromJSON(Params, static_cast(R), P))
+  if (!fromJSON(Params, static_cast(R), P) ||
+  !mapOptOrNull(Params, "limit", R.limit, P))
 return false;
   if (auto *Context = Params.getAsObject()->get("context"))
 return fromJSON(*Context, R.context, P.field("context"));
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -781,7 +781,7 @@
 const WorkspaceSymbolParams ,
 Callback> Reply) {
   Server->workspaceSymbols(
-  Params.query, Opts.CodeComplete.Limit,
+  Params.query, Params.limit.getValueOr(Opts.CodeComplete.Limit),
   [Reply = std::move(Reply),
this](llvm::Expected> Items) mutable {
 if (!Items)
@@ -1055,21 +1055,24 @@
 vlog("ignored auto-triggered completion, preceding char did not match");
 return Reply(CompletionList());
   }
-  Server->codeComplete(
-  Params.textDocument.uri.file(), Params.position, Opts.CodeComplete,
-  [Reply = std::move(Reply),
-   this](llvm::Expected List) mutable {
-if (!List)
-  return Reply(List.takeError());
-CompletionList LSPList;
-LSPList.isIncomplete = List->HasMore;
-for (const auto  : List->Completions) {
-  CompletionItem C = R.render(Opts.CodeComplete);
-  C.kind = adjustKindToCapability(C.kind, SupportedCompletionItemKinds);
-  LSPList.items.push_back(std::move(C));
-}
-return Reply(std::move(LSPList));
-  });
+  auto Opts = this->Opts.CodeComplete;
+  if (Params.limit && *Params.limit >= 0)
+Opts.Limit = *Params.limit;
+  Server->codeComplete(Params.textDocument.uri.file(), Params.position, Opts,
+   [Reply = std::move(Reply), Opts,
+this](llvm::Expected List) mutable {
+ if (!List)
+   return Reply(List.takeError());
+ CompletionList LSPList;
+ LSPList.isIncomplete = List->HasMore;
+ for (const auto  : List->Completions) {
+   CompletionItem C = R.render(Opts);
+   C.kind = adjustKindToCapability(
+   C.kind, SupportedCompletionItemKinds);
+   LSPList.items.push_back(std::move(C));
+ }
+ return Reply(std::move(LSPList));
+   });
 }
 
 void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams ,
___
cfe-commits 

[PATCH] D97777: Add __builtin_isnan(__fp16) testcase

[Thomas Preud'homme via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe77b5c40d576: Add __builtin_isnan(__fp16) testcase (authored 
by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D9

Files:
  clang/test/CodeGen/strictfp_builtins.c


Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -129,6 +129,25 @@
   return;
 }
 
+// CHECK-LABEL: @test_fp16_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[H_ADDR:%.*]] = alloca half, align 2
+// CHECK-NEXT:store half [[H:%.*]], half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load half, half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast half [[TMP0]] to i16
+// CHECK-NEXT:[[ABS:%.*]] = and i16 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i16 [[#%u,0x7C00]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i16 [[TMP1]], 15
+// CHECK-NEXT:[[RES:%.*]] = zext i16 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* 
@.str.[[#STRID:STRID+1]], i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_fp16_isnan(__fp16 h) {
+  P(isnan, (h));
+
+  return;
+}
+
 // CHECK-LABEL: @test_float_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4


Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -129,6 +129,25 @@
   return;
 }
 
+// CHECK-LABEL: @test_fp16_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[H_ADDR:%.*]] = alloca half, align 2
+// CHECK-NEXT:store half [[H:%.*]], half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load half, half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast half [[TMP0]] to i16
+// CHECK-NEXT:[[ABS:%.*]] = and i16 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i16 [[#%u,0x7C00]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i16 [[TMP1]], 15
+// CHECK-NEXT:[[RES:%.*]] = zext i16 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.[[#STRID:STRID+1]], i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_fp16_isnan(__fp16 h) {
+  P(isnan, (h));
+
+  return;
+}
+
 // CHECK-LABEL: @test_float_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e77b5c4 - Add __builtin_isnan(__fp16) testcase

[Thomas Preud'homme via cfe-commits]

Author: Thomas Preud'homme
Date: 2021-03-02T21:01:51Z
New Revision: e77b5c40d57633a66842e75410585696895ecf4d

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

LOG: Add __builtin_isnan(__fp16) testcase

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/test/CodeGen/strictfp_builtins.c

Removed: 




diff  --git a/clang/test/CodeGen/strictfp_builtins.c 
b/clang/test/CodeGen/strictfp_builtins.c
index 618bb70d5ed3d..3afc9d8c8a402 100644
--- a/clang/test/CodeGen/strictfp_builtins.c
+++ b/clang/test/CodeGen/strictfp_builtins.c
@@ -129,6 +129,25 @@ void test_isinf_sign(double d) {
   return;
 }
 
+// CHECK-LABEL: @test_fp16_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[H_ADDR:%.*]] = alloca half, align 2
+// CHECK-NEXT:store half [[H:%.*]], half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load half, half* [[H_ADDR]], align 2
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast half [[TMP0]] to i16
+// CHECK-NEXT:[[ABS:%.*]] = and i16 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i16 [[#%u,0x7C00]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i16 [[TMP1]], 15
+// CHECK-NEXT:[[RES:%.*]] = zext i16 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* 
@.str.[[#STRID:STRID+1]], i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_fp16_isnan(__fp16 h) {
+  P(isnan, (h));
+
+  return;
+}
+
 // CHECK-LABEL: @test_float_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4



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

[PATCH] D97320: Use a fast path when initializing LineOffsetMapping

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

Nice, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97320

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

[PATCH] D97773: [clangd] Make WorkspaceSymbols request work with empty queries

[Sam McCall via Phabricator via cfe-commits]

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

LGTM

OK, I think we've established that the request was for a way to dump out 
clangd's whole index, and workspace/symbol isn't actually a good way to do that.
Nevertheless, this seems useful and endorsed by the spec.

https://reviews.llvm.org/D44882#inline-394499 was the original motivation for 
the special case, but it predates the spec text and things have probably 
changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97773

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

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I am in favor of a new option, `IncludeIsMainAllowBraces`. It keeps things 
simple and does not cause any backwards-compatibility issues. The `${}` 
variables are clever, but if "users unconcerned with formatting ... could get 
the current functionality by including a simple rule [that includes a 
variable]", I think it might be asking too much. Being able to write (and read) 
an include regex without referring to documentation seems ideal to me. You 
mentioned automatically adding variables in rules for backwards-compatibility, 
but I'd be a bit concerned about robustness and too much magic going on behind 
the scenes for users to understand if something goes wrong or has 
unexpected/unexplained results.

Admittedly I don't fully comprehend the proposed additions and all their 
ins-and-outs, but I do understand a new, simple option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96744

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

[PATCH] D97796: [test] Fix apparent typo in clang/test/Driver/std.c

[Daniel McIntosh via Phabricator via cfe-commits]

DanielMcIntosh-IBM created this revision.
DanielMcIntosh-IBM added a reviewer: thakis.
DanielMcIntosh-IBM requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently the test on line 3 is identical to the test on line 1.
Looking at the rest of the file (particularily the use of FOVERRIDE
as the check-prefix), I think it's pretty clear that this line
was supposed to use `-ftrigraphs` instead of `-trigraphs`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97796

Files:
  clang/test/Driver/std.c


Index: clang/test/Driver/std.c
===
--- clang/test/Driver/std.c
+++ clang/test/Driver/std.c
@@ -1,6 +1,6 @@
 // RUN: %clang -w -std=c99 -trigraphs -std=gnu99 %s -E -o - | FileCheck 
-check-prefix=OVERRIDE %s
 // OVERRIDE: ??(??)
-// RUN: %clang -w -std=c99 -trigraphs -std=gnu99 %s -E -o - | FileCheck 
-check-prefix=FOVERRIDE %s
+// RUN: %clang -w -std=c99 -ftrigraphs -std=gnu99 %s -E -o - | FileCheck 
-check-prefix=FOVERRIDE %s
 // FOVERRIDE: ??(??)
 // RUN: %clang -w -ansi %s -E -o - | FileCheck -check-prefix=ANSI %s
 // ANSI: []


Index: clang/test/Driver/std.c
===
--- clang/test/Driver/std.c
+++ clang/test/Driver/std.c
@@ -1,6 +1,6 @@
 // RUN: %clang -w -std=c99 -trigraphs -std=gnu99 %s -E -o - | FileCheck -check-prefix=OVERRIDE %s
 // OVERRIDE: ??(??)
-// RUN: %clang -w -std=c99 -trigraphs -std=gnu99 %s -E -o - | FileCheck -check-prefix=FOVERRIDE %s
+// RUN: %clang -w -std=c99 -ftrigraphs -std=gnu99 %s -E -o - | FileCheck -check-prefix=FOVERRIDE %s
 // FOVERRIDE: ??(??)
 // RUN: %clang -w -ansi %s -E -o - | FileCheck -check-prefix=ANSI %s
 // ANSI: []
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97187: [Clang][Sema] Warn when function argument is less aligned than parameter

[Diogo N. Sampaio via Phabricator via cfe-commits]

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

LGTM . Just a few coding style nits to fix. But please wait a couple of days to 
see if someone else has anything else to say.




Comment at: clang/lib/Sema/SemaChecking.cpp:4475
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+return;
+  CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);

empty line after return.



Comment at: clang/lib/Sema/SemaChecking.cpp:4545
+auto *FT = FDecl->getFunctionType();
+if (FT && isa(FT))
+  Proto = cast(FDecl->getFunctionType());

Follow clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97187

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

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Albion Fung via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8593
+  return !convertToNonDenormSingle(ArgAPFloat);
+}
+

stefanp wrote:
> I'm wondering if it would not be better to just inline this. It's just "not" 
> of another call. That would simplify the patch a little.
so in the process of removing this, I thought I might as well just write a 
non-destructive test in its place. The tests didn't have any problems with just 
`checkConvertToNonDenormSingle` but might as well be on the safer side of 
things.


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

https://reviews.llvm.org/D95458

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

[PATCH] D93031: Enable fexec-charset option

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Lex/Preprocessor.h:145
   ModuleLoader  
+  LiteralConverter  LT;
 

Please give this a longer name. Abbreviation names should only be used in 
fairly small scopes where it's easy to look up what they refer to.

Also: why `LT`? What does the `T` stand for?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6178
+  CmdArgs.push_back(Args.MakeArgString(Triple.getSystemCharset()));
+  for (auto it = vList.begin(), ie = vList.end(); it != ie; ++it) {
+llvm::ErrorOr ErrorOrConverter =

Looping over all the arguments is a little unusual. Normally we'd get the last 
argument value and only check that one. Do you need to pass more than one value 
onto the frontend?



Comment at: clang/lib/Lex/LiteralSupport.cpp:237
+SmallString<8> ResultCharConv;
+Converter->convert(std::string(1, ByteChar), ResultCharConv);
+assert(ResultCharConv.size() == 1 &&

Can you avoid using `std::string` here? Eg, pass a `StringRef`, extending the 
converter to be able to take one if necessary.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1363-1364
+  if (!HadError && Converter) {
+assert(Kind != tok::wide_char_constant &&
+   "Wide character translation not supported");
+SmallString<1> ConvertedChar;

Why is this case not possible?



Comment at: clang/lib/Lex/LiteralSupport.cpp:1367
+Converter->convert(StringRef((char *)tmp_out_start), 
ConvertedChar);
+memmove((void *)tmp_out_start, ConvertedChar.data(), 1);
+  }

What assurance do we have that 1 output character is correct? I would expect we 
need to reject with a diagnostic if the character doesn't fit in one converted 
character.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1700-1702
 // Point into the \n inside the \r\n sequence and operate on the
 // remaining portion of the literal.
 RemainingTokenSpan = AfterCRLF.substr(1);

Do we need to convert the newline character too?

Perhaps for raw string literals it'd be better to do the normal processing here 
and then convert the entire string at once?



Comment at: clang/lib/Lex/LiteralSupport.cpp:234
+SmallString<8> ResultCharConv;
+Converter->convert(std::string(1, ByteChar), ResultCharConv);
+memcpy((void *), ResultCharConv.data(), sizeof(unsigned));

abhina.sreeskantharajan wrote:
> tahonermann wrote:
> > Conversion can fail here, particularly in the scenario corresponding to the 
> > default switch case above; `ResultChar` could contain, for example, a lead 
> > byte of a UTF-8 sequence.  Something sensible should be done here; either 
> > rejecting the code with an error or substituting `?` (in the execution 
> > encoding) seems appropriate to me.
> Thanks, I added the substitution with the '?' character for invalid escapes.
This is a regression. Our prior behavior for unknown escapes was to leave the 
character alone. We should still do that wherever possible -- eg, `\q` should 
produce `q` -- and take fallback action only if the character is unencodable. 
Producing a `?` seems unlikely to ever be what anyone wants; producing a hard 
error would seem preferable.



Comment at: clang/lib/Lex/LiteralSupport.cpp:238
+  void *Pointer = 
+  memcpy(Pointer, ResultCharConv.data(), sizeof(unsigned));
+}

abhina.sreeskantharajan wrote:
> abhina.sreeskantharajan wrote:
> > rsmith wrote:
> > > What should happen if the result doesn't fit into an `unsigned`? This 
> > > also appears to be making problematic assumptions about the endianness of 
> > > the host. If we really want to pack multiple bytes of encoded output into 
> > > a single `unsigned` result value (which itself seems dubious), we should 
> > > do so with an endianness that doesn't depend on the host.
> > This may be a problem we need to revisit since ResultChar is expecting a 
> > char.
> I added an assertion for this case where the size of the character increases 
> after translation. I've also removed the memcpy to avoid endianness issues.
Is there any guarantee the assertion will not fail?



Comment at: clang/test/Driver/cl-options.c:214
+// RUN: %clang_cl /execution-charset:invalid-charset -### -- %s 2>&1 | 
FileCheck -check-prefix=execution-charset-invalid %s
+// execution-charset-invalid: invalid value 'invalid-charset' in 
'-fexec-charset'
 //

Please use the given spelling of the flag in the diagnostic. (You can ask the 
argument how it was spelled.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

___
cfe-commits mailing list

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 327537.
Conanap added a comment.

Addressed Stefan's comments, converted the check to a mirror of the original 
function for XXSPLTIDP except non-destructive.


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

https://reviews.llvm.org/D95458

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/constant-pool.ll
  llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
  llvm/test/CodeGen/PowerPC/pcrel-call-linkage-leaf.ll
  llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
  llvm/test/CodeGen/PowerPC/pcrel.ll

Index: llvm/test/CodeGen/PowerPC/pcrel.ll
===
--- llvm/test/CodeGen/PowerPC/pcrel.ll
+++ llvm/test/CodeGen/PowerPC/pcrel.ll
@@ -8,13 +8,14 @@
 
 ; Constant Pool Index.
 ; CHECK-S-LABEL: ConstPool
-; CHECK-S:   plfd f1, .LCPI0_0@PCREL(0), 1
+; CHECK-S:   xxsplti32dx vs1, 0, 1081002676
+; CHECK-S-NEXT:   xxsplti32dx vs1, 1, 962072674
 ; CHECK-S:   blr
 
 ; CHECK-O-LABEL: ConstPool
-; CHECK-O:   plfd 1, 0(0), 1
-; CHECK-O-NEXT:  R_PPC64_PCREL34  .rodata.cst8
-; CHECK-O:   blr
+; CHECK-O:   xxsplti32dx 1, 0, 1081002676
+; CHECK-O-NEXT:  xxsplti32dx 1, 1, 962072674
+; CHECK-O-NEXT:  blr
 define dso_local double @ConstPool() local_unnamed_addr {
   entry:
 ret double 0x406ECAB439581062
Index: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
===
--- llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
+++ llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
@@ -35,6 +35,12 @@
 @FuncPtrOut = external local_unnamed_addr global void (...)*, align 8
 
 define dso_local void @ReadWrite8() local_unnamed_addr #0 {
+; In this test the stb r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lbz r3, 0(r3)
+; which is defined between the pld and the stb.
+; In this test the stb r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lbz r3, 0(r3)
+; which is defined between the pld and the stb.
 ; CHECK-LABEL: ReadWrite8:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, input8@got@pcrel(0), 1
@@ -44,9 +50,6 @@
 ; CHECK-NEXT:lbz r3, 0(r3)
 ; CHECK-NEXT:stb r3, 0(r4)
 ; CHECK-NEXT:blr
-; In this test the stb r3, 0(r4) cannot be optimized because it
-; uses the register r3 and that register is defined by lbz r3, 0(r3)
-; which is defined between the pld and the stb.
 entry:
   %0 = load i8, i8* @input8, align 1
   store i8 %0, i8* @output8, align 1
@@ -54,6 +57,12 @@
 }
 
 define dso_local void @ReadWrite16() local_unnamed_addr #0 {
+; In this test the sth r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lhz r3, 0(r3)
+; which is defined between the pld and the sth.
+; In this test the sth r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lhz r3, 0(r3)
+; which is defined between the pld and the sth.
 ; CHECK-LABEL: ReadWrite16:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, input16@got@pcrel(0), 1
@@ -63,9 +72,6 @@
 ; CHECK-NEXT:lhz r3, 0(r3)
 ; CHECK-NEXT:sth r3, 0(r4)
 ; CHECK-NEXT:blr
-; In this test the sth r3, 0(r4) cannot be optimized because it
-; uses the register r3 and that register is defined by lhz r3, 0(r3)
-; which is defined between the pld and the sth.
 entry:
   %0 = load i16, i16* @input16, align 2
   store i16 %0, i16* @output16, align 2
@@ -144,7 +150,8 @@
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, inputf64@got@pcrel(0), 1
 ; CHECK-NEXT:  .Lpcrel5:
-; CHECK-NEXT:plfd f1, .LCPI6_0@PCREL(0), 1
+; CHECK-NEXT:xxsplti32dx vs1, 0, 1075524403
+; CHECK-NEXT:xxsplti32dx vs1, 1, 858993459
 ; CHECK-NEXT:.reloc .Lpcrel5-8,R_PPC64_PCREL_OPT,.-(.Lpcrel5-8)
 ; CHECK-NEXT:lfd f0, 0(r3)
 ; CHECK-NEXT:pld r3, outputf64@got@pcrel(0), 1
Index: llvm/test/CodeGen/PowerPC/pcrel-call-linkage-leaf.ll
===
--- llvm/test/CodeGen/PowerPC/pcrel-call-linkage-leaf.ll
+++ llvm/test/CodeGen/PowerPC/pcrel-call-linkage-leaf.ll
@@ -173,7 +173,9 @@
 ; CHECK-LARGE: add r2, r2, r12
 ; CHECK-S-NOT:   .localentry
 ; CHECK-ALL:   # %bb.0: # %entry
-; CHECK-S-NEXT:plfd f1, .LCPI7_0@PCREL(0), 1
+; CHECK-S-NEXT:xxsplti32dx vs1, 0, 1078011044
+; CHECK-S-NEXT:xxsplti32dx vs1, 1, -337824948
+; CHECK-S-NEXT:# kill: def $f1 killed $f1 killed $vsl1
 ; CHECK-S-NEXT:blr
 entry:
   ret double 0x404124A4EBDD334C
Index: llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
===
--- llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
+++ 

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:489
 
+  if (TC.getSanitizerArgs().needsScudoRt()) {
+for (const auto  : {"scudo", "scudo_cxx"}) {

We need this for both legacy and standalone scudo?





Comment at: compiler-rt/lib/scudo/scudo_platform.h:19
 
-#if !SANITIZER_LINUX && !SANITIZER_FUCHSIA
+#if !SANITIZER_LINUX && !SANITIZER_FUCHSIA && !SANITIZER_WINDOWS
 # error "The Scudo hardened allocator is not supported on this platform."

For the reason @kcc mentioned, if possible could you not update legacy scudo at 
at all, and keep only stuff needed for standalone version?

BTW. Patches to remove legacy one are welcomed.


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

https://reviews.llvm.org/D96120

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem closed this revision.
gulfem added a comment.

Since this patch has been merged and the issue in the test has been resolved, 
I'm closing this review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, but please make sure that Richard is happy


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

https://reviews.llvm.org/D95396

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

@rsmith already gave his blessing, so please go ahead.

I hope you guys have a plan to enable this by default at some point as features 
behind a flag get rotten and no one uses them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

For sure. I'll upload a rebased patch shortly and give it another day or so for 
people to look, and then push if there aren't any issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D97777: Add __builtin_isnan(__fp16) testcase

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall 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/D9/new/

https://reviews.llvm.org/D9

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Any more comments/opinions?
Gui, would you like to push it to main if no one objects?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/DraftStore.cpp:66
 // We treat versions as opaque, but the protocol says they increase.
-if (*Version <= D.Version)
-  log("File version went from {0} to {1}", D.Version, Version);
-D.Version = *Version;
+if (SpecifiedVersion.compare_numeric(D.Version) <= 0)
+  log("File version went from {0} to {1}", D.Version, SpecifiedVersion);

sammccall wrote:
> kadircet wrote:
> > why not a not_equals instead? we are going to override the version anyway
> I don't really understand the suggestion.
> 
> The purpose here is to notice and log when client-specified versions go 
> backwards.
> This is a protocol violation, which won't cause problems for clangd but may 
> indicate something broken going on.
> 
> We don't want to log when versions go forwards!
ah nvm, i was reading this wrong. i thought it was logging when versions go 
"forward"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97738

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

[PATCH] D95017: [clang-format] Add case aware include sorting.

[Chris Johnson via Phabricator via cfe-commits]

PragmaNull added a comment.

In D95017#2595230 , 
@HazardyKnusperkeks wrote:

> 



> Do you make a change? Otherwise I will do, but that will take some time, 
> because I'm rather busy.

Sorry, I have not; unfortunately I've been rather busy myself and haven't had a 
moment to get on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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