[PATCH] D14484: [clang-format] Formatting constructor initializer lists by putting them always on different lines

[Nikolai Hlubek via Phabricator via cfe-commits]

nhlubek added a comment.

In order to push this forward I have written a bug ticket that highlights this 
issue:

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

The patch is also really simple with the clang-format options that are 
available nowadays
https://github.com/Nikolai-Hlubek/clang/tree/ConstructorInitializer_AlwaysBreakAfterColon


Repository:
  rL LLVM

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

https://reviews.llvm.org/D14484

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

[PATCH] D103025: [analyzer] Handle NTTP invocation in CallContext.getCalleeDecl()

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

Ping,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103025

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

[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:895
 
+using VisitorCallback = llvm::function_ref;

`function_ref` is a reference, it doesn't own the function. It means that it 
shouldn't outlive the actual function object (very similar to `StringRef`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103434

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

[PATCH] D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

[Nikolai Hlubek via Phabricator via cfe-commits]

nhlubek added a comment.

I tried to highlight the story with this bug that this commit tries to fix in 
the following bug report:
https://bugs.llvm.org/show_bug.cgi?id=50549

It goes back to 2015 and several people have tried to come up with solutions. 
The solutions are not complicated but due to the complex nature of C++ 
formatting and the clang-format options everyone so far has failed a code 
review and doesn't really know how to proceed.
I think this can only be resolved by a clang-format core dev who needs to adopt 
the changes in a form suitable for clang-format. A small task that will make a 
lot of people happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90232

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

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 348900.
DmitryPolukhin marked 2 inline comments as done.
DmitryPolukhin added a comment.

Resolved comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Lex/HeaderSearch.h"
+#include "HeaderMapTestUtils.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -45,6 +46,21 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void addHeaderMap(llvm::StringRef Filename,
+std::unique_ptr Buf) {
+VFS->addFile(Filename, 0, std::move(Buf), /*User=*/None, /*Group=*/None,
+ llvm::sys::fs::file_type::regular_file);
+auto FE = FileMgr.getFile(Filename, true);
+assert(FE);
+
+// Test class supports only one HMap at a time.
+assert(!HMap);
+HMap = HeaderMap::Create(*FE, FileMgr);
+auto DL =
+DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
+Search.AddSearchPath(DL, /*isAngled=*/false);
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -55,6 +71,7 @@
   std::shared_ptr TargetOpts;
   IntrusiveRefCntPtr Target;
   HeaderSearch Search;
+  std::unique_ptr HMap;
 };
 
 TEST_F(HeaderSearchTest, NoSearchDir) {
@@ -136,5 +153,31 @@
 "y/z/t.h");
 }
 
+// Helper struct with null terminator character to make MemoryBuffer happy.
+template 
+struct NullTerminatedFile : public FileTy {
+  PaddingTy Padding = 0;
+};
+
+TEST_F(HeaderSearchTest, HeaderMapReverseLookup) {
+  typedef NullTerminatedFile, char> FileTy;
+  FileTy File;
+  File.init();
+
+  test::HMapFileMockMaker Maker(File);
+  auto a = Maker.addString("d.h");
+  auto b = Maker.addString("b/");
+  auto c = Maker.addString("c.h");
+  Maker.addBucket("d.h", a, b, c);
+
+  addHeaderMap("/x/y/z.hmap", File.getBuffer());
+  addSearchDir("/a");
+
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c.h",
+   /*WorkingDir=*/"",
+   /*MainFile=*/""),
+"d.h");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1834,7 +1834,7 @@
   };
 
   for (unsigned I = 0; I != SearchDirs.size(); ++I) {
-// FIXME: Support this search within frameworks and header maps.
+// FIXME: Support this search within frameworks.
 if (!SearchDirs[I].isNormalDir())
   continue;
 
@@ -1848,6 +1848,19 @@
   if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
 *IsSystem = false;
 
+  // Try resolving resulting filename via reverse search in header maps,
+  // key from header name is user prefered name for the include file.
+  StringRef Filename = File.drop_front(BestPrefixLength);
+  for (unsigned I = 0; I != SearchDirs.size(); ++I) {
+if (!SearchDirs[I].isHeaderMap())
+  continue;
 
-  return path::convert_to_slash(File.drop_front(BestPrefixLength));
+StringRef SpelledFilename =
+SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
+if (!SpelledFilename.empty()) {
+  Filename = SpelledFilename;
+  break;
+}
+  }
+  return path::convert_to_slash(Filename);
 }
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -240,3 +240,27 @@
 return StringRef(DestPath.begin(), DestPath.size());
   }
 }
+
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+  if (!ReverseMap.empty())
+return ReverseMap.lookup(DestPath);
+
+  const HMapHeader &Hdr = getHeader();
+  unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
+  for (unsigned i = 0; i != NumBuckets; ++i) {
+HMapBucket B = getBucket(i);
+if (B.Key == HMAP_EmptyBucketKey)
+  continue;
+
+Optional Key = getString(B.Key);
+Optional Prefix = getString(B.Prefix);
+Optional Suffix = getString(B.Suffix);
+if (Key && Prefix && Suffix) {
+  SmallVector Buf;
+  Buf.append(Prefix->begin(), Prefix->end());
+  Buf.append(Suffix->begin(), Suffix->end());
+  ReverseMap[StringRef(Buf.begin(), Buf.size())] = *Key;
+}
+  }
+  return ReverseMap.lookup(DestPath);
+}
Index: clang/include/cl

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added inline comments.



Comment at: clang/lib/Lex/HeaderMap.cpp:245
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+  if (ReverseMap.empty()) {
+const HMapHeader &Hdr = getHeader();

bruno wrote:
> Please rewrite this to early return in case this isn't empty. 
Done, but it causes a bit of code duplication due to second lookup. Please let 
me know if you thought about rewriting it somehow else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

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

[clang-tools-extra] 5b74719 - [clangd] Fix -Wunused-variable warning (NFC)

[Yang Fan via cfe-commits]

Author: Yang Fan
Date: 2021-06-01T16:15:09+08:00
New Revision: 5b747197f8fb83bb7c256fa6cb2010445deb0a85

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

LOG: [clangd] Fix -Wunused-variable warning (NFC)

GCC warning:
```
/llvm-project/clang-tools-extra/clangd/InlayHints.cpp: In member function ‘bool 
clang::clangd::InlayHintVisitor::VisitVarDecl(clang::VarDecl*)’:
/llvm-project/clang-tools-extra/clangd/InlayHints.cpp:81:15: warning: unused 
variable ‘AT’ [-Wunused-variable]
   81 | if (auto *AT = D->getType()->getContainedAutoType()) {
  |   ^~

```

Added: 


Modified: 
clang-tools-extra/clangd/InlayHints.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 9fbce419edd2..d0e0e961abcf 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -78,7 +78,7 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 if (isa(D))
   return true;
 
-if (auto *AT = D->getType()->getContainedAutoType()) {
+if (D->getType()->getContainedAutoType()) {
   if (!D->getType()->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type



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

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Manas Gupta via Phabricator via cfe-commits]

manas created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: teemperor.
manas requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add test cases for symbolic addition operator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103440

Files:
  clang/test/Analysis/constant-folding.c


Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -251,3 +251,57 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testAdditionRules(unsigned int a, unsigned int b, int c, int d) {
+  if (a == 0) {
+clang_analyzer_eval((a + 0) == 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for overflows
+  if (a >= INT_MAX) {
+clang_analyzer_eval((a + 0) >= 0); // expected-warning{{TRUE}}
+
+if (a > INT_MAX) {
+  clang_analyzer_eval((a + b) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + b) > 0); // expected-warning{{FALSE}}
+}
+  }
+
+  // Both operands are positive
+  if (a >= 0 && b >= 0) {
+clang_analyzer_eval((a + b) < 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) < UINT_MAX); // expected-warning{{UNKNOWN}}
+
+if (a <= UINT_MAX && b <= UINT_MAX) {
+  clang_analyzer_eval((a + b) < 0); // expected-warning{{UNKNOWN}}
+}
+  }
+
+  // Checks for unsigned ints
+  if (a == UINT_MAX && b == UINT_MAX) {
+clang_analyzer_eval((a + b) >= 0); // expected-warning{{FALSE}}
+  }
+
+  // Checks for negative signed ints
+  if (c < 0 && d < 0) {
+clang_analyzer_eval((c + d) == 0); // expected-warning{{TRUE}}
+
+if (c > INT_MIN) {
+  clang_analyzer_eval((c + d) == 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((c + d) == INT_MAX<<1); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for positive ints
+  if (c > 0 && d > 0) {
+clang_analyzer_eval((c + d) != 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) < 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for inclusive ranges
+  if (a >= 0 && a <= 10 && b >= -20 && b <= 20) {
+clang_analyzer_eval((a + b) >= -20); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) <= 30); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) == -20); // expected-warning{{TRUE}}
+  }
+}


Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -251,3 +251,57 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testAdditionRules(unsigned int a, unsigned int b, int c, int d) {
+  if (a == 0) {
+clang_analyzer_eval((a + 0) == 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for overflows
+  if (a >= INT_MAX) {
+clang_analyzer_eval((a + 0) >= 0); // expected-warning{{TRUE}}
+
+if (a > INT_MAX) {
+  clang_analyzer_eval((a + b) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + b) > 0); // expected-warning{{FALSE}}
+}
+  }
+
+  // Both operands are positive
+  if (a >= 0 && b >= 0) {
+clang_analyzer_eval((a + b) < 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) < UINT_MAX); // expected-warning{{UNKNOWN}}
+
+if (a <= UINT_MAX && b <= UINT_MAX) {
+  clang_analyzer_eval((a + b) < 0); // expected-warning{{UNKNOWN}}
+}
+  }
+
+  // Checks for unsigned ints
+  if (a == UINT_MAX && b == UINT_MAX) {
+clang_analyzer_eval((a + b) >= 0); // expected-warning{{FALSE}}
+  }
+
+  // Checks for negative signed ints
+  if (c < 0 && d < 0) {
+clang_analyzer_eval((c + d) == 0); // expected-warning{{TRUE}}
+
+if (c > INT_MIN) {
+  clang_analyzer_eval((c + d) == 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((c + d) == INT_MAX<<1); // expected-warning{{TRUE}}
+}
+  }
+
+  // Checks for positive ints
+  if (c > 0 && d > 0) {
+clang_analyzer_eval((c + d) != 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) < 0); // expected-warning{{TRUE}}
+  }
+
+  // Checks for inclusive ranges
+  if (a >= 0 && a <= 10 && b >= -20 && b <= 20) {
+clang_analyzer_eval((a + b) >= -20); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) <= 30); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) == -20); // expected-warning{{TRUE}}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103393: [clangd] Bump recommended gRPC version (1.33.2 -> 1.36.3)

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Makes sense, thanks! Putting this on hold.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103393

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Thanks for taking this up! I never got the time for it.

I'd like to see the test check that there are no unused argument warnings when 
there are multiple values. I missed that with another patch in this area.

Beyond that I'm not sure the logic is right here. You've got `-Wa,-march` being 
additive, in contrast to the compiler option which only uses the last value:

  $ ./bin/clang --target=aarch64-arm-none-eabi -march=armv8.1-a 
-march=armv8.2-a /tmp/test.c -###
  <...> "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" 
"+v8.2a" <...>

Maybe you're working to make some specific build system work but from the clang 
side we'd want consistent behaviour for Arm and AArch64 options.

Let me restate the Arm behaviour, minus the `mcpu` stuff that was also in that 
patch:

- Only compiler options apply to non assembly files
- Compiler and assembler options apply to assembly files
- For assembly files, if you have both kinds of option, you prefer the 
assembler option(s)
- Of the options that apply (or are preferred), the last value wins (it's not 
additive)

Do you disagree with that set of rules?

I wouldn't think that AArch64 would have that different needs, but then again 
I'm not trying to build existing projects, just make the clang side logical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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

[PATCH] D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> A small task that will make a lot of people happy

Its not the happy people that concern me, its those who end up not happy that 
cause us the most pain ;-)

1. I question if the rst was generated via docs/tools/dump_format_style.py or 
by hand, it has to be generated so it stay in lock step.
2. We need to keep the old style and just make it initialise your new style if 
present, the easiest way of doing that is take the old style to be an enum, 
have the parser handle true/false, then provide a mapping of the name "if you 
cannot stomach the old option name"
  - people use the latest version doc to look things up, they don't always find 
the doc for their version
  - we cannot have the 100,000 of uses of the old option cause a runtime 
failure or cause people to be confused because they cannot find it in the docs 
(we have to give them a reference even it its a "use this now")
  - we can add a new style and put a big note on the old name to say its been 
deprecated

3. Fundamentally this patch looks ok, I would like to see more cases where the 
`:` wasn't always on a new line
4. I'd like to know how this is impacted by the new "AfterComma" style


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90232

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

[PATCH] D103097: Add DWARF address spaces mapping for SPIR

[Jason Zheng via Phabricator via cfe-commits]

jzzheng22 updated this revision to Diff 348910.
jzzheng22 added a comment.

Simplified test following review comments.


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

https://reviews.llvm.org/D103097

Files:
  clang/lib/Basic/Targets/SPIR.h
  clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl


Index: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir-unknown-unknown -o - %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 
-emit-llvm -O0 -triple spir64-unknown-unknown -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GLOBAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 1)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_CONSTANT:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 2)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_LOCAL:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 3)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_PRIVATE:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 0)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GENERIC:[0-9]+]] = !DIDerivedType(tag: 
DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, 
dwarfAddressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], 
isLocal: false, isDefinition: true)
+global int *FileVar0;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar1", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_CONSTANT]], 
isLocal: false, isDefinition: true)
+constant int *FileVar1;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar2", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_LOCAL]], 
isLocal: false, isDefinition: true)
+local int *FileVar2;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar3", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_PRIVATE]], 
isLocal: false, isDefinition: true)
+private int *FileVar3;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar4", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GENERIC]], 
isLocal: false, isDefinition: true)
+int *FileVar4;
Index: clang/lib/Basic/Targets/SPIR.h
===
--- clang/lib/Basic/Targets/SPIR.h
+++ clang/lib/Basic/Targets/SPIR.h
@@ -117,6 +117,11 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
+  Optional
+  getDWARFAddressSpace(unsigned AddressSpace) const override {
+return AddressSpace;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 return (CC == CC_SpirFunction || CC == CC_OpenCLKernel) ? CCCR_OK
 : CCCR_Warning;


Index: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 -emit-llvm -O0 -triple spir-unknown-unknown -o - %s | FileCheck %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -debug-info-kind=limited -dwarf-version=5 -emit-llvm -O0 -triple spir64-unknown-unknown -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GLOBAL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 1)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_CONSTANT:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 2)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_LOCAL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 3)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_PRIVATE:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 0)
+// CHECK-DAG: ![[DWARF_ADDRESS_SPACE_GENERIC:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, dwarfAddressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], isLocal: false, isDefinition: true)
+global int *FileVar0;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar1

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:405
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");

This means we now have two separate places that set 
`OpenCLGenericAddressSpace`, the other place being in 
`CompilerInvocation::setLangDefaults()`.  That feels like a maintenance hazard. 
 Do you think it makes sense to set this field in one single place instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

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

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo 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 rG82f92e35c646: [WebAssembly][CodeGen] IR support for 
WebAssembly local variables (authored by wingo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-

[PATCH] D102689: [C++4OpenCL] Allow address space conversion in reinterpret_cast

[Ole Strohm via Phabricator via cfe-commits]

olestrohm added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:2359
 
-  if (SrcType == DestType) {
+  if (SrcType == Self.Context.removeAddrSpaceQualType(DestType)) {
 // C++ 5.2.10p2 has a note that mentions that, subject to all other

rjmccall wrote:
> I think the upshot of the conversation Richard and I just had is that there's 
> a bug here for all qualifiers; that is, `reinterpret_cast(5)` 
> should be allowed.  Probably the right thing to do is to strip qualifiers 
> from DestType in the CastOperation constructor when the type is not a class 
> or an array.  Richard, do you agree?  This isn't strictly implied by the C++ 
> wording since the semantic analysis of the cast is logically prior to the 
> introduction of a qualifier pr-value expression.
Ah, I see, I did indeed misunderstand the conversation above. I see now that 
with what Richard mentioned with `If a prvalue initially has the type “cv T”, 
where T is a cv-unqualified non-class, non-array type, the type of the 
expression is adjusted to T prior to any further analysis.` it makes sense to 
strip qualifiers for those cases. I also checked out gcc to compare to another 
implementation, and it does work there, so this behaviour doesn't seem 
unreasonable.

I'll rework this to strip all qualifiers somewhere in the CastOperation 
constructor as you suggested then.


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

https://reviews.llvm.org/D102689

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

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:732
+if (Adjuster)
   DiagLevel = Adjuster(DiagLevel, Info);
 

If I'm reading this right:
 - we previously discarded the diagnostic "quickly" without creating a Diag
 - now, we construct the Diag as normal but never emit it
 - ... but we still use the old early-exit codepath for notes, if the primary 
diagnostic will be discarded

The new scheme seems functionally correct, but it potentially constructs more 
Diags. Question is, is it ever enough to care about performance?

I believe *warnings in headers* are ultimately filtered out by the adjuster and 
are now extra work/allocations after this patch. Mostly this only affects 
preamble (exceptions: things like llvm tablegen includes) which is already 
slow, maybe it doesn't matter.

If you want to avoid regressing it, I'd suggest keeping the early bail out 
here, but instead of setting LastPrimaryDiagnosticWasSuppressed, just construct 
an empty Diag and set its level to Ignored. Rest of the logic can stay the same 
I think.

Doesn't seem like much extra code, but if you don't think it's worthwhile 
that's fine too of course.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:785
 
-  if (isExcluded(*LastDiag))
+  if (LastDiag->Severity == DiagnosticsEngine::Ignored || 
isExcluded(*LastDiag))
 return;

(Hmm, the isExcluded case seems like it belongs next to the level adjuster 
logic rather than at the end.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103387

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

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.

Thanks for fixing!




Comment at: clang-tools-extra/clangd/CompileCommands.cpp:225
 
+  Cmd = tooling::getInsertArgumentAdjuster(
+  ToAppend, tooling::ArgumentInsertPosition::END)(Cmd, "");

This extra copy is really silly :-(
please `move(ToAppend)` and make the whole thing conditional on 
`!ToAppend.empty()` (even though that'll be pretty much always...). Sadly 
move(Cmd) won't work.

One day if we feel like shaving a yak, we could add migrate to an ArgsAdjuster 
interface that takes a mutable CompileCommand&...



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:271
   Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
+  auto It = llvm::find(Args, "--");

Hmm, with this new logic in place, if we "normalized" command lines by moving 
the filename to the end, we'd resolve 
https://github.com/clangd/clangd/issues/555 without having to complicate the 
config file model.

I wonder how hard that is...



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:272
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
-  Args.insert(Args.end(), Add.begin(), Add.end());
+  auto It = llvm::find(Args, "--");
+  Args.insert(It, Add.begin(), Add.end());

maybe a comment "The point to insert at - usually the end"?
(Since this looks suspiciously like an unhandled error case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99523

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

[PATCH] D103377: [clangd] Add ability to change storage directory of index files

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is definitely an interface that makes sense for the purpose you describe 
(profiling index performance) but I'm less convinced it makes sense for 
clangd's *primary* use cases.

For example, our default policy gives different answers for code in different 
paths, which has some desirable properties (e.g. once you stop working on a 
project, its index files can be removed in an obvious way without disturbing 
other projects).
A mechanism that allows customization but has its own limitations can add 
confusion without actually solving many more people's problems.
And generally we're trying to phase out use of global command line flags in 
favor of config files while carefully limiting the amount of options users need 
to understand.

Given that there are some workarounds available (compile-commands dir, making 
the `.cache` directory a symlink, maybe even using `clangd-indexer` instead 
depending on your purpose) I think we probably shouldn't add this option, or 
should at least defer it until we have more info on what kind of customization 
people need while using clangd together with an editor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103377

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

[PATCH] D103252: [C++4OpenCL] Fix missing address space on implicit move assignment operator

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

> I also added a new test file to check the output AST. If there's a better 
> place for this test I will move it there.

If you're primarily interested in the AST, then it doesn't have to be a Sema 
test I guess.  In that case `clang/test/AST` seems to be a better place, and 
you could also remove the `-verify` parts of the test perhaps (unless you think 
it adds value that isn't tested elsewhere)?




Comment at: clang/test/SemaOpenCLCXX/addrspace-assignment.clcpp:10
+// expected-note@+1{{candidate function (the implicit move assignment 
operator) not viable: no known conversion from 'int' to '__generic Implicit' 
for 1st argument}}
+struct Implicit {};
+

Maybe rename this struct to avoid any potential confusion with the `implicit` 
keyword in the dumps above?  Even something as simple as `S` should do I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103252

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

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Hey, great start!
I added my comments inline and other mentors as reviewers.




Comment at: clang/test/Analysis/constant-folding.c:275
+
+if (a <= UINT_MAX && b <= UINT_MAX) {
+  clang_analyzer_eval((a + b) < 0); // expected-warning{{UNKNOWN}}

I think this is always true.



Comment at: clang/test/Analysis/constant-folding.c:287
+  if (c < 0 && d < 0) {
+clang_analyzer_eval((c + d) == 0); // expected-warning{{TRUE}}
+

I'm not sure I understand why do you make this assumption.



Comment at: clang/test/Analysis/constant-folding.c:290-291
+if (c > INT_MIN) {
+  clang_analyzer_eval((c + d) == 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval((c + d) == INT_MAX<<1); // expected-warning{{TRUE}}
+}

Here is the same note, `clang_analyzer_eval` doesn't produce `TRUE` only 
because this condition might be true.



Comment at: clang/test/Analysis/constant-folding.c:298
+clang_analyzer_eval((c + d) != 0); // expected-warning{{TRUE}}
+clang_analyzer_eval((c + d) < 0); // expected-warning{{TRUE}}
+  }

If `c == d == 1` this doesn't hold. Did you want to check here that we account 
for overflows?



Comment at: clang/test/Analysis/constant-folding.c:302
+  // Checks for inclusive ranges
+  if (a >= 0 && a <= 10 && b >= -20 && b <= 20) {
+clang_analyzer_eval((a + b) >= -20); // expected-warning{{TRUE}}

What about other cases here? I mean specifically when two ranges are have a 
form `[A, B]`



Comment at: clang/test/Analysis/constant-folding.c:305
+clang_analyzer_eval((a + b) <= 30); // expected-warning{{TRUE}}
+clang_analyzer_eval((a + b) == -20); // expected-warning{{TRUE}}
+  }

`clang_analyzer_eval` returns `TRUE` only if we can deduct that this expression 
on a particular path is **always** true.  Here it's not going to hold.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103440

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

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This is causing weird code patterns in various places and I can't see
any difference between None and empty change list. Neither in the current use
cases nor in the spec.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103449

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


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,10 @@
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
+  if (WE.changes.empty())
 return llvm::json::Object{};
   llvm::json::Object FileChanges;
-  for (auto &Change : *WE.changes)
+  for (auto &Change : WE.changes)
 FileChanges[Change.first] = llvm::json::Array(Change.second);
   return llvm::json::Object{{"changes", std::move(FileChanges)}};
 }
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -372,8 +372,7 @@
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);
   Action.edit.emplace();
-  Action.edit->changes.emplace();
-  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()};
   return Action;
 }
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -748,9 +748,8 @@
   return Reply(std::move(Err));
 
 WorkspaceEdit WE;
-WE.changes.emplace();
 for (const auto &It : R->ApplyEdits) {
-  (*WE.changes)[URI::createFile(It.first()).toString()] =
+  WE.changes[URI::createFile(It.first()).toString()] =
   It.second.asTextEdits();
 }
 // ApplyEdit will take care of calling Reply().
@@ -813,22 +812,20 @@
   if (!Server->getDraft(File))
 return Reply(llvm::make_error(
 "onRename called for non-added file", ErrorCode::InvalidParams));
-  Server->rename(
-  File, Params.position, Params.newName, Opts.Rename,
-  [File, Params, Reply = std::move(Reply),
-   this](llvm::Expected R) mutable {
-if (!R)
-  return Reply(R.takeError());
-if (auto Err = validateEdits(*Server, R->GlobalChanges))
-  return Reply(std::move(Err));
-WorkspaceEdit Result;
-Result.changes.emplace();
-for (const auto &Rep : R->GlobalChanges) {
-  (*Result.changes)[URI::createFile(Rep.first()).toString()] =
-  Rep.second.asTextEdits();
-}
-Reply(Result);
-  });
+  Server->rename(File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+  this](llvm::Expected R) mutable {
+   if (!R)
+ return Reply(R.takeError());
+   if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+   WorkspaceEdit Result;
+   for (const auto &Rep : R->GlobalChanges) {
+ Result.changes[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+   }
+   Reply(Result);
+ });
 }
 
 void ClangdLSPServer::onDocumentDidClose(


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/cla

[PATCH] D102849: [flang][driver] Add support for the "-init-only" option

[Chirag Khandelwal via Phabricator via cfe-commits]

AMDChirag added a comment.

LGTM. I'll let someone else take a look too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102849

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

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:405
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");

svenvh wrote:
> This means we now have two separate places that set 
> `OpenCLGenericAddressSpace`, the other place being in 
> `CompilerInvocation::setLangDefaults()`.  That feels like a maintenance 
> hazard.  Do you think it makes sense to set this field in one single place 
> instead?
I think we should try to set it in `CompilerInvocation.cpp` directly, we should 
be able to query `TargetOptions` there. Although that place is expected to be 
for the language-specific defaults but we broke the standard flow by having the 
language mode controlled by the target settings anyway.

I can't remember though why we have decided to add dedicated `LangOpts` entries 
for generic address space instead of just using `OpenCLOptions` from `Sema`? I 
think it simplifies the handling of some builtin functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

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

[PATCH] D103452: [clang] Fix reading long doubles with va_arg on x86_64 mingw

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, mati865.
Herald added a subscriber: pengfei.
mstorsjo requested review of this revision.
Herald added a project: clang.

On x86_64 mingw, long doubles are always passed indirectly as
arguments (see an existing case in WinX86_64ABIInfo::classify); we
need to take this into account when manually reading back arguments
with `va_arg` too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103452

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/mingw-long-double.c


Index: clang/test/CodeGen/mingw-long-double.c
===
--- clang/test/CodeGen/mingw-long-double.c
+++ clang/test/CodeGen/mingw-long-double.c
@@ -45,3 +45,16 @@
 // GNU32: declare dso_local void @__mulxc3
 // GNU64: declare dso_local void @__mulxc3
 // MSC64: declare dso_local void @__muldc3
+
+void VarArgLD(int a, ...) {
+  // GNU32-LABEL: define{{.*}} void @VarArgLD
+  // GNU64-LABEL: define{{.*}} void @VarArgLD
+  // MSC64-LABEL: define{{.*}} void @VarArgLD
+  __builtin_va_list ap;
+  __builtin_va_start(ap, a);
+  long double LD = __builtin_va_arg(ap, long double);
+  // GNU32: bitcast i8* %argp.cur to x86_fp80*
+  // GNU64: bitcast i8* %argp.cur to x86_fp80**
+  // MSC64: bitcast i8* %argp.cur to double*
+  __builtin_va_end(ap);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4366,6 +4366,16 @@
   if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {
 uint64_t Width = getContext().getTypeSize(Ty);
 IsIndirect = Width > 64 || !llvm::isPowerOf2_64(Width);
+  } else if (IsMingw64) {
+if (const BuiltinType *BT = Ty->getAs()) {
+  if (BT->getKind() == BuiltinType::LongDouble) {
+// Mingw64 GCC uses the old 80 bit extended precision floating point
+// unit. It passes them indirectly through memory.
+const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
+if (LDF == &llvm::APFloat::x87DoubleExtended())
+  IsIndirect = true;
+  }
+}
   }
 
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,


Index: clang/test/CodeGen/mingw-long-double.c
===
--- clang/test/CodeGen/mingw-long-double.c
+++ clang/test/CodeGen/mingw-long-double.c
@@ -45,3 +45,16 @@
 // GNU32: declare dso_local void @__mulxc3
 // GNU64: declare dso_local void @__mulxc3
 // MSC64: declare dso_local void @__muldc3
+
+void VarArgLD(int a, ...) {
+  // GNU32-LABEL: define{{.*}} void @VarArgLD
+  // GNU64-LABEL: define{{.*}} void @VarArgLD
+  // MSC64-LABEL: define{{.*}} void @VarArgLD
+  __builtin_va_list ap;
+  __builtin_va_start(ap, a);
+  long double LD = __builtin_va_arg(ap, long double);
+  // GNU32: bitcast i8* %argp.cur to x86_fp80*
+  // GNU64: bitcast i8* %argp.cur to x86_fp80**
+  // MSC64: bitcast i8* %argp.cur to double*
+  __builtin_va_end(ap);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4366,6 +4366,16 @@
   if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {
 uint64_t Width = getContext().getTypeSize(Ty);
 IsIndirect = Width > 64 || !llvm::isPowerOf2_64(Width);
+  } else if (IsMingw64) {
+if (const BuiltinType *BT = Ty->getAs()) {
+  if (BT->getKind() == BuiltinType::LongDouble) {
+// Mingw64 GCC uses the old 80 bit extended precision floating point
+// unit. It passes them indirectly through memory.
+const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
+if (LDF == &llvm::APFloat::x87DoubleExtended())
+  IsIndirect = true;
+  }
+}
   }
 
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103191: [OpenCL] Add support of __opencl_c_program_scope_global_variables feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/test/SemaOpenCL/storageclass.cl:22
+extern generic float g_generic_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-17 {{program scope variable must reside in constant address 
space}}

svenvh wrote:
> Not sure if this is the best way to restructure this test?  Now the 
> diagnostics are quite far away from the declarations, and it is harder to see 
> which diagnostic belongs to which declaration.
> 
> I wonder if the following would be a better structure instead:
> ```
> int G3 = 0;
> #ifndef __opencl_c_program_scope_global_variables
> // expected-error@-2 {{program scope variable must reside in constant address 
> space}}
> #endif
> 
> global int G4 = 0;
> #ifndef __opencl_c_program_scope_global_variables
> // expected-error@-2 {{program scope variable must reside in constant address 
> space}}
> #endif
> 
> ... etc. ...
> ```
> 
> The downside is that it is a bit more verbose due to the repetition of the 
> `#if` guards, but it makes the test more readable/maintainable in my opinion.
Yeah, that's reasonable. I'll change that in the following patch. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103191

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

[PATCH] D103241: [OpenCL] Add memory_scope_all_devices

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:113
 OPENCL_OPTIONALCOREFEATURE(__opencl_c_atomic_order_seq_cst, false, 300, 
OCL_C_30)
+OPENCL_OPTIONALCOREFEATURE(__opencl_c_atomic_scope_all_devices, false, 300, 
OCL_C_30)
 OPENCL_OPTIONALCOREFEATURE(__opencl_c_subgroups, false, 300, OCL_C_30)

azabaznov wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > Anastasia wrote:
> > > > azabaznov wrote:
> > > > > This feature is header only. We had a lot of discussions on that and 
> > > > > the main idea was not to declare header only features/extensions in 
> > > > > `OpenCLExtensions.def` and use 
> > > > > `-D__opencl_c_atomic_scope_all_devices=1` instead, @Anastasia can 
> > > > > comment on this.
> > > > > 
> > > > > I personally would like to introduce new flag for OpenCL options in 
> > > > > `OpenCLExtensions.def` which will indicate that feature/extension is 
> > > > > header-only, and thus all of such options can be declared in 
> > > > > `OpenCLExtensions.def`: if flag is set to true it can be stripped out 
> > > > > from the parser. What do you think about this?
> > > > Yes, I agree the idea is to align with C/C++ directions for scalability 
> > > > i.e. we should only add what is absolutely necessary to the compiler 
> > > > and implement the rest using libraries - just like regular C and C++. 
> > > > We won't be able to scale if we keep adding everything in the compiler. 
> > > > In fact, we already have a huge scalability issue with our builtin 
> > > > functions. If we look at modern C++ - more than 70% of features are not 
> > > > in the compiler at all.
> > > > 
> > > > Would it be possible to do something like suggested here: 
> > > > https://reviews.llvm.org/D91531#change-AKXhB4ko4nAO for 
> > > > `cl_khr_depth_images`?
> > > > 
> > > > > I personally would like to introduce new flag for OpenCL options in 
> > > > > OpenCLExtensions.def which will indicate that feature/extension is 
> > > > > header-only, and thus all of such options can be declared in 
> > > > > OpenCLExtensions.def: if flag is set to true it can be stripped out 
> > > > > from the parser. What do you think about this?
> > > > 
> > > > Hmm, I think the macros should either be declared in the headers or 
> > > > using a flag `-D`. I don't know why would adding them in 
> > > > `OpenCLExtensions.def` be beneficial if we can use conventional 
> > > > approaches? This allows avoiding the complexity and makes things more 
> > > > modular. If we look at the OpenCL vendor extensions for example - we 
> > > > probably don't want them all in one place?
> > > > This feature is header only.
> > > 
> > > Good catch!  I have updated the patch to define the feature macro in the 
> > > header instead.  Currently that definition is not optional, since we 
> > > don't have finalized the solution for handling this yet (though the 
> > > __undef proposal seems to be compatible with this change).
> > > 
> > > > I personally would like to introduce new flag for OpenCL options in 
> > > > OpenCLExtensions.def which will indicate that feature/extension is 
> > > > header-only
> > > 
> > > If we still need to add header-only features to OpenCLExtensions.def, 
> > > then they aren't really header-only anymore I'd argue (as @Anastasia 
> > > pointed out above).  So I'm not sure we need it either, or perhaps I 
> > > missed something.
> > FYI we have already added extended subgroups extension macros for SPIR in 
> > `opencl-c-base.h` without the `__undef<...>` trick.
> > 
> > ```
> > #if defined(__SPIR__)
> > #define cl_khr_subgroup_extended_types 1
> > #define cl_khr_subgroup_non_uniform_vote 1
> > #define cl_khr_subgroup_ballot 1
> > #define cl_khr_subgroup_non_uniform_arithmetic 1
> > #define cl_khr_subgroup_shuffle 1
> > #define cl_khr_subgroup_shuffle_relative 1
> > #define cl_khr_subgroup_clustered_reduce 1
> > #endif // defined(__SPIR__)
> > ```
> > 
> > But extra conditions can be added any time if we get the agreement on the 
> > route forward. 
> > Hmm, I think the macros should either be declared in the headers or using a 
> > flag -D. I don't know why would adding them in OpenCLExtensions.def be 
> > beneficial if we can use conventional approaches? This allows avoiding the 
> > complexity and makes things more modular. If we look at the OpenCL vendor 
> > extensions for example - we probably don't want them all in one place?
> 
> Well, IMO separating extensions/features into two classes of options exactly 
> brings new complexities :) I'm not sure why do we need to have a separate 
> interface for them if there already exists unified one. For example, Intel 
> compute-runtime uses `-cl-ext` flag to forward options : 
> https://github.com/intel/compute-runtime/blob/master/opencl/source/platform/extensions.cpp#L156.
>  
> 
> Can we use this header  mechanism  to define header-only features/extensions 
> while `-cl-ext`interface is preserved?
I think if we can hav

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong marked 5 inline comments as done.
martong added a comment.

In D103314#2789754 , @vsavchenko 
wrote:

> I had another thought, `merge` is usually called in situations when we found 
> out that two symbols should be marked equal (and checked that it's possible 
> to begin with), which is not true in your case.
>
> If we update my case from before, we can get: `a + b == c` and `a != c` as 
> given, and `b == 0` as a new constraint. In this situation, you will merge 
> classes `{a + b, c}` and `{a}`, which contradicts our existing disequality 
> information.

Yes, we must check the disequivalence classes to discover such a contradiction, 
I updated the code to do so.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1566-1569
+ConstraintRangeTy Constraints = State->get();
+for (std::pair ClassConstraint : Constraints) {
+  EquivalenceClass Class = ClassConstraint.first;
+  SymbolSet ClassMembers = Class.getClassMembers(State);

vsavchenko wrote:
> You don't actually use constraints here, so (let me write it in python) 
> instead of:
> ```
> [update(classMap[class]) for class, constraint in constraints.items()]
> ```
> you can use
> ```
> [update(members) for class, members in classMap.items()]
> ```
Actually, trivial equivalence classes (those that have only one symbol member) 
are not stored in the State. Thus, we must skim through the constraints as well 
in order to be able to simplify symbols in the constraints.
In short, we have to iterate both collections.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1576-1577
+  // Add the newly simplified symbol to the equivalence class.
+  State =
+  Class.merge(this->getBasicVals(), F, State,
+  EquivalenceClass::find(State, SimplifiedMemberSym));

vsavchenko wrote:
> Uh-oh, almost let yet another null-state bug to happen!  During this 
> iteration, `State` can become null, so we need to check for it.
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 348945.
martong marked 2 inline comments as done.
martong added a comment.

- Simplify equivalence classes when iterate over ClassMap, simplify constraints 
by iterating over the ConstraintsMap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/find-binop-constraints.cpp

Index: clang/test/Analysis/find-binop-constraints.cpp
===
--- /dev/null
+++ clang/test/Analysis/find-binop-constraints.cpp
@@ -0,0 +1,145 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_legacy_behavior(int x, int y) {
+  if (y != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return y / (x + y);  // expected-warning{{Division by zero}}
+}
+
+int test_rhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (x != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_and_rhs_further_constrained(int x, int y) {
+  if (x % y != 1)
+return 0;
+  if (x != 1)
+return 0;
+  if (y != 2)
+return 0;
+  clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_commutativity(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(y + x == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_binop_when_height_is_2_r(int a, int x, int y, int z) {
+  switch (a) {
+  case 1: {
+if (x + y + z != 0)
+  return 0;
+if (z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 2: {
+if (x + y + z != 0)
+  return 0;
+if (y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 3: {
+if (x + y + z != 0)
+  return 0;
+if (x != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 4: {
+if (x + y + z != 0)
+  return 0;
+if (x + y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 5: {
+if (z != 0)
+  return 0;
+if (x + y + z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+if (y != 0)
+  return 0;
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+break;
+  }
+
+  }
+  return 0;
+}
+
+void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a != d)
+return;
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
+  return;
+}
+
+void test_contradiction(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a == c)
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // Bring in the contradiction.
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_warnIfReached(); // no-warning, i.e. UNREACHABLE
+  return;
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:405
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");

Anastasia wrote:
> svenvh wrote:
> > This means we now have two separate places that set 
> > `OpenCLGenericAddressSpace`, the other place being in 
> > `CompilerInvocation::setLangDefaults()`.  That feels like a maintenance 
> > hazard.  Do you think it makes sense to set this field in one single place 
> > instead?
> I think we should try to set it in `CompilerInvocation.cpp` directly, we 
> should be able to query `TargetOptions` there. Although that place is 
> expected to be for the language-specific defaults but we broke the standard 
> flow by having the language mode controlled by the target settings anyway.
> 
> I can't remember though why we have decided to add dedicated `LangOpts` 
> entries for generic address space instead of just using `OpenCLOptions` from 
> `Sema`? I think it simplifies the handling of some builtin functions?
> This means we now have two separate places that set 
> OpenCLGenericAddressSpace, the other place being in 
> CompilerInvocation::setLangDefaults(). That feels like a maintenance hazard. 
> Do you think it makes sense to set this field in one single place instead?

>I think we should try to set it in CompilerInvocation.cpp directly, we should 
>be able to query TargetOptions there. 

I don't think that we are able to access target options at that stage without 
modifying current interfaces.  `CompilerInvocation::setLangDefaults()` is a 
static member function.

> I can't remember though why we have decided to add dedicated LangOpts entries 
> for generic address space instead of just using OpenCLOptions from Sema? I 
> think it simplifies the handling of some builtin functions?

That's correct. Also, the idea was to reuse generic keyword in other languages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

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

[PATCH] D103386: [PowerPC] Fix x86 vector intrinsics wrapper compilation under C++

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added subscribers: phosek, bjope.
bjope added inline comments.



Comment at: clang/test/CodeGen/ppc-xmmintrin.c:10
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns

Unfortunately I get some failures with this. Maybe because of an unstandard 
build setup.

We've started to use `-DCLANG_DEFAULT_RTLIB=compiler-rt 
-DCLANG_DEFAULT_CXX_STDLIB=libc++` when building clang. And we also use 
`-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON`. Not sure if that is a setup that is 
asking for trouble. Anyway, when running this test case we end up with

```
: 'RUN: at line 10';   /workspace/llvm/build/bin/clang -x c++ -fsyntax-only 
-target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding 
-DNO_WARN_X86_INTRINSICS /workspace/clang/test/CodeGen/ppc-xmmintrin.c
-fno-discard-value-names -mllvm -disable-llvm-optzns
--
Exit Code: 1

Command Output (stderr):
--
In file included from /workspace/clang/test/CodeGen/ppc-xmmintrin.c:13:
In file included from 
/workspace/llvm/build/lib/clang/13.0.0/include/ppc_wrappers/xmmintrin.h:42:
In file included from 
/workspace/llvm/build/lib/clang/13.0.0/include/altivec.h:44:
In file included from /workspace/llvm/build/bin/../include/c++/v1/stddef.h:39:
/workspace/llvm/build/bin/../include/c++/v1/__config:13:10: fatal error: 
'__config_site' file not found
#include <__config_site>
 ^~~
1 error generated.
```

Not sure really how to solve that.

Maybe we should stop building like that?

Or there is a bug somewhere (such as that we only get the __config_site headers 
in target specific dirs for targets that we actually build runtimes for, maybe 
something that was missing in https://reviews.llvm.org/D97572)?
(Maybe @phosek  could comment on that?)

Or this test case is missing some options to make it a bit more independent on 
the runtimes build setup?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103386

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

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a subscriber: gkistanova.
hubert.reinterpretcast added a comment.

In D102723#2790047 , @bjope wrote:

> But it looks like the workers here 
> (https://lab.llvm.org/staging/#/workers/109) are paused so hard to tell if it 
> helped.

@gkistanova, none of the staging builders appear to be active: 
https://lab.llvm.org/staging/#/builders


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102723

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

[clang] 94b0aec - [OpenCL] Fix ICE with invalid use of half

[Ole Strohm via cfe-commits]

Author: Ole Strohm
Date: 2021-06-01T13:43:07+01:00
New Revision: 94b0aec0f5c6b4f6a27cf3a542f795bbba72e851

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

LOG: [OpenCL] Fix ICE with invalid use of half

Because half is limited to the `cl_khr_fp16` extension being enabled,
`DefaultLvalueConversion` can fail when it's not enabled.
The original assumption that it will never fail is therefore wrong now.

Fixes: PR47976

Reviewed By: Anastasia

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

Added: 
clang/test/SemaOpenCLCXX/half.clcpp

Modified: 
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index b31c484b11700..75ebeaea1072c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4193,7 +4193,9 @@ Sema::PerformImplicitConversion(Expr *From, QualType 
ToType,
   case ICK_Lvalue_To_Rvalue: {
 assert(From->getObjectKind() != OK_ObjCProperty);
 ExprResult FromRes = DefaultLvalueConversion(From);
-assert(!FromRes.isInvalid() && "Can't perform deduced conversion?!");
+if (FromRes.isInvalid())
+  return ExprError();
+
 From = FromRes.get();
 FromType = From->getType();
 break;

diff  --git a/clang/test/SemaOpenCLCXX/half.clcpp 
b/clang/test/SemaOpenCLCXX/half.clcpp
new file mode 100644
index 0..bcb422fe58bf3
--- /dev/null
+++ b/clang/test/SemaOpenCLCXX/half.clcpp
@@ -0,0 +1,15 @@
+//RUN: %clang_cc1 %s -triple spir -verify -fsyntax-only
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+typedef half half2 __attribute__((ext_vector_type(2)));
+
+half f(half2 h2) { // expected-error{{declaring function return value of type 
'half' is not allowed ; did you forget * ?}}
+return h2.s0; // expected-error{{loading directly from pointer to type 
'__private half' requires cl_khr_fp16. Use vector data load builtin functions 
instead}}
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+half f(half2 h2) {
+return h2.s0;
+}



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

[PATCH] D103175: [C++4OpenCL] Fix ICE with invalid use of half

[Ole Strohm 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 rG94b0aec0f5c6: [OpenCL] Fix ICE with invalid use of half 
(authored by olestrohm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103175

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaOpenCLCXX/half.clcpp


Index: clang/test/SemaOpenCLCXX/half.clcpp
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/half.clcpp
@@ -0,0 +1,15 @@
+//RUN: %clang_cc1 %s -triple spir -verify -fsyntax-only
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+typedef half half2 __attribute__((ext_vector_type(2)));
+
+half f(half2 h2) { // expected-error{{declaring function return value of type 
'half' is not allowed ; did you forget * ?}}
+return h2.s0; // expected-error{{loading directly from pointer to type 
'__private half' requires cl_khr_fp16. Use vector data load builtin functions 
instead}}
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+half f(half2 h2) {
+return h2.s0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4193,7 +4193,9 @@
   case ICK_Lvalue_To_Rvalue: {
 assert(From->getObjectKind() != OK_ObjCProperty);
 ExprResult FromRes = DefaultLvalueConversion(From);
-assert(!FromRes.isInvalid() && "Can't perform deduced conversion?!");
+if (FromRes.isInvalid())
+  return ExprError();
+
 From = FromRes.get();
 FromType = From->getType();
 break;


Index: clang/test/SemaOpenCLCXX/half.clcpp
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/half.clcpp
@@ -0,0 +1,15 @@
+//RUN: %clang_cc1 %s -triple spir -verify -fsyntax-only
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+
+typedef half half2 __attribute__((ext_vector_type(2)));
+
+half f(half2 h2) { // expected-error{{declaring function return value of type 'half' is not allowed ; did you forget * ?}}
+return h2.s0; // expected-error{{loading directly from pointer to type '__private half' requires cl_khr_fp16. Use vector data load builtin functions instead}}
+}
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+half f(half2 h2) {
+return h2.s0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4193,7 +4193,9 @@
   case ICK_Lvalue_To_Rvalue: {
 assert(From->getObjectKind() != OK_ObjCProperty);
 ExprResult FromRes = DefaultLvalueConversion(From);
-assert(!FromRes.isInvalid() && "Can't perform deduced conversion?!");
+if (FromRes.isInvalid())
+  return ExprError();
+
 From = FromRes.get();
 FromType = From->getType();
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added a comment.

I was wondering if there is a direct way to check the equivalence classes?
I am thinking about to add a `clang_annalyzer_dump_equivalence_classes` 
function to the ExprInspection checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[clang] 97d2349 - [clang][Parse] Add parsing support for C++ attributes on using-declarations

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2021-06-01T08:47:50-04:00
New Revision: 97d234935f1514af128277943f30efc469525371

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

LOG: [clang][Parse] Add parsing support for C++ attributes on using-declarations

This is a re-application of dc67299 which was reverted in f63adf5b because
it broke the build. The issue should now be fixed.

Attribution note: The original author of this patch is Erik Pilkington.
I'm only trying to land it after rebasing.

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

Added: 
clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp

Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/Features.def
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/Parser/cxx0x-attributes.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 129e73345b2a2..c60b8b39e1c9e 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -632,6 +632,20 @@ Attributes on the ``enum`` declaration do not apply to 
individual enumerators.
 
 Query for this feature with ``__has_extension(enumerator_attributes)``.
 
+C++11 Attributes on using-declarations
+==
+
+Clang allows C++-style ``[[]]`` attributes to be written on using-declarations.
+For instance:
+
+.. code-block:: c++
+
+  [[clang::using_if_exists]] using foo::bar;
+  using foo::baz [[clang::using_if_exists]];
+
+You can test for support for this extension with
+``__has_extension(cxx_attributes_on_using_declarations)``.
+
 'User-Specified' System Frameworks
 ==
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cbf0f9b0e2c49..11bcac075c241 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -113,6 +113,9 @@ Attribute Changes in Clang
 
 - ...
 
+- Added support for C++11-style ``[[]]`` attributes on using-declarations, as a
+  clang extension.
+
 Windows Support
 ---
 

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index b13c382c16ebc..fe6e4206a9223 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -693,6 +693,9 @@ def ext_using_attribute_ns : ExtWarn<
 def err_using_attribute_ns_conflict : Error<
   "attribute with scope specifier cannot follow default scope specifier">;
 def err_attributes_not_allowed : Error<"an attribute list cannot appear here">;
+def ext_cxx11_attr_placement : ExtWarn<
+  "ISO C++ does not allow an attribute list to appear here">,
+  InGroup>;
 def err_attributes_misplaced : Error<"misplaced attributes; expected 
attributes here">;
 def err_l_square_l_square_not_attribute : Error<
   "C++11 only allows consecutive left square brackets when "

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4b653b8b47370..80130c2584fe5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3957,6 +3957,9 @@ def warn_attribute_sentinel_named_arguments : Warning<
 def warn_attribute_sentinel_not_variadic : Warning<
   "'sentinel' attribute only supported for variadic 
%select{functions|blocks}0">,
   InGroup;
+def warn_deprecated_ignored_on_using : Warning<
+  "%0 currently has no effect on a using declaration">,
+  InGroup;
 def err_attribute_sentinel_less_than_zero : Error<
   "'sentinel' parameter 1 less than zero">;
 def err_attribute_sentinel_not_zero_or_one : Error<

diff  --git a/clang/include/clang/Basic/Features.def 
b/clang/include/clang/Basic/Features.def
index a7a5e06a937e0..592e3e33baf1a 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -259,6 +259,7 @@ EXTENSION(gnu_asm, LangOpts.GNUAsm)
 EXTENSION(gnu_asm_goto_with_outputs, LangOpts.GNUAsm)
 EXTENSION(matrix_types, LangOpts.MatrixTypes)
 EXTENSION(matrix_types_scalar_division, true)
+EXTENSION(cxx_attributes_on_using_declarations, LangOpts.CPlusPlus11)
 
 FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus && 
LangOpts.RelativeCXXABIVTables)
 

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index c32ae6d2cf406..605292f08df47 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2635,6 +2635,10 

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG97d234935f15: [clang][Parse] Add parsing support for C++ 
attributes on using-declarations (authored by ldionne).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91630

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp

Index: clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx11-attributes-on-using-declaration.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -pedantic -triple x86_64-apple-macos11 -std=c++20 -fsyntax-only -verify %s
+
+static_assert(__has_extension(cxx_attributes_on_using_declarations), "");
+
+namespace NS { typedef int x; }
+
+[[clang::annotate("foo")]] using NS::x; // expected-warning{{ISO C++ does not allow an attribute list to appear here}}
+
+
+[[deprecated]] using NS::x;// expected-warning {{'deprecated' currently has no effect on a using declaration}} expected-warning{{ISO C++ does not allow}}
+using NS::x [[deprecated]];// expected-warning {{'deprecated' currently has no effect on a using declaration}} expected-warning{{ISO C++ does not allow}}
+using NS::x __attribute__((deprecated));   // expected-warning {{'deprecated' currently has no effect on a using declaration}}
+using NS::x __attribute__((availability(macos,introduced=1))); // expected-warning {{'availability' currently has no effect on a using declaration}}
+
+[[clang::availability(macos,introduced=1)]] using NS::x; // expected-warning {{'availability' currently has no effect on a using declaration}} expected-warning{{ISO C++ does not allow}}
+
+// expected-warning@+1 3 {{ISO C++ does not allow an attribute list to appear here}}
+[[clang::annotate("A")]] using NS::x [[clang::annotate("Y")]], NS::x [[clang::annotate("Z")]];
+
+template 
+struct S : T {
+  [[deprecated]] using typename T::x; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on a using declaration}}
+  [[deprecated]] using T::y;  // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on a using declaration}}
+
+  using typename T::z [[deprecated]]; // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on a using declaration}}
+  using T::a [[deprecated]];  // expected-warning{{ISO C++ does not allow}} expected-warning {{'deprecated' currently has no effect on a using declaration}}
+};
+
+struct Base {};
+
+template 
+struct DepBase1 : B {
+  using B::B [[]];
+
+};
+template 
+struct DepBase2 : B {
+  using B::B __attribute__(());
+};
+
+DepBase1 db1;
+DepBase2 db2;
Index: clang/test/Parser/cxx0x-attributes.cpp
===
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -131,12 +131,12 @@
 [[]] static_assert(true, ""); //expected-error {{an attribute list cannot appear here}}
 [[]] asm(""); // expected-error {{an attribute list cannot appear here}}
 
-[[]] using ns::i; // expected-error {{an attribute list cannot appear here}}
+[[]] using ns::i;
 [[unknown]] using namespace ns; // expected-warning {{unknown attribute 'unknown' ignored}}
 [[noreturn]] using namespace ns; // expected-error {{'noreturn' attribute only applies to functions}}
 namespace [[]] ns2 {} // expected-warning {{attributes on a namespace declaration are a C++17 extension}}
 
-using [[]] alignas(4) [[]] ns::i; // expected-error {{an attribute list cannot appear here}}
+using[[]] alignas(4)[[]] ns::i;  // expected-error {{an attribute list cannot appear here}} expected-error {{'alignas' attribute only applies to variables, data members and tag types}} expected-warning {{ISO C++}}
 using [[]] alignas(4) [[]] foobar = int; // expected-error {{an attribute list cannot appear here}} expected-error {{'alignas' attribute only applies to}}
 
 void bad_attributes_in_do_while() {
@@ -157,7 +157,16 @@
 [[]] using T = int; // expected-error {{an attribute list cannot appear here}}
 using T [[]] = int; // ok
 template using U [[]] = T;
-using ns::i [[]]; // expected-error {{an attribute list cannot appear here}}
+using ns::i [[]];
+using ns::i [[]], ns::i [[]]; // expected-warning {{use of multiple declarators in a single using declar

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 348952.
ldionne added a comment.

Rebase onto main to trigger CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90188

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Parser/using-if-exists-attr.cpp
  clang/test/SemaCXX/attr-deprecated.cpp
  clang/test/SemaCXX/using-if-exists.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6466,6 +6466,7 @@
   case Decl::Concept:
   case Decl::LifetimeExtendedTemporary:
   case Decl::RequiresExprBody:
+  case Decl::UnresolvedUsingIfExists:
 return C;
 
   // Declaration kinds that don't make any sense here, but are
Index: clang/test/SemaCXX/using-if-exists.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/using-if-exists.cpp
@@ -0,0 +1,218 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %s -verify
+
+#define UIE __attribute__((using_if_exists))
+
+namespace test_basic {
+namespace NS {}
+
+using NS::x UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+x usex(); // expected-error{{reference to unresolved using declaration}}
+
+using NotNS::x UIE; // expected-error{{use of undeclared identifier 'NotNS'}}
+
+using NS::NotNS::x UIE; // expected-error{{no member named 'NotNS' in namespace 'test_basic::NS'}}
+} // test_basic
+
+namespace test_redecl {
+namespace NS {}
+
+using NS::x UIE;
+using NS::x UIE;
+
+namespace NS1 {}
+namespace NS2 {}
+namespace NS3 {
+int A(); // expected-note{{target of using declaration}}
+struct B {}; // expected-note{{target of using declaration}}
+int C(); // expected-note{{conflicting declaration}}
+struct D {}; // expected-note{{conflicting declaration}}
+}
+
+using NS1::A UIE;
+using NS2::A UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}} expected-note{{conflicting declaration}}
+using NS3::A UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+int i = A(); // expected-error{{reference to unresolved using declaration}}
+
+using NS1::B UIE;
+using NS2::B UIE; // expected-note{{conflicting declaration}} expected-note{{using declaration annotated with 'using_if_exists' here}}
+using NS3::B UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+B myB; // expected-error{{reference to unresolved using declaration}}
+
+using NS3::C UIE;
+using NS2::C UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+int j = C();
+
+using NS3::D UIE;
+using NS2::D UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+D myD;
+} // test_redecl
+
+namespace test_dependent {
+template 
+struct S : B {
+  using B::mf UIE; // expected-note 3 {{using declaration annotated with 'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+};
+
+struct BaseEmpty {
+};
+struct BaseNonEmpty {
+  void mf();
+  typedef int mt;
+};
+
+template 
+struct UseCtor : Base {
+  using Base::Base UIE; // expected-error{{'using_if_exists' attribute cannot be applied to an inheriting constructor}}
+};
+struct BaseCtor {};
+
+void f() {
+  S empty;
+  S nonempty;
+  empty.mf(); // expected-error {{reference to unresolved using declaration}}
+  nonempty.mf();
+  (&empty)->mf(); // expected-error {{reference to unresolved using declaration}}
+  (&nonempty)->mf();
+
+  S::mt y; // expected-error {{reference to unresolved using declaration}}
+  S::mt z;
+
+  S::mf(); // expected-error {{reference to unresolved using declaration}}
+
+  UseCtor usector;
+}
+
+template 
+struct Implicit : B {
+  using B::mf UIE; // expected-note {{using declaration annotated with 'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note 2 {{using declaration annotated with 'using_if_exists' here}}
+
+  void use() {
+mf(); // expe

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Awesome!
I know, I said that we are ready to land, but I think I was too excited about 
this change. We probably should have some data on how it performs on real-life 
codebases.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1559
+  // absolute minimum.
+  LLVM_NODISCARD ProgramStateRef simplifyEquivalenceClass(
+  ProgramStateRef State, EquivalenceClass Class, SymbolSet ClassMembers) {

Maybe it should be a `simplify` method of the class itself?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1568-1573
+EquivalenceClass ClassOfSimplifiedSym =
+EquivalenceClass::find(State, SimplifiedMemberSym);
+// We are about to add the newly simplified symbol to the existing
+// equivalence class, but they are known to be non-equal. This is a
+// contradiction.
+if (DisequalClasses.contains(ClassOfSimplifiedSym))

I think we can add a method `isDisequalTo` or just use `areEqual` in a this way:
are equal?
  [Yes] -> nothing to do here
  [No]  -> return nullptr
  [Don't know] -> merge



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1566-1569
+ConstraintRangeTy Constraints = State->get();
+for (std::pair ClassConstraint : Constraints) {
+  EquivalenceClass Class = ClassConstraint.first;
+  SymbolSet ClassMembers = Class.getClassMembers(State);

martong wrote:
> vsavchenko wrote:
> > You don't actually use constraints here, so (let me write it in python) 
> > instead of:
> > ```
> > [update(classMap[class]) for class, constraint in constraints.items()]
> > ```
> > you can use
> > ```
> > [update(members) for class, members in classMap.items()]
> > ```
> Actually, trivial equivalence classes (those that have only one symbol 
> member) are not stored in the State. Thus, we must skim through the 
> constraints as well in order to be able to simplify symbols in the 
> constraints.
> In short, we have to iterate both collections.
> 
Ah, I see.  Then I would say that your previous solution is more readable (if 
we keep `simplify`, of course).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[PATCH] D102849: [flang][driver] Add support for the "-init-only" option

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Hi @stuartellis ,

Thank you for preparing this! This looks good to me, but I would appreciate a 
comment somewhere (e.g. in the commit message) comparing `-init-only` and 
`-test-io`. Essentially, both options are for testing purposes only. IMHO, we 
should only require one of them (I would be in favor of `-init-only`), but it's 
a bit tricky with input files being managed by the prescanner rather than the 
driver. So for the time being I would keep both, but just make it clear that:

- `-init-only` ignores the input (because it never calls the prescanner)
- `-test-io` is similar to `-init-only`, but does read and print the input 
without calling the prescanner.

Please let me know if any of this is unclear!




Comment at: clang/include/clang/Driver/Options.td:5692
   HelpText<"Emit native object files">;
 
+def init_only : Flag<["-"], "init-only">,

[nit] Remove empty line



Comment at: flang/include/flang/Frontend/FrontendOptions.h:77
+  /// Only execute frontend initialization
+  InitializationOnly
 

[Nit] Whenever possible, we've been adopting names from Clang (hopefully, this 
way it will  be easier for developers to switch between the drivers). So, in 
this case, it would be `InitOnly` rather than `InitializationOnly`: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L2385.



Comment at: flang/test/Driver/init-only.f90:2
+! Verify that -init-only flag generates a diagnostic as expected
+
+! RUN: %flang_fc1 -init-only 2>&1 | FileCheck %s

Missing `! REQUIRES: new-flang-driver`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102849

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

[PATCH] D103286: [clang-format] Add PPIndentWidth option

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap updated this revision to Diff 348955.
gergap added a comment.

trigger build again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103286

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3431,6 +3431,48 @@
   verifyFormat("a\f\\");
 }
 
+TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) {
+  FormatStyle style = getChromiumStyle(FormatStyle::LK_Cpp);
+  style.IndentWidth = 4;
+  style.PPIndentWidth = 1;
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef __linux__\n"
+   "void foo() {\n"
+   "int x = 0;\n"
+   "}\n"
+   "#define FOO\n"
+   "#endif\n"
+   "void bar() {\n"
+   "int y = 0;\n"
+   "}\n",
+   style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+  verifyFormat("#ifdef __linux__\n"
+   "void foo() {\n"
+   "int x = 0;\n"
+   "}\n"
+   "# define FOO foo\n"
+   "#endif\n"
+   "void bar() {\n"
+   "int y = 0;\n"
+   "}\n",
+   style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  verifyFormat("#ifdef __linux__\n"
+   "void foo() {\n"
+   "int x = 0;\n"
+   "}\n"
+   " #define FOO foo\n"
+   "#endif\n"
+   "void bar() {\n"
+   "int y = 0;\n"
+   "}\n",
+   style);
+}
+
 TEST_F(FormatTest, IndentsPPDirectiveInReducedSpace) {
   verifyFormat("#define A(BB)", getLLVMStyleWithColumns(13));
   verifyFormat("#define A( \\\nBB)", getLLVMStyleWithColumns(12));
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -57,7 +57,9 @@
 while (IndentForLevel.size() <= Line.Level)
   IndentForLevel.push_back(-1);
 if (Line.InPPDirective) {
-  Indent = Line.Level * Style.IndentWidth + AdditionalIndent;
+  unsigned IndentWidth =
+  (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
+  Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(IndentForLevel, Line.Level);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -664,6 +664,7 @@
 IO.mapOptional("PenaltyIndentedWhitespace",
Style.PenaltyIndentedWhitespace);
 IO.mapOptional("PointerAlignment", Style.PointerAlignment);
+IO.mapOptional("PPIndentWidth", Style.PPIndentWidth);
 IO.mapOptional("RawStringFormats", Style.RawStringFormats);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
@@ -1021,6 +1022,7 @@
   LLVMStyle.IndentRequires = false;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
+  LLVMStyle.PPIndentWidth = -1;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2659,6 +2659,20 @@
   /// Pointer and reference alignment style.
   PointerAlignmentStyle PointerAlignment;
 
+  /// The number of columns to use for indentation of preprocessor statements.
+  /// When set to -1 (default) ``IndentWidth`` is used also for preprocessor
+  /// statements.
+  /// \code
+  ///PPIndentWidth: 1
+  ///
+  ///#ifdef __linux__
+  ///# define FOO
+  ///#else
+  ///# define BAR
+  ///#endif
+  /// \endcode
+  signed PPIndentWidth;
+
   /// See documentation of ``RawStringFormats``.
   struct RawStringFormat {
 /// The language of this raw string.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -228,6 +228,9 @@
 - Option ``IndentAccessModifiers`` has been added to be able to give access
   modifiers their own indentation level inside records.
 
+- Option ``PPIndentWidth`` has been added to be ab

[PATCH] D103457: [analyzer] Add forwarding `addVisitor` method

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, 
manas, RedDocMD.
Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The majority of all `addVisitor` callers follow the same pattern:

  addVisitor(std::make_unique(arg1, arg2, ...));

This patches introduces additional overload for `addVisitor` to simplify
that pattern:

  addVisitor(arg1, arg2, ...);


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103457

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -852,10 +852,10 @@
 bool EnableNullFPSuppression, PathSensitiveBugReport &BR,
 const SVal V) {
 AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
-if (EnableNullFPSuppression &&
-Options.ShouldSuppressNullReturnPaths && V.getAs())
-  BR.addVisitor(std::make_unique(
-  R->getAs(), V));
+if (EnableNullFPSuppression && Options.ShouldSuppressNullReturnPaths &&
+V.getAs())
+  BR.addVisitor(R->getAs(),
+   V);
   }
 
   void* getTag() const {
@@ -1011,14 +1011,12 @@
 AnalyzerOptions &Options = State->getAnalysisManager().options;
 
 bool EnableNullFPSuppression = false;
-if (InEnableNullFPSuppression &&
-Options.ShouldSuppressNullReturnPaths)
+if (InEnableNullFPSuppression && Options.ShouldSuppressNullReturnPaths)
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-BR.addVisitor(std::make_unique(CalleeContext,
-   EnableNullFPSuppression,
-   Options, TKind));
+BR.addVisitor(CalleeContext, EnableNullFPSuppression,
+ Options, TKind);
   }
 
   PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N,
@@ -1589,8 +1587,8 @@
   dyn_cast_or_null(V.getAsRegion())) {
   if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
 if (auto KV = State->getSVal(OriginalR).getAs())
-  BR.addVisitor(std::make_unique(
-  *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC));
+  BR.addVisitor(
+  *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC);
   }
 }
   }
@@ -2070,8 +2068,7 @@
   // TODO: Shouldn't we track control dependencies of every bug location, rather
   // than only tracked expressions?
   if (LVState->getAnalysisManager().getAnalyzerOptions().ShouldTrackConditions)
-report.addVisitor(std::make_unique(
-  InputNode));
+report.addVisitor(InputNode);
 
   // The message send could be nil due to the receiver being nil.
   // At this point in the path, the receiver should be live since we are at the
@@ -2098,8 +2095,8 @@
 // got initialized.
 if (RR && !LVIsNull)
   if (auto KV = LVal.getAs())
-report.addVisitor(std::make_unique(
-*KV, RR, EnableNullFPSuppression, TKind, SFC));
+report.addVisitor(
+*KV, RR, EnableNullFPSuppression, TKind, SFC);
 
 // In case of C++ references, we want to differentiate between a null
 // reference and reference to null pointer.
@@ -2112,20 +2109,19 @@
 
   // Mark both the variable region and its contents as interesting.
   SVal V = LVState->getRawSVal(loc::MemRegionVal(R));
-  report.addVisitor(
-  std::make_unique(cast(R), TKind));
+  report.addVisitor(cast(R), TKind);
 
   MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary(
   LVNode, R, EnableNullFPSuppression, report, V);
 
   report.markInteresting(V, TKind);
-  report.addVisitor(std::make_unique(R));
+  report.addVisitor(R);
 
   // If the contents are symbolic and null, find out when they became null.
   if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true))
 if (LVState->isNull(V).isConstrainedTrue())
-  report.addVisitor(std::make_unique(
-  V.castAs(), false));
+  report.addVisitor(V.castAs(),
+  false);
 
   // Add visitor

[PATCH] D103165: Threading: use independent llvm::thread implementation on Apple platforms to increase stack size

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 348958.
t.p.northover added a comment.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, hiraditya.
Herald added a project: clang-tools-extra.

- Add `llvm::thread` for all potential platforms, allowing stack size to be 
specified.
- `llvm::thread` based on the `Threading.inc` where possible, else 
`std::thread`, else in-thread synchronous execution (with progressively more 
failures).
- Remove `llvm_execute_on_thread*` in favour of this new `llvm::thread`.

I've managed to test it on all 3 major platforms, and the fallbacks by fiddling 
`#if`s. So it should at least compile everywhere.

> Also this way llvm::thread users that don't need full-sized stacks can 
> continue getting the Darwin default of smaller pages.

I'm not sure I've implemented this, since I've just made 8MB the Darwin default 
here. The case I care about is LTOBackend via `ThreadPool`. Would you prefer me 
to push an optional `StackSize` argument into `ThreadPool` too and make 
LTOBackend specify it?

I kind of think this has come up often enough (it's at least the second time 
I've had to fix it somewhere), and 8MB is small enough for anything running 
Clang that it's better to make sure it doesn't happen again.


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

https://reviews.llvm.org/D103165

Files:
  clang-tools-extra/clangd/support/Threading.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/Threading.h
  llvm/include/llvm/Support/thread.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Threading.cpp
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -63,7 +63,8 @@
 ThreadFinished.notify();
   };
 
-  llvm::llvm_execute_on_thread_async(ThreadFunc);
+  llvm::thread Thread(ThreadFunc);
+  Thread.detach();
   ASSERT_TRUE(ThreadStarted.wait());
   ThreadAdvanced.notify();
   ASSERT_TRUE(ThreadFinished.wait());
@@ -71,11 +72,23 @@
 
 TEST(Threading, RunOnThreadSync) {
   std::atomic_bool Executed(false);
-  llvm::llvm_execute_on_thread(
+  llvm::thread Thread(
   [](void *Arg) { *static_cast(Arg) = true; },
   &Executed);
+  Thread.join();
   ASSERT_EQ(Executed, true);
 }
+
+#if defined(__APPLE__)
+TEST(Threading, AppleStackSize) {
+  llvm::thread Thread([] {
+volatile unsigned char Var[8 * 1024 * 1024 - 1024];
+Var[0] = 0xff;
+ASSERT_EQ(Var[0], 0xff);
+  });
+  Thread.join();
+}
+#endif
 #endif
 
 } // end anon namespace
Index: llvm/lib/Support/Windows/Threading.inc
===
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -23,22 +23,10 @@
 #undef MemoryFence
 #endif
 
-static unsigned __stdcall threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast(Arg);
-  TI->UserFn(TI->UserData);
-  return 0;
-}
-
-static unsigned __stdcall threadFuncAsync(void *Arg) {
-  std::unique_ptr Info(static_cast(Arg));
-  (*Info)();
-  return 0;
-}
-
-static void
-llvm_execute_on_thread_impl(unsigned (__stdcall *ThreadFunc)(void *), void *Arg,
-llvm::Optional StackSizeInBytes,
-JoiningPolicy JP) {
+HANDLE
+llvm::llvm_execute_on_thread_impl(unsigned(__stdcall *ThreadFunc)(void *),
+  void *Arg,
+  llvm::Optional StackSizeInBytes) {
   HANDLE hThread = (HANDLE)::_beginthreadex(
   NULL, StackSizeInBytes.getValueOr(0), ThreadFunc, Arg, 0, NULL);
 
@@ -46,11 +34,16 @@
 ReportLastErrorFatal("_beginthreadex failed");
   }
 
-  if (JP == JoiningPolicy::Join) {
-if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
-  ReportLastErrorFatal("WaitForSingleObject failed");
-}
+  return hThread;
+}
+
+void llvm::llvm_thread_join_impl(HANDLE hThread) {
+  if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
+ReportLastErrorFatal("WaitForSingleObject failed");
   }
+}
+
+void llvm::llvm_thread_detach_impl(HANDLE hThread) {
   if (::CloseHandle(hThread) == FALSE) {
 ReportLastErrorFatal("CloseHandle failed");
   }
Index: llvm/lib/Support/Unix/Threading.inc
===
--- llvm/lib/Support/Unix/Threading.inc
+++ llvm/lib/Support/Unix/Threading.inc
@@ -48,22 +48,9 @@
 #include   // For syscall()
 #endif
 
-static void *threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast(Arg);
-  TI->UserFn(TI->UserData);
-  return nullptr;
-}
-
-static void *threadFuncAsync(void *Arg) {
-  std::unique_ptr Info(static_cast(Arg));
-  (*Info)();
-  return nullptr;
-}
-
-static void
-llvm_execute_on_thread_impl

[PATCH] D103165: Threading: use independent llvm::thread implementation on Apple platforms to increase stack size

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: llvm/include/llvm/Support/thread.h:168
+/// stack size request.
+class thread {
+public:

An alternative here would have been to inherit from `std::thread` but that 
seemed a bit icky to me. Happy to switch if the consensus is otherwise.


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

https://reviews.llvm.org/D103165

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

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Frankly, I don't think that length-based criteria is reasonable one. 
Readability of code is much more important than line/file length.




Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:633
+  if (!Descriptor.ElemType.isNull()) {
+auto FullType = Descriptor.ElemType.getUnqualifiedType();
+if (Descriptor.ElemType->isFundamentalType() ||

Please don't use auto unless type is explicitly spelled in same statement or 
iterator.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:163
 
+AutoTypeNameLength option
+

See other documentation file for syntax for options.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:166
+
+If TypeName length is strictly above this threshold, `auto` will be used.
+

Please highlight TypeName (option) is single back-ticks and auto (language 
construct) with double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:168
+
+The default is 0, so `auto` will be used for all types except C++ fundamental 
types.
+

Please highlight 0 (option value) is single back-ticks and auto (language 
construct) with double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

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

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 348965.
DmitryPolukhin added a comment.

Rebase + try build on Windows again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Lex/HeaderSearch.h"
+#include "HeaderMapTestUtils.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -45,6 +46,21 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void addHeaderMap(llvm::StringRef Filename,
+std::unique_ptr Buf) {
+VFS->addFile(Filename, 0, std::move(Buf), /*User=*/None, /*Group=*/None,
+ llvm::sys::fs::file_type::regular_file);
+auto FE = FileMgr.getFile(Filename, true);
+assert(FE);
+
+// Test class supports only one HMap at a time.
+assert(!HMap);
+HMap = HeaderMap::Create(*FE, FileMgr);
+auto DL =
+DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
+Search.AddSearchPath(DL, /*isAngled=*/false);
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -55,6 +71,7 @@
   std::shared_ptr TargetOpts;
   IntrusiveRefCntPtr Target;
   HeaderSearch Search;
+  std::unique_ptr HMap;
 };
 
 TEST_F(HeaderSearchTest, NoSearchDir) {
@@ -136,5 +153,31 @@
 "y/z/t.h");
 }
 
+// Helper struct with null terminator character to make MemoryBuffer happy.
+template 
+struct NullTerminatedFile : public FileTy {
+  PaddingTy Padding = 0;
+};
+
+TEST_F(HeaderSearchTest, HeaderMapReverseLookup) {
+  typedef NullTerminatedFile, char> FileTy;
+  FileTy File;
+  File.init();
+
+  test::HMapFileMockMaker Maker(File);
+  auto a = Maker.addString("d.h");
+  auto b = Maker.addString("b/");
+  auto c = Maker.addString("c.h");
+  Maker.addBucket("d.h", a, b, c);
+
+  addHeaderMap("/x/y/z.hmap", File.getBuffer());
+  addSearchDir("/a");
+
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c.h",
+   /*WorkingDir=*/"",
+   /*MainFile=*/""),
+"d.h");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1834,7 +1834,7 @@
   };
 
   for (unsigned I = 0; I != SearchDirs.size(); ++I) {
-// FIXME: Support this search within frameworks and header maps.
+// FIXME: Support this search within frameworks.
 if (!SearchDirs[I].isNormalDir())
   continue;
 
@@ -1848,6 +1848,19 @@
   if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
 *IsSystem = false;
 
+  // Try resolving resulting filename via reverse search in header maps,
+  // key from header name is user prefered name for the include file.
+  StringRef Filename = File.drop_front(BestPrefixLength);
+  for (unsigned I = 0; I != SearchDirs.size(); ++I) {
+if (!SearchDirs[I].isHeaderMap())
+  continue;
 
-  return path::convert_to_slash(File.drop_front(BestPrefixLength));
+StringRef SpelledFilename =
+SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
+if (!SpelledFilename.empty()) {
+  Filename = SpelledFilename;
+  break;
+}
+  }
+  return path::convert_to_slash(Filename);
 }
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -240,3 +240,27 @@
 return StringRef(DestPath.begin(), DestPath.size());
   }
 }
+
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+  if (!ReverseMap.empty())
+return ReverseMap.lookup(DestPath);
+
+  const HMapHeader &Hdr = getHeader();
+  unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
+  for (unsigned i = 0; i != NumBuckets; ++i) {
+HMapBucket B = getBucket(i);
+if (B.Key == HMAP_EmptyBucketKey)
+  continue;
+
+Optional Key = getString(B.Key);
+Optional Prefix = getString(B.Prefix);
+Optional Suffix = getString(B.Suffix);
+if (Key && Prefix && Suffix) {
+  SmallVector Buf;
+  Buf.append(Prefix->begin(), Prefix->end());
+  Buf.append(Suffix->begin(), Suffix->end());
+  ReverseMap[StringRef(Buf.begin(), Buf.size())] = *Key;
+}
+  }
+  return ReverseMap.lookup(DestPath);
+}
Index: clang/include/clang/Lex/HeaderMap.h
===

[PATCH] D102923: [clang][lex] Remark for used header search paths

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D102923#2787702 , @dexonsmith 
wrote:

> Yup, seems like something that could be added as a follow up (although 
> probably using the same remark). Is there a good place to leave behind a 
> FIXME?

I'll put a FIXME at an appropriate place in the next revision.




Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:427
+def remark_pp_include_header_search_usage : Remark<
+  "user-provided search path used: '%0'">,
+  InGroup>;

dexonsmith wrote:
> I suggest, more simply, "search path used:" (drop the "user-provided"). If 
> you think it's useful for information purposes to know whether it was a user 
> or system search path, I'd suggest using a select (in that case, maybe you 
> want to add an optional "framework" descriptor, and/or "header map" when 
> applicable, etc.).
I chose this spelling to distinguish user-provided vs default include paths. 
The default ones are not important for header search pruning in dependency 
scanner, as they always get generated in 
`InitHeaderSearch::AddDefaultIncludePaths`.



Comment at: clang/include/clang/Lex/HeaderSearch.h:164-165
 
+  /// Mapping from SearchDir to HeaderSearchOptions::Entry indices.
+  llvm::DenseMap SearchDirToHSEntry;
+

dexonsmith wrote:
> I think this can just be a vector of `unsigned`, since the key is densely 
> packed and counting from 0. You can use `~0U` for a sentinel for the 
> non-entries. Maybe it's not too important though.
I'm not sure what's our approach on early micro-optimizations like this. I 
don't think it will have measurable impact on memory or runtime.



Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:581-583
+// Check whether this DirectoryLookup maps to a HeaderSearch::UserEntry.
+if (Infos[I].UserEntryIdx)
+  LookupsToUserEntries.insert({I, *Infos[I].UserEntryIdx});

dexonsmith wrote:
> I don't see why we'd want to filter out system includes.
> - Users sometimes manually configure "system" search paths, and this remark 
> is fairly special-purpose, so I'm not sure the distinction is interesting to 
> preserve.
> - This remark is already going to be "noisy" and hit a few times on 
> essentially every compilation. Adding the system paths that get hit won't 
> change that much.
> - The motivation for the patch is to test the logic for detecting which 
> search paths are used in isolation from the 
> canonicalize-explicit-module-build-commands logic in clang-scan-deps. We need 
> to know that the logic works for system search paths as well.
I'm not sure what you mean by system includes. 
`HeaderSearchOptions::UserEntries` should contain all search paths provided 
through the command-line including `-isystem` and `-isysroot`: 
https://github.com/llvm/llvm-project/blob/97d234935f1514af128277943f30efc469525371/clang/lib/Frontend/CompilerInvocation.cpp#L2985-L3056.

"System header prefixes" only control whether a header found through 
`DirectoryLookup` should be marked as system.



Comment at: clang/test/Preprocessor/header-search-user-entries.c:9-11
+// CHECK: remark: user-provided search path used: 
'{{.*}}/header-search-user-entries/a'
+// CHECK: remark: user-provided search path used: 
'{{.*}}/header-search-user-entries/a_next'
+// CHECK: remark: user-provided search path used: 
'{{.*}}/header-search-user-entries/d'

dexonsmith wrote:
> If `-verify` doesn't work (hopefully it does), I think you'll need to add 
> some `CHECK-NOT` / `CHECK-NEXT` / etc. or else you're not testing for the 
> absence of other diagnostics.
> 
> Please also test:
> - Framework search path (used vs. not used)
> - System search path (used vs. not used)
> - One search path described relative to `-isysroot` (used vs. not used)
> - Header map (matched and used vs. matched but not used vs. not matched -- 
> for the middle case, I'm not sure what result we actually want)
> - A path only used via `#if __has_include()` (when it's not followed by an 
> `#include` or `#import`)
> - A path only used via ObjC's `#import`
> 
> If one of them doesn't work and it's complex enough to separate into a follow 
> up, I think that'd be fine, but please find somewhere to leave a FIXME.
Good point, I'll work on improving the coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102923

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

[PATCH] D103461: [clang][deps] NFC: Do not adjust the original action

[Jan Svoboda via Phabricator via cfe-commits]

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

This patch stops adjusting the frontend action when `clang-scan-deps` is 
configured to use the full output format.

In a future patch, the dependency scanner needs to check whether the original 
compiler invocation builds a PCH. That's impossible when `-Eonly` et al. 
overrides `-emit-pch`.

The `-Eonly` flag is not needed - the dependency scanner explicitly sets up its 
own frontend action.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103461

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -439,6 +439,8 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  //  instead of parsing and adjusting the raw command-line.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -509,7 +511,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -530,8 +532,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -439,6 +439,8 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  //  instead of parsing and adjusting the raw command-line.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -509,7 +511,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -530,8 +532,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103252: [C++4OpenCL] Fix missing address space on implicit move assignment operator

[Ole Strohm via Phabricator via cfe-commits]

olestrohm updated this revision to Diff 348969.
olestrohm added a comment.

Cleaned up the test by renaming the struct and making the test compile.

The test has also been moved to `clang/test/AST` as suggested, since it really
just makes sure that the generated AST contains the correct implicit methods.


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

https://reviews.llvm.org/D103252

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/AST/ast-dump-implicit-members.clcpp


Index: clang/test/AST/ast-dump-implicit-members.clcpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-implicit-members.clcpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -ast-dump -ast-dump-filter S | FileCheck 
-strict-whitespace %s
+
+struct S {};
+
+void f() {
+S i;
+i = i;
+}
+
+// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr S 'void () 
__generic noexcept'
+// CHECK: CXXConstructorDecl {{.*}} implicit constexpr S 'void (const 
__generic S &) __generic'
+// CHECK: CXXConstructorDecl {{.*}} implicit constexpr S 'void (__generic S 
&&) __generic'
+// CHECK: CXXMethodDecl {{.*}} implicit used constexpr operator= '__generic S 
&(const __generic S &) __generic noexcept'
+// CHECK: CXXMethodDecl {{.*}} implicit constexpr operator= '__generic S 
&(__generic S &&) __generic'
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -14299,10 +14299,7 @@
 /* Diagnose */ false);
   }
 
-  // Build an exception specification pointing back at this member.
-  FunctionProtoType::ExtProtoInfo EPI =
-  getImplicitMethodEPI(*this, MoveAssignment);
-  MoveAssignment->setType(Context.getFunctionType(RetType, ArgType, EPI));
+  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, MoveAssignment,


Index: clang/test/AST/ast-dump-implicit-members.clcpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-implicit-members.clcpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -ast-dump -ast-dump-filter S | FileCheck -strict-whitespace %s
+
+struct S {};
+
+void f() {
+S i;
+i = i;
+}
+
+// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr S 'void () __generic noexcept'
+// CHECK: CXXConstructorDecl {{.*}} implicit constexpr S 'void (const __generic S &) __generic'
+// CHECK: CXXConstructorDecl {{.*}} implicit constexpr S 'void (__generic S &&) __generic'
+// CHECK: CXXMethodDecl {{.*}} implicit used constexpr operator= '__generic S &(const __generic S &) __generic noexcept'
+// CHECK: CXXMethodDecl {{.*}} implicit constexpr operator= '__generic S &(__generic S &&) __generic'
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -14299,10 +14299,7 @@
 /* Diagnose */ false);
   }
 
-  // Build an exception specification pointing back at this member.
-  FunctionProtoType::ExtProtoInfo EPI =
-  getImplicitMethodEPI(*this, MoveAssignment);
-  MoveAssignment->setType(Context.getFunctionType(RetType, ArgType, EPI));
+  setupImplicitSpecialMemberType(MoveAssignment, RetType, ArgType);
 
   // Add the parameter to the operator.
   ParmVarDecl *FromParam = ParmVarDecl::Create(Context, MoveAssignment,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:271
   Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
+  auto It = llvm::find(Args, "--");

sammccall wrote:
> Hmm, with this new logic in place, if we "normalized" command lines by moving 
> the filename to the end, we'd resolve 
> https://github.com/clangd/clangd/issues/555 without having to complicate the 
> config file model.
> 
> I wonder how hard that is...
yes actually that sounds like a good idea. Ofc, but at the cost of one extra 
command line arg parsing. I don't think we have any other reliable way to 
detect input argument in the flags.

Within `CommandMangler::adjust`, we can start by doing the normalization, 
parsing args considering normalized if it has `--` already, and moving filename 
to the end with a `--` before otherwise.

Shouldn't be too hard, can't think of a better place in the pipeline. We can 
also do this inside OverlayCDB before calling the adjusters, but it feels a 
little bit less natural.

Happy to send out a patch if you think that's a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99523

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

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 348972.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99523

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -123,12 +123,12 @@
 TEST_F(ConfigCompileTests, CompileCommands) {
   Frag.CompileFlags.Add.emplace_back("-foo");
   Frag.CompileFlags.Remove.emplace_back("--include-directory=");
-  std::vector Argv = {"clang", "-I", "bar/", "a.cc"};
+  std::vector Argv = {"clang", "-I", "bar/", "--", "a.cc"};
   EXPECT_TRUE(compileAndApply());
   EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
   for (auto &Edit : Conf.CompileFlags.Edits)
 Edit(Argv);
-  EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
+  EXPECT_THAT(Argv, ElementsAre("clang", "-foo", "--", "a.cc"));
 }
 
 TEST_F(ConfigCompileTests, CompilationDatabase) {
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -41,13 +41,14 @@
   Mangler.ClangPath = testPath("fake/clang");
   Mangler.ResourceDir = testPath("fake/resources");
   Mangler.Sysroot = testPath("fake/sysroot");
-  std::vector Cmd = {"clang++", "-Xclang", "-load", "-Xclang",
-  "plugin",  "-MF", "dep",   "foo.cc"};
+  std::vector Cmd = {"clang++", "-Xclang", "-load",
+  "-Xclang", "plugin",  "-MF",
+  "dep", "--",  "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "foo.cc",
-   "-fsyntax-only",
+  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-fsyntax-only",
"-resource-dir=" + testPath("fake/resources"),
-   "-isysroot", testPath("fake/sysroot")));
+   "-isysroot", testPath("fake/sysroot"), "--",
+   "foo.cc"));
 }
 
 TEST(CommandMangler, ResourceDir) {
@@ -378,4 +379,3 @@
 } // namespace
 } // namespace clangd
 } // namespace clang
-
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -47,6 +47,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include 
 #include 
 
 namespace clang {
@@ -270,7 +271,9 @@
 Add.push_back(std::move(*A));
   Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
-  Args.insert(Args.end(), Add.begin(), Add.end());
+  // The point to insert at. Just append when `--` isn't present.
+  auto It = llvm::find(Args, "--");
+  Args.insert(It, Add.begin(), Add.end());
 });
   });
 }
Index: clang-tools-extra/clangd/CompileCommands.cpp
===
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -209,14 +211,20 @@
   Cmd = tooling::getStripPluginsAdjuster()(Cmd, "");
   Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
 
+  std::vector ToAppend;
   if (ResourceDir && !Has("-resource-dir"))
-Cmd.push_back(("-resource-dir=" + *ResourceDir));
+ToAppend.push_back(("-resource-dir=" + *ResourceDir));
 
   // Don't set `-isysroot` if it is already set or if `--sysroot` is set.
   // `--sysroot` is a superset of the `-isysroot` argument.
   if (Sysroot && !Has("-isysroot") && !Has("--sysroot")) {
-Cmd.push_back("-isysroot");
-Cmd.push_back(*Sysroot);
+ToAppend.push_back("-isysroot");
+ToAppend.push_back(*Sysroot);
+  }
+
+  if (!ToAppend.empty()) {
+Cmd = tooling::getInsertArgumentAdjuster(
+std::move(ToAppend), tooling::ArgumentInsertPosition::END)(Cmd, "");
   }
 
   if (!Cmd.empty()) {
@@ -504,7 +512,6 @@
   Args.resize(Write);
 }
 
-
 std::string printArgv(llvm::ArrayRef Args) {
   std::string Buf;
   llvm::raw_string_ostream OS(Buf);
_

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

Friendly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101645

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

[PATCH] D103252: [C++4OpenCL] Fix missing address space on implicit move assignment operator

[Sven van Haastregt via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D103252

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

[PATCH] D103179: [clangd] Handle queries without an originating file in ProjectAwareIndex

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry for lots of comments on a fairly small patch.

---

I want to mention one alternative we haven't considered. Ultimately the problem 
here is that we have actions that depend on config but we don't know what file 
to pull the config from.
This corresponds to passing Path="" to TUScheduler::run (3 calls) or runQuick 
(0 such calls).
Our decision is to effectively use the config from the last accessed file.
Why don't we put this policy in TUScheduler, rather than inferring it at the 
edges?
Because TUScheduler isn't threadsafe, I think this is as simple as adding a 
`std::string LastFile` member, and reading/writing to it in all the run* 
methods.

---

> But I'd rather not only store Path in the Config since we might end up 
> needing other environment variables in future.

Right. However this patch goes further, and says we must expose *all* other 
parameters, in the same way and with the same semantics (since we only get to 
document them once).
I don't find the argument totally convincing, because currently we have 2 
params, and:

- fresh time shouldn't be exposed at all, unless i'm missing something
- the path is exposed for fairly subtle reasons, and we want to use it 
sparingly and so probably give it quite weak semantics (e.g. wouldn't be 
appropriate to grab "the current filename" for logging purposes, I think)




Comment at: clang-tools-extra/clangd/Config.h:60
+/// slashes. Empty if not configuring a particular file.
+llvm::StringRef Path;
+/// Hint that stale data is OK to improve performance (e.g. avoid IO).

who owns the underlying string, with what lifetime?

The "root" that creates the config doesn't currently have any obligation to 
keep the params around for as long as the config might be active (which can be 
async via context). I think in config this should be a std::string



Comment at: clang-tools-extra/clangd/Config.h:60
+/// slashes. Empty if not configuring a particular file.
+llvm::StringRef Path;
+/// Hint that stale data is OK to improve performance (e.g. avoid IO).

sammccall wrote:
> who owns the underlying string, with what lifetime?
> 
> The "root" that creates the config doesn't currently have any obligation to 
> keep the params around for as long as the config might be active (which can 
> be async via context). I think in config this should be a std::string
How do we expect this to be used?
If it's for determining the active project, then I'd expect us to give some 
weaker semantic like "representative file from the active project" so we can 
fill in e.g. the main file associated with a TU.

But actually we only use the boolean "is this set". Maybe we should just 
provide that?
"bool WasConfiguredForPath" or something? Generalizing past that might be 
premature.



Comment at: clang-tools-extra/clangd/Config.h:69
+  /// Information about the environment used when generating the config.
+  Params Parm;
+

try to avoid using multiple abbreviations of the same word :-)



Comment at: clang-tools-extra/clangd/index/ProjectAware.cpp:152
+  // files open from projects with and without an external index.
+  if (C.Index.External || !C.Parm.Path.empty())
+LastIndex = GetIndexFromSpec(C.Index.External);

This logic feels unnecessarily tricky: you check whether the spec is set both 
here and above.
Consider something longer but more direct

```
SymbolIndex *Specified = C.Index.External ? GetIndexFromSpec(*C.Index.External) 
: nullptr;
if (!Specified && C.Parm.Path.empty()) {
  // There was no index specified, but we're also not sure which path we're 
using.
  // Use the index from the last operation that had a valid path.
  return LastIndex.
}
// Cache the last-used index, even if it's null ("no external index" is a valid 
config!)
LastIndex = Specified;
return Specified;
```



Comment at: clang-tools-extra/clangd/index/ProjectAware.cpp:154
+LastIndex = GetIndexFromSpec(C.Index.External);
+  return LastIndex;
 }

this is racy: if there *is* a well-defined external index for the current 
config, then we should return it even if another thread is able to concurrently 
write to LastIndex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103179

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

[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

@tlively thanks for the comments. I am currently taking some time off but I 
will address your concerns upon my return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95425

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

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added a comment.

In D103080#2790139 , @sunlin wrote:

> Hi @kristof.beyls The  original `lib/clang/12.0.1/include` is about total 
> ~10M, and the `arm*.h` take about ~5M. Ignore these unused header files will 
> save the developers who work on the low storage device.

Thanks for sharing that rationale!
I have two immediate thoughts on this patch:

- arm_neon.h and other header files need to be kept also for the AArch64 
backend. As is, I think the patch will remove them resulting in a broken 
toolchain if the AArch64 backend is requested but the Arm backend is not.
- We should implement either (a) remove all target-specific header files if a 
target is not built or (b) keep all of them. Selecting ad hoc (e.g. applying 
this design principle only for the Arm backend) which ones to apply this policy 
to doesn't seem like a good design to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103080

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

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:732
+if (Adjuster)
   DiagLevel = Adjuster(DiagLevel, Info);
 

sammccall wrote:
> If I'm reading this right:
>  - we previously discarded the diagnostic "quickly" without creating a Diag
>  - now, we construct the Diag as normal but never emit it
>  - ... but we still use the old early-exit codepath for notes, if the primary 
> diagnostic will be discarded
> 
> The new scheme seems functionally correct, but it potentially constructs more 
> Diags. Question is, is it ever enough to care about performance?
> 
> I believe *warnings in headers* are ultimately filtered out by the adjuster 
> and are now extra work/allocations after this patch. Mostly this only affects 
> preamble (exceptions: things like llvm tablegen includes) which is already 
> slow, maybe it doesn't matter.
> 
> If you want to avoid regressing it, I'd suggest keeping the early bail out 
> here, but instead of setting LastPrimaryDiagnosticWasSuppressed, just 
> construct an empty Diag and set its level to Ignored. Rest of the logic can 
> stay the same I think.
> 
> Doesn't seem like much extra code, but if you don't think it's worthwhile 
> that's fine too of course.
I also had the same hesitation, but it looked like this is only suppressing 
diags based on config and tidy suppression comments (ones from header are still 
handled at flush).

So didn't feel like a big deal, But still it can be regression in cases when 
these diags have some complicated fixes attached to them (we'll compute them 
just for dropping on the floor).

The main reasoning for dropping the early exit was because I wanted the logic 
to look more uniform between diagcb and adjusters (to not think about it again 
when the day comes and we merge the two).

So how do you feel about filling in the diagbase in any case (which has some 
string copies, especially around diag message), then running `Adjuster`, 
`DiagCB` and `isExcluded`, and after that we perform early return before adding 
fixes or running include fixer?



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:785
 
-  if (isExcluded(*LastDiag))
+  if (LastDiag->Severity == DiagnosticsEngine::Ignored || 
isExcluded(*LastDiag))
 return;

sammccall wrote:
> (Hmm, the isExcluded case seems like it belongs next to the level adjuster 
> logic rather than at the end.)
i agree, but didn't want to raise eyebrows :P moving it next to adjuster logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103387

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

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

[Sam McCall via Phabricator via cfe-commits]

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

This looks good from my side, thanks for fixing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101645

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

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

IMHO, it's possible to write a frontend test, which includes, say `arm_neon.h`, 
but does not really require the `ARM` or `AArch64` backends to be configured 
(e.g. `CodeGen/arm-vector-align.c`?)
If `arm_neon.h` is not built, then the test would need the appropriate 
`REQUIRES` line, but than that means the frontend test coverage would decrease 
for people, who
are not interested in Arm backends. Mind you, even if a test is Arm specific, 
that does not mean it does not depend on generic code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103080

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

[PATCH] D103380: [C++20] Support for lambdas in unevaluated context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for working on this!

> It does not implement temp.deduct/9.

Is there a reason why?




Comment at: clang/lib/Sema/SemaConcept.cpp:44
 } else if (auto *OO = dyn_cast(E)) {
-  Op = OO->getOperator();
-  LHS = OO->getArg(0);
-  RHS = OO->getArg(1);
+  // If OO is not || or && it might not have exactly 2 arguments
+  if (OO->getNumArgs() == 2) {





Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp:1
+// RUN: %clang_cc1 -std=c++20 %s -verify
+

This test doesn't really have anything to do with [expr.prim.lambda]p2 as it 
appears in C++20 and later, it's more testing the words that used to live in p2 
in C++17 and earlier no longer apply.

I think the functionality in the test is good, but should either be moved to 
the correct file based on what property is being tested or should be moved out 
of the `CXX` directory and into `SemaCXX` (or a combination of both). I'd 
recommend going through the examples presented in the wording of 
https://wg21.link/p0315r4 and make a test for each.



Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp:4
+template  struct Nothing {};
+Nothing<[]() { return 0; }()> nothing;
+

I'd appreciate a test that shows you can also use a lambda that is not 
immediately invoked.



Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp:20
+
+
+

I think it's important to have some tests that show two lambdas are never 
equivalent (http://eel.is/c++draft/temp.over.link#5.sentence-4). Given the 
intent specified in the standard, I think a super useful case would be:
```
template 
auto func() -> decltype(Ty{}.operator()(Uy{})) { return 1; }

int main() {
  auto lambda = [](auto i){ return i; };
  func();
}
```
I *think* that will cause the lambda to be mangled as part of the trailing 
return type.

Other tests would be typical type equivalence tests (like using `std::is_same`).



Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp:26
+// expected-note@-2 {{substitution failure}}
\ No newline at end of file


Can you add the newline to the end of the file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103380

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

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:405
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");

azabaznov wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > This means we now have two separate places that set 
> > > `OpenCLGenericAddressSpace`, the other place being in 
> > > `CompilerInvocation::setLangDefaults()`.  That feels like a maintenance 
> > > hazard.  Do you think it makes sense to set this field in one single 
> > > place instead?
> > I think we should try to set it in `CompilerInvocation.cpp` directly, we 
> > should be able to query `TargetOptions` there. Although that place is 
> > expected to be for the language-specific defaults but we broke the standard 
> > flow by having the language mode controlled by the target settings anyway.
> > 
> > I can't remember though why we have decided to add dedicated `LangOpts` 
> > entries for generic address space instead of just using `OpenCLOptions` 
> > from `Sema`? I think it simplifies the handling of some builtin functions?
> > This means we now have two separate places that set 
> > OpenCLGenericAddressSpace, the other place being in 
> > CompilerInvocation::setLangDefaults(). That feels like a maintenance 
> > hazard. Do you think it makes sense to set this field in one single place 
> > instead?
> 
> >I think we should try to set it in CompilerInvocation.cpp directly, we 
> >should be able to query TargetOptions there. 
> 
> I don't think that we are able to access target options at that stage without 
> modifying current interfaces.  `CompilerInvocation::setLangDefaults()` is a 
> static member function.
> 
> > I can't remember though why we have decided to add dedicated LangOpts 
> > entries for generic address space instead of just using OpenCLOptions from 
> > Sema? I think it simplifies the handling of some builtin functions?
> 
> That's correct. Also, the idea was to reuse generic keyword in other 
> languages.
> I don't think that we are able to access target options at that stage without 
> modifying current interfaces. CompilerInvocation::setLangDefaults() is a 
> static member function.

I wonder if the function is static due to an old interface or something because 
it seems to be only called from `CompilerInvocation::ParseLangArgs` which isn't 
a static member as far as I can see. I wonder if it should just become a 
non-static member instead?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

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

[PATCH] D101816: [clangd] don't rename if the triggering loc is not actually being renamed.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:250
+case ReasonToReject::UnrenamableLoc:
+  return "no rename at the given location";
 }

I don't know what a user would make of this message :-)
It's useful for debugging the implementation, but all we're saying is that we 
generated a list of edits and then our sanity check failed.

This case occurs when the user selected some code that resolved to a renamable 
node, but wasn't actually the name of it. 
It's basically the same as trying to rename foo with `void foo() {^}`, which 
fails with NoSymbolFound.

So I think we should either return NoSymbolFound here or at least use the same 
error message.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:758
+
+  // Check the rename-triggering location is actually being renamed.
+  // This is a robust check to avoid surprising rename results -- if the

nit: robust check -> robustness check

And
"to avoid surprising results if the triggering location *is not actually the 
name of the node we identified* (e.g. for broken code)."
rather than just repeating "is not renamed"



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:1098
+TU.ExtraArgs.push_back("-Xclang");
+TU.ExtraArgs.push_back("-fno-recovery-ast");
 auto AST = TU.build();

this seems like a weird lever to have to use, and will make it awkward if we 
want to test behavior in the presence of recovery AST.

Haha, I remember now, I had a fix to generate recovery initializers in more 
cases which probably made tests harder to right.

That code only works if the field can be resolved though. Maybe try `A() : 
inva^lid(0) {}` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101816

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

[PATCH] D101192: Add support for #elifdef and #elifndef

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:594
+if (CondInfo.FoundElse)
+  Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 

Hi @aaron.ballman 

This change is missing from 
https://reviews.llvm.org/rG8edd3464afbff65d7d5945b3a8b20009d6ff5deb , while you 
instead got it when doing the pp_err_else_after_else diagnostic a couple of 
lines above this.

It causes asserts (in DIagnostic::getArgKind) for me in test cases verifying 
the "elif after else" scenario using a test case basically doing
```
#if 1
#else
#elif
#endif
```
So maybe such a test case should be added as well?

But it seems like the patch you committed doesn't match with the reviewed 
patch. So maybe you can look into this and fix it?
(let me know if you need some additional help)


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

https://reviews.llvm.org/D101192

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

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[Sam McCall via Phabricator via cfe-commits]

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

Thanks for the detailed comments, this makes a lot more sense to me now!




Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:160
+  //
+  // e.g. to complete `- (void)doSomething:(id)argument`:
+  // - Completion name: `doSomething:`

Does completing the no-args declaration `- (void) foo` work as expected?

We never get to this part of the code in this case because endswith(":") is 
never true. The comment above says "safe to treat as c++" but not sure this is 
true for declaration, just method calls. Maybe it works for some other reason 
though?

In any case, it's OK if this case doesn't work (this patch still improves 
things a lot). We should probably have a test showing what the state is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

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

[PATCH] D103221: [HIP] Change default lang std to c++14

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/LangStandards.def:196-197
 // CUDA
 LANGSTANDARD(cuda, "cuda", CUDA, "NVIDIA CUDA(tm)",
  LineComment | CPlusPlus | Digraphs)
 

It would make sense to bump C++ version for CUDA as well. 



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

https://reviews.llvm.org/D103221

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

[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added reviewers: dblaikie, rsmith.
aeubanks published this revision for review.
aeubanks added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is definitely WIP, but I'd like to make sure that this is the right 
direction before continuing. And are there any other obvious places that also 
require this?


With pointee types going away in llvm::PointerType, the frontend needs
to keep track of pointee types.

Add a PointeeType to Address and LValue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103465

Files:
  clang/lib/CodeGen/Address.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGValue.h

Index: clang/lib/CodeGen/CGValue.h
===
--- clang/lib/CodeGen/CGValue.h
+++ clang/lib/CodeGen/CGValue.h
@@ -175,6 +175,7 @@
   } LVType;
 
   llvm::Value *V;
+  llvm::Type *PointeeType;
 
   union {
 // Index into a vector subscript: V[i]
@@ -327,7 +328,7 @@
 return V;
   }
   Address getAddress(CodeGenFunction &CGF) const {
-return Address(getPointer(CGF), getAlignment());
+return Address(getPointer(CGF), PointeeType, getAlignment());
   }
   void setAddress(Address address) {
 assert(isSimple());
@@ -395,6 +396,7 @@
 R.LVType = Simple;
 assert(address.getPointer()->getType()->isPointerTy());
 R.V = address.getPointer();
+R.PointeeType = address.getElementType();
 R.Initialize(type, qs, address.getAlignment(), BaseInfo, TBAAInfo);
 return R;
   }
@@ -405,6 +407,7 @@
 LValue R;
 R.LVType = VectorElt;
 R.V = vecAddress.getPointer();
+R.PointeeType = vecAddress.getElementType();
 R.VectorIdx = Idx;
 R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
  BaseInfo, TBAAInfo);
@@ -417,6 +420,7 @@
 LValue R;
 R.LVType = ExtVectorElt;
 R.V = vecAddress.getPointer();
+R.PointeeType = vecAddress.getElementType();
 R.VectorElts = Elts;
 R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
  BaseInfo, TBAAInfo);
@@ -435,6 +439,7 @@
 LValue R;
 R.LVType = BitField;
 R.V = Addr.getPointer();
+R.PointeeType = Addr.getElementType();
 R.BitFieldInfo = &Info;
 R.Initialize(type, type.getQualifiers(), Addr.getAlignment(), BaseInfo,
  TBAAInfo);
@@ -445,6 +450,7 @@
 LValue R;
 R.LVType = GlobalReg;
 R.V = Reg.getPointer();
+R.PointeeType = Reg.getElementType();
 R.Initialize(type, type.getQualifiers(), Reg.getAlignment(),
  LValueBaseInfo(AlignmentSource::Decl), TBAAAccessInfo());
 return R;
@@ -456,6 +462,7 @@
 LValue R;
 R.LVType = MatrixElt;
 R.V = matAddress.getPointer();
+R.PointeeType = matAddress.getElementType();
 R.VectorIdx = Idx;
 R.Initialize(type, type.getQualifiers(), matAddress.getAlignment(),
  BaseInfo, TBAAInfo);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -70,7 +70,7 @@
  llvm::Value *ArraySize) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getAsAlign());
-  return Address(Alloca, Align);
+  return Address(Alloca, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
@@ -100,7 +100,7 @@
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
   }
 
-  return Address(V, Align);
+  return Address(V, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates an alloca and inserts it into the entry
@@ -156,7 +156,7 @@
 /*ArraySize=*/nullptr, Alloca);
 
   if (Ty->isConstantMatrixType()) {
-auto *ArrayTy = cast(Result.getType()->getElementType());
+auto *ArrayTy = cast(Result.getElementType());
 auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),
 ArrayTy->getNumElements());
 
@@ -1790,8 +1790,7 @@
 // MatrixType), if it points to a array (the memory type of MatrixType).
 static Address MaybeConvertMatrixAddress(Address Addr, CodeGenFunction &CGF,
  bool IsVector = true) {
-  auto *ArrayTy = dyn_cast(
-  cast(Addr.getPointer()->getType())->getElementType());
+  auto *ArrayTy = dyn_cast(Addr.getElementType());
   if (ArrayTy && IsVector) {
 auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),
 ArrayTy->getNumElements());
@@ -1799,7 +1798,7 @@
 return Address(CGF.Builder.CreateElementBitCast(Addr, VectorTy));
   }
   auto *VectorTy = dyn_cast(
-  cast(Addr.getPointer()->getType())->getElementType());
+  cast(Addr.getElementType()));
   if (VectorTy && !IsVector) 

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 348988.
mgartmann added a comment.

- added fixes for private destructors
- separated fixes for private destructors into notes
- added a test case for a `= default;` constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,203 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:7

[PATCH] D103097: Add DWARF address spaces mapping for SPIR

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


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

https://reviews.llvm.org/D103097

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

[PATCH] D102850: [C++4OpenCL] Fix overloading resolution of addrspace constructors

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9870
 
+  if (S.getLangOpts().OpenCL) {
+if (const auto *CD1 = 
dyn_cast_or_null(Cand1.Function)) {

olestrohm wrote:
> Anastasia wrote:
> > olestrohm wrote:
> > > Anastasia wrote:
> > > > olestrohm wrote:
> > > > > Anastasia wrote:
> > > > > > I think we should remove the OpenCL check since it is not OpenCL 
> > > > > > specific rule and you are using common helpers indeed!
> > > > > > 
> > > > > > I also wonder if this should be applied to all member functions not 
> > > > > > only ctors since `isBetterOverloadCandidate` should be used for 
> > > > > > everything?
> > > > > > 
> > > > > > However it seems that other members are already handled somehow 
> > > > > > https://godbolt.org/z/MrWKPKed7. Do you know where this handling 
> > > > > > comes from?
> > > > > It's handled in SemaOverload.cpp:1 in 
> > > > > `OverloadCandidateSet::BestViableFunction` which checks if the given 
> > > > > function is viable or not. For member functions the address space 
> > > > > they're defined in and the address space of the variable have to 
> > > > > match. Therefore only one of them is valid at a time and they never 
> > > > > get checked against each other in `isBetterOverloadCandidate`.
> > > > I am quite sure that for the example above both `__private` and 
> > > > `__generic `overloads are viable. In fact, if you remove the 
> > > > `__private` overload the `__generic` one gets picked.
> > > > 
> > > > So I think the logic should be checking for compatibility rather than 
> > > > the exact match i.e. using `isAddressSpaceSupersetOf` although it might 
> > > > not be calling that helper directly but through the other Qualifier 
> > > > logic.
> > > Ah, I think I checked it wrong last time. Going through it again it's 
> > > actually determined by `QualType::isMoreQualifiedThan` in the end. I'll 
> > > look into it more.
> > So did you happen to find where the address space ranking for the members 
> > is currently happening for this test case https://godbolt.org/z/MrWKPKed7?
> > 
> > I just want to make sure we don't add any redundant checks... so if let's 
> > say we already have a similar logic elsewhere it would be better if we 
> > attempt to generalize it or move it entirely here if it's doable. In 
> > general, we should try to use general qualifier logic if it works instead 
> > of specializing for address spaces.
> It's handled on line 9661 with a call to CompareImplicitConversionSequences, 
> which then goes through a couple of checks and discovers that the return type 
> of one of the functions is more specialized than the other. However this uses 
> the Conversions part of the candidates, which the constructors don't have.
> 
> I did attempt to generalize this further than address spaces, but that caused 
> some tests to fail, so I think it makes sense to only check address spaces 
> here.
Ah I see. Is it because the object parameter is not added implicitly in the 
ctors?

If this is only working for ctors I wonder if we should cast to 
`CXXConstructorDecl` instead of `CXXMethodDecl` or/and also add a comment 
explaining that?


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

https://reviews.llvm.org/D102850

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

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think I'm fine with the general direction, but it would be nice to have a 
CMake option to force the headers for all targets to be built if someone needs 
them.  For static analysis or something like that, I can imagine someone 
building LLVM without any backends at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103080

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

[PATCH] D103048: [IR] make -stack-alignment= into a module attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

bumping for review (or suggestions of additional reviewers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103048

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

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Zhaomo Yang via Phabricator via cfe-commits]

zhaomo updated this revision to Diff 348998.
zhaomo marked an inline comment as done.

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

https://reviews.llvm.org/D103195

Files:
  clang/include/clang/ASTMatchers/GtestMatchers.h
  clang/lib/ASTMatchers/GtestMatchers.cpp
  clang/unittests/ASTMatchers/GtestMatchersTest.cpp

Index: clang/unittests/ASTMatchers/GtestMatchersTest.cpp
===
--- clang/unittests/ASTMatchers/GtestMatchersTest.cpp
+++ clang/unittests/ASTMatchers/GtestMatchersTest.cpp
@@ -42,6 +42,14 @@
 #define EXPECT_PRED_FORMAT2(pred_format, v1, v2) \
 GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_NONFATAL_FAILURE_)
 
+#define GTEST_PRED_FORMAT1_(pred_format, v1, on_failure) \
+  GTEST_ASSERT_(pred_format(#v1, v1), on_failure)
+
+#define EXPECT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_NONFATAL_FAILURE_)
+#define ASSERT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_FATAL_FAILURE_)
+
 #define EXPECT_EQ(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define EXPECT_NE(val1, val2) \
@@ -55,11 +63,29 @@
 #define EXPECT_LT(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::CmpHelperLT, val1, val2)
 
+#define ASSERT_THAT(value, matcher) \
+  ASSERT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+#define EXPECT_THAT(value, matcher) \
+  EXPECT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+
 #define ASSERT_EQ(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define ASSERT_NE(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)
 
+#define GMOCK_ON_CALL_IMPL_(mock_expr, Setter, call)\
+  ((mock_expr).gmock_##call)(::testing::internal::GetWithoutMatchers(), \
+ nullptr)   \
+  .Setter(nullptr, 0, #mock_expr, #call)
+
+#define ON_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalDefaultActionSetAt, call)
+
+#define EXPECT_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalExpectedAt, call)
+
   namespace testing {
   namespace internal {
   class EqHelper {
@@ -96,8 +122,77 @@
   const T2& val2) {
 return 0;
   }
+
+  // For implementing ASSERT_THAT() and EXPECT_THAT().  The template
+  // argument M must be a type that can be converted to a matcher.
+  template 
+  class PredicateFormatterFromMatcher {
+   public:
+explicit PredicateFormatterFromMatcher(M m) : matcher_(m) {}
+
+// This template () operator allows a PredicateFormatterFromMatcher
+// object to act as a predicate-formatter suitable for using with
+// Google Test's EXPECT_PRED_FORMAT1() macro.
+template 
+int operator()(const char* value_text, const T& x) const {
+  return 0;
+}
+
+   private:
+const M matcher_;
+  };
+
+  template 
+  inline PredicateFormatterFromMatcher MakePredicateFormatterFromMatcher(
+  M matcher) {
+return PredicateFormatterFromMatcher(matcher);
+  }
+
+  bool GetWithoutMatchers() { return false; }
+
+  template 
+  class MockSpec {
+   public:
+MockSpec() {}
+
+bool InternalDefaultActionSetAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+bool InternalExpectedAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+MockSpec operator()(bool, void*) {
+  return *this;
+}
+  };  // class MockSpec
+
   }  // namespace internal
+
+  template 
+  int StrEq(T val) {
+return 0;
+  }
+  template 
+  int Eq(T val) {
+return 0;
+  }
+
   }  // namespace testing
+
+  class Mock {
+public:
+Mock() {}
+testing::internal::MockSpec gmock_TwoArgsMethod(int, int) {
+  return testing::internal::MockSpec();
+}
+testing::internal::MockSpec gmock_TwoArgsMethod(bool, void*) {
+  return testing::internal::MockSpec();
+}
+  };  // class Mock
 )cc";
 
 static std::string wrapGtest(llvm::StringRef Input) {
@@ -187,5 +282,137 @@
   matches(wrapGtest(Input), gtestExpect(GtestCmp::Gt, expr(), expr(;
 }
 
+TEST(GtestExpectTest, ThatShouldMatchAssertThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { ASSERT_THAT(2, Eq(2)); }
+  )cc";
+  EXPECT_TRUE(matches(
+  wrapGtest(Input),
+  gtestAssertThat(
+  expr(), callExpr(callee(functionDecl(hasName("::testing::Eq")));
+}
+
+TEST(GtestExpectTest, ThatShouldMatchExpectThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { EXPECT_THAT(2, Eq(2)); }
+  )cc";
+  EXPECT_TRUE(matches(
+  wrapGtest(Input),
+  gtestExpectThat(
+  expr(), callExpr(callee(functionDecl(hasName("::testing::Eq")));
+

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Zhaomo Yang via Phabricator via cfe-commits]

zhaomo added inline comments.



Comment at: clang/lib/ASTMatchers/GtestMatchers.cpp:41
   }
-  llvm_unreachable("Unhandled GtestCmp enum");
 }

hokein wrote:
> zhaomo wrote:
> > hokein wrote:
> > > why remove this `llvm_unreachable`? I think this is a common practice in 
> > > LLVM.
> > ymandel@ suggested me removing it as the switch covers all the possible 
> > values of the enum.
> either seems fine to me (this is just a coding style), I think the problem is 
> that we have inconsistencies in the patch - e.g. on Line95, `default: 
> llvm_unreachable`, we should stick with one style.
The switch statement there does not cover all the cases so `llvm_unreachable` 
is necessary.



Comment at: clang/lib/ASTMatchers/GtestMatchers.cpp:104
+static internal::BindableMatcher
+gtestComparisonInternal(MacroType Macro, GtestCmp Cmp, StatementMatcher Left,
+StatementMatcher Right) {

hokein wrote:
> hokein wrote:
> > As the function creates an AST matcher to match the gtest method, 'd rename 
> > the function name like `gtestComparisonIMatcher`, the same to following 
> > functions.
> this comment seems undone.
Changed it a bit and moved it to the top of this file.


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

https://reviews.llvm.org/D103195

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

[PATCH] D103380: [C++20] Support for lambdas in unevaluated context

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 349000.
cor3ntin added a comment.

Typos, add more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103380

Files:
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp
  clang/test/SemaCXX/anonymous-struct.cpp
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1011,7 +1011,7 @@
 
   Lambdas in unevaluated contexts
   https://wg21.link/p0315r4";>P0315R4
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -49,7 +49,7 @@
 : B { // expected-note {{type is not C-compatible due to this base class}}
 } C; // expected-note {{type is given name 'C' for linkage purposes by this 
typedef declaration}}
 
-#if __cplusplus > 201703L
+#if __cplusplus > 201703L && __cplusplus < 202002L
 typedef struct { // expected-warning {{anonymous non-C-compatible type given 
name for linkage purposes by typedef declaration; add a tag name here}}
   static_assert([]{ return true; }()); // expected-note {{type is not 
C-compatible due to this lambda expression}}
 } Lambda1; // expected-note {{type is given name 'Lambda1' for linkage 
purposes by this typedef declaration}}
Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp
===
--- /dev/null
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+
+template  struct Nothing {};
+Nothing<[]() { return 0; }()> nothing;
+
+template  struct NothingT {};
+Nothing<[]() { return 0; }> nothingT;
+
+template 
+concept True = [] { return true; }();
+static_assert(True);
+
+static_assert(sizeof([] { return 0; }));
+static_assert(sizeof([] { return 0; }()));
+
+void f()  noexcept(noexcept([] { return 0; }()));
+
+using a = decltype([] { return 0; });
+using b = decltype([] { return 0; }());
+using c = decltype([]() noexcept(noexcept([] { return 0; }())) { return 0; });
+using d = decltype(sizeof([] { return 0; }));
+
+template 
+int unique_test1();
+static_assert(&unique_test1<[](){}> != &unique_test1<[](){}>);
+
+template 
+auto g(T) -> decltype([]() { T::invalid; } ());
+auto e = g(0); // expected-error{{no matching function for call}}
+// expected-note@-2 {{substitution failure}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16657,8 +16657,10 @@
 
   if (!Rec.Lambdas.empty()) {
 using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
-if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || 
Rec.isUnevaluated() ||
-(Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
+if (!getLangOpts().CPlusPlus20 &&
+(Rec.ExprContext == ExpressionKind::EK_TemplateArgument ||
+ Rec.isUnevaluated() ||
+ (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17))) {
   unsigned D;
   if (Rec.isUnevaluated()) {
 // C++11 [expr.prim.lambda]p2:
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -41,9 +41,12 @@
   LHS = BO->getLHS();
   RHS = BO->getRHS();
 } else if (auto *OO = dyn_cast(E)) {
-  Op = OO->getOperator();
-  LHS = OO->getArg(0);
-  RHS = OO->getArg(1);
+  // If OO is not || or && it might not have exactly 2 arguments.
+  if (OO->getNumArgs() == 2) {
+Op = OO->getOperator();
+LHS = OO->getArg(0);
+RHS = OO->getArg(1);
+  }
 }
   }
 


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1011,7 +1011,7 @@
 
   Lambdas in unevaluated contexts
   https://wg21.link/p0315r4";>P0315R4
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -49,7 +49,7 @@
 : B { // expected-note {{type is not C-compatible due to this base class}}
 } C; // expected-note {{type is given name 'C' for linkage purposes by this typedef declaration}}
 
-#if __cplusplus > 201703L
+#if __cplusplus > 201703L && __cplusplus < 202002L
 typedef struct { // expected-warning {{anonymous non-C-compatible type given name for

[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

The changes look like the right direction to me - though I don't know/couldn't 
confirm whether more changes will be needed in other places.




Comment at: clang/lib/CodeGen/Address.h:29-30
 public:
   Address(llvm::Value *pointer, CharUnits alignment)
-  : Pointer(pointer), Alignment(alignment) {
+  : Address(pointer, nullptr, alignment) {}
+  Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)

At some point will this include an assertion that 'pointer' isn't a 
PointerType? I guess some uses of PointerTyped values won't need to know their 
pointee type?

(or are all values in Address PointerTyped? (owing to them being "addresses"))?



Comment at: clang/lib/CodeGen/Address.h:56-57
   ///
   /// When IR pointer types lose their element type, we should simply
   /// store it in Address instead for the convenience of writing code.
+  llvm::Type *getElementType() const { return PointeeType; }

hey, someone wrote a handy comment here - could possibly delete this comment 
now that that reality has come to pass



Comment at: clang/lib/CodeGen/CGExpr.cpp:159
   if (Ty->isConstantMatrixType()) {
-auto *ArrayTy = cast(Result.getType()->getElementType());
+auto *ArrayTy = cast(Result.getElementType());
 auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),

Could potentially pull these sort of changes out as a separate patch/commit 
without pre-commit review, since they rely on the existing API.



Comment at: clang/lib/CodeGen/CGValue.h:230-231
 private:
   void Initialize(QualType Type, Qualifiers Quals, CharUnits Alignment,
   LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
 assert((!Alignment.isZero() || Type->isIncompleteType()) &&

Maybe this could use some assertions added to check the PointeeType is correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103465

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

[PATCH] D103380: [C++20] Support for lambdas in unevaluated context

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 349002.
cor3ntin added a comment.

Move tests to SemaCXX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103380

Files:
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/anonymous-struct.cpp
  clang/test/SemaCXX/lambda-unevaluated.cpp
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1011,7 +1011,7 @@
 
   Lambdas in unevaluated contexts
   https://wg21.link/p0315r4";>P0315R4
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+
+template  struct Nothing {};
+Nothing<[]() { return 0; }()> nothing;
+
+template  struct NothingT {};
+Nothing<[]() { return 0; }> nothingT;
+
+template 
+concept True = [] { return true; }();
+static_assert(True);
+
+static_assert(sizeof([] { return 0; }));
+static_assert(sizeof([] { return 0; }()));
+
+void f()  noexcept(noexcept([] { return 0; }()));
+
+using a = decltype([] { return 0; });
+using b = decltype([] { return 0; }());
+using c = decltype([]() noexcept(noexcept([] { return 0; }())) { return 0; });
+using d = decltype(sizeof([] { return 0; }));
+
+template 
+int unique_test1();
+static_assert(&unique_test1<[](){}> != &unique_test1<[](){}>);
+
+template 
+auto g(T) -> decltype([]() { T::invalid; } ());
+auto e = g(0); // expected-error{{no matching function for call}}
+// expected-note@-2 {{substitution failure}}
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -49,7 +49,7 @@
 : B { // expected-note {{type is not C-compatible due to this base class}}
 } C; // expected-note {{type is given name 'C' for linkage purposes by this 
typedef declaration}}
 
-#if __cplusplus > 201703L
+#if __cplusplus > 201703L && __cplusplus < 202002L
 typedef struct { // expected-warning {{anonymous non-C-compatible type given 
name for linkage purposes by typedef declaration; add a tag name here}}
   static_assert([]{ return true; }()); // expected-note {{type is not 
C-compatible due to this lambda expression}}
 } Lambda1; // expected-note {{type is given name 'Lambda1' for linkage 
purposes by this typedef declaration}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16657,8 +16657,10 @@
 
   if (!Rec.Lambdas.empty()) {
 using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
-if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || 
Rec.isUnevaluated() ||
-(Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
+if (!getLangOpts().CPlusPlus20 &&
+(Rec.ExprContext == ExpressionKind::EK_TemplateArgument ||
+ Rec.isUnevaluated() ||
+ (Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17))) {
   unsigned D;
   if (Rec.isUnevaluated()) {
 // C++11 [expr.prim.lambda]p2:
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -41,9 +41,12 @@
   LHS = BO->getLHS();
   RHS = BO->getRHS();
 } else if (auto *OO = dyn_cast(E)) {
-  Op = OO->getOperator();
-  LHS = OO->getArg(0);
-  RHS = OO->getArg(1);
+  // If OO is not || or && it might not have exactly 2 arguments.
+  if (OO->getNumArgs() == 2) {
+Op = OO->getOperator();
+LHS = OO->getArg(0);
+RHS = OO->getArg(1);
+  }
 }
   }
 


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1011,7 +1011,7 @@
 
   Lambdas in unevaluated contexts
   https://wg21.link/p0315r4";>P0315R4
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/lambda-unevaluated.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+
+template  struct Nothing {};
+Nothing<[]() { return 0; }()> nothing;
+
+template  struct NothingT {};
+Nothing<[]() { return 0; }> nothingT;
+
+template 
+concept True = [] { return true; }();
+static_assert(True);
+
+static_assert(sizeof([] { return 0; }));
+static_assert(sizeof([] { return 0; }()));
+
+void f()  noexcept(noexcept([] { return 0; }()));
+
+using a = decltype([] { return 0; }

[PATCH] D103131: support debug info for alias variable

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D103131#2789493 , @probinson wrote:

>> Mixed feelings - somewhat in favor of "do the thing that's probably already 
>> fairly tested/known to work" (GCC's thing). But open to the idea that that 
>> approach has problems, for sure.
>
> "Known to work" for whom?  gdb, sure, because gcc/gdb cooperate on many 
> things.  I'll have to check with my debugger guys whether this would work for 
> us; and I'll just reiterate that it's certainly not what the DWARF spec 
> suggests should be done.

The Sony debugger will not be able to figure out the address of "newname" given 
the DWARF as emitted by gcc (because it looks like an external declaration with 
no address).  We'd much rather have the DW_TAG_imported_declaration that the 
original patch had.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103131

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

[PATCH] D103131: support debug info for alias variable

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Of course, if it turns out that gdb can't handle the imported_declaration, we 
might end up having to do this two different ways under the tuning option.  I'd 
*really* prefer not to do that though, and I'd argue it's a gdb bug if it 
cannot understand imported_declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103131

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

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek added a subscriber: Eugene.Zelenko.
eddy-geek added a comment.

@Eugene.Zelenko wrote:

> Frankly, I don't think that length-based criteria is reasonable one. 
> Readability of code is much more important than line/file length.

It provides an added configuration flexibility without being more complex than 
a boolean. Arguably some people have found the similar 
`modernize-use-auto.MinTypeNameLength` useful (PR here 
)
 -- and I find its default of 5 makes sense as `MyTyp` can be argued as always 
better than `auto` even for people that like `auto`.

That said, I absolutely don't mind changing this PR to define a boolean option 
`UseAuto` instead, if requested.

//(otherwise I'll address review comments tomorrow. Thanks for the review!)//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

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

[PATCH] D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

@yln sorry, I didn't get notifications for this somehow. This got fixed as part 
of the more general support in 
https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98995

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

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 349010.
ldionne added a comment.

Run clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90188

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Parser/using-if-exists-attr.cpp
  clang/test/SemaCXX/attr-deprecated.cpp
  clang/test/SemaCXX/using-if-exists.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6466,6 +6466,7 @@
   case Decl::Concept:
   case Decl::LifetimeExtendedTemporary:
   case Decl::RequiresExprBody:
+  case Decl::UnresolvedUsingIfExists:
 return C;
 
   // Declaration kinds that don't make any sense here, but are
Index: clang/test/SemaCXX/using-if-exists.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/using-if-exists.cpp
@@ -0,0 +1,226 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %s -verify
+
+#define UIE __attribute__((using_if_exists))
+
+namespace test_basic {
+namespace NS {}
+
+using NS::x UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+x usex();// expected-error{{reference to unresolved using declaration}}
+
+using NotNS::x UIE; // expected-error{{use of undeclared identifier 'NotNS'}}
+
+using NS::NotNS::x UIE; // expected-error{{no member named 'NotNS' in namespace 'test_basic::NS'}}
+} // namespace test_basic
+
+namespace test_redecl {
+namespace NS {}
+
+using NS::x UIE;
+using NS::x UIE;
+
+namespace NS1 {}
+namespace NS2 {}
+namespace NS3 {
+int A(); // expected-note{{target of using declaration}}
+struct B {}; // expected-note{{target of using declaration}}
+int C(); // expected-note{{conflicting declaration}}
+struct D {}; // expected-note{{conflicting declaration}}
+} // namespace NS3
+
+using NS1::A UIE;
+using NS2::A UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}} expected-note{{conflicting declaration}}
+using NS3::A UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+int i = A();  // expected-error{{reference to unresolved using declaration}}
+
+using NS1::B UIE;
+using NS2::B UIE; // expected-note{{conflicting declaration}} expected-note{{using declaration annotated with 'using_if_exists' here}}
+using NS3::B UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
+B myB;// expected-error{{reference to unresolved using declaration}}
+
+using NS3::C UIE;
+using NS2::C UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+int j = C();
+
+using NS3::D UIE;
+using NS2::D UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
+D myD;
+} // namespace test_redecl
+
+namespace test_dependent {
+template 
+struct S : B {
+  using B::mf UIE;  // expected-note 3 {{using declaration annotated with 'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}}
+};
+
+struct BaseEmpty {
+};
+struct BaseNonEmpty {
+  void mf();
+  typedef int mt;
+};
+
+template 
+struct UseCtor : Base {
+  using Base::Base UIE; // expected-error{{'using_if_exists' attribute cannot be applied to an inheriting constructor}}
+};
+struct BaseCtor {};
+
+void f() {
+  S empty;
+  S nonempty;
+  empty.mf(); // expected-error {{reference to unresolved using declaration}}
+  nonempty.mf();
+  (&empty)->mf(); // expected-error {{reference to unresolved using declaration}}
+  (&nonempty)->mf();
+
+  S::mt y; // expected-error {{reference to unresolved using declaration}}
+  S::mt z;
+
+  S::mf(); // expected-error {{reference to unresolved using declaration}}
+
+  UseCtor usector;
+}
+
+template 
+struct Implicit : B {
+  using B::mf UIE;  // expected-note {{using declaration annotated with 'using_if_exists' here}}
+  using typename B::mt UIE; // expected-note 2 {{using declaration anno

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 349014.
dgoldman added a comment.

Add another test for a simple method decl (no arg selector)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

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

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2783,6 +2783,79 @@
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCSimpleMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (void)foo;
+  @end
+  @implementation Foo
+  fo^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("foo")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (void)")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (int)")));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c secondArgument:(id)object")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationPrefixTyped) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c;
+  @end
+  @implementation Foo
+  - (int)valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationFromMiddle) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  - (int)valueForCharacter:(char)c second^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("secondArgument:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(id)object")));
+}
+
 TEST(CompletionTest, CursorInSnippets) {
   clangd::CodeCompleteOptions Options;
   Options.EnableSnippets = true;
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@
   }
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
+  bool HadInformativeChunks = false;
   for (const auto &Chunk : CCS) {
 // Informative qualifier chunks only clutter completion results, skip
 // them.
@@ -129,10 +130,14 @@
   //   reclassified as qualifiers.
   //
   // Objective-C:
-  //   Objective-C methods may have multiple typed-text chunks, so we must
-  //   treat them carefully. For Objective-C methods, all typed-text chunks
-  //   will end in ':' (unless there are no arguments, in which case we
-  //   can safely treat them as C++).
+  //   Objective-C methods expressions may have multiple typed-text chunks,
+  //   so we must treat them carefully. For Objective-C methods, all
+  //   typed-text and informative chunks will end in ':' (unless there are
+  //   no arguments, in which case we can safely treat them as C++).
+  //
+  //   Completing a method declaration itself (not a method expression) is
+  //   similar except that we use the `RequiredQualifiers` to store the
+  //   text before the selector, e.g. `- (void)`.
   if (!llvm::StringRef(Chunk.Text).endswith(":")) { // Treat as C++.
 if (RequiredQualifiers)
   *RequiredQualifiers = std::move(*Signature);
@@ -147,6 +152,28 @@
 // methods.
 if (!HadObjCArguments) {
   HadObjCArguments = true;
+  // If we have no previous informative chunks (informative sel

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via Phabricator via cfe-commits]

dgoldman marked an inline comment as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:160
+  //
+  // e.g. to complete `- (void)doSomething:(id)argument`:
+  // - Completion name: `doSomething:`

sammccall wrote:
> Does completing the no-args declaration `- (void) foo` work as expected?
> 
> We never get to this part of the code in this case because endswith(":") is 
> never true. The comment above says "safe to treat as c++" but not sure this 
> is true for declaration, just method calls. Maybe it works for some other 
> reason though?
> 
> In any case, it's OK if this case doesn't work (this patch still improves 
> things a lot). We should probably have a test showing what the state is.
Yeah, it does. I added a test case for it, it does work, the current behavior 
is fine, the issue for ObjC is specifically selectors with arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

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

[clang-tools-extra] 2a030e6 - [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2021-06-01T13:35:05-04:00
New Revision: 2a030e680e0812c652ed4ae2b012e285a5514ffa

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

LOG: [clangd][ObjC] Fix issue completing a method decl by name

When completing an Objective-C method declaration by name, we need to
preserve the leading text as a `qualifier` so we insert it properly
before the first typed text chunk.

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeCompletionStrings.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp 
b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index d4a3bdafcae0..8205c88a5b66 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@ void getSignature(const CodeCompletionString &CCS, 
std::string *Signature,
   }
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
+  bool HadInformativeChunks = false;
   for (const auto &Chunk : CCS) {
 // Informative qualifier chunks only clutter completion results, skip
 // them.
@@ -129,10 +130,14 @@ void getSignature(const CodeCompletionString &CCS, 
std::string *Signature,
   //   reclassified as qualifiers.
   //
   // Objective-C:
-  //   Objective-C methods may have multiple typed-text chunks, so we must
-  //   treat them carefully. For Objective-C methods, all typed-text chunks
-  //   will end in ':' (unless there are no arguments, in which case we
-  //   can safely treat them as C++).
+  //   Objective-C methods expressions may have multiple typed-text chunks,
+  //   so we must treat them carefully. For Objective-C methods, all
+  //   typed-text and informative chunks will end in ':' (unless there are
+  //   no arguments, in which case we can safely treat them as C++).
+  //
+  //   Completing a method declaration itself (not a method expression) is
+  //   similar except that we use the `RequiredQualifiers` to store the
+  //   text before the selector, e.g. `- (void)`.
   if (!llvm::StringRef(Chunk.Text).endswith(":")) { // Treat as C++.
 if (RequiredQualifiers)
   *RequiredQualifiers = std::move(*Signature);
@@ -147,6 +152,28 @@ void getSignature(const CodeCompletionString &CCS, 
std::string *Signature,
 // methods.
 if (!HadObjCArguments) {
   HadObjCArguments = true;
+  // If we have no previous informative chunks (informative selector
+  // fragments in practice), we treat any previous chunks as
+  // `RequiredQualifiers` so they will be added as a prefix during the
+  // completion.
+  //
+  // e.g. to complete `- (void)doSomething:(id)argument`:
+  // - Completion name: `doSomething:`
+  // - RequiredQualifiers: `- (void)`
+  // - Snippet/Signature suffix: `(id)argument`
+  //
+  // This 
diff ers from the case when we're completing a method
+  // expression with a previous informative selector fragment.
+  //
+  // e.g. to complete `[self doSomething:nil ^somethingElse:(id)]`:
+  // - Previous Informative Chunk: `doSomething:`
+  // - Completion name: `somethingElse:`
+  // - Snippet/Signature suffix: `(id)`
+  if (!HadInformativeChunks) {
+if (RequiredQualifiers)
+  *RequiredQualifiers = std::move(*Signature);
+Snippet->clear();
+  }
   Signature->clear();
 } else { // Subsequent argument, considered part of snippet/signature.
   *Signature += Chunk.Text;
@@ -173,6 +200,7 @@ void getSignature(const CodeCompletionString &CCS, 
std::string *Signature,
   *Snippet += '}';
   break;
 case CodeCompletionString::CK_Informative:
+  HadInformativeChunks = true;
   // For example, the word "const" for a const method, or the name of
   // the base class for methods that are part of the base class.
   *Signature += Chunk.Text;

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e4e56f2b5e38..78f6e7c4aa9f 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2796,6 +2796,79 @@ TEST(CompletionTest, 
ObjectiveCMethodTwoArgumentsFromMiddle) {
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCSimpleMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (void)foo;
+  @end

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dgoldman marked an inline comment as done.
Closed by commit rG2a030e680e08: [clangd][ObjC] Fix issue completing a method 
decl by name (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

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

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2796,6 +2796,79 @@
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCSimpleMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (void)foo;
+  @end
+  @implementation Foo
+  fo^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("foo")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (void)")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (int)")));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c secondArgument:(id)object")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationPrefixTyped) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c;
+  @end
+  @implementation Foo
+  - (int)valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationFromMiddle) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  - (int)valueForCharacter:(char)c second^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("secondArgument:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(id)object")));
+}
+
 TEST(CompletionTest, CursorInSnippets) {
   clangd::CodeCompleteOptions Options;
   Options.EnableSnippets = true;
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@
   }
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
+  bool HadInformativeChunks = false;
   for (const auto &Chunk : CCS) {
 // Informative qualifier chunks only clutter completion results, skip
 // them.
@@ -129,10 +130,14 @@
   //   reclassified as qualifiers.
   //
   // Objective-C:
-  //   Objective-C methods may have multiple typed-text chunks, so we must
-  //   treat them carefully. For Objective-C methods, all typed-text chunks
-  //   will end in ':' (unless there are no arguments, in which case we
-  //   can safely treat them as C++).
+  //   Objective-C methods expressions may have multiple typed-text chunks,
+  //   so we must treat them carefully. For Objective-C methods, all
+  //   typed-text and informative chunks will end in ':' (unless there are
+  //   no arguments, in which case we can safely treat them as C++).
+  //
+  //   Completing a method declaration itself (not a method expression) is
+  //   similar except that we use the `RequiredQualifiers` to store the
+  //   text before the selector, e.g. `- (void)`.
   if (!llvm::StringRef(Chunk.Text).endswith(":")) { // Treat as C++.
 if (RequiredQualifiers)
   *RequiredQualifiers = std::move(*Signature);
@@ -147,6 +152,28 @@
 

[PATCH] D103472: [clang] Fix a crash during code completion

[Adam Czachorowski via Phabricator via cfe-commits]

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

During code completion, lookupInDeclContext() calls
CodeCompletionDeclConsumer::FoundDecl(),which can mutate StoredDeclsMap,
over which lookupInDeclContext() iterates. This can lead to invalidation
of iterators and an assert()-crash.

Example code where this happens:
 #include 
 int main() {

  std::list;
  std::^

}
with code completion on ^ with -std=c++20.

I do not have a repro case that does not need standard library.

This fix stores pointers to NamedDecls in a temporary vector, then
visits them outside of the main loop, when StoredDeclsMap iterators are
gone.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103472

Files:
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3835,6 +3835,7 @@
 if (CXXRecordDecl *Class = dyn_cast(Ctx))
   Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
+llvm::SmallVector DeclsToVisit;
 // We sometimes skip loading namespace-level results (they tend to be 
huge).
 bool Load = LoadExternal ||
 !(isa(Ctx) || isa(Ctx));
@@ -3844,12 +3845,21 @@
   : Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
   for (auto *D : R) {
 if (auto *ND = Result.getAcceptableDecl(D)) {
-  Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-  Visited.add(ND);
+  // Rather than visit immediatelly, we put ND into a vector and visit
+  // all decls, in order, outside of this loop. The reason is that
+  // Consumer.FoundDecl() may invalidate the iterators used in the two
+  // loops above.
+  DeclsToVisit.push_back(ND);
 }
   }
 }
 
+for (auto *ND : DeclsToVisit) {
+  Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+  Visited.add(ND);
+}
+DeclsToVisit.clear();
+
 // Traverse using directives for qualified name lookup.
 if (QualifiedNameLookup) {
   ShadowContextRAII Shadow(Visited);


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3835,6 +3835,7 @@
 if (CXXRecordDecl *Class = dyn_cast(Ctx))
   Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
+llvm::SmallVector DeclsToVisit;
 // We sometimes skip loading namespace-level results (they tend to be huge).
 bool Load = LoadExternal ||
 !(isa(Ctx) || isa(Ctx));
@@ -3844,12 +3845,21 @@
   : Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
   for (auto *D : R) {
 if (auto *ND = Result.getAcceptableDecl(D)) {
-  Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-  Visited.add(ND);
+  // Rather than visit immediatelly, we put ND into a vector and visit
+  // all decls, in order, outside of this loop. The reason is that
+  // Consumer.FoundDecl() may invalidate the iterators used in the two
+  // loops above.
+  DeclsToVisit.push_back(ND);
 }
   }
 }
 
+for (auto *ND : DeclsToVisit) {
+  Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+  Visited.add(ND);
+}
+DeclsToVisit.clear();
+
 // Traverse using directives for qualified name lookup.
 if (QualifiedNameLookup) {
   ShadowContextRAII Shadow(Visited);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103472: [clang] Fix a crash during code completion

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a reviewer: kadircet.
adamcz added a comment.

This is https://github.com/clangd/clangd/issues/771

Sending for review mostly to see if you have any comments on this approach. 
Trying to reproduce this without ,  or something like this didn't 
work for me so far, I must be holding it wrong or something ;-) Still, 
submitting without a nice test case seems wrong.
The whole C++20, as far as I can tell, simply adds a bunch of code that's 
behind ifdef, which is what causes the crash. I don't think this is related to 
any C++20 features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103472

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

[clang] baa2b8d - Fix a git apply that went bad somehow.

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-06-01T14:06:39-04:00
New Revision: baa2b8d08502acfa91a8dfd699d25f7b4e25edbb

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

LOG: Fix a git apply that went bad somehow.

When applying the changes in 8edd3464afbff65d7d5945b3a8b20009d6ff5deb,
it seems that this bit got merged incorrectly and no test coverage
caught the issue. This fixes the diagnostic and adds a test.

Added: 


Modified: 
clang/lib/Lex/PPDirectives.cpp
clang/test/Preprocessor/elifdef.c

Removed: 




diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 8fe70668a4060..87741b0a024a2 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -587,7 +587,7 @@ void 
Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
 
 // If this is a #else with a #else before it, report the error.
 if (CondInfo.FoundElse)
-  Diag(Tok, diag::pp_err_else_after_else) << PED_Elif;
+  Diag(Tok, diag::pp_err_else_after_else);
 
 // Note that we've seen a #else in this conditional.
 CondInfo.FoundElse = true;
@@ -611,7 +611,8 @@ void 
Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
 PPConditionalInfo &CondInfo = CurPPLexer->peekConditionalLevel();
 
 // If this is a #elif with a #else before it, report the error.
-if (CondInfo.FoundElse) Diag(Tok, diag::pp_err_elif_after_else);
+if (CondInfo.FoundElse)
+  Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 
 // If this is in a skipping block or if we're already handled this #if
 // block, don't bother parsing the condition.

diff  --git a/clang/test/Preprocessor/elifdef.c 
b/clang/test/Preprocessor/elifdef.c
index 3954159c5e08e..6bc467d70011f 100644
--- a/clang/test/Preprocessor/elifdef.c
+++ b/clang/test/Preprocessor/elifdef.c
@@ -105,3 +105,9 @@
 #elifdef
 #elifndef
 #endif
+
+/* expected-error@+3 {{#elif after #else}}*/
+#if 1
+#else
+#elif
+#endif



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

[PATCH] D101192: Add support for #elifdef and #elifndef

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:594
+if (CondInfo.FoundElse)
+  Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 

bjope wrote:
> Hi @aaron.ballman 
> 
> This change is missing from 
> https://reviews.llvm.org/rG8edd3464afbff65d7d5945b3a8b20009d6ff5deb , while 
> you instead got it when doing the pp_err_else_after_else diagnostic a couple 
> of lines above this.
> 
> It causes asserts (in DIagnostic::getArgKind) for me in test cases verifying 
> the "elif after else" scenario using a test case basically doing
> ```
> #if 1
> #else
> #elif
> #endif
> ```
> So maybe such a test case should be added as well?
> 
> But it seems like the patch you committed doesn't match with the reviewed 
> patch. So maybe you can look into this and fix it?
> (let me know if you need some additional help)
Great catch and thank you for the test case! It seems that when I applied the 
patch, it misapplied, but not in a way that caused a test failure. I have no 
idea how that happened, but I assume I screwed up something with git somewhere 
along the way and caused the failure.

I've pushed baa2b8d08502acfa91a8dfd699d25f7b4e25edbb to fix it. Thanks!


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

https://reviews.llvm.org/D101192

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

[PATCH] D103401: [OpenCL] Add support of __opencl_c_generic_address_space feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/lib/Basic/TargetInfo.cpp:405
+  const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+  Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+  OpenCLFeaturesMap, "__opencl_c_generic_address_space");

Anastasia wrote:
> azabaznov wrote:
> > Anastasia wrote:
> > > svenvh wrote:
> > > > This means we now have two separate places that set 
> > > > `OpenCLGenericAddressSpace`, the other place being in 
> > > > `CompilerInvocation::setLangDefaults()`.  That feels like a maintenance 
> > > > hazard.  Do you think it makes sense to set this field in one single 
> > > > place instead?
> > > I think we should try to set it in `CompilerInvocation.cpp` directly, we 
> > > should be able to query `TargetOptions` there. Although that place is 
> > > expected to be for the language-specific defaults but we broke the 
> > > standard flow by having the language mode controlled by the target 
> > > settings anyway.
> > > 
> > > I can't remember though why we have decided to add dedicated `LangOpts` 
> > > entries for generic address space instead of just using `OpenCLOptions` 
> > > from `Sema`? I think it simplifies the handling of some builtin functions?
> > > This means we now have two separate places that set 
> > > OpenCLGenericAddressSpace, the other place being in 
> > > CompilerInvocation::setLangDefaults(). That feels like a maintenance 
> > > hazard. Do you think it makes sense to set this field in one single place 
> > > instead?
> > 
> > >I think we should try to set it in CompilerInvocation.cpp directly, we 
> > >should be able to query TargetOptions there. 
> > 
> > I don't think that we are able to access target options at that stage 
> > without modifying current interfaces.  
> > `CompilerInvocation::setLangDefaults()` is a static member function.
> > 
> > > I can't remember though why we have decided to add dedicated LangOpts 
> > > entries for generic address space instead of just using OpenCLOptions 
> > > from Sema? I think it simplifies the handling of some builtin functions?
> > 
> > That's correct. Also, the idea was to reuse generic keyword in other 
> > languages.
> > I don't think that we are able to access target options at that stage 
> > without modifying current interfaces. CompilerInvocation::setLangDefaults() 
> > is a static member function.
> 
> I wonder if the function is static due to an old interface or something 
> because it seems to be only called from `CompilerInvocation::ParseLangArgs` 
> which isn't a static member as far as I can see. I wonder if it should just 
> become a non-static member instead?
> 
Actually `CompilerInvocation::ParseLangArgs` is static as well. Anyway, I think 
this change would be really impactful. I believe `::adjust` is a prefect place 
to coordinate language options with target information (at least for now).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103401

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

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This enables requests like workspaceSymbols to be dispatched using the
file user was most recently operating on. A replacement for D103179 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103476

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1264,6 +1264,69 @@
   << "association still valid";
 }
 
+TEST_F(TUSchedulerTests, PreservesLastActiveFile) {
+  for (bool Sync : {false, true}) {
+auto Opts = optsForTest();
+if (Sync)
+  Opts.AsyncThreadsCount = 0;
+TUScheduler S(CDB, Opts);
+
+auto CheckNoFileActionsSeesLastActiveFile =
+[&](llvm::StringRef LastActiveFile) {
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  std::atomic Counter(0);
+  // We only check for run and runQuick as runWithAST and
+  // runWithPreamble is always bound to a file.
+  S.run("run-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  S.runQuick("runQuick-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(2, Counter.load());
+};
+
+// Check that we see no file initially
+CheckNoFileActionsSeesLastActiveFile("");
+
+// Now check that every action scheduled with a particular file changes the
+// LastActiveFile.
+auto Path = testPath("run.cc");
+S.run(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runQuick.cc");
+S.runQuick(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithAST(Path, Path, [](llvm::Expected Inp) {
+  EXPECT_TRUE(bool(Inp));
+});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithPreamble.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithPreamble(
+Path, Path, TUScheduler::Stale,
+[](llvm::Expected Inp) { EXPECT_TRUE(bool(Inp)); });
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("update.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+// An update with the same contents should not change LastActiveFile.
+auto LastActive = Path;
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(LastActive);
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -341,6 +342,9 @@
   // asynchronously.
   llvm::Optional PreambleTasks;
   llvm::Optional WorkerThreads;
+  // Used to create contexts for operations that are not bound to a particular
+  // file (e.g. index queries).
+  std::string LastActiveFile;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1506,6 +1506,10 @@
 FD->Contents = Inputs.Contents;
   }
   FD->Worker->update(std::move(Inputs), WantDiags, ContentChanged);
+  // There might be synthetic update requests, don't change the LastActiveFile
+  // in such cases.
+  if (ContentChanged)
+LastActiveFile = File.str();
   return NewFile;
 }
 
@@ -1535,16 +1539,18 @@
 void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action,
Semaphore &Sem) {
+  if (!Path.empty())
+LastActiveFile = Path.str();
   if (!PreambleTasks) {
-WithContext WithProvidedContext(Opts.ContextProvider(Path));
+WithContext WithProvidedContext(Opts.ContextProvider(LastAct

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

[Zhaomo Yang via Phabricator via cfe-commits]

zhaomo updated this revision to Diff 349030.

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

https://reviews.llvm.org/D103195

Files:
  clang/include/clang/ASTMatchers/GtestMatchers.h
  clang/lib/ASTMatchers/GtestMatchers.cpp
  clang/unittests/ASTMatchers/GtestMatchersTest.cpp

Index: clang/unittests/ASTMatchers/GtestMatchersTest.cpp
===
--- clang/unittests/ASTMatchers/GtestMatchersTest.cpp
+++ clang/unittests/ASTMatchers/GtestMatchersTest.cpp
@@ -42,6 +42,14 @@
 #define EXPECT_PRED_FORMAT2(pred_format, v1, v2) \
 GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_NONFATAL_FAILURE_)
 
+#define GTEST_PRED_FORMAT1_(pred_format, v1, on_failure) \
+  GTEST_ASSERT_(pred_format(#v1, v1), on_failure)
+
+#define EXPECT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_NONFATAL_FAILURE_)
+#define ASSERT_PRED_FORMAT1(pred_format, v1) \
+  GTEST_PRED_FORMAT1_(pred_format, v1, GTEST_FATAL_FAILURE_)
+
 #define EXPECT_EQ(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define EXPECT_NE(val1, val2) \
@@ -55,11 +63,29 @@
 #define EXPECT_LT(val1, val2) \
 EXPECT_PRED_FORMAT2(::testing::internal::CmpHelperLT, val1, val2)
 
+#define ASSERT_THAT(value, matcher) \
+  ASSERT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+#define EXPECT_THAT(value, matcher) \
+  EXPECT_PRED_FORMAT1(  \
+  ::testing::internal::MakePredicateFormatterFromMatcher(matcher), value)
+
 #define ASSERT_EQ(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 #define ASSERT_NE(val1, val2) \
 ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)
 
+#define GMOCK_ON_CALL_IMPL_(mock_expr, Setter, call)\
+  ((mock_expr).gmock_##call)(::testing::internal::GetWithoutMatchers(), \
+ nullptr)   \
+  .Setter(nullptr, 0, #mock_expr, #call)
+
+#define ON_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalDefaultActionSetAt, call)
+
+#define EXPECT_CALL(obj, call) \
+  GMOCK_ON_CALL_IMPL_(obj, InternalExpectedAt, call)
+
   namespace testing {
   namespace internal {
   class EqHelper {
@@ -96,8 +122,77 @@
   const T2& val2) {
 return 0;
   }
+
+  // For implementing ASSERT_THAT() and EXPECT_THAT().  The template
+  // argument M must be a type that can be converted to a matcher.
+  template 
+  class PredicateFormatterFromMatcher {
+   public:
+explicit PredicateFormatterFromMatcher(M m) : matcher_(m) {}
+
+// This template () operator allows a PredicateFormatterFromMatcher
+// object to act as a predicate-formatter suitable for using with
+// Google Test's EXPECT_PRED_FORMAT1() macro.
+template 
+int operator()(const char* value_text, const T& x) const {
+  return 0;
+}
+
+   private:
+const M matcher_;
+  };
+
+  template 
+  inline PredicateFormatterFromMatcher MakePredicateFormatterFromMatcher(
+  M matcher) {
+return PredicateFormatterFromMatcher(matcher);
+  }
+
+  bool GetWithoutMatchers() { return false; }
+
+  template 
+  class MockSpec {
+   public:
+MockSpec() {}
+
+bool InternalDefaultActionSetAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+bool InternalExpectedAt(
+const char* file, int line, const char* obj, const char* call) {
+  return false;
+}
+
+MockSpec operator()(bool, void*) {
+  return *this;
+}
+  };  // class MockSpec
+
   }  // namespace internal
+
+  template 
+  int StrEq(T val) {
+return 0;
+  }
+  template 
+  int Eq(T val) {
+return 0;
+  }
+
   }  // namespace testing
+
+  class Mock {
+public:
+Mock() {}
+testing::internal::MockSpec gmock_TwoArgsMethod(int, int) {
+  return testing::internal::MockSpec();
+}
+testing::internal::MockSpec gmock_TwoArgsMethod(bool, void*) {
+  return testing::internal::MockSpec();
+}
+  };  // class Mock
 )cc";
 
 static std::string wrapGtest(llvm::StringRef Input) {
@@ -187,5 +282,137 @@
   matches(wrapGtest(Input), gtestExpect(GtestCmp::Gt, expr(), expr(;
 }
 
+TEST(GtestExpectTest, ThatShouldMatchAssertThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { ASSERT_THAT(2, Eq(2)); }
+  )cc";
+  EXPECT_TRUE(matches(
+  wrapGtest(Input),
+  gtestAssertThat(
+  expr(), callExpr(callee(functionDecl(hasName("::testing::Eq")));
+}
+
+TEST(GtestExpectTest, ThatShouldMatchExpectThat) {
+  std::string Input = R"cc(
+using ::testing::Eq;
+void Test() { EXPECT_THAT(2, Eq(2)); }
+  )cc";
+  EXPECT_TRUE(matches(
+  wrapGtest(Input),
+  gtestExpectThat(
+  expr(), callExpr(callee(functionDecl(hasName("::testing::Eq")));
+}
+
+TEST(GtestOnCallTest, CallShouldMatc

[PATCH] D103477: [Fuchsia] Add compat multilibs to cache file

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.
Herald added a subscriber: mgorny.
leonardchan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103477

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


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -195,6 +195,12 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
+# Compat multilibs.
+set(RUNTIMES_${target}+compat_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
+set(RUNTIMES_${target}+compat_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL 
"")
+set(RUNTIMES_${target}+compat_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
+set(RUNTIMES_${target}+compat_CMAKE_CXX_FLAGS 
"${FUCHSIA_${target}_COMPILER_FLAGS} -fc++-abi=itanium" CACHE STRING "")
+
 set(RUNTIMES_${target}+asan_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
 set(RUNTIMES_${target}+asan_LLVM_USE_SANITIZER "Address" CACHE STRING "")
 set(RUNTIMES_${target}+asan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF 
CACHE BOOL "")
@@ -237,9 +243,10 @@
 list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
   endforeach()
 
-  set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;asan+noexcept;relative-vtables;relative-vtables+noexcept;relative-vtables+asan;relative-vtables+asan+noexcept"
 CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;compat;asan+noexcept;relative-vtables;relative-vtables+noexcept;relative-vtables+asan;relative-vtables+asan+noexcept"
 CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_relative-vtables_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_relative-vtables+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -195,6 +195,12 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
+# Compat multilibs.
+set(RUNTIMES_${target}+compat_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
+set(RUNTIMES_${target}+compat_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
+set(RUNTIMES_${target}+compat_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
+set(RUNTIMES_${target}+compat_CMAKE_CXX_FLAGS "${FUCHSIA_${target}_COMPILER_FLAGS} -fc++-abi=itanium" CACHE STRING "")
+
 set(RUNTIMES_${target}+asan_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
 set(RUNTIMES_${target}+asan_LLVM_USE_SANITIZER "Address" CACHE STRING "")
 set(RUNTIMES_${target}+asan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
@@ -237,9 +243,10 @@
 list(APPEND RUNTIME_BUILD_ID_LINK "${target}")
   endforeach()
 
-  set(LLVM_RUNTIME_MULTILIBS "asan;noexcept;asan+noexcept;relative-vtables;relative-vtables+noexcept;relative-vtables+asan;relative-vtables+asan+noexcept" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIBS "asan;noexcept;compat;asan+noexcept;relative-vtables;relative-vtables+noexcept;relative-vtables+asan;relative-vtables+asan+noexcept" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_asan_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_relative-vtables_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
   set(LLVM_RUNTIME_MULTILIB_relative-vtables+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101192: Add support for #elifdef and #elifndef

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:594
+if (CondInfo.FoundElse)
+  Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 

aaron.ballman wrote:
> bjope wrote:
> > Hi @aaron.ballman 
> > 
> > This change is missing from 
> > https://reviews.llvm.org/rG8edd3464afbff65d7d5945b3a8b20009d6ff5deb , while 
> > you instead got it when doing the pp_err_else_after_else diagnostic a 
> > couple of lines above this.
> > 
> > It causes asserts (in DIagnostic::getArgKind) for me in test cases 
> > verifying the "elif after else" scenario using a test case basically doing
> > ```
> > #if 1
> > #else
> > #elif
> > #endif
> > ```
> > So maybe such a test case should be added as well?
> > 
> > But it seems like the patch you committed doesn't match with the reviewed 
> > patch. So maybe you can look into this and fix it?
> > (let me know if you need some additional help)
> Great catch and thank you for the test case! It seems that when I applied the 
> patch, it misapplied, but not in a way that caused a test failure. I have no 
> idea how that happened, but I assume I screwed up something with git 
> somewhere along the way and caused the failure.
> 
> I've pushed baa2b8d08502acfa91a8dfd699d25f7b4e25edbb to fix it. Thanks!
Big thanks for the quick fix! While I managed to track down this problem I 
didn't have time to properly fix it right away. Now I'll have one thing less to 
worry about tomorrow :-)


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

https://reviews.llvm.org/D101192

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

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13a8aa3ee15a: [clang] RecursiveASTVisitor visits 
ObjCPropertyRefExpr's class receiver (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101645

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/SemaAvailability.cpp
  clang/test/Index/Core/index-source.m

Index: clang/test/Index/Core/index-source.m
===
--- clang/test/Index/Core/index-source.m
+++ clang/test/Index/Core/index-source.m
@@ -405,15 +405,15 @@
 
 void classReceivers() {
   ClassReceivers.p1 = 0;
-// CHECK: [[@LINE-1]]:3 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OBJC_CLASS_$_ClassReceivers | Ref,RelCont | rel: 1
-// CHECK: [[@LINE-2]]:18 | instance-property/ObjC | p1 | c:objc(cs)ClassReceivers(cpy)p1 |  | Ref,Writ,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:18 | instance-property/ObjC | p1 | c:objc(cs)ClassReceivers(cpy)p1 |  | Ref,Writ,RelCont | rel: 1
 // CHECK-NEXT: RelCont | classReceivers | c:@F@classReceivers
+// CHECK: [[@LINE-3]]:3 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OBJC_CLASS_$_ClassReceivers | Ref,RelCont | rel: 1
 // CHECK: [[@LINE-4]]:18 | class-method/acc-set/ObjC | setP1: | c:objc(cs)ClassReceivers(cm)setP1: | +[ClassReceivers setP1:] | Ref,Call,Impl,RelCall,RelCont | rel: 1
 // CHECK-NEXT: RelCall,RelCont | classReceivers | c:@F@classReceivers
   (void)ClassReceivers.p1;
-// CHECK: [[@LINE-1]]:9 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OBJC_CLASS_$_ClassReceivers | Ref,RelCont | rel: 1
-// CHECK: [[@LINE-2]]:24 | instance-property/ObjC | p1 | c:objc(cs)ClassReceivers(cpy)p1 |  | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:24 | instance-property/ObjC | p1 | c:objc(cs)ClassReceivers(cpy)p1 |  | Ref,RelCont | rel: 1
 // CHECK-NEXT: RelCont | classReceivers | c:@F@classReceivers
+// CHECK: [[@LINE-3]]:9 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OBJC_CLASS_$_ClassReceivers | Ref,RelCont | rel: 1
 // CHECK: [[@LINE-4]]:24 | class-method/acc-get/ObjC | p1 | c:objc(cs)ClassReceivers(cm)p1 | +[ClassReceivers p1] | Ref,Call,Impl,RelCall,RelCont | rel: 1
 // CHECK-NEXT: RelCall,RelCont | classReceivers | c:@F@classReceivers
 
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -683,11 +683,7 @@
   // to any useful diagnostics.
   bool TraverseCaseStmt(CaseStmt *CS) { return TraverseStmt(CS->getSubStmt()); }
 
-  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
-if (PRE->isClassReceiver())
-  DiagnoseDeclAvailability(PRE->getClassReceiver(), PRE->getReceiverLocation());
-return true;
-  }
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) { return true; }
 
   bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
 if (ObjCMethodDecl *D = Msg->getMethodDecl()) {
Index: clang/lib/Index/IndexBody.cpp
===
--- clang/lib/Index/IndexBody.cpp
+++ clang/lib/Index/IndexBody.cpp
@@ -286,9 +286,6 @@
   }
 
   bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
-if (E->isClassReceiver())
-  IndexCtx.handleReference(E->getClassReceiver(), E->getReceiverLocation(),
-   Parent, ParentDC);
 if (E->isExplicitProperty()) {
   SmallVector Relations;
   SymbolRoleSet Roles = getRolesForRef(E, Relations);
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2638,7 +2638,16 @@
 TRY_TO(TraverseTypeLoc(TInfo->getTypeLoc()));
 })
 
-DEF_TRAVERSE_STMT(ObjCPropertyRefExpr, {})
+DEF_TRAVERSE_STMT(ObjCPropertyRefExpr, {
+  if (S->isClassReceiver()) {
+ObjCInterfaceDecl *IDecl = S->getClassReceiver();
+QualType Type = IDecl->getASTContext().getObjCInterfaceType(IDecl);
+ObjCInterfaceLocInfo Data;
+Data.NameLoc = S->getReceiverLocation();
+Data.NameEndLoc = Data.NameLoc;
+TRY_TO(TraverseTypeLoc(TypeLoc(Type, &Data)));
+  }
+})
 DEF_TRAVERSE_STMT(ObjCSubscriptRefExpr, {})
 DEF_TRAVERSE_STMT(ObjCProtocolExpr, {})
 DEF_TRAVERSE_STMT(ObjCSelectorExpr, {})
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -981,8 +981,7 @@
   id value = [[Foo]].sharedInstance;
 }
   )cpp";
-  // FIXME: We currently can't identify t

[clang] 13a8aa3 - [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2021-06-01T14:45:25-04:00
New Revision: 13a8aa3ee15a67048deeb193fe8b86005fdf9d10

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

LOG: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

We now make up a TypeLoc for the class receiver to simplify visiting,
notably for indexing, availability, and clangd.

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang/include/clang/AST/RecursiveASTVisitor.h
clang/lib/Index/IndexBody.cpp
clang/lib/Sema/SemaAvailability.cpp
clang/test/Index/Core/index-source.m

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 4789d28ecf48c..c8eca4e7ca8c7 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -306,9 +306,6 @@ struct TargetFinder {
 Outer.add(OME->getMethodDecl(), Flags);
   }
   void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *OPRE) {
-// FIXME: We miss visiting the class receiver if one exists, which
-// means we skip the corresponding ObjCInterfaceDecl ref since it
-// doesn't have a corresponding node.
 if (OPRE->isExplicitProperty())
   Outer.add(OPRE->getExplicitProperty(), Flags);
 else {
@@ -766,13 +763,6 @@ llvm::SmallVector refInStmt(const Stmt *S,
 }
 
 void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) {
-  // There's no contained TypeLoc node for a class receiver type.
-  if (E->isClassReceiver()) {
-Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
-E->getReceiverLocation(),
-/*IsDecl=*/false,
-{E->getClassReceiver()}});
-  }
   Refs.push_back(ReferenceLoc{
   NestedNameSpecifierLoc(), E->getLocation(),
   /*IsDecl=*/false,

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index beca7c292583d..67dcb574d500a 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -981,8 +981,7 @@ TEST_F(TargetDeclTest, ObjC) {
   id value = [[Foo]].sharedInstance;
 }
   )cpp";
-  // FIXME: We currently can't identify the interface here.
-  EXPECT_DECLS("ObjCPropertyRefExpr", "+ (id)sharedInstance");
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface Foo");
 
   Code = R"cpp(
 @interface Foo

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index b3ea6f7817ef9..0623a5c8d5ea8 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2638,7 +2638,16 @@ DEF_TRAVERSE_STMT(ObjCMessageExpr, {
 TRY_TO(TraverseTypeLoc(TInfo->getTypeLoc()));
 })
 
-DEF_TRAVERSE_STMT(ObjCPropertyRefExpr, {})
+DEF_TRAVERSE_STMT(ObjCPropertyRefExpr, {
+  if (S->isClassReceiver()) {
+ObjCInterfaceDecl *IDecl = S->getClassReceiver();
+QualType Type = IDecl->getASTContext().getObjCInterfaceType(IDecl);
+ObjCInterfaceLocInfo Data;
+Data.NameLoc = S->getReceiverLocation();
+Data.NameEndLoc = Data.NameLoc;
+TRY_TO(TraverseTypeLoc(TypeLoc(Type, &Data)));
+  }
+})
 DEF_TRAVERSE_STMT(ObjCSubscriptRefExpr, {})
 DEF_TRAVERSE_STMT(ObjCProtocolExpr, {})
 DEF_TRAVERSE_STMT(ObjCSelectorExpr, {})

diff  --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp
index 7279e3b53c815..fa35f749d0284 100644
--- a/clang/lib/Index/IndexBody.cpp
+++ b/clang/lib/Index/IndexBody.cpp
@@ -286,9 +286,6 @@ class BodyIndexer : public RecursiveASTVisitor 
{
   }
 
   bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
-if (E->isClassReceiver())
-  IndexCtx.handleReference(E->getClassReceiver(), E->getReceiverLocation(),
-   Parent, ParentDC);
 if (E->isExplicitProperty()) {
   SmallVector Relations;
   SymbolRoleSet Roles = getRolesForRef(E, Relations);

diff  --git a/clang/lib/Sema/SemaAvailability.cpp 
b/clang/lib/Sema/SemaAvailability.cpp
index d69600aaf94ea..bb704b1066cf8 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -683,11 +683,7 @@ class DiagnoseUnguardedAvailability
   // to any useful diagnostics.
   bool TraverseCaseStmt(CaseStmt *CS) { return TraverseStmt(CS->getSubStmt()); 
}
 
-  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
-if (PRE->isClassReceiver())
-  DiagnoseDeclAvailability(PRE->getClassReceiver(), 
PRE->getRecei

[PATCH] D101192: Add support for #elifdef and #elifndef

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:594
+if (CondInfo.FoundElse)
+  Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 

bjope wrote:
> aaron.ballman wrote:
> > bjope wrote:
> > > Hi @aaron.ballman 
> > > 
> > > This change is missing from 
> > > https://reviews.llvm.org/rG8edd3464afbff65d7d5945b3a8b20009d6ff5deb , 
> > > while you instead got it when doing the pp_err_else_after_else diagnostic 
> > > a couple of lines above this.
> > > 
> > > It causes asserts (in DIagnostic::getArgKind) for me in test cases 
> > > verifying the "elif after else" scenario using a test case basically doing
> > > ```
> > > #if 1
> > > #else
> > > #elif
> > > #endif
> > > ```
> > > So maybe such a test case should be added as well?
> > > 
> > > But it seems like the patch you committed doesn't match with the reviewed 
> > > patch. So maybe you can look into this and fix it?
> > > (let me know if you need some additional help)
> > Great catch and thank you for the test case! It seems that when I applied 
> > the patch, it misapplied, but not in a way that caused a test failure. I 
> > have no idea how that happened, but I assume I screwed up something with 
> > git somewhere along the way and caused the failure.
> > 
> > I've pushed baa2b8d08502acfa91a8dfd699d25f7b4e25edbb to fix it. Thanks!
> Big thanks for the quick fix! While I managed to track down this problem I 
> didn't have time to properly fix it right away. Now I'll have one thing less 
> to worry about tomorrow :-)
Happy to fix my own messes!


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

https://reviews.llvm.org/D101192

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

[PATCH] D100276: [clang] p1099 3/5: using Enum::member

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan updated this revision to Diff 349044.
urnathan added a comment.

Rebase and fix conflict


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

https://reviews.llvm.org/D100276

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
  clang/test/SemaCXX/enum-scoped.cpp

Index: clang/test/SemaCXX/enum-scoped.cpp
===
--- clang/test/SemaCXX/enum-scoped.cpp
+++ clang/test/SemaCXX/enum-scoped.cpp
@@ -301,8 +301,8 @@
   int E::*p; // expected-error {{does not point into a class}}
   using E::f; // expected-error {{no member named 'f'}}
 
-  using E::a; // expected-error {{using declaration cannot refer to a scoped enumerator}}
-  E b = a; // expected-error {{undeclared}}
+  using E::a; // expected-warning {{using declaration naming a scoped enumerator is a C++20 extension}}
+  E b = a;
 }
 
 namespace test11 {
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
@@ -1,4 +1,11 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s
 
 enum class EC { ec };
-using EC::ec; // expected-error {{using declaration cannot refer to a scoped enumerator}}
+using EC::ec;
+#if __cplusplus < 202002
+// expected-warning@-2 {{using declaration naming a scoped enumerator is a C++20 extension}}
+#else
+// expected-no-diagnostics
+#endif
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+
+// p1099 'using SCOPEDENUM::MEMBER;'
+
+namespace Zero {
+namespace Bob {
+enum class Kevin {
+  Stuart,
+  AlsoStuart
+#if __cplusplus >= 202002L
+// expected-note@-3{{target of using declaration}}
+// expected-note@-3{{target of using declaration}}
+#endif
+};
+} // namespace Bob
+
+using Bob::Kevin::Stuart;
+#if __cplusplus < 202002L
+// expected-warning@-2{{using declaration naming a scoped enumerator is a C++20 extension}}
+#else
+using Bob::Kevin::Stuart;
+
+auto b = Stuart;
+
+namespace Foo {
+int Stuart;   // expected-note{{conflicting declaration}}
+using Bob::Kevin::Stuart; // expected-error{{target of using declaration conflicts}}
+
+using Bob::Kevin::AlsoStuart; // expected-note{{using declaration}}
+int AlsoStuart;   // expected-error{{declaration conflicts with target}}
+} // namespace Foo
+#endif
+
+} // namespace Zero
+
+namespace One {
+
+// derived from [namespace.udecl]/3
+enum class button { up,
+down };
+struct S {
+  using button::up;
+#if __cplusplus < 202002L
+  // expected-warning@-2{{a C++20 extension}}
+  // expected-error@-3{{using declaration in class}}
+#else
+  button b = up;
+#endif
+};
+
+#if __cplusplus >= 202002L
+// some more
+struct T : S {
+  button c = up;
+};
+#endif
+enum E2 { e2 };
+} // namespace One
+
+namespace Two {
+enum class E1 { e1 };
+
+struct S {
+  using One::e2;
+#if __cplusplus < 202002L
+  // expected-error@-2{{using declaration in class}}
+#else
+  One::E2 c = e2;
+#endif
+};
+
+} // namespace Two
+
+namespace Three {
+
+enum E3 { e3 };
+struct e3;
+
+struct S {
+  using Three::e3; // expected-error{{using declaration in class}}
+
+  enum class E4 { e4 };
+  enum E5 { e5 };
+};
+
+using S::e5;
+using S::E4::e4;
+#if __cplusplus < 202002L
+// expected-error@-3{{using declaration cannot refer to class member}}
+// expected-note@-4{{use a constexpr variable instead}}
+// expected-warning@-4{{a C++20 extension}}
+// expected-error@-5{{using declaration cannot refer to class member}}
+// expected-note@-6{{use a constexpr variable instead}}
+#else
+auto a = e4;
+auto b = e5;
+#endif
+} // namespace Three
+
+namespace Four {
+
+template 
+struct TPL {
+  enum class E1 { e1 };
+  struct IN {
+enum class E2 { e2 };
+  };
+
+protected:
+  enum class E3 { e3 }; // expected-note{{declared protected here}}
+};
+
+using TPL::E1::e1;
+#if __cplusplus < 202002L
+// expected-warning@-2{{a C++20 extension}}
+// expected-error@-3{{using declaration cannot refer to class member}}
+// expected-note@-4{{use a constexpr variable instead}}
+#else
+using TPL::IN::E2::e2;
+
+auto a = e1;
+auto b = e2;
+#endif
+
+enum class E4 { e4 };
+template 
+struct DER : TPL {
+  using TPL::E1::e1;

[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan updated this revision to Diff 349045.
urnathan added a comment.

rebase and fix conflict


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

https://reviews.llvm.org/D102241

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Index/IndexSymbol.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Index/IndexSymbol.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-using-enum.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1428,6 +1428,22 @@
   usingDecl(hasAnyUsingShadowDecl(hasName("a");
 }
 
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesUsingEnumDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("namespace X { enum x {}; } using enum X::x;", usingEnumDecl()));
+}
+
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesShadowUsingDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("namespace f { enum a {b}; } using enum f::a;",
+  usingEnumDecl(hasAnyUsingShadowDecl(hasName("b");
+}
+
 TEST_P(ASTMatchersTest, UsingDirectiveDecl_MatchesUsingNamespace) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -844,7 +844,15 @@
   testImport("namespace foo { int bar; }"
  "void declToImport() { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- functionDecl(hasDescendant(usingDecl(;
+ functionDecl(hasDescendant(usingDecl(hasName("bar");
+}
+
+TEST_P(ImportDecl, ImportUsingEnumDecl) {
+  MatchVerifier Verifier;
+  testImport("namespace foo { enum bar { baz, toto, quux }; }"
+ "void declToImport() { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ functionDecl(hasDescendant(usingEnumDecl(hasName("bar");
 }
 
 /// \brief Matches shadow declarations introduced into a scope by a
@@ -862,10 +870,16 @@
 
 TEST_P(ImportDecl, ImportUsingShadowDecl) {
   MatchVerifier Verifier;
+  // from using-decl
   testImport("namespace foo { int bar; }"
  "namespace declToImport { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- namespaceDecl(has(usingShadowDecl(;
+ namespaceDecl(has(usingShadowDecl(hasName("bar");
+  // from using-enum-decl
+  testImport("namespace foo { enum bar {baz, toto, quux }; }"
+ "namespace declToImport { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ namespaceDecl(has(usingShadowDecl(hasName("baz");
 }
 
 TEST_P(ImportExpr, ImportUnresolvedLookupExpr) {
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6540,6 +6540,7 @@
   }
 
   case Decl::Using:
+  case Decl::UsingEnum:
 return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
TU);
 
Index: clang/test/SemaCXX/cxx20-using-enum.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-using-enum.cpp

[PATCH] D101630: [HIP] Fix device-only compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D101630#2787714 , @yaxunl wrote:

> How does nvcc --genco behave when there are multiple GPU arch's? Does it 
> output a fat binary containing multiple ISA's? Also, does it support 
> device-only compilation for intermediate outputs?

It does not allow multiple outputs for `-ptx` and `-cubin` compilations, same 
as clang behaves now:

  ~/local/cuda-11.3/bin/nvcc -gencode=arch=compute_60,code=sm_60 
-gencode=arch=compute_70,code=sm_70 -ptx foo.cu
  nvcc fatal   : Option '--ptx (-ptx)' is not allowed when compiling for 
multiple GPU architectures

NVCC does allow `-E` with multiple targets, but it does produce output for only 
*one* of them.

NVCC does bundle outputs for multiple GPU variants if `-cubin` is used.


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

https://reviews.llvm.org/D101630

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

[PATCH] D102923: [clang][lex] Remark for used header search paths

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

dexonsmith added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:427
+def remark_pp_include_header_search_usage : Remark<
+  "user-provided search path used: '%0'">,
+  InGroup>;

jansvoboda11 wrote:
> dexonsmith wrote:
> > I suggest, more simply, "search path used:" (drop the "user-provided"). If 
> > you think it's useful for information purposes to know whether it was a 
> > user or system search path, I'd suggest using a select (in that case, maybe 
> > you want to add an optional "framework" descriptor, and/or "header map" 
> > when applicable, etc.).
> I chose this spelling to distinguish user-provided vs default include paths. 
> The default ones are not important for header search pruning in dependency 
> scanner, as they always get generated in 
> `InitHeaderSearch::AddDefaultIncludePaths`.
Ah, I thought that logic was all moved to the driver. Looking at 
AddDefaultIncludePaths, you're right that only some things have moved, and some 
haven't (yet)...

For example, it looks like the only logic remaining in `-cc1` on Darwin is:
```
  // All header search logic is handled in the Driver for Darwin.
  if (triple.isOSDarwin()) {
if (HSOpts.UseStandardSystemIncludes) {
  // Add the default framework include paths on Darwin.
  AddPath("/System/Library/Frameworks", System, true);
  AddPath("/Library/Frameworks", System, true);
}
return;
  }
```
(We should probably clean that up and move it to the driver as well...)

I think `-cc1` can be agnostic about the source of the search path (user, IDE, 
driver, etc.).



Comment at: clang/include/clang/Lex/HeaderSearch.h:164-165
 
+  /// Mapping from SearchDir to HeaderSearchOptions::Entry indices.
+  llvm::DenseMap SearchDirToHSEntry;
+

jansvoboda11 wrote:
> dexonsmith wrote:
> > I think this can just be a vector of `unsigned`, since the key is densely 
> > packed and counting from 0. You can use `~0U` for a sentinel for the 
> > non-entries. Maybe it's not too important though.
> I'm not sure what's our approach on early micro-optimizations like this. I 
> don't think it will have measurable impact on memory or runtime.
Heh, maybe with the principle that the small things add up (or maybe just out 
of habit), I probably tend too much toward making small things efficient when 
it's easy. I also personally find `Vector` just as natural a map data 
structure as `DenseMap`, but with more obvious allocation / etc. But yeah, 
probably not important.



Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:581-583
+// Check whether this DirectoryLookup maps to a HeaderSearch::UserEntry.
+if (Infos[I].UserEntryIdx)
+  LookupsToUserEntries.insert({I, *Infos[I].UserEntryIdx});

jansvoboda11 wrote:
> dexonsmith wrote:
> > I don't see why we'd want to filter out system includes.
> > - Users sometimes manually configure "system" search paths, and this remark 
> > is fairly special-purpose, so I'm not sure the distinction is interesting 
> > to preserve.
> > - This remark is already going to be "noisy" and hit a few times on 
> > essentially every compilation. Adding the system paths that get hit won't 
> > change that much.
> > - The motivation for the patch is to test the logic for detecting which 
> > search paths are used in isolation from the 
> > canonicalize-explicit-module-build-commands logic in clang-scan-deps. We 
> > need to know that the logic works for system search paths as well.
> I'm not sure what you mean by system includes. 
> `HeaderSearchOptions::UserEntries` should contain all search paths provided 
> through the command-line including `-isystem` and `-isysroot`: 
> https://github.com/llvm/llvm-project/blob/97d234935f1514af128277943f30efc469525371/clang/lib/Frontend/CompilerInvocation.cpp#L2985-L3056.
> 
> "System header prefixes" only control whether a header found through 
> `DirectoryLookup` should be marked as system.
Thanks for explaining; you're right, I was confused. (Maybe this should be 
renamed from UserEntries to CommandLineEntries if it has everything on the 
command-line?)

As I mentioned above, the distinction between driver-generated and 
`-cc1`-generated options is a bit murky (and moving in the direction of 
removing the latter), so I'm not sure it's meaningful to filter out 
`-cc1`-generated options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102923

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