[clang] 2b1e25b - [AArch64] Adding ACLE intrinsics for the LS64 extension

2021-01-14 Thread Lucas Prates via cfe-commits

Author: Lucas Prates
Date: 2021-01-14T09:43:58Z
New Revision: 2b1e25befefc20f012aa49011f46e11e8530ee21

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

LOG: [AArch64] Adding ACLE intrinsics for the LS64 extension

This introduces the ARMv8.7-A LS64 extension's intrinsics for 64 bytes
atomic loads and stores: `__arm_ld64b`, `__arm_st64b`, `__arm_st64bv`,
and `__arm_st64bv0`. These are selected into the LS64 instructions
LD64B, ST64B, ST64BV and ST64BV0, respectively.

Based on patches written by Simon Tatham.

Reviewed By: tmatheson

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

Added: 
clang/test/CodeGen/aarch64-ls64.c
llvm/test/CodeGen/AArch64/ls64-intrinsics.ll

Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/arm_acle.h
clang/test/Preprocessor/aarch64-target-features.c
llvm/include/llvm/IR/IntrinsicsAArch64.td
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index c684105908de..b35510f8b691 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -99,6 +99,12 @@ BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
+// Armv8.7-A load/store 64-byte intrinsics
+BUILTIN(__builtin_arm_ld64b, "vvC*WUi*", "n")
+BUILTIN(__builtin_arm_st64b, "vv*WUiC*", "n")
+BUILTIN(__builtin_arm_st64bv, "WUiv*WUiC*", "n")
+BUILTIN(__builtin_arm_st64bv0, "WUiv*WUiC*", "n")
+
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 312c822ebb05..f17134623b8b 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -356,6 +356,9 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
&Opts,
   if (Opts.BranchTargetEnforcement)
 Builder.defineMacro("__ARM_FEATURE_BTI_DEFAULT", "1");
 
+  if (HasLS64)
+Builder.defineMacro("__ARM_FEATURE_LS64", "1");
+
   switch (ArchKind) {
   default:
 break;

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index cf84ad34e1ec..7fa4e4d270ad 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -8979,6 +8979,46 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
 CGM.getIntrinsic(Intrinsic::aarch64_fjcvtzs), Arg);
   }
 
+  if (BuiltinID == AArch64::BI__builtin_arm_ld64b ||
+  BuiltinID == AArch64::BI__builtin_arm_st64b ||
+  BuiltinID == AArch64::BI__builtin_arm_st64bv ||
+  BuiltinID == AArch64::BI__builtin_arm_st64bv0) {
+llvm::Value *MemAddr = EmitScalarExpr(E->getArg(0));
+llvm::Value *ValPtr = EmitScalarExpr(E->getArg(1));
+
+if (BuiltinID == AArch64::BI__builtin_arm_ld64b) {
+  // Load from the address via an LLVM intrinsic, receiving a
+  // tuple of 8 i64 words, and store each one to ValPtr.
+  Function *F = CGM.getIntrinsic(Intrinsic::aarch64_ld64b);
+  llvm::Value *Val = Builder.CreateCall(F, MemAddr);
+  llvm::Value *ToRet;
+  for (size_t i = 0; i < 8; i++) {
+llvm::Value *ValOffsetPtr = Builder.CreateGEP(ValPtr, 
Builder.getInt32(i));
+Address Addr(ValOffsetPtr, CharUnits::fromQuantity(8));
+ToRet = Builder.CreateStore(Builder.CreateExtractValue(Val, i), Addr);
+  }
+  return ToRet;
+} else {
+  // Load 8 i64 words from ValPtr, and store them to the address
+  // via an LLVM intrinsic.
+  SmallVector Args;
+  Args.push_back(MemAddr);
+  for (size_t i = 0; i < 8; i++) {
+llvm::Value *ValOffsetPtr = Builder.CreateGEP(ValPtr, 
Builder.getInt32(i));
+Address Addr(ValOffsetPtr, CharUnits::fromQuantity(8));
+Args.push_back(Builder.CreateLoad(Addr));
+  }
+
+  auto Intr = (BuiltinID == AArch64::BI__builtin_arm_st64b
+   ? Intrinsic::aarch64_st64b
+   : BuiltinID == AArch64::BI__builtin_arm_st64bv
+ ? Intrinsic::aarch64_st64bv
+ : Intrinsic::aarch64_st64bv0);
+  Function *F = CGM.getIntrinsic(Intr);
+  return Builder.CreateCall(F, Args);
+}
+  }
+
   if (BuiltinID == AArch64::BI__clear_cache) {
 assert(E->getNumArgs() == 2 && "__

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

2021-01-14 Thread Lucas Prates 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 rG2b1e25befefc: [AArch64] Adding ACLE intrinsics for the LS64 
extension (authored by pratlucas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93232

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/aarch64-ls64.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/ls64-intrinsics.ll

Index: llvm/test/CodeGen/AArch64/ls64-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/ls64-intrinsics.ll
@@ -0,0 +1,92 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64 -mattr=+ls64 -verify-machineinstrs -o - %s | FileCheck %s
+; RUN: llc -mtriple=aarch64_be -mattr=+ls64 -verify-machineinstrs -o - %s | FileCheck %s
+
+define void @test_ld64b({ i64, i64, i64, i64, i64, i64, i64, i64 }* %out, i8* %addr) {
+; CHECK-LABEL: test_ld64b:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:ld64b x2, [x1]
+; CHECK-NEXT:stp x8, x9, [x0, #48]
+; CHECK-NEXT:stp x6, x7, [x0, #32]
+; CHECK-NEXT:stp x4, x5, [x0, #16]
+; CHECK-NEXT:stp x2, x3, [x0]
+; CHECK-NEXT:ret
+entry:
+  %val = tail call { i64, i64, i64, i64, i64, i64, i64, i64 } @llvm.aarch64.ld64b(i8* %addr)
+  store { i64, i64, i64, i64, i64, i64, i64, i64 } %val, { i64, i64, i64, i64, i64, i64, i64, i64 }* %out, align 8
+  ret void
+}
+
+define void @test_st64b({ i64, i64, i64, i64, i64, i64, i64, i64 }* %in, i8* %addr) {
+; CHECK-LABEL: test_st64b:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:ldp x8, x9, [x0, #48]
+; CHECK-NEXT:ldp x6, x7, [x0, #32]
+; CHECK-NEXT:ldp x4, x5, [x0, #16]
+; CHECK-NEXT:ldp x2, x3, [x0]
+; CHECK-NEXT:st64b x2, [x1]
+; CHECK-NEXT:ret
+entry:
+  %val = load { i64, i64, i64, i64, i64, i64, i64, i64 }, { i64, i64, i64, i64, i64, i64, i64, i64 }* %in, align 8
+  %v0 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 0
+  %v1 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 1
+  %v2 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 2
+  %v3 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 3
+  %v4 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 4
+  %v5 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 5
+  %v6 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 6
+  %v7 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 7
+  tail call void @llvm.aarch64.st64b(i8* %addr, i64 %v0, i64 %v1, i64 %v2, i64 %v3, i64 %v4, i64 %v5, i64 %v6, i64 %v7)
+  ret void
+}
+
+define i64 @test_st64bv({ i64, i64, i64, i64, i64, i64, i64, i64 }* %in, i8* %addr) {
+; CHECK-LABEL: test_st64bv:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:ldp x8, x9, [x0, #48]
+; CHECK-NEXT:ldp x6, x7, [x0, #32]
+; CHECK-NEXT:ldp x4, x5, [x0, #16]
+; CHECK-NEXT:ldp x2, x3, [x0]
+; CHECK-NEXT:st64bv x0, x2, [x1]
+; CHECK-NEXT:ret
+entry:
+  %val = load { i64, i64, i64, i64, i64, i64, i64, i64 }, { i64, i64, i64, i64, i64, i64, i64, i64 }* %in, align 8
+  %v0 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 0
+  %v1 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 1
+  %v2 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 2
+  %v3 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 3
+  %v4 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 4
+  %v5 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 5
+  %v6 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 6
+  %v7 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 7
+  %status = tail call i64 @llvm.aarch64.st64bv(i8* %addr, i64 %v0, i64 %v1, i64 %v2, i64 %v3, i64 %v4, i64 %v5, i64 %v6, i64 %v7)
+  ret i64 %status
+}
+
+define i64 @test_st64bv0({ i64, i64, i64, i64, i64, i64, i64, i64 }* %in, i8* %addr) {
+; CHECK-LABEL: test_st64bv0:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:ldp x8, x9, [x0, #48]
+; CHECK-NEXT:ldp x6, x7, [x0, #32]
+; CHECK-NEXT:ldp x4, x5, [x0, #16]
+; CHECK-NEXT:ldp x2, x3, [x0]
+; CHECK-NEXT:st64bv0 x0, x2, [x1]
+; CHECK-NEXT:ret
+entry:
+  %val = load { i64, i64, i64, i64, i64, i64, i64, i64 }, { i64, i64, i64, i64, i64, i64, i64, i64 }* %in, align 8
+  %v0 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 0
+  %v1 = extractvalue { i64, i64, i64, i64, i64, i64, i64, i64 } %val, 1
+  %v2 = extractvalue { i64, i64, i64, i64, i64, 

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

2021-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:819
+Expanded.IndentCaseLabels = true;
+Expanded.IndentCaseBlocks = false;
 break;

I guess its fine to define these defaults here but what if people turn them off 
in their config what will happen?



Comment at: clang/unittests/Format/FormatTest.cpp:13707
WhitesmithsBraceStyle);
 
   verifyFormat("enum X\n"

timwoj wrote:
> MyDeveloperDay wrote:
> > any reason why this is being removed?
> This is an artifact of there not being any sort of actual guide for 
> Whitesmiths. I have it set now to always indent the case labels (and it 
> should just ignore the IndentCaseLabels option entirely, like it does for 
> IndentCaseBlocks). I can certainly fix that option to work again though.
see the images from {D93806}, you are saying that anyone with 
bracewrappingsytle of Whitesmith cannot decide if they can or cannot indent 
case labels?

Could you show me a spec that show Whitesmiths is the other way around?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

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


[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

My assumption is that you want to stick with the minimum and maximum is that 
correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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


[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-01-14 Thread Jakub Budiský via Phabricator via cfe-commits
Budovi created this revision.
Budovi added reviewers: MyDeveloperDay, rsmith.
Budovi added a project: clang-format.
Budovi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds support for coding styles that make a separate indentation level for 
access modifiers, such as Code::Blocks or QtCreator.

The new option, `IndentAccessModifiers`, if enabled, forces the content inside 
classes, structs and unions (“records”) to be indented twice while removing a 
level for access modifiers. The value of `AccessModifierOffset` is disregarded 
in this case, aiming towards an ease of use.




The PR (https://bugs.llvm.org/show_bug.cgi?id=19056) had an implementation 
attempt by @MyDeveloperDay already (https://reviews.llvm.org/D60225) but I've 
decided to start from scratch. They differ in functionality, chosen approaches, 
and even the option name. The code tries to re-use the existing functionality 
to achieve this behavior, limiting possibility of breaking something else.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94661

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.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
@@ -14279,6 +14279,7 @@
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
+  CHECK_PARSE_BOOL(IndentAccessModifiers);
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
@@ -17862,6 +17863,68 @@
Style));
 }
 
+TEST_F(FormatTest, IndentAccessModifiers) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AccessModifierOffset = 4;
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.IndentAccessModifiers = true;
+  verifyFormat("class C {\n"
+   "int i;\n"
+   "};\n",
+   Style);
+  verifyFormat("class C {\n"
+   "  public:\n"
+   "int i;\n"
+   "};\n",
+   Style);
+  {
+const char *Expected = "struct S {\n"
+   "  private:\n"
+   "class C {\n"
+   "int j;\n"
+   "\n"
+   "  public:\n"
+   "C();\n"
+   "};\n"
+   "\n"
+   "  public:\n"
+   "int i;\n"
+   "};\n";
+const char *ToFormat = "struct S {\n"
+   "private:\n"
+   "class C {\n"
+   "int j;\n"
+   "public:\n"
+   "C();\n"
+   "};\n"
+   "public:\n"
+   "int i;\n"
+   "};\n";
+
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+  verifyFormat("union C {\n"
+   "int i;\n"
+   "unsigned u;\n"
+   "};\n",
+   Style);
+  verifyFormat("enum class E {\n"
+   "  A,\n"
+   "  B\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  class C {\n"
+   "  int i;\n"
+   "  };\n"
+   "  return;\n"
+   "}\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -85,7 +85,7 @@
   void reset();
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
-  void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+  void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective(unsigned Level);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -579,7 +579,7 @@
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool Must

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-01-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D92257#2497535 , @MyDeveloperDay 
wrote:

> My assumption is that you want to stick with the minimum and maximum is that 
> correct?

Otherwise I have to make a breaking change, or not achieve at all what I want. 
So either abandon this or we need a minimum and maximum.
Although right now I have changed an existing test because in that case the 
behavior changed (as noted inline) and a test case still to decide how to 
advance.

If that little break is not acceptable I see no further base to pursue this and 
have to decide to drop it totally or only apply it locally.

And thanks for bringing this up again, currently I have very little time to 
work on clang-format and only react on the mails. Even while I have many things 
I want to add/change or cases where it is currently misformatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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


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

2021-01-14 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 316599.
njames93 added a comment.

Refactor RefCntString out of DraftStore class
Remove MTime from Draft, other users of DraftStore don't need to see it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94554

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/DraftStore.cpp
  clang-tools-extra/clangd/DraftStore.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -74,7 +74,8 @@
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
 Range.second, [&](SelectionTree ST) {
   Tweak::Selection S(Index, AST, Range.first,
- Range.second, std::move(ST));
+ Range.second, std::move(ST),
+ nullptr);
   if (auto T = prepareTweak(TweakID, S)) {
 Result = (*T)->apply(S);
 return true;
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -33,6 +33,19 @@
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
 
+llvm::IntrusiveRefCntPtr
+createOverlay(llvm::IntrusiveRefCntPtr Base,
+  llvm::IntrusiveRefCntPtr Overlay) {
+  auto OFS =
+  llvm::makeIntrusiveRefCnt(std::move(Base));
+  OFS->pushOverlay(std::move(Overlay));
+  return OFS;
+}
+
+llvm::IntrusiveRefCntPtr getVFSFromAST(ParsedAST &AST) {
+  return &AST.getSourceManager().getFileManager().getVirtualFileSystem();
+}
+
 // Convert a Range to a Ref.
 Ref refWithRange(const clangd::Range &Range, const std::string &URI) {
   Ref Result;
@@ -833,8 +846,8 @@
 TU.ExtraArgs.push_back("-xobjective-c++");
 auto AST = TU.build();
 for (const auto &RenamePos : Code.points()) {
-  auto RenameResult =
-  rename({RenamePos, NewName, AST, testPath(TU.Filename)});
+  auto RenameResult = rename(
+  {RenamePos, NewName, AST, testPath(TU.Filename), getVFSFromAST(AST)});
   ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
   ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
   EXPECT_EQ(
@@ -1040,8 +1053,8 @@
 }
 auto AST = TU.build();
 llvm::StringRef NewName = Case.NewName;
-auto Results =
-rename({T.point(), NewName, AST, testPath(TU.Filename), Case.Index});
+auto Results = rename({T.point(), NewName, AST, testPath(TU.Filename),
+   getVFSFromAST(AST), Case.Index});
 bool WantRename = true;
 if (T.ranges().empty())
   WantRename = false;
@@ -1081,8 +1094,8 @@
   auto AST = TU.build();
   llvm::StringRef NewName = "abcde";
 
-  auto RenameResult =
-  rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  auto RenameResult = rename(
+  {Code.point(), NewName, AST, testPath(TU.Filename), getVFSFromAST(AST)});
   ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point();
   ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
   EXPECT_EQ(applyEdits(std::move(RenameResult->GlobalChanges)).front().second,
@@ -1098,7 +,8 @@
   )cpp";
   TU.HeaderFilename = "protobuf.pb.h";
   auto AST = TU.build();
-  auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename)});
+  auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename),
+ getVFSFromAST(AST)});
   EXPECT_FALSE(Results);
   EXPECT_THAT(llvm::toString(Results.takeError()),
   testing::HasSubstr("not a supported kind"));
@@ -1170,12 +1184,10 @@
 
   Annotations MainCode("class  [[Fo^o]] {};");
   auto MainFilePath = testPath("main.cc");
-  // Dirty buffer for foo.cc.
-  auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
-if (Path == FooPath)

[PATCH] D92080: [Clang] Mutate long-double math builtins into f128 under IEEE-quad

2021-01-14 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a reviewer: hfinkel.
RKSimon added a comment.

LGTM but a PPC-guru needs to signoff


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

https://reviews.llvm.org/D92080

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


[clang] 17f8c45 - [clang] Use SourceLocations in unions [NFCI]

2021-01-14 Thread Mikhail Maltsev via cfe-commits

Author: Mikhail Maltsev
Date: 2021-01-14T10:56:53Z
New Revision: 17f8c458de631c0311828931a5bdf72b1a13c29d

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

LOG: [clang] Use SourceLocations in unions [NFCI]

Currently, there are many instances where `SourceLocation` objects are
converted to raw representation to be stored in structs that are
used as fields of tagged unions.

This is done to make the corresponding structs trivial.
Triviality allows avoiding undefined behavior when implicitly changing
the active member of the union.

However, in most cases, we can explicitly construct an active member
using placement new. This patch adds the required active member
selections and replaces `SourceLocation`-s represented as
`unsigned int` with proper `SourceLocation`-s.

One notable exception is `DeclarationNameLoc`: the objects of this class
are often not properly initialized (so the code currently relies on
its default constructor which uses memset). This class will be fixed
in a separate patch.

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/include/clang/AST/DependentDiagnostic.h
clang/include/clang/AST/Expr.h
clang/include/clang/AST/TemplateBase.h
clang/include/clang/Basic/SourceManager.h
clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Designator.h
clang/include/clang/Sema/Initialization.h
clang/lib/AST/Expr.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DependentDiagnostic.h 
b/clang/include/clang/AST/DependentDiagnostic.h
index 5a8624608e74..18276d54d540 100644
--- a/clang/include/clang/AST/DependentDiagnostic.h
+++ b/clang/include/clang/AST/DependentDiagnostic.h
@@ -48,7 +48,7 @@ class DependentDiagnostic {
  QualType BaseObjectType,
  const PartialDiagnostic &PDiag) {
 DependentDiagnostic *DD = Create(Context, Parent, PDiag);
-DD->AccessData.Loc = Loc.getRawEncoding();
+DD->AccessData.Loc = Loc;
 DD->AccessData.IsMember = IsMemberAccess;
 DD->AccessData.Access = AS;
 DD->AccessData.TargetDecl = TargetDecl;
@@ -73,7 +73,7 @@ class DependentDiagnostic {
 
   SourceLocation getAccessLoc() const {
 assert(getKind() == Access);
-return SourceLocation::getFromRawEncoding(AccessData.Loc);
+return AccessData.Loc;
   }
 
   NamedDecl *getAccessTarget() const {
@@ -112,7 +112,7 @@ class DependentDiagnostic {
   PartialDiagnostic Diag;
 
   struct {
-unsigned Loc;
+SourceLocation Loc;
 unsigned Access : 2;
 unsigned IsMember : 1;
 NamedDecl *TargetDecl;

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index c8d87ec48a3f..a44d06967431 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4994,10 +4994,10 @@ class DesignatedInitExpr final
 uintptr_t NameOrField;
 
 /// The location of the '.' in the designated initializer.
-unsigned DotLoc;
+SourceLocation DotLoc;
 
 /// The location of the field name in the designated initializer.
-unsigned FieldLoc;
+SourceLocation FieldLoc;
   };
 
   /// An array or GNU array-range designator, e.g., "[9]" or "[10..15]".
@@ -5006,12 +5006,12 @@ class DesignatedInitExpr final
 /// initializer expression's list of subexpressions.
 unsigned Index;
 /// The location of the '[' starting the array range designator.
-unsigned LBracketLoc;
+SourceLocation LBracketLoc;
 /// The location of the ellipsis separating the start and end
 /// indices. Only valid for GNU array-range designators.
-unsigned EllipsisLoc;
+SourceLocation EllipsisLoc;
 /// The location of the ']' terminating the array range designator.
-unsigned RBracketLoc;
+SourceLocation RBracketLoc;
   };
 
   /// Represents a single C99 designator.
@@ -5043,29 +5043,32 @@ class DesignatedInitExpr final
 Designator(const IdentifierInfo *FieldName, SourceLocation DotLoc,
SourceLocation FieldLoc)
   : Kind(FieldDesignator) {
+  new (&Field) DesignatedInitExpr::FieldDesignator;
   Field.NameOrField = reinterpret_cast(FieldName) | 0x01;
-  Field.DotLoc = DotLoc.getRawEncoding();
-  Field.FieldLoc = FieldLoc.getRawEncoding();
+  Field.DotLoc = DotLoc;
+  Field.FieldLoc = FieldLoc;
 }
 
 /// Initializes an array designator.
 Designator(unsigned Index, SourceLocation LBracketLoc,
SourceLocation RBracketLoc)
   : Kind(ArrayDesignator) {
+  new (&ArrayOrRange) Desi

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

2021-01-14 Thread Mikhail Maltsev 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 rG17f8c458de63: [clang] Use SourceLocations in unions [NFCI] 
(authored by miyuki).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94237

Files:
  clang/include/clang/AST/DependentDiagnostic.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Designator.h
  clang/include/clang/Sema/Initialization.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3125,11 +3125,11 @@
   diag::warn_qual_return_type,
   PTI.TypeQuals,
   SourceLocation(),
-  SourceLocation::getFromRawEncoding(PTI.ConstQualLoc),
-  SourceLocation::getFromRawEncoding(PTI.VolatileQualLoc),
-  SourceLocation::getFromRawEncoding(PTI.RestrictQualLoc),
-  SourceLocation::getFromRawEncoding(PTI.AtomicQualLoc),
-  SourceLocation::getFromRawEncoding(PTI.UnalignedQualLoc));
+  PTI.ConstQualLoc,
+  PTI.VolatileQualLoc,
+  PTI.RestrictQualLoc,
+  PTI.AtomicQualLoc,
+  PTI.UnalignedQualLoc);
   return;
 }
 
@@ -6086,7 +6086,7 @@
   }
 
   // Finally fill in MemberPointerLocInfo fields.
-  TL.setStarLoc(SourceLocation::getFromRawEncoding(Chunk.Mem.StarLoc));
+  TL.setStarLoc(Chunk.Mem.StarLoc);
   TL.setClassTInfo(ClsTInfo);
 }
 void VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc TL) {
@@ -6163,7 +6163,7 @@
 llvm_unreachable("cannot be _Atomic qualified");
 
   case DeclaratorChunk::Pointer:
-Loc = SourceLocation::getFromRawEncoding(Chunk.Ptr.AtomicQualLoc);
+Loc = Chunk.Ptr.AtomicQualLoc;
 break;
 
   case DeclaratorChunk::BlockPointer:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2851,8 +2851,7 @@
   // turn this into Self->ivar, just return a BareIVarExpr or something.
   IdentifierInfo &II = Context.Idents.get("self");
   UnqualifiedId SelfName;
-  SelfName.setIdentifier(&II, SourceLocation());
-  SelfName.setKind(UnqualifiedIdKind::IK_ImplicitSelfParam);
+  SelfName.setImplicitSelfParam(&II);
   CXXScopeSpec SelfScopeSpec;
   SourceLocation TemplateKWLoc;
   ExprResult SelfExpr =
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5344,8 +5344,8 @@
   case UnqualifiedIdKind::IK_OperatorFunctionId:
 NameInfo.setName(Context.DeclarationNames.getCXXOperatorName(
Name.OperatorFunctionId.Operator));
-NameInfo.getInfo().CXXOperatorName.BeginOpNameLoc
-  = Name.OperatorFunctionId.SymbolLocations[0];
+NameInfo.getInfo().CXXOperatorName.BeginOpNameLoc =
+Name.OperatorFunctionId.SymbolLocations[0].getRawEncoding();
 NameInfo.getInfo().CXXOperatorName.EndOpNameLoc
   = Name.EndLocation.getRawEncoding();
 return NameInfo;
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -191,28 +191,29 @@
   I.Kind= Function;
   I.Loc = LocalRangeBegin;
   I.EndLoc  = LocalRangeEnd;
+  new (&I.Fun) FunctionTypeInfo;
   I.Fun.hasPrototype= hasProto;
   I.Fun.isVariadic  = EllipsisLoc.isValid();
   I.Fun.isAmbiguous = isAmbiguous;
-  I.Fun.LParenLoc   = LParenLoc.getRawEncoding();
-  I.Fun.EllipsisLoc = EllipsisLoc.getRawEncoding();
-  I.Fun.RParenLoc   = RParenLoc.getRawEncoding();
+  I.Fun.LParenLoc   = LParenLoc;
+  I.Fun.EllipsisLoc = EllipsisLoc;
+  I.Fun.RParenLoc   = RParenLoc;
   I.Fun.DeleteParams= false;
   I.Fun.NumParams   = NumParams;
   I.Fun.Params  = nullptr;
   I.Fun.RefQualifierIsLValueRef = RefQualifierIsLvalueRef;
-  I.Fun.RefQualifierLoc = RefQualifierLoc.getRawEncoding();
-  I.Fun.MutableLoc  = MutableLoc.getRawEncoding();
+  I.Fun.RefQualifierLoc = RefQualifierLoc;
+  I.Fun.MutableLoc  = MutableLoc;
   I.Fun.ExceptionSpecType   = ESpecType;
-  I.Fun.ExceptionSpecLocBeg = ESpecRange.getBegin().getRawEncoding();
-  I.Fun.

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

2021-01-14 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

> This revision was landed with ongoing or failed builds.

The only failures were clang-format warnings, they looked bogus to me.


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] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

2021-01-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

It seems quite a challenge to hook the `Preprocessor` for all possible 
configurations for every `CompilerInvocation`.
The underlying machinery is somewhat complex and spaghetti to me.

Here is what I suggest:
For now, this expansion is better than the previous was. Macro expansions will 
work for the main TU even in any CTU configuration. For the imported TUs, it 
will just not expand any macros.
This is a regression in some way, but we should live with that until we 
implement it completely.
I think the users would prefer a non-crashing partial implementation to a 
crashing one.

If you think it's an appropriate, I will update the CTU tests to expect no 
macro expansion in any imported TU.
And also remove the `XFAIL`.
This way we could still target clang-12.

Do you think it's acceptable?
@xazax.hun @NoQ


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] D93942: [OpenCL] Improve online documentation.

2021-01-14 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh accepted this revision.
svenvh added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


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

https://reviews.llvm.org/D93942

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


[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-01-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

I would add a test where you have a member before the first access modifier.
Also this is not exactly what they wanted in the bug, as far as I can see 
members of structs or classes with no access modifier at all should only be 
indented once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94661

___
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

2021-01-14 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D93224#2497632 , @steakhal wrote:

> Do you think it's acceptable?
> @xazax.hun @NoQ

It is fine by me. I believe, currently is Ericsson the biggest contributor and 
user of the CTU functionality. So in case they are onboard with your plans, 
that should be OK.


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


[clang-tools-extra] 2e25be0 - [clangd] Add main file macros into the main-file index.

2021-01-14 Thread Aleksandr Platonov via cfe-commits

Author: Aleksandr Platonov
Date: 2021-01-14T15:10:17+03:00
New Revision: 2e25be0b6134e9544f7cee7bb7b31a921ca37cc0

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

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

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.

Reviewed By: sammccall

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

Added: 


Modified: 
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

Removed: 




diff  --git a/clang-tools-extra/clangd/CollectMacros.cpp 
b/clang-tools-extra/clangd/CollectMacros.cpp
index bd6165ef10ba..0e89b35d9d56 100644
--- a/clang-tools-extra/clangd/CollectMacros.cpp
+++ b/clang-tools-extra/clangd/CollectMacros.cpp
@@ -13,8 +13,8 @@
 namespace clang {
 namespace clangd {
 
-void CollectMainFileMacros::add(const Token &MacroNameTok,
-const MacroInfo *MI) {
+void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
+bool IsDefinition) {
   if (!InMainFile)
 return;
   auto Loc = MacroNameTok.getLocation();
@@ -26,9 +26,9 @@ void CollectMainFileMacros::add(const Token &MacroNameTok,
   auto Range = halfOpenToRange(
   SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
   if (auto SID = getSymbolID(Name, MI, SM))
-Out.MacroRefs[SID].push_back(Range);
+Out.MacroRefs[SID].push_back({Range, IsDefinition});
   else
-Out.UnknownMacros.push_back(Range);
+Out.UnknownMacros.push_back({Range, IsDefinition});
 }
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/CollectMacros.h 
b/clang-tools-extra/clangd/CollectMacros.h
index eecea0455be2..3240111e5a33 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -21,15 +21,20 @@
 namespace clang {
 namespace clangd {
 
-struct MainFileMacros {
-  llvm::StringSet<> Names;
+struct MacroOccurrence {
   // Instead of storing SourceLocation, we have to store the token range 
because
   // SourceManager from preamble is not available when we build the AST.
-  llvm::DenseMap> MacroRefs;
+  Range Rng;
+  bool IsDefinition;
+};
+
+struct MainFileMacros {
+  llvm::StringSet<> Names;
+  llvm::DenseMap> MacroRefs;
   // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
   // reference to an undefined macro. Store them separately, e.g. for semantic
   // highlighting.
-  std::vector UnknownMacros;
+  std::vector UnknownMacros;
   // Ranges skipped by the preprocessor due to being inactive.
   std::vector SkippedRanges;
 };
@@ -49,7 +54,7 @@ class CollectMainFileMacros : public PPCallbacks {
   }
 
   void MacroDefined(const Token &MacroName, const MacroDirective *MD) override 
{
-add(MacroName, MD->getMacroInfo());
+add(MacroName, MD->getMacroInfo(), /*IsDefinition=*/true);
   }
 
   void MacroExpands(const Token &MacroName, const MacroDefinition &MD,
@@ -87,7 +92,8 @@ class CollectMainFileMacros : public PPCallbacks {
   }
 
 private:
-  void add(const Token &MacroNameTok, const MacroInfo *MI);
+  void add(const Token &MacroNameTok, const MacroInfo *MI,
+   bool IsDefinition = false);
   const SourceManager &SM;
   bool InMainFile = true;
   MainFileMacros &Out;

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index be0a5437cde6..5e70baf73310 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -397,10 +397,10 @@ std::vector 
getSemanticHighlightings(ParsedAST &AST) {
   // Add highlightings for macro references.
   for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
 for (const auto &M : SIDToRefs.second)
-  Builder.addToken({HighlightingKind::Macro, M});
+  Builder.addToken({HighlightingKind::Macro, M.Rng});
   }
   for (const auto &M : AST.getMacros().UnknownMacros)
-Builder.addToken({Highlig

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

2021-01-14 Thread Aleksandr Platonov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e25be0b6134: [clangd] Add main file macros into the 
main-file index. (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

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,10 +341,10 @@
   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.Rng.start);
   }
   for (const auto &R : AST.getMacros().UnknownMacros)
-MacroExpansionPositions.push_back(R.start);
+MacroExpansionPositions.push_back(R.Rng.start);
   EXPECT_THAT(MacroExpansionPositions,
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
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,14 +95,18 @@
   assert(Macro);
   auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
 
-  EXPECT_THAT(ExpectedRefs,
-  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID]))
+  std::vector Ranges;
+  for (const auto &Ref : ActualMacroRefs.MacroRefs[SID])
+Ranges.push_back(Ref.Rng);
+  EXPECT_THAT(ExpectedRefs, UnorderedElementsAreArray(Ranges))
   << "Annotation=" << I << ", MacroName=" << Macro->Name
   << ", Test = " << Test;
 }
 // Unknown macros.
-EXPECT_THAT(AST.getMacros().UnknownMacros,
-UnorderedElementsAreArray(T.ranges("Unknown")))
+std::vector Ranges;
+for (const auto &Ref : AST.getMacros().UnknownMacros)
+  Ranges.push_back(Ref.Rng);
+EXPECT_THAT(Ranges, UnorderedElementsAreArray(T.ranges("Unknown")))
 << "Unknown macros doesn't match in " << Test;
   }
 }
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -386,16 +386,31 @@
   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 &MacroRef : IDToRefs.second) {
+  const auto &Range = MacroRef.Rng;
+  bool IsDefinition = MacroRef.IsDefinition;
   Ref R;
   R.Location.Start.setLine(Range.start.line);
   R.Location.Start.setColumn(Range.start.character);
   R.Location.End.setLine(Range.end.line);
   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 = cantFail(sourceLocationInMainFile(SM, Range.start));
+auto EndLoc 

[clang] 3bccd87 - [clang][cli] NFC: Remove SSPBufferSize assignment

2021-01-14 Thread Jan Svoboda via cfe-commits

Author: Jan Svoboda
Date: 2021-01-14T13:21:44+01:00
New Revision: 3bccd87a588b3c320b669686c8f006b92ff72182

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

LOG: [clang][cli] NFC: Remove SSPBufferSize assignment

This should've been part of D84669, but got overlooked. Removing the assignment 
is NFC, as it's also done by the marshalling infrastructure for the 
stack_protector_buffer_size option.

Reviewed By: dexonsmith in D94488

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f496c1d65bc5..cbe038224323 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1142,8 +1142,6 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, 
ArgList &Args, InputKind IK,
 }
 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));



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


[clang] fa2fe96 - [clang][cli] Port more CodeGenOptions to marshalling infrastructure

2021-01-14 Thread Jan Svoboda via cfe-commits

Author: Jan Svoboda
Date: 2021-01-14T13:21:44+01:00
New Revision: fa2fe9608c1c1b402296960b1edc157230c30062

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

LOG: [clang][cli] Port more CodeGenOptions to marshalling infrastructure

Leveraging the recently added TableGen constructs (ShouldParseIf and 
MarshallingInfoStringInt) to shift from manual command line parsing to 
automatic TableGen-driver marshalling.

Reviewed By: dexonsmith

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

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 9ea3feccddff..2123ac226ffc 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1162,12 +1162,22 @@ def fno_profile_instr_use : Flag<["-"], 
"fno-profile-instr-use">,
 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 all 
the regexes separated by a semi-colon">,
+MarshallingInfoString<"CodeGenOpts.ProfileExcludeFiles">,
+ShouldParseIf;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
 Group, Flags<[CC1Option, CoreOption]>, 
Values<"atomic,prefer-atomic,single">,
 MetaVarName<"">, HelpText<"Set update method of profile counters 
(atomic,prefer-atomic,single)">,
@@ -1318,9 +1328,10 @@ defm eliminate_unused_debug_types : 
OptOutFFlag<"eliminate-unused-debug-types",
 def femit_all_decls : Flag<["-"], "femit-all-decls">, Group, 
Flags<[CC1Option]>,
   HelpText<"Emit all declarations, even if unused">,
   MarshallingInfoFlag<"LangOpts->EmitAllDecls">;
-def femulated_tls : Flag<["-"], "femulated-tls">, Group, 
Flags<[CC1Option]>,
-  HelpText<"Use emutls functions to access thread_local variables">;
-def fno_emulated_tls : Flag<["-"], "fno-emulated-tls">, Group, 
Flags<[CC1Option]>;
+defm emulated_tls : BoolFOption<"emulated-tls",
+  "CodeGenOpts.EmulatedTLS", DefaultsToFalse,
+  ChangedBy,
+  ResetBy, BothFlags<[CC1Option]>>;
 def fencoding_EQ : Joined<["-"], "fencoding=">, Group;
 def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, 
Flags<[CoreOption]>;
 defm exceptions : BoolFOption<"exceptions",
@@ -1759,12 +1770,14 @@ def fxray_instrumentation_bundle :
 def fxray_function_groups :
   Joined<["-"], "fxray-function-groups=">,
   Group, Flags<[CC1Option]>,
-  HelpText<"Only instrument 1 of N groups">;
+  HelpText<"Only instrument 1 of N groups">,
+  MarshallingInfoStringInt<"CodeGenOpts.XRayTotalFunctionGroups", "1">;
 
 def fxray_selected_function_group :
   Joined<["-"], "fxray-selected-function-group=">,
   Group, Flags<[CC1Option]>,
-  HelpText<"When using -fxray-function-groups, select which group of functions 
to instrument. Valid range is 0 to fxray-function-groups - 1">;
+  HelpText<"When using -fxray-function-groups, select which group of functions 
to instrument. Valid range is 0 to fxray-function-groups - 1">,
+  MarshallingInfoStringInt<"CodeGenOpts.XRaySelectedFunctionGroup", "0">;
 
 
 defm fine_grained_bitfield_accesses : 
BoolOption<"fine-grained-bitfield-accesses",
@@ -2223,9 +2236,6 @@ defm preserve_as_comments : 
BoolFOption<"preserve-as-comments",
   "CodeGenOpts.PreserveAsmComments", DefaultsToTrue,
   ChangedBy,
   ResetBy>;
-defm profile_arcs : BoolFOption<"profile-arcs",
-  "CodeGenOpts.EmitGcovArcs", DefaultsToFalse,
-  ChangedBy, ResetBy>;
 def framework : Separate<["-"], "framework">, Flags<[LinkerInput]>;
 def frandom_seed_EQ : Joined<["-"], "frandom-seed=">, 
Group;
 def freg_struct_return : Flag<["-"], "freg-struct-return">, Group, 
Flags<[CC1Option]>,
@@ -2348,9 +2358,6 @@ def foptimization_record_passes_EQ : 

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

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa2fe9608c1c: [clang][cli] Port more CodeGenOptions to 
marshalling infrastructure (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D94488?vs=316045&id=316619#toc

Repository:
  rG LLVM Github Monorepo

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

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;
@@ -1143,26 +1120,9 @@
 Opts.LinkBitcodeFiles.push_back(F);
   }
 
-  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
@@ -1162,12 +1162,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 all the regexes separated by a semi-colon">,
+M

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

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

Thanks for taking a look! Committed with more detailed message.




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

dexonsmith wrote:
> 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..
Makes sense, committed in 
https://reviews.llvm.org/rG3bccd87a588b3c320b669686c8f006b92ff72182.


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] D43002: [CodeView] Emit S_OBJNAME record

2021-01-14 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 8 inline comments as done.
aganea added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:212
+  /// Output filename used in the COFF debug information.
+  std::string COFFOutputFilename;
+

rnk wrote:
> Let's bikeshed the name a bit. This thing is the `-o`/`/Fo` value, plus some 
> processing. It can be symbolic, as in `-`. It could be the name of a bitcode 
> file with `-flto`. It could be the name of an assembly file if you do `clang 
> -S --target=x86_64-windows-msvc -g t.cpp -o t.s`. It could be the name of an 
> ELF file if you try hard. I think in any of these cases where the user 
> directly emits something that isn't a COFF object, it's OK to use that name 
> for the S_OBJNAME record.
> 
> What it is, is the name of the compiler's output file, as we would like it to 
> appear in debug info. With that in mind, here are some ideas:
> - CodeViewObjectName: very directly referring to S_OBJNAME
> - ObjectFilename: very general
> - ObjectFilenameForDebug: generalizing over cv/dwarf
> - OutputFilenameForDebug: generalizing over assembly, bc, and obj
> 
> I think I like ObjectFilenameForDebug best. DebugObjectFilename seems like a 
> no-go, since that sounds like the dwo file name.
> 
> ---
> 
> This brings me to the case of `-save-temps` / `/Fa`. In these cases, the 
> compile action is broken into pieces, and the assembler is invoked in a 
> follow-up process. Does that mean the driver needs to pass an extra flag 
> along to the -cc1 action? That would be a bummer.
> 
I think we should fix `-save-temps` and `/Fa`, thanks for raising that! Would 
you mind if I added a new cc1 flag for that purpose? Something like 
`-target-file-name`.
When using `-save-temps`, each cc1 command doesn't know about the final 
filename, except for the last cc1 command. We would need to provide that name 
somehow when passing `-S`.



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752
 
+static void unescapeSlashes(SmallVectorImpl &Str) {
+  auto Read = Str.begin();

rnk wrote:
> This isn't unescaping them, it's just canonicalizing double slashes to one 
> slash, right? Would `llvm::sys::native` suffice?
`llvm::sys::path::native()` doesn't remove consecutive (back)slashes. I think 
@zturner's main intent was when debugging in Visual Studio with arguments from 
LIT tests, they sometimes contain double backslashes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002

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


[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

2021-01-14 Thread Liao Chunyu via Phabricator via cfe-commits
liaolucy added a comment.

In D92715#2495897 , @HsiangKai wrote:

> @liaolucy RISC-V vector types are sizeless types. Sizeless is kind of 
> characteristic for builtin types. If we use attribute to declare RISC-V 
> vector types, the frontend does not know anything about it. I still think to 
> define RISC-V vector types as builtin types is a better way.

Thanks, It's OK for me.


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

https://reviews.llvm.org/D92715

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


[PATCH] D43002: [CodeView] Emit S_OBJNAME record

2021-01-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:212
+  /// Output filename used in the COFF debug information.
+  std::string COFFOutputFilename;
+

aganea wrote:
> rnk wrote:
> > Let's bikeshed the name a bit. This thing is the `-o`/`/Fo` value, plus 
> > some processing. It can be symbolic, as in `-`. It could be the name of a 
> > bitcode file with `-flto`. It could be the name of an assembly file if you 
> > do `clang -S --target=x86_64-windows-msvc -g t.cpp -o t.s`. It could be the 
> > name of an ELF file if you try hard. I think in any of these cases where 
> > the user directly emits something that isn't a COFF object, it's OK to use 
> > that name for the S_OBJNAME record.
> > 
> > What it is, is the name of the compiler's output file, as we would like it 
> > to appear in debug info. With that in mind, here are some ideas:
> > - CodeViewObjectName: very directly referring to S_OBJNAME
> > - ObjectFilename: very general
> > - ObjectFilenameForDebug: generalizing over cv/dwarf
> > - OutputFilenameForDebug: generalizing over assembly, bc, and obj
> > 
> > I think I like ObjectFilenameForDebug best. DebugObjectFilename seems like 
> > a no-go, since that sounds like the dwo file name.
> > 
> > ---
> > 
> > This brings me to the case of `-save-temps` / `/Fa`. In these cases, the 
> > compile action is broken into pieces, and the assembler is invoked in a 
> > follow-up process. Does that mean the driver needs to pass an extra flag 
> > along to the -cc1 action? That would be a bummer.
> > 
> I think we should fix `-save-temps` and `/Fa`, thanks for raising that! Would 
> you mind if I added a new cc1 flag for that purpose? Something like 
> `-target-file-name`.
> When using `-save-temps`, each cc1 command doesn't know about the final 
> filename, except for the last cc1 command. We would need to provide that name 
> somehow when passing `-S`.
Yes, I think we'll need a new cc1 flag. I'd avoid "target" in the flag name, 
since it makes me thing of target as in platform, OS, ISA, etc. Maybe just 
-object-file-name? It's very boring, and reasonably specific.



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752
 
+static void unescapeSlashes(SmallVectorImpl &Str) {
+  auto Read = Str.begin();

aganea wrote:
> rnk wrote:
> > This isn't unescaping them, it's just canonicalizing double slashes to one 
> > slash, right? Would `llvm::sys::native` suffice?
> `llvm::sys::path::native()` doesn't remove consecutive (back)slashes. I think 
> @zturner's main intent was when debugging in Visual Studio with arguments 
> from LIT tests, they sometimes contain double backslashes.
I see. I think "unescape" shouldn't be in the name, this isn't about escape 
characters, it's about cleaning up or canonicalizing the path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002

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


[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

2021-01-14 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai updated this revision to Diff 316556.
HsiangKai added a comment.

Recover comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92715

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/RISCVVTypes.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/module.modulemap
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
  clang/test/Sema/riscv-types.c
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1548,6 +1548,8 @@
 #include "clang/Basic/AArch64SVEACLETypes.def"
 #define PPC_VECTOR_TYPE(Name, Id, Size) case BuiltinType::Id:
 #include "clang/Basic/PPCTypes.def"
+#define RVV_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/RISCVVTypes.def"
 #define BUILTIN_TYPE(Id, SingletonId)
 #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
 #define UNSIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
Index: clang/test/Sema/riscv-types.c
===
--- /dev/null
+++ clang/test/Sema/riscv-types.c
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -ast-print %s \
+// RUN:| FileCheck %s
+
+void bar(void) {
+  // CHECK: __rvv_int64m1_t x0;
+  __rvv_int64m1_t x0;
+
+  // CHECK: __rvv_float64m1_t x1;
+  __rvv_float64m1_t x1;
+
+  // CHECK: __rvv_int64m2_t x2;
+  __rvv_int64m2_t x2;
+
+  // CHECK: __rvv_float64m2_t x3;
+  __rvv_float64m2_t x3;
+
+  // CHECK: __rvv_int64m4_t x4;
+  __rvv_int64m4_t x4;
+
+  // CHECK: __rvv_float64m4_t x5;
+  __rvv_float64m4_t x5;
+
+  // CHECK: __rvv_int64m8_t x6;
+  __rvv_int64m8_t x6;
+
+  // CHECK: __rvv_float64m8_t x7;
+  __rvv_float64m8_t x7;
+
+  // CHECK: __rvv_int32m1_t x8;
+  __rvv_int32m1_t x8;
+
+  // CHECK: __rvv_float32m1_t x9;
+  __rvv_float32m1_t x9;
+
+  // CHECK: __rvv_int32m2_t x10;
+  __rvv_int32m2_t x10;
+
+  // CHECK: __rvv_float32m2_t x11;
+  __rvv_float32m2_t x11;
+
+  // CHECK: __rvv_int32m4_t x12;
+  __rvv_int32m4_t x12;
+
+  // CHECK: __rvv_float32m4_t x13;
+  __rvv_float32m4_t x13;
+
+  // CHECK: __rvv_int32m8_t x14;
+  __rvv_int32m8_t x14;
+
+  // CHECK: __rvv_float32m8_t x15;
+  __rvv_float32m8_t x15;
+
+  // CHECK: __rvv_int16m1_t x16;
+  __rvv_int16m1_t x16;
+
+  // CHECK: __rvv_float16m1_t x17;
+  __rvv_float16m1_t x17;
+
+  // CHECK: __rvv_int16m2_t x18;
+  __rvv_int16m2_t x18;
+
+  // CHECK: __rvv_float16m2_t x19;
+  __rvv_float16m2_t x19;
+
+  // CHECK: __rvv_int16m4_t x20;
+  __rvv_int16m4_t x20;
+
+  // CHECK: __rvv_float16m4_t x21;
+  __rvv_float16m4_t x21;
+
+  // CHECK: __rvv_int16m8_t x22;
+  __rvv_int16m8_t x22;
+
+  // CHECK: __rvv_float16m8_t x23;
+  __rvv_float16m8_t x23;
+
+  // CHECK: __rvv_int8m1_t x24;
+  __rvv_int8m1_t x24;
+
+  // CHECK: __rvv_int8m2_t x25;
+  __rvv_int8m2_t x25;
+
+  // CHECK: __rvv_int8m4_t x26;
+  __rvv_int8m4_t x26;
+
+  // CHECK: __rvv_int8m8_t x27;
+  __rvv_int8m8_t x27;
+
+  // CHECK: __rvv_bool64_t x28;
+  __rvv_bool64_t x28;
+
+  // CHECK: __rvv_bool32_t x29;
+  __rvv_bool32_t x29;
+
+  // CHECK: __rvv_bool16_t x30;
+  __rvv_bool16_t x30;
+
+  // CHECK: __rvv_bool8_t x31;
+  __rvv_bool8_t x31;
+
+  // CHECK: __rvv_bool8_t x32;
+  __rvv_bool8_t x32;
+
+  // CHECK: __rvv_bool8_t x33;
+  __rvv_bool8_t x33;
+
+  // CHECK: __rvv_bool8_t x34;
+  __rvv_bool8_t x34;
+
+  // CHECK: __rvv_int32mf2_t x35;
+  __rvv_int32mf2_t x35;
+
+  // CHECK: __rvv_float32mf2_t x36;
+  __rvv_float32mf2_t x36;
+
+  // CHECK: __rvv_int16mf4_t x37;
+  __rvv_int16mf4_t x37;
+
+  // CHECK: __rvv_float16mf4_t x38;
+  __rvv_float16mf4_t x38;
+
+  // CHECK: __rvv_int16mf2_t x39;
+  __rvv_int16mf2_t x39;
+
+  // CHECK: __rvv_float16mf2_t x40;
+  __rvv_float16mf2_t x40;
+
+  // CHECK: __rvv_int8mf8_t x41;
+  __rvv_int8mf8_t x41;
+
+  // CHECK: __rvv_int8mf4_t x42;
+  __rvv_int8mf4_t x42;
+
+  // CHECK: __rvv_int8mf2_t x43;
+  __rvv_int8mf2_t x43;
+}
Index: clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
===
--- /dev/null
+++ clan

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

2021-01-14 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai updated this revision to Diff 316553.
HsiangKai added a comment.

Refine debug info for RVV types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92715

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/RISCVVTypes.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/module.modulemap
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
  clang/test/Sema/riscv-types.c
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1548,6 +1548,8 @@
 #include "clang/Basic/AArch64SVEACLETypes.def"
 #define PPC_VECTOR_TYPE(Name, Id, Size) case BuiltinType::Id:
 #include "clang/Basic/PPCTypes.def"
+#define RVV_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/RISCVVTypes.def"
 #define BUILTIN_TYPE(Id, SingletonId)
 #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
 #define UNSIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
Index: clang/test/Sema/riscv-types.c
===
--- /dev/null
+++ clang/test/Sema/riscv-types.c
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -ast-print %s \
+// RUN:| FileCheck %s
+
+void bar(void) {
+  // CHECK: __rvv_int64m1_t x0;
+  __rvv_int64m1_t x0;
+
+  // CHECK: __rvv_float64m1_t x1;
+  __rvv_float64m1_t x1;
+
+  // CHECK: __rvv_int64m2_t x2;
+  __rvv_int64m2_t x2;
+
+  // CHECK: __rvv_float64m2_t x3;
+  __rvv_float64m2_t x3;
+
+  // CHECK: __rvv_int64m4_t x4;
+  __rvv_int64m4_t x4;
+
+  // CHECK: __rvv_float64m4_t x5;
+  __rvv_float64m4_t x5;
+
+  // CHECK: __rvv_int64m8_t x6;
+  __rvv_int64m8_t x6;
+
+  // CHECK: __rvv_float64m8_t x7;
+  __rvv_float64m8_t x7;
+
+  // CHECK: __rvv_int32m1_t x8;
+  __rvv_int32m1_t x8;
+
+  // CHECK: __rvv_float32m1_t x9;
+  __rvv_float32m1_t x9;
+
+  // CHECK: __rvv_int32m2_t x10;
+  __rvv_int32m2_t x10;
+
+  // CHECK: __rvv_float32m2_t x11;
+  __rvv_float32m2_t x11;
+
+  // CHECK: __rvv_int32m4_t x12;
+  __rvv_int32m4_t x12;
+
+  // CHECK: __rvv_float32m4_t x13;
+  __rvv_float32m4_t x13;
+
+  // CHECK: __rvv_int32m8_t x14;
+  __rvv_int32m8_t x14;
+
+  // CHECK: __rvv_float32m8_t x15;
+  __rvv_float32m8_t x15;
+
+  // CHECK: __rvv_int16m1_t x16;
+  __rvv_int16m1_t x16;
+
+  // CHECK: __rvv_float16m1_t x17;
+  __rvv_float16m1_t x17;
+
+  // CHECK: __rvv_int16m2_t x18;
+  __rvv_int16m2_t x18;
+
+  // CHECK: __rvv_float16m2_t x19;
+  __rvv_float16m2_t x19;
+
+  // CHECK: __rvv_int16m4_t x20;
+  __rvv_int16m4_t x20;
+
+  // CHECK: __rvv_float16m4_t x21;
+  __rvv_float16m4_t x21;
+
+  // CHECK: __rvv_int16m8_t x22;
+  __rvv_int16m8_t x22;
+
+  // CHECK: __rvv_float16m8_t x23;
+  __rvv_float16m8_t x23;
+
+  // CHECK: __rvv_int8m1_t x24;
+  __rvv_int8m1_t x24;
+
+  // CHECK: __rvv_int8m2_t x25;
+  __rvv_int8m2_t x25;
+
+  // CHECK: __rvv_int8m4_t x26;
+  __rvv_int8m4_t x26;
+
+  // CHECK: __rvv_int8m8_t x27;
+  __rvv_int8m8_t x27;
+
+  // CHECK: __rvv_bool64_t x28;
+  __rvv_bool64_t x28;
+
+  // CHECK: __rvv_bool32_t x29;
+  __rvv_bool32_t x29;
+
+  // CHECK: __rvv_bool16_t x30;
+  __rvv_bool16_t x30;
+
+  // CHECK: __rvv_bool8_t x31;
+  __rvv_bool8_t x31;
+
+  // CHECK: __rvv_bool8_t x32;
+  __rvv_bool8_t x32;
+
+  // CHECK: __rvv_bool8_t x33;
+  __rvv_bool8_t x33;
+
+  // CHECK: __rvv_bool8_t x34;
+  __rvv_bool8_t x34;
+
+  // CHECK: __rvv_int32mf2_t x35;
+  __rvv_int32mf2_t x35;
+
+  // CHECK: __rvv_float32mf2_t x36;
+  __rvv_float32mf2_t x36;
+
+  // CHECK: __rvv_int16mf4_t x37;
+  __rvv_int16mf4_t x37;
+
+  // CHECK: __rvv_float16mf4_t x38;
+  __rvv_float16mf4_t x38;
+
+  // CHECK: __rvv_int16mf2_t x39;
+  __rvv_int16mf2_t x39;
+
+  // CHECK: __rvv_float16mf2_t x40;
+  __rvv_float16mf2_t x40;
+
+  // CHECK: __rvv_int8mf8_t x41;
+  __rvv_int8mf8_t x41;
+
+  // CHECK: __rvv_int8mf4_t x42;
+  __rvv_int8mf4_t x42;
+
+  // CHECK: __rvv_int8mf2_t x43;
+  __rvv_int8mf2_t x43;
+}
Index: clang/test/CodeGen/RISCV/riscv-v-debuginfo.c
===
--- /de

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

Thanks for reporting that, @rupprecht. I'll look into the issue in a couple of 
days.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84673

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


[PATCH] D94601: [clang-tidy] Use DenseSet in UpgradeDurationConversionsCheck, NFCI

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

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94601

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


[PATCH] D94601: [clang-tidy] Use DenseSet in UpgradeDurationConversionsCheck, NFCI

2021-01-14 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision.
njames93 added a comment.

Seems reasonable, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94601

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


[PATCH] D94674: [clang][cli] NFC: Decrease the scope of ParseLangArgs parameters

2021-01-14 Thread 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 reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94674

Files:
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1889,7 +1889,7 @@
 
 void CompilerInvocation::setLangDefaults(LangOptions &Opts, InputKind IK,
  const llvm::Triple &T,
- PreprocessorOptions &PPOpts,
+ std::vector &Includes,
  LangStandard::Kind LangStd) {
   // Set some properties which depend solely on the input kind; it would be 
nice
   // to move these to the language standard, and have the driver resolve the
@@ -2000,9 +2000,9 @@
 if (Opts.IncludeDefaultHeader) {
   if (Opts.DeclareOpenCLBuiltins) {
 // Only include base header file for builtin types and constants.
-PPOpts.Includes.push_back("opencl-c-base.h");
+Includes.push_back("opencl-c-base.h");
   } else {
-PPOpts.Includes.push_back("opencl-c.h");
+Includes.push_back("opencl-c.h");
   }
 }
   }
@@ -2138,8 +2138,8 @@
 }
 
 static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
-  const TargetOptions &TargetOpts,
-  PreprocessorOptions &PPOpts,
+  const llvm::Triple &T,
+  std::vector &Includes,
   DiagnosticsEngine &Diags) {
   // FIXME: Cleanup per-file based stuff.
   LangStandard::Kind LangStd = LangStandard::lang_unspecified;
@@ -2212,8 +2212,7 @@
 
   Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
 
-  llvm::Triple T(TargetOpts.Triple);
-  CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangStd);
+  CompilerInvocation::setLangDefaults(Opts, IK, T, Includes, LangStd);
 
   // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0.
   // This option should be deprecated for CL > 1.0 because
@@ -2490,7 +2489,6 @@
   Diags.Report(diag::err_drv_argument_not_allowed_with)
   << A->getSpelling() << "-fdefault-calling-conv";
 else {
-  llvm::Triple T(TargetOpts.Triple);
   if (T.getArch() != llvm::Triple::x86)
 Diags.Report(diag::err_drv_argument_not_allowed_with)
 << A->getSpelling() << T.getTriple();
@@ -2527,8 +2525,7 @@
   // Add unsupported host targets here:
   case llvm::Triple::nvptx:
   case llvm::Triple::nvptx64:
-Diags.Report(diag::err_drv_omp_host_target_not_supported)
-<< TargetOpts.Triple;
+Diags.Report(diag::err_drv_omp_host_target_not_supported) << T.str();
 break;
   }
 }
@@ -2960,8 +2957,8 @@
   } else {
 // Other LangOpts are only initialized when the input is not AST or LLVM 
IR.
 // FIXME: Should we really be calling this for an Language::Asm input?
-ParseLangArgs(LangOpts, Args, DashX, Res.getTargetOpts(),
-  Res.getPreprocessorOpts(), Diags);
+ParseLangArgs(LangOpts, Args, DashX, T, Res.getPreprocessorOpts().Includes,
+  Diags);
 if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC)
   LangOpts.ObjCExceptions = 1;
 if (T.isOSDarwin() && DashX.isPreprocessed()) {
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -176,11 +176,12 @@
   /// \param Opts - The LangOptions object to set up.
   /// \param IK - The input language.
   /// \param T - The target triple.
-  /// \param PPOpts - The PreprocessorOptions affected.
+  /// \param Includes - The affected list of included files.
   /// \param LangStd - The input language standard.
-  static void setLangDefaults(LangOptions &Opts, InputKind IK,
-   const llvm::Triple &T, PreprocessorOptions &PPOpts,
-   LangStandard::Kind LangStd = 
LangStandard::lang_unspecified);
+  static void
+  setLangDefaults(LangOptions &Opts, InputKind IK, const llvm::Triple &T,
+  std::vector &Includes,
+  LangStandard::Kind LangStd = LangStandard::lang_unspecified);
 
   /// Retrieve a module hash string that is suitable for uniquely
   /// identifying the conditions under which the module was built.


Index: clang/lib/Frontend/CompilerInvocation.cpp
===

[PATCH] D94675: [clang][cli] NFC: Decrease the scope of ParseCodeGenArgs parameters

2021-01-14 Thread 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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94675

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -405,6 +405,8 @@
   llvm::Triple T(TargetOpts.Triple);
   llvm::Triple::ArchType Arch = T.getArch();
 
+  CodeGenOpts.CodeModel = TargetOpts.CodeModel;
+
   if (LangOpts.getExceptionHandling() != llvm::ExceptionHandling::None &&
   T.isWindowsMSVCEnvironment())
 Diags.Report(diag::err_fe_invalid_exception_model)
@@ -901,11 +903,9 @@
 }
 
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
- DiagnosticsEngine &Diags,
- const TargetOptions &TargetOpts,
- const FrontendOptions &FrontendOpts) {
+ DiagnosticsEngine &Diags, const llvm::Triple &T,
+ const std::string &OutputFile) {
   bool Success = true;
-  llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
   // TODO: This could be done in Driver
@@ -964,7 +964,6 @@
   llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
   llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
-  llvm::Triple T(TargetOpts.Triple);
   if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
@@ -990,8 +989,6 @@
   if (!Opts.ProfileInstrumentUsePath.empty())
 setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
 
-  Opts.CodeModel = TargetOpts.CodeModel;
-
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
 
@@ -1036,8 +1033,8 @@
   if (Arg *A = Args.getLastArg(OPT_save_temps_EQ))
 Opts.SaveTempsFilePrefix =
 llvm::StringSwitch(A->getValue())
-.Case("obj", FrontendOpts.OutputFile)
-.Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str());
+.Case("obj", OutputFile)
+.Default(llvm::sys::path::filename(OutputFile).str());
 
   // The memory profile runtime appends the pid to make this name more unique.
   const char *MemProfileBasename = "memprof.profraw";
@@ -2937,11 +2934,11 @@
   InputKind DashX = ParseFrontendArgs(Res.getFrontendOpts(), Args, Diags,
   LangOpts.IsHeaderFile);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
-  Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
-  Res.getTargetOpts(), Res.getFrontendOpts());
+  llvm::Triple T(Res.getTargetOpts().Triple);
+  Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags, T,
+  Res.getFrontendOpts().OutputFile);
   ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
 Res.getFileSystemOpts().WorkingDir);
-  llvm::Triple T(Res.getTargetOpts().Triple);
   if (DashX.getFormat() == InputKind::Precompiled ||
   DashX.getLanguage() == Language::LLVM_IR) {
 // ObjCAAutoRefCount and Sanitize LangOpts are used to setup the


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -405,6 +405,8 @@
   llvm::Triple T(TargetOpts.Triple);
   llvm::Triple::ArchType Arch = T.getArch();
 
+  CodeGenOpts.CodeModel = TargetOpts.CodeModel;
+
   if (LangOpts.getExceptionHandling() != llvm::ExceptionHandling::None &&
   T.isWindowsMSVCEnvironment())
 Diags.Report(diag::err_fe_invalid_exception_model)
@@ -901,11 +903,9 @@
 }
 
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
- DiagnosticsEngine &Diags,
- const TargetOptions &TargetOpts,
- const FrontendOptions &FrontendOpts) {
+ DiagnosticsEngine &Diags, const llvm::Triple &T,
+ const std::string &OutputFile) {
   bool Success = true;
-  llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
   // TODO: This could be done in Driver
@@ -964,7 +964,6 @@
   llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
   llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
-  llvm::Triple T(TargetOpts.Triple);
   if (Opts.OptimizationLevel > 0 && Opts.hasReduc

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

2021-01-14 Thread Nathan James via Phabricator via cfe-commits
njames93 added a reviewer: aaron.ballman.
njames93 added a comment.

Is this just flagging all these functions, if so I don't think there's much 
value in here.


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

https://reviews.llvm.org/D94621

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


[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

2021-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.

One last nit, otherwise LGTM.




Comment at: clang/unittests/Format/FormatTest.cpp:8891
+TEST_F(FormatTest, FormatsAccessModifiers) {
+  verifyFormat("struct foo {\n"
+   "private:\n"

For the ease of understanding that you test `LogicalBlock` in this first part, 
I'd add:
```
  FormatStyle Style = getLLVMStyle();
  EXPECT_EQ(Style.EmptyLineBeforeAccessModifier, 
FormatStyle::ELBAMS_LogicalBlock);
```
and use this `Style` in `verifyFormat`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93846

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


[PATCH] D92080: [Clang] Mutate long-double math builtins into f128 under IEEE-quad

2021-01-14 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM.
I think it would be useful to run a functional test with a toolchain that has 
these library functions. That shouldn't gate this change though.


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

https://reviews.llvm.org/D92080

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


[PATCH] D94678: [clang][cli] Parse & generate options necessary for LangOptions defaults manually

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94678

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
@@ -2137,6 +2137,20 @@
   llvm_unreachable("unknown input language");
 }
 
+static std::string GetOptName(llvm::opt::OptSpecifier OptSpecifier) {
+  static const OptTable &OptTable = getDriverOptTable();
+  return OptTable.getOption(OptSpecifier).getPrefixedName();
+}
+
+static void GenerateLangArgs(const LangOptions &Opts,
+ SmallVectorImpl &Args,
+ CompilerInvocation::StringAllocator SA) {
+  if (Opts.IncludeDefaultHeader)
+Args.push_back(SA(GetOptName(OPT_finclude_default_header)));
+  if (Opts.DeclareOpenCLBuiltins)
+Args.push_back(SA(GetOptName(OPT_fdeclare_opencl_builtins)));
+}
+
 static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
   const llvm::Triple &T,
   std::vector &Includes,
@@ -2212,6 +2226,10 @@
 
   Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
 
+  // These need to be parsed now. They are used to set OpenCL defaults.
+  Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
+
   CompilerInvocation::setLangDefaults(Opts, IK, T, Includes, LangStd);
 
   // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0.
@@ -3163,6 +3181,8 @@
 #undef DIAG_OPTION_WITH_MARSHALLING
 #undef OPTION_WITH_MARSHALLING
 #undef GENERATE_OPTION_WITH_MARSHALLING
+
+  GenerateLangArgs(*LangOpts, Args, SA);
 }
 
 IntrusiveRefCntPtr
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5208,11 +5208,9 @@
   NormalizedValues<["DCC_CDecl", "DCC_FastCall", "DCC_StdCall", 
"DCC_VectorCall", "DCC_RegCall"]>,
   MarshallingInfoString, "DCC_None">, 
AutoNormalizeEnum;
 def finclude_default_header : Flag<["-"], "finclude-default-header">,
-  HelpText<"Include default header file for OpenCL">,
-  MarshallingInfoFlag>;
+  HelpText<"Include default header file for OpenCL">;
 def fdeclare_opencl_builtins : Flag<["-"], "fdeclare-opencl-builtins">,
-  HelpText<"Add OpenCL builtin function declarations (experimental)">,
-  MarshallingInfoFlag>;
+  HelpText<"Add OpenCL builtin function declarations (experimental)">;
 def fpreserve_vec3_type : Flag<["-"], "fpreserve-vec3-type">,
   HelpText<"Preserve 3-component vector type">,
   MarshallingInfoFlag>;


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2137,6 +2137,20 @@
   llvm_unreachable("unknown input language");
 }
 
+static std::string GetOptName(llvm::opt::OptSpecifier OptSpecifier) {
+  static const OptTable &OptTable = getDriverOptTable();
+  return OptTable.getOption(OptSpecifier).getPrefixedName();
+}
+
+static void GenerateLangArgs(const LangOptions &Opts,
+ SmallVectorImpl &Args,
+ CompilerInvocation::StringAllocator SA) {
+  if (Opts.IncludeDefaultHeader)
+Args.push_back(SA(GetOptName(OPT_finclude_default_header)));
+  if (Opts.DeclareOpenCLBuiltins)
+Args.push_back(SA(GetOptName(OPT_fdeclare_opencl_builtins)));
+}
+
 static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
   const llvm::Triple &T,
   std::vector &Includes,
@@ -2212,6 +2226,10 @@
 
   Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
 
+  // These need to be parsed now. They are used to set OpenCL defaults.
+  Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
+
   CompilerInvocation::setLangDefaults(Opts, IK, T, Includes, LangStd);
 
   // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0.
@@ -3163,6 +3181,8 @@
 #undef DIAG_OPTION_WITH_MARSHALLING
 #undef OPTION_WITH_MARSHALLING
 #undef GENERATE_OPTION_WITH_MARSHALLING
+
+  GenerateLangArgs(*LangOpts, Args, SA);
 }
 
 IntrusiveRefCntPtr
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/cl

[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

Thank you for working on this! I think this change should have some test 
coverage in `clang-tools-extra/test/clang-query` where a test file that emits 
some output is run with and without the `--use-color` option enabled, if 
possible. IIRC, there are some clang tests that you can probably use to find 
out the right command line arguments to pass to get the ANSI escape codes to 
print out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94679: [clang][cli] NFC: Add PIE parsing for precompiled input and IR

2021-01-14 Thread 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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94679

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2970,6 +2970,7 @@
 // PIClevel and PIELevel are needed during code generation and this should 
be
 // set regardless of the input type.
 LangOpts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
+LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
   } else {


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2970,6 +2970,7 @@
 // PIClevel and PIELevel are needed during code generation and this should be
 // set regardless of the input type.
 LangOpts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
+LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94621: [clang-tidy] add concurrency-async-fs

2021-01-14 Thread Vasily Kulikov via Phabricator via cfe-commits
segoon added a comment.

njames93, the purpose is to flag it indeed. This approach was found valueable 
in Yandex.Taxi, as it is very easy to forget that you're in a coroutine and may 
not use blocking API. The bug does affect performance (e.g. all coroutine 
threads wait for fs), it cannot be found by functional tests (as it is not a 
functional invariant violation) and may be rather tricky to debug (as the 
performance harm depends on many things like IO limits, page cache size, 
current load, etc.). It can be caught during code review, but it suffers from 
human errors. Rather than playing catch-me-if-you-can games, the check can be 
automated. As C/C++ standard libraries contain quite many blocking functions 
and C++20 gains official coroutine support, I find it valuable for the C++ 
community to have an already compiled list of such blocking functions and the 
check that uses it.


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

https://reviews.llvm.org/D94621

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


[PATCH] D94680: [clang][cli] NFC: Parse some LangOpts after the defaults are set

2021-01-14 Thread 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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94680

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2196,12 +2196,6 @@
 }
   }
 
-  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
-StringRef Name = A->getValue();
-if (Name == "full" || Name == "branch") {
-  Opts.CFProtectionBranch = 1;
-}
-  }
   // -cl-std only applies for OpenCL language standards.
   // Override the -std option in this case.
   if (const Arg *A = Args.getLastArg(OPT_cl_std_EQ)) {
@@ -2224,14 +2218,21 @@
   LangStd = OpenCLLangStd;
   }
 
-  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
-
   // These need to be parsed now. They are used to set OpenCL defaults.
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
   CompilerInvocation::setLangDefaults(Opts, IK, T, Includes, LangStd);
 
+  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
+StringRef Name = A->getValue();
+if (Name == "full" || Name == "branch") {
+  Opts.CFProtectionBranch = 1;
+}
+  }
+
+  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
+
   // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0.
   // This option should be deprecated for CL > 1.0 because
   // this option was added for compatibility with OpenCL 1.0.


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2196,12 +2196,6 @@
 }
   }
 
-  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
-StringRef Name = A->getValue();
-if (Name == "full" || Name == "branch") {
-  Opts.CFProtectionBranch = 1;
-}
-  }
   // -cl-std only applies for OpenCL language standards.
   // Override the -std option in this case.
   if (const Arg *A = Args.getLastArg(OPT_cl_std_EQ)) {
@@ -2224,14 +2218,21 @@
   LangStd = OpenCLLangStd;
   }
 
-  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
-
   // These need to be parsed now. They are used to set OpenCL defaults.
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
   Opts.DeclareOpenCLBuiltins = Args.hasArg(OPT_fdeclare_opencl_builtins);
 
   CompilerInvocation::setLangDefaults(Opts, IK, T, Includes, LangStd);
 
+  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
+StringRef Name = A->getValue();
+if (Name == "full" || Name == "branch") {
+  Opts.CFProtectionBranch = 1;
+}
+  }
+
+  Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
+
   // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0.
   // This option should be deprecated for CL > 1.0 because
   // this option was added for compatibility with OpenCL 1.0.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D92277: [OpenCL] Refactor of targets OpenCL option settings

2021-01-14 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov updated this revision to Diff 316627.
azabaznov added a comment.

Changes in the latest patch:

1. Removed XFAIL and OpenCL C 2.0 test running for r600 and NVPTX target

2. Fixed comments.

3. Core features are being set in `TargetInfo::adjust`.


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

https://reviews.llvm.org/D92277

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/include/clang/Basic/OpenCLOptions.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TargetOptions.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Misc/nvptx.languageOptsOpenCL.cl
  clang/test/Misc/r600.languageOptsOpenCL.cl

Index: clang/test/Misc/r600.languageOptsOpenCL.cl
===
--- clang/test/Misc/r600.languageOptsOpenCL.cl
+++ clang/test/Misc/r600.languageOptsOpenCL.cl
@@ -2,27 +2,21 @@
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -target-cpu cayman
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cayman
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cayman
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -target-cpu cypress
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cypress
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu cypress
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -target-cpu turks
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -target-cpu turks
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -target-cpu turks
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -target-cpu turks
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu turks
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu turks
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu turks
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu turks
 
 // Extensions in all versions
 #ifndef cl_clang_storage_class_specifiers
Index: clang/test/Misc/nvptx.languageOptsOpenCL.cl
===
--- clang/test/Misc/nvptx.languageOptsOpenCL.cl
+++ clang/test/Misc/nvptx.languageOptsOpenCL.cl
@@ -2,19 +2,15 @@
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple nvptx-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple nvptx-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple nvptx-unknown-unknown
-// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple nvptx-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple nvptx-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x 

[PATCH] D94681: [clang][cli] NFC: Promote ParseLangArgs and ParseCodeGenArgs to members

2021-01-14 Thread 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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94681

Files:
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -902,9 +902,11 @@
 Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
 }
 
-static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
- DiagnosticsEngine &Diags, const llvm::Triple &T,
- const std::string &OutputFile) {
+bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
+  InputKind IK,
+  DiagnosticsEngine &Diags,
+  const llvm::Triple &T,
+  const std::string &OutputFile) {
   bool Success = true;
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
@@ -2151,10 +2153,10 @@
 Args.push_back(SA(GetOptName(OPT_fdeclare_opencl_builtins)));
 }
 
-static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
-  const llvm::Triple &T,
-  std::vector &Includes,
-  DiagnosticsEngine &Diags) {
+void CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
+   InputKind IK, const llvm::Triple &T,
+   std::vector &Includes,
+   DiagnosticsEngine &Diags) {
   // FIXME: Cleanup per-file based stuff.
   LangStandard::Kind LangStd = LangStandard::lang_unspecified;
   if (const Arg *A = Args.getLastArg(OPT_std_EQ)) {
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -247,6 +247,18 @@
   /// \returns - True if parsing was successful, false otherwise
   bool parseSimpleArgs(const llvm::opt::ArgList &Args,
DiagnosticsEngine &Diags);
+
+  /// Parse command line options that map to LangOptions.
+  static void ParseLangArgs(LangOptions &Opts, llvm::opt::ArgList &Args,
+InputKind IK, const llvm::Triple &T,
+std::vector &Includes,
+DiagnosticsEngine &Diags);
+
+  /// Parse command line options that map to CodeGenOptions.
+  static bool ParseCodeGenArgs(CodeGenOptions &Opts, llvm::opt::ArgList &Args,
+   InputKind IK, DiagnosticsEngine &Diags,
+   const llvm::Triple &T,
+   const std::string &OutputFile);
 };
 
 IntrusiveRefCntPtr


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -902,9 +902,11 @@
 Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
 }
 
-static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
- DiagnosticsEngine &Diags, const llvm::Triple &T,
- const std::string &OutputFile) {
+bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
+  InputKind IK,
+  DiagnosticsEngine &Diags,
+  const llvm::Triple &T,
+  const std::string &OutputFile) {
   bool Success = true;
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
@@ -2151,10 +2153,10 @@
 Args.push_back(SA(GetOptName(OPT_fdeclare_opencl_builtins)));
 }
 
-static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
-  const llvm::Triple &T,
-  std::vector &Includes,
-  DiagnosticsEngine &Diags) {
+void CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
+   InputKind IK, const llvm::Triple &T,
+   std::vector &Includes,
+   DiagnosticsEngine &Diags) {
   // FIXME: Cleanup per-file based stuff.
   LangStandard::Kind LangStd = LangStandard::lang_unspecified;
   if (const Arg *A = Args.getLastArg(OPT_std_EQ)) {
Index: clang/include/clang/Frontend/CompilerInvocati

[PATCH] D94682: [clang][cli] Parse Lang and CodeGen options separately

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94682

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/diagnostics-order.c

Index: clang/test/Frontend/diagnostics-order.c
===
--- clang/test/Frontend/diagnostics-order.c
+++ clang/test/Frontend/diagnostics-order.c
@@ -7,6 +7,6 @@
 //
 // CHECK:  error: invalid value '-foo' in '-verify='
 // CHECK-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
-// CHECK-NEXT: warning: optimization level '-O999' is not supported
 // CHECK-NEXT: error: invalid value 'bogus' in '-std=bogus'
 // CHECK-NEXT: note: use {{.*}} for {{.*}} standard
+// CHECK: warning: optimization level '-O999' is not supported
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -902,11 +902,27 @@
 Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
 }
 
+#define PARSE_OPTION_WITH_MARSHALLING(ARGS, DIAGS, SUCCESS, ID, FLAGS, PARAM,  \
+  SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,\
+  IMPLIED_CHECK, IMPLIED_VALUE,\
+  NORMALIZER, MERGER, TABLE_INDEX) \
+  if ((FLAGS)&options::CC1Option) {\
+KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE);  \
+if (IMPLIED_CHECK) \
+  KEYPATH = MERGER(KEYPATH, IMPLIED_VALUE);\
+if (SHOULD_PARSE)  \
+  if (auto MaybeValue =\
+  NORMALIZER(OPT_##ID, TABLE_INDEX, ARGS, DIAGS, SUCCESS)) \
+KEYPATH =  \
+MERGER(KEYPATH, static_cast(*MaybeValue));  \
+  }
+
 bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   InputKind IK,
   DiagnosticsEngine &Diags,
   const llvm::Triple &T,
-  const std::string &OutputFile) {
+  const std::string &OutputFile,
+  const LangOptions &LangOptsRef) {
   bool Success = true;
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
@@ -921,6 +937,25 @@
   }
   Opts.OptimizationLevel = OptimizationLevel;
 
+  // The key paths of codegen options defined in Options.td start with
+  // "CodeGenOpts.". Let's provide the expected variable name and type.
+  CodeGenOptions &CodeGenOpts = Opts;
+  // Some codegen options depend on language options. Let's provide the expected
+  // variable name and type.
+  const LangOptions *LangOpts = &LangOptsRef;
+
+#define CODEGEN_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)\
+  PARSE_OPTION_WITH_MARSHALLING(Args, Diags, Success, ID, FLAGS, PARAM,\
+SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,  \
+IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,  \
+MERGER, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef CODEGEN_OPTION_WITH_MARSHALLING
+
   // At O0 we want to fully disable inlining outside of cases marked with
   // 'alwaysinline' that are required for correctness.
   Opts.setInlining((Opts.OptimizationLevel == 0)
@@ -1362,21 +1397,6 @@
   return Success;
 }
 
-#define PARSE_OPTION_WITH_MARSHALLING(ARGS, DIAGS, SUCCESS, ID, FLAGS, PARAM,  \
-  SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,\
-  IMPLIED_CHECK, IMPLIED_VALUE,\
-  NORMALIZER, MERGER, TABLE_INDEX) \
-  if ((FLAGS)&options::CC1Option) {\
-KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE); 

[PATCH] D92277: [OpenCL] Refactor of targets OpenCL option settings

2021-01-14 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov marked 14 inline comments as done.
azabaznov added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:1431
 
   /// Set supported OpenCL extensions and optional core features.
+  virtual void setSupportedOpenCLOpts(const LangOptions &Opts) {}

Anastasia wrote:
> I feel this comment should get updated?
I'll change the naming in changes for OpenCL C 3.0 features.


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

https://reviews.llvm.org/D92277

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


[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-01-14 Thread Jakub Budiský via Phabricator via cfe-commits
Budovi added a comment.

In D94661#2497744 , 
@HazardyKnusperkeks wrote:

> I would add a test where you have a member before the first access modifier.

The very first unit test has no modifiers, the third one has a member before a 
modifier in the nested class C.

In D94661#2497744 , 
@HazardyKnusperkeks wrote:

> Also this is not exactly what they wanted in the bug, as far as I can see 
> members of structs or classes with no access modifier at all should only be 
> indented once.

I don't think there is a consensus about how this should actually work, see e. 
g. https://bugs.llvm.org/show_bug.cgi?id=19056#c11 .

I see qtcreator's style a bit more relevant than some astyle configuration of a 
specific project. I also find it personally ugly, can't use it myself and thus 
would have to make it configurable. In that case, this change could be viewed 
as a "possible stepping stone", even though it would probably require a major 
overhaul of the logic.

The previous patch, if I read it correctly, implements this detail in the same 
way as I did.




Comment at: clang/unittests/Format/FormatTest.cpp:17882-17908
+  {
+const char *Expected = "struct S {\n"
+   "  private:\n"
+   "class C {\n"
+   "int j;\n"
+   "\n"
+   "  public:\n"

When you re-create this test using `verifyFormat`, you get a weird results, 
i.e. if you keep the newlines before access modifiers, the test fails because 
it thinks they should not be there, but if you remove them, test fails again, 
because the formatter includes them instead.

It's a good question whether it is a side-effect of `test::messUp` or a 
different bug, I'm not sure.



Comment at: clang/unittests/Format/FormatTest.cpp:17914-17918
+  verifyFormat("enum class E {\n"
+   "  A,\n"
+   "  B\n"
+   "};\n",
+   Style);

This unit test currently fails, I believe it is a non-related bug in the 
formatter. Nevertheless I wanted to add a check that enums (non-records) are 
preserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94661

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

Can we make using color the default too? We already use colors for Decls I 
think, so this just adds colors for other Node types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

aaron.ballman wrote:
> Does it make sense to return `true` when there are no inner matchers? I would 
> have thought that that if there are no matchers, nothing would match (so we'd 
> return `false`)?
When debugging a matcher like

```
callExpr(anyOf(
hasType(pointerType()),
callee(functionDecl(hasName("foo")))
))
```

and commenting out inner matchers to get 

```
callExpr(anyOf(
#hasType(pointerType()),
#callee(functionDecl(hasName("foo")))
))
```

it would be very surprising for this to not match callExprs anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94126

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D94624#2497992 , @steveire wrote:

> Can we make using color the default too? We already use colors for Decls I 
> think, so this just adds colors for other Node types.

I thought we already did? At least, for me on Windows, the effects of the patch 
are that I can pass `--use-colors=0` to disable colors, but if I don't pass 
`--use-colors` or if I pass `--use-colors=1`, I get colors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

I also don't think we need to add unit tests for this, just because we can. The 
tests would be more complex than the code and wouldn't add much value. 
https://softwareengineering.stackexchange.com/a/147342 There are lots of 
resources about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

Doesn't this set the default to false?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

steveire wrote:
> Doesn't this set the default to false?
It does, but it does not disable colors by default (I had to double-check this 
myself), which is why I was asking for the regression tests. That will clearly 
show the behavior and help ensure we don't regress it in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[clang-tools-extra] 176f5e9 - [clang-tidy] Use DenseSet in UpgradeDurationConversionsCheck, NFCI

2021-01-14 Thread Mikhail Maltsev via cfe-commits

Author: Mikhail Maltsev
Date: 2021-01-14T13:50:16Z
New Revision: 176f5e95e1afad75ff045a00f0fa9c781bd5f54a

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

LOG: [clang-tidy] Use DenseSet in 
UpgradeDurationConversionsCheck, NFCI

This change replaces `unordered_set` (which used to store
internal representation of `SourceLocation`-s) with
`DenseSet` (which stores `SourceLocation`-s directly).

Reviewed By: aaron.ballman, njames93

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp 
b/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
index 208d1df27763..539b575d1880 100644
--- a/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
@@ -128,7 +128,7 @@ void UpgradeDurationConversionsCheck::check(
 
   if (!match(isInTemplateInstantiation(), *OuterExpr, *Result.Context)
.empty()) {
-if (MatchedTemplateLocations.count(Loc.getRawEncoding()) == 0) {
+if (MatchedTemplateLocations.count(Loc) == 0) {
   // For each location matched in a template instantiation, we check if the
   // location can also be found in `MatchedTemplateLocations`. If it is not
   // found, that means the expression did not create a match without the
@@ -144,7 +144,7 @@ void UpgradeDurationConversionsCheck::check(
   internal::Matcher IsInsideTemplate =
   hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl(;
   if (!match(IsInsideTemplate, *ArgExpr, *Result.Context).empty())
-MatchedTemplateLocations.insert(Loc.getRawEncoding());
+MatchedTemplateLocations.insert(Loc);
 
   DiagnosticBuilder Diag = diag(Loc, Message);
   CharSourceRange SourceRange = Lexer::makeFileCharRange(

diff  --git 
a/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h 
b/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
index 7a450a8e9249..23af29299f78 100644
--- a/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
+++ b/clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
@@ -11,7 +11,8 @@
 
 #include "../ClangTidyCheck.h"
 
-#include 
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -32,7 +33,7 @@ class UpgradeDurationConversionsCheck : public ClangTidyCheck 
{
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unordered_set MatchedTemplateLocations;
+  llvm::DenseSet MatchedTemplateLocations;
 };
 
 } // namespace abseil



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


[PATCH] D94601: [clang-tidy] Use DenseSet in UpgradeDurationConversionsCheck, NFCI

2021-01-14 Thread Mikhail Maltsev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG176f5e95e1af: [clang-tidy] Use 
DenseSet in UpgradeDurationConversionsCheck… (authored by 
miyuki).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94601

Files:
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h


Index: clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
+++ clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
@@ -11,7 +11,8 @@
 
 #include "../ClangTidyCheck.h"
 
-#include 
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -32,7 +33,7 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unordered_set MatchedTemplateLocations;
+  llvm::DenseSet MatchedTemplateLocations;
 };
 
 } // namespace abseil
Index: clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
@@ -128,7 +128,7 @@
 
   if (!match(isInTemplateInstantiation(), *OuterExpr, *Result.Context)
.empty()) {
-if (MatchedTemplateLocations.count(Loc.getRawEncoding()) == 0) {
+if (MatchedTemplateLocations.count(Loc) == 0) {
   // For each location matched in a template instantiation, we check if the
   // location can also be found in `MatchedTemplateLocations`. If it is not
   // found, that means the expression did not create a match without the
@@ -144,7 +144,7 @@
   internal::Matcher IsInsideTemplate =
   hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl(;
   if (!match(IsInsideTemplate, *ArgExpr, *Result.Context).empty())
-MatchedTemplateLocations.insert(Loc.getRawEncoding());
+MatchedTemplateLocations.insert(Loc);
 
   DiagnosticBuilder Diag = diag(Loc, Message);
   CharSourceRange SourceRange = Lexer::makeFileCharRange(


Index: clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
+++ clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
@@ -11,7 +11,8 @@
 
 #include "../ClangTidyCheck.h"
 
-#include 
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -32,7 +33,7 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unordered_set MatchedTemplateLocations;
+  llvm::DenseSet MatchedTemplateLocations;
 };
 
 } // namespace abseil
Index: clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
@@ -128,7 +128,7 @@
 
   if (!match(isInTemplateInstantiation(), *OuterExpr, *Result.Context)
.empty()) {
-if (MatchedTemplateLocations.count(Loc.getRawEncoding()) == 0) {
+if (MatchedTemplateLocations.count(Loc) == 0) {
   // For each location matched in a template instantiation, we check if the
   // location can also be found in `MatchedTemplateLocations`. If it is not
   // found, that means the expression did not create a match without the
@@ -144,7 +144,7 @@
   internal::Matcher IsInsideTemplate =
   hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl(;
   if (!match(IsInsideTemplate, *ArgExpr, *Result.Context).empty())
-MatchedTemplateLocations.insert(Loc.getRawEncoding());
+MatchedTemplateLocations.insert(Loc);
 
   DiagnosticBuilder Diag = diag(Loc, Message);
   CharSourceRange SourceRange = Lexer::makeFileCharRange(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

aaron.ballman wrote:
> steveire wrote:
> > Doesn't this set the default to false?
> It does, but it does not disable colors by default (I had to double-check 
> this myself), which is why I was asking for the regression tests. That will 
> clearly show the behavior and help ensure we don't regress it in the future.
Did you build and run with the patch? 

Current `clang-query` dumps with color. After this patch, when not using the 
command line option, there is now no color. with `-use-color`, the color is 
restored.

@tomrittervg I know this is based on our discussions, but it's not clear to me 
what the intent of the patch is. Do you want to be able to disable color?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Tom Ritter via Phabricator via cfe-commits
tomrittervg added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > Doesn't this set the default to false?
> > It does, but it does not disable colors by default (I had to double-check 
> > this myself), which is why I was asking for the regression tests. That will 
> > clearly show the behavior and help ensure we don't regress it in the future.
> Did you build and run with the patch? 
> 
> Current `clang-query` dumps with color. After this patch, when not using the 
> command line option, there is now no color. with `-use-color`, the color is 
> restored.
> 
> @tomrittervg I know this is based on our discussions, but it's not clear to 
> me what the intent of the patch is. Do you want to be able to disable color?
Let me double check - my current clang-query did not dump with color (which is 
why I made the patch). Maybe this was a mix-up I had.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D78105: [CSInfo][ISEL] Call site info generation support for Mips

2021-01-14 Thread Nikola Tesic via Phabricator via cfe-commits
ntesic added a comment.

In D78105#2487169 , @nickdesaulniers 
wrote:

> Compiler crash reported in: https://bugs.llvm.org/show_bug.cgi?id=48695

@nickdesaulniers Thanks for pointing this out!
Fix patch proposal at D94685 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78105

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

tomrittervg wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > steveire wrote:
> > > > Doesn't this set the default to false?
> > > It does, but it does not disable colors by default (I had to double-check 
> > > this myself), which is why I was asking for the regression tests. That 
> > > will clearly show the behavior and help ensure we don't regress it in the 
> > > future.
> > Did you build and run with the patch? 
> > 
> > Current `clang-query` dumps with color. After this patch, when not using 
> > the command line option, there is now no color. with `-use-color`, the 
> > color is restored.
> > 
> > @tomrittervg I know this is based on our discussions, but it's not clear to 
> > me what the intent of the patch is. Do you want to be able to disable color?
> Let me double check - my current clang-query did not dump with color (which 
> is why I made the patch). Maybe this was a mix-up I had.
https://reviews.llvm.org/D62056 already made dumps use color.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94682: [clang][cli] Parse Lang and CodeGen options separately

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 updated this revision to Diff 316643.
jansvoboda11 added a comment.

Stop setting the default for LaxVectorConversions in CompilerInvocation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94682

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/diagnostics-order.c

Index: clang/test/Frontend/diagnostics-order.c
===
--- clang/test/Frontend/diagnostics-order.c
+++ clang/test/Frontend/diagnostics-order.c
@@ -7,6 +7,6 @@
 //
 // CHECK:  error: invalid value '-foo' in '-verify='
 // CHECK-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
-// CHECK-NEXT: warning: optimization level '-O999' is not supported
 // CHECK-NEXT: error: invalid value 'bogus' in '-std=bogus'
 // CHECK-NEXT: note: use {{.*}} for {{.*}} standard
+// CHECK: warning: optimization level '-O999' is not supported
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -902,11 +902,27 @@
 Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
 }
 
+#define PARSE_OPTION_WITH_MARSHALLING(ARGS, DIAGS, SUCCESS, ID, FLAGS, PARAM,  \
+  SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,\
+  IMPLIED_CHECK, IMPLIED_VALUE,\
+  NORMALIZER, MERGER, TABLE_INDEX) \
+  if ((FLAGS)&options::CC1Option) {\
+KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE);  \
+if (IMPLIED_CHECK) \
+  KEYPATH = MERGER(KEYPATH, IMPLIED_VALUE);\
+if (SHOULD_PARSE)  \
+  if (auto MaybeValue =\
+  NORMALIZER(OPT_##ID, TABLE_INDEX, ARGS, DIAGS, SUCCESS)) \
+KEYPATH =  \
+MERGER(KEYPATH, static_cast(*MaybeValue));  \
+  }
+
 bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   InputKind IK,
   DiagnosticsEngine &Diags,
   const llvm::Triple &T,
-  const std::string &OutputFile) {
+  const std::string &OutputFile,
+  const LangOptions &LangOptsRef) {
   bool Success = true;
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
@@ -921,6 +937,25 @@
   }
   Opts.OptimizationLevel = OptimizationLevel;
 
+  // The key paths of codegen options defined in Options.td start with
+  // "CodeGenOpts.". Let's provide the expected variable name and type.
+  CodeGenOptions &CodeGenOpts = Opts;
+  // Some codegen options depend on language options. Let's provide the expected
+  // variable name and type.
+  const LangOptions *LangOpts = &LangOptsRef;
+
+#define CODEGEN_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)\
+  PARSE_OPTION_WITH_MARSHALLING(Args, Diags, Success, ID, FLAGS, PARAM,\
+SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,  \
+IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,  \
+MERGER, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef CODEGEN_OPTION_WITH_MARSHALLING
+
   // At O0 we want to fully disable inlining outside of cases marked with
   // 'alwaysinline' that are required for correctness.
   Opts.setInlining((Opts.OptimizationLevel == 0)
@@ -1362,21 +1397,6 @@
   return Success;
 }
 
-#define PARSE_OPTION_WITH_MARSHALLING(ARGS, DIAGS, SUCCESS, ID, FLAGS, PARAM,  \
-  SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,\
-  IMPLIED_CHECK, IMPLIED_VALUE,\
-  NORMALIZER, MERGER, TABLE_INDEX) \
-  if ((FLAGS)&options::CC1Option) {\
-KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE); 

[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Tom Ritter via Phabricator via cfe-commits
tomrittervg added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

steveire wrote:
> tomrittervg wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > steveire wrote:
> > > > > Doesn't this set the default to false?
> > > > It does, but it does not disable colors by default (I had to 
> > > > double-check this myself), which is why I was asking for the regression 
> > > > tests. That will clearly show the behavior and help ensure we don't 
> > > > regress it in the future.
> > > Did you build and run with the patch? 
> > > 
> > > Current `clang-query` dumps with color. After this patch, when not using 
> > > the command line option, there is now no color. with `-use-color`, the 
> > > color is restored.
> > > 
> > > @tomrittervg I know this is based on our discussions, but it's not clear 
> > > to me what the intent of the patch is. Do you want to be able to disable 
> > > color?
> > Let me double check - my current clang-query did not dump with color (which 
> > is why I made the patch). Maybe this was a mix-up I had.
> https://reviews.llvm.org/D62056 already made dumps use color.
Is that what you're running in your CE instance? You're right, I forgot about 
that; and that does make it output color on the terminal. But not CE.

I _think_ what's happening is that CE isn't acting like a tty, so clang won't 
output it in color even if I specify `--extra-arg="-fdiagnostics-color"` (which 
the analogous `--extra-arg="-fno-diagnostics-color"` _will_ turn off color on 
the command-line.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

tomrittervg wrote:
> steveire wrote:
> > tomrittervg wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > steveire wrote:
> > > > > > Doesn't this set the default to false?
> > > > > It does, but it does not disable colors by default (I had to 
> > > > > double-check this myself), which is why I was asking for the 
> > > > > regression tests. That will clearly show the behavior and help ensure 
> > > > > we don't regress it in the future.
> > > > Did you build and run with the patch? 
> > > > 
> > > > Current `clang-query` dumps with color. After this patch, when not 
> > > > using the command line option, there is now no color. with 
> > > > `-use-color`, the color is restored.
> > > > 
> > > > @tomrittervg I know this is based on our discussions, but it's not 
> > > > clear to me what the intent of the patch is. Do you want to be able to 
> > > > disable color?
> > > Let me double check - my current clang-query did not dump with color 
> > > (which is why I made the patch). Maybe this was a mix-up I had.
> > https://reviews.llvm.org/D62056 already made dumps use color.
> Is that what you're running in your CE instance? You're right, I forgot about 
> that; and that does make it output color on the terminal. But not CE.
> 
> I _think_ what's happening is that CE isn't acting like a tty, so clang won't 
> output it in color even if I specify `--extra-arg="-fdiagnostics-color"` 
> (which the analogous `--extra-arg="-fno-diagnostics-color"` _will_ turn off 
> color on the command-line.)
Yes, you're probably right that it's tty related. 

It looks to me like the only problem is that you specified the default as 
`false` instead of `true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D93453: [flang][driver] Add support for `-I`

2021-01-14 Thread sameeran joshi via Phabricator via cfe-commits
sameeranjoshi added inline comments.



Comment at: flang/test/Flang-Driver/include-header.f90:38
+!-
+! EXPECTED OUTPUT FOR /Inputs/ FOLDER SPECIFIED FIRST
+!-

Nit - Shouldn't this be `Inputs/` starting with a `/` changes the meaning(at 
least on Linux systems) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93453

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


[clang] adb77a7 - [OpenCL] Improve online documentation.

2021-01-14 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-01-14T14:56:10Z
New Revision: adb77a7456920a46908c7e20b2d3008789274975

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

LOG: [OpenCL] Improve online documentation.

Update UsersManual and OpenCLSupport pages to reflect
recent functionality i.e. SPIR-V generation,
C++ for OpenCL, OpenCL 3.0 development plans.

Tags: #clang

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

Added: 


Modified: 
clang/docs/OpenCLSupport.rst
clang/docs/UsersManual.rst

Removed: 




diff  --git a/clang/docs/OpenCLSupport.rst b/clang/docs/OpenCLSupport.rst
index b00a9ef41064..9c17bd8f2692 100644
--- a/clang/docs/OpenCLSupport.rst
+++ b/clang/docs/OpenCLSupport.rst
@@ -17,34 +17,92 @@
 OpenCL Support
 ==
 
-Clang fully supports all OpenCL C versions from 1.1 to 2.0.
+Clang has complete support of OpenCL C versions from 1.0 to 2.0.
 
-Please refer to `Bugzilla
-`__
-for the most up to date bug reports.
+Clang also supports :ref:`the C++ for OpenCL kernel language 
`.
 
+There is an ongoing work to support :ref:`OpenCL 3.0 `.
+
+There are also other :ref:`new and experimental features ` 
available.
+
+For general issues and bugs with OpenCL in clang refer to `Bugzilla
+`__.
+
+.. _cxx_for_opencl_impl:
 
 C++ for OpenCL Implementation Status
 
 
-Bugzilla bugs for this functionality are typically prefixed
-with '[C++]'.
+Clang implements language version 1.0 published in `the official
+release of C++ for OpenCL Documentation
+`_.
 
-Differences to OpenCL C

+Limited support of experimental C++ libraries is described in the 
`experimental features `.
+
+Bugzilla bugs for this functionality are typically prefixed
+with '[C++4OpenCL]' - click `here
+`_
+to view the full bug list.
 
-TODO!
 
 Missing features or with limited support
 
 
-- Use of ObjC blocks is disabled.
-
-- Global destructor invocation is not generated correctly.
-
-- Initialization of objects in `__constant` address spaces is not guaranteed 
to work.
-
-- `addrspace_cast` operator is not supported.
+- Use of ObjC blocks is disabled and therefore the ``enqueue_kernel`` builtin
+  function is not supported currently. It is expected that if support for this
+  feature is added in the future, it will utilize C++ lambdas instead of ObjC
+  blocks.
+
+- IR generation for global destructors is incomplete (See:
+  `PR48047 `_).
+
+- There is no distinct file extension for sources that are to be compiled
+  in C++ for OpenCL mode (See: `PR48097 `_)
+
+.. _opencl_300:
+
+OpenCL 3.0 Implementation Status
+
+
+The following table provides an overview of features in OpenCL C 3.0 and their
+implementation status. 
+
++--+--+--+---
+
+| Category | Feature   
   | Status   | Reviews 
  |
++==+==+==+===
+
+| Command line interface   | New value for ``-cl-std`` flag
   | :good:`done` | https://reviews.llvm.org/D88300 
  |
++--+--+--+---
+
+| Predefined macros| New version macro 
   | :good:`done` | https://reviews.llvm.org/D88300 
  |
++--+--+--+---
+
+| Predefined macros| Feature macros  

[PATCH] D93942: [OpenCL] Improve online documentation.

2021-01-14 Thread Anastasia Stulova via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGadb77a745692: [OpenCL] Improve online documentation. 
(authored by Anastasia).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D93942?vs=315447&id=316648#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93942

Files:
  clang/docs/OpenCLSupport.rst
  clang/docs/UsersManual.rst

Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -41,7 +41,8 @@
variants depending on base language.
 -  :ref:`C++ Language `
 -  :ref:`Objective C++ Language `
--  :ref:`OpenCL C Language `: v1.0, v1.1, v1.2, v2.0.
+-  :ref:`OpenCL Kernel Language `: OpenCL C v1.0, v1.1, v1.2, v2.0,
+   plus C++ for OpenCL.
 
 In addition to these base languages and their dialects, Clang supports a
 broad variety of language extensions, which are documented in the
@@ -2796,8 +2797,8 @@
 ===
 
 Clang can be used to compile OpenCL kernels for execution on a device
-(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMD or
-Nvidia targets) that can be uploaded to run directly on a device (e.g. using
+(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMDGPU)
+that can be uploaded to run directly on a device (e.g. using
 `clCreateProgramWithBinary
 `_) or
 into generic bitcode files loadable into other toolchains.
@@ -2824,13 +2825,26 @@
 
  $ clang -c -emit-llvm test.cl
 
-This will produce a generic test.bc file that can be used in vendor toolchains
+This will produce a file `test.bc` that can be used in vendor toolchains
 to perform machine code generation.
 
-Clang currently supports OpenCL C language standards up to v2.0. Starting from
-clang 9 a C++ mode is available for OpenCL (see
+Note that if compiled to bitcode for generic targets such as SPIR,
+portable IR is produced that can be used with various vendor
+tools as well as open source tools such as `SPIRV-LLVM Translator
+`_
+to produce SPIR-V binary.
+
+
+Clang currently supports OpenCL C language standards up to v2.0. Clang mainly
+supports full profile. There is only very limited support of the embedded
+profile. 
+Starting from clang 9 a C++ mode is available for OpenCL (see
 :ref:`C++ for OpenCL `).
 
+There is ongoing support for OpenCL v3.0 that is documented along with other
+experimental functionality and features in development on :doc:`OpenCLSupport`
+page.
+
 OpenCL Specific Options
 ---
 
@@ -2847,24 +2861,31 @@
 
 .. option:: -finclude-default-header
 
-Loads standard includes during compilations. By default OpenCL headers are not
-loaded and therefore standard library includes are not available. To load them
-automatically a flag has been added to the frontend (see also :ref:`the section
-on the OpenCL Header `):
+Adds most of builtin types and function declarations during compilations. By
+default the OpenCL headers are not loaded and therefore certain builtin
+types and most of builtin functions are not declared. To load them
+automatically this flag can be passed to the frontend (see also :ref:`the
+section on the OpenCL Header `):
 
.. code-block:: console
 
  $ clang -Xclang -finclude-default-header test.cl
 
-Alternatively ``-include`` or ``-I`` followed by the path to the header location
-can be given manually.
+Note that this is a frontend-only flag and therefore it requires the use of
+flags that forward options to the frontend, e.g. ``-cc1`` or ``-Xclang``.
+
+Alternatively the internal header `opencl-c.h` containing the declarations
+can be included manually using ``-include`` or ``-I`` followed by the path
+to the header location. The header can be found in the clang source tree or
+installation directory.
 
.. code-block:: console
 
- $ clang -I/lib/Headers/opencl-c.h test.cl
+ $ clang -I/lib/Headers/opencl-c.h test.cl
+ $ clang -I/lib/clang//include/opencl-c.h/opencl-c.h test.cl
 
-In this case the kernel code should contain ``#include `` just as a
-regular C include.
+In this example it is assumed that the kernel code contains
+``#include `` just as a regular C include.
 
 .. _opencl_cl_ext:
 
@@ -2874,10 +2895,14 @@
 of extensions that they support. Clang allows to amend this using the ``-cl-ext``
 flag with a comma-separated list of extensions prefixed with ``'+'`` or ``'-'``.
 The syntax: ``-cl-ext=<(['-'|'+'][,])+>``,  where extensions
-can be either one of `the OpenCL specification extensions
-`_
-or any known vendor extension. Alternatively, ``'all'`` can be used to enable
+can be either one of `the OpenCL published extensions
+

[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+

tomrittervg wrote:
> steveire wrote:
> > tomrittervg wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > steveire wrote:
> > > > > > Doesn't this set the default to false?
> > > > > It does, but it does not disable colors by default (I had to 
> > > > > double-check this myself), which is why I was asking for the 
> > > > > regression tests. That will clearly show the behavior and help ensure 
> > > > > we don't regress it in the future.
> > > > Did you build and run with the patch? 
> > > > 
> > > > Current `clang-query` dumps with color. After this patch, when not 
> > > > using the command line option, there is now no color. with 
> > > > `-use-color`, the color is restored.
> > > > 
> > > > @tomrittervg I know this is based on our discussions, but it's not 
> > > > clear to me what the intent of the patch is. Do you want to be able to 
> > > > disable color?
> > > Let me double check - my current clang-query did not dump with color 
> > > (which is why I made the patch). Maybe this was a mix-up I had.
> > https://reviews.llvm.org/D62056 already made dumps use color.
> Is that what you're running in your CE instance? You're right, I forgot about 
> that; and that does make it output color on the terminal. But not CE.
> 
> I _think_ what's happening is that CE isn't acting like a tty, so clang won't 
> output it in color even if I specify `--extra-arg="-fdiagnostics-color"` 
> (which the analogous `--extra-arg="-fno-diagnostics-color"` _will_ turn off 
> color on the command-line.)
> Did you build and run with the patch?

Yes.

> Current clang-query dumps with color. After this patch, when not using the 
> command line option, there is now no color. with -use-color, the color is 
> restored.

I think we're looking at different parts of the output. Diagnostic text and 
underlines are always colored for me, regardless of what options I pass. AST 
dump output only gets controlled by this new flag, which is different from how 
it works in clang-tidy, where `--use-color=0` will disable colors from 
diagnostic text output as well.

So I think there are two issues: 1) This isn't controlling all of the colors, 
2) The default on/off value is no longer tied to whether the output is able to 
display colors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

steveire wrote:
> aaron.ballman wrote:
> > Does it make sense to return `true` when there are no inner matchers? I 
> > would have thought that that if there are no matchers, nothing would match 
> > (so we'd return `false`)?
> When debugging a matcher like
> 
> ```
> callExpr(anyOf(
> hasType(pointerType()),
> callee(functionDecl(hasName("foo")))
> ))
> ```
> 
> and commenting out inner matchers to get 
> 
> ```
> callExpr(anyOf(
> #hasType(pointerType()),
> #callee(functionDecl(hasName("foo")))
> ))
> ```
> 
> it would be very surprising for this to not match callExprs anymore.
On the one hand, I see what you're saying. On the other hand, I think the 
behavior actually is surprising to some folks (like me). For instance, 
`std::any_of()` returns `false` when the range is empty, while `std::all_of()` 
returns `true`.

To be conservative with the change, rather than allowing zero matchers with 
potentially surprising behavior, we could require there be at least one 
matcher. In that case, if you want to comment out all of the inner matchers, 
you need to comment out the variadic one as well. e.g.,
```
# This is an error
callExpr(anyOf(
#hasType(pointerType()),
#callee(functionDecl(hasName("foo")))
))

# Do this instead
callExpr(
# anyOf(
#hasType(pointerType()),
#callee(functionDecl(hasName("foo")))
# )
)
```
It's a bit more commenting for the person experimenting, but it's also reduces 
the chances for surprising behavior. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94126

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query

2021-01-14 Thread Tom Ritter via Phabricator via cfe-commits
tomrittervg updated this revision to Diff 316652.
tomrittervg added a comment.

Actually, I think I need to be smarter than changing the default. We want to 
let clang auto-detect the tty and behave that way by default if the option 
isn't specified. Otherwise you'd get ASNI color codes when you pipe to a file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

Files:
  clang-tools-extra/clang-query/Query.cpp
  clang-tools-extra/clang-query/QuerySession.h
  clang-tools-extra/clang-query/tool/ClangQuery.cpp


Index: clang-tools-extra/clang-query/tool/ClangQuery.cpp
===
--- clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -49,6 +49,14 @@
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 static cl::OptionCategory ClangQueryCategory("clang-query options");
 
+static cl::opt
+UseColor("use-color",
+ cl::desc(
+ R"(Use colors in detailed AST output. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+
 static cl::list Commands("c", cl::desc("Specify command to run"),
   cl::value_desc("command"),
   cl::cat(ClangQueryCategory));
@@ -124,7 +132,12 @@
 assert(Status == 0 && "Unexpected status returned");
   }
 
-  QuerySession QS(ASTs);
+  // If a command-line switch is not specified, let clang auto-detect a tty and
+  // behave that way
+  bool useColor = UseColor.getNumOccurrences() > 0
+  ? UseColor
+  : SM.getDiagnostics().getShowColors();
+  QuerySession QS(ASTs, useColor);
 
   if (!Commands.empty()) {
 for (auto I = Commands.begin(), E = Commands.end(); I != E; ++I) {
Index: clang-tools-extra/clang-query/QuerySession.h
===
--- clang-tools-extra/clang-query/QuerySession.h
+++ clang-tools-extra/clang-query/QuerySession.h
@@ -23,9 +23,9 @@
 /// Represents the state for a particular clang-query session.
 class QuerySession {
 public:
-  QuerySession(llvm::ArrayRef> ASTs)
+  QuerySession(llvm::ArrayRef> ASTs, bool UseColor)
   : ASTs(ASTs), PrintOutput(false), DiagOutput(true),
-DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
+DetailedASTOutput(false), ColorOutput(UseColor), BindRoot(true), 
PrintMatcher(false),
 Terminate(false), TK(ast_type_traits::TK_AsIs) {}
 
   llvm::ArrayRef> ASTs;
@@ -34,6 +34,8 @@
   bool DiagOutput;
   bool DetailedASTOutput;
 
+  bool ColorOutput;
+
   bool BindRoot;
   bool PrintMatcher;
   bool Terminate;
Index: clang-tools-extra/clang-query/Query.cpp
===
--- clang-tools-extra/clang-query/Query.cpp
+++ clang-tools-extra/clang-query/Query.cpp
@@ -157,7 +157,7 @@
   OS << "Binding for \"" << BI->first << "\":\n";
   const ASTContext &Ctx = AST->getASTContext();
   const SourceManager &SM = Ctx.getSourceManager();
-  ASTDumper Dumper(OS, Ctx, SM.getDiagnostics().getShowColors());
+  ASTDumper Dumper(OS, Ctx, QS.ColorOutput);
   Dumper.SetTraversalKind(QS.TK);
   Dumper.Visit(BI->second);
   OS << "\n";


Index: clang-tools-extra/clang-query/tool/ClangQuery.cpp
===
--- clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -49,6 +49,14 @@
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 static cl::OptionCategory ClangQueryCategory("clang-query options");
 
+static cl::opt
+UseColor("use-color",
+ cl::desc(
+ R"(Use colors in detailed AST output. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.)"),
+ cl::init(false), cl::cat(ClangQueryCategory));
+
 static cl::list Commands("c", cl::desc("Specify command to run"),
   cl::value_desc("command"),
   cl::cat(ClangQueryCategory));
@@ -124,7 +132,12 @@
 assert(Status == 0 && "Unexpected status returned");
   }
 
-  QuerySession QS(ASTs);
+  // If a command-line switch is not specified, let clang auto-detect a tty and
+  // behave that way
+  bool useColor = UseColor.getNumOccurrences() > 0
+  ? UseColor
+  : SM.getDiagnostics().getShowColors();
+  QuerySession QS(ASTs, useColor);
 
   if (!Commands.empty()) {
 for (auto I = Commands.begin(), E = Commands.end(); I != E; ++I) {
Index: clang-tools-extra/clang-query/QuerySession.h

[PATCH] D92277: [OpenCL] Refactor of targets OpenCL option settings

2021-01-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

LGTM about r600 changes. Thanks.


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

https://reviews.llvm.org/D92277

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


[PATCH] D82862: [ThinLTO] Always parse module level inline asm with At&t dialect

2021-01-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment.
Herald added a subscriber: pengfei.

Hi @hans , we're having some issues with using the AssemblerDialect mechanism 
to support both the GNU assembler and the IBM HLASM assembler in the SystemZ 
back-end. See also the discussion started here: 
https://lists.llvm.org/pipermail/llvm-dev/2020-November/146875.html

One of the issues that showed up is what you're refering to above:

> That flag however should not affect the *parsing* of inline assembly in the 
> program.

I'm wondering why this is true.  I mean, it is true that the flag currently 
does not affect parsing of inline asm, but I'm wondering whether it *should* be 
that way.

Note that the `-x86-asm-syntax=intel` LLVM flag is used to implement the 
`-masm=intel` clang **command line option**, which exists also in GCC and where 
hopefully the two compilers should be compatible.  But in GCC, that flag is 
documented to affect parsing of inline assembly just like it affects output.   
See https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/x86-Options.html#x86-Options

> -masm=dialect
>
>   Output assembly instructions using selected dialect. Also affects which 
> dialect is used for basic asm (see Basic Asm) and extended asm (see Extended 
> Asm). Supported choices (in dialect order) are ‘att’ or ‘intel’. The default 
> is ‘att’. Darwin does not support ‘intel’.

What is the reason for treating this differently in LLVM?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82862

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query to allow forcing the behavior

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D94624#2498201 , @tomrittervg wrote:

> Actually, I think I need to be smarter than changing the default. We want to 
> let clang auto-detect the tty and behave that way by default if the option 
> isn't specified. Otherwise you'd get ASNI color codes when you pipe to a file.

+1 to this, but also, you need to thread the option through to the diagnostics 
engine as well.

For reference, here are some screenshots of what I'm seeing on Windows with 
your original patch applied when running clang-query and clang-tidy. Note the 
difference in diagnostic text colorization where clang-tidy controls it via 
`use-color` and clang-query currently does not.

F15036446: tidy-no-opt.PNG 

F15036445: query-explicit-on.PNG 

F15036444: tidy-explicit-on.PNG 

F15036443: tidy-explicit-off.PNG 

F15036442: query-explicit-off.PNG 

F15036441: query-no-opt.PNG 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94624: [PATCH] [clang-query] Add a --use-color option to clang-query to allow forcing the behavior

2021-01-14 Thread Tom Ritter via Phabricator via cfe-commits
tomrittervg added a comment.

In D94624#2498244 , @aaron.ballman 
wrote:

> In D94624#2498201 , @tomrittervg 
> wrote:
>
>> Actually, I think I need to be smarter than changing the default. We want to 
>> let clang auto-detect the tty and behave that way by default if the option 
>> isn't specified. Otherwise you'd get ASNI color codes when you pipe to a 
>> file.
>
> +1 to this, but also, you need to thread the option through to the 
> diagnostics engine as well.

Let me fiddle with this, I think I have an idea of how to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94624

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


[PATCH] D94621: [clang-tidy] add concurrency-async-fs

2021-01-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst:6
+
+Search for filesystem accesses that might block current system thread.
+Asynchronous code may deal with it in numerous ways, the most widespread

Please make first statement same as in Release Notes.



Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst:10
+
+* use asynchronous API like aio or io_uring
+* delegate all filesystem access to a thread pool

Please highlight aio and io_uring with single back-ticks.



Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst:15
+a non-issue, e.g. it is known that all filesystem code doesn't block
+system threads as it resides in memory (e.g. tmpfs), or blocking functions
+are used in the startup code only, or asynchronous API/thread pool is not

Please highlight tmpfs with single back-ticks.



Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst:32
+
+  Specifies additional functions to search for, separated with semicolon.
+

Please add information about default value. Same below.


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

https://reviews.llvm.org/D94621

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


[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

aaron.ballman wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > Does it make sense to return `true` when there are no inner matchers? I 
> > > would have thought that that if there are no matchers, nothing would 
> > > match (so we'd return `false`)?
> > When debugging a matcher like
> > 
> > ```
> > callExpr(anyOf(
> > hasType(pointerType()),
> > callee(functionDecl(hasName("foo")))
> > ))
> > ```
> > 
> > and commenting out inner matchers to get 
> > 
> > ```
> > callExpr(anyOf(
> > #hasType(pointerType()),
> > #callee(functionDecl(hasName("foo")))
> > ))
> > ```
> > 
> > it would be very surprising for this to not match callExprs anymore.
> On the one hand, I see what you're saying. On the other hand, I think the 
> behavior actually is surprising to some folks (like me). For instance, 
> `std::any_of()` returns `false` when the range is empty, while 
> `std::all_of()` returns `true`.
> 
> To be conservative with the change, rather than allowing zero matchers with 
> potentially surprising behavior, we could require there be at least one 
> matcher. In that case, if you want to comment out all of the inner matchers, 
> you need to comment out the variadic one as well. e.g.,
> ```
> # This is an error
> callExpr(anyOf(
> #hasType(pointerType()),
> #callee(functionDecl(hasName("foo")))
> ))
> 
> # Do this instead
> callExpr(
> # anyOf(
> #hasType(pointerType()),
> #callee(functionDecl(hasName("foo")))
> # )
> )
> ```
> It's a bit more commenting for the person experimenting, but it's also 
> reduces the chances for surprising behavior. WDYT?
> On the one hand, I see what you're saying. On the other hand, I think the 
> behavior actually is surprising to some folks (like me). For instance, 
> `std::any_of()` returns `false` when the range is empty, while 
> `std::all_of()` returns `true`.

Yes, I know this diverges from those. However, I think the semantic in this 
patch is the semantic that makes sense for AST Matchers. This patch prioritizes 
usefulness and consistency in the context of writing and debugging AST Matchers 
instead of prioritizing consistency with `std::any_of` (which would be 
non-useful and surprising in the context of debugging an AST Matcher).

> if you want to comment out all of the inner matchers, you need to comment out 
> the variadic one as well

Yes, that's what I'm trying to make unnecessary.

> It's a bit more commenting for the person experimenting, but it's also 
> reduces the chances for surprising behavior. WDYT?

I think your suggestion leaves zero-arg `anyOf` matcher inconsistent with 
`allOf` matcher (fails to compile versus does something useful).





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94126

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


[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

2021-01-14 Thread Albertas Vyšniauskas via Phabricator via cfe-commits
thezbyg updated this revision to Diff 316660.
thezbyg added a comment.

Improved default style tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93846

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/test/Format/access-modifiers.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8887,6 +8887,299 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, FormatsAccessModifiers) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.EmptyLineBeforeAccessModifier,
+FormatStyle::ELBAMS_LogicalBlock);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo { /* comment */\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  Style = getLLVMStyle();
+  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never;
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "private:\n"
+   "  int i;\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo { /* comment */\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo { /* comment */\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "  // comment\n"
+   "\n"
+   "private:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   "struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "#ifdef FOO\n"
+   "\n"
+   "private:\n"
+   "#endif\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n"
+   "  int j;\n"
+   "};\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  void f() {}\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "\n"
+   "protected:\n

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

2021-01-14 Thread Vasily Kulikov via Phabricator via cfe-commits
segoon updated this revision to Diff 316664.
segoon edited the summary of this revision.
segoon added a comment.

- fix the first document line
- add default values


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

https://reviews.llvm.org/D94622

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.h
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-async-no-new-threads.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.c
  
clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-no-new-threads.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s concurrency-async-no-new-threads %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-no-new-threads.FunctionsExtra", value: "::my::create_thread"},{key: "concurrency-async-no-new-threads.TypesExtra", value: "::my::thread"}]}'
+
+namespace std {
+
+class thread {
+public:
+  void join() {}
+};
+
+class jthread {};
+
+class nothread {};
+
+namespace execution {
+
+class sequenced_policy {};
+
+class parallel_policy {};
+
+class parallel_unsequenced_policy {};
+
+class unsequenced_policy {};
+
+sequenced_policy seq;
+parallel_policy par;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+parallel_unsequenced_policy par_unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'parallel_unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+unsequenced_policy unseq;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: type 'unsequenced_policy' creates new threads [concurrency-async-no-new-threads]
+
+} // namespace execution
+
+void async(int (*)()) {
+}
+
+template 
+void sort(T1 &&, T2, T3, T4);
+
+} // namespace std
+
+int pthread_create(int *thread, const int *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int thrd_create(int *thr, int func, void *arg);
+
+int fork();
+
+int vfork();
+
+int clone(int (*fn)(void *), void *child_stack,
+  int flags, void *arg);
+
+long clone3(int *cl_args, int size);
+
+namespace boost {
+
+class thread {};
+
+class scoped_thread {};
+
+void async(int (*)()) {}
+
+namespace compute {
+class device {};
+
+} // namespace compute
+
+} // namespace boost
+
+void test_threads() {
+  std::thread t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+
+  std::jthread j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'jthread' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::parallel_policy pp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'parallel_policy' creates new threads [concurrency-async-no-new-threads]
+
+  std::execution::sequenced_policy sp;
+
+  std::sort(std::execution::par, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 'par' creates new threads [concurrency-async-no-new-threads]
+
+  boost::thread bt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'thread' creates new threads [concurrency-async-no-new-threads]
+  boost::scoped_thread bst;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'scoped_thread' creates new threads [concurrency-async-no-new-threads]
+
+  boost::compute::device bcd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'device' creates new threads [concurrency-async-no-new-threads]
+
+  std::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  boost::async(fork);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'async' creates new threads [concurrency-async-no-new-threads]
+
+  pthread_create(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'pthread_create' creates new threads [concurrency-async-no-new-threads]
+
+  thrd_create(0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'thrd_create' creates new threads [concurrency-async-no-new-threads]
+
+  fork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'fork' creates new threads [concurrency-async-no-new-threads]
+
+  vfork();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'vfork' creates new threads [concurrency-async-no-new-threads]
+
+  clone(0, 0, 0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'clone' creates new threads [concurrency-async-no-new-threads]
+
+  clone3(0, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: fu

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

2021-01-14 Thread Vasily Kulikov via Phabricator via cfe-commits
segoon updated this revision to Diff 316667.
segoon added a comment.

- use back-ticks
- fix the first document line
- add default values


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

https://reviews.llvm.org/D94621

Files:
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/AsyncFsCheck.h
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/concurrency-async-fs.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s concurrency-async-fs %t -- \
+// RUN: -config='{CheckOptions: [{key: "concurrency-async-fs.FunctionsExtra", value: "::my::block"},{key: "concurrency-async-fs.TypesExtra", value: "::my::file"}]}'
+
+void chdir(const char *);
+
+namespace std {
+namespace filesystem {
+bool exists(const char *);
+
+void copy(const char *, const char *);
+
+class directory_iterator {
+public:
+  directory_iterator(const char *);
+
+  bool operator!=(const directory_iterator &) const;
+
+  directory_iterator &operator++();
+
+  int operator*() const;
+};
+
+directory_iterator begin(directory_iterator iter) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+directory_iterator end(const directory_iterator &) noexcept;
+
+} // namespace filesystem
+
+template 
+class basic_fstream {};
+
+template 
+class basic_ofstream {};
+
+template 
+class basic_ifstream {};
+
+typedef basic_fstream fstream;
+typedef basic_ofstream ofstream;
+typedef basic_ifstream ifstream;
+
+} // namespace std
+
+void copy();
+
+void test_core() {
+  chdir("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'chdir' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::exists("/");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'exists' may block on filesystem access [concurrency-async-fs]
+
+  std::filesystem::copy("/a", "/b");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'copy' may block on filesystem access [concurrency-async-fs]
+
+  copy();
+
+  for (const auto &f : std::filesystem::directory_iterator("/")) {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: type 'directory_iterator' may access filesystem in a blocking way [concurrency-async-fs]
+  }
+}
+
+void test_fstream() {
+  std::fstream fs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ofstream of;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ofstream' may access filesystem in a blocking way [concurrency-async-fs]
+  std::ifstream ifs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_ifstream' may access filesystem in a blocking way [concurrency-async-fs]
+
+  std::basic_fstream bfs;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'basic_fstream' may access filesystem in a blocking way [concurrency-async-fs]
+}
+
+namespace my {
+class file {};
+
+void block();
+} // namespace my
+
+void test_user() {
+  my::file f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: type 'file' may access filesystem in a blocking way [concurrency-async-fs]
+
+  my::block();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'block' may block on filesystem access [concurrency-async-fs]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -139,6 +139,7 @@
`clang-analyzer-valist.CopyToSelf `_,
`clang-analyzer-valist.Uninitialized `_,
`clang-analyzer-valist.Unterminated `_,
+   `concurrency-async-fs `_,
`concurrency-mt-unsafe `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
Index: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - concurrency-async-fs
+
+concurrency-async-fs
+
+
+Finds filesystem accesses that might block current system thread.
+Asynchronous code may deal with it in numerous ways, the most widespread
+ways are the following:
+
+* use asynchronous API like `aio` or `io_uring`
+* delegate all filesystem access to a thread pool
+
+Some projects may consider filesystem access from asyn

[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > Does it make sense to return `true` when there are no inner matchers? I 
> > > > would have thought that that if there are no matchers, nothing would 
> > > > match (so we'd return `false`)?
> > > When debugging a matcher like
> > > 
> > > ```
> > > callExpr(anyOf(
> > > hasType(pointerType()),
> > > callee(functionDecl(hasName("foo")))
> > > ))
> > > ```
> > > 
> > > and commenting out inner matchers to get 
> > > 
> > > ```
> > > callExpr(anyOf(
> > > #hasType(pointerType()),
> > > #callee(functionDecl(hasName("foo")))
> > > ))
> > > ```
> > > 
> > > it would be very surprising for this to not match callExprs anymore.
> > On the one hand, I see what you're saying. On the other hand, I think the 
> > behavior actually is surprising to some folks (like me). For instance, 
> > `std::any_of()` returns `false` when the range is empty, while 
> > `std::all_of()` returns `true`.
> > 
> > To be conservative with the change, rather than allowing zero matchers with 
> > potentially surprising behavior, we could require there be at least one 
> > matcher. In that case, if you want to comment out all of the inner 
> > matchers, you need to comment out the variadic one as well. e.g.,
> > ```
> > # This is an error
> > callExpr(anyOf(
> > #hasType(pointerType()),
> > #callee(functionDecl(hasName("foo")))
> > ))
> > 
> > # Do this instead
> > callExpr(
> > # anyOf(
> > #hasType(pointerType()),
> > #callee(functionDecl(hasName("foo")))
> > # )
> > )
> > ```
> > It's a bit more commenting for the person experimenting, but it's also 
> > reduces the chances for surprising behavior. WDYT?
> > On the one hand, I see what you're saying. On the other hand, I think the 
> > behavior actually is surprising to some folks (like me). For instance, 
> > `std::any_of()` returns `false` when the range is empty, while 
> > `std::all_of()` returns `true`.
> 
> Yes, I know this diverges from those. However, I think the semantic in this 
> patch is the semantic that makes sense for AST Matchers. This patch 
> prioritizes usefulness and consistency in the context of writing and 
> debugging AST Matchers instead of prioritizing consistency with `std::any_of` 
> (which would be non-useful and surprising in the context of debugging an AST 
> Matcher).
> 
> > if you want to comment out all of the inner matchers, you need to comment 
> > out the variadic one as well
> 
> Yes, that's what I'm trying to make unnecessary.
> 
> > It's a bit more commenting for the person experimenting, but it's also 
> > reduces the chances for surprising behavior. WDYT?
> 
> I think your suggestion leaves zero-arg `anyOf` matcher inconsistent with 
> `allOf` matcher (fails to compile versus does something useful).
> 
> 
> 
> Yes, I know this diverges from those. However, I think the semantic in this 
> patch is the semantic that makes sense for AST Matchers. This patch 
> prioritizes usefulness and consistency in the context of writing and 
> debugging AST Matchers instead of prioritizing consistency with std::any_of 
> (which would be non-useful and surprising in the context of debugging an AST 
> Matcher).

I'm not suggesting that it needs to be consistent for the sake of consistency, 
I'm saying that the behavior you are proposing for the any-of-like matchers 
(`anyOf` and `eachOf`) is confusing to me in the presence of no arguments *and* 
is inconsistent with other APIs that do set inclusion operations.

> I think your suggestion leaves zero-arg anyOf matcher inconsistent with allOf 
> matcher (fails to compile versus does something useful).

Then my preference is for the any-of-like matchers to return `false` when given 
zero arguments while the all-of-like matchers return `true`. Testing for 
existence requires at least one element while testing for universality does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94126

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


[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

That looks like a strange style option to me. Is there any bigger codebase 
formatting the code this way?




Comment at: clang/docs/ClangFormatStyleOptions.rst:2039
 
+**IndentAccessModifiers** (``bool``)
+  Makes an indentation level for record (``class``, ``struct``, ``union``)

The name `IndentAccessModifiers` is misleading for me. Access modifiers are 
public, protected, private.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2039
 
+**IndentAccessModifiers** (``bool``)
+  Makes an indentation level for record (``class``, ``struct``, ``union``)

curdeius wrote:
> The name `IndentAccessModifiers` is misleading for me. Access modifiers are 
> public, protected, private.
Hmm, actually that's the description that is misleading.



Comment at: clang/unittests/Format/FormatTest.cpp:17868
+  FormatStyle Style = getLLVMStyle();
+  Style.AccessModifierOffset = 4;
+  Style.AllowShortEnumsOnASingleLine = false;

Why are those non-related style options here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94661

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


[PATCH] D94614: [FPEnv][X86] Platform builtins edition: clang should get from the AST the metadata for constrained FP builtins

2021-01-14 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

This doesn't add metadata to llvm intrinsics that are not constrained.

The metadata is added by the IRBuilder when the IRBuilder is used to add 
constrained intrinsics. When adding non-constrained intrinsics that have a 
direct mapping to constrained intrinsics, and constrained mode is enabled, the 
IRBuilder will add a constrained intrinsic instead of the requested 
non-constrained intrinsic. This keeps the changes to the front-end smaller.

But the metadata is _only_ added when the IRBuilder knows it is adding a 
constrained intrinsic. So we're not adding anything to any of the other llvm 
intrinsics. The target-specific llvm intrinsics are no exception to this rule.

Some of the clang builtins get lowered in whole or in part to llvm instructions 
(or constrained FP calls). Currently they use the metadata specified by the 
command line arguments. This change causes them to pick up the metadata that 
the AST says should be used. If no #pragma was used then the AST will be 
carrying the metadata from the command line. If a relevant #pragma is used then 
without this change the metadata in the #pragma's scope will be wrong. I'm 
testing for this by having the RUN lines specify "maytrap" but then use the 
#pragma float_control to switch to "fpexcept.strict".

The IRBuilder's idea of the current metadata is updated by CGFPOptionsRAII. 
That's why you see it used right around places where we leave the clang AST and 
call into code that doesn't have access to the AST. Like the IRBuilder.

A similar issue exists with the rounding-mode metadata. But both rounding and 
exception behavior are set by CGFPOptionsRAII, and if that changes then other 
tests already in the tree will fail. So I'm not bothering with rounding 
metadata testing here.

It's true that the middle-end optimizers know nothing about the constrained 
intrinsics. Today. I plan on changing that in the near future.

If use of the constrained intrinsics will cause breakage of the target-specific 
clang builtins then that's important to know and to fix. And I don't know all 
the targets well enough to know if that is a problem. So I'm going around 
asking target-specific developers to take another look and make sure we aren't 
setting ourselves up for problems later by having these builtins lower with the 
correct metadata for this point in the AST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94614

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


[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Stephen Kelly via Phabricator via cfe-commits
steveire abandoned this revision.
steveire added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

aaron.ballman wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > Does it make sense to return `true` when there are no inner matchers? 
> > > > > I would have thought that that if there are no matchers, nothing 
> > > > > would match (so we'd return `false`)?
> > > > When debugging a matcher like
> > > > 
> > > > ```
> > > > callExpr(anyOf(
> > > > hasType(pointerType()),
> > > > callee(functionDecl(hasName("foo")))
> > > > ))
> > > > ```
> > > > 
> > > > and commenting out inner matchers to get 
> > > > 
> > > > ```
> > > > callExpr(anyOf(
> > > > #hasType(pointerType()),
> > > > #callee(functionDecl(hasName("foo")))
> > > > ))
> > > > ```
> > > > 
> > > > it would be very surprising for this to not match callExprs anymore.
> > > On the one hand, I see what you're saying. On the other hand, I think the 
> > > behavior actually is surprising to some folks (like me). For instance, 
> > > `std::any_of()` returns `false` when the range is empty, while 
> > > `std::all_of()` returns `true`.
> > > 
> > > To be conservative with the change, rather than allowing zero matchers 
> > > with potentially surprising behavior, we could require there be at least 
> > > one matcher. In that case, if you want to comment out all of the inner 
> > > matchers, you need to comment out the variadic one as well. e.g.,
> > > ```
> > > # This is an error
> > > callExpr(anyOf(
> > > #hasType(pointerType()),
> > > #callee(functionDecl(hasName("foo")))
> > > ))
> > > 
> > > # Do this instead
> > > callExpr(
> > > # anyOf(
> > > #hasType(pointerType()),
> > > #callee(functionDecl(hasName("foo")))
> > > # )
> > > )
> > > ```
> > > It's a bit more commenting for the person experimenting, but it's also 
> > > reduces the chances for surprising behavior. WDYT?
> > > On the one hand, I see what you're saying. On the other hand, I think the 
> > > behavior actually is surprising to some folks (like me). For instance, 
> > > `std::any_of()` returns `false` when the range is empty, while 
> > > `std::all_of()` returns `true`.
> > 
> > Yes, I know this diverges from those. However, I think the semantic in this 
> > patch is the semantic that makes sense for AST Matchers. This patch 
> > prioritizes usefulness and consistency in the context of writing and 
> > debugging AST Matchers instead of prioritizing consistency with 
> > `std::any_of` (which would be non-useful and surprising in the context of 
> > debugging an AST Matcher).
> > 
> > > if you want to comment out all of the inner matchers, you need to comment 
> > > out the variadic one as well
> > 
> > Yes, that's what I'm trying to make unnecessary.
> > 
> > > It's a bit more commenting for the person experimenting, but it's also 
> > > reduces the chances for surprising behavior. WDYT?
> > 
> > I think your suggestion leaves zero-arg `anyOf` matcher inconsistent with 
> > `allOf` matcher (fails to compile versus does something useful).
> > 
> > 
> > 
> > Yes, I know this diverges from those. However, I think the semantic in this 
> > patch is the semantic that makes sense for AST Matchers. This patch 
> > prioritizes usefulness and consistency in the context of writing and 
> > debugging AST Matchers instead of prioritizing consistency with std::any_of 
> > (which would be non-useful and surprising in the context of debugging an 
> > AST Matcher).
> 
> I'm not suggesting that it needs to be consistent for the sake of 
> consistency, I'm saying that the behavior you are proposing for the 
> any-of-like matchers (`anyOf` and `eachOf`) is confusing to me in the 
> presence of no arguments *and* is inconsistent with other APIs that do set 
> inclusion operations.
> 
> > I think your suggestion leaves zero-arg anyOf matcher inconsistent with 
> > allOf matcher (fails to compile versus does something useful).
> 
> Then my preference is for the any-of-like matchers to return `false` when 
> given zero arguments while the all-of-like matchers return `true`. Testing 
> for existence requires at least one element while testing for universality 
> does not.
> Then my preference is for the any-of-like matchers to return `false` when 
> given zero arguments while the all-of-like matchers return `true`. Testing 
> for existence requires at least one element while testing for universality 
> does not.

That doesn't achieve the goal I have for this patch. 

If that's a blocker, then the only remaining possibility is a different name 
for a matcher which returns true on empty and otherwise behaves like anyOf. I 
doubt a good name for such a thing exists.

So, I'll abandon it for now.


Repository:
  rG LLVM Github Monorepo

CHANGES

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

2021-01-14 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 316669.
usaxena95 marked 13 inline comments as done.
usaxena95 added a comment.
Herald added a subscriber: mgorny.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94424

Files:
  clang-tools-extra/clangd/ASTSignals.cpp
  clang-tools-extra/clangd/ASTSignals.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -14,6 +14,7 @@
 #include "Preamble.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "support/Cancellation.h"
 #include "support/Context.h"
 #include "support/Path.h"
@@ -42,12 +43,14 @@
 namespace clangd {
 namespace {
 
+using ::testing::_;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::IsEmpty;
+using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
@@ -679,12 +682,12 @@
 cantFail(std::move(Preamble)).Preamble->Preamble.getBounds().Size,
 0u);
   });
-  // Wait for the preamble is being built.
+  // Wait while the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 
   // Update the file which results in an empty preamble.
   S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto);
-  // Wait for the preamble is being built.
+  // Wait while the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   S.runWithPreamble(
   "getEmptyPreamble", Foo, TUScheduler::Stale,
@@ -696,6 +699,47 @@
   });
 }
 
+TEST_F(TUSchedulerTests, ASTSignalsSmokeTests) {
+  TUScheduler S(CDB, optsForTest());
+  auto Foo = testPath("foo.cpp");
+  auto Header = testPath("foo.h");
+
+  FS.Files[Header] = "namespace tar { int foo(); }";
+  const char *Contents = R"cpp(
+  #include "foo.h"
+  namespace ns {
+  int func() {
+return tar::foo());
+  }
+  } // namespace ns
+  )cpp";
+  // Update the file which results in an empty preamble.
+  S.update(Foo, getInputs(Foo, Contents), WantDiagnostics::Yes);
+  // Wait while the preamble is being built.
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  Notification TaskRun;
+  S.runWithPreamble(
+  "ASTSignals", Foo, TUScheduler::Stale,
+  [&](Expected IP) {
+ASSERT_FALSE(!IP);
+std::vector> NS;
+for (const auto &P : IP->Signals->RelatedNamespaces)
+  NS.emplace_back(P.getKey(), P.getValue());
+EXPECT_THAT(NS,
+UnorderedElementsAre(Pair("ns::", 1), Pair("tar::", 1)));
+
+std::vector> Sym;
+for (const auto &P : IP->Signals->ReferencedSymbols)
+  Sym.emplace_back(P.getFirst(), P.getSecond());
+EXPECT_THAT(Sym, UnorderedElementsAre(Pair(ns("tar").ID, 1),
+  Pair(ns("ns").ID, 1),
+  Pair(func("tar::foo").ID, 1),
+  Pair(func("ns::func").ID, 1)));
+TaskRun.notify();
+  });
+  TaskRun.wait();
+}
+
 TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
   // Testing strategy: we update the file and schedule a few preamble reads at
   // the same time. All reads should get the same non-null preamble.
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -35,6 +35,7 @@
 add_unittest(ClangdUnitTests ClangdTests
   Annotations.cpp
   ASTTests.cpp
+  ASTSignalsTests.cpp
   BackgroundIndexTests.cpp
   CallHierarchyTests.cpp
   CanonicalIncludesTests.cpp
Index: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
@@ -0,0 +1,75 @@
+//===-- ASTSignalsTests.cpp -*- C++ -*-===//
+//
+// 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 "AST.h"
+
+#include "ParsedAST.h"
+#include "TestIndex.h"
+#include "TestTU.h"
+#include "llvm/ADT/StringR

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

2021-01-14 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:841
 
+void ASTWorker::updateASTSignals(ParsedAST &AST) {
+  ASTSignals Signals;

sammccall wrote:
> This implementation doesn't belong in TUScheduler, which is all about 
> managing lifecycles and threading, rather than inspecting ASTs.
> 
> Best suggestion I have is to create a separate header ASTSignals.h, which 
> defines the ASTSignals struct and a `static ASTSignals 
> ASTSignals::derive(const ParsedAST&)` function to extract them.
> 
> (It's pretty awkward to even directly depend on it, since this feels more of 
> a "feature" like go-to-definition that's independent of to TUScheduler and 
> injected by ClangdServer, rather than core things like ParsedAST that are 
> layered below TUScheduler. But fundamentally we can't avoid this without 
> somehow obfuscating the data structure we're storing).
Moved struct and calculation to a separate file ASTSignals.h/cpp



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:851
+Signals.Symbols[ID] += 1;
+  // FIXME: Do not count the primary namespace as a related namespace.
+  // Eg. clang::clangd:: for this file.

sammccall wrote:
> why not?
My mental model suggested these must be namespaces from outside the file which 
are relevant to this file.
I suppose we can take care of this while using this (in CC signals calculation) 
and let these represent all namespaces including the primary one.

On a second thought: If we do not explicitly exclude primary namespace, the 
results from primary namespace will be boosted much more than the results from 
index which might be acceptable too.




Comment at: clang-tools-extra/clangd/TUScheduler.cpp:853
+  // Eg. clang::clangd:: for this file.
+  if (const auto *NSD = dyn_cast(ND->getDeclContext()))
+if (!ND->isInAnonymousNamespace()) {

sammccall wrote:
> as mentioned above, you may want to do this only if the per-symbol-count was 
> incremented from 0 to 1.
> You'd have to ignore namespaces for symbols with no symbolID, but that 
> doesn't matter.
Ignored NS without a valid SymbolID.
Counted distinct number of Symbols used per namespace.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:854
+  if (const auto *NSD = dyn_cast(ND->getDeclContext()))
+if (!ND->isInAnonymousNamespace()) {
+  std::string NS;

sammccall wrote:
> why not NSD->isAnonymous()?
> 
> (and generally prefer early continue rather than indenting the whole loop 
> body)
I guess `isAnonymous` would cover most cases. I thought of excluding named NS 
in an anonymous NS but those are super rare I suppose and including those is 
not harmful too.


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


[clang-tools-extra] 17fb21f - [clangd] Remove another option that was effectively always true. NFC

2021-01-14 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2021-01-14T17:19:47+01:00
New Revision: 17fb21f875f4aaf6ad2cf9499cb75d76588167f2

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

LOG: [clangd] Remove another option that was effectively always true. NFC

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index d5e21cfb063e..4f3a47dff05d 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -243,15 +243,11 @@ void ClangdServer::codeComplete(PathRef File, Position 
Pos,
   // No speculation in Fallback mode, as it's supposed to be much faster
   // without compiling.
   vlog("Build for file {0} is not ready. Enter fallback mode.", File);
-} else {
-  if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
-SpecFuzzyFind.emplace();
-{
-  std::lock_guard Lock(
-  CachedCompletionFuzzyFindRequestMutex);
-  SpecFuzzyFind->CachedReq =
-  CachedCompletionFuzzyFindRequestByFile[File];
-}
+} else if (CodeCompleteOpts.Index) {
+  SpecFuzzyFind.emplace();
+  {
+std::lock_guard 
Lock(CachedCompletionFuzzyFindRequestMutex);
+SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
 ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};

diff  --git a/clang-tools-extra/clangd/CodeComplete.h 
b/clang-tools-extra/clangd/CodeComplete.h
index f7ac3c7e5aba..ddcbd487ecc6 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -82,16 +82,6 @@ struct CodeCompleteOptions {
   /// Expose origins of completion items in the label (for debugging).
   bool ShowOrigins = false;
 
-  /// If set to true, this will send an asynchronous speculative index request,
-  /// based on the index request for the last code completion on the same file
-  /// and the filter text typed before the cursor, before sema code completion
-  /// is invoked. This can reduce the code completion latency (by roughly
-  /// latency of sema code completion) if the speculative request is the same 
as
-  /// the one generated for the ongoing code completion from sema. As a 
sequence
-  /// of code completions often have the same scopes and proximity paths etc,
-  /// this should be effective for a number of code completions.
-  bool SpeculativeIndexRequest = false;
-
   // Populated internally by clangd, do not set.
   /// If `Index` is set, it is used to augment the code completion
   /// results.

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 9c75cafdb08e..d3859103b0f0 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -828,7 +828,6 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
 Opts.CodeComplete.IncludeIndicator.Insert.clear();
 Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
   }
-  Opts.CodeComplete.SpeculativeIndexRequest = Opts.StaticIndex;
   Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
   Opts.CodeComplete.AllScopes = AllScopesCompletion;
   Opts.CodeComplete.RunParser = CodeCompletionParse;

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 43a557d6c73e..9842bcb315e5 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2339,7 +2339,6 @@ TEST(CompletionTest, EnableSpeculativeIndexRequest) {
 
   IndexRequestCollector Requests;
   Opts.Index = &Requests;
-  Opts.SpeculativeIndexRequest = true;
 
   auto CompleteAtPoint = [&](StringRef P) {
 cantFail(runCodeComplete(Server, File, Test.point(P), Opts));



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


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

2021-01-14 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 316670.
usaxena95 added a comment.

Removed unused headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94424

Files:
  clang-tools-extra/clangd/ASTSignals.cpp
  clang-tools-extra/clangd/ASTSignals.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -14,6 +14,7 @@
 #include "Preamble.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "support/Cancellation.h"
 #include "support/Context.h"
 #include "support/Path.h"
@@ -42,12 +43,14 @@
 namespace clangd {
 namespace {
 
+using ::testing::_;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::IsEmpty;
+using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
@@ -679,12 +682,12 @@
 cantFail(std::move(Preamble)).Preamble->Preamble.getBounds().Size,
 0u);
   });
-  // Wait for the preamble is being built.
+  // Wait while the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 
   // Update the file which results in an empty preamble.
   S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto);
-  // Wait for the preamble is being built.
+  // Wait while the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   S.runWithPreamble(
   "getEmptyPreamble", Foo, TUScheduler::Stale,
@@ -696,6 +699,47 @@
   });
 }
 
+TEST_F(TUSchedulerTests, ASTSignalsSmokeTests) {
+  TUScheduler S(CDB, optsForTest());
+  auto Foo = testPath("foo.cpp");
+  auto Header = testPath("foo.h");
+
+  FS.Files[Header] = "namespace tar { int foo(); }";
+  const char *Contents = R"cpp(
+  #include "foo.h"
+  namespace ns {
+  int func() {
+return tar::foo());
+  }
+  } // namespace ns
+  )cpp";
+  // Update the file which results in an empty preamble.
+  S.update(Foo, getInputs(Foo, Contents), WantDiagnostics::Yes);
+  // Wait while the preamble is being built.
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  Notification TaskRun;
+  S.runWithPreamble(
+  "ASTSignals", Foo, TUScheduler::Stale,
+  [&](Expected IP) {
+ASSERT_FALSE(!IP);
+std::vector> NS;
+for (const auto &P : IP->Signals->RelatedNamespaces)
+  NS.emplace_back(P.getKey(), P.getValue());
+EXPECT_THAT(NS,
+UnorderedElementsAre(Pair("ns::", 1), Pair("tar::", 1)));
+
+std::vector> Sym;
+for (const auto &P : IP->Signals->ReferencedSymbols)
+  Sym.emplace_back(P.getFirst(), P.getSecond());
+EXPECT_THAT(Sym, UnorderedElementsAre(Pair(ns("tar").ID, 1),
+  Pair(ns("ns").ID, 1),
+  Pair(func("tar::foo").ID, 1),
+  Pair(func("ns::func").ID, 1)));
+TaskRun.notify();
+  });
+  TaskRun.wait();
+}
+
 TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
   // Testing strategy: we update the file and schedule a few preamble reads at
   // the same time. All reads should get the same non-null preamble.
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -35,6 +35,7 @@
 add_unittest(ClangdUnitTests ClangdTests
   Annotations.cpp
   ASTTests.cpp
+  ASTSignalsTests.cpp
   BackgroundIndexTests.cpp
   CallHierarchyTests.cpp
   CanonicalIncludesTests.cpp
Index: clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ASTSignalsTests.cpp
@@ -0,0 +1,75 @@
+//===-- ASTSignalsTests.cpp -*- C++ -*-===//
+//
+// 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 "AST.h"
+
+#include "ParsedAST.h"
+#include "TestIndex.h"
+#include "TestTU.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang

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

2021-01-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.

Thanks!




Comment at: clang-tools-extra/clangd/ASTSignals.cpp:9
+  ASTSignals Signals;
+  llvm::DenseMap> NSDToSymbols;
+  const SourceManager &SM = AST.getSourceManager();

this requires storing all the SymbolIDs again.

You could just have a counter instead. The trick is that you're already 
counting the number of times a symbol has been seen...

```
unsigned &SymbolCount = Signals.ReferencedSymbols[ID];
++SymbolCount;
if (SymbolCount == 1) { // first time seeing the symbol
  // increment counter for its namespace
}
```


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] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D20689#2457808 , @whisperity wrote:

> Right, let's bump.

Thank you for all of the detailed information on the performance of the check! 
I worked on a similar check in a recent past life and my intuition is that over 
a large corpus of code, this will still have quite a bit of false positives and 
false negatives. However, I think those can likely be handled by post-commit 
improvements like noticing abbreviations (`def` vs `define`) or synonyms 
(`number` vs `numeral`), being smarter about patterned names, etc. I think this 
check is worth moving forward with, but I'm not certain how @alexfh feels about 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689

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


[PATCH] D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input

2021-01-14 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94655

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


[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

2021-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:402
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;

steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > steveire wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does it make sense to return `true` when there are no inner 
> > > > > > matchers? I would have thought that that if there are no matchers, 
> > > > > > nothing would match (so we'd return `false`)?
> > > > > When debugging a matcher like
> > > > > 
> > > > > ```
> > > > > callExpr(anyOf(
> > > > > hasType(pointerType()),
> > > > > callee(functionDecl(hasName("foo")))
> > > > > ))
> > > > > ```
> > > > > 
> > > > > and commenting out inner matchers to get 
> > > > > 
> > > > > ```
> > > > > callExpr(anyOf(
> > > > > #hasType(pointerType()),
> > > > > #callee(functionDecl(hasName("foo")))
> > > > > ))
> > > > > ```
> > > > > 
> > > > > it would be very surprising for this to not match callExprs anymore.
> > > > On the one hand, I see what you're saying. On the other hand, I think 
> > > > the behavior actually is surprising to some folks (like me). For 
> > > > instance, `std::any_of()` returns `false` when the range is empty, 
> > > > while `std::all_of()` returns `true`.
> > > > 
> > > > To be conservative with the change, rather than allowing zero matchers 
> > > > with potentially surprising behavior, we could require there be at 
> > > > least one matcher. In that case, if you want to comment out all of the 
> > > > inner matchers, you need to comment out the variadic one as well. e.g.,
> > > > ```
> > > > # This is an error
> > > > callExpr(anyOf(
> > > > #hasType(pointerType()),
> > > > #callee(functionDecl(hasName("foo")))
> > > > ))
> > > > 
> > > > # Do this instead
> > > > callExpr(
> > > > # anyOf(
> > > > #hasType(pointerType()),
> > > > #callee(functionDecl(hasName("foo")))
> > > > # )
> > > > )
> > > > ```
> > > > It's a bit more commenting for the person experimenting, but it's also 
> > > > reduces the chances for surprising behavior. WDYT?
> > > > On the one hand, I see what you're saying. On the other hand, I think 
> > > > the behavior actually is surprising to some folks (like me). For 
> > > > instance, `std::any_of()` returns `false` when the range is empty, 
> > > > while `std::all_of()` returns `true`.
> > > 
> > > Yes, I know this diverges from those. However, I think the semantic in 
> > > this patch is the semantic that makes sense for AST Matchers. This patch 
> > > prioritizes usefulness and consistency in the context of writing and 
> > > debugging AST Matchers instead of prioritizing consistency with 
> > > `std::any_of` (which would be non-useful and surprising in the context of 
> > > debugging an AST Matcher).
> > > 
> > > > if you want to comment out all of the inner matchers, you need to 
> > > > comment out the variadic one as well
> > > 
> > > Yes, that's what I'm trying to make unnecessary.
> > > 
> > > > It's a bit more commenting for the person experimenting, but it's also 
> > > > reduces the chances for surprising behavior. WDYT?
> > > 
> > > I think your suggestion leaves zero-arg `anyOf` matcher inconsistent with 
> > > `allOf` matcher (fails to compile versus does something useful).
> > > 
> > > 
> > > 
> > > Yes, I know this diverges from those. However, I think the semantic in 
> > > this patch is the semantic that makes sense for AST Matchers. This patch 
> > > prioritizes usefulness and consistency in the context of writing and 
> > > debugging AST Matchers instead of prioritizing consistency with 
> > > std::any_of (which would be non-useful and surprising in the context of 
> > > debugging an AST Matcher).
> > 
> > I'm not suggesting that it needs to be consistent for the sake of 
> > consistency, I'm saying that the behavior you are proposing for the 
> > any-of-like matchers (`anyOf` and `eachOf`) is confusing to me in the 
> > presence of no arguments *and* is inconsistent with other APIs that do set 
> > inclusion operations.
> > 
> > > I think your suggestion leaves zero-arg anyOf matcher inconsistent with 
> > > allOf matcher (fails to compile versus does something useful).
> > 
> > Then my preference is for the any-of-like matchers to return `false` when 
> > given zero arguments while the all-of-like matchers return `true`. Testing 
> > for existence requires at least one element while testing for universality 
> > does not.
> > Then my preference is for the any-of-like matchers to return `false` when 
> > given zero arguments while the all-of-like matchers return `true`. Testing 
> > for existence requires at least one element while testing for universality 
> > does not.
> 
> That doesn't achieve the goal I have for this patch. 
> 
> If that's a blocker, then the only remaining possibility is a different name 
>

[PATCH] D94674: [clang][cli] NFC: Decrease the scope of ParseLangArgs parameters

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94674

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


[PATCH] D94675: [clang][cli] NFC: Decrease the scope of ParseCodeGenArgs parameters

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94675

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


[PATCH] D94676: [clang][cli] Specify KeyPath prefixes via TableGen classes

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94676

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


[PATCH] D94678: [clang][cli] Parse & generate options necessary for LangOptions defaults manually

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM with the comment.




Comment at: clang/include/clang/Driver/Options.td:5210-5213
 def finclude_default_header : Flag<["-"], "finclude-default-header">,
-  HelpText<"Include default header file for OpenCL">,
-  MarshallingInfoFlag>;
+  HelpText<"Include default header file for OpenCL">;
 def fdeclare_opencl_builtins : Flag<["-"], "fdeclare-opencl-builtins">,
+  HelpText<"Add OpenCL builtin function declarations (experimental)">;

You should add a comment here explaining why these can't be marshaled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94678

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


[PATCH] D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input

2021-01-14 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Is there any way to condition this on the type of the output, rather than the 
input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not 
need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR 
anyway?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94655

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


[PATCH] D94679: [clang][cli] NFC: Add PIE parsing for precompiled input and IR

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94679

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


[PATCH] D94682: [clang][cli] Parse Lang and CodeGen options separately

2021-01-14 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1831
+  MarshallingInfoString,
+!strconcat(open_cl.KeyPath, " ? 
LangOptions::LaxVectorConversionKind::None"
+" : 
LangOptions::LaxVectorConversionKind::All")>,

This replaces the conditional in `CompilerInvocation::setLangDefaults`.

The enum cases need to be fully qualified now, because we can't use 
`NormalizedValuesScope<"LangOptions::LaxVectorConversionKind">` due to the 
"arbitrary" expression for default value.



Comment at: clang/test/Frontend/diagnostics-order.c:12
 // CHECK-NEXT: note: use {{.*}} for {{.*}} standard
+// CHECK: warning: optimization level '-O999' is not supported

The order of these diagnostics depends on the order of argument parsing. 
Because we've moved `CodeGenOpts` parsing after `LangOpts`, this needs to be 
adjusted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94682

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


[PATCH] D94680: [clang][cli] NFC: Parse some LangOpts after the defaults are set

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94680

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


[PATCH] D94697: [clangd] Update CC Ranking model with better sampling.

2021-01-14 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 created this revision.
usaxena95 added a reviewer: adamcz.
Herald added subscribers: kadircet, arphaman.
usaxena95 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

A better sampling strategy was used to generate the dataset for this
model.
New signals introduced in this model:

- NumNameInContext: Number of words in the context that matches the name

of the candidate.

- FractionNameInContext: Fraction of the words in context matching the

name of the candidate.

We remove the signal `IsForbidden` from the model and down rank
forbidden signals aggresively.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94697

Files:
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/quality/model/features.json
  clang-tools-extra/clangd/quality/model/forest.json

Index: clang-tools-extra/clangd/quality/model/features.json
===
--- clang-tools-extra/clangd/quality/model/features.json
+++ clang-tools-extra/clangd/quality/model/features.json
@@ -1,8 +1,4 @@
 [
-{
-"name": "FilterLength",
-"kind": "NUMBER"
-},
 {
 "name": "IsDeprecated",
 "kind": "NUMBER"
@@ -20,11 +16,15 @@
 "kind": "NUMBER"
 },
 {
-"name": "IsNameInContext",
+"name": "NumNameInContext",
 "kind": "NUMBER"
 },
 {
-"name": "IsForbidden",
+"name": "FractionNameInContext",
+"kind": "NUMBER"
+},
+{
+"name": "IsNameInContext",
 "kind": "NUMBER"
 },
 {
@@ -32,7 +32,7 @@
 "kind": "NUMBER"
 },
 {
-"name": "FileProximityDistance",
+"name": "FileProximityDistanceCost",
 "kind": "NUMBER"
 },
 {
@@ -40,7 +40,7 @@
 "kind": "NUMBER"
 },
 {
-"name": "SymbolScopeDistance",
+"name": "SymbolScopeDistanceCost",
 "kind": "NUMBER"
 },
 {
@@ -81,4 +81,4 @@
 "type": "clang::clangd::SymbolRelevanceSignals::AccessibleScope",
 "header": "Quality.h"
 }
-]
\ No newline at end of file
+]
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -501,12 +501,23 @@
 
   SymbolRelevanceSignals::DerivedSignals Derived =
   Relevance.calculateDerivedSignals();
-  E.setIsNameInContext(Derived.NameMatchesContext);
-  E.setIsForbidden(Relevance.Forbidden);
+  int NumMatch = 0;
+  if (Relevance.ContextWords) {
+for (const auto &Word : Relevance.ContextWords->keys()) {
+  if (Relevance.Name.contains_lower(Word)) {
+++NumMatch;
+  }
+}
+  }
+  E.setIsNameInContext(NumMatch > 0);
+  E.setNumNameInContext(NumMatch);
+  E.setFractionNameInContext(
+  Relevance.ContextWords ? NumMatch * 1.0 / Relevance.ContextWords->size()
+ : 0);
   E.setIsInBaseClass(Relevance.InBaseClass);
-  E.setFileProximityDistance(Derived.FileProximityDistance);
+  E.setFileProximityDistanceCost(Derived.FileProximityDistance);
   E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
-  E.setSymbolScopeDistance(Derived.ScopeProximityDistance);
+  E.setSymbolScopeDistanceCost(Derived.ScopeProximityDistance);
   E.setSemaSaysInScope(Relevance.SemaSaysInScope);
   E.setScope(Relevance.Scope);
   E.setContextKind(Relevance.Context);
@@ -514,7 +525,6 @@
   E.setHadContextType(Relevance.HadContextType);
   E.setHadSymbolType(Relevance.HadSymbolType);
   E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
-  E.setFilterLength(Relevance.FilterLength);
 
   DecisionForestScores Scores;
   // Exponentiating DecisionForest prediction makes the score of each tree a
@@ -525,6 +535,9 @@
   // data that needs fixits is not-feasible.
   if (Relevance.NeedsFixIts)
 Scores.ExcludingName *= 0.5;
+  if (Relevance.Forbidden)
+Scores.ExcludingName *= 0;
+
   // NameMatch should be a multiplier on total score to support rescoring.
   Scores.Total = Relevance.NameMatch * Scores.ExcludingName;
   return Scores;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94681: [clang][cli] NFC: Promote ParseLangArgs and ParseCodeGenArgs to members

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94681

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


[PATCH] D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input

2021-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D94655#2498504 , @dblaikie wrote:

> Is there any way to condition this on the type of the output, rather than the 
> input? (or, more specifically, on whether machine code is being generated)
>
> Or maybe we could always pass the split-dwarf-file down through LLVM and not 
> need to conditionalize it at all? It'd be a no-op if there's no DWARF in the 
> IR anyway?

I tried replacing `if (IRInput || Args.hasArg(options::OPT_g_Group)) {` with 
`if (1)`, -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Since we already have the

  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
  // complicated if you've disabled inline info in the skeleton CUs
  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
  // line-tables-only, so let those compose naturally in that case.

logic, I think altering `DwarfFission` in the driver is fine. If not, I'd hope 
the backend to process `DwarfFission` ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94655

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


[PATCH] D94682: [clang][cli] Parse Lang and CodeGen options separately

2021-01-14 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94682

___
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`.

2021-01-14 Thread James Y Knight via Phabricator via cfe-commits
jyknight 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) {

ldionne wrote:
> 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).
It defaults to the latest version of clang (usually; some vendors pin it to an 
old version). Yes, it is theoretically an ABI break -- but one which is 
extremely unlikely to cause trouble in any real code.

This is in line with other similar C++ ABI breaks we make in Clang.


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] D94699: [clangd] Set correct CWD when using compile_flags.txt

2021-01-14 Thread Adam Czachorowski via Phabricator via cfe-commits
adamcz created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

This fixes a bug where clangd would attempt to set CWD to the
compile_flags.txt file itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94699

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -279,6 +279,17 @@
   << "x/build/compile_flags.json only applicable to x/";
 }
 
+TEST(GlobalCompilationDatabaseTest, CompileFlagsDirectory) {
+  MockFS FS;
+  FS.Files[testPath("x/compile_flags.txt")] = "-DFOO";
+  DirectoryBasedGlobalCompilationDatabase CDB(FS);
+  auto Commands = CDB.getCompileCommand(testPath("x/y.cpp"));
+  ASSERT_TRUE(Commands.hasValue());
+  EXPECT_THAT(Commands.getValue().CommandLine, Contains("-DFOO"));
+  // Make sure we pick the right working directory.
+  EXPECT_EQ(testPath("x"), Commands.getValue().Directory);
+}
+
 TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
   OverlayCDB DB(nullptr);
   std::vector DiscoveredFiles;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -271,7 +271,8 @@
 }
 static std::unique_ptr
 parseFixed(PathRef Path, llvm::StringRef Data, std::string &Error) {
-  return tooling::FixedCompilationDatabase::loadFromBuffer(Path, Data, Error);
+  return tooling::FixedCompilationDatabase::loadFromBuffer(
+  llvm::sys::path::parent_path(Path), Data, Error);
 }
 
 bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -279,6 +279,17 @@
   << "x/build/compile_flags.json only applicable to x/";
 }
 
+TEST(GlobalCompilationDatabaseTest, CompileFlagsDirectory) {
+  MockFS FS;
+  FS.Files[testPath("x/compile_flags.txt")] = "-DFOO";
+  DirectoryBasedGlobalCompilationDatabase CDB(FS);
+  auto Commands = CDB.getCompileCommand(testPath("x/y.cpp"));
+  ASSERT_TRUE(Commands.hasValue());
+  EXPECT_THAT(Commands.getValue().CommandLine, Contains("-DFOO"));
+  // Make sure we pick the right working directory.
+  EXPECT_EQ(testPath("x"), Commands.getValue().Directory);
+}
+
 TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
   OverlayCDB DB(nullptr);
   std::vector DiscoveredFiles;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -271,7 +271,8 @@
 }
 static std::unique_ptr
 parseFixed(PathRef Path, llvm::StringRef Data, std::string &Error) {
-  return tooling::FixedCompilationDatabase::loadFromBuffer(Path, Data, Error);
+  return tooling::FixedCompilationDatabase::loadFromBuffer(
+  llvm::sys::path::parent_path(Path), Data, Error);
 }
 
 bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >