[clang] 215ed9b - Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via cfe-commits]

Author: Stephan Bergmann
Date: 2021-01-12T09:41:03+01:00
New Revision: 215ed9b33ccbe9858aeb65b357bdcff354be

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

LOG: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

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

Added: 


Modified: 
clang/lib/AST/Expr.cpp
clang/unittests/Tooling/CastExprTest.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index a274bf37a407..94b6adf489fe 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1765,7 +1765,7 @@ Expr *CastExpr::getSubExprAsWritten() {
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+
skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&

diff  --git a/clang/unittests/Tooling/CastExprTest.cpp 
b/clang/unittests/Tooling/CastExprTest.cpp
index a9e78d2155b4..cda963a6a897 100644
--- a/clang/unittests/Tooling/CastExprTest.cpp
+++ b/clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@ TEST(CastExprTest, 
GetSubExprAsWrittenThroughMaterializedTemporary) {
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }



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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG215ed9b33ccb: Adapt CastExpr::getSubExprAsWritten to 
ConstantExpr (authored by sberg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87030

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1765,7 +1765,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+
skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1765,7 +1765,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93873: [clangd] Cache preambles of closed files

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D93873#2490314 , @sammccall wrote:

> 1. a *persistent* cache so closing+reopening clangd loses less state. (This 
> is complicated because only the PCH is easily serializable, the rest of the 
> PreambleData struct isn't)
> 2. building caches of preambles while background-indexing (this would be 
> great for modules but is probably way too big for whole preambles)
> 3. reusing the "wrong" preamble initially when you open a new file, to give 
> some basic functionality (using existing preamble patching logic, just in a 
> more aggressive scenario)
> 4. having the disk-based storage unlink the file preemptively, to eliminate 
> any chance of leaking the *.pch

It feels like a mixture of 1 and 3 is going to provide the most value for 
decreasing time until semantic features (but I might be a little biased :D, 
also we might hit a nice sweet spot with pseudoparsing too).
I don't think having a cache for previously built preambles will ever be 
enough. As Sam pointed out, scaling is one of the biggest problems, as I don't 
think it would be feasible to have tens of preambles lying around on the disk, 
especially when they are costly the built (as it implies increased size).
Surely it optimizes the case of users working on a small set of files but 
frequently closes and re-opens them. But that's just one use case, it is also 
quite common to open tens of library headers while investigating an issue, or 
trying to understand details of some code through chains of go-to-definitions.
Users won't have any preambles for a while on those files and even after 
building the preamble they'll just be sitting in the cache probably only to be 
evicted.

So I think having a cache of preambles while optimizing for reusability (by 
keeping a small set of preambles that cover different set of files, as we can't 
use a preamble for a source file if it covers the source file in question) and 
then patching those to be applicable for current file at hand sounds like a 
better compromise. Surely it won't be as effective for frequent close/re-open 
use case, but I think the costs of such a cache isn't justified if it is only 
applicable to a single workflow.

As for mixing idea-1 into the equation, all of these will require clangd to do 
the work from scratch per instance, if we can have some sort of persistent 
on-disk cache, we can both share the work (and associated storage costs) across 
clangd instances and ensure clangd is also responsive even at startup without 
requiring user to build a bunch of preambles with every new clangd instance 
first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93873

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

[clang] 7ab8030 - [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-01-12T10:00:23+01:00
New Revision: 7ab803095ae58445996dc4694acb216e3a32ee64

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

LOG: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

This patch removes the -f[no-]trapping-math flags from the -cc1 command line. 
These flags are ignored in the command line parser and their semantics is fully 
handled by -ffp-exception-mode.

This patch does not remove -f[no-]trapping-math from the driver command line. 
The driver flags are being used and do affect compilation.

Reviewed By: dexonsmith, SjoerdMeijer

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/fpconstrained.c
clang/test/CodeGen/fpconstrained.cpp
clang/test/CodeGen/noexceptionsfpmath.c
clang/test/CodeGenCUDA/propagate-metadata.cu
clang/test/Driver/fast-math.c
clang/test/Driver/fp-model.c
clang/test/Parser/fp-floatcontrol-syntax.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b18c89931cee..35643701f97e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1600,8 +1600,8 @@ def frounding_math : Flag<["-"], "frounding-math">, 
Group, Flags<[CC1Op
   MarshallingInfoFlag<"LangOpts->FPRoundingMode", 
"llvm::RoundingMode::NearestTiesToEven">,
   Normalizer<"makeFlagToValueNormalizer(llvm::RoundingMode::Dynamic)">;
 def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group, 
Flags<[CC1Option]>;
-def ftrapping_math : Flag<["-"], "ftrapping-math">, Group, 
Flags<[CC1Option]>;
-def fno_trapping_math : Flag<["-"], "fno-trapping-math">, Group, 
Flags<[CC1Option]>;
+def ftrapping_math : Flag<["-"], "ftrapping-math">, Group;
+def fno_trapping_math : Flag<["-"], "fno-trapping-math">, Group;
 def ffp_contract : Joined<["-"], "ffp-contract=">, Group,
   Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs):"
   " fast (fuses across statements disregarding pragmas)"

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index be4fe7f8eddd..4a20936ddda1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2833,9 +2833,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   if (TrappingMath) {
 // FP Exception Behavior is also set to strict
 assert(FPExceptionBehavior.equals("strict"));
-CmdArgs.push_back("-ftrapping-math");
-  } else if (TrappingMathPresent)
-CmdArgs.push_back("-fno-trapping-math");
+  }
 
   // The default is IEEE.
   if (DenormalFPMath != llvm::DenormalMode::getIEEE()) {

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 07906f4a36ef..cc3b038a7746 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2684,14 +2684,6 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
 
-  if (Args.hasArg(OPT_ftrapping_math)) {
-Opts.setFPExceptionMode(LangOptions::FPE_Strict);
-  }
-
-  if (Args.hasArg(OPT_fno_trapping_math)) {
-Opts.setFPExceptionMode(LangOptions::FPE_Ignore);
-  }
-
   LangOptions::FPExceptionModeKind FPEB = LangOptions::FPE_Ignore;
   if (Arg *A = Args.getLastArg(OPT_ffp_exception_behavior_EQ)) {
 StringRef Val = A->getValue();

diff  --git a/clang/test/CodeGen/fpconstrained.c 
b/clang/test/CodeGen/fpconstrained.c
index 0307ebbd357f..643c0120eac5 100644
--- a/clang/test/CodeGen/fpconstrained.c
+++ b/clang/test/CodeGen/fpconstrained.c
@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -ftrapping-math -frounding-math 
-ffp-exception-behavior=strict -fexperimental-strict-floating-point -emit-llvm 
-o - %s | FileCheck %s -check-prefix=FPMODELSTRICT
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=strict 
-fexperimental-strict-floating-point -emit-llvm -o - %s | FileCheck %s 
-check-prefix=FPMODELSTRICT
 // RUN: %clang_cc1 -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s 
-check-prefix=PRECISE
 // RUN: %clang_cc1 -ffast-math -ffp-contract=fast -emit-llvm -o - %s | 
FileCheck %s -check-prefix=FAST
 // RUN: %clang_cc1 -ffast-math -emit-llvm -o - %s | FileCheck %s 
-check-prefix=FASTNOCONTRACT
 // RUN: %clang_cc1 -ffast-math -ffp-contract=fast 
-ffp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s 
-check-prefix=FAST
 // RUN: %clang_cc1 -ffast-math -ffp-contract=fast 
-ffp-exception-behavior=strict -fexperimental-strict-floating-point -emit-llvm 

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

[Jan Svoboda 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 rG7ab803095ae5: [clang][cli] Remove -f[no-]trapping-math from 
-cc1 command line (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93395

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/CodeGen/noexceptionsfpmath.c
  clang/test/CodeGenCUDA/propagate-metadata.cu
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp

Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -19,9 +19,9 @@
 }
 #endif
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fdenormal-fp-math=preserve-sign,preserve-sign -ftrapping-math -fsyntax-only %s -DDEFAULT -verify
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fdenormal-fp-math=preserve-sign,preserve-sign -fsyntax-only %s -DDEFAULT -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ffp-contract=fast -DPRECISE -verify
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ftrapping-math -ffp-contract=off -frounding-math -ffp-exception-behavior=strict -DSTRICT -verify
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ffp-contract=off -frounding-math -ffp-exception-behavior=strict -DSTRICT -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -menable-no-infs -menable-no-nans -menable-unsafe-fp-math -fno-signed-zeros -mreassociate -freciprocal-math -ffp-contract=fast -ffast-math -ffinite-math-only -fsyntax-only %s -DFAST -verify
 double a = 0.0;
 double b = 1.0;
Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -80,7 +80,6 @@
 // RUN: %clang -### -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-TRAP %s
 // CHECK-TRAP: "-cc1"
-// CHECK-TRAP: "-ftrapping-math"
 // CHECK-TRAP: "-ffp-exception-behavior=strict"
 
 // RUN: %clang -### -nostdinc -ffp-model=fast -c %s 2>&1 \
@@ -106,16 +105,9 @@
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-STRICT %s
 // CHECK-FPM-STRICT: "-cc1"
-// CHECK-FPM-STRICT: "-ftrapping-math"
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
-// RUN: %clang -### -nostdinc -ftrapping-math -ffp-exception-behavior=ignore -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-TRAP-IGNORE %s
-// CHECK-TRAP-IGNORE: "-cc1"
-// CHECK-TRAP-IGNORE: "-fno-rounding-math"
-// CHECK-TRAP-IGNORE: "-ffp-exception-behavior=ignore"
-
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-STRICT %s
Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -292,10 +292,6 @@
 // CHECK-NO-REASSOC-NO-UNSAFE-MATH: "-o"
 
 
-// RUN: %clang -### -ftrapping-math -fno-trapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-TRAPPING-MATH %s
-// CHECK-NO-TRAPPING-MATH: "-fno-trapping-math"
-
 // This isn't fast-math, but the option is handled in the same place as other FP params.
 // Last option wins, and strict behavior is assumed by default. 
 
Index: clang/test/CodeGenCUDA/propagate-metadata.cu
===
--- clang/test/CodeGenCUDA/propagate-metadata.cu
+++ clang/test/CodeGenCUDA/propagate-metadata.cu
@@ -4,28 +4,24 @@
 //
 // In particular, we check that ftz and unsafe-math are propagated into the
 // bitcode library as appropriate.
-//
-// In addition, we set -ftrapping-math on the bitcode library, but then set
-// -fno-trapping-math on the main compilations, and ensure that the latter flag
-// overrides the flag on the bitcode library.
 
 // Build the bitcode library.  This is not built in CUDA mode, otherwise it
 // might have incompatible attributes.  This mirrors how libdevice is built.
-// RUN: %clang_cc1 -x c++ -fconvergent-functions -emit-llvm-bc -ftrapping-math -DLIB \
+// RUN: %clang_cc1 -x c++ -fconvergent-functions -emit-llvm-bc -DLIB \
 // RUN:   %s -o %t.bc -triple nvptx-unknown-unknown
 
 // RUN: %clang_cc1 -x cuda %s -emit-llvm -mlink-builtin-bitcode %t.bc -o - \
-// RUN:   -fno-trapping-math -fcuda-is-device -triple nvptx-unknown-unknown \
+// RUN:   -fcuda-is-device -triple nvptx-unknown-unknown \
 // RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=NOFTZ --check-prefix=NOFAST
 
 

[PATCH] D94472: [clang][cli] Manually generate header search arguments

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements generation of remaining header search arguments.
It's done manually in C++ as opposed to TableGen, because we need the 
flexibility and don't anticipate reuse.

This patch doesn't include tests. Instead, a follow-up patch Dxxx runs "parse, 
generate, parse" round-trip on every compiler invocation. That way, this code 
gets exercised by the suite of Clang's end-to-end tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94472

Files:
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1812,6 +1812,122 @@
   return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
+static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
+ SmallVectorImpl &Args,
+ CompilerInvocation::StringAllocator SA) {
+  const OptTable &OptTable = getDriverOptTable();
+
+  std::string Dash("-");
+  std::string OptName;
+
+  if (Opts.UseLibcxx) {
+OptName = Dash + OptTable.getOptionName(OPT_stdlib_EQ);
+Args.emplace_back(SA(OptName + "libc++"));
+  }
+
+  if (!Opts.ModuleCachePath.empty()) {
+OptName = Dash + OptTable.getOptionName(OPT_fmodules_cache_path);
+Args.emplace_back(SA(OptName + Opts.ModuleCachePath));
+  }
+
+  OptName = Dash + OptTable.getOptionName(OPT_fmodule_file);
+  for (const auto &File : Opts.PrebuiltModuleFiles)
+Args.emplace_back(SA(OptName + File.first + "=" + File.second));
+
+  OptName = Dash + OptTable.getOptionName(OPT_fprebuilt_module_path);
+  for (const auto &Path : Opts.PrebuiltModulePaths)
+Args.emplace_back(SA(OptName + Path));
+
+  OptName = Dash + OptTable.getOptionName(OPT_fmodules_ignore_macro);
+  for (const auto &Macro : Opts.ModulesIgnoreMacros)
+Args.emplace_back(SA(OptName + Macro.val()));
+
+  auto matches = [](const HeaderSearchOptions::Entry &Entry,
+frontend::IncludeDirGroup Group, bool IsFramework,
+bool IgnoreSysRoot) {
+return Entry.Group == Group && Entry.IsFramework == IsFramework &&
+   Entry.IgnoreSysRoot == IgnoreSysRoot;
+  };
+
+  for (const HeaderSearchOptions::Entry &E : Opts.UserEntries) {
+Optional Opt;
+
+// Add -I..., -F..., and -index-header-map options in order.
+
+if (E.Group == frontend::IndexHeaderMap) {
+  OptName = OptTable.getOptionName(OPT_index_header_map);
+  Args.emplace_back(SA(OptName));
+}
+
+if (matches(E, frontend::IndexHeaderMap, true, true))
+  Opt = OPT_F;
+else if (matches(E, frontend::IndexHeaderMap, false, true))
+  Opt = OPT_I;
+else if (matches(E, frontend::Angled, true, true))
+  Opt = OPT_F;
+else if (matches(E, frontend::Angled, false, true))
+  // Also handles [-iprefix=xx] -iwithprefixbefore=yy as -I[xx]yy.
+  Opt = OPT_I;
+
+if (Opt) {
+  OptName = Dash + OptTable.getOptionName(*Opt);
+  Args.emplace_back(SA(OptName + E.Path));
+  continue;
+}
+
+if (matches(E, frontend::After, false, true))
+  // Also handles [-iprefix=xx] -iwithprefix=yy as -idirafter=[xx]yy.
+  Opt = OPT_idirafter;
+else if (matches(E, frontend::Quoted, false, true))
+  Opt = OPT_iquote;
+else if (matches(E, frontend::System, false, true))
+  Opt = OPT_isystem;
+else if (matches(E, frontend::System, false, false))
+  Opt = OPT_iwithsysroot;
+else if (matches(E, frontend::System, true, true))
+  Opt = OPT_iframework;
+else if (matches(E, frontend::System, true, false))
+  Opt = OPT_iframeworkwithsysroot;
+
+else if (matches(E, frontend::CSystem, false, true))
+  // Add the paths for the various language specific isystem flags.
+  Opt = OPT_c_isystem;
+else if (matches(E, frontend::CXXSystem, false, true))
+  Opt = OPT_cxx_isystem;
+else if (matches(E, frontend::ObjCSystem, false, true))
+  Opt = OPT_objc_isystem;
+else if (matches(E, frontend::ObjCXXSystem, false, true))
+  Opt = OPT_objcxx_isystem;
+
+else if (matches(E, frontend::System, false, true))
+  // Add the internal paths from a driver that detects standard include
+  // paths.
+  Opt = OPT_internal_isystem;
+else if (matches(E, frontend::ExternCSystem, false, true))
+  Opt = OPT_internal_externc_isystem;
+else
+  llvm_unreachable("Tried to marshall invalid HeaderSearchOptions::Entry");
+
+OptName = Dash + OptTable.getOptionName(*Opt);
+Args.emplace_back(SA(OptName));
+Args.emplace_back(SA(E.Path));
+  }
+
+  // Add the path prefixes which are implicitly treated as being system heade

[PATCH] D94473: [clangd] Use AST-based signals in CodeCompletion.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94473

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h
  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
@@ -15,6 +15,7 @@
 #include "Quality.h"
 #include "SourceCode.h"
 #include "SyncAPI.h"
+#include "TUScheduler.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
@@ -51,6 +52,8 @@
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.Name == Name; }
+MATCHER_P(MainFileRefs, Refs, "") { return arg.MainFileRefs == Refs; }
+MATCHER_P(ScopeRefs, Refs, "") { return arg.ScopeRefsInFile == Refs; }
 MATCHER_P(NameStartsWith, Prefix, "") {
   return llvm::StringRef(arg.Name).startswith(Prefix);
 }
@@ -1115,6 +1118,63 @@
   UnorderedElementsAre(Named("xy1"), Named("xy2")));
 }
 
+TEST(CompletionTest, ASTSignals) {
+  struct Completion {
+std::string Name;
+unsigned MainFileRefs;
+unsigned ScopeRefsInFile;
+  };
+  CodeCompleteOptions Opts;
+  std::vector RecordedCompletions;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &R,
+   float Score) {
+RecordedCompletions.push_back({CC.Name, R.MainFileRefs, R.ScopeRefsInFile});
+  };
+  ASTSignals MainFileSignals;
+  MainFileSignals.Symbols[var("xy1").ID] = 3;
+  MainFileSignals.Symbols[var("xy2").ID] = 1;
+  MainFileSignals.Symbols[var("xyindex").ID] = 10;
+  MainFileSignals.RelatedNamespaces["tar::"] = 5;
+  MainFileSignals.RelatedNamespaces["clang::"] = 3;
+  Opts.MainFileSignals = &MainFileSignals;
+  Opts.AllScopes = true;
+  completions(R"cpp(
+  int xy1;
+  int xy2;
+  namespace clang {
+  const int xybar = 1;
+  int a = xy^
+  }
+  )cpp",
+  /*IndexSymbols=*/{var("xyindex"), var("tar::xyfoo")}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(
+  AllOf(Named("xy1"), MainFileRefs(3u), ScopeRefs(0u)),
+  AllOf(Named("xy2"), MainFileRefs(1u), ScopeRefs(0u)),
+  AllOf(Named("xyindex"), MainFileRefs(10u), ScopeRefs(0u)),
+  AllOf(Named("xyfoo"), MainFileRefs(0u), ScopeRefs(5u)),
+  AllOf(Named("xybar"), MainFileRefs(0u), ScopeRefs(3u;
+  //   Annotations Test(R"cpp(
+  // #include "foo.h"
+  // Fun^
+  //   )cpp");
+  //   auto TU = TestTU::withCode(Test.code());
+  //   TU.AdditionalFiles["foo.h"] = "";
+
+  //   std::string DeclFile = URI::create(testPath("foo")).toString();
+  //   Symbol Sym = func("Func");
+  //   Sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
+  //   Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
+  //   Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
+
+  //   EXPECT_THAT(completions(TU, Test.point(), {Sym}).Completions,
+  //   UnorderedElementsAre(AllOf(Named("Func"),
+  //   HasInclude("\"foo.h\""),
+  //  Not(InsertInclude();
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/Quality.h
===
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -29,6 +29,7 @@
 
 #include "ExpectedTypes.h"
 #include "FileDistance.h"
+#include "TUScheduler.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -140,6 +141,10 @@
   /// CompletionPrefix.
   unsigned FilterLength = 0;
 
+  const ASTSignals *MainFileSignals = nullptr;
+  unsigned MainFileRefs = 0;
+  unsigned ScopeRefsInFile = 0;
+
   /// Set of derived signals computed by calculateDerivedSignals(). Must not be
   /// set explicitly.
   struct DerivedSignals {
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -294,6 +294,13 @@
   if (!(IndexResult.Flags & Symbol::VisibleOutside

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
Herald added a subscriber: hiraditya.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This patch tests the manual header search argument generation by performing a 
"parse, generate, parse" round-trip on each compiler invocation. This way, the 
manually written C++ code gets exercised by the suite of Clang's end-to-end 
tests. Moreover, this ensures people adding new command line options are force 
the implement the generation as well, either via TableGen marshalling 
infrastructure or manually.

It makes sense to enable this only for assert builds, so that we don't perform 
useless work in release builds.

Depending on our roll-out strategy, we might create more patches like this for 
other groups of options and upstream the work one-by-one, or put all option 
groups into this patch and enable round-tripping for all of -cc1 in a single 
commit.

Please note this is a work-in-progress. Ideally, we'll be able to get rid of 
the duplicate methods (parseSimpleArgs, generateCC1CommandLine) and the 
HeaderSearchOptSpecs array.

Depends on D94472 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94474

Files:
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/ArgList.h
  llvm/lib/Option/ArgList.cpp

Index: llvm/lib/Option/ArgList.cpp
===
--- llvm/lib/Option/ArgList.cpp
+++ llvm/lib/Option/ArgList.cpp
@@ -95,6 +95,23 @@
   return std::vector(Values.begin(), Values.end());
 }
 
+void ArgList::AddAllArgsExcept(ArgStringList &Output,
+  ArrayRef ExcludeIds) const {
+  for (const Arg *Arg : *this) {
+bool Excluded = false;
+for (OptSpecifier Id : ExcludeIds) {
+  if (Arg->getOption().matches(Id)) {
+Excluded = true;
+break;
+  }
+}
+if (!Excluded) {
+  Arg->claim();
+  Arg->render(*this, Output);
+}
+  }
+}
+
 void ArgList::AddAllArgsExcept(ArgStringList &Output,
ArrayRef Ids,
ArrayRef ExcludeIds) const {
Index: llvm/include/llvm/Option/ArgList.h
===
--- llvm/include/llvm/Option/ArgList.h
+++ llvm/include/llvm/Option/ArgList.h
@@ -308,6 +308,10 @@
   A->render(*this, Output);
   }
 
+  /// AddAllArgsExcept - Render all arguments not matching any of the excluded
+  /// ids.
+  void AddAllArgsExcept(ArgStringList &Output,
+ArrayRef ExcludeIds = {}) const;
   /// AddAllArgsExcept - Render all arguments matching any of the given ids
   /// and not matching any of the excluded ids.
   void AddAllArgsExcept(ArgStringList &Output, ArrayRef Ids,
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1439,6 +1439,33 @@
   return Success;
 }
 
+bool CompilerInvocation::parseSimpleArgs(
+const ArgList &Args, DiagnosticsEngine &Diags,
+ArrayRef Included) {
+  bool Success = true;
+
+#define OPTION_WITH_MARSHALLING(   \
+PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
+HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
+MERGER, EXTRACTOR, TABLE_INDEX)\
+  if ((FLAGS)&options::CC1Option &&\
+  find(Included, OPT_##ID) != Included.end()) {\
+this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  \
+if (IMPLIED_CHECK) \
+  this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);\
+if (SHOULD_PARSE)  \
+  if (auto MaybeValue =\
+  NORMALIZER(OPT_##ID, TABLE_INDEX, Args, Diags, Success)) \
+this->KEYPATH = MERGER(\
+this->KEYPATH, static_castKEYPATH)>(*MaybeValue)); \
+  }
+#include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING
+
+  return Success;
+}
+
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
 DiagnosticsEngine *Diags,
 bool DefaultDiagColor) {
@@ -3099,8 +3126,92 @@
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts()

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 planned changes to this revision.
jansvoboda11 added a comment.

Marking this as "Changes Planned" to not occupy reviewers' Ready to Review 
queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94474

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

[PATCH] D93232: [AArch64] Adding ACLE intrinsics for the LS64 extension

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93232

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

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: vsavchenko, xazax.hun, martong, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, mgorny.
NoQ requested review of this revision.

This `clang::DiagnosticConsumer` can take normal clang diagnostics (eg., Sema 
errors or Clang-Tidy warnings) and turn them into static analyzer's 
`PathDiagnostic`s which are then fed into `PathDiagnosticConsumer`s. It's an 
essential component for my clang-tidy/static analyzer cross-integration efforts.

The consumer isn't magically aware of information that isn't present in Clang 
diagnostics but expected in path diagnostics. Such info includes checker name / 
bug type / category; the consumer expects an external provider to provide such 
information for every incoming diagnostic.

Additionally, `PathDiagnostic`'s declaration-with-issue and 
uniqueing-declaration are currently set to `nullptr`. I'm slowly thinking about 
auto-detecting those based on the source location but technically all path 
diagnostic consumer facilities can function just fine without them 
(declaration-with-issue is there purely for displaying user-visible names and 
uniqueing declaration only makes sense for path-sensitive warnings that require 
exotic uniqueing which will probably never be fed into this consumer).

There are also a few features missing here and there, such as fix-it hint 
support, which i plan to add later.

Though technically possible, this consumer is never supposed to consume the 
output of `TextPathDiagnosticConsumer`; a lot of information is lost that way 
that can't be recovered. In particular, the proper way to teach clang-tidy to 
emit path diagnostics while keeping in mind that it already ships with static 
analyzer integration is to allow the integrated static analyzer to talk to path 
diagnostic consumers directly; static analyzer warnings should never go through 
this new consumer.


https://reviews.llvm.org/D94476

Files:
  clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

Index: clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
@@ -0,0 +1,157 @@
+//===- unittests/Analysis/CFGTest.cpp - CFG tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ento;
+using namespace llvm;
+
+namespace {
+class TestPathDiagnosticConsumer : public PathDiagnosticConsumer {
+public:
+  struct ExpectedPieceTy {
+SourceLocation Loc;
+std::string Text;
+  };
+  struct ExpectedDiagTy {
+SourceLocation Loc;
+std::string VerboseDescription;
+std::string ShortDescription;
+std::string CheckerName;
+std::string BugType;
+std::string Category;
+std::vector Pieces;
+  };
+  typedef std::vector ExpectedDiagsTy;
+
+private:
+  ExpectedDiagsTy ExpectedDiags;
+
+public:
+  TestPathDiagnosticConsumer(ExpectedDiagsTy ExpectedDiags)
+: ExpectedDiags(ExpectedDiags)
+  {}
+
+  virtual StringRef getName() const override { return "test"; }
+
+  virtual void FlushDiagnosticsImpl(std::vector &Diags,
+FilesMade *filesMade) override {
+EXPECT_EQ(Diags.size(), ExpectedDiags.size());
+for (size_t I = 0, E = Diags.size(); I < E; ++I) {
+  const PathDiagnostic &Diag = *Diags[I];
+  const ExpectedDiagTy &ExpectedDiag = ExpectedDiags[I];
+
+  EXPECT_EQ(Diag.getLocation().asLocation(), ExpectedDiag.Loc);
+  EXPECT_EQ(Diag.getVerboseDescription(), ExpectedDiag.VerboseDescription);
+  EXPECT_EQ(Diag.getShortDescription(), ExpectedDiag.ShortDescription);
+  EXPECT_EQ(Diag.getCheckerName(), ExpectedDiag.CheckerName);
+  EXPECT_EQ(Diag.getBugType(), ExpectedDiag.BugType);
+  EXPECT_EQ(Diag.getCategory(), ExpectedDiag.Category);
+
+  const PathPieces &Pieces = Diag.path;
+  EXPECT_EQ(Pieces.size(), ExpectedDiag.Pieces.size());
+  size_t PieceI = 0;
+  for (const auto &Piece: Pieces) {
+const ExpectedPieceTy &ExpectedPiece = ExpectedDiag.Pieces[PieceI];
+EXPECT_EQ(Piece->getLocation().asLocation(), ExpectedPiece.Loc);
+

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Artem Dergachev via Phabricator via cfe-commits]

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

Add a few comments here and there.


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

https://reviews.llvm.org/D94476

Files:
  clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

Index: clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
@@ -0,0 +1,157 @@
+//===- unittests/Analysis/CFGTest.cpp - CFG tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ento;
+using namespace llvm;
+
+namespace {
+class TestPathDiagnosticConsumer : public PathDiagnosticConsumer {
+public:
+  struct ExpectedPieceTy {
+SourceLocation Loc;
+std::string Text;
+  };
+  struct ExpectedDiagTy {
+SourceLocation Loc;
+std::string VerboseDescription;
+std::string ShortDescription;
+std::string CheckerName;
+std::string BugType;
+std::string Category;
+std::vector Pieces;
+  };
+  typedef std::vector ExpectedDiagsTy;
+
+private:
+  ExpectedDiagsTy ExpectedDiags;
+
+public:
+  TestPathDiagnosticConsumer(ExpectedDiagsTy ExpectedDiags)
+: ExpectedDiags(ExpectedDiags)
+  {}
+
+  virtual StringRef getName() const override { return "test"; }
+
+  virtual void FlushDiagnosticsImpl(std::vector &Diags,
+FilesMade *filesMade) override {
+EXPECT_EQ(Diags.size(), ExpectedDiags.size());
+for (size_t I = 0, E = Diags.size(); I < E; ++I) {
+  const PathDiagnostic &Diag = *Diags[I];
+  const ExpectedDiagTy &ExpectedDiag = ExpectedDiags[I];
+
+  EXPECT_EQ(Diag.getLocation().asLocation(), ExpectedDiag.Loc);
+  EXPECT_EQ(Diag.getVerboseDescription(), ExpectedDiag.VerboseDescription);
+  EXPECT_EQ(Diag.getShortDescription(), ExpectedDiag.ShortDescription);
+  EXPECT_EQ(Diag.getCheckerName(), ExpectedDiag.CheckerName);
+  EXPECT_EQ(Diag.getBugType(), ExpectedDiag.BugType);
+  EXPECT_EQ(Diag.getCategory(), ExpectedDiag.Category);
+
+  const PathPieces &Pieces = Diag.path;
+  EXPECT_EQ(Pieces.size(), ExpectedDiag.Pieces.size());
+  size_t PieceI = 0;
+  for (const auto &Piece: Pieces) {
+const ExpectedPieceTy &ExpectedPiece = ExpectedDiag.Pieces[PieceI];
+EXPECT_EQ(Piece->getLocation().asLocation(), ExpectedPiece.Loc);
+EXPECT_EQ(Piece->getString(), ExpectedPiece.Text);
+++PieceI;
+  }
+}
+  }
+};
+
+PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo bugTypeInfoProvider(
+const Diagnostic &) {
+  return PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo{
+  "test check name", "test bug type", "test bug category"};
+}
+
+class PathDiagnosticConverterDiagnosticConsumerTestConsumer
+: public ASTConsumer {
+  SourceManager &SM;
+
+  void performTest(const Decl *D) {
+TestPathDiagnosticConsumer::ExpectedDiagsTy ExpectedDiags{
+TestPathDiagnosticConsumer::ExpectedDiagTy{
+D->getBeginLoc(),
+"Test warning", // Auto-capitalization!
+"Test warning", // Auto-capitalization!
+"test check name",
+"test bug type",
+"test bug category",
+{TestPathDiagnosticConsumer::ExpectedPieceTy{
+ D->getBeginLoc(),
+ "Test warning", // Auto-capitalization!
+ },
+ TestPathDiagnosticConsumer::ExpectedPieceTy{
+ D->getEndLoc(),
+ "Test note" // Auto-capitalization!
+ ;
+
+TestPathDiagnosticConsumer TestPDC {ExpectedDiags};
+PathDiagnosticConsumers PathConsumers = {&TestPDC};
+
+llvm::IntrusiveRefCntPtr DiagEngine =
+new DiagnosticsEngine(new DiagnosticIDs, new DiagnosticOptions);
+PathDiagnosticConverterDiagnosticConsumer DiagConsumer(
+PathConsumers, bugTypeInfoProvider,
+/*ShouldDisplayNotesAsEvents=*/true);
+DiagEngine->setClient(&DiagConsumer, /*ShouldOwnClient=*/false);
+DiagEngine->setSourceManager(&SM);
+
+unsigned WarningDiagID =
+DiagEngine->getCustomDiagID(DiagnosticsEngine::Warning, "test warning");
+DiagEngine->Report(D->getBeginLoc(), WarningDiagI

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

[Aleksandr Platonov via Phabricator via cfe-commits]

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

This patch is a try to fix `WorkspaceSymbols.Macros` test after D93796 
.
If a macro definition is in the preamble section, then it appears to be in the 
preamble (static) index and not in the main-file (dynamic) index.
Thus, a such macro could not be found at a symbol search according to the logic 
that we skip symbols from the static index if the location of these symbols is 
inside the dynamic index files.
To fix this behavior this patch adds main file macros into the main-file 
(dynamic) index.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94477

Files:
  clang-tools-extra/clangd/CollectMacros.cpp
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -341,7 +341,7 @@
   std::vector MacroExpansionPositions;
   for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
 for (const auto &R : SIDToRefs.second)
-  MacroExpansionPositions.push_back(R.start);
+  MacroExpansionPositions.push_back(R.first.start);
   }
   for (const auto &R : AST.getMacros().UnknownMacros)
 MacroExpansionPositions.push_back(R.start);
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -52,17 +52,7 @@
   return *SymbolInfos;
 }
 
-// FIXME: We update two indexes during main file processing:
-//- preamble index (static)
-//- main-file index (dynamic)
-//The macro in this test appears to be in the preamble index and not
-//in the main-file index. According to our logic of indexes merging, we
-//do not take this macro from the static (preamble) index, because it
-//location within the file from the dynamic (main-file) index.
-//
-//Possible solution is to exclude main-file symbols from the preamble
-//index, after that we can enable this test again.
-TEST(WorkspaceSymbols, DISABLED_Macros) {
+TEST(WorkspaceSymbols, Macros) {
   TestTU TU;
   TU.Code = R"cpp(
#define MACRO X
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -95,8 +95,10 @@
   assert(Macro);
   auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
 
-  EXPECT_THAT(ExpectedRefs,
-  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID]))
+  std::vector MacroRefs;
+  for (const auto &RangeAndIsDefinition : ActualMacroRefs.MacroRefs[SID])
+MacroRefs.push_back(RangeAndIsDefinition.first);
+  EXPECT_THAT(ExpectedRefs, UnorderedElementsAreArray(MacroRefs))
   << "Annotation=" << I << ", MacroName=" << Macro->Name
   << ", Test = " << Test;
 }
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -386,7 +386,9 @@
   const auto MainFileURI = toURI(SM, MainFileEntry->getName(), Opts);
   // Add macro references.
   for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
-for (const auto &Range : IDToRefs.second) {
+for (const auto &RangeAndIsDefinition : IDToRefs.second) {
+  const auto &Range = RangeAndIsDefinition.first;
+  bool IsDefinition = RangeAndIsDefinition.second;
   Ref R;
   R.Location.Start.setLine(Range.start.line);
   R.Location.Start.setColumn(Range.start.character);
@@ -394,8 +396,26 @@
   R.Location.End.setColumn(Range.end.character);
   R.Location.FileURI = MainFileURI.c_str();
   // FIXME: Add correct RefKind information to MainFileMacros.
-  R.Kind = RefKind::Reference;
+  R.Kind = IsDefinition ? RefKind::Definition : RefKind::Reference;
   Refs.insert(IDToRefs.first, R);
+  if (IsDefinition) {
+Symbol S;
+S.ID = IDToRefs.first;
+auto StartLoc = sourceLocationInMainFile(SM, Range.start);
+assert(StartLoc);
+   

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added reviewers: sammccall, kadircet.
ArcsinX added a comment.

I am not happy with my solution, hope to hear an advice.
Initial discussion about this problem: https://reviews.llvm.org/D93683#2468842


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477

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

[clang] c1e08f0 - [clang][AST] Get rid of an alignment hack in DeclObjC.h [NFCI]

[Mikhail Maltsev via cfe-commits]

Author: Mikhail Maltsev
Date: 2021-01-12T10:22:35Z
New Revision: c1e08f0073e35cf17c0a0343cf7efff914dbd66d

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

LOG: [clang][AST] Get rid of an alignment hack in DeclObjC.h [NFCI]

This code currently uses a union object to increase the
alignment of the type ObjCTypeParamList. The original intent of this
trick was to be able to use the expression `this + 1` to access the
beginning of a tail-allocated array of `ObjCTypeParamDecl *` pointers.

The code has since been refactored and uses `llvm::TrailingObjects` to
manage the tail-allocated array. This template takes care of
alignment, so the hack is no longer necessary.

This patch removes the union so that the `SourceRange` class can be
used directly instead of being re-implemented with raw representations
of source locations.

Reviewed By: aprantl

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

Added: 


Modified: 
clang/include/clang/AST/DeclObjC.h
clang/lib/AST/DeclObjC.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclObjC.h 
b/clang/include/clang/AST/DeclObjC.h
index 88cedbd91b6d..b1bce069920c 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -656,20 +656,8 @@ class ObjCTypeParamDecl : public TypedefNameDecl {
 /// \endcode
 class ObjCTypeParamList final
 : private llvm::TrailingObjects {
-  /// Stores the components of a SourceRange as a POD.
-  struct PODSourceRange {
-unsigned Begin;
-unsigned End;
-  };
-
-  union {
-/// Location of the left and right angle brackets.
-PODSourceRange Brackets;
-
-// Used only for alignment.
-ObjCTypeParamDecl *AlignmentHack;
-  };
-
+  /// Location of the left and right angle brackets.
+  SourceRange Brackets;
   /// The number of parameters in the list, which are tail-allocated.
   unsigned NumParams;
 
@@ -717,17 +705,9 @@ class ObjCTypeParamList final
 return *(end() - 1);
   }
 
-  SourceLocation getLAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.Begin);
-  }
-
-  SourceLocation getRAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.End);
-  }
-
-  SourceRange getSourceRange() const {
-return SourceRange(getLAngleLoc(), getRAngleLoc());
-  }
+  SourceLocation getLAngleLoc() const { return Brackets.getBegin(); }
+  SourceLocation getRAngleLoc() const { return Brackets.getEnd(); }
+  SourceRange getSourceRange() const { return Brackets; }
 
   /// Gather the default set of type arguments to be substituted for
   /// these type parameters when dealing with an unspecialized type.

diff  --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp
index 961230fb54ce..5f82fcec90e3 100644
--- a/clang/lib/AST/DeclObjC.cpp
+++ b/clang/lib/AST/DeclObjC.cpp
@@ -1461,9 +1461,7 @@ SourceRange ObjCTypeParamDecl::getSourceRange() const {
 ObjCTypeParamList::ObjCTypeParamList(SourceLocation lAngleLoc,
  ArrayRef typeParams,
  SourceLocation rAngleLoc)
-: NumParams(typeParams.size()) {
-  Brackets.Begin = lAngleLoc.getRawEncoding();
-  Brackets.End = rAngleLoc.getRawEncoding();
+: Brackets(lAngleLoc, rAngleLoc), NumParams(typeParams.size()) {
   std::copy(typeParams.begin(), typeParams.end(), begin());
 }
 



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

[PATCH] D94224: [clang][AST] Get rid of an alignment hack in DeclObjC.h [NFCI]

[Mikhail Maltsev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc1e08f0073e3: [clang][AST] Get rid of an alignment hack in 
DeclObjC.h [NFCI] (authored by miyuki).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94224

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/lib/AST/DeclObjC.cpp


Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -1461,9 +1461,7 @@
 ObjCTypeParamList::ObjCTypeParamList(SourceLocation lAngleLoc,
  ArrayRef typeParams,
  SourceLocation rAngleLoc)
-: NumParams(typeParams.size()) {
-  Brackets.Begin = lAngleLoc.getRawEncoding();
-  Brackets.End = rAngleLoc.getRawEncoding();
+: Brackets(lAngleLoc, rAngleLoc), NumParams(typeParams.size()) {
   std::copy(typeParams.begin(), typeParams.end(), begin());
 }
 
Index: clang/include/clang/AST/DeclObjC.h
===
--- clang/include/clang/AST/DeclObjC.h
+++ clang/include/clang/AST/DeclObjC.h
@@ -656,20 +656,8 @@
 /// \endcode
 class ObjCTypeParamList final
 : private llvm::TrailingObjects {
-  /// Stores the components of a SourceRange as a POD.
-  struct PODSourceRange {
-unsigned Begin;
-unsigned End;
-  };
-
-  union {
-/// Location of the left and right angle brackets.
-PODSourceRange Brackets;
-
-// Used only for alignment.
-ObjCTypeParamDecl *AlignmentHack;
-  };
-
+  /// Location of the left and right angle brackets.
+  SourceRange Brackets;
   /// The number of parameters in the list, which are tail-allocated.
   unsigned NumParams;
 
@@ -717,17 +705,9 @@
 return *(end() - 1);
   }
 
-  SourceLocation getLAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.Begin);
-  }
-
-  SourceLocation getRAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.End);
-  }
-
-  SourceRange getSourceRange() const {
-return SourceRange(getLAngleLoc(), getRAngleLoc());
-  }
+  SourceLocation getLAngleLoc() const { return Brackets.getBegin(); }
+  SourceLocation getRAngleLoc() const { return Brackets.getEnd(); }
+  SourceRange getSourceRange() const { return Brackets; }
 
   /// Gather the default set of type arguments to be substituted for
   /// these type parameters when dealing with an unspecialized type.


Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -1461,9 +1461,7 @@
 ObjCTypeParamList::ObjCTypeParamList(SourceLocation lAngleLoc,
  ArrayRef typeParams,
  SourceLocation rAngleLoc)
-: NumParams(typeParams.size()) {
-  Brackets.Begin = lAngleLoc.getRawEncoding();
-  Brackets.End = rAngleLoc.getRawEncoding();
+: Brackets(lAngleLoc, rAngleLoc), NumParams(typeParams.size()) {
   std::copy(typeParams.begin(), typeParams.end(), begin());
 }
 
Index: clang/include/clang/AST/DeclObjC.h
===
--- clang/include/clang/AST/DeclObjC.h
+++ clang/include/clang/AST/DeclObjC.h
@@ -656,20 +656,8 @@
 /// \endcode
 class ObjCTypeParamList final
 : private llvm::TrailingObjects {
-  /// Stores the components of a SourceRange as a POD.
-  struct PODSourceRange {
-unsigned Begin;
-unsigned End;
-  };
-
-  union {
-/// Location of the left and right angle brackets.
-PODSourceRange Brackets;
-
-// Used only for alignment.
-ObjCTypeParamDecl *AlignmentHack;
-  };
-
+  /// Location of the left and right angle brackets.
+  SourceRange Brackets;
   /// The number of parameters in the list, which are tail-allocated.
   unsigned NumParams;
 
@@ -717,17 +705,9 @@
 return *(end() - 1);
   }
 
-  SourceLocation getLAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.Begin);
-  }
-
-  SourceLocation getRAngleLoc() const {
-return SourceLocation::getFromRawEncoding(Brackets.End);
-  }
-
-  SourceRange getSourceRange() const {
-return SourceRange(getLAngleLoc(), getRAngleLoc());
-  }
+  SourceLocation getLAngleLoc() const { return Brackets.getBegin(); }
+  SourceLocation getRAngleLoc() const { return Brackets.getEnd(); }
+  SourceRange getSourceRange() const { return Brackets; }
 
   /// Gather the default set of type arguments to be substituted for
   /// these type parameters when dealing with an unspecialized type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Ah this is really unfortunate :( In theory we already know which slabs belong 
to main file while updating the index for the preamble, as we shard them per 
file. But it is really inconvenient layering wise to transfer that slab from 
`updatePreamble` to `updateMain` in parsing callbacks :/
Another layer to fix the issue would be to somehow make main file parsing, 
re-parse those macro definitions through replay mechanism or preamble patching, 
but these might have unwanted consequences as we would see a re-definition of 
the macro now. Also they are considerably hard :/

So it feels like the solution proposed here isn't so bad after all (unless 
there are other alternatives i am missing ATM). I would suggest having a 
separate container for macro definitions though, rather than modifying the 
existing "references container". We could directly have 
SymbolSlab/vector for the definitions and merge them with 
SymbolCollector's existing slab during the post-processing. That way we can 
also re-use the logic in `SymbolCollector::handleMacroOccurence` to generate a 
`Symbol` from `MacroInfo`.

On a side note, we can also easily skip the shard belonging to main file inside 
FileIndex::updatePreamble, which is going to be suppressed by new merged-index 
logic anyway. Though I think it deserves a separate patch. Also I don't fully 
grasp the workings of the suppression logic yet, so it might not be wise to do 
so if there are any cases in which we still merge information from static index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477

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

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h:23
+
+class PathDiagnosticConverterDiagnosticConsumer : public DiagnosticConsumer {
+public:

I think that this class (or file?) needs to have some comments on how it is 
supposed to be used.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:38
+  Msg[0] = toupper(Msg[0]);
+  std::string CleanMsg = Msg.str().substr(0, Msg.str().find('[') - 1).str();
+

I think this type of stuff should be covered by a comment or a descriptive 
function name.



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:1
+//===- unittests/Analysis/CFGTest.cpp - CFG tests 
-===//
+//

Looks like a copy-paste error :)



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:63
+  size_t PieceI = 0;
+  for (const auto &Piece: Pieces) {
+const ExpectedPieceTy &ExpectedPiece = ExpectedDiag.Pieces[PieceI];

nit



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:63
+  size_t PieceI = 0;
+  for (const auto &Piece: Pieces) {
+const ExpectedPieceTy &ExpectedPiece = ExpectedDiag.Pieces[PieceI];

vsavchenko wrote:
> nit
nit: `llvm::enumerate` or `llvm::zip`?



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:83
+
+  void performTest(const Decl *D) {
+TestPathDiagnosticConsumer::ExpectedDiagsTy ExpectedDiags{

TBH it is pretty hard to follow the test and from it's structure see what is 
the input and what is expected output.  Maybe it can be reorganized a bit?



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:139
+
+class PathDiagnosticConverterDiagnosticConsumerTestAction
+: public ASTFrontendAction {

WE NEED MORE NOUNS!



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:149-150
+
+// Constructing a CFG for a range-based for over a dependent type fails (but
+// should not crash).
+TEST(PathDiagnosticConverter, ConsumeWarning) {

Also doesn't seem relevant



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:154
+  std::make_unique(),
+  "void foo() {}", "input.c"));
+}

It is also about the structure, I guess it would've been nice to have all the 
inputs and expected outputs to be specified here in the actual test, so the 
reader can figure out what is tested without diving deep into the classes. And 
it also seems that with the current structure you'll need a couple more classes 
for every new test.


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

https://reviews.llvm.org/D94476

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

[PATCH] D94488: [clang][cli] Port more CodeGenOptions to marshalling infrastructure

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Leveraging the recently added marshalling constructs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94488

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -991,7 +991,6 @@
 setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
 
   Opts.CodeModel = TargetOpts.CodeModel;
-  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
@@ -1051,14 +1050,6 @@
 Opts.MemoryProfileOutput = MemProfileBasename;
 
   if (Opts.EmitGcovArcs || Opts.EmitGcovNotes) {
-Opts.CoverageDataFile =
-std::string(Args.getLastArgValue(OPT_coverage_data_file));
-Opts.CoverageNotesFile =
-std::string(Args.getLastArgValue(OPT_coverage_notes_file));
-Opts.ProfileFilterFiles =
-std::string(Args.getLastArgValue(OPT_fprofile_filter_files_EQ));
-Opts.ProfileExcludeFiles =
-std::string(Args.getLastArgValue(OPT_fprofile_exclude_files_EQ));
 if (Args.hasArg(OPT_coverage_version_EQ)) {
   StringRef CoverageVersion = Args.getLastArgValue(OPT_coverage_version_EQ);
   if (CoverageVersion.size() != 4) {
@@ -1091,11 +1082,6 @@
 }
   }
 
-  Opts.XRayTotalFunctionGroups =
-  getLastArgIntValue(Args, OPT_fxray_function_groups, 1, Diags);
-  Opts.XRaySelectedFunctionGroup =
-  getLastArgIntValue(Args, OPT_fxray_selected_function_group, 0, Diags);
-
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
   if (XRayInstrBundles.empty())
@@ -1120,15 +1106,6 @@
 }
   }
 
-  if (const Arg *A = Args.getLastArg(OPT_compress_debug_sections_EQ)) {
-auto DCT = llvm::StringSwitch(A->getValue())
-   .Case("none", llvm::DebugCompressionType::None)
-   .Case("zlib", llvm::DebugCompressionType::Z)
-   .Case("zlib-gnu", llvm::DebugCompressionType::GNU)
-   .Default(llvm::DebugCompressionType::None);
-Opts.setCompressDebugSections(DCT);
-  }
-
   for (auto *A :
Args.filtered(OPT_mlink_bitcode_file, OPT_mlink_builtin_bitcode)) {
 CodeGenOptions::BitcodeFileToLink F;
@@ -1142,29 +1119,10 @@
 }
 Opts.LinkBitcodeFiles.push_back(F);
   }
-  Opts.SSPBufferSize =
-  getLastArgIntValue(Args, OPT_stack_protector_buffer_size, 8, Diags);
-
-  Opts.StackProtectorGuard =
-  std::string(Args.getLastArgValue(OPT_mstack_protector_guard_EQ));
-
-  if (Arg *A = Args.getLastArg(OPT_mstack_protector_guard_offset_EQ)) {
-StringRef Val = A->getValue();
-unsigned Offset = Opts.StackProtectorGuardOffset;
-Val.getAsInteger(10, Offset);
-Opts.StackProtectorGuardOffset = Offset;
-  }
-
-  Opts.StackProtectorGuardReg =
-  std::string(Args.getLastArgValue(OPT_mstack_protector_guard_reg_EQ,
-   "none"));
-
 
   if (Args.getLastArg(OPT_femulated_tls) ||
   Args.getLastArg(OPT_fno_emulated_tls)) {
 Opts.ExplicitEmulatedTLS = true;
-Opts.EmulatedTLS =
-Args.hasFlag(OPT_femulated_tls, OPT_fno_emulated_tls, false);
   }
 
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1159,12 +1159,22 @@
 HelpText<"Disable using instrumentation data for profile-guided optimization">;
 def fno_profile_use : Flag<["-"], "fno-profile-use">,
 Alias;
+defm profile_arcs : BoolFOption<"profile-arcs",
+  "CodeGenOpts.EmitGcovArcs", DefaultsToFalse,
+  ChangedBy, ResetBy>;
+defm test_coverage : BoolFOption<"test-coverage",
+  "CodeGenOpts.EmitGcovNotes", DefaultsToFalse,
+  ChangedBy, ResetBy>;
 def fprofile_filter_files_EQ : Joined<["-"], "fprofile-filter-files=">,
 Group, Flags<[CC1Option, CoreOption]>,
-HelpText<"Instrument only functions from files where names match any regex separated by a semi-colon">;
+HelpText<"Instrument only functions from files where names match any regex separated by a semi-colon">,
+MarshallingInfoString<"CodeGenOpts.ProfileFilterFiles">,
+ShouldParseIf;
 def fprofile_exclude_files_EQ : Joined<["-"], "fprofile-exclude-files=">,
 Group, Flags<[CC1Option, CoreOption]>,
-HelpText<"Instrument only functions from files where names don't match all the regexes separated by a semi-colon">;
+HelpText<"Instrument only functions from files where names don't match 

[PATCH] D94488: [clang][cli] Port more CodeGenOptions to marshalling infrastructure

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1165
+  ChangedBy, ResetBy>;
+defm test_coverage : BoolFOption<"test-coverage",
+  "CodeGenOpts.EmitGcovNotes", DefaultsToFalse,

The `profile_arcs` and `test_coverage` options were moved here so that 
`fprofile_filter_files_EQ` and `fprofile_exclude_files_EQ` can refer to their 
keypaths.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1146
-  Opts.SSPBufferSize =
-  getLastArgIntValue(Args, OPT_stack_protector_buffer_size, 8, Diags);
-

This piece of code should've been removed in D84669 which added marshalling 
info to `stack_protector_buffer_size`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94488

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

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

[Richard Barton via Phabricator via cfe-commits]

richard.barton.arm added a comment.

I think the right long-term solution is going to be SUGGESTION 2, or a variant 
of that. We have implemented this feature downstream in Arm Compiler for Linux, 
i.e. a separate, normally longer and more detailed, Doc string for the 
CommandLineReference.rst file from the --help string. Such a mechanism could 
perhaps be extended to add Clang- and/or Flang- specific doc strings or 
additional doc strings. That's a big piece of work, but very worthy IMO.

To unblock development today I would support Andrzej's SUGGESTION 1. Perhaps 
saying "For C++ input ..." to distinguish the Clang-only behaviour for now.

In writing the above I double checked the Options.td to see if anything 
upstream exists today. There is a field DocBrief that appears to be preferred 
over HelpText in the ClangCommandLineReference.rst. Perhaps the new, more 
detailed but C++ specific message could be a DocBrief field, so for the 
ClangCommandLineReference.rst only and we can revert to the original shorter 
HelpText that applies for both C++ and Fortran? Perhaps that would actually be 
a better short-term compromise in that it meets @andreil99 's needs as well as 
Flangs immediate needs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

[Sam McCall via Phabricator via cfe-commits]

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

Ah, this is a bit ugly... I'm not really happy with our representation of info 
extracted from the preprocessor, but that's something to tackle another time.

Thanks for fixing this!




Comment at: clang-tools-extra/clangd/CollectMacros.h:31
   // SourceManager from preamble is not available when we build the AST.
-  llvm::DenseMap> MacroRefs;
+  llvm::DenseMap>> MacroRefs;
   // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a

this pair should be a struct instead - MacroOccurrence?



Comment at: clang-tools-extra/clangd/CollectMacros.h:35
   // highlighting.
   std::vector UnknownMacros;
   // Ranges skipped by the preprocessor due to being inactive.

this should probably also be MacroOccurrence?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:398
   R.Location.FileURI = MainFileURI.c_str();
   // FIXME: Add correct RefKind information to MainFileMacros.
+  R.Kind = IsDefinition ? RefKind::Definition : RefKind::Reference;

this fixme is now addressed



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:405
+auto StartLoc = sourceLocationInMainFile(SM, Range.start);
+assert(StartLoc);
+auto EndLoc = sourceLocationInMainFile(SM, Range.end);

nit: prefer SourceLocation StartLoc = cantFail(sourceLocationMainFile(...));



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:408
+assert(EndLoc);
+S.Name = Lexer::getSourceText(
+CharSourceRange::getCharRange(SourceRange(*StartLoc, *EndLoc)), SM,

nit: we have `toSourceCode(SourceRange(StartLoc, EndLoc))` for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477

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

[clang] c4944a6 - [Fixed Point] Add codegen for conversion between fixed-point and floating point.

[Bjorn Pettersson via cfe-commits]

Author: Bevin Hansson
Date: 2021-01-12T13:53:01+01:00
New Revision: c4944a6f53f6d1876e76563599f5f149328e7f8f

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

LOG: [Fixed Point] Add codegen for conversion between fixed-point and floating 
point.

The patch adds the required methods to FixedPointBuilder
for converting between fixed-point and floating point,
and uses them from Clang.

This depends on D54749.

Reviewed By: leonardchan

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

Added: 
clang/test/Frontend/fixed_point_conversions_half.c

Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/Frontend/fixed_point_compound.c
clang/test/Frontend/fixed_point_conversions.c
llvm/include/llvm/IR/FixedPointBuilder.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index d6d5ec544c08..6f7e8263fa10 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1212,13 +1212,14 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value 
*Src, QualType SrcType,
   // padding is enabled because overflow into this bit is undefined
   // behavior.
   return Builder.CreateIsNotNull(Src, "tobool");
-if (DstType->isFixedPointType() || DstType->isIntegerType())
+if (DstType->isFixedPointType() || DstType->isIntegerType() ||
+DstType->isRealFloatingType())
   return EmitFixedPointConversion(Src, SrcType, DstType, Loc);
 
 llvm_unreachable(
 "Unhandled scalar conversion from a fixed point type to another 
type.");
   } else if (DstType->isFixedPointType()) {
-if (SrcType->isIntegerType())
+if (SrcType->isIntegerType() || SrcType->isRealFloatingType())
   // This also includes converting booleans and enums to fixed point types.
   return EmitFixedPointConversion(Src, SrcType, DstType, Loc);
 
@@ -1434,19 +1435,29 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value 
*Src, QualType SrcType,
 Value *ScalarExprEmitter::EmitFixedPointConversion(Value *Src, QualType SrcTy,
QualType DstTy,
SourceLocation Loc) {
-  auto SrcFPSema = CGF.getContext().getFixedPointSemantics(SrcTy);
-  auto DstFPSema = CGF.getContext().getFixedPointSemantics(DstTy);
   llvm::FixedPointBuilder FPBuilder(Builder);
   llvm::Value *Result;
-  if (DstTy->isIntegerType())
-Result = FPBuilder.CreateFixedToInteger(Src, SrcFPSema,
-DstFPSema.getWidth(),
-DstFPSema.isSigned());
-  else if (SrcTy->isIntegerType())
-Result =  FPBuilder.CreateIntegerToFixed(Src, SrcFPSema.isSigned(),
- DstFPSema);
-  else
-Result = FPBuilder.CreateFixedToFixed(Src, SrcFPSema, DstFPSema);
+  if (SrcTy->isRealFloatingType())
+Result = FPBuilder.CreateFloatingToFixed(Src,
+CGF.getContext().getFixedPointSemantics(DstTy));
+  else if (DstTy->isRealFloatingType())
+Result = FPBuilder.CreateFixedToFloating(Src,
+CGF.getContext().getFixedPointSemantics(SrcTy),
+ConvertType(DstTy));
+  else {
+auto SrcFPSema = CGF.getContext().getFixedPointSemantics(SrcTy);
+auto DstFPSema = CGF.getContext().getFixedPointSemantics(DstTy);
+
+if (DstTy->isIntegerType())
+  Result = FPBuilder.CreateFixedToInteger(Src, SrcFPSema,
+  DstFPSema.getWidth(),
+  DstFPSema.isSigned());
+else if (SrcTy->isIntegerType())
+  Result =  FPBuilder.CreateIntegerToFixed(Src, SrcFPSema.isSigned(),
+   DstFPSema);
+else
+  Result = FPBuilder.CreateFixedToFixed(Src, SrcFPSema, DstFPSema);
+  }
   return Result;
 }
 

diff  --git a/clang/test/Frontend/fixed_point_compound.c 
b/clang/test/Frontend/fixed_point_compound.c
index 897ba2e22636..5dcc7fba0da7 100644
--- a/clang/test/Frontend/fixed_point_compound.c
+++ b/clang/test/Frontend/fixed_point_compound.c
@@ -16,6 +16,8 @@ int i;
 unsigned int u;
 signed char c;
 
+float fl;
+
 
 // CHECK-LABEL: @add_shfa(
 // CHECK-NEXT:  entry:
@@ -358,6 +360,66 @@ void add_sshsuf() {
   sshf += suf;
 }
 
+// CHECK-LABEL: @add_afl(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load float, float* @fl, align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, i32* @a, align 4
+// CHECK-NEXT:[[TMP2:%.*]] = sitofp i32 [[TMP1]] to float
+// CHECK-NEXT:[[TMP3:%.*]] = fmul float [[TMP2]], 0x3F00
+// CHECK-NEXT:[[ADD:%.*]] = fadd float [[TMP3]], [[TMP0]]
+// CHECK-NEXT:[[TMP4:%.*]] = fmul float [[ADD]], 3.276800e+04
+// CHECK-NEXT:[[TMP5:%.

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

[Bjorn Pettersson 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 rGc4944a6f53f6: [Fixed Point] Add codegen for conversion 
between fixed-point and floating point. (authored by ebevhan, committed by 
bjope).
Herald added a subscriber: dexonsmith.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/Frontend/fixed_point_compound.c
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_conversions_half.c
  llvm/include/llvm/IR/FixedPointBuilder.h

Index: llvm/include/llvm/IR/FixedPointBuilder.h
===
--- llvm/include/llvm/IR/FixedPointBuilder.h
+++ llvm/include/llvm/IR/FixedPointBuilder.h
@@ -120,6 +120,16 @@
 C.isSigned(), C.isSaturated(), BothPadded);
   }
 
+  /// Given a floating point type and a fixed-point semantic, return a floating
+  /// point type which can accommodate the fixed-point semantic. This is either
+  /// \p Ty, or a floating point type with a larger exponent than Ty.
+  Type *getAccommodatingFloatType(Type *Ty, const FixedPointSemantics &Sema) {
+const fltSemantics *FloatSema = &Ty->getFltSemantics();
+while (!Sema.fitsInFloatSemantics(*FloatSema))
+  FloatSema = APFixedPoint::promoteFloatSemantics(FloatSema);
+return Type::getFloatingPointTy(Ty->getContext(), *FloatSema);
+  }
+
 public:
   FixedPointBuilder(IRBuilderTy &Builder) : B(Builder) {}
 
@@ -159,6 +169,55 @@
DstSema, false);
   }
 
+  Value *CreateFixedToFloating(Value *Src, const FixedPointSemantics &SrcSema,
+   Type *DstTy) {
+Value *Result;
+Type *OpTy = getAccommodatingFloatType(DstTy, SrcSema);
+// Convert the raw fixed-point value directly to floating point. If the
+// value is too large to fit, it will be rounded, not truncated.
+Result = SrcSema.isSigned() ? B.CreateSIToFP(Src, OpTy)
+: B.CreateUIToFP(Src, OpTy);
+// Rescale the integral-in-floating point by the scaling factor. This is
+// lossless, except for overflow to infinity which is unlikely.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, -(int)SrcSema.getScale(;
+if (OpTy != DstTy)
+  Result = B.CreateFPTrunc(Result, DstTy);
+return Result;
+  }
+
+  Value *CreateFloatingToFixed(Value *Src, const FixedPointSemantics &DstSema) {
+bool UseSigned = DstSema.isSigned() || DstSema.hasUnsignedPadding();
+Value *Result = Src;
+Type *OpTy = getAccommodatingFloatType(Src->getType(), DstSema);
+if (OpTy != Src->getType())
+  Result = B.CreateFPExt(Result, OpTy);
+// Rescale the floating point value so that its significant bits (for the
+// purposes of the conversion) are in the integral range.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, DstSema.getScale(;
+
+Type *ResultTy = B.getIntNTy(DstSema.getWidth());
+if (DstSema.isSaturated()) {
+  Intrinsic::ID IID =
+  UseSigned ? Intrinsic::fptosi_sat : Intrinsic::fptoui_sat;
+  Result = B.CreateIntrinsic(IID, {ResultTy, OpTy}, {Result});
+} else {
+  Result = UseSigned ? B.CreateFPToSI(Result, ResultTy)
+ : B.CreateFPToUI(Result, ResultTy);
+}
+
+// When saturating unsigned-with-padding using signed operations, we may
+// get negative values. Emit an extra clamp to zero.
+if (DstSema.isSaturated() && DstSema.hasUnsignedPadding()) {
+  Constant *Zero = Constant::getNullValue(Result->getType());
+  Result =
+  B.CreateSelect(B.CreateICmpSLT(Result, Zero), Zero, Result, "satmin");
+}
+
+return Result;
+  }
+
   /// Add two fixed-point values and return the result in their common semantic.
   /// \p LHS - The left hand side
   /// \p LHSSema - The semantic of the left hand side
Index: clang/test/Frontend/fixed_point_conversions_half.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_conversions_half.c
@@ -0,0 +1,309 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
+short _Fract sf;
+long _Fract lf;
+
+short _Accum sa;
+long _Accum la;
+
+unsigned short _Accum usa;
+unsigned long _Accum ula;
+
+_Sat short _Fract sf_sat;
+_Sat long _Fract lf_sat;
+
+_Sat short _Accum sa_sat;
+_Sat long _Accum la_sat;
+
+_Sat unsigned short _Accum usa_sat;
+_Sat unsigned long _Accum ula_sat;
+
+_Float16 h

[PATCH] D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)

[Michael Kiausch via Phabricator via cfe-commits]

kiausch added a comment.

Could somebody with write access please commit this patch if there are no 
further objections? Thanks!


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

https://reviews.llvm.org/D92935

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

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

I've just landed this on behalf of @ebevhan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

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

[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a few small issues to fix.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:893
+def err_pragma_pack_identifer_not_supported : Error<
+  "specifying an identifier within pragma pack is not supported, identifier is 
ignored">;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;

The identifier is no longer ignored now that this is an error rather than a 
warning.



Comment at: clang/test/Sema/aix-pragma-pack-and-align.c:231
+
+// expected-no-warning

Xiangling_L wrote:
> aaron.ballman wrote:
> > Is this comment intentional?
> Yes, my intention is to test no pragma pack or prama align is unterminated.
I don't think we have such a construct that's checked by `-verify` (and if we 
did, I'd say the test file is broken because it contains `expected-warning` 
comments).

Because you're passing `-verify`, any warnings that are triggered on a line 
without a matching `expected-warning` comment will be reported as a test 
failure, so you can safely remove this comment and still get the test coverage 
you want.


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

https://reviews.llvm.org/D87702

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf updated this revision to Diff 316068.
tinloaf added a comment.

Fixed formatting issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11330,8 +11330,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -11569,8 +11569,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -12405,9 +12405,9 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
-  Style.AlignConsecutiveDeclarations = true;
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
 
   verifyFormat("#define a 3\n"
"#define  4\n"
@@ -12431,7 +12431,7 @@
"#define (x, y) (x - y)",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a3\n"
"#define  4\n"
"#define ccc  (5)",
@@ -12471,7 +12471,7 @@
"};",
Style);
 
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
   Style.ColumnLimit = 20;
 
   verifyFormat("#define a  \\\n"
@@ -12485,7 +12485,7 @@
"  \"\"\n",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a  \\\n"
"  \"aa\"\n"
"#define D  \\\n"
@@ -12496,12 +12496,766 @@
"  \"F\"  \\\n"
"  \"\"\n",
Style);
+
+  // Test across comments
+  Style.MaxEmptyLinesToKeep = 10;
+  Style.ReflowComments = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_AcrossComments;
+  EXPECT_EQ("#define a3\n"
+"// line comment\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "// line comment\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "/* block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* multi-line *\n"
+" * block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "/* multi-line *\n"
+   " * block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"// multi-line line comment\n"
+"//\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "// multi-line line comment\n"
+   "//\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a 3\n"
+"// empty lines still break.\n"
+"\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "// empty lines still break.\n"
+   "\n"
+   "#define  4\n"
+   "#define ccc  (5)",
+   Style));
+
+  // Test across empty lines
+  Style.AlignConsecutive

[PATCH] D94473: [clangd] Use AST-based signals in CodeCompletion.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 316071.
usaxena95 added a comment.

Edge cases fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94473

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h
  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
@@ -15,6 +15,7 @@
 #include "Quality.h"
 #include "SourceCode.h"
 #include "SyncAPI.h"
+#include "TUScheduler.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
@@ -51,6 +52,8 @@
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.Name == Name; }
+MATCHER_P(MainFileRefs, Refs, "") { return arg.MainFileRefs == Refs; }
+MATCHER_P(ScopeRefs, Refs, "") { return arg.ScopeRefsInFile == Refs; }
 MATCHER_P(NameStartsWith, Prefix, "") {
   return llvm::StringRef(arg.Name).startswith(Prefix);
 }
@@ -1115,6 +1118,46 @@
   UnorderedElementsAre(Named("xy1"), Named("xy2")));
 }
 
+TEST(CompletionTest, ASTSignals) {
+  struct Completion {
+std::string Name;
+unsigned MainFileRefs;
+unsigned ScopeRefsInFile;
+  };
+  CodeCompleteOptions Opts;
+  std::vector RecordedCompletions;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &R,
+   float Score) {
+RecordedCompletions.push_back({CC.Name, R.MainFileRefs, R.ScopeRefsInFile});
+  };
+  ASTSignals MainFileSignals;
+  MainFileSignals.Symbols[var("xy1").ID] = 3;
+  MainFileSignals.Symbols[var("xy2").ID] = 1;
+  MainFileSignals.Symbols[var("xyindex").ID] = 10;
+  MainFileSignals.RelatedNamespaces["tar::"] = 5;
+  MainFileSignals.RelatedNamespaces["clang::"] = 3;
+  Opts.MainFileSignals = &MainFileSignals;
+  Opts.AllScopes = true;
+  completions(R"cpp(
+  int xy1;
+  int xy2;
+  namespace clang {
+  int xybar = 1;
+  int a = xy^
+  }
+  )cpp",
+  /*IndexSymbols=*/{var("xyindex"), var("tar::xyfoo")}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(
+  AllOf(Named("xy1"), MainFileRefs(3u), ScopeRefs(0u)),
+  AllOf(Named("xy2"), MainFileRefs(1u), ScopeRefs(0u)),
+  AllOf(Named("xyindex"), MainFileRefs(10u), ScopeRefs(0u)),
+  AllOf(Named("xyfoo"), MainFileRefs(0u), ScopeRefs(5u)),
+  AllOf(Named("xybar"), MainFileRefs(0u), ScopeRefs(3u;
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/Quality.h
===
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -29,6 +29,7 @@
 
 #include "ExpectedTypes.h"
 #include "FileDistance.h"
+#include "TUScheduler.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -140,6 +141,10 @@
   /// CompletionPrefix.
   unsigned FilterLength = 0;
 
+  const ASTSignals *MainFileSignals = nullptr;
+  unsigned MainFileRefs = 0;
+  unsigned ScopeRefsInFile = 0;
+
   /// Set of derived signals computed by calculateDerivedSignals(). Must not be
   /// set explicitly.
   struct DerivedSignals {
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -294,6 +294,13 @@
   if (!(IndexResult.Flags & Symbol::VisibleOutsideFile)) {
 Scope = AccessibleScope::FileScope;
   }
+  if (MainFileSignals) {
+MainFileRefs =
+std::max(MainFileRefs, MainFileSignals->Symbols.lookup(IndexResult.ID));
+ScopeRefsInFile =
+std::max(ScopeRefsInFile,
+ MainFileSignals->RelatedNamespaces.lookup(IndexResult.Scope));
+  }
 }
 
 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
@@ -314,6 +321,20 @@
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
 InBaseClass |= SemaCCResult.InBaseClass;
   }
+  if (MainFileSignals)
+if ((SemaCCResult.Kind == CodeCompletionResult::RK_Declaration) ||
+(SemaCCResult.Kind == CodeCompletionResult::RK_Pattern))
+  if (const NamedDecl *ND = SemaCCResult.getDeclaration()) {
+  

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 316074.
awarzynski added a comment.

Use `DocBrief`, as suggested by @richard.barton.arm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -649,12 +649,13 @@
  "remove current directory from include path">;
 def I : JoinedOrSeparate<["-"], "I">, Group,
 Flags<[CC1Option,CC1AsOption]>, MetaVarName<"">,
-HelpText<"Add directory to include search path. If there are multiple -I "
- "options, these directories are searched in the order they are "
- "given before the standard system directories are searched. "
- "If the same directory is in the SYSTEM include search paths, for 
"
- "example if also specified with -isystem, the -I option will be "
- "ignored">;
+HelpText<"Add directory to include search path">,
+DocBrief<[{Add directory to include search path. For C++ inputs, if
+there are multiple -I options, these directories are searched
+in the order they are given before the standard system directories
+are searched. If the same directory is in the SYSTEM include search
+paths, for example if also specified with -isystem, the -I option
+will be ignored}]>;
 def L : JoinedOrSeparate<["-"], "L">, Flags<[RenderJoined]>, Group,
 MetaVarName<"">, HelpText<"Add directory to library search path">;
 def MD : Flag<["-"], "MD">, Group,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -1016,7 +1016,12 @@
 
 .. option:: -I, --include-directory , --include-directory=
 
-Add directory to include search path. If there are multiple -I options, these 
directories are searched in the order they are given before the standard system 
directories are searched. If the same directory is in the SYSTEM include search 
paths, for example if also specified with -isystem, the -I option will be 
ignored
+Add directory to include search path. For C++ input, if
+there are multiple -I options, these directories are searched
+in the order they are given before the standard system directories
+are searched. If the same directory is in the SYSTEM include search
+paths, for example if also specified with -isystem, the -I option
+will be ignored
 
 .. option:: -I-, --include-barrier
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -649,12 +649,13 @@
  "remove current directory from include path">;
 def I : JoinedOrSeparate<["-"], "I">, Group,
 Flags<[CC1Option,CC1AsOption]>, MetaVarName<"">,
-HelpText<"Add directory to include search path. If there are multiple -I "
- "options, these directories are searched in the order they are "
- "given before the standard system directories are searched. "
- "If the same directory is in the SYSTEM include search paths, for "
- "example if also specified with -isystem, the -I option will be "
- "ignored">;
+HelpText<"Add directory to include search path">,
+DocBrief<[{Add directory to include search path. For C++ inputs, if
+there are multiple -I options, these directories are searched
+in the order they are given before the standard system directories
+are searched. If the same directory is in the SYSTEM include search
+paths, for example if also specified with -isystem, the -I option
+will be ignored}]>;
 def L : JoinedOrSeparate<["-"], "L">, Flags<[RenderJoined]>, Group,
 MetaVarName<"">, HelpText<"Add directory to library search path">;
 def MD : Flag<["-"], "MD">, Group,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -1016,7 +1016,12 @@
 
 .. option:: -I, --include-directory , --include-directory=
 
-Add directory to include search path. If there are multiple -I options, these directories are searched in the order they are given before the standard system directories are searched. If the same directory is in the SYSTEM include search paths, for example if also specified with -isystem, the -I option will be ignored
+Add directory to include search path. For C++ input, if
+there are multiple -I options, these directories are searched
+in the order they are given before the standard system directories
+are searched. If the same directory is 

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for looking into this feature!

I do have to ask a fairly naive question though - what's it useful for, beyond 
protocol completeness? The obvious alternative to a user is looking at a 
function's implementation, which is an extra context switch but also provides a 
lot more information. I'm having trouble understanding when this could be 
better enough to justify much extra complexity/resources.

(Adding a new method to the index interface seems like a fairly substantial 
thing, and 5-10% to index memory usage is substantial as you say).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93829

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

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

Can you rebase onto `main` and re-upload the diff? It will trigger CI. If the 
CI passes, this is good to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Seems pretty straightforward and clean.

The cleanup of the report's message should be reworked.
Besides, that looks good to me.

I think these cases should be tested as well:

- [Warning, Warning, Warning]
- [Warning, Note, Note]
- [Warning, Note, Note, Warning, Note]
- [Warning, Note, Remark, Warning, Remark]

PS: clang-format; sorry for the nit-spam :D




Comment at: 
clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h:53-56
+  /// Inform the consumer that the last diagnostic has been sent. This is
+  /// necessary because the consumer does not know whether more notes are
+  /// going to be attached to the last warning.
+  void finalize() { flushPartialDiagnostic(); }

Overriding the `virtual void clang::DiagnosticConsumer::finish()` can't 
accomplish the same thing?



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:1
+//===--- PathDiagnosticConverterDiagnosticConsumer.cpp --*- C++ 
-*-===//
+//

I've seen this a few times, and I still don't know why we use it.
The file extension should be enough to recognize that this is a C++ file.
Can someone clarify this?



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:14
+
+#include 
+

I frequently see `#include "clang/..."`-ish includes but never `#include 
`.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:23
+
+  for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+Consumer->HandlePathDiagnostic(std::move(PartialPDs[Consumer]));





Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:37
+  Info.FormatDiagnostic(Msg);
+  Msg[0] = toupper(Msg[0]);
+  std::string CleanMsg = Msg.str().substr(0, Msg.str().find('[') - 1).str();





Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:38
+  Msg[0] = toupper(Msg[0]);
+  std::string CleanMsg = Msg.str().substr(0, Msg.str().find('[') - 1).str();
+

vsavchenko wrote:
> I think this type of stuff should be covered by a comment or a descriptive 
> function name.
Eh, I don't think it's gonna work this way.
We can not assume that the `[` won't appear in the payload of the message.
Eg.:
`NewDelete-checker-test.cpp:193`
```
// newdelete-warning{{Argument to 'delete[]' is the address of the local 
variable 'i', which is not memory allocated by 'new[]'}}
```

The best you could do is to do a reverse search.
Do we emit the `[mypackage.mychecker]` suffix for all the reports? If not, then 
we have a problem.



Comment at: 
clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:45-52
+  // For now we assume that every diagnostic is either a warning (or something
+  // similar, like an error) or a note logically attached to a previous 
warning.
+  // Notes do not come in actually attached to their respective warnings
+  // but are fed to us independently. The good news is they always follow their
+  // respective warnings and come in in the right order so we can
+  // automatically re-attach them.
+  // FIXME: Handle other stuff, like remarks(?)

So, `Error` and `Fatal`is treated in the same way as `Warning`.
What about the `Ignored`?



Comment at: 
clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:54-60
+  PathDiagnosticPieceRef Piece;
+  if (ShouldDisplayNotesAsEvents) {
+Piece = std::make_shared(PDLoc, Msg);
+  } else {
+Piece = std::make_shared(PDLoc, Msg);
+  }
+  PartialPDs[Consumer]->getMutablePieces().push_back(Piece);

The ternary operator could simplify this inside the `push_back` function - 
`emplace_back`?



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:71
+  BTI.CheckName, /*DeclWithIssue=*/nullptr, BTI.BugType,
+  CleanMsg, CleanMsg, BTI.BugCategory, PDLoc, /*DeclToUnique=*/nullptr,
+  std::make_unique());

Should Desc and ShortDesc be the same?



Comment at: 
clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:74-77
+  PathDiagnosticPieceRef Piece =
+  std::make_shared(PDLoc, CleanMsg);
+
+  PD->setEndOfPath(std::move(Piece));

Why not construct in place?



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:44
+
+  virtual StringRef getName() const override { return "test"; }
+

I would suggest removing the redundant `virtual`. The same applies to the other 
decls.



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:63
+  size_t PieceI = 0;
+  for (const auto &Piece: Pieces) {
+const ExpectedPieceTy &ExpectedPiece = Expect

[clang] 3f7b4ce - [PowerPC] Add support for embedded devices with EFPU2

[Nemanja Ivanovic via cfe-commits]

Author: Nemanja Ivanovic
Date: 2021-01-12T09:47:00-06:00
New Revision: 3f7b4ce96065eea66bf4344973173e76ec1a4255

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

LOG: [PowerPC] Add support for embedded devices with EFPU2

PowerPC cores like e200z759n3 [1] using an efpu2 only support single precision
hardware floating point instructions. The single precision instructions efs*
and evfs* are identical to the spe float instructions while efd* and evfd*
instructions trigger a not implemented exception.

This patch introduces a new command line option -mefpu2 which leads to
single-hardware / double-software code generation.

[1] Core reference:
  https://www.nxp.com/files-static/32bit/doc/ref_manual/e200z759CRM.pdf

Differential revision: https://reviews.llvm.org/D92935

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/PPC.cpp
clang/test/Driver/ppc-features.cpp
llvm/lib/Target/PowerPC/PPC.td
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCSubtarget.cpp
llvm/lib/Target/PowerPC/PPCSubtarget.h
llvm/test/CodeGen/PowerPC/spe.ll

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index b46008970f57..ac97f6fed935 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -3145,6 +3145,8 @@ PowerPC
 
 .. option:: -mdirect-move, -mno-direct-move
 
+.. option:: -mefpu2
+
 .. option:: -mfloat128, -mno-float128
 
 .. option:: -mfprnd, -mno-fprnd

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 35643701f97e..d9586e086a9c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3040,6 +3040,7 @@ def mpcrel: Flag<["-"], "mpcrel">, 
Group;
 def mno_pcrel: Flag<["-"], "mno-pcrel">, Group;
 def mspe : Flag<["-"], "mspe">, Group;
 def mno_spe : Flag<["-"], "mno-spe">, Group;
+def mefpu2 : Flag<["-"], "mefpu2">, Group;
 def mabi_EQ_vec_extabi : Flag<["-"], "mabi=vec-extabi">, Group, 
Flags<[CC1Option]>,
   HelpText<"Enable the extended Altivec ABI on AIX (AIX only). Uses volatile 
and nonvolatile vector registers">;
 def mabi_EQ_vec_default : Flag<["-"], "mabi=vec-default">, Group, 
Flags<[CC1Option]>,

diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 2be7555102f8..cfede6e6e756 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -56,7 +56,7 @@ bool 
PPCTargetInfo::handleTargetFeatures(std::vector &Features,
   HasP10Vector = true;
 } else if (Feature == "+pcrelative-memops") {
   HasPCRelativeMemops = true;
-} else if (Feature == "+spe") {
+} else if (Feature == "+spe" || Feature == "+efpu2") {
   HasSPE = true;
   LongDoubleWidth = LongDoubleAlign = 64;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();
@@ -402,6 +402,8 @@ bool PPCTargetInfo::hasFeature(StringRef Feature) const {
 void PPCTargetInfo::setFeatureEnabled(llvm::StringMap &Features,
   StringRef Name, bool Enabled) const {
   if (Enabled) {
+if (Name == "efpu2")
+  Features["spe"] = true;
 // If we're enabling any of the vsx based features then enable vsx and
 // altivec. We'll diagnose any problems later.
 bool FeatureHasVSX = llvm::StringSwitch(Name)
@@ -425,6 +427,8 @@ void PPCTargetInfo::setFeatureEnabled(llvm::StringMap 
&Features,
 else
   Features[Name] = true;
   } else {
+if (Name == "spe")
+  Features["efpu2"] = false;
 // If we're disabling altivec or vsx go ahead and disable all of the vsx
 // features.
 if ((Name == "altivec") || (Name == "vsx"))

diff  --git a/clang/test/Driver/ppc-features.cpp 
b/clang/test/Driver/ppc-features.cpp
index 85060951aa16..def96c351b34 100644
--- a/clang/test/Driver/ppc-features.cpp
+++ b/clang/test/Driver/ppc-features.cpp
@@ -155,6 +155,9 @@
 // CHECK-SPE: "-target-feature" "+spe"
 // CHECK-NOSPE: "-target-feature" "-spe"
 
+// RUN: %clang -target powerpc %s -mefpu2 -c -### 2>&1 | FileCheck 
-check-prefix=CHECK-EFPU2 %s
+// CHECK-EFPU2: "-target-feature" "+efpu2"
+
 // Assembler features
 // RUN: %clang -target powerpc-unknown-linux-gnu %s -### -o %t.o 
-no-integrated-as 2>&1 | FileCheck -check-prefix=CHECK_32_BE_AS_ARGS %s
 // CHECK_32_BE_AS_ARGS: "-mppc"

diff  --git a/llvm/lib/Target/PowerPC/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index 2975ae161aaa..06403f5e55a2 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -72,6 +72,9 @@ def FeatureAltivec   : 
SubtargetFeature<"altivec","HasAltivec", "true",
 def FeatureSPE   : SubtargetFeature<"spe","HasSPE"

[PATCH] D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)

[Nemanja Ivanovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f7b4ce96065: [PowerPC] Add support for embedded devices 
with EFPU2 (authored by nemanjai).

Changed prior to commit:
  https://reviews.llvm.org/D92935?vs=315119&id=316093#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92935

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Driver/ppc-features.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/test/CodeGen/PowerPC/spe.ll

Index: llvm/test/CodeGen/PowerPC/spe.ll
===
--- llvm/test/CodeGen/PowerPC/spe.ll
+++ llvm/test/CodeGen/PowerPC/spe.ll
@@ -1,6 +1,18 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \
-; RUN:  -mattr=+spe |  FileCheck %s
+; RUN: split-file %s %t
+; RUN: llc -verify-machineinstrs < %t/single.ll -mtriple=powerpc-unknown-linux-gnu \
+; RUN:  -mattr=+spe |  FileCheck %t/single.ll
+; RUN: llc -verify-machineinstrs < %t/double.ll -mtriple=powerpc-unknown-linux-gnu \
+; RUN:  -mattr=+spe |  FileCheck %t/double.ll -check-prefix=SPE
+; RUN: llc -verify-machineinstrs < %t/hwdouble.ll -mtriple=powerpc-unknown-linux-gnu \
+; RUN:  -mattr=+spe |  FileCheck %t/hwdouble.ll -check-prefix=SPE
+; RUN: llc -verify-machineinstrs < %t/single.ll -mtriple=powerpc-unknown-linux-gnu \
+; RUN:  -mattr=+efpu2 |  FileCheck %t/single.ll
+; RUN: llc -verify-machineinstrs < %t/double.ll -mtriple=powerpc-unknown-linux-gnu \
+; RUN:  -mattr=+efpu2 |  FileCheck %t/double.ll -check-prefix=EFPU2
+
+;--- single.ll
+; single tests (identical for -mattr=+spe and -mattr=+efpu2)
 
 declare float @llvm.fabs.float(float)
 define float @test_float_abs(float %a) #0 {
@@ -24,7 +36,7 @@
 ret float %sub
 }
 
-define float @test_fdiv(float %a, float %b) {
+define float @test_fdiv(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fdiv:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:efsdiv 3, 3, 4
@@ -35,7 +47,7 @@
 
 }
 
-define float @test_fmul(float %a, float %b) {
+define float @test_fmul(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fmul:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:efsmul 3, 3, 4
@@ -45,7 +57,7 @@
   ret float %v
 }
 
-define float @test_fadd(float %a, float %b) {
+define float @test_fadd(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fadd:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:efsadd 3, 3, 4
@@ -55,7 +67,7 @@
   ret float %v
 }
 
-define float @test_fsub(float %a, float %b) {
+define float @test_fsub(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fsub:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:efssub 3, 3, 4
@@ -65,7 +77,7 @@
   ret float %v
 }
 
-define float @test_fneg(float %a) {
+define float @test_fneg(float %a) #0 {
 ; CHECK-LABEL: test_fneg:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:efsneg 3, 3
@@ -75,30 +87,18 @@
   ret float %v
 }
 
-define float @test_dtos(double %a) {
-; CHECK-LABEL: test_dtos:
-; CHECK:   # %bb.0: # %entry
-; CHECK-NEXT:evmergelo 3, 3, 4
-; CHECK-NEXT:efscfd 3, 3
-; CHECK-NEXT:blr
-  entry:
-  %v = fptrunc double %a to float
-  ret float %v
-}
-
-define i32 @test_fcmpgt(float %a, float %b) {
+define i32 @test_fcmpgt(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fcmpgt:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:stwu 1, -16(1)
-; CHECK-NEXT:.cfi_def_cfa_offset 16
 ; CHECK-NEXT:efscmpgt 0, 3, 4
-; CHECK-NEXT:ble 0, .LBB8_2
+; CHECK-NEXT:ble 0, .LBB7_2
 ; CHECK-NEXT:  # %bb.1: # %tr
 ; CHECK-NEXT:li 3, 1
-; CHECK-NEXT:b .LBB8_3
-; CHECK-NEXT:  .LBB8_2: # %fa
+; CHECK-NEXT:b .LBB7_3
+; CHECK-NEXT:  .LBB7_2: # %fa
 ; CHECK-NEXT:li 3, 0
-; CHECK-NEXT:  .LBB8_3: # %ret
+; CHECK-NEXT:  .LBB7_3: # %ret
 ; CHECK-NEXT:stw 3, 12(1)
 ; CHECK-NEXT:lwz 3, 12(1)
 ; CHECK-NEXT:addi 1, 1, 16
@@ -118,25 +118,24 @@
   ret i32 %0
 }
 
-define i32 @test_fcmpugt(float %a, float %b) {
+define i32 @test_fcmpugt(float %a, float %b) #0 {
 ; CHECK-LABEL: test_fcmpugt:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:stwu 1, -16(1)
-; CHECK-NEXT:.cfi_def_cfa_offset 16
 ; CHECK-NEXT:efscmpeq 0, 4, 4
-; CHECK-NEXT:bc 4, 1, .LBB9_4
+; CHECK-NEXT:bc 4, 1, .LBB8_4
 ; CHECK-NEXT:  # %bb.1: # %entry
 ; CHECK-NEXT:efscmpeq 0, 3, 3
-; CHECK-NEXT:bc 4, 1, .LBB9_4
+; CHECK-NEXT:bc 4, 1, .LBB8_4
 ; CHECK-NEXT:  # %bb.2: # %entry
 ; CHECK-NEXT:efscmpgt 0, 3, 4
-; CHECK-NEXT:bc 12, 1, .LBB9_4
+; CHECK-NEXT:bc 12, 1, .LBB8_4
 ; CHECK-NEXT:  # %bb.3: # %fa
 ; CHECK-NEXT:li 3, 0
-; CHECK-NEXT:b .LBB9_5
-; CHECK-NEXT:  .L

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h:38
+
+  std::map>
+  PartialPDs;

A pointer pair might be small enough for a DenseMap?



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:20
+void PathDiagnosticConverterDiagnosticConsumer::flushPartialDiagnostic() {
+  if (PartialPDs.empty())
+return;

Do we need this early return? We might get the same behavior by simply omitting 
this check. I have no strong preference about keeping or removing it.


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

https://reviews.llvm.org/D94476

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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

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

This commit removes the old way of handling Whitesmiths mode in favor of just 
setting the
levels during parsing and letting the formatter handle it from there. It 
requires a bit of
special-casing during the parsing, but ends up a bit cleaner than before. It 
also removes
some of switch/case unit tests that don't really make much sense when dealing 
with
Whitesmiths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94500

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnalyzer.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13525,7 +13525,7 @@
   /*
   verifyFormat("class A;\n"
"namespace B\n"
-   "  {\n"
+   "{\n"
"class C;\n"
"// Comment\n"
"class D\n"
@@ -13539,12 +13539,12 @@
"F\n"
"}\n"
"  };\n"
-   "  } // namespace B\n",
+   "} // namespace B\n",
WhitesmithsBraceStyle);
   */
 
   verifyFormat("namespace a\n"
-   "  {\n"
+   "{\n"
"class A\n"
"  {\n"
"  void f()\n"
@@ -13564,7 +13564,7 @@
"  {\n"
"  int x;\n"
"  };\n"
-   "  } // namespace a",
+   "} // namespace a",
WhitesmithsBraceStyle);
 
   verifyFormat("void f()\n"
@@ -13597,11 +13597,11 @@
"  do\n"
"{\n"
"c();\n"
-   "} while (false)\n"
+   "} while (false);\n"
+   "  return;\n"
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13609,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13619,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13627,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,65 +13642,12 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
"  {\n"
"  foo(1);\n"
"  }\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  WhitesmithsBraceStyle.IndentCaseBlocks = false;
-
-  verifyFormat("void switchTest4(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 2:\n"
-   "{\n"
-   "}\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  verifyFormat("void switchTest5(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 0:\n"
-   "break;\n"
-   "  case 1:\n"
-   "{\n"
-   "foo();\n"
-   "break;\n"
-   "}\n"
-   "  case 2:\n"
-   "{\n"
-   "}\n"
-   "break;\n"
-   "  default:\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  verifyFormat("void switchTest6(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 0:\n"
-   "{\n"
-   "foo(x);\n"
-   "}\n"
-   "break;\n"
-   "  default:\n"
-   "{\n"
-   "foo(1

[PATCH] D94424: [clangd] Make AST-based signals available to runWithPreamble.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:858
+std::string RelatedNS = OS.str();
+if (!RelatedNS.empty()) {
+  Signals.RelatedNamespaces[RelatedNS + "::"] += 1;

nit: I think you can just use OS.str() here and below instead of RelatedNS to 
avoid extra string copy.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:716
+  };
+  } // namespace bar
+  )cpp";

bar or tar?



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:720
+  #include "foo.h"
+  namespace ns1 {
+  namespace ns2 {

Can you add anonymous namespaces as well?



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:733
+  S.update(Foo, getInputs(Foo, Contents), WantDiagnostics::Yes);
+  // Wait for the preamble is being built.
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));

s/for/while?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94424

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

[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

can you document this in ClangOffloadBundler.rst ? I think we need a clear 
description about how clang-offload-bundler knows which file in the .a file 
belongs to which target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525

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

[PATCH] D94503: [clangd] Avoid having the same file in the queue twice in the background index. (For future reference. We decided not to do this for now in favor of simply not re-indexing the same CD

[Sam McCall via Phabricator via cfe-commits]

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

...updated and in practice it's often a net negative)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94503

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundQueue.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -9,6 +9,7 @@
 #include "index/BackgroundRebuild.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"
@@ -829,6 +830,30 @@
   }
 }
 
+TEST(BackgroundQueueTest, Duplicates) {
+  std::string Sequence;
+  BackgroundQueue::Task A([&] { Sequence.push_back('A'); });
+  A.QueuePri = 100;
+  A.Key = 1;
+  BackgroundQueue::Task B([&] { Sequence.push_back('B'); });
+  // B has no key, and is not subject to duplicate detection.
+  B.QueuePri = 50;
+
+  BackgroundQueue Q;
+  Q.append({A, B, A, B}); // One A is dropped, the other is high priority.
+  Q.work(/*OnIdle=*/[&] {
+// The first time we go idle, we enqueue the same task again.
+if (!llvm::is_contained(Sequence, ' ')) {
+  Sequence.push_back(' ');
+  Q.append({A, B, A, B}); // One A is dropped, the other is zero priority.
+} else {
+  Q.stop();
+}
+  });
+
+  EXPECT_EQ("ABB BBA", Sequence);
+}
+
 TEST(BackgroundQueueTest, Progress) {
   using testing::AnyOf;
   BackgroundQueue::Stats S;
Index: clang-tools-extra/clangd/index/BackgroundQueue.cpp
===
--- clang-tools-extra/clangd/index/BackgroundQueue.cpp
+++ clang-tools-extra/clangd/index/BackgroundQueue.cpp
@@ -33,6 +33,8 @@
   std::pop_heap(Queue.begin(), Queue.end());
   Task = std::move(Queue.back());
   Queue.pop_back();
+  if (Task->Key)
+ActiveKeys.erase(Task->Key);
   notifyProgress();
 }
 
@@ -72,10 +74,23 @@
   CV.notify_all();
 }
 
+// Tweaks the priority of a newly-enqueued task, or returns false to cancel it.
+bool BackgroundQueue::adjust(Task &T) {
+  if (T.Key) {
+if (!ActiveKeys.insert(T.Key).second)
+  return false;
+if (!SeenKeys.insert(T.Key).second)
+  T.QueuePri = 0;
+  }
+  T.QueuePri = std::max(T.QueuePri, Boosts.lookup(T.Tag));
+  return true;
+}
+
 void BackgroundQueue::push(Task T) {
   {
 std::lock_guard Lock(Mu);
-T.QueuePri = std::max(T.QueuePri, Boosts.lookup(T.Tag));
+if (!adjust(T))
+  return;
 Queue.push_back(std::move(T));
 std::push_heap(Queue.begin(), Queue.end());
 ++Stat.Enqueued;
@@ -87,11 +102,13 @@
 void BackgroundQueue::append(std::vector Tasks) {
   {
 std::lock_guard Lock(Mu);
-for (Task &T : Tasks)
-  T.QueuePri = std::max(T.QueuePri, Boosts.lookup(T.Tag));
-std::move(Tasks.begin(), Tasks.end(), std::back_inserter(Queue));
+for (Task &T : Tasks) {
+  if (!adjust(T))
+continue;
+  Queue.push_back(std::move(T));
+  ++Stat.Enqueued;
+}
 std::make_heap(Queue.begin(), Queue.end());
-Stat.Enqueued += Tasks.size();
 notifyProgress();
   }
   CV.notify_all();
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -76,6 +76,9 @@
 llvm::ThreadPriority ThreadPri = llvm::ThreadPriority::Background;
 unsigned QueuePri = 0; // Higher-priority tasks will run first.
 std::string Tag;   // Allows priority to be boosted later.
+uint64_t Key = 0;  // If the key matches a previous task:
+   //  - in the queue ==> new task is dropped
+   //  - completed==> new task gets priority 0
 
 bool operator<(const Task &O) const { return QueuePri < O.QueuePri; }
   };
@@ -114,6 +117,7 @@
 
 private:
   void notifyProgress() const; // Requires lock Mu
+  bool adjust(Task &T);
 
   std::mutex Mu;
   Stats Stat;
@@ -122,6 +126,8 @@
   std::vector Queue; // max-heap
   llvm::StringMap Boosts;
   std::function OnProgress;
+  llvm::DenseSet ActiveKeys;
+  llvm::DenseSet SeenKeys;
 };
 
 // Builds an in-memory index by by running the static indexer action over
@@ -213,6 +219,7 @@
 
   // from lowest to highest priority
   enum QueuePriority {
+ReindexFile = 0, // Implicitly applied by BackgroundQueue
 IndexFile,
 IndexBoostedFile,
 LoadShard

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 316115.
tbaeder added a comment.

Okay, I've added a test and made sure it fails before and succeeds after this 
patch.


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

https://reviews.llvm.org/D93375

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/ansi.c


Index: clang/test/Driver/ansi.c
===
--- /dev/null
+++ clang/test/Driver/ansi.c
@@ -0,0 +1,19 @@
+// Check that -std=c89 and -ansi are both not forwarded to the linker
+// RUN: %clang -std=c89 %s -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK 
%s
+// CHECK: -std=c89
+// CHECK-NOT: -std=c89
+
+// Same check with -ansi (which is an alias for -std=c89)
+// RUN: %clang -ansi %s -### -o %t.o 2>&1 | FileCheck --check-prefix=ANSI %s
+// ANSI: -std=c89
+// ANSI-NOT: -ansi
+
+// Passing -std to linking and compiling is both valid
+// RUN: %clang -std=c89 %s -c -o %t1.o && %clang %t1.o -Werror -std=c89 -o %t1
+
+// Same for -ansi
+// RUN: %clang -ansi %s -c -o %t2.o && %clang %t2.o -Werror -ansi -o %t1
+
+int main(int argc, char **argv)
+{
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -800,7 +800,7 @@
 def Z_Joined : Joined<["-"], "Z">;
 def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
-def ansi : Flag<["-", "--"], "ansi">;
+def ansi : Flag<["-", "--"], "ansi">, Group;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
 def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
 def arch__only : Separate<["-"], "arch_only">;


Index: clang/test/Driver/ansi.c
===
--- /dev/null
+++ clang/test/Driver/ansi.c
@@ -0,0 +1,19 @@
+// Check that -std=c89 and -ansi are both not forwarded to the linker
+// RUN: %clang -std=c89 %s -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK %s
+// CHECK: -std=c89
+// CHECK-NOT: -std=c89
+
+// Same check with -ansi (which is an alias for -std=c89)
+// RUN: %clang -ansi %s -### -o %t.o 2>&1 | FileCheck --check-prefix=ANSI %s
+// ANSI: -std=c89
+// ANSI-NOT: -ansi
+
+// Passing -std to linking and compiling is both valid
+// RUN: %clang -std=c89 %s -c -o %t1.o && %clang %t1.o -Werror -std=c89 -o %t1
+
+// Same for -ansi
+// RUN: %clang -ansi %s -c -o %t2.o && %clang %t2.o -Werror -ansi -o %t1
+
+int main(int argc, char **argv)
+{
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -800,7 +800,7 @@
 def Z_Joined : Joined<["-"], "Z">;
 def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
-def ansi : Flag<["-", "--"], "ansi">;
+def ansi : Flag<["-", "--"], "ansi">, Group;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
 def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
 def arch__only : Separate<["-"], "arch_only">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 316118.

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

https://reviews.llvm.org/D93375

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/ansi.c


Index: clang/test/Driver/ansi.c
===
--- /dev/null
+++ clang/test/Driver/ansi.c
@@ -0,0 +1,19 @@
+// Check that -std=c89 and -ansi are both not forwarded to the linker
+// RUN: %clang -std=c89 %s -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK 
%s
+// CHECK: -std=c89
+// CHECK-NOT: -std=c89
+
+// Same check with -ansi (which is an alias for -std=c89)
+// RUN: %clang -ansi %s -### -o %t.o 2>&1 | FileCheck --check-prefix=ANSI %s
+// ANSI: -std=c89
+// ANSI-NOT: -ansi
+
+// Passing -std to linking and compiling is both valid
+// RUN: %clang -std=c89 %s -c -o %t1.o && %clang %t1.o -Werror -std=c89 -o %t1
+
+// Same for -ansi
+// RUN: %clang -ansi %s -c -o %t2.o && %clang %t2.o -Werror -ansi -o %t2
+
+int main(int argc, char **argv)
+{
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -800,7 +800,7 @@
 def Z_Joined : Joined<["-"], "Z">;
 def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
-def ansi : Flag<["-", "--"], "ansi">;
+def ansi : Flag<["-", "--"], "ansi">, Group;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
 def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
 def arch__only : Separate<["-"], "arch_only">;


Index: clang/test/Driver/ansi.c
===
--- /dev/null
+++ clang/test/Driver/ansi.c
@@ -0,0 +1,19 @@
+// Check that -std=c89 and -ansi are both not forwarded to the linker
+// RUN: %clang -std=c89 %s -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK %s
+// CHECK: -std=c89
+// CHECK-NOT: -std=c89
+
+// Same check with -ansi (which is an alias for -std=c89)
+// RUN: %clang -ansi %s -### -o %t.o 2>&1 | FileCheck --check-prefix=ANSI %s
+// ANSI: -std=c89
+// ANSI-NOT: -ansi
+
+// Passing -std to linking and compiling is both valid
+// RUN: %clang -std=c89 %s -c -o %t1.o && %clang %t1.o -Werror -std=c89 -o %t1
+
+// Same for -ansi
+// RUN: %clang -ansi %s -c -o %t2.o && %clang %t2.o -Werror -ansi -o %t2
+
+int main(int argc, char **argv)
+{
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -800,7 +800,7 @@
 def Z_Joined : Joined<["-"], "Z">;
 def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
-def ansi : Flag<["-", "--"], "ansi">;
+def ansi : Flag<["-", "--"], "ansi">, Group;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
 def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
 def arch__only : Separate<["-"], "arch_only">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93223: [RFC][analyzer] Create MacroExpansionContext member in AnalysisConsumer and pass down to the diagnostics consumers

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 316126.
steakhal marked 2 inline comments as done.
steakhal added a comment.

Updates:

- New the construction of `MacroExpansionContext` won't hook the `Preprocessor` 
in the constructor. Hooking is done via the `registerForPreprocessor(PP)` 
member function.
- Hooking the PP now depends on the `ShouldDisplayMacroExpansions` analyzer 
option.
- Both `getExpandedText` and `getOriginalText` returns `Optional`. 
Semantics and comments changed accordingly. If the 
`ShouldDisplayMacroExpansions` analyzer flag is not set, both of these 
functions always return `None`.
- Removed the accidental `createHTMLSingleFileDiagnosticConsumer` prototype in 
`PathDiagnostic.h`.
- New unittest case added `NoneForNonExpansionLocations`.
- The rest of the tests were uplifted to unwrap the Optional to accommodate the 
new API.
- The last assertion of the `RedefUndef` unittest case was changed to match the 
new behavior for non-expansion locations.


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

https://reviews.llvm.org/D93223

Files:
  clang/include/clang/Analysis/MacroExpansionContext.h
  clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
  clang/lib/Analysis/MacroExpansionContext.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp

Index: clang/unittests/Analysis/MacroExpansionContextTest.cpp
===
--- clang/unittests/Analysis/MacroExpansionContextTest.cpp
+++ clang/unittests/Analysis/MacroExpansionContextTest.cpp
@@ -67,7 +67,8 @@
 /*OwnsHeaderSearch =*/false);
 
 PP.Initialize(*Target);
-auto Ctx = std::make_unique(PP, LangOpts);
+auto Ctx = std::make_unique(LangOpts);
+Ctx->registerForPreprocessor(PP);
 
 // Lex source text.
 PP.EnterMainSourceFile();
@@ -91,6 +92,46 @@
   }
 };
 
+TEST_F(MacroExpansionContextTest, NoneForNonExpansionLocations) {
+  const auto Ctx = getMacroExpansionContextFor(R"code(
+  #define EMPTY
+  A b cd EMPTY ef EMPTY gh
+EMPTY zz
+  )code");
+  // After preprocessing:
+  //  A b cd ef gh
+  //  zz
+
+  // That's the beginning of the definition of EMPTY.
+  EXPECT_FALSE(Ctx->getExpandedText(at(2, 11)).hasValue());
+  EXPECT_FALSE(Ctx->getOriginalText(at(2, 11)).hasValue());
+
+  // The space before the first expansion of EMPTY.
+  EXPECT_FALSE(Ctx->getExpandedText(at(3, 9)).hasValue());
+  EXPECT_FALSE(Ctx->getOriginalText(at(3, 9)).hasValue());
+
+  // The beginning of the first expansion of EMPTY.
+  EXPECT_TRUE(Ctx->getExpandedText(at(3, 10)).hasValue());
+  EXPECT_TRUE(Ctx->getOriginalText(at(3, 10)).hasValue());
+
+  // Pointing inside of the token EMPTY, but not at the beginning.
+  // FIXME: We only deal with begin locations.
+  EXPECT_FALSE(Ctx->getExpandedText(at(3, 11)).hasValue());
+  EXPECT_FALSE(Ctx->getOriginalText(at(3, 11)).hasValue());
+
+  // Same here.
+  EXPECT_FALSE(Ctx->getExpandedText(at(3, 12)).hasValue());
+  EXPECT_FALSE(Ctx->getOriginalText(at(3, 12)).hasValue());
+
+  // The beginning of the last expansion of EMPTY.
+  EXPECT_TRUE(Ctx->getExpandedText(at(4, 1)).hasValue());
+  EXPECT_TRUE(Ctx->getOriginalText(at(4, 1)).hasValue());
+
+  // Same as for the 3:11 case.
+  EXPECT_FALSE(Ctx->getExpandedText(at(4, 2)).hasValue());
+  EXPECT_FALSE(Ctx->getOriginalText(at(4, 2)).hasValue());
+}
+
 TEST_F(MacroExpansionContextTest, EmptyExpansions) {
   const auto Ctx = getMacroExpansionContextFor(R"code(
   #define EMPTY
@@ -101,14 +142,14 @@
   //  A b cd ef gh
   //  zz
 
-  EXPECT_EQ("", Ctx->getExpandedText(at(3, 10)));
-  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(3, 10)));
+  EXPECT_EQ("", Ctx->getExpandedText(at(3, 10)).getValue());
+  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(3, 10)).getValue());
 
-  EXPECT_EQ("", Ctx->getExpandedText(at(3, 19)));
-  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(3, 19)));
+  EXPECT_EQ("", Ctx->getExpandedText(at(3, 19)).getValue());
+  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(3, 19)).getValue());
 
-  EXPECT_EQ("", Ctx->getExpandedText(at(4, 1)));
-  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(4, 1)));
+  EXPECT_EQ("", Ctx->getExpandedText(at(4, 1)).getValue());
+  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(4, 1)).getValue());
 }
 
 TEST_F(MacroExpansionContextTest, TransitiveExpansions) {
@@ -120,11 +161,10 @@
   // After preprocessing:
   //  A b cd ) 1 ef gh
 
-  EXPECT_EQ(")1", Ctx->getExpandedText(at(4, 10)));
-  EXPECT_EQ("WOOF", Ctx->getOriginalText(at(4, 10)));
+  EXPECT_EQ("WOOF", Ctx->getOriginalText(at(4, 10)).getValue());
 
-  EXPECT_EQ("", Ctx->getExpandedText(at(4, 18)));
-  EXPECT_EQ("EMPTY", Ctx->getOriginalText(at(4, 18)));
+  EXPECT_EQ("", Ctx->getExpandedText

[PATCH] D94513: [clangd] Remove "decision-forest-base" experimental flag.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

The value of this flag can only be fine tuned by using A/B testing on large
user base.
We do not expect individual users to use and fine tune this flag.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94513

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -194,14 +194,6 @@
 Hidden,
 };
 
-opt DecisionForestBase{
-"decision-forest-base",
-cat(Features),
-desc("Base for exponentiating the prediction from DecisionForest."),
-init(CodeCompleteOptions().DecisionForestBase),
-Hidden,
-};
-
 // FIXME: also support "plain" style where signatures are always omitted.
 enum CompletionStyleFlag { Detailed, Bundled };
 opt CompletionStyle{
@@ -841,7 +833,6 @@
   Opts.CodeComplete.AllScopes = AllScopesCompletion;
   Opts.CodeComplete.RunParser = CodeCompletionParse;
   Opts.CodeComplete.RankingModel = RankingModel;
-  Opts.CodeComplete.DecisionForestBase = DecisionForestBase;
 
   RealThreadsafeFS TFS;
   std::vector> ProviderStack;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -194,14 +194,6 @@
 Hidden,
 };
 
-opt DecisionForestBase{
-"decision-forest-base",
-cat(Features),
-desc("Base for exponentiating the prediction from DecisionForest."),
-init(CodeCompleteOptions().DecisionForestBase),
-Hidden,
-};
-
 // FIXME: also support "plain" style where signatures are always omitted.
 enum CompletionStyleFlag { Detailed, Bundled };
 opt CompletionStyle{
@@ -841,7 +833,6 @@
   Opts.CodeComplete.AllScopes = AllScopesCompletion;
   Opts.CodeComplete.RunParser = CodeCompletionParse;
   Opts.CodeComplete.RankingModel = RankingModel;
-  Opts.CodeComplete.DecisionForestBase = DecisionForestBase;
 
   RealThreadsafeFS TFS;
   std::vector> ProviderStack;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Updates:

- Rebased.

---

Unfortunately, I could not come up with a proper CTU implementation.
It seems that when we load the AST/dump, no preprocessor events are replayed.
Without those events, my `PPCallbacks` implementation and tokenwatcher would 
not record anything, drawing macro expansion useless for CTU.

How should I continue to get this working with CTU?
@xazax.hun @martong @balazske 
Previously we had some sort of macro expansions (and crashes), after these 
patches we would not anymore :(




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:20-21
 #include "clang/Basic/Version.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Lex/Preprocessor.h"

NoQ wrote:
> steakhal wrote:
> > NoQ wrote:
> > > Will these two includes be ultimately removed? Like, even in the CTU case?
> > I think `PlistDiagnostics` can depend on `CTU`.
> > `ASTUnit` is a different thing though. I think I just accidentally left 
> > that include.
> > 
> > I will remove the `#include "clang/Frontend/ASTUnit.h"` line.
> > 
> > ---
> > What do you mean by the 'CTU case'?
> > I think PlistDiagnostics can depend on CTU.
> 
> So, like, my problem is that i want to move all `PathDiagnosticConsumer`s 
> (with implementations) into `libAnalysis` so that other tools could use it 
> but `libAnalysis` can't link-time-depend on `libCrossTU` because that'd be a 
> circular dependency. Basically `libAnalysis` is a tiny library that contains 
> analyses whereas `libCrossTU` is a high-level library that manages entire 
> clang invocations. Unless we rethink our library hierarchies entirely, i 
> believe we're stuck with this constraint.
> 
> Removing the dependency on `libFrontend` is insufficient because `libCrossTU` 
> depends on `libFrontend` anyway. 
> 
> If you make `MacroExpansionContext` abstract and put the concrete 
> implementation in `libCrossTU`, thus replacing my (currently reverted) 
> attempt in D92432 to break the dependency, that'd fix the problem entirely. 
> Otherwise i'll probably wait for your work to land and do this myself anyway.
> 
> > What do you mean by the 'CTU case'?
> 
> Nothing really (: It was an attempt to emotionally highlight the importance 
> of not having a link-time dependency on `libCrossTU` even while handling the 
> CTU execution path, according to whatever definition of that you were 
> alluding to in the review summary.
After you land D92432, these two includes could be substituted by 
`clang/Analysis/CrossTUAnalysisHelper.h`.


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

https://reviews.llvm.org/D93224

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

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for this!


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

https://reviews.llvm.org/D93375

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

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

And thank you for reviewing my patches :)


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

https://reviews.llvm.org/D93375

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

[clang] dd95577 - Fix typo in diagnostic message

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2021-01-12T09:58:11-08:00
New Revision: dd955771240289fbcba5fa1312cb8c78f20cd78f

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

LOG: Fix typo in diagnostic message

rdar://66684531

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 19b003398b02..717bf6e12ccd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3263,7 +3263,7 @@ def warn_attribute_dllexport_explicit_instantiation_def : 
Warning<
   "'dllexport' attribute ignored on explicit instantiation definition">,
   InGroup;
 def warn_invalid_initializer_from_system_header : Warning<
-  "invalid constructor form class in system header, should not be explicit">,
+  "invalid constructor from class in system header, should not be explicit">,
   InGroup>;
 def note_used_in_initialization_here : Note<"used in initialization here">;
 def err_attribute_dll_member_of_dll_class : Error<



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

[PATCH] D92797: APINotes: add initial stub of APINotesWriter

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Ping x 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92797

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

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D93224#2493434 , @steakhal wrote:

> How should I continue to get this working with CTU?

We have two CTU modes. One loading the dump and the other loading from AST 
source file. I assume that you refer to the former? Could you validate that 
this solution works for the latter?
If we cannot make CTU fully functional, is it possible to at least make it work 
for the current TU (as opposed to functions inlined from another TU)?

While it would be great to have a fully working solution, not having macro 
expansions in a single relatively narrow scenario might worth the trade-off to 
get rid of the related bugs and throw out a half-baked reimplementation of the 
preprocessor.

If we really want to make it work for the CTU mode with dumps, we could dump 
the macro expansions to a separate file together with the AST and read it back 
for the error reporting.


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

https://reviews.llvm.org/D93224

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

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D93375#2493480 , @tbaeder wrote:

> And thank you for reviewing my patches :)

Happy to help! Btw, do you need someone to commit them on your behalf?


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

https://reviews.llvm.org/D93375

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

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Yes, I was gonna ask somebody else but if you have time, committing this one 
and https://reviews.llvm.org/D94478 would be nice


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

https://reviews.llvm.org/D93375

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

[PATCH] D94503: [clangd] Avoid having the same file in the queue twice in the background index.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 316152.
sammccall retitled this revision from "[clangd] Avoid having the same file in 
the queue twice in the background index.

(For future reference. We decided not to do this for now in favor of simply not
re-indexing the same CDB multiple times, as too much infrastructure needs to 
be..." to "[clangd] Avoid having the same file in the queue twice in the 
background index.".
sammccall edited the summary of this revision.
sammccall added a comment.

In fact, never index the same file twice at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94503

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundQueue.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -9,6 +9,7 @@
 #include "index/BackgroundRebuild.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"
@@ -666,25 +667,45 @@
   // Make sure we only store the Cmd for main file.
   EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
 
-  {
-tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
-EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
-EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
-  }
+  tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+  EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+  EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
+}
 
-  // FIXME: Changing compile commands should be enough to invalidate the cache.
-  FS.Files[testPath("A.cc")] = " ";
-  Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
-  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+TEST_F(BackgroundIndexTest, Reindex) {
+  MockFS FS;
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+  /*Opts=*/{});
+
+  // Index a file.
+  FS.Files[testPath("A.cc")] = "int theOldFunction();";
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = "../A.cc";
+  Cmd.Directory = testPath("build");
+  Cmd.CommandLine = {"clang++", "../A.cc", "-fsyntax-only"};
+  CDB.setCompileCommand(testPath("A.cc"), Cmd);
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
 
-  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
+  // Verify the result is indexed and stored.
+  EXPECT_EQ(1u, runFuzzyFind(Idx, "theOldFunction").size());
+  EXPECT_EQ(0u, runFuzzyFind(Idx, "theNewFunction").size());
+  std::string OldShard = Storage.lookup(testPath("A.cc"));
+  EXPECT_NE("", OldShard);
 
-  {
-tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
-EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
-EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
-  }
+  // Change the content and command, and notify to reindex it.
+  Cmd.CommandLine.push_back("-DFOO");
+  FS.Files[testPath("A.cc")] = "int theNewFunction();";
+  CDB.setCompileCommand(testPath("A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  // Currently, we will never index the same main file again.
+  EXPECT_EQ(1u, runFuzzyFind(Idx, "theOldFunction").size());
+  EXPECT_EQ(0u, runFuzzyFind(Idx, "theNewFunction").size());
+  EXPECT_EQ(OldShard, Storage.lookup(testPath("A.cc")));
 }
 
 class BackgroundIndexRebuilderTest : public testing::Test {
@@ -829,6 +850,31 @@
   }
 }
 
+TEST(BackgroundQueueTest, Duplicates) {
+  std::string Sequence;
+  BackgroundQueue::Task A([&] { Sequence.push_back('A'); });
+  A.QueuePri = 100;
+  A.Key = 1;
+  BackgroundQueue::Task B([&] { Sequence.push_back('B'); });
+  // B has no key, and is not subject to duplicate detection.
+  B.QueuePri = 50;
+
+  BackgroundQueue Q;
+  Q.append({A, B, A, B}); // One A is dropped, the other is high priority.
+  Q.work(/*OnIdle=*/[&] {
+// The first time we go idle, we enqueue the same task again.
+if (!llvm::is_contained(Sequence, ' ')) {
+  Sequence.push_back(' ');
+  Q.append({A, B, A, B}); // One A is dropped, the other is zero priority.
+} else {
+  Q.stop();
+}
+  });
+
+  // This could reasonably be "ABB BBA", if we had good *re*indexing support.
+  EXPECT_EQ("ABB BB", Sequence);
+}
+
 TEST(BackgroundQueueTest, Progress) {
   using testing::AnyOf;
   BackgroundQueue::Stats S;
Index: clang-tools-extra/clangd/index/BackgroundQueue.cpp
==

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I think that, as with incoming calls, the incremental value is in seeing the 
tree at a glance. So, you might query the outgoing calls for a function, expand 
the tree, and look over the transitive callees to see if the function ends up 
performing a certain type of operation (say, a filesystem access).

That said, I will admit that in ~10 years of using Eclipse CDT, which supports 
both incoming calls and outgoing calls, I can't recall using outgoing calls 
even once (whereas I did use incoming calls). Of course, other users' 
experiences may vary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93829

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

[clang] 3484715 - Add -ansi option to CompileOnly group

[Aaron Ballman via cfe-commits]

Author: Timm Bäder
Date: 2021-01-12T13:16:49-05:00
New Revision: 348471575d9c24bbfb124ca5eac1589de075da88

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

LOG: Add -ansi option to CompileOnly group

-ansi is documented as being the "same as -std=c89", but there are
differences when passing it to a link.

Adding -ansi to said group makes sense since it's supposed to be an
alias for -std=c89 and resolves this inconsistency.

Added: 


Modified: 
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d9586e086a9c..b441c1b4c169 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -800,7 +800,7 @@ def Z_Flag : Flag<["-"], "Z">, Group;
 def Z_Joined : Joined<["-"], "Z">;
 def all__load : Flag<["-"], "all_load">;
 def allowable__client : Separate<["-"], "allowable_client">;
-def ansi : Flag<["-", "--"], "ansi">;
+def ansi : Flag<["-", "--"], "ansi">, Group;
 def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
 def arch : Separate<["-"], "arch">, Flags<[NoXarchOption]>;
 def arch__only : Separate<["-"], "arch_only">;



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

[PATCH] D93375: [clang][driver] Add -ansi option to CompileOnly group

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit on your behalf in 348471575d9c24bbfb124ca5eac1589de075da88 
, thank 
you for the patch!


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

https://reviews.llvm.org/D93375

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

[PATCH] D94453: [libTooling] Add function to Transformer for creating a diagnostic-only rule.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 316158.
ymandel added a comment.

Discarded API changes, reduced to new clang-tidy lib test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

Files:
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,8 +10,10 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -25,20 +27,21 @@
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
+using transformer::noopEdit;
 using transformer::RewriteRule;
+using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule =
-  makeRule(ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
-  hasElse(stmt().bind(E))),
-   change(statement(std::string(RewriteRule::RootID)),
-  cat("if(!(", node(std::string(C)), ")) ",
-  statement(std::string(E)), " else ",
-  statement(std::string(T,
-   cat("negate condition and reverse `then` and `else` branches"));
+  RewriteRule Rule = makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  change(statement(RootID), cat("if(!(", node(std::string(C)), ")) ",
+statement(std::string(E)), " else ",
+statement(std::string(T,
+  cat("negate condition and reverse `then` and `else` branches"));
   return Rule;
 }
 
@@ -68,6 +71,25 @@
   EXPECT_EQ(Expected, test::runCheckOnCode(Input));
 }
 
+TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
+  class DiagOnlyCheck : public TransformerClangTidyCheck {
+  public:
+DiagOnlyCheck(StringRef Name, ClangTidyContext *Context)
+: TransformerClangTidyCheck(
+  makeRule(returnStmt(), noopEdit(node(RootID)), cat("message")),
+  Name, Context) {}
+  };
+  std::string Input = "int h() { return 5; }";
+  std::vector Errors;
+  EXPECT_EQ(Input, test::runCheckOnCode(Input, &Errors));
+  EXPECT_EQ(Errors.size(), 1U);
+  EXPECT_EQ(Errors[0].Message.Message, "message");
+  EXPECT_THAT(Errors[0].Ranges, testing::IsEmpty());
+
+  // The diagnostic is anchored to the match, "return 5".
+  EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
+}
+
 class IntLitCheck : public TransformerClangTidyCheck {
 public:
   IntLitCheck(StringRef Name, ClangTidyContext *Context)


Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,8 +10,10 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -25,20 +27,21 @@
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
+using transformer::noopEdit;
 using transformer::RewriteRule;
+using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule =
-  makeRule(ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
-  hasElse(stmt().bind(E))),
-   change(statement(std::string(RewriteRule::RootID)),
-  cat("if(!(", node(std::string(C)), ")) ",
-  statement(std::string(E)), " else ",
-  statement(std::string(T,
-   cat("negate condition and reverse `then` and `else` branches"));
+  RewriteRule Rule = makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  change(statement(RootID), cat("if(!(", node(

[clang] ef3800e - Return false from __has_declspec_attribute() if not explicitly enabled

[Aaron Ballman via cfe-commits]

Author: Timm Bäder
Date: 2021-01-12T13:20:08-05:00
New Revision: ef3800e82169c674219501d9ac09ef12b28e6359

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

LOG: Return false from __has_declspec_attribute() if not explicitly enabled

Currently, projects can check for __has_declspec_attribute() and use
it accordingly, but the check for __has_declspec_attribute will return
true even if declspec attributes are not enabled for the target.

This changes Clang to instead return false when declspec attributes are
not supported for the target.

Added: 


Modified: 
clang/lib/Lex/PPMacroExpansion.cpp

Removed: 




diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index 3969630f2002..43d31d6c5732 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1693,8 +1693,14 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
   [this](Token &Tok, bool &HasLexedNextToken) -> int {
 IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
diag::err_feature_check_malformed);
-return II ? hasAttribute(AttrSyntax::Declspec, nullptr, II,
- getTargetInfo(), getLangOpts()) : 0;
+if (II) {
+  const LangOptions &LangOpts = getLangOpts();
+  return LangOpts.DeclSpecKeyword &&
+ hasAttribute(AttrSyntax::Declspec, nullptr, II,
+  getTargetInfo(), LangOpts);
+}
+
+return false;
   });
   } else if (II == Ident__has_cpp_attribute ||
  II == Ident__has_c_attribute) {



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

[PATCH] D94488: [clang][cli] Port more CodeGenOptions to marshalling infrastructure

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM, with one nit. It'd be nice to add more detail to the commit message as 
well.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1146
-  Opts.SSPBufferSize =
-  getLastArgIntValue(Args, OPT_stack_protector_buffer_size, 8, Diags);
-

jansvoboda11 wrote:
> This piece of code should've been removed in D84669 which added marshalling 
> info to `stack_protector_buffer_size`.
In that case please commit this separately. Since I presume it's NFC and pretty 
straightforward, likely you don't need a separate review, but please make sure 
the commit message is good / refers to what made it NFC / etc..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94488

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

[PATCH] D94453: [clang-tidy][NFC] Add test for Transformer-based checks with diagnostics.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Reworked, per our discussion. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

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

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:199-203
   void generateCC1CommandLine(llvm::SmallVectorImpl &Args,
   StringAllocator SA) const;
+  void generateCC1CommandLine(llvm::SmallVectorImpl &Args,
+  StringAllocator SA,
+  ArrayRef Included) 
const;

You can avoid the duplication at least for the WIP with:
```
void generateCC1CommandLine(llvm::SmallVectorImpl &Args,
  StringAllocator SA,
  ArrayRef Included = 
None) const;
```
and then in the body:
```
auto IsIncluded = Included.empty()
? [](OptSpecifier) { return true; }
: [&Included](OptSpecifier Opt) { return Included.count(Opt); };

// ...
if (... && IsIncluded(Opt)) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94474

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

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 316161.
smeenai added a comment.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

Files:
  libcxx/docs/DesignDocs/VisibilityMacros.rst
  libcxx/include/__config


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -715,7 +715,7 @@
 #endif
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
-#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && 
__has_attribute(__type_visibility__)
+#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
 #define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __attribute__ 
((__visibility__("default")))
 #  else
 #define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
Index: libcxx/docs/DesignDocs/VisibilityMacros.rst
===
--- libcxx/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/docs/DesignDocs/VisibilityMacros.rst
@@ -131,12 +131,6 @@
   specified on the primary template and to export the member functions produced
   by the explicit instantiation in the dylib.
 
-  **GCC Behavior**: GCC ignores visibility attributes applied the type in
-  extern template declarations and applying an attribute results in a warning.
-  However since `_LIBCPP_TEMPLATE_VIS` is the same as
-  `__attribute__((visibility("default"))` the visibility is already correct.
-  The macro has an empty definition with GCC.
-
   **Windows Behavior**: `extern template` and `dllexport` are fundamentally
   incompatible *on a class template* on Windows; the former suppresses
   instantiation, while the latter forces it. Specifying both on the same


Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -715,7 +715,7 @@
 #endif
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
-#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && __has_attribute(__type_visibility__)
+#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
 #define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __attribute__ ((__visibility__("default")))
 #  else
 #define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
Index: libcxx/docs/DesignDocs/VisibilityMacros.rst
===
--- libcxx/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/docs/DesignDocs/VisibilityMacros.rst
@@ -131,12 +131,6 @@
   specified on the primary template and to export the member functions produced
   by the explicit instantiation in the dylib.
 
-  **GCC Behavior**: GCC ignores visibility attributes applied the type in
-  extern template declarations and applying an attribute results in a warning.
-  However since `_LIBCPP_TEMPLATE_VIS` is the same as
-  `__attribute__((visibility("default"))` the visibility is already correct.
-  The macro has an empty definition with GCC.
-
   **Windows Behavior**: `extern template` and `dllexport` are fundamentally
   incompatible *on a class template* on Windows; the former suppresses
   instantiation, while the latter forces it. Specifying both on the same
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

It's been a long time since I've contributed to libc++, and the pre-merge CI 
setup is a massive improvement over what we had before. A huge kudos to 
everyone who made it possible :)

This is still showing up as Needs Review. I'm not sure if that's because of 
@EricWF's prior "Needs Revision" or because it needs to be explicitly accepted 
as the libc++ group (or some combination of the two).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

@aaron.ballman thank you for taking a look!
@NoQ thank you for commenting.

Indeed, i very much don't want to diagnose just the cases
where the overflow can be proven to happen, doing that
i believe, would remove most of the true-positive reports.

So indeed, this is beginning to look like a coding guideline,
so let's circumvent all this FUD, and move onto clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I'm wondering if we can get this in incrementally without needing to list in 
code which options are correctly handled. Here's one way to do it:

- Add a tool, `clang-cc1-args`, that shows the diff between round-tripping when 
given a set of args. (This could potentially grow other functionality vs. 
diffing. So maybe `clang-cc1-args diff [args]` would be the usage?)
- Add `RUN` lines to tests that test the args we think should round-trip 
correctly and confirm there's no diff. This can be done incrementally as 
options get handled.
- Only once we think we have all the options covered, add the assertion from 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94474

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

[PATCH] D94472: [clang][cli] Manually generate header search arguments

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

It'd be much nicer to add the tests as part of the patch. I've added a comment 
to D94474  with a possible way to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

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

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4009
+// mangling. Previously, it used a special-cased nonstandard extension.
+if (Context.getASTContext().getLangOpts().getClangABICompat() >=
+LangOptions::ClangABI::Ver11) {

Please forgive my ignorance, but when left unspecified, what does the Clang ABI 
compatibility flag default to? I'm sure you've thought about it already, but 
I'm trying to understand whether that is an ABI break for people compiling 
without specifying the Clang ABI compatibility version (which is most users).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93922

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

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D93224#2493515 , @xazax.hun wrote:

> In D93224#2493434 , @steakhal wrote:
>
>> How should I continue to get this working with CTU?
>
> We have two CTU modes. One loading the dump and the other loading from AST 
> source file. I assume that you refer to the former?

Yes, I was referring to that.

> Could you validate that this solution works for the latter?

Sure, I suspect that will have the required PP events since the Preprocessor is 
parsing source code :D
I'm going to check this just to be sure.

> If we cannot make CTU fully functional, is it possible to at least make it 
> work for the current TU (as opposed to functions inlined from another TU)?

This element of the patch stack implements exactly that.

> While it would be great to have a fully working solution, not having macro 
> expansions in a single relatively narrow scenario might worth the trade-off 
> to get rid of the related bugs and throw out a half-baked reimplementation of 
> the preprocessor.

Even in the CTU case, the main TU is still parsed via the main Preprocessor - 
hence we will track the macro expansions accordingly.
I would definitely favor this 'option' instead of dropping this patch stack.
I mean, it really fixes a bunch of crashes and expands macros much more 
accurately especially for the corner-cases.

> If we really want to make it work for the CTU mode with dumps, we could dump 
> the macro expansions to a separate file together with the AST and read it 
> back for the error reporting.

If we chose this path, the local `getExpandedMacro` function needs to be 
simplified. That would help Artem's dependency issue.


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

https://reviews.llvm.org/D93224

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

[PATCH] D94533: [clang] Add AddClang.cmake to the list of the installed CMake modules

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski created this revision.
Herald added subscribers: rriddle, mgorny.
awarzynski requested review of this revision.
Herald added subscribers: cfe-commits, stephenneuendorffer.
Herald added a project: clang.

This makes sure that AddClang.cmake is installed alongside other Clang
CMake modules. This mirrors LLVM and MLIR in this respect and is
required when building the new Flang driver out of tree (as it depends
on Clang and includes AddClang.cmake).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94533

Files:
  clang/cmake/modules/CMakeLists.txt


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -61,6 +61,7 @@
 
   install(FILES
 ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/ClangConfig.cmake
+${CMAKE_CURRENT_SOURCE_DIR}/AddClang.cmake
 DESTINATION ${CLANG_INSTALL_PACKAGE_DIR}
 COMPONENT clang-cmake-exports)
 


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -61,6 +61,7 @@
 
   install(FILES
 ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/ClangConfig.cmake
+${CMAKE_CURRENT_SOURCE_DIR}/AddClang.cmake
 DESTINATION ${CLANG_INSTALL_PACKAGE_DIR}
 COMPONENT clang-cmake-exports)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

[Tony Tye via Phabricator via cfe-commits]

t-tye added a comment.

In D93525#2493024 , @yaxunl wrote:

> can you document this in ClangOffloadBundler.rst ? I think we need a clear 
> description about how clang-offload-bundler knows which file in the .a file 
> belongs to which target.

How does the .a relate to bundled code objects? Does the .a have a number of 
bundled code objects? If so wouldn't the identity of code objects be defined by 
the existing bundled code object ABI already documented? If the .a is a set of 
non-bundled code objects then defining how they are identified is not part of 
the clang-offload-bundler documentation as there are no bundled code objects 
involved. It would seem that the documentation belongs with the OpenMP 
runtime/compiler that is choosing to use .a files in this manner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525

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

[clang-tools-extra] 4718ec0 - [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-01-12T13:57:54-05:00
New Revision: 4718ec01669b01373180f4cd1256c6e2dd6f3999

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

LOG: [clangd] Avoid recursion in TargetFinder::add()

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

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 9a502a84e36f..84316659daad 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@ struct TargetFinder {
   llvm::SmallDenseMap>
   Decls;
+  llvm::SmallDenseMap Seen;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -359,6 +360,15 @@ struct TargetFinder {
 if (!D)
   return;
 debug(*D, Flags);
+
+// Avoid recursion (which can arise in the presence of heuristic
+// resolution of dependent names) by exiting early if we have
+// already seen this decl with all flags in Flags.
+auto Res = Seen.try_emplace(D);
+if (!Res.second && Res.first->second.contains(Flags))
+  return;
+Res.first->second |= Flags;
+
 if (const UsingDirectiveDecl *UDD = llvm::dyn_cast(D))
   D = UDD->getNominatedNamespaceAsWritten();
 

diff  --git a/clang-tools-extra/clangd/FindTarget.h 
b/clang-tools-extra/clangd/FindTarget.h
index 435e4f4ac038..92e4354d1eaa 100644
--- a/clang-tools-extra/clangd/FindTarget.h
+++ b/clang-tools-extra/clangd/FindTarget.h
@@ -194,6 +194,9 @@ class DeclRelationSet {
 S &= Other.S;
 return *this;
   }
+  bool contains(DeclRelationSet Other) const {
+return (S & Other.S) == Other.S;
+  }
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, DeclRelationSet);
 };
 // The above operators can't be looked up if both sides are enums.

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index dd7e9878a6d5..46e17dc053c0 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@ TEST_F(TargetDeclTest, DependentTypes) {
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias | Rel::Underlying});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(



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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4718ec01669b: [clangd] Avoid recursion in 
TargetFinder::add() (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias | Rel::Underlying});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.h
===
--- clang-tools-extra/clangd/FindTarget.h
+++ clang-tools-extra/clangd/FindTarget.h
@@ -194,6 +194,9 @@
 S &= Other.S;
 return *this;
   }
+  bool contains(DeclRelationSet Other) const {
+return (S & Other.S) == Other.S;
+  }
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, DeclRelationSet);
 };
 // The above operators can't be looked up if both sides are enums.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@
   llvm::SmallDenseMap>
   Decls;
+  llvm::SmallDenseMap Seen;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -359,6 +360,15 @@
 if (!D)
   return;
 debug(*D, Flags);
+
+// Avoid recursion (which can arise in the presence of heuristic
+// resolution of dependent names) by exiting early if we have
+// already seen this decl with all flags in Flags.
+auto Res = Seen.try_emplace(D);
+if (!Res.second && Res.first->second.contains(Flags))
+  return;
+Res.first->second |= Flags;
+
 if (const UsingDirectiveDecl *UDD = llvm::dyn_cast(D))
   D = UDD->getNominatedNamespaceAsWritten();
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias | Rel::Underlying});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.h
===
--- clang-tools-extra/clangd/FindTarget.h
+++ clang-tool

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

[Wei Mi via Phabricator via cfe-commits]

wmi added inline comments.



Comment at: llvm/include/llvm/IR/PseudoProbe.h:41
   //  [18:3] - probe id
-  //  [25:19] - reserved
+  //  [25:19] - probe distribution factor
   //  [28:26] - probe type, see PseudoProbeType

The bits in discriminator is a scare resource. Have you considered using less 
bits to represent probe distribution factor? I guess it is possible that using 
a little more coarse grain distribution factor won't affect performance.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:273
   IRChangedPrinter PrintChangedIR;
+  PseudoProbeVerifier PseudoProbeVerification;
   VerifyInstrumentation Verify;

Before PseudoProbeUpdate pass, there is no need to verify because 
PseudoProbeUpdate will make distribution factor consistent. PseudoProbeUpdate 
run in a late stage in the lto/thinlto prelink pipeline, and no many passes 
need the verification, so what is the major usage of PseudoProbeVerifier?  



Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:133-134
+  float PrevProbeFactor = PrevProbeFactors[I.first];
+  if (std::abs(CurProbeFactor - PrevProbeFactor) >
+  DistributionFactorVariance) {
+if (!BannerPrinted) {

Why not issue warning/error message when verification fails? That will make 
enabling the verification in release compiler possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93264

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

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Louis Dionne via Phabricator via cfe-commits]

ldionne removed a reviewer: EricWF.
ldionne added a subscriber: EricWF.
ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

In D35388#2493696 , @smeenai wrote:

> It's been a long time since I've contributed to libc++, and the pre-merge CI 
> setup is a massive improvement over what we had before. A huge kudos to 
> everyone who made it possible :)

I'm glad this is helpful! It's a huge time saver for me so far.

> This is still showing up as Needs Review. I'm not sure if that's because of 
> @EricWF's prior "Needs Revision" or because it needs to be explicitly 
> accepted as the libc++ group (or some combination of the two).

It must be because of Eric's "Needs Revision", because I accepted as libc++. 
I'll remove Eric as a reviewer to fix this (Eric, if you see this, it's not 
against you!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

[PATCH] D94537: [IR] move nomerge attribute from function declaration/definition to callsites

[Zequan Wu via Phabricator via cfe-commits]

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

Move nomerge attribute from function declaration/definition to callsites to
allow virtual function calls attach the attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94537

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-nomerge.cpp

Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -3,7 +3,7 @@
 class A {
 public:
   [[clang::nomerge]] A();
-  [[clang::nomerge]] ~A();
+  [[clang::nomerge]] virtual ~A();
   [[clang::nomerge]] void f();
   [[clang::nomerge]] virtual void g();
   [[clang::nomerge]] static void f1();
@@ -14,14 +14,14 @@
   void g() override;
 };
 
-[[clang::nomerge]] bool bar();
+bool bar();
 [[clang::nomerge]] void f(bool, bool);
 
 void foo(int i, A *ap, B *bp) {
   [[clang::nomerge]] bar();
   [[clang::nomerge]] (i = 4, bar());
   [[clang::nomerge]] (void)(bar());
-  [[clang::nomerge]] f(bar(), bar());
+  f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
   [[clang::nomerge]] { asm("nop"); }
@@ -37,6 +37,9 @@
 
   B b;
   b.g();
+
+  A *newA = new B();
+  delete newA;
 }
 
 int g(int i);
@@ -57,37 +60,34 @@
   g(1);
 }
 
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0:[0-9]+]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call void @_Z1fbb({{.*}}){{$}}
-// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0:[0-9]+]]
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
+// CHECK: call void @_Z1fbb({{.*}}) #[[ATTR0]]
+// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK-LABEL: for.cond:
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK-LABEL: for.inc:
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call void asm sideeffect "nop"{{.*}} #[[ATTR1:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
 // CHECK: %[[AG:.*]] = load void (%class.A*)*, void (%class.A*)**
-// CHECK-NEXT: call void %[[AG]](%class.A* nonnull dereferenceable
+// CHECK-NEXT: call void %[[AG]](%class.A* {{.*}}) #[[ATTR0]]
 // CHECK: %[[BG:.*]] = load void (%class.B*)*, void (%class.B*)**
 // CHECK-NEXT: call void %[[BG]](%class.B* nonnull dereferenceable
-
-
-// CHECK-DAG: declare zeroext i1 @_Z3barv() #[[ATTR2:[0-9]+]]
-// CHECK-DAG: declare void @_Z1fbb(i1 zeroext, i1 zeroext) #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AC1Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A1fEv{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A1gEv{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A2f1Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AC2Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AD1Ev{{.*}} #[[ATTR3:[0-9]+]]
-// CHECK-DAG: declare void @_ZN1AD2Ev{{.*}} #[[ATTR3]]
-// CHECK-DAG: define{{.*}} i32 @_Z1gi(i32 %i) #[[ATTR4:[0-9]+]] {
+// CHECK: call void @_ZN1AC1Ev({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A1fEv({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A1gEv({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A2f1Ev() #[[ATTR0]]
+// CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
+// CHECK: call void @_ZN1B1gEv({{.*}}){{$}}
+// CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
+// CHECK: %[[AG:.*]] = load void (%class.A*)*, void (%class.A*)**
+// CHECK-NEXT: call void %[[AG]](%class.A* {{.*}}) #[[ATTR1]]
+// CHECK: call void  @_ZN1AD1Ev(%class.A* {{.*}}) #[[ATTR1]]
 
 // CHECK-DAG: attributes #[[ATTR0]] = {{{.*}}nomerge{{.*}}}
 // CHECK-DAG: attributes #[[ATTR1]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR2]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR3]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR4]] = {{{.*}}nomerge{{.*}}}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1762,9 +1762,6 @@
   B.addAttribute(llvm::Attribute::MinSize);
   }
 
-  if (D->hasAttr())
-B.addAttribute(llvm::Attribute::NoMerge);
-
   F->addAttributes(llvm::AttributeList::FunctionIndex, B);
 
   unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen

[PATCH] D94237: [clang] Use SourceLocations in unions [NFCI]

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Do you need to run the destructor before placement new in these situations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94237

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

[PATCH] D94537: [IR] move nomerge attribute from function declaration/definition to callsites

[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, thanks!




Comment at: clang/lib/CodeGen/CGCall.cpp:1973
   }
-  if (!AttrOnCallSite && TargetDecl->hasAttr())
 FuncAttrs.addAttribute(llvm::Attribute::NoMerge);

This is the key change, I think this is worth a comment. Now we only place LLVM 
nomerge attributes on call sites, never functions, and this allows them to work 
on indirect virtual function calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94537

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

[clang] e5f51fd - [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

[David Truby via cfe-commits]

Author: David Truby
Date: 2021-01-12T19:44:01Z
New Revision: e5f51fdd650c6d20c81fedb8e856e9858aa10991

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

LOG: [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

MSVC on WoA64 includes isCXX14Aggregate in its definition. This is de-facto
specification on that platform, so match msvc's behaviour.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=47611

Co-authored-by: Peter Waller 

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

Added: 


Modified: 
clang/lib/CodeGen/CGCXXABI.h
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGenCXX/homogeneous-aggregates.cpp
llvm/test/CodeGen/AArch64/arm64-windows-calls.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 171428a3525d..ea839db7528e 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -146,6 +146,13 @@ class CGCXXABI {
   /// 'this' parameter of C++ instance methods.
   virtual bool isSRetParameterAfterThis() const { return false; }
 
+  /// Returns true if the ABI permits the argument to be a homogeneous
+  /// aggregate.
+  virtual bool
+  isPermittedToBeHomogeneousAggregate(const CXXRecordDecl *RD) const {
+return true;
+  };
+
   /// Find the LLVM type used to represent the given member pointer
   /// type.
   virtual llvm::Type *

diff  --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index c16c72dc93d5..cb0dc1d5d717 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -771,6 +771,9 @@ class MicrosoftCXXABI : public CGCXXABI {
   LoadVTablePtr(CodeGenFunction &CGF, Address This,
 const CXXRecordDecl *RD) override;
 
+  virtual bool
+  isPermittedToBeHomogeneousAggregate(const CXXRecordDecl *RD) const override;
+
 private:
   typedef std::pair VFTableIdTy;
   typedef llvm::DenseMap VTablesMapTy;
@@ -1070,7 +1073,7 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl GD) 
const {
   return isDeletingDtor(GD);
 }
 
-static bool isCXX14Aggregate(const CXXRecordDecl *RD) {
+static bool isTrivialForAArch64MSVC(const CXXRecordDecl *RD) {
   // For AArch64, we use the C++14 definition of an aggregate, so we also
   // check for:
   //   No private or protected non static data members.
@@ -1107,7 +1110,7 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo 
&FI) const {
   bool isTrivialForABI = RD->isPOD();
   bool isAArch64 = CGM.getTarget().getTriple().isAArch64();
   if (isAArch64)
-isTrivialForABI = RD->canPassInRegisters() && isCXX14Aggregate(RD);
+isTrivialForABI = RD->canPassInRegisters() && isTrivialForAArch64MSVC(RD);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
@@ -4358,3 +4361,12 @@ MicrosoftCXXABI::LoadVTablePtr(CodeGenFunction &CGF, 
Address This,
   performBaseAdjustment(CGF, This, QualType(RD->getTypeForDecl(), 0));
   return {CGF.GetVTablePtr(This, CGM.Int8PtrTy, RD), RD};
 }
+
+bool MicrosoftCXXABI::isPermittedToBeHomogeneousAggregate(
+const CXXRecordDecl *CXXRD) const {
+  // MSVC Windows on Arm64 considers a type not HFA if it is not an
+  // aggregate according to the C++14 spec. This is not consistent with the
+  // AAPCS64, but is defacto spec on that platform.
+  return !CGM.getTarget().getTriple().isAArch64() ||
+ isTrivialForAArch64MSVC(CXXRD);
+}

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index d36c7344e284..9a11a0720f3c 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -5065,8 +5065,12 @@ bool ABIInfo::isHomogeneousAggregate(QualType Ty, const 
Type *&Base,
 
 Members = 0;
 
-// If this is a C++ record, check the bases first.
+// If this is a C++ record, check the properties of the record such as
+// bases and ABI specific restrictions
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
+  if (!getCXXABI().isPermittedToBeHomogeneousAggregate(CXXRD))
+return false;
+
   for (const auto &I : CXXRD->bases()) {
 // Ignore empty records.
 if (isEmptyRecord(getContext(), I.getType(), true))

diff  --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp 
b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
index 2b3af4226407..0fa30b2663bf 100644
--- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -mfloat-abi hard -triple armv7-unknown-linux-gnueabi 
-emit-llvm -o - %s | FileCheck %s --check-prefix=ARM32
 // RUN: %clang_cc1 -mfloat-abi hard -triple aarch64-unknown-lin

[PATCH] D92751: [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

[David Truby via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5f51fdd650c: [clang][aarch64] Precondition 
isHomogeneousAggregate on isCXX14Aggregate (authored by DavidTruby).

Changed prior to commit:
  https://reviews.llvm.org/D92751?vs=315788&id=316181#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92751

Files:
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/homogeneous-aggregates.cpp
  llvm/test/CodeGen/AArch64/arm64-windows-calls.ll

Index: llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
===
--- llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
+++ llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
@@ -98,3 +98,80 @@
   %this1 = load %class.C*, %class.C** %this.addr, align 8
   ret void
 }
+
+; The following tests correspond to tests in
+; clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+
+; Pod is a trivial HFA
+%struct.Pod = type { [2 x double] }
+; Not an aggregate according to C++14 spec => not HFA according to MSVC
+%struct.NotCXX14Aggregate  = type { %struct.Pod }
+; NotPod is a C++14 aggregate. But not HFA, because it contains
+; NotCXX14Aggregate (which itself is not HFA because it's not a C++14
+; aggregate).
+%struct.NotPod = type { %struct.NotCXX14Aggregate }
+
+; CHECK-LABEL: copy_pod:
+define dso_local %struct.Pod @copy_pod(%struct.Pod* %x) {
+  %x1 = load %struct.Pod, %struct.Pod* %x, align 8
+  ret %struct.Pod %x1
+  ; CHECK: ldp d0, d1, [x0]
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)
+
+; CHECK-LABEL: copy_notcxx14aggregate:
+define dso_local void
+@copy_notcxx14aggregate(%struct.NotCXX14Aggregate* inreg noalias sret(%struct.NotCXX14Aggregate) align 8 %agg.result,
+%struct.NotCXX14Aggregate* %x) {
+  %1 = bitcast %struct.NotCXX14Aggregate* %agg.result to i8*
+  %2 = bitcast %struct.NotCXX14Aggregate* %x to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %1, i8* align 8 %2, i64 16, i1 false)
+  ret void
+  ; CHECK: str q0, [x0]
+}
+
+; CHECK-LABEL: copy_notpod:
+define dso_local [2 x i64] @copy_notpod(%struct.NotPod* %x) {
+  %x1 = bitcast %struct.NotPod* %x to [2 x i64]*
+  %x2 = load [2 x i64], [2 x i64]* %x1
+  ret [2 x i64] %x2
+  ; CHECK: ldp x8, x1, [x0]
+  ; CHECK: mov x0, x8
+}
+
+@Pod = external global %struct.Pod
+
+; CHECK-LABEL: call_copy_pod:
+define void @call_copy_pod() {
+  %x = call %struct.Pod @copy_pod(%struct.Pod* @Pod)
+  store %struct.Pod %x, %struct.Pod* @Pod
+  ret void
+  ; CHECK: bl copy_pod
+  ; CHECK-NEXT: stp d0, d1, [{{.*}}]
+}
+
+@NotCXX14Aggregate = external global %struct.NotCXX14Aggregate
+
+; CHECK-LABEL: call_copy_notcxx14aggregate:
+define void @call_copy_notcxx14aggregate() {
+  %x = alloca %struct.NotCXX14Aggregate
+  call void @copy_notcxx14aggregate(%struct.NotCXX14Aggregate* %x, %struct.NotCXX14Aggregate* @NotCXX14Aggregate)
+  %x1 = load %struct.NotCXX14Aggregate, %struct.NotCXX14Aggregate* %x
+  store %struct.NotCXX14Aggregate %x1, %struct.NotCXX14Aggregate* @NotCXX14Aggregate
+  ret void
+  ; CHECK: bl copy_notcxx14aggregate
+  ; CHECK-NEXT: ldp {{.*}}, {{.*}}, [sp]
+}
+
+@NotPod = external global %struct.NotPod
+
+; CHECK-LABEL: call_copy_notpod:
+define void @call_copy_notpod() {
+  %x = call [2 x i64] @copy_notpod(%struct.NotPod* @NotPod)
+  %notpod = bitcast %struct.NotPod* @NotPod to [2 x i64]*
+  store [2 x i64] %x, [2 x i64]* %notpod
+  ret void
+  ; CHECK: bl copy_notpod
+  ; CHECK-NEXT: stp x0, x1, [{{.*}}]
+}
Index: clang/test/CodeGenCXX/homogeneous-aggregates.cpp
===
--- clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -mfloat-abi hard -triple armv7-unknown-linux-gnueabi -emit-llvm -o - %s | FileCheck %s --check-prefix=ARM32
 // RUN: %clang_cc1 -mfloat-abi hard -triple aarch64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=ARM64
 // RUN: %clang_cc1 -mfloat-abi hard -triple x86_64-unknown-windows-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 -mfloat-abi hard -triple aarch64-unknown-windows-msvc -emit-llvm -o - %s | FileCheck %s --check-prefix=WOA64
 
 #if defined(__x86_64__)
 #define CC __attribute__((vectorcall))
@@ -104,3 +105,71 @@
 // ARM32: define{{.*}} arm_aapcs_vfpcc void @_Z19with_empty_bitfield20HVAWithEmptyBitField(%struct.HVAWithEmptyBitField %a.coerce)
 // X64: define dso_local x86_vectorcallcc void @"\01_Z19with_empty_bitfield20HVAWithEmptyBitField@@16"(%struct.HVAWithEmptyBitField inreg %a.coerce)
 void CC with_empty_bitfield(HVAWithEmptyBitField a) {}
+
+namespace pr47611 {
+// MSVC on Arm includes "isCXX14Aggregate" as part of its defin

[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: JDevlieghere.
dblaikie added a comment.

In D94391#2491229 , @MaskRay wrote:

> In D94391#2491178 , @dblaikie wrote:
>
>> Any particular bug you're trying to fix? (this looks like it changes the 
>> debug locations, so it would need tests)
>>
>> Not necessarily clear that these are better debug locations?
>
> No particular bug. `CGDebugInfo::getLineNumber` returning CurLoc just looks 
> strange. (The CurLoc may be far below (irrelevant) when getLineNumber is 
> called.)
>
> This patch is hopefully NFC. Dropping `CurLoc` from 
> `getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc)` 
> will cause a difference, which may reveal missing `SourceLocation` 
> information in ObjC constructs.

Ah, OK. Yeah, I don't have strong feelings about this - a bit hesitant to make 
the change without a stronger motivation though. Perhaps other reviewers have 
some thoughts to contribute (also maybe @JDevlieghere ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

[clang] e53bbd9 - [IR] move nomerge attribute from function declaration/definition to callsites

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2021-01-12T12:10:46-08:00
New Revision: e53bbd99516fc7b612df1ae08d48288d0b8784ea

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

LOG: [IR] move nomerge attribute from function declaration/definition to 
callsites

Move nomerge attribute from function declaration/definition to callsites to
allow virtual function calls attach the attribute.

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

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/attr-nomerge.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 2cc7203d1194..42801372189b 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1985,7 +1985,9 @@ void CodeGenModule::ConstructAttributeList(
   FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
 NBA = Fn->getAttr();
   }
-  if (!AttrOnCallSite && TargetDecl->hasAttr())
+  // Only place nomerge attribute on call sites, never functions. This
+  // allows it to work on indirect virtual function calls.
+  if (AttrOnCallSite && TargetDecl->hasAttr())
 FuncAttrs.addAttribute(llvm::Attribute::NoMerge);
 }
 
@@ -5018,13 +5020,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 Attrs.addAttribute(getLLVMContext(), 
llvm::AttributeList::FunctionIndex,
llvm::Attribute::StrictFP);
 
-  // Add nomerge attribute to the call-site if the callee function doesn't have
-  // the attribute.
-  if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl))
-if (!FD->hasAttr() && InNoMergeAttributedStmt)
-  Attrs = Attrs.addAttribute(getLLVMContext(),
- llvm::AttributeList::FunctionIndex,
- llvm::Attribute::NoMerge);
+  // Add call-site nomerge attribute if exists.
+  if (InNoMergeAttributedStmt)
+Attrs =
+Attrs.addAttribute(getLLVMContext(), 
llvm::AttributeList::FunctionIndex,
+   llvm::Attribute::NoMerge);
 
   // Apply some call-site-specific attributes.
   // TODO: work this into building the attribute set.

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index da5b03b138bf..bee51715bdc6 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1772,9 +1772,6 @@ void 
CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   B.addAttribute(llvm::Attribute::MinSize);
   }
 
-  if (D->hasAttr())
-B.addAttribute(llvm::Attribute::NoMerge);
-
   F->addAttributes(llvm::AttributeList::FunctionIndex, B);
 
   unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();

diff  --git a/clang/test/CodeGen/attr-nomerge.cpp 
b/clang/test/CodeGen/attr-nomerge.cpp
index d93f4a7c96d6..fc26af379fdb 100644
--- a/clang/test/CodeGen/attr-nomerge.cpp
+++ b/clang/test/CodeGen/attr-nomerge.cpp
@@ -3,7 +3,7 @@
 class A {
 public:
   [[clang::nomerge]] A();
-  [[clang::nomerge]] ~A();
+  [[clang::nomerge]] virtual ~A();
   [[clang::nomerge]] void f();
   [[clang::nomerge]] virtual void g();
   [[clang::nomerge]] static void f1();
@@ -14,14 +14,14 @@ class B : public A {
   void g() override;
 };
 
-[[clang::nomerge]] bool bar();
+bool bar();
 [[clang::nomerge]] void f(bool, bool);
 
 void foo(int i, A *ap, B *bp) {
   [[clang::nomerge]] bar();
   [[clang::nomerge]] (i = 4, bar());
   [[clang::nomerge]] (void)(bar());
-  [[clang::nomerge]] f(bar(), bar());
+  f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the 
anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
   [[clang::nomerge]] { asm("nop"); }
@@ -37,6 +37,9 @@ void foo(int i, A *ap, B *bp) {
 
   B b;
   b.g();
+
+  A *newA = new B();
+  delete newA;
 }
 
 int g(int i);
@@ -57,37 +60,34 @@ void something_else_again() {
   g(1);
 }
 
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0:[0-9]+]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call void @_Z1fbb({{.*}}){{$}}
-// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0:[0-9]+]]
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
+// CHECK: call void @_Z1fbb({{.*}}) #[[ATTR0]]
+// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK-LABEL: for.cond:
+

[PATCH] D94537: [IR] move nomerge attribute from function declaration/definition to callsites

[Zequan Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
zequanwu marked an inline comment as done.
Closed by commit rGe53bbd99516f: [IR] move nomerge attribute from function 
declaration/definition to callsites (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D94537?vs=316174&id=316194#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94537

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-nomerge.cpp

Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -3,7 +3,7 @@
 class A {
 public:
   [[clang::nomerge]] A();
-  [[clang::nomerge]] ~A();
+  [[clang::nomerge]] virtual ~A();
   [[clang::nomerge]] void f();
   [[clang::nomerge]] virtual void g();
   [[clang::nomerge]] static void f1();
@@ -14,14 +14,14 @@
   void g() override;
 };
 
-[[clang::nomerge]] bool bar();
+bool bar();
 [[clang::nomerge]] void f(bool, bool);
 
 void foo(int i, A *ap, B *bp) {
   [[clang::nomerge]] bar();
   [[clang::nomerge]] (i = 4, bar());
   [[clang::nomerge]] (void)(bar());
-  [[clang::nomerge]] f(bar(), bar());
+  f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
   [[clang::nomerge]] { asm("nop"); }
@@ -37,6 +37,9 @@
 
   B b;
   b.g();
+
+  A *newA = new B();
+  delete newA;
 }
 
 int g(int i);
@@ -57,37 +60,34 @@
   g(1);
 }
 
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0:[0-9]+]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call void @_Z1fbb({{.*}}){{$}}
-// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0:[0-9]+]]
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
-// CHECK: call zeroext i1 @_Z3barv(){{$}}
+// CHECK: call void @_Z1fbb({{.*}}) #[[ATTR0]]
+// CHECK: call void @"_ZZ3fooiP1AP1BENK3$_0clEv"{{.*}} #[[ATTR0]]
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK-LABEL: for.cond:
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
+// CHECK-LABEL: for.inc:
+// CHECK: call zeroext i1 @_Z3barv() #[[ATTR0]]
 // CHECK: call void asm sideeffect "nop"{{.*}} #[[ATTR1:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv(){{$}}
 // CHECK: %[[AG:.*]] = load void (%class.A*)*, void (%class.A*)**
-// CHECK-NEXT: call void %[[AG]](%class.A* nonnull dereferenceable
+// CHECK-NEXT: call void %[[AG]](%class.A* {{.*}}) #[[ATTR0]]
 // CHECK: %[[BG:.*]] = load void (%class.B*)*, void (%class.B*)**
 // CHECK-NEXT: call void %[[BG]](%class.B* nonnull dereferenceable
-
-
-// CHECK-DAG: declare zeroext i1 @_Z3barv() #[[ATTR2:[0-9]+]]
-// CHECK-DAG: declare void @_Z1fbb(i1 zeroext, i1 zeroext) #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AC1Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A1fEv{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A1gEv{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1A2f1Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AC2Ev{{.*}} #[[ATTR2]]
-// CHECK-DAG: declare void @_ZN1AD1Ev{{.*}} #[[ATTR3:[0-9]+]]
-// CHECK-DAG: declare void @_ZN1AD2Ev{{.*}} #[[ATTR3]]
-// CHECK-DAG: define{{.*}} i32 @_Z1gi(i32 %i) #[[ATTR4:[0-9]+]] {
+// CHECK: call void @_ZN1AC1Ev({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A1fEv({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A1gEv({{.*}}) #[[ATTR0]]
+// CHECK: call void @_ZN1A2f1Ev() #[[ATTR0]]
+// CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
+// CHECK: call void @_ZN1B1gEv({{.*}}){{$}}
+// CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
+// CHECK: %[[AG:.*]] = load void (%class.A*)*, void (%class.A*)**
+// CHECK-NEXT: call void %[[AG]](%class.A* {{.*}}) #[[ATTR1]]
+// CHECK: call void  @_ZN1AD1Ev(%class.A* {{.*}}) #[[ATTR1]]
 
 // CHECK-DAG: attributes #[[ATTR0]] = {{{.*}}nomerge{{.*}}}
 // CHECK-DAG: attributes #[[ATTR1]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR2]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR3]] = {{{.*}}nomerge{{.*}}}
-// CHECK-DAG: attributes #[[ATTR4]] = {{{.*}}nomerge{{.*}}}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1772,9 +1772,6 @@
   B.addAttribute(llvm::Attribute::MinSize);
   }
 
-  if (D->hasAttr())
-B.addAttribute(llvm::Attribute::NoMerge);
-
   F->addAttributes(llvm::AttributeList::FunctionIndex, B);
 
   unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();
Index: clang/lib/CodeGen/CGCall.cpp

[clang-tools-extra] 922a5b8 - [clang-tidy] Add test for Transformer-based checks with diagnostics.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-01-12T20:15:22Z
New Revision: 922a5b894114defb5302e514973de8c9cd23af6a

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

LOG: [clang-tidy] Add test for Transformer-based checks with diagnostics.

Adds a test that checks the diagnostic output of the tidy.

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

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index e8df4bb60071..24b6bea98787 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,8 +10,10 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -25,20 +27,21 @@ using transformer::change;
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
+using transformer::noopEdit;
 using transformer::RewriteRule;
+using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule =
-  makeRule(ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
-  hasElse(stmt().bind(E))),
-   change(statement(std::string(RewriteRule::RootID)),
-  cat("if(!(", node(std::string(C)), ")) ",
-  statement(std::string(E)), " else ",
-  statement(std::string(T,
-   cat("negate condition and reverse `then` and `else` branches"));
+  RewriteRule Rule = makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  change(statement(RootID), cat("if(!(", node(std::string(C)), ")) ",
+statement(std::string(E)), " else ",
+statement(std::string(T,
+  cat("negate condition and reverse `then` and `else` branches"));
   return Rule;
 }
 
@@ -68,6 +71,25 @@ TEST(TransformerClangTidyCheckTest, Basic) {
   EXPECT_EQ(Expected, test::runCheckOnCode(Input));
 }
 
+TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
+  class DiagOnlyCheck : public TransformerClangTidyCheck {
+  public:
+DiagOnlyCheck(StringRef Name, ClangTidyContext *Context)
+: TransformerClangTidyCheck(
+  makeRule(returnStmt(), noopEdit(node(RootID)), cat("message")),
+  Name, Context) {}
+  };
+  std::string Input = "int h() { return 5; }";
+  std::vector Errors;
+  EXPECT_EQ(Input, test::runCheckOnCode(Input, &Errors));
+  EXPECT_EQ(Errors.size(), 1U);
+  EXPECT_EQ(Errors[0].Message.Message, "message");
+  EXPECT_THAT(Errors[0].Ranges, testing::IsEmpty());
+
+  // The diagnostic is anchored to the match, "return 5".
+  EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
+}
+
 class IntLitCheck : public TransformerClangTidyCheck {
 public:
   IntLitCheck(StringRef Name, ClangTidyContext *Context)



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

[PATCH] D94453: [clang-tidy][NFC] Add test for Transformer-based checks with diagnostics.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG922a5b894114: [clang-tidy] Add test for Transformer-based 
checks with diagnostics. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

Files:
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,8 +10,10 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -25,20 +27,21 @@
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
+using transformer::noopEdit;
 using transformer::RewriteRule;
+using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule =
-  makeRule(ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
-  hasElse(stmt().bind(E))),
-   change(statement(std::string(RewriteRule::RootID)),
-  cat("if(!(", node(std::string(C)), ")) ",
-  statement(std::string(E)), " else ",
-  statement(std::string(T,
-   cat("negate condition and reverse `then` and `else` branches"));
+  RewriteRule Rule = makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  change(statement(RootID), cat("if(!(", node(std::string(C)), ")) ",
+statement(std::string(E)), " else ",
+statement(std::string(T,
+  cat("negate condition and reverse `then` and `else` branches"));
   return Rule;
 }
 
@@ -68,6 +71,25 @@
   EXPECT_EQ(Expected, test::runCheckOnCode(Input));
 }
 
+TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
+  class DiagOnlyCheck : public TransformerClangTidyCheck {
+  public:
+DiagOnlyCheck(StringRef Name, ClangTidyContext *Context)
+: TransformerClangTidyCheck(
+  makeRule(returnStmt(), noopEdit(node(RootID)), cat("message")),
+  Name, Context) {}
+  };
+  std::string Input = "int h() { return 5; }";
+  std::vector Errors;
+  EXPECT_EQ(Input, test::runCheckOnCode(Input, &Errors));
+  EXPECT_EQ(Errors.size(), 1U);
+  EXPECT_EQ(Errors[0].Message.Message, "message");
+  EXPECT_THAT(Errors[0].Ranges, testing::IsEmpty());
+
+  // The diagnostic is anchored to the match, "return 5".
+  EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
+}
+
 class IntLitCheck : public TransformerClangTidyCheck {
 public:
   IntLitCheck(StringRef Name, ClangTidyContext *Context)


Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,8 +10,10 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -25,20 +27,21 @@
 using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
+using transformer::noopEdit;
 using transformer::RewriteRule;
+using transformer::RootID;
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
 RewriteRule invertIf() {
   StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule =
-  makeRule(ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
-  hasElse(stmt().bind(E))),
-   change(statement(std::string(RewriteRule::RootID)),
-  cat("if(!(", node(std::string(C)), ")) ",
-  statement(std::string(E)), " else ",
-  statement(std::string(T,
-   cat("negate condition and reverse `then` and `else` branches"));
+  RewriteRule Rule = makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(s

[PATCH] D94381: [test] Add Clang side tests for -fdebug-info-for-profiling

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp:14-16
+// RUN: echo > %t.proftext
+// RUN: llvm-profdata merge %t.proftext -o %t.profdata
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager 
-O1 -fprofile-instrument-use-path=%t.profdata -fdebug-info-for-profiling %s -o 
- 2>&1 | FileCheck %s --check-prefix=DISCR

What's this test for? (I'd hesitate to add a test that has to run llvm-profdata 
and use that profile, etc) I guess it's a different codepath that sets up the 
pass pipeline for this case compared to the previous case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.

This patch looks ready to land. Are there any other concerns or feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

[Andrei Lebedev via Phabricator via cfe-commits]

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

Thanks for suggesting `DocBrief`, Richard!

Looks good to me with a nit.




Comment at: clang/include/clang/Driver/Options.td:652
 Flags<[CC1Option,CC1AsOption]>, MetaVarName<"">,
-HelpText<"Add directory to include search path. If there are multiple -I "
- "options, these directories are searched in the order they are "
- "given before the standard system directories are searched. "
- "If the same directory is in the SYSTEM include search paths, for 
"
- "example if also specified with -isystem, the -I option will be "
- "ignored">;
+HelpText<"Add directory to include search path">,
+DocBrief<[{Add directory to include search path. For C++ inputs, if

How about `Add directory to the end of the list of include search paths` or 
something similar? There is an order in which the directories are used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: llvm/include/llvm/IR/PseudoProbe.h:41
   //  [18:3] - probe id
-  //  [25:19] - reserved
+  //  [25:19] - probe distribution factor
   //  [28:26] - probe type, see PseudoProbeType

wmi wrote:
> The bits in discriminator is a scare resource. Have you considered using less 
> bits to represent probe distribution factor? I guess it is possible that 
> using a little more coarse grain distribution factor won't affect performance.
That's a good point. We are using seven bits to represent [0, 100] so that 
integral numbers can be distinguished. Yes, we could use fewer bits to 
represent, say 4 bits to represent only even numbers. We could also not use any 
bits here but instead use the distribution factor of the outer block probes 
when the competition of those bits are high. I can do an experiment to see how 
well that works.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:273
   IRChangedPrinter PrintChangedIR;
+  PseudoProbeVerifier PseudoProbeVerification;
   VerifyInstrumentation Verify;

wmi wrote:
> Before PseudoProbeUpdate pass, there is no need to verify because 
> PseudoProbeUpdate will make distribution factor consistent. PseudoProbeUpdate 
> run in a late stage in the lto/thinlto prelink pipeline, and no many passes 
> need the verification, so what is the major usage of PseudoProbeVerifier?  
Yeah, there's no need to verify intermediate passes. The verifier pass is just 
a handy utility that tracks those passes that do code duplication for 
debugging. Perhaps I should give it a better name like PseudoCloningTracker?



Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:133-134
+  float PrevProbeFactor = PrevProbeFactors[I.first];
+  if (std::abs(CurProbeFactor - PrevProbeFactor) >
+  DistributionFactorVariance) {
+if (!BannerPrinted) {

wmi wrote:
> Why not issue warning/error message when verification fails? That will make 
> enabling the verification in release compiler possible.
The verifier is for debugging only. It doesn't really do any verification. It 
just helps to track code duplication. Sorry for the naming confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93264

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

[PATCH] D94381: [test] Add Clang side tests for -fdebug-info-for-profiling

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp:14-16
+// RUN: echo > %t.proftext
+// RUN: llvm-profdata merge %t.proftext -o %t.profdata
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager 
-O1 -fprofile-instrument-use-path=%t.profdata -fdebug-info-for-profiling %s -o 
- 2>&1 | FileCheck %s --check-prefix=DISCR

dblaikie wrote:
> What's this test for? (I'd hesitate to add a test that has to run 
> llvm-profdata and use that profile, etc) I guess it's a different codepath 
> that sets up the pass pipeline for this case compared to the previous case?
Test that -fdebug-info-for-profiling affects both driver -fprofile-use (CC1 
-fprofile-instrument-use-path) and  driver -fprofile-generate (CC1 
-fprofile-instrument-path). IIUC: generate/use needs to have compatible flags 
so that their CFG is identical.

The profile reading pass will error on an invalid file (e.g. an empty file).

I hesitated a bit for the usage of llvm-profdata, but I searched for 
llvm-profdata and there are 20+ other tests using llvm-profdata so I believe it 
is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D94391#2493978 , @dblaikie wrote:

> In D94391#2491229 , @MaskRay wrote:
>
>> In D94391#2491178 , @dblaikie wrote:
>>
>>> Any particular bug you're trying to fix? (this looks like it changes the 
>>> debug locations, so it would need tests)
>>>
>>> Not necessarily clear that these are better debug locations?
>>
>> No particular bug. `CGDebugInfo::getLineNumber` returning CurLoc just looks 
>> strange. (The CurLoc may be far below (irrelevant) when getLineNumber is 
>> called.)
>>
>> This patch is hopefully NFC. Dropping `CurLoc` from 
>> `getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc)` 
>> will cause a difference, which may reveal missing `SourceLocation` 
>> information in ObjC constructs.
>
> Ah, OK. Yeah, I don't have strong feelings about this - a bit hesitant to 
> make the change without a stronger motivation though. Perhaps other reviewers 
> have some thoughts to contribute (also maybe @JDevlieghere ).

Using `CurLoc` in `getLineNumber` is not ideal. Trying to fix one call site 
reveals a probably unsatisfactory debug location for ObjC `ImplicitParamDecl`.
The location is set to the closing brace. I do not investigate further as I 
know nearly nothing about ObjC...

  --- a/clang/test/CodeGenObjC/debug-info-blocks.m
  +++ b/clang/test/CodeGenObjC/debug-info-blocks.m
  @@ -65,7 +65,7 @@ static void run(void (^block)(void))
 // CHECK-DAG: !DILocalVariable(arg: 2, scope: ![[COPY_SP]], {{.*}}, 
flags: DIFlagArtificial)
 // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[DESTROY_SP]], {{.*}}, 
flags: DIFlagArtificial)
 run(^{
  -  // CHECK-DAG: ![[SELF]] = !DILocalVariable(name: "self", 
scope:{{.*}}, line: [[@LINE+4]],
  +  // CHECK-DAG: ![[SELF]] = !DILocalVariable(name: "self", 
scope:{{.*}}, line: [[@LINE-7]],
 // CHECK-DAG: ![[D]] = !DILocalVariable(name: "d", scope:{{.*}}, 
line: [[@LINE+1]],
 NSMutableDictionary *d = [[NSMutableDictionary alloc] init]; 
 ivar = 42 + (int)[d count];


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

[PATCH] D94381: [test] Add Clang side tests for -fdebug-info-for-profiling

[David Blaikie via Phabricator via cfe-commits]

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

Sounds good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

[clang] f706486 - Fix for crash in __builtin_return_address in template context.

[Sunil Srivastava via cfe-commits]

Author: Sunil Srivastava
Date: 2021-01-12T12:37:18-08:00
New Revision: f706486eaf07020b11f2088274c757e4070fe6d1

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

LOG: Fix for crash in __builtin_return_address in template context.

The check for argument value needs to be guarded by !isValueDependent().

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/builtin-returnaddress.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 15b5934224f0..2d3d36f4adad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1943,7 +1943,8 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
 // -Wframe-address warning if non-zero passed to builtin
 // return/frame address.
 Expr::EvalResult Result;
-if (TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
+if (!TheCall->getArg(0)->isValueDependent() &&
+TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
 Result.Val.getInt() != 0)
   Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
   << ((BuiltinID == Builtin::BI__builtin_return_address)

diff  --git a/clang/test/Sema/builtin-returnaddress.c 
b/clang/test/Sema/builtin-returnaddress.c
index 3ebbdc6048d8..16d2a517ac12 100644
--- a/clang/test/Sema/builtin-returnaddress.c
+++ b/clang/test/Sema/builtin-returnaddress.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wframe-address -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wmost -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wframe-address -verify %s
 
 void* a(unsigned x) {
 return __builtin_return_address(0);
@@ -17,3 +18,14 @@ void* d(unsigned x) {
 return __builtin_frame_address(1); // expected-warning{{calling 
'__builtin_frame_address' with a nonzero argument is unsafe}}
 }
 
+#ifdef __cplusplus
+template void *RA()
+{
+  return __builtin_return_address(N); // expected-warning{{calling 
'__builtin_return_address' with a nonzero argument is unsafe}}
+}
+
+void *foo()
+{
+ return RA<2>(); // expected-note{{in instantiation of function template 
specialization 'RA<2>' requested here}}
+}
+#endif



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

[PATCH] D94438: Fis for Assertion failure on dependent expression.

[Sunil Srivastava via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Sunil_Srivastava marked an inline comment as done.
Closed by commit rGf706486eaf07: Fix for crash in __builtin_return_address in 
template context. (authored by Sunil_Srivastava).

Changed prior to commit:
  https://reviews.llvm.org/D94438?vs=315873&id=316201#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94438

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-returnaddress.c


Index: clang/test/Sema/builtin-returnaddress.c
===
--- clang/test/Sema/builtin-returnaddress.c
+++ clang/test/Sema/builtin-returnaddress.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wframe-address -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wmost -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wframe-address -verify %s
 
 void* a(unsigned x) {
 return __builtin_return_address(0);
@@ -17,3 +18,14 @@
 return __builtin_frame_address(1); // expected-warning{{calling 
'__builtin_frame_address' with a nonzero argument is unsafe}}
 }
 
+#ifdef __cplusplus
+template void *RA()
+{
+  return __builtin_return_address(N); // expected-warning{{calling 
'__builtin_return_address' with a nonzero argument is unsafe}}
+}
+
+void *foo()
+{
+ return RA<2>(); // expected-note{{in instantiation of function template 
specialization 'RA<2>' requested here}}
+}
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1943,7 +1943,8 @@
 // -Wframe-address warning if non-zero passed to builtin
 // return/frame address.
 Expr::EvalResult Result;
-if (TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
+if (!TheCall->getArg(0)->isValueDependent() &&
+TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
 Result.Val.getInt() != 0)
   Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
   << ((BuiltinID == Builtin::BI__builtin_return_address)


Index: clang/test/Sema/builtin-returnaddress.c
===
--- clang/test/Sema/builtin-returnaddress.c
+++ clang/test/Sema/builtin-returnaddress.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wframe-address -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wmost -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wframe-address -verify %s
 
 void* a(unsigned x) {
 return __builtin_return_address(0);
@@ -17,3 +18,14 @@
 return __builtin_frame_address(1); // expected-warning{{calling '__builtin_frame_address' with a nonzero argument is unsafe}}
 }
 
+#ifdef __cplusplus
+template void *RA()
+{
+  return __builtin_return_address(N); // expected-warning{{calling '__builtin_return_address' with a nonzero argument is unsafe}}
+}
+
+void *foo()
+{
+ return RA<2>(); // expected-note{{in instantiation of function template specialization 'RA<2>' requested here}}
+}
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1943,7 +1943,8 @@
 // -Wframe-address warning if non-zero passed to builtin
 // return/frame address.
 Expr::EvalResult Result;
-if (TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
+if (!TheCall->getArg(0)->isValueDependent() &&
+TheCall->getArg(0)->EvaluateAsInt(Result, getASTContext()) &&
 Result.Val.getInt() != 0)
   Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
   << ((BuiltinID == Builtin::BI__builtin_return_address)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thank you for the reviewing @andreil99 !




Comment at: clang/include/clang/Driver/Options.td:652
 Flags<[CC1Option,CC1AsOption]>, MetaVarName<"">,
-HelpText<"Add directory to include search path. If there are multiple -I "
- "options, these directories are searched in the order they are "
- "given before the standard system directories are searched. "
- "If the same directory is in the SYSTEM include search paths, for 
"
- "example if also specified with -isystem, the -I option will be "
- "ignored">;
+HelpText<"Add directory to include search path">,
+DocBrief<[{Add directory to include search path. For C++ inputs, if

andreil99 wrote:
> How about `Add directory to the end of the list of include search paths` or 
> something similar? There is an order in which the directories are used.
I'll update this before merging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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

[clang] 2a49b7c - [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

[via cfe-commits]

Author: modimo
Date: 2021-01-12T13:43:48-08:00
New Revision: 2a49b7c64a33566cf5db1a5b4042d6037ccc7cf5

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

LOG: [Inliner] Change inline remark format and update ReplayInlineAdvisor to 
use it

This change modifies the source location formatting from:
LineNumber.Discriminator
to:
LineNumber:ColumnNumber.Discriminator

The motivation here is to enhance location information for inline replay that 
currently exists for the SampleProfile inliner. This will be leveraged further 
in inline replay for the CGSCC inliner in the related diff.

The ReplayInlineAdvisor is also modified to read the new format and now takes 
into account the callee for greater accuracy.

Testing:
ninja check-llvm

Reviewed By: mtrofin

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

Added: 


Modified: 
clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
clang/test/Frontend/optimization-remark-with-hotness.c
llvm/include/llvm/Analysis/InlineAdvisor.h
llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
llvm/lib/Analysis/InlineAdvisor.cpp
llvm/lib/Analysis/ReplayInlineAdvisor.cpp
llvm/lib/Transforms/IPO/SampleProfile.cpp
llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll
llvm/test/Transforms/SampleProfile/Inputs/inline-replay.txt
llvm/test/Transforms/SampleProfile/inline-replay.ll
llvm/test/Transforms/SampleProfile/remarks-hotness.ll
llvm/test/Transforms/SampleProfile/remarks.ll

Removed: 




diff  --git a/clang/test/Frontend/optimization-remark-with-hotness-new-pm.c 
b/clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
index 76802bfdcdb8..5e4641d92313 100644
--- a/clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
+++ b/clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
@@ -73,7 +73,7 @@ void bar(int x) {
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization 
information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information
-  // expected-remark@+1 {{foo inlined into bar with (cost=always): always 
inline attribute at callsite bar:8 (hotness:}}
+  // expected-remark@+1 {{foo inlined into bar with (cost=always): always 
inline attribute at callsite bar:8:10; (hotness:}}
   sum += foo(x, x - 2);
 }
 

diff  --git a/clang/test/Frontend/optimization-remark-with-hotness.c 
b/clang/test/Frontend/optimization-remark-with-hotness.c
index 96be3524db16..0961e6da11f4 100644
--- a/clang/test/Frontend/optimization-remark-with-hotness.c
+++ b/clang/test/Frontend/optimization-remark-with-hotness.c
@@ -66,7 +66,7 @@ void bar(int x) {
   // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization 
information
   // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information
-  // expected-remark@+1 {{foo inlined into bar with (cost=always): always 
inliner at callsite bar:8 (hotness:}}
+  // expected-remark@+1 {{foo inlined into bar with (cost=always): always 
inliner at callsite bar:8:10; (hotness:}}
   sum += foo(x, x - 2);
 }
 

diff  --git a/llvm/include/llvm/Analysis/InlineAdvisor.h 
b/llvm/include/llvm/Analysis/InlineAdvisor.h
index f051706dca16..2967aa911699 100644
--- a/llvm/include/llvm/Analysis/InlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/InlineAdvisor.h
@@ -121,6 +121,25 @@ class InlineAdvice {
   bool Recorded = false;
 };
 
+class DefaultInlineAdvice : public InlineAdvice {
+public:
+  DefaultInlineAdvice(InlineAdvisor *Advisor, CallBase &CB,
+  Optional OIC, OptimizationRemarkEmitter &ORE,
+  bool EmitRemarks = true)
+  : InlineAdvice(Advisor, CB, ORE, OIC.hasValue()), OriginalCB(&CB),
+OIC(OIC), EmitRemarks(EmitRemarks) {}
+
+private:
+  void recordUnsuccessfulInliningImpl(const InlineResult &Result) override;
+  void recordInliningWithCalleeDeletedImpl() override;
+  void recordInliningImpl() override;
+
+private:
+  CallBase *const OriginalCB;
+  Optional OIC;
+  bool EmitRemarks;
+};
+
 /// Interface for deciding whether to inline a call site or not.
 class InlineAdvisor {
 public:

diff  --git a/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h 
b/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
index 19e9079a20f5..daed84603541 100644
--- a/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
@@ -25,13 +25,14 @@ class OptimizationRemarkEmitter;
 class ReplayInlineAdvisor : public InlineAdvisor {
 public:
   ReplayInlineAdvisor(FunctionAnalysisManager &FAM, LLVMContext &Context,
-  StringRef RemarksFile);
+  StringRef RemarksFile, bool EmitRe

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

[Di Mo via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a49b7c64a33: [Inliner] Change inline remark format and 
update ReplayInlineAdvisor to use it (authored by modimo).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94333

Files:
  clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
  clang/test/Frontend/optimization-remark-with-hotness.c
  llvm/include/llvm/Analysis/InlineAdvisor.h
  llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
  llvm/lib/Analysis/InlineAdvisor.cpp
  llvm/lib/Analysis/ReplayInlineAdvisor.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll
  llvm/test/Transforms/SampleProfile/Inputs/inline-replay.txt
  llvm/test/Transforms/SampleProfile/inline-replay.ll
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll
  llvm/test/Transforms/SampleProfile/remarks.ll

Index: llvm/test/Transforms/SampleProfile/remarks.ll
===
--- llvm/test/Transforms/SampleProfile/remarks.ll
+++ llvm/test/Transforms/SampleProfile/remarks.ll
@@ -21,8 +21,8 @@
 
 ; We are expecting foo() to be inlined in main() (almost all the cycles are
 ; spent inside foo).
-; CHECK: remark: remarks.cc:13:21: _Z3foov inlined into main to match profiling context with (cost=130, threshold=225) at callsite main:0
-; CHECK: remark: remarks.cc:9:19: rand inlined into main to match profiling context with (cost=always): always inline attribute at callsite _Z3foov:6 @ main:0
+; CHECK: remark: remarks.cc:13:21: _Z3foov inlined into main to match profiling context with (cost=130, threshold=225) at callsite main:0:21;
+; CHECK: remark: remarks.cc:9:19: rand inlined into main to match profiling context with (cost=always): always inline attribute at callsite _Z3foov:6:19 @ main:0:21;
 
 ; The back edge for the loop is the hottest edge in the loop subgraph.
 ; CHECK: remark: remarks.cc:6:9: most popular destination for conditional branches at remarks.cc:5:3
@@ -53,6 +53,9 @@
 ;YAML-NEXT:- String:  main
 ;YAML-NEXT:- String:  ':'
 ;YAML-NEXT:- Line:'0'
+;YAML-NEXT:- String:  ':'
+;YAML-NEXT:- Column:  '21'
+;YAML-NEXT:- String:  ';'
 ;YAML-NEXT:  ...
 ;YAML:   --- !Passed
 ;YAML-NEXT:  Pass:sample-profile-inline
@@ -74,10 +77,15 @@
 ;YAML-NEXT:- String:  _Z3foov
 ;YAML-NEXT:- String:  ':'
 ;YAML-NEXT:- Line:'6'
+;YAML-NEXT:- String:  ':'
+;YAML-NEXT:- Column:  '19'
 ;YAML-NEXT:- String:  ' @ '
 ;YAML-NEXT:- String:  main
 ;YAML-NEXT:- String:  ':'
 ;YAML-NEXT:- Line:'0'
+;YAML-NEXT:- String:  ':'
+;YAML-NEXT:- Column:  '21'
+;YAML-NEXT:- String:  ';'
 ;YAML:  --- !Analysis
 ;YAML-NEXT:  Pass:sample-profile
 ;YAML-NEXT:  Name:AppliedSamples
Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- llvm/test/Transforms/SampleProfile/remarks-hotness.ll
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -36,7 +36,7 @@
 ; YAML-MISS-NEXT: Function:_Z7caller2v
 ; YAML-MISS-NEXT: Hotness: 2
 
-; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1:10; (hotness: 401)
 ; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
 
 ; ModuleID = 'remarks-hotness.cpp'
@@ -93,4 +93,3 @@
 !17 = distinct !DISubprogram(name: "caller2", linkageName: "_Z7caller2v", scope: !1, file: !1, line: 13, type: !8, scopeLine: 13, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
 !18 = !DILocation(line: 14, column: 10, scope: !17)
 !19 = !DILocation(line: 14, column: 3, scope: !17)
-
Index: llvm/test/Transforms/SampleProfile/inline-replay.ll
===
--- llvm/test/Transforms/SampleProfile/inline-replay.ll
+++ llvm/test/Transforms/SampleProfile/inline-replay.ll
@@ -119,4 +119,4 @@
 
 ; REPLAY: _Z3sumii inlined into main
 ; REPLAY: _Z3subii inlined into main
-; REPLA-NOT: _Z3subii inlined into _Z3sumii
+; REPLAY-NOT: _Z3subii inlined into _Z3sumii
Index: llvm/test/Transforms/SampleProfile/Inputs/inline-replay.txt
===
--- llvm/test/Transforms/SampleProfile/Inputs/inline-replay.txt
+++ llvm/test/Transforms/SampleProfile/Inputs/inline-replay.txt
@@ -1,2 +1,2 @@
-remark: call

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:1
+//===--- PathDiagnosticConverterDiagnosticConsumer.cpp --*- C++ 
-*-===//
+//

steakhal wrote:
> I've seen this a few times, and I still don't know why we use it.
> The file extension should be enough to recognize that this is a C++ file.
> Can someone clarify this?
https://llvm.org/docs/CodingStandards.html#file-headers

(with our file name it doesn't look like there's much space even for a short 
description)
(i should probably convert the long description into a doxygen comment as well)



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:14
+
+#include 
+

steakhal wrote:
> I frequently see `#include "clang/..."`-ish includes but never `#include 
> `.
Whoops thanks.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:20
+void PathDiagnosticConverterDiagnosticConsumer::flushPartialDiagnostic() {
+  if (PartialPDs.empty())
+return;

xazax.hun wrote:
> Do we need this early return? We might get the same behavior by simply 
> omitting this check. I have no strong preference about keeping or removing it.
We need it. @steakhal figured this out correctly in the next comment.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:38
+  Msg[0] = toupper(Msg[0]);
+  std::string CleanMsg = Msg.str().substr(0, Msg.str().find('[') - 1).str();
+

steakhal wrote:
> vsavchenko wrote:
> > I think this type of stuff should be covered by a comment or a descriptive 
> > function name.
> Eh, I don't think it's gonna work this way.
> We can not assume that the `[` won't appear in the payload of the message.
> Eg.:
> `NewDelete-checker-test.cpp:193`
> ```
> // newdelete-warning{{Argument to 'delete[]' is the address of the local 
> variable 'i', which is not memory allocated by 'new[]'}}
> ```
> 
> The best you could do is to do a reverse search.
> Do we emit the `[mypackage.mychecker]` suffix for all the reports? If not, 
> then we have a problem.
Uh-oh, mmm, indeed. I should definitely make this optional as well.



Comment at: 
clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:54-60
+  PathDiagnosticPieceRef Piece;
+  if (ShouldDisplayNotesAsEvents) {
+Piece = std::make_shared(PDLoc, Msg);
+  } else {
+Piece = std::make_shared(PDLoc, Msg);
+  }
+  PartialPDs[Consumer]->getMutablePieces().push_back(Piece);

steakhal wrote:
> The ternary operator could simplify this inside the `push_back` function - 
> `emplace_back`?
Can't easily use `?:` here because LHS and RHS are of different types and 
operator `?:` doesn't do implicit casts.



Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:71
+  BTI.CheckName, /*DeclWithIssue=*/nullptr, BTI.BugType,
+  CleanMsg, CleanMsg, BTI.BugCategory, PDLoc, /*DeclToUnique=*/nullptr,
+  std::make_unique());

steakhal wrote:
> Should Desc and ShortDesc be the same?
They don't need to be the same but typically clang diagnostics don't provide 
two different wordings for the same warning.



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:44
+
+  virtual StringRef getName() const override { return "test"; }
+

steakhal wrote:
> I would suggest removing the redundant `virtual`. The same applies to the 
> other decls.
Yup, should've said `override` instead.



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:154
+  std::make_unique(),
+  "void foo() {}", "input.c"));
+}

vsavchenko wrote:
> It is also about the structure, I guess it would've been nice to have all the 
> inputs and expected outputs to be specified here in the actual test, so the 
> reader can figure out what is tested without diving deep into the classes. 
> And it also seems that with the current structure you'll need a couple more 
> classes for every new test.
We have to follow the `libTooling` architecture where we have to have a 
`FrontendAction` class and an `ASTConsumer` class with specific callback 
signatures. I'll try to think of something.


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

https://reviews.llvm.org/D94476

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

[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I removed `CurLoc` from call sites and tried a stage 2 build.

  0x00062228:   DW_TAG_structure_type
  DW_AT_calling_convention(DW_CC_pass_by_value)
  DW_AT_name  ("__va_list_tag")
  DW_AT_byte_size (0x18)
  DW_AT_decl_file 
("/home/maskray/llvm/clang/tools/driver/driver.cpp")
  DW_AT_decl_line (23)

driver.cpp:23 is a `#include`. So this looks strange. The 
DW_AT_decl_file/DW_AT_decl_line attributes are undesired due to `CurLoc` 
getLineNumber.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

[PATCH] D94463: [Driver] Fix assertion failure when -fprofile-generate -fcs-profile-generate are used together

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 316233.
MaskRay retitled this revision from "[Driver] Make -fcs-profile-generate 
require -fprofile-use" to "[Driver] Fix assertion failure when 
-fprofile-generate -fcs-profile-generate are used together".
MaskRay edited the summary of this revision.
MaskRay added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fcs-profile-generate.c


Index: clang/test/Driver/fcs-profile-generate.c
===
--- /dev/null
+++ clang/test/Driver/fcs-profile-generate.c
@@ -0,0 +1,13 @@
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate %s 2>&1 
| FileCheck %s
+// CHECK:  "-fprofile-instrument=csllvm" 
"-fprofile-instrument-use-path=a.profdata"
+
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate=dir %s 
2>&1 | FileCheck %s --check-prefix=CHECK1
+// CHECK1: "-fprofile-instrument=csllvm" 
"-fprofile-instrument-path=dir/default_%m.profraw" 
"-fprofile-instrument-use-path=a.profdata"
+
+/// Degradation case. This usage does not make much sense.
+// RUN: %clang -### -c -fcs-profile-generate %s 2>&1 | FileCheck %s 
--check-prefix=NOUSE
+// NOUSE: "-fprofile-instrument=csllvm"
+// NOUSE-NOT: "-fprofile-instrument-path=
+
+// RUN: %clang -### -c -fprofile-generate -fcs-profile-generate %s 2>&1 | 
FileCheck %s --check-prefix=CONFLICT
+// CONFLICT: error: invalid argument '-fcs-profile-generate' not allowed with 
'-fprofile-generate'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -766,9 +766,11 @@
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << ProfileGenerateArg->getSpelling() << ProfileUseArg->getSpelling();
 
-  if (CSPGOGenerateArg && PGOGenerateArg)
+  if (CSPGOGenerateArg && PGOGenerateArg) {
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << CSPGOGenerateArg->getSpelling() << PGOGenerateArg->getSpelling();
+PGOGenerateArg = nullptr;
+  }
 
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(


Index: clang/test/Driver/fcs-profile-generate.c
===
--- /dev/null
+++ clang/test/Driver/fcs-profile-generate.c
@@ -0,0 +1,13 @@
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate %s 2>&1 | FileCheck %s
+// CHECK:  "-fprofile-instrument=csllvm" "-fprofile-instrument-use-path=a.profdata"
+
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK1
+// CHECK1: "-fprofile-instrument=csllvm" "-fprofile-instrument-path=dir/default_%m.profraw" "-fprofile-instrument-use-path=a.profdata"
+
+/// Degradation case. This usage does not make much sense.
+// RUN: %clang -### -c -fcs-profile-generate %s 2>&1 | FileCheck %s --check-prefix=NOUSE
+// NOUSE: "-fprofile-instrument=csllvm"
+// NOUSE-NOT: "-fprofile-instrument-path=
+
+// RUN: %clang -### -c -fprofile-generate -fcs-profile-generate %s 2>&1 | FileCheck %s --check-prefix=CONFLICT
+// CONFLICT: error: invalid argument '-fcs-profile-generate' not allowed with '-fprofile-generate'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -766,9 +766,11 @@
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << ProfileGenerateArg->getSpelling() << ProfileUseArg->getSpelling();
 
-  if (CSPGOGenerateArg && PGOGenerateArg)
+  if (CSPGOGenerateArg && PGOGenerateArg) {
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << CSPGOGenerateArg->getSpelling() << PGOGenerateArg->getSpelling();
+PGOGenerateArg = nullptr;
+  }
 
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94463: [Driver] Fix assertion failure when -fprofile-generate -fcs-profile-generate are used together

[Rong Xu via Phabricator via cfe-commits]

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

looks good to me. Thanks for working on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

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