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

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 316331.
ArcsinX added a comment.

- std::pair => struct MacroOccurrence
- remove addressed fixme comment
- assert() => cantFail()
- use toSourceCode() to get the macro name


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

[PATCH] D94466: [X86] merge "={eax}" and "~{eax}" into "=&eax" for MSInlineASM

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2493
+}
+std::string::size_type position2 = Constraints.find("=A");
+if (position2 != std::string::npos) {

Should consider the `Clobber == "edx"`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94466

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

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

[Hsiangkai Wang via Phabricator via cfe-commits]

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

D93611 , D93612 
, D93613 , 
and D93614  have been reviewed and accepted. I 
change the version number of vector extension to v1.020201218, i.e., append the 
date of commit 08a0b464 in riscv-v-spec to v1.0.

Currently, there is no v1.0 tag in riscv-v-spec. That is why I use the date of 
the commit as the draft version number. We hope v1.0 could be merged into LLVM 
12. I am not sure whether it is an acceptable way to do it or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94583

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-arch.c
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -80,10 +80,10 @@
 // CHECK-DOUBLE-NOT: __riscv_float_abi_single
 
 // RUN: %clang -target riscv32-unknown-linux-gnu 
-menable-experimental-extensions \
-// RUN:   -march=rv32iv0p9 -x c -E -dM %s \
+// RUN:   -march=rv32iv1p020201218 -x c -E -dM %s \
 // RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu 
-menable-experimental-extensions \
-// RUN:   -march=rv64iv0p9 -x c -E -dM %s \
+// RUN:   -march=rv64iv1p020201218 -x c -E -dM %s \
 // RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
 // CHECK-V-EXT: __riscv_vector 1
 //
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -380,7 +380,7 @@
 // RV32-EXPERIMENTAL-V-BADVERS: error: invalid arch name 'rv32iv0p1'
 // RV32-EXPERIMENTAL-V-BADVERS: unsupported version number 0.1 for 
experimental extension
 
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iv0p9 
-menable-experimental-extensions -### %s -c 2>&1 | \
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iv1p020201218 
-menable-experimental-extensions -### %s -c 2>&1 | \
 // RUN:   FileCheck -check-prefix=RV32-EXPERIMENTAL-V-GOODVERS %s
 // RV32-EXPERIMENTAL-V-GOODVERS: "-target-feature" "+experimental-v"
 
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -63,7 +63,7 @@
   Ext == "zbs" || Ext == "zbt" || Ext == "zbproposedc")
 return RISCVExtensionVersion{"0", "92"};
   if (Ext == "v")
-return RISCVExtensionVersion{"0", "9"};
+return RISCVExtensionVersion{"1", "020201218"};
   if (Ext == "zfh")
 return RISCVExtensionVersion{"0", "1"};
   return None;


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -80,10 +80,10 @@
 // CHECK-DOUBLE-NOT: __riscv_float_abi_single
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -menable-experimental-extensions \
-// RUN:   -march=rv32iv0p9 -x c -E -dM %s \
+// RUN:   -march=rv32iv1p020201218 -x c -E -dM %s \
 // RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -menable-experimental-extensions \
-// RUN:   -march=rv64iv0p9 -x c -E -dM %s \
+// RUN:   -march=rv64iv1p020201218 -x c -E -dM %s \
 // RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
 // CHECK-V-EXT: __riscv_vector 1
 //
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -380,7 +380,7 @@
 // RV32-EXPERIMENTAL-V-BADVERS: error: invalid arch name 'rv32iv0p1'
 // RV32-EXPERIMENTAL-V-BADVERS: unsupported version number 0.1 for experimental extension
 
-// RUN: %clang -target riscv32-unknown-elf -march=rv32iv0p9 -menable-experimental-extensions -### %s -c 2>&1 | \
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iv1p020201218 -menable-experimental-extensions -### %s -c 2>&1 | \
 // RUN:   FileCheck -check-prefix=RV32-EXPERIMENTAL-V-GOODVERS %s
 // RV32-EXPERIMENTAL-V-GOODVERS: "-target-feature" "+experimental-v"
 
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp

[PATCH] D94466: [X86] merge "={eax}" and "~{eax}" into "=&eax" for MSInlineASM

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 316324.
FreddyYe added a comment.

consider "=A" into the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94466

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGenCXX/ms-inline-asm-return.cpp

Index: clang/test/CodeGenCXX/ms-inline-asm-return.cpp
===
--- clang/test/CodeGenCXX/ms-inline-asm-return.cpp
+++ clang/test/CodeGenCXX/ms-inline-asm-return.cpp
@@ -13,7 +13,7 @@
   }
 }
 // CHECK-LABEL: define dso_local i64 @f_i64()
-// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=A,~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&A,{{.*}}"
 // CHECK: ret i64 %[[r]]
 
 int f_i32() {
@@ -23,7 +23,7 @@
   }
 }
 // CHECK-LABEL: define dso_local i32 @f_i32()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},{{.*}}"
 // CHECK: ret i32 %[[r]]
 
 short f_i16() {
@@ -33,7 +33,7 @@
   }
 }
 // CHECK-LABEL: define dso_local signext i16 @f_i16()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},{{.*}}"
 // CHECK: %[[r_i16:[^ ]*]] = trunc i32 %[[r]] to i16
 // CHECK: ret i16 %[[r_i16]]
 
@@ -44,7 +44,7 @@
   }
 }
 // CHECK-LABEL: define dso_local signext i8 @f_i8()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},{{.*}}"
 // CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
 // CHECK: ret i8 %[[r_i8]]
 
@@ -55,7 +55,7 @@
   }
 }
 // CHECK-LABEL: define dso_local zeroext i1 @f_i1()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=&{eax},{{.*}}"
 // CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
 // CHECK: store i8 %[[r_i8]], i8* %{{.*}}
 // CHECK: %[[r_i1:[^ ]*]] = load i1, i1* %{{.*}}
@@ -70,7 +70,7 @@
   }
 }
 // CHECK-LABEL: define dso_local i32 @f_s4()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$16843009", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$16843009", "=&{eax},{{.*}}"
 // CHECK: store i32 %[[r]], i32* %{{.*}}
 // CHECK: %[[r_i32:[^ ]*]] = load i32, i32* %{{.*}}
 // CHECK: ret i32 %[[r_i32]]
@@ -85,7 +85,7 @@
   }
 }
 // CHECK-LABEL: define dso_local i64 @f_s8()
-// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$16843009\0A\09mov edx, $$85", "=A,~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$16843009\0A\09mov edx, $$85", "=&A,{{.*}}"
 // CHECK: store i64 %[[r]], i64* %{{.*}}
 // CHECK: %[[r_i64:[^ ]*]] = load i64, i64* %{{.*}}
 // CHECK: ret i64 %[[r_i64]]
@@ -96,5 +96,5 @@
   __asm xor eax, eax
 }
 // CHECK-LABEL: define dso_local i32 @main()
-// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "={eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "=&{eax},{{.*}}"
 // CHECK: ret i32 %[[r]]
Index: clang/test/CodeGen/ms-inline-asm.c
===
--- clang/test/CodeGen/ms-inline-asm.c
+++ clang/test/CodeGen/ms-inline-asm.c
@@ -114,7 +114,7 @@
 // CHECK: call i32 asm sideeffect inteldialect
 // CHECK-SAME: mov eax, $2
 // CHECK-SAME: mov $0, eax
-// CHECK-SAME: "=*m,={eax},*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
+// CHECK-SAME: "=*m,=&{eax},*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
 // CHECK: [[RET:%[a-zA-Z0-9]+]] = load i32, i32* [[J]], align 4
 // CHECK: ret i32 [[RET]]
 }
@@ -140,7 +140,7 @@
 // CHECK-SAME: mov $0, eax
 // CHECK-SAME: mov eax, $4
 // CHECK-SAME: mov $1, eax
-// CHECK-SAME: "=*m,=*m,={eax},*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
+// CHECK-SAME: "=*m,=*m,=&{eax},*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
 }
 
 void t13() {
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2483,6 +2483,20 @@
   }
 }
 
+if (isa(&S)) {
+  if (Clobber == "eax") {
+std::string::size_type position1 = Constraints.find("={eax}");
+  

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

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

tbaeder added a comment.

Thanks for the commits, Aaron. If you don't mind I have a short question: both 
commits don't seem to list the test cases, have they not been committed at all 
or are they filtered out in github and/or phabricator? Did I mess something up?


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

https://reviews.llvm.org/D93375

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

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

[Wenlei He via Phabricator via cfe-commits]

wenlei added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412
+
+  Remark << ";";
 }

nit: any special reason for adding this? doesn't seem consistent with other 
remarks we have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94333

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

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@nullptr.cpp: I may or may not come up with more review comments, but either 
way I'm a dead end — I don't consider myself authorized to "accept" Clang 
changes.
You're still going to have to attract the attention of someone with the 
authority and/or force-of-will to say "okay ship it."

I notice a lack of any explicitly `co_return`-related tests and/or code in this 
patch. I'm just going to assume that is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I'll leave this one to @aprantl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp updated this revision to Diff 316302.
nullptr.cpp added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1203,6 +1203,11 @@
   https://wg21.link/p0593r6";>P0593R6 (DR)
   Clang 11
 
+
+  More implicit moves
+  https://wg21.link/p1825r0";>P1825R0 (DR)
+  Clang 12
+
 
 
 
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,12 +1,17 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++20 -verify=cxx20 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 
 // definitions for std::move
 namespace std {
 inline namespace foo {
 template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
 
 template  typename remove_reference::type &&move(T &&t);
 } // namespace foo
@@ -76,11 +81,8 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
-Derived d3;
-return d3;  // e2-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying on older compilers}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d3)"
+  Derived d3;
+  return d3; // ok
 }
 ConstructFromBase test4() {
 Derived d4;
@@ -153,10 +155,7 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConstructFromDerived testParam3(Derived d) {
-return d;  // e7-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying on older compilers}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
+  return d; // ok
 }
 ConstructFromBase testParam4(Derived d) {
 return d;  // e8
@@ -218,8 +217,10 @@
 // But if the return type is a reference type, then moving would be wrong.
 Derived& testRetRef1(Derived&& d) { return d; }
 Base& testRetRef2(Derived&& d) { return d; }
+#if __cplusplus >= 201402L
 auto&& testRetRef3(Derived&& d) { return d; }
 decltype(auto) testRetRef4(Derived&& d) { return (d); }
+#endif
 
 // As long as we're checking parentheses, make sure parentheses don't disable the warning.
 Base testParens1() {
@@ -230,14 +231,10 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:15}:"std::move(d)"
 }
 ConstructFromDerived testParens2() {
-Derived d;
-return (d);  // e18-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:15}:"std::move(d)"
+  Derived d;
+  return (d); // ok
 }
 
-
 // If the target is a catch-handler parameter, do apply the diagnostic.
 void throw_derived();
 Derived testEParam1() {
@@ -339,7 +336,7 @@
 namespace test_delete {
 struct Base {
   Base();
-  Base(Base &&) = delete;
+  Base(Base &&) = delete; // cxx20-note {{'Base' has been explicitly marked deleted here}}
   Base(Base const &);
 };
 
@@ -347,6 +344,6 @@
 
 Base

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

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0066a09579ca: [libc++] Give extern templates default 
visibility on gcc (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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


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


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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D94391#2494531 , @dblaikie wrote:

> In D94391#2494316 , @MaskRay wrote:
>
>> I removed `CurLoc` from call sites and tried a stage 2 build. There is such 
>> a difference:
>>
>>   0x00062228:   DW_TAG_structure_type
>>   DW_AT_calling_convention(DW_CC_pass_by_value)
>>   DW_AT_name  ("__va_list_tag")
>>   DW_AT_byte_size (0x18)
>>   DW_AT_decl_file 
>> ("/home/maskray/llvm/clang/tools/driver/driver.cpp")
>>   DW_AT_decl_line (23)
>>
>> driver.cpp:23 is a `#include`. So this looks strange. The 
>> DW_AT_decl_file/DW_AT_decl_line attributes are undesired due to `CurLoc` 
>> getLineNumber.
>
> I don't understand what you're describing - could you describe it in more 
> detail/help me connect the dots?

The ObjC example `debug-info-blocks.m` is: `CurLoc` has been advanced to the 
closing brace. Then `getLineNumber` is called on `ImplicitVarParameter` and the 
code attaches the location of `}` to this implicit parameter which misses 
SourceLocation. The location is a bit arbitrary - perhaps the location of `^{` 
is better.

A worse example is that `CurLoc` moves further away and `getLineNumber` is used 
to attach the line information to a far-away structure. This may potentially 
unintentionally apply to future AST nodes.

The `DW_AT_decl_file` `__va_list_tag` example is about attaching the line of a 
`#include` to some declaration deep in the included hierarchy.

> It sounds like you're saying you removed the CurLoc fallback and then did a 
> stage 2 build of clang and found an example of worse debug info (I'm not sure 
> what file/line this struct was attributed to currently/without the change you 
> were experimenting with) - that would suggest to me that the CurLoc fallback 
> is helping, by providing a better location than the one you've mentioned here?

Clarified the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 316286.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Add another workaround. stage 2 -DCMAKE_BUILD_TYPE=Debug clang is byte 
identical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -493,11 +493,10 @@
 }
 
 unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
-  if (Loc.isInvalid() && CurLoc.isInvalid())
+  if (Loc.isInvalid())
 return 0;
   SourceManager &SM = CGM.getContext().getSourceManager();
-  PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
-  return PLoc.isValid() ? PLoc.getLine() : 0;
+  return SM.getPresumedLoc(Loc).getLine();
 }
 
 unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -1037,7 +1036,8 @@
   if (llvm::DIType *T = getTypeOrNull(CGM.getContext().getRecordType(RD)))
 return cast(T);
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
-  unsigned Line = getLineNumber(RD->getLocation());
+  const unsigned Line =
+  getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc);
   StringRef RDName = getClassName(RD);
 
   uint64_t Size = 0;
@@ -1358,7 +1358,7 @@
 
   // Get the location for the field.
   llvm::DIFile *file = getOrCreateFile(loc);
-  unsigned line = getLineNumber(loc);
+  const unsigned line = getLineNumber(loc.isValid() ? loc : CurLoc);
 
   uint64_t SizeInBits = 0;
   auto Align = AlignInBits;
@@ -3336,7 +3336,8 @@
 
   // Get overall information about the record type for the debug info.
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
-  unsigned Line = getLineNumber(RD->getLocation());
+  const unsigned Line =
+  getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc);
   StringRef RDName = getClassName(RD);
 
   llvm::DIScope *RDContext = getDeclContextDescriptor(RD);
@@ -3863,7 +3864,7 @@
   llvm::DISubprogram::DISPFlags SPFlagsForDef =
   SPFlags | llvm::DISubprogram::SPFlagDefinition;
 
-  unsigned LineNo = getLineNumber(Loc);
+  const unsigned LineNo = getLineNumber(Loc.isValid() ? Loc : CurLoc);
   unsigned ScopeLine = getLineNumber(ScopeLoc);
   llvm::DISubroutineType *DIFnType = getOrCreateFunctionType(D, FnType, Unit);
   llvm::DISubprogram *Decl = nullptr;
@@ -4366,7 +4367,8 @@
   Ty = CreateSelfType(VD->getType(), Ty);
 
   // Get location information.
-  unsigned Line = getLineNumber(VD->getLocation());
+  const unsigned Line =
+  getLineNumber(VD->getLocation().isValid() ? VD->getLocation() : CurLoc);
   unsigned Column = getColumnNumber(VD->getLocation());
 
   const llvm::DataLayout &target = CGM.getDataLayout();
@@ -4826,6 +4828,8 @@
   if (!NSDecl->isAnonymousNamespace() ||
   CGM.getCodeGenOpts().DebugExplicitImport) {
 auto Loc = UD.getLocation();
+if (!Loc.isValid())
+  Loc = CurLoc;
 DBuilder.createImportedModule(
 getCurrentContextDescriptor(cast(UD.getDeclContext())),
 getOrCreateNamespace(NSDecl), getOrCreateFile(Loc), 
getLineNumber(Loc));


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -493,11 +493,10 @@
 }
 
 unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
-  if (Loc.isInvalid() && CurLoc.isInvalid())
+  if (Loc.isInvalid())
 return 0;
   SourceManager &SM = CGM.getContext().getSourceManager();
-  PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
-  return PLoc.isValid() ? PLoc.getLine() : 0;
+  return SM.getPresumedLoc(Loc).getLine();
 }
 
 unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -1037,7 +1036,8 @@
   if (llvm::DIType *T = getTypeOrNull(CGM.getContext().getRecordType(RD)))
 return cast(T);
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
-  unsigned Line = getLineNumber(RD->getLocation());
+  const unsigned Line =
+  getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc);
   StringRef RDName = getClassName(RD);
 
   uint64_t Size = 0;
@@ -1358,7 +1358,7 @@
 
   // Get the location for the field.
   llvm::DIFile *file = getOrCreateFile(loc);
-  unsigned line = getLineNumber(loc);
+  const unsigned line = getLineNumber(loc.isValid() ? loc : CurLoc);
 
   uint64_t SizeInBits = 0;
   auto Align = AlignInBits;
@@ -3336,7 +3336,8 @@
 
   // Get overall information about the record type for the debug info.
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
-  unsigned Line = getLineNumber(RD->getLocation());
+  const unsigned Line =
+  getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc);
   StringRef RDName = getClassName(RD);

[PATCH] D94364: [clang] Allow specifying the aapcs and aapcs-vfp for windows on arm

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

I think that it makes more sense to ignore the `aapcs-vfp` calling convention 
attribute (returning `CCCR_Ignore`), and warn on the `aapcs`.  The redundant 
calling convention attribute does nothing, and I think that simply ignoring it 
would solve your issue.  However, changing the calling convention to `aapcs` 
would break interoperability, so warning on that seems reasonable still.  I 
think that if Wine is willing to adjust their use of the attribute, that would 
really be nicer - it allows us to keep the behaviour as close to MSVC as 
possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94364

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

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

[Jason Liu via Phabricator via cfe-commits]

jasonliu accepted this revision.
jasonliu added a comment.

LGTM with minor nits.




Comment at: clang/lib/Sema/SemaAttr.cpp:74
 return;
-  // The #pragma align/pack affected a record in an included file, so Clang
-  // should warn when that the pragma was written in a file that included the
+  // The #pragma align/pack affected a record in an included file,  so Clang
+  // should warn when that pragma was written in a file that included the





Comment at: clang/lib/Sema/SemaAttr.cpp:347-349
+AlignmentVal = CurVal.getPackNumber();
+if (!CurVal.IsPackSet())
   AlignmentVal = 8;

nit



Comment at: clang/lib/Sema/SemaAttr.cpp:372
+  // XL pragma pack does not support identifier syntax.
+  if (IsXLPragma && !SlotLabel.empty()) {
+Diag(PragmaLoc, diag::err_pragma_pack_identifer_not_supported);

Could we move this to the top of the function so that we could have a quick 
exit when we sees this?
If we are going to emit an error, the `AlignPackStack.Act(PragmaLoc, Action, 
StringRef(), Info);` is not necessary any more.



Comment at: clang/lib/Sema/SemaAttr.cpp:555
+  });
+  // If we found the label so pop from there.
+  if (I != Stack.rend()) {





Comment at: clang/lib/Sema/SemaAttr.cpp:581
+} else if (!Stack.empty()) {
+  // xl '#pragma align' sets the base line,
+  // and pragma pack cannot pop over the base line.





Comment at: clang/lib/Sema/SemaAttr.cpp:582
+  // xl '#pragma align' sets the base line,
+  // and pragma pack cannot pop over the base line.
+  if (Value.IsXLStack() && Value.IsPackAttr() && 
CurrentValue.IsAlignAttr())




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

https://reviews.llvm.org/D87702

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

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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I like that this makes getLineNumber behave less "magic". I'm slightly worried 
that this isn't quite as NFC as think (and the other cases are just not caught 
by the testsuite). Perhaps you could compile Clang (and a sample of Clang's 
ObjC(xx) tests) with the patch and diff the dwarfdump output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D94391#2494316 , @MaskRay wrote:

> I removed `CurLoc` from call sites and tried a stage 2 build. There is such a 
> difference:
>
>   0x00062228:   DW_TAG_structure_type
>   DW_AT_calling_convention(DW_CC_pass_by_value)
>   DW_AT_name  ("__va_list_tag")
>   DW_AT_byte_size (0x18)
>   DW_AT_decl_file 
> ("/home/maskray/llvm/clang/tools/driver/driver.cpp")
>   DW_AT_decl_line (23)
>
> driver.cpp:23 is a `#include`. So this looks strange. The 
> DW_AT_decl_file/DW_AT_decl_line attributes are undesired due to `CurLoc` 
> getLineNumber.

I don't understand what you're describing - could you describe it in more 
detail/help me connect the dots?

It sounds like you're saying you removed the CurLoc fallback and then did a 
stage 2 build of clang and found an example of worse debug info (I'm not sure 
what file/line this struct was attributed to currently/without the change you 
were experimenting with) - that would suggest to me that the CurLoc fallback is 
helping, by providing a better location than the one you've mentioned here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: arphaman, javed.absar.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Before I start, this should probably be on clangd-dev, but that list seems 
kinda dead so I've put my working here instead.

This definitely needs splitting up and tweaking most of the specifics but here 
is a proof of concept so to say.
The majority of changes here are:

- Change Drafts to hold ref counted strings.
- Add a ThreadsafeFS to Draftstore that overlays its contents over another 
ThreadsafeFS.
- Use said Filesystem in the ClangdServer to provide a way to access its 
contents in a much friendlier way.
- Add a command line option to control using that Filesystem when building 
pre-ambles.
- Always use this Filesystem when performing x-file renames and tweaks.

With all these changes I personally find the UX so much nicer.

I've put the Option for using DirtyBuffers when building pre-ambles as a hidden 
option for now, defaulted to off.
However as per LSP spec, we should think about in the future, once this is 
fully ready and tested to default it to on.
Also I know we shouldn't be using the command line really, when we have 
`.clangd` config instead.
This will be migrated to there in due time.


Repository:
  rG LLVM Github Monorepo

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(

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

[Fangrui Song 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 rGcf45731f0eae: [Driver] Fix assertion failure when 
-fprofile-generate -fcs-profile-generate… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

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


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


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

[clang] cf45731 - [Driver] Fix assertion failure when -fprofile-generate -fcs-profile-generate are used together

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-01-12T14:19:55-08:00
New Revision: cf45731f0eaead79e1ac501b397e330df41ec152

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

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

If conflicting `-fprofile-generate -fcs-profile-generate` are used together,
there is currently an assertion failure. Fix the failure.

Also add some driver tests.

Reviewed By: xur

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

Added: 
clang/test/Driver/fcs-profile-generate.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 4a20936ddda1..c03b513150b3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -766,9 +766,11 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << ProfileGenerateArg->getSpelling() << ProfileUseArg->getSpelling();
 
-  if (CSPGOGenerateArg && PGOGenerateArg)
+  if (CSPGOGenerateArg && PGOGenerateArg) {
 D.Diag(diag::err_drv_argument_not_allowed_with)
 << CSPGOGenerateArg->getSpelling() << PGOGenerateArg->getSpelling();
+PGOGenerateArg = nullptr;
+  }
 
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(

diff  --git a/clang/test/Driver/fcs-profile-generate.c 
b/clang/test/Driver/fcs-profile-generate.c
new file mode 100644
index ..6be7f758c3e6
--- /dev/null
+++ b/clang/test/Driver/fcs-profile-generate.c
@@ -0,0 +1,15 @@
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate %s 2>&1 
| FileCheck %s
+// CHECK:  "-fprofile-instrument=csllvm"
+// CHECK-NOT:  "-fprofile-instrument-path=
+// CHECK-SAME: "-fprofile-instrument-use-path=a.profdata"
+
+// RUN: %clang -### -c -fprofile-use=a.profdata -fcs-profile-generate=dir %s 
2>&1 | FileCheck %s --check-prefix=CHECK1
+// CHECK1: "-fprofile-instrument=csllvm"{{.*}} 
"-fprofile-instrument-path=dir{{/|}}default_%m.profraw" 
"-fprofile-instrument-use-path=a.profdata"
+
+/// Degradation case. This usage does not make much sense.
+// RUN: %clang -### -c -fcs-profile-generate %s 2>&1 | FileCheck %s 
--check-prefix=NOUSE
+// NOUSE: "-fprofile-instrument=csllvm"
+// NOUSE-NOT: "-fprofile-instrument-path=
+
+// RUN: %clang -### -c -fprofile-generate -fcs-profile-generate %s 2>&1 | 
FileCheck %s --check-prefix=CONFLICT
+// CONFLICT: error: invalid argument '-fcs-profile-generate' not allowed with 
'-fprofile-generate'



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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 316238.
MaskRay added a comment.

Fix test on Windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

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


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


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

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

[Rong Xu via Phabricator via cfe-commits]

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

looks good to me. Thanks for working on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

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

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

[Fangrui Song via Phabricator via cfe-commits]

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

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94463

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


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


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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



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

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

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



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

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



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

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



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

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



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

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



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

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



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

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



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

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


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

https://reviews.llvm.org/D94476

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

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

[Di Mo via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94333

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

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

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

[via cfe-commits]

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

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

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

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

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

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

Testing:
ninja check-llvm

Reviewed By: mtrofin

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

Added: 


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

Removed: 




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

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

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

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

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

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thank you for the reviewing @andreil99 !




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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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

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

[Sunil Srivastava via Phabricator via cfe-commits]

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94438

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


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


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

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

[Sunil Srivastava via cfe-commits]

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

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

LOG: Fix for crash in __builtin_return_address in template context.

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

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

Added: 


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

Removed: 




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

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



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

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

[David Blaikie via Phabricator via cfe-commits]

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

Sounds good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D94391#2493978 , @dblaikie wrote:

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



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

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

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

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



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

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



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

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



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93264

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

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

[Andrei Lebedev via Phabricator via cfe-commits]

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

Thanks for suggesting `DocBrief`, Richard!

Looks good to me with a nit.




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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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

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

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94381

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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

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


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


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

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

[Yitzhak Mandelbaum via cfe-commits]

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

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

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

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

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

Added: 


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

Removed: 




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



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

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

[Zequan Wu via Phabricator via cfe-commits]

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94537

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

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

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

[Zequan Wu via cfe-commits]

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

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

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

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

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

Added: 


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

Removed: 




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

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

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

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

[David Blaikie via Phabricator via cfe-commits]

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

In D94391#2491229 , @MaskRay wrote:

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391

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

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

[David Truby via Phabricator via cfe-commits]

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92751

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

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

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

[David Truby via cfe-commits]

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

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

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

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

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

Co-authored-by: Peter Waller 

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

Added: 


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

Removed: 




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

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

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

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

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

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!




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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94537

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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94237

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

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

[Zequan Wu via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94537

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

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

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

[Louis Dionne via Phabricator via cfe-commits]

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

In D35388#2493696 , @smeenai wrote:

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

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

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

[Wei Mi via Phabricator via cfe-commits]

wmi added inline comments.



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

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



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

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



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93264

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

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

[Nathan Ridge via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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


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


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

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

[Nathan Ridge via cfe-commits]

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

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

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

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

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

Added: 


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

Removed: 




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

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

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



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

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

[Tony Tye via Phabricator via cfe-commits]

t-tye added a comment.

In D93525#2493024 , @yaxunl wrote:

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525

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

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

[Andrzej Warzynski via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94533

Files:
  clang/cmake/modules/CMakeLists.txt


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


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

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

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

steakhal added a comment.

In D93224#2493515 , @xazax.hun wrote:

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

Yes, I was referring to that.

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

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

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

This element of the patch stack implements exactly that.

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

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

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

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


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

https://reviews.llvm.org/D93224

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

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93922

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

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

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

dexonsmith added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

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

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

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

dexonsmith added a comment.

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94474

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

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

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

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

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

[Shoaib Meenai via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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


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


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

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

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

dexonsmith added inline comments.



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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94474

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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Reworked, per our discussion. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

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

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

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

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

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




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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94488

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

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

[Aaron Ballman via cfe-commits]

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

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

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

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

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

Added: 


Modified: 
clang/lib/Lex/PPMacroExpansion.cpp

Removed: 




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



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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94453

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


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


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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

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


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

https://reviews.llvm.org/D93375

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

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

[Aaron Ballman via cfe-commits]

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

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

LOG: Add -ansi option to CompileOnly group

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

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

Added: 


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

Removed: 




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



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

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

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93829

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

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

[Sam McCall via Phabricator via cfe-commits]

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94503

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

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

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

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

tbaeder added a comment.

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


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

https://reviews.llvm.org/D93375

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D93375#2493480 , @tbaeder wrote:

> And thank you for reviewing my patches :)

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


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

https://reviews.llvm.org/D93375

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

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

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

xazax.hun added a comment.

In D93224#2493434 , @steakhal wrote:

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

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

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

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


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

https://reviews.llvm.org/D93224

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

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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Ping x 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92797

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

[clang] dd95577 - Fix typo in diagnostic message

[Akira Hatanaka via cfe-commits]

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

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

LOG: Fix typo in diagnostic message

rdar://66684531

Added: 


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

Removed: 




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



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

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

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

tbaeder added a comment.

And thank you for reviewing my patches :)


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

https://reviews.llvm.org/D93375

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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for this!


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

https://reviews.llvm.org/D93375

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

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

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

steakhal added a comment.

Updates:

- Rebased.

---

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

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




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

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


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

https://reviews.llvm.org/D93224

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

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

[Utkarsh Saxena via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94513

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


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


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

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

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

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

Updates:

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


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

https://reviews.llvm.org/D93223

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

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

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

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

tbaeder updated this revision to Diff 316118.

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

https://reviews.llvm.org/D93375

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


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


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

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

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

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

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


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

https://reviews.llvm.org/D93375

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


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


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

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

[Sam McCall via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94503

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

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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525

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

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

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.



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

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



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

bar or tar?



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

Can you add anonymous namespaces as well?



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

s/for/while?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94424

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

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

[Tim Wojtulewicz via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94500

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

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

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

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

xazax.hun added inline comments.



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

A pointer pair might be small enough for a DenseMap?



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

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


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

https://reviews.llvm.org/D94476

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

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

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92935

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

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

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

[Nemanja Ivanovic via cfe-commits]

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

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

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

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

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

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

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

Added: 


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

Removed: 




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

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

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

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

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

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

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

steakhal added a comment.

Seems pretty straightforward and clean.

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

I think these cases should be tested as well:

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

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




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

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



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

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



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

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



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





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





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

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

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



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

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



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

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



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

Should Desc and ShortDesc be the same?



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

Why not construct in place?



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

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



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

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D35388

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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for looking into this feature!

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93829

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

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

[Andrzej Warzynski via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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


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


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

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

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Edge cases fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94473

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

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

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

[Lukas Barth via Phabricator via cfe-commits]

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

Fixed formatting issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a few small issues to fix.




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

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



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

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

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


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

https://reviews.llvm.org/D87702

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

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

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

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

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

[Michael Kiausch via Phabricator via cfe-commits]

kiausch added a comment.

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


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

https://reviews.llvm.org/D92935

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

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

[Bjorn Pettersson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc4944a6f53f6: [Fixed Point] Add codegen for conversion 
between fixed-point and floating point. (authored by ebevhan, committed by 
bjope).
Herald added a subscriber: dexonsmith.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

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

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

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

[Bjorn Pettersson via cfe-commits]

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

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

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

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

This depends on D54749.

Reviewed By: leonardchan

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

Added: 
clang/test/Frontend/fixed_point_conversions_half.c

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

Removed: 




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

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

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

[Sam McCall via Phabricator via cfe-commits]

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

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

Thanks for fixing this!




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

this pair should be a struct instead - MacroOccurrence?



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

this should probably also be MacroOccurrence?



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

this fixme is now addressed



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

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



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477

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

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

[Richard Barton via Phabricator via cfe-commits]

richard.barton.arm added a comment.

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94169

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