[PATCH] D118879: [clang-format] Avoid merging macro definitions.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 405539.
curdeius added a comment.

Force CI after changing parent revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118879

Files:
  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
@@ -1805,6 +1805,22 @@
   verifyFormat("#define xor(x) (^(x))");
   verifyFormat("#define __except(x)");
   verifyFormat("#define __try(x)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  // Test that a macro definition gets never merged with the following
+  // definition.
+  // FIXME: The AAA macro definition should probably not be splitted into 3
+  // lines.
+  verifyFormat("#define AAA
"
+   "\\\n"
+   "  N
"
+   "\\\n"
+   "  {\n"
+   "#define BBB }\n",
+   Style);
+  // verifyFormat("#define AAA N { //\n", Style);
 }
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -621,6 +621,10 @@
   tryMergeSimpleBlock(SmallVectorImpl::const_iterator I,
   SmallVectorImpl::const_iterator E,
   unsigned Limit) {
+// Don't merge with a preprocessor directive.
+if (I[1]->Type == LT_PreprocessorDirective)
+  return 0;
+
 AnnotatedLine &Line = **I;
 
 // Don't merge ObjC @ keywords and methods.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1805,6 +1805,22 @@
   verifyFormat("#define xor(x) (^(x))");
   verifyFormat("#define __except(x)");
   verifyFormat("#define __try(x)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  // Test that a macro definition gets never merged with the following
+  // definition.
+  // FIXME: The AAA macro definition should probably not be splitted into 3
+  // lines.
+  verifyFormat("#define AAA"
+   "\\\n"
+   "  N"
+   "\\\n"
+   "  {\n"
+   "#define BBB }\n",
+   Style);
+  // verifyFormat("#define AAA N { //\n", Style);
 }
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -621,6 +621,10 @@
   tryMergeSimpleBlock(SmallVectorImpl::const_iterator I,
   SmallVectorImpl::const_iterator E,
   unsigned Limit) {
+// Don't merge with a preprocessor directive.
+if (I[1]->Type == LT_PreprocessorDirective)
+  return 0;
+
 AnnotatedLine &Line = **I;
 
 // Don't merge ObjC @ keywords and methods.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118104: Make run-clang-tidy.py print the configured checks correctly

[Jesko Appelfeller via Phabricator via cfe-commits]

JesApp added a comment.

Well, since this was more of source of confusion than actual incorrect 
behaviour, I don't think there should be a test for it.

In general though, I think the script is complex enough to warrant some 
testing. That being said: I don't think they should be part of this patch. 
Also, I'm doing this on company time and I don't think my boss would be to 
happy if I wrote a testsuite from scratch when I just wanted to fix one bug. :D


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

https://reviews.llvm.org/D118104

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

[clang] bb1b53d - [clang-format] Remove unnecessary non-null check and assert instead. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-03T09:50:36+01:00
New Revision: bb1b53da6eeb90d3c101719f569abce1d689a959

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

LOG: [clang-format] Remove unnecessary non-null check and assert instead. NFC.

After a non-eof token, there is at least an eof token.

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 8d06277caba37..cdc2740ba9642 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -514,9 +514,10 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, 
IfStmtKind *IfKind) {
   FormatToken *Next;
   do {
 Next = Tokens->getNextToken();
+assert(Next);
   } while (Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
-  if (Next && Next->isNot(tok::colon)) {
+  if (Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
 // an identifier.
 parseStructuralElement();



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

[clang] 7cc3e02 - [clang-format] Use back() instead of rbegin(). NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-03T10:07:02+01:00
New Revision: 7cc3e0204210a8c9c12f29fddbfec9dfe786e931

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

LOG: [clang-format] Use back() instead of rbegin(). NFC.

Added: 


Modified: 
clang/lib/Format/TokenAnalyzer.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnalyzer.cpp 
b/clang/lib/Format/TokenAnalyzer.cpp
index 2bd5a1fd6230..348da1f03f29 100644
--- a/clang/lib/Format/TokenAnalyzer.cpp
+++ b/clang/lib/Format/TokenAnalyzer.cpp
@@ -110,7 +110,7 @@ std::pair 
TokenAnalyzer::process() {
   UnwrappedLineParser Parser(Style, Lex.getKeywords(),
  Env.getFirstStartColumn(), Tokens, *this);
   Parser.parse();
-  assert(UnwrappedLines.rbegin()->empty());
+  assert(UnwrappedLines.back().empty());
   unsigned Penalty = 0;
   for (unsigned Run = 0, RunE = UnwrappedLines.size(); Run + 1 != RunE; ++Run) 
{
 const auto &Lines = UnwrappedLines[Run];



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

[clang] 564f9be - Remove -Wweak-template-vtables

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2022-02-03T10:15:16+01:00
New Revision: 564f9be11c9cb8d131f48df07538fab7a19b41a7

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

LOG: Remove -Wweak-template-vtables

as it was planned for removal in clang 15 and we're now past the branch point

See https://github.com/llvm/llvm-project/issues/19107

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/SemaCXX/warn-weak-vtables.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fa8860ba2027f..dafa261e1fdae 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@ Modified Compiler Flags
 Removed Compiler Flags
 -
 
+- -Wweak-template-vtables, which was deprecated in the previous release and no
+  longer had any effect, has been removed.
+
 New Pragmas in Clang
 
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a8cf00c1263ff..46316bd5d6b2b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1651,9 +1651,6 @@ def warn_weak_vtable : Warning<
   "%0 has no out-of-line virtual method definitions; its vtable will be "
   "emitted in every translation unit">,
   InGroup>, DefaultIgnore;
-def warn_weak_template_vtable : Warning<
-  "this warning is no longer in use and will be removed in the next release">,
-  InGroup>, DefaultIgnore;
 
 def ext_using_undefined_std : ExtWarn<
   "using directive refers to implicitly-defined namespace 'std'">;

diff  --git a/clang/test/SemaCXX/warn-weak-vtables.cpp 
b/clang/test/SemaCXX/warn-weak-vtables.cpp
index 083209fa5e315..9355af50310d4 100644
--- a/clang/test/SemaCXX/warn-weak-vtables.cpp
+++ b/clang/test/SemaCXX/warn-weak-vtables.cpp
@@ -3,9 +3,6 @@
 // Check that this warning is disabled on MS ABI targets which don't have key
 // functions.
 // RUN: %clang_cc1 %s -fsyntax-only -triple %ms_abi_triple -Werror 
-Wweak-vtables
-//
-// -Wweak-template-vtables is deprecated but we still parse it.
-// RUN: %clang_cc1 %s -fsyntax-only -Werror -Wweak-template-vtables
 
 struct A { // expected-warning {{'A' has no out-of-line virtual method 
definitions; its vtable will be emitted in every translation unit}}
   virtual void f() { } 



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

[PATCH] D118762: Remove -Wweak-template-vtables

[Hans Wennborg 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 rG564f9be11c9c: Remove -Wweak-template-vtables (authored by 
hans).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118762

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/warn-weak-vtables.cpp


Index: clang/test/SemaCXX/warn-weak-vtables.cpp
===
--- clang/test/SemaCXX/warn-weak-vtables.cpp
+++ clang/test/SemaCXX/warn-weak-vtables.cpp
@@ -3,9 +3,6 @@
 // Check that this warning is disabled on MS ABI targets which don't have key
 // functions.
 // RUN: %clang_cc1 %s -fsyntax-only -triple %ms_abi_triple -Werror 
-Wweak-vtables
-//
-// -Wweak-template-vtables is deprecated but we still parse it.
-// RUN: %clang_cc1 %s -fsyntax-only -Werror -Wweak-template-vtables
 
 struct A { // expected-warning {{'A' has no out-of-line virtual method 
definitions; its vtable will be emitted in every translation unit}}
   virtual void f() { } 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1651,9 +1651,6 @@
   "%0 has no out-of-line virtual method definitions; its vtable will be "
   "emitted in every translation unit">,
   InGroup>, DefaultIgnore;
-def warn_weak_template_vtable : Warning<
-  "this warning is no longer in use and will be removed in the next release">,
-  InGroup>, DefaultIgnore;
 
 def ext_using_undefined_std : ExtWarn<
   "using directive refers to implicitly-defined namespace 'std'">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
 Removed Compiler Flags
 -
 
+- -Wweak-template-vtables, which was deprecated in the previous release and no
+  longer had any effect, has been removed.
+
 New Pragmas in Clang
 
 


Index: clang/test/SemaCXX/warn-weak-vtables.cpp
===
--- clang/test/SemaCXX/warn-weak-vtables.cpp
+++ clang/test/SemaCXX/warn-weak-vtables.cpp
@@ -3,9 +3,6 @@
 // Check that this warning is disabled on MS ABI targets which don't have key
 // functions.
 // RUN: %clang_cc1 %s -fsyntax-only -triple %ms_abi_triple -Werror -Wweak-vtables
-//
-// -Wweak-template-vtables is deprecated but we still parse it.
-// RUN: %clang_cc1 %s -fsyntax-only -Werror -Wweak-template-vtables
 
 struct A { // expected-warning {{'A' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit}}
   virtual void f() { } 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1651,9 +1651,6 @@
   "%0 has no out-of-line virtual method definitions; its vtable will be "
   "emitted in every translation unit">,
   InGroup>, DefaultIgnore;
-def warn_weak_template_vtable : Warning<
-  "this warning is no longer in use and will be removed in the next release">,
-  InGroup>, DefaultIgnore;
 
 def ext_using_undefined_std : ExtWarn<
   "using directive refers to implicitly-defined namespace 'std'">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
 Removed Compiler Flags
 -
 
+- -Wweak-template-vtables, which was deprecated in the previous release and no
+  longer had any effect, has been removed.
+
 New Pragmas in Clang
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118880: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

[Kristóf Umann via Phabricator via cfe-commits]

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

The problem with leak bug reports is that the most interesting event in the 
code is likely the one that did //not// happen -- lack of ownership change and 
lack of deallocation, which is often present within the same function that the 
analyzer inlined anyway, but not on the path of execution on which the bug 
occured. We struggle to understand that a function was responsible for freeing 
the memory, but failed.

D105819  added a new visitor to improve 
memory leak bug reports. In addition to inspecting the ExplodedNodes of the bug 
pat, the visitor tries to guess whether the function was supposed to free 
memory, but failed to. Initially (in D108753 
), this was done by checking whether a 
`CXXDeleteExpr` is present in the function. If so, we assume that the function 
was at least party responsible, and prevent the analyzer from pruning bug 
report notes in it. This patch improves this heuristic by recognizing all 
deallocator functions that MallocChecker itself recognizes, by reusing 
`MallocChecker::isFreeingCall`.

However, we are only able to match a `CallEvent` against a `CallDescription`. 
`CallEvent`s are created during symbolic execution, providing a lot of 
additional information about the call, but the grand idea behind this visitor 
that it checks code that was //not// executed smybolically (well, not on the 
path of execution on which the bug was found). For this reason, I added a set 
of functions to allow matching a `CallExpr` against a `CallDescription`.

I am aware that this idea may induce strong opinions -- after all, I'm adding a 
a new interface called `.*Imprecise`, discourage people from using it in the 
comments, while we already have one too many confusing interfaces in the 
analyzer, not to mention that `CallDescription` was meant to be among the 
better documented, newcomer friendlier and more intuitive tools we have. Also, 
its possible that matching function calls in syntactic-only cases should be 
done by `StdLibraryFunctionChecker::Signature` instead.

I argue for this patch because

- A lot of checkers use `CallDescriptionMap` already, and we either never 
intend to change to `Signature`, or it would be an enormous effort upfront
- Leak checkers like `StreamChecker` could benefit from this as well
- The comments I believe resolve the unintuitiveness of the new interface well
- In the case of this patch, matching `CallEvent`s for plain C function calls 
is pretty much matching against a `CallExpr` anyways.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118880

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallDescription.cpp
  clang/test/Analysis/NewDeleteLeaks.cpp

Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -35,7 +35,9 @@
 
 } // namespace memory_allocated_in_fn_call
 
-namespace memory_passed_to_fn_call {
+// Realize that sink() intends to deallocate memory, assume that it should've
+// taken care of the leaked object as well.
+namespace memory_passed_to_fn_call_delete {
 
 void sink(int *P) {
   if (coin()) // ownership-note {{Assuming the condition is false}}
@@ -50,7 +52,24 @@
 } // expected-warning {{Potential leak of memory pointed to by 'ptr' [cplusplus.NewDeleteLeaks]}}
 // expected-note@-1 {{Potential leak}}
 
-} // namespace memory_passed_to_fn_call
+} // namespace memory_passed_to_fn_call_delete
+
+namespace memory_passed_to_fn_call_free {
+
+void sink(int *P) {
+  if (coin()) // ownership-note {{Assuming the condition is false}}
+  // ownership-note@-1 {{Taking false branch}}
+free(P);
+} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+
+void foo() {
+  int *ptr = (int *)malloc(sizeof(int)); // expected-note {{Memory is allocated}}
+  sink(ptr); // ownership-note {{Calling 'sink'}}
+ // ownership-note@-1 {{Returning from 'sink'}}
+} // expected-warning {{Potential leak of memory pointed to by 'ptr' [unix.Malloc]}}
+// expected-note@-1 {{Potential leak}}
+
+} // namespace memory_passed_to_fn_call_free
 
 namespace memory_shared_with_ptr_of_shorter_lifetime {
 
@@ -123,6 +142,24 @@
 
 } // namespace memory_passed_into_fn_that_doesnt_intend_to_free
 
+namespace memory_passed_into_fn_that_do

[PATCH] D118428: [clang-cl] Support the /JMC flag

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118428

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

[clang] 768a619 - [clang-format] Reserve vectors when the number of items is known beforehand. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-03T10:38:23+01:00
New Revision: 768a6192dfc680c0c941e713c824b9046429538d

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

LOG: [clang-format] Reserve vectors when the number of items is known 
beforehand. NFC.

Added: 


Modified: 
clang/lib/Format/FormatToken.cpp
clang/lib/Format/TokenAnalyzer.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.cpp 
b/clang/lib/Format/FormatToken.cpp
index 59d6f29bb54d2..40aa8f5cacb25 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -186,6 +186,9 @@ void CommaSeparatedList::precomputeFormattingInfos(const 
FormatToken *Token) {
   // The lengths of an item if it is put at the end of the line. This includes
   // trailing comments which are otherwise ignored for column alignment.
   SmallVector EndOfLineItemLength;
+  MustBreakBeforeItem.reserve(Commas.size() + 1);
+  EndOfLineItemLength.reserve(Commas.size() + 1);
+  ItemLengths.reserve(Commas.size() + 1);
 
   bool HasSeparatingComment = false;
   for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) {

diff  --git a/clang/lib/Format/TokenAnalyzer.cpp 
b/clang/lib/Format/TokenAnalyzer.cpp
index 348da1f03f299..0a775c0a87eda 100644
--- a/clang/lib/Format/TokenAnalyzer.cpp
+++ b/clang/lib/Format/TokenAnalyzer.cpp
@@ -116,6 +116,7 @@ std::pair 
TokenAnalyzer::process() {
 const auto &Lines = UnwrappedLines[Run];
 LLVM_DEBUG(llvm::dbgs() << "Run " << Run << "...\n");
 SmallVector AnnotatedLines;
+AnnotatedLines.reserve(Lines.size());
 
 TokenAnnotator Annotator(Style, Lex.getKeywords());
 for (const UnwrappedLine &Line : Lines) {



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

[PATCH] D118428: [clang-cl] Support the /JMC flag

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

It seems Phabricator ate my comment, but I meant to say:

My understanding is still that IR passes generally live in Transforms/ and that 
CodeGen/ deals with lower levels such as MachineInstrs, SelectionDAG, etc. with 
CodegenPrepare as the big exception.

Thinking about it another way, what about this pass makes it a "codegen IR" 
pass as opposed to all the other passes that add instrumentation to the IR and 
live in Transforms/Instrumentation/ ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118428

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

[PATCH] D118873: [clang-format] Revert a feature in RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

Ok, so now (until a new fix), it will always remove the braces for a single 
statement even if it's multi-line. That's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118873

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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 405550.
hokein marked 12 inline comments as done.
hokein added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
@@ -0,0 +1,102 @@
+//===--- GrammarTests.cpp - grammar tests  --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Pseudo/Grammar.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+namespace {
+
+using testing::AllOf;
+using testing::ElementsAre;
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(TargetID, SID, "") { return arg.Target == SID; }
+template  testing::Matcher Sequence(T... IDs) {
+  return testing::Property(&Rule::seq, ElementsAre(IDs...));
+}
+
+class GrammarTest : public ::testing::Test {
+public:
+  void build(llvm::StringRef BNF) {
+Diags.clear();
+G = Grammar::parseBNF(BNF, Diags);
+  }
+
+  SymbolID lookup(llvm::StringRef Name) const {
+for (unsigned I = 0; I < NumTerminals; ++I)
+  if (G->table().Terminals[I] == Name)
+return tokenSymbol(static_cast(I));
+for (SymbolID ID = 0; ID < G->table().Nonterminals.size(); ++ID)
+  if (G->table().Nonterminals[ID].Name == Name)
+return ID;
+ADD_FAILURE() << "No such symbol found: " << Name;
+return 0;
+  }
+
+protected:
+  std::unique_ptr G;
+  std::vector Diags;
+};
+
+TEST_F(GrammarTest, Basic) {
+  build("expression := IDENTIFIER + expression # comment");
+  EXPECT_THAT(Diags, IsEmpty());
+
+  auto ExpectedRule =
+  AllOf(TargetID(lookup("expression")),
+Sequence(lookup("IDENTIFIER"), lookup("+"), lookup("expression")));
+  auto ExpressionID = lookup("expression");
+  EXPECT_EQ(G->symbolName(ExpressionID), "expression");
+  EXPECT_THAT(G->rulesFor(ExpressionID), UnorderedElementsAre(ExpectedRule));
+  const auto &Rule = G->lookupRule(/*RID=*/0);
+  EXPECT_THAT(Rule, ExpectedRule);
+  EXPECT_THAT(G->symbolName(Rule.seq()[0]), "IDENTIFIER");
+  EXPECT_THAT(G->symbolName(Rule.seq()[1]), "+");
+  EXPECT_THAT(G->symbolName(Rule.seq()[2]), "expression");
+}
+
+TEST_F(GrammarTest, EliminatedOptional) {
+  build("_ := CONST_opt INT ;_opt");
+  EXPECT_THAT(Diags, IsEmpty());
+  EXPECT_THAT(G->table().Rules,
+  UnorderedElementsAre(
+  Sequence(lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT"), lookup(";")),
+  Sequence(lookup("INT"), lookup(";";
+}
+
+TEST_F(GrammarTest, Diagnostics) {
+  build(R"cpp(
+_ := ,_opt
+_ := undefined-sym
+null :=
+_ := IDENFIFIE # a typo of the terminal IDENFITIER
+
+invalid
+  )cpp");
+
+  EXPECT_THAT(Diags, UnorderedElementsAre(
+ "Rule '_ := ,_opt' has a nullable RHS",
+ "Rule 'null := ' has a nullable RHS",
+ "No rules for nonterminal: undefined-sym",
+ "Failed to parse 'invalid': no separator :=",
+ "Token-like name IDENFIFIE is used as a nonterminal",
+ "No rules for nonterminal: IDENFIFIE"));
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace syntax
+} // namespace clang
Index: clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
@@ -0,0 +1,20 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_unittest(ClangPseudoTests
+  GrammarTests.cpp
+)
+
+clang_target_link_libraries(ClangPseudoTests
+  PRIVATE
+  clangBasic
+  clangLex
+  clangSyntaxPseudo
+  clangTesting
+  )
+
+target_link_libraries(ClangPseudoTests
+  PRIVATE
+  LLVMTestingSupport
+  )
Index: clang/unittests/Tooling/Syntax/CMakeLists.txt
===
--- clang/unittests/Tooling/Syntax/CMakeLists.txt
+++ clang/unittests/Tooling

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:85
+  static constexpr unsigned SizeBits = 4;
+  static_assert(SizeBits + SymbolBits <= 16,
+"Must be able to store symbol ID + size efficiently");

sammccall wrote:
> We're being quite fiddly with layout but actually pretty wasteful.
> 
> wc says 652 with 1323 total RHS elements on the RHS - average ~2.
> So we could aim for 6 bytes per rule = 4KB but we're using 34 bytes per rule 
> = 22KB.
> 
> Can't see a nice concrete way to achieve that though.
Yes :(
I'd keep it as-is (and decrease the MaxElements to 9). One idea is to use a 
flat array as a center storage of all sequences, then Rule class just need an 
index, and size. But there will be more data (annotations, hooks) in this 
class, we could figure it out later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D118792: [lld][clang][cmake] Clean up a few things

[Nikita Popov via Phabricator via cfe-commits]

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

Not very familiar with cmake, but this looks reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118792

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

[clang] d15e7dd - [clang][Hexagon] Match -lc option more specifically in toolchain test

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2022-02-03T10:08:20Z
New Revision: d15e7dd1238df20e9c09cc91930f716e0d3d5b05

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

LOG: [clang][Hexagon] Match -lc option more specifically in toolchain test

In https://lab.llvm.org/buildbot/#/builders/185/builds/1341
our bot happened to generate a temporary file path with -lc in
it.

Match with "" around the options so this doesn't happen again.

Added: 


Modified: 
clang/test/Driver/hexagon-toolchain-linux.c

Removed: 




diff  --git a/clang/test/Driver/hexagon-toolchain-linux.c 
b/clang/test/Driver/hexagon-toolchain-linux.c
index da59590371b90..05ae1733992d9 100644
--- a/clang/test/Driver/hexagon-toolchain-linux.c
+++ b/clang/test/Driver/hexagon-toolchain-linux.c
@@ -39,8 +39,8 @@
 // CHECK002:   
"-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
 // CHECK002-NOT:   
{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crti.o
 // CHECK002-NOT:   
{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o
-// CHECK002-NOT:   -lclang_rt.builtins-hexagon
-// CHECK002-NOT:   -lc
+// CHECK002-NOT:   "-lclang_rt.builtins-hexagon"
+// CHECK002-NOT:   "-lc"
 // 
-
 // Passing --musl -nostartfiles
 // 
-
@@ -65,8 +65,8 @@
 // RUN:   | FileCheck -check-prefix=CHECK004 %s
 // CHECK004:   
"-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
 // CHECK004:   
"{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o"
-// CHECK004-NOT:   -lclang_rt.builtins-hexagon
-// CHECK004-NOT:   -lc
+// CHECK004-NOT:   "-lclang_rt.builtins-hexagon"
+// CHECK004-NOT:   "-lc"
 // 
-
 // Not Passing -fno-use-init-array when musl is selected
 // 
-



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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:85
+  static constexpr unsigned SizeBits = 4;
+  static_assert(SizeBits + SymbolBits <= 16,
+"Must be able to store symbol ID + size efficiently");

hokein wrote:
> sammccall wrote:
> > We're being quite fiddly with layout but actually pretty wasteful.
> > 
> > wc says 652 with 1323 total RHS elements on the RHS - average ~2.
> > So we could aim for 6 bytes per rule = 4KB but we're using 34 bytes per 
> > rule = 22KB.
> > 
> > Can't see a nice concrete way to achieve that though.
> Yes :(
> I'd keep it as-is (and decrease the MaxElements to 9). One idea is to use a 
> flat array as a center storage of all sequences, then Rule class just need an 
> index, and size. But there will be more data (annotations, hooks) in this 
> class, we could figure it out later.
> Yes :(
> I'd keep it as-is (and decrease the MaxElements to 9).
SGTM

> One idea is to use a flat array as a center storage of all sequences, then 
> Rule class just need an index, and size.

This makes APIs awkward though: Rule.seq() needs extra params to build an 
ArrayRef, unless you want to carry around a pointer to the big array (which 
gives back most of the size savings)



Comment at: clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp:140
+for (llvm::StringRef Line : llvm::split(Lines, '\n')) {
+  Line = Line.trim();
+  // Strip anything coming after the '#' (comment).

move this after the take_while, so that `   # this is a comment` is still 
skipped?



Comment at: clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp:153
+
+  bool ParseLine(llvm::StringRef Line, RuleSpec &Out) {
+auto Parts = Line.split(":=");

ParseLine -> parseLine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D118700: Add support to --gcc-toolchain flag for GCC compiled with --enable-version-specific-runtime-libs.

[Raúl Peñacoba via Phabricator via cfe-commits]

rpenacob added a comment.

That is an interesting case. It looks like we're hitting 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32415 , so gcc doesn't seem to 
search for its own libgcc_s.so when configured with 
--enable-version-specific-runtime-libs

Most users don't notice this issue because a system-wide libgcc_s.so is usually 
found instead.

The gcc bug manifests like this:

With --enable-version-specific-runtime-libs
$ /bin/g++  -print-file-name=libgcc_s.so
/lib/../lib64/libgcc_s.so

Without --enable-version-specific-runtime-libs
$ /bin/g++  -print-file-name=libgcc_s.so
/bin/../lib/gcc/x86_64-pc-linux-gnu/11.2.0/../../../../lib64/libgcc_s.so

However clang doesn't get to find the system wide library, so this is why we 
need an extra library path.


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

https://reviews.llvm.org/D118700

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

[clang] 20e05b9 - [syntax][pseudo] Add Grammar for the clang pseudo-parser

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-02-03T11:28:27+01:00
New Revision: 20e05b9f0ebea35076b96c89257becd35d6de859

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

LOG: [syntax][pseudo] Add Grammar for the clang pseudo-parser

This patch introduces the Grammar class, which is a critial piece for 
constructing
a tabled-based parser.

As the first patch, the scope is limited to:
  - define base types (symbol, rules) of modeling the grammar
  - construct Grammar by parsing the BNF file (annotations are excluded for now)

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

Added: 
clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp

Modified: 
clang/lib/Tooling/Syntax/CMakeLists.txt
clang/unittests/Tooling/Syntax/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h 
b/clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
new file mode 100644
index 0..80db9f268ff13
--- /dev/null
+++ b/clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
@@ -0,0 +1,170 @@
+//===--- Grammar.h - grammar used by clang pseudo parser  *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines base structures for parsing & modeling a grammar for a
+//  programming language:
+//
+//# This is a fake C++ BNF grammar
+//_ := translation-unit
+//translation-unit := declaration-seq_opt
+//declaration-seq := declaration
+//declaration-seq := declaration-seq declaration
+//
+//  A grammar formally describes a language, and it is constructed by a set of
+//  production rules. A rule is of BNF form (AAA := BBB CCC). A symbol is 
either
+//  non-terminal or terminal, identified by a SymbolID.
+//
+//  Notions about the BNF grammar:
+//  - "_" is the augmented symbol, formed by start symbols.
+//  - single-line comment is supported, starting with a #
+//  - A rule describes how a nonterminal (left side of :=) is constructed, and
+//it is *per line* in the grammar file
+//  - Terminals (also called tokens) correspond to the clang::TokenKind; they
+//are written in the grammar like "IDENTIFIER", "USING", "+"
+//  - Nonterminals are specified with "lower-case" names in the grammar; they
+//shouldn't be nullable (has an empty sequence)
+//  - optional symbols are supported (specified with a _opt suffix), and they
+//will be eliminated during the grammar parsing stage
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_SYNTAX_GRAMMAR_H
+#define LLVM_CLANG_TOOLING_SYNTAX_GRAMMAR_H
+
+#include "clang/Basic/TokenKinds.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+// A SymbolID uniquely identifies a terminal/non-terminal symbol in a grammar.
+// Non-terminal IDs are indexes into a table of non-terminal symbols.
+// Terminal IDs correspond to the clang TokenKind enum.
+using SymbolID = uint16_t;
+// SymbolID is only 12 bits wide.
+// There are maximum 2^11 terminals (aka tokens) and 2^11 nonterminals.
+static constexpr uint16_t SymbolBits = 12;
+static constexpr uint16_t NumTerminals = tok::NUM_TOKENS;
+// SymbolIDs with the top bit set are tokens/terminals.
+static constexpr SymbolID TokenFlag = 1 << (SymbolBits - 1);
+inline bool isToken(SymbolID ID) { return ID & TokenFlag; }
+inline bool isNonterminal(SymbolID ID) { return !isToken(ID); }
+// The terminals are always the clang tok::TokenKind (not all are used).
+inline tok::TokenKind symbolToToken(SymbolID SID) {
+  assert(isToken(SID));
+  SID &= ~TokenFlag;
+  assert(SID < NumTerminals);
+  return static_cast(SID);
+}
+inline SymbolID tokenSymbol(tok::TokenKind TK) {
+  return TokenFlag | static_cast(TK);
+}
+
+// A RuleID uniquely identifies a production rule in a grammar.
+// It is an index into a table of rules.
+using RuleID = uint16_t;
+// There are maximum 2^12 rules.
+static constexpr unsigned RuleBits = 12;
+
+// Represent a production rule in the grammar, e.g.
+//   expression := a b c
+//   ^Target   ^Sequence
+struct Rule {
+  Rule(SymbolID Target, llvm::ArrayRef Seq);
+
+  // We occupy 4 bits for the sequence, in theory, it can be at most 2^4 tokens
+  // long

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Haojian Wu 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 rG20e05b9f0ebe: [syntax][pseudo] Add Grammar for the clang 
pseudo-parser (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D114790?vs=405550&id=405552#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
@@ -0,0 +1,102 @@
+//===--- GrammarTests.cpp - grammar tests  --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Pseudo/Grammar.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+namespace {
+
+using testing::AllOf;
+using testing::ElementsAre;
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(TargetID, SID, "") { return arg.Target == SID; }
+template  testing::Matcher Sequence(T... IDs) {
+  return testing::Property(&Rule::seq, ElementsAre(IDs...));
+}
+
+class GrammarTest : public ::testing::Test {
+public:
+  void build(llvm::StringRef BNF) {
+Diags.clear();
+G = Grammar::parseBNF(BNF, Diags);
+  }
+
+  SymbolID lookup(llvm::StringRef Name) const {
+for (unsigned I = 0; I < NumTerminals; ++I)
+  if (G->table().Terminals[I] == Name)
+return tokenSymbol(static_cast(I));
+for (SymbolID ID = 0; ID < G->table().Nonterminals.size(); ++ID)
+  if (G->table().Nonterminals[ID].Name == Name)
+return ID;
+ADD_FAILURE() << "No such symbol found: " << Name;
+return 0;
+  }
+
+protected:
+  std::unique_ptr G;
+  std::vector Diags;
+};
+
+TEST_F(GrammarTest, Basic) {
+  build("expression := IDENTIFIER + expression # comment");
+  EXPECT_THAT(Diags, IsEmpty());
+
+  auto ExpectedRule =
+  AllOf(TargetID(lookup("expression")),
+Sequence(lookup("IDENTIFIER"), lookup("+"), lookup("expression")));
+  auto ExpressionID = lookup("expression");
+  EXPECT_EQ(G->symbolName(ExpressionID), "expression");
+  EXPECT_THAT(G->rulesFor(ExpressionID), UnorderedElementsAre(ExpectedRule));
+  const auto &Rule = G->lookupRule(/*RID=*/0);
+  EXPECT_THAT(Rule, ExpectedRule);
+  EXPECT_THAT(G->symbolName(Rule.seq()[0]), "IDENTIFIER");
+  EXPECT_THAT(G->symbolName(Rule.seq()[1]), "+");
+  EXPECT_THAT(G->symbolName(Rule.seq()[2]), "expression");
+}
+
+TEST_F(GrammarTest, EliminatedOptional) {
+  build("_ := CONST_opt INT ;_opt");
+  EXPECT_THAT(Diags, IsEmpty());
+  EXPECT_THAT(G->table().Rules,
+  UnorderedElementsAre(
+  Sequence(lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT"), lookup(";")),
+  Sequence(lookup("INT"), lookup(";";
+}
+
+TEST_F(GrammarTest, Diagnostics) {
+  build(R"cpp(
+_ := ,_opt
+_ := undefined-sym
+null :=
+_ := IDENFIFIE # a typo of the terminal IDENFITIER
+
+invalid
+  )cpp");
+
+  EXPECT_THAT(Diags, UnorderedElementsAre(
+ "Rule '_ := ,_opt' has a nullable RHS",
+ "Rule 'null := ' has a nullable RHS",
+ "No rules for nonterminal: undefined-sym",
+ "Failed to parse 'invalid': no separator :=",
+ "Token-like name IDENFIFIE is used as a nonterminal",
+ "No rules for nonterminal: IDENFIFIE"));
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace syntax
+} // namespace clang
Index: clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
@@ -0,0 +1,20 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_unittest(ClangPseudoTests
+  GrammarTests.cpp
+)
+
+clang_target_link_libraries(ClangPseudoTests
+  PRIVATE
+  clangBasic
+  clangLex
+  clangSyntaxPseudo
+  clangTesting
+  )
+
+target_link_libraries(ClangPseudoTests
+  PRIVATE
+  LLVMTestingSupport
+  )

[PATCH] D118873: [clang-format] Revert a feature in RemoveBracesLLVM

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D118873#3292956 , @curdeius wrote:

> Ok, so now (until a new fix), it will always remove the braces for a single 
> statement even if it's multi-line. That's fine.

Yep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118873

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

[PATCH] D118882: [clangd] IncludeCleaner: Decrease API dependency on clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118882

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h

Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -50,6 +50,9 @@
 /// - don't attempt to describe where symbols were referenced from in
 ///   ambiguous cases (e.g. implicitly used symbols, multiple declarations)
 /// - err on the side of reporting all possible locations
+ReferencedLocations findReferencedLocations(const SourceManager &SM,
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer &Tokens);
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
 struct ReferencedFiles {
@@ -60,6 +63,9 @@
 /// Retrieves IDs of all files containing SourceLocations from \p Locs.
 /// The output only includes things SourceManager sees as files (not macro IDs).
 /// This can include ,  etc that are not true files.
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+std::function HeaderResponsible);
 ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
 const IncludeStructure &Includes,
 const SourceManager &SM);
@@ -81,8 +87,8 @@
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code);
 
-/// Affects whether standard library includes should be considered for removal.
-/// This is off by default for now due to implementation limitations:
+/// Affects whether standard library includes should be considered for
+/// removal. This is off by default for now due to implementation limitations:
 /// - macros are not tracked
 /// - symbol names without a unique associated header are not tracked
 /// - references to std-namespaced C types are not properly tracked:
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -14,6 +14,7 @@
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
@@ -23,6 +24,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -207,10 +209,10 @@
 
 // Finds locations of macros referenced from within the main file. That includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
-void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
+  const syntax::TokenBuffer &Tokens,
+  ReferencedLocations &Result) {
   trace::Span Tracer("IncludeCleaner::findReferencedMacros");
-  auto &SM = AST.getSourceManager();
-  auto &PP = AST.getPreprocessor();
   // FIXME(kirillbobyrev): The macros from the main file are collected in
   // ParsedAST's MainFileMacros. However, we can't use it here because it
   // doesn't handle macro references that were not expanded, e.g. in macro
@@ -220,8 +222,7 @@
   // this mechanism (as opposed to iterating through all tokens) will improve
   // the performance of findReferencedMacros and also improve other features
   // relying on MainFileMacros.
-  for (const syntax::Token &Tok :
-   AST.getTokens().spelledTokens(SM.getMainFileID())) {
+  for (const syntax::Token &Tok : Tokens.spelledTokens(SM.getMainFileID())) {
 auto Macro = locateMacroAt(Tok, PP);
 if (!Macro)
   continue;
@@ -287,18 +288,25 @@
 
 } // namespace
 
-ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+ReferencedLocations findReferencedLocations(const SourceManager &SM,
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer &Tokens) {
   trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
-  ReferencedLocationCrawler Crawler(Result, AST.getSourceManager());
-  Crawler.TraverseAST(AST.getASTContext());
-  findReferencedMacros(AST, Result);
+  ReferencedLocationCrawler Crawler(Result, SM);
+  Crawler.TraverseAST(Ct

[clang] eaef54f - [clang-format] Revert a feature in RemoveBracesLLVM

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2022-02-03T02:56:09-08:00
New Revision: eaef54f21388350ca72d4dadf33728f70566e531

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

LOG: [clang-format] Revert a feature in RemoveBracesLLVM

Revert the handling of a single-statement block that gets wrapped.

See issue #53543.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index cdc2740ba9642..c43c8da6f3984 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "TokenAnnotator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -438,34 +437,8 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() 
const {
  (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
 
-bool UnwrappedLineParser::mightFitOnOneLine() const {
-  const auto ColumnLimit = Style.ColumnLimit;
-  if (ColumnLimit == 0)
-return true;
-
-  if (Lines.empty())
-return true;
-
-  const auto &PreviousLine = Lines.back();
-  const auto &Tokens = PreviousLine.Tokens;
-  assert(!Tokens.empty());
-  const auto *LastToken = Tokens.back().Tok;
-  assert(LastToken);
-  if (!LastToken->isOneOf(tok::semi, tok::comment))
-return true;
-
-  AnnotatedLine Line(PreviousLine);
-  assert(Line.Last == LastToken);
-
-  TokenAnnotator Annotator(Style, Keywords);
-  Annotator.annotate(Line);
-  Annotator.calculateFormattingInformation(Line);
-
-  return Line.Level * Style.IndentWidth + LastToken->TotalLength <= 
ColumnLimit;
-}
-
 // Returns true if a simple block, or false otherwise. (A simple block has a
-// single statement that fits on a single line.)
+// single statement.)
 bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind) 
{
   const bool IsPrecededByCommentOrPPDirective =
   !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
@@ -502,9 +475,7 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, 
IfStmtKind *IfKind) {
 precededByCommentOrPPDirective())
   return false;
 const FormatToken *Next = Tokens->peekNextToken();
-if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
-  return false;
-return mightFitOnOneLine();
+return Next->isNot(tok::comment) || Next->NewlinesBefore > 0;
   }
   nextToken();
   addUnwrappedLine();

diff  --git a/clang/lib/Format/UnwrappedLineParser.h 
b/clang/lib/Format/UnwrappedLineParser.h
index 3f64d57c7bff7..f39d76187f440 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -92,7 +92,6 @@ class UnwrappedLineParser {
   void reset();
   void parseFile();
   bool precededByCommentOrPPDirective() const;
-  bool mightFitOnOneLine() const;
   bool parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind = nullptr);
   IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 
1u,
 bool MunchSemi = true,

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 866847a531355..720c6e9b6b2f1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23965,6 +23965,19 @@ TEST_F(FormatTest, RemoveBraces) {
"};",
Style);
 
+  // FIXME: See https://github.com/llvm/llvm-project/issues/53543.
+#if 0
+  Style.ColumnLimit = 65;
+
+  verifyFormat("if (condition) {\n"
+   "  ff(Indices,\n"
+   " [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
+   "} else {\n"
+   "  ff(Indices,\n"
+   " [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
+   "}",
+   Style);
+
   Style.ColumnLimit = 20;
 
   verifyFormat("if (a) {\n"
@@ -23981,6 +23994,9 @@ TEST_F(FormatTest, RemoveBraces) {
"  b = c >= 0 ? d : e;\n"
"}",
Style);
+#endif
+
+  Style.ColumnLimit = 20;
 
   verifyFormat("if (a)\n"
"  b = c > 0 ? d : e;",



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

[PATCH] D118873: [clang-format] Revert a feature in RemoveBracesLLVM

[Owen Pan 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 rGeaef54f21388: [clang-format] Revert a feature in 
RemoveBracesLLVM (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D118873?vs=405525&id=405560#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118873

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23965,6 +23965,19 @@
"};",
Style);
 
+  // FIXME: See https://github.com/llvm/llvm-project/issues/53543.
+#if 0
+  Style.ColumnLimit = 65;
+
+  verifyFormat("if (condition) {\n"
+   "  ff(Indices,\n"
+   " [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
+   "} else {\n"
+   "  ff(Indices,\n"
+   " [&](unsigned LHSI, unsigned RHSI) { return true; });\n"
+   "}",
+   Style);
+
   Style.ColumnLimit = 20;
 
   verifyFormat("if (a) {\n"
@@ -23981,6 +23994,9 @@
"  b = c >= 0 ? d : e;\n"
"}",
Style);
+#endif
+
+  Style.ColumnLimit = 20;
 
   verifyFormat("if (a)\n"
"  b = c > 0 ? d : e;",
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -92,7 +92,6 @@
   void reset();
   void parseFile();
   bool precededByCommentOrPPDirective() const;
-  bool mightFitOnOneLine() const;
   bool parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind = nullptr);
   IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
 bool MunchSemi = true,
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "TokenAnnotator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -438,34 +437,8 @@
  (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
 
-bool UnwrappedLineParser::mightFitOnOneLine() const {
-  const auto ColumnLimit = Style.ColumnLimit;
-  if (ColumnLimit == 0)
-return true;
-
-  if (Lines.empty())
-return true;
-
-  const auto &PreviousLine = Lines.back();
-  const auto &Tokens = PreviousLine.Tokens;
-  assert(!Tokens.empty());
-  const auto *LastToken = Tokens.back().Tok;
-  assert(LastToken);
-  if (!LastToken->isOneOf(tok::semi, tok::comment))
-return true;
-
-  AnnotatedLine Line(PreviousLine);
-  assert(Line.Last == LastToken);
-
-  TokenAnnotator Annotator(Style, Keywords);
-  Annotator.annotate(Line);
-  Annotator.calculateFormattingInformation(Line);
-
-  return Line.Level * Style.IndentWidth + LastToken->TotalLength <= ColumnLimit;
-}
-
 // Returns true if a simple block, or false otherwise. (A simple block has a
-// single statement that fits on a single line.)
+// single statement.)
 bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind) {
   const bool IsPrecededByCommentOrPPDirective =
   !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
@@ -502,9 +475,7 @@
 precededByCommentOrPPDirective())
   return false;
 const FormatToken *Next = Tokens->peekNextToken();
-if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
-  return false;
-return mightFitOnOneLine();
+return Next->isNot(tok::comment) || Next->NewlinesBefore > 0;
   }
   nextToken();
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118882: [clangd] IncludeCleaner: Decrease API dependency on clangd

[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/IncludeCleaner.h:55
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer &Tokens);
 ReferencedLocations findReferencedLocations(ParsedAST &AST);

we can make Tokens & PP optional, and only track macros if they are present?

This lowers the barrier to entry a bit



Comment at: clang-tools-extra/clangd/IncludeCleaner.h:65
 /// The output only includes things SourceManager sees as files (not macro 
IDs).
 /// This can include ,  etc that are not true files.
+ReferencedFiles

doc HeaderResponsible



Comment at: clang-tools-extra/clangd/IncludeCleaner.h:68
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+std::function HeaderResponsible);
 ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,

std::function -> llvm::function_ref?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118882

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

[clang] d97a4df - [OpenCL] Move most _explicit atomics into multiclass; NFC

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2022-02-03T11:09:41Z
New Revision: d97a4dfea6c2781494f6fe54ce265128f5c08dc2

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

LOG: [OpenCL] Move most _explicit atomics into multiclass; NFC

This will simplify future conditionalization for OpenCL 3.0
optionality of atomic features.

The only set of atomic functions not using the multiclass is
atomic_compare_exchange_strong/weak, as these don't fit the common
pattern due to having 2 MemoryOrder arguments.

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index cd704ba2df13d..516653681331e 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -1040,6 +1040,18 @@ let Extension = FuncExtOpenCLCxx in {
   }
 }
 
+// An atomic builtin with 2 additional _explicit variants.
+multiclass BuiltinAtomicExplicit Types> {
+  // Without explicit MemoryOrder or MemoryScope.
+  def : Builtin;
+
+  // With an explicit MemoryOrder argument.
+  def : Builtin;
+
+  // With explicit MemoryOrder and MemoryScope arguments.
+  def : Builtin;
+}
+
 // OpenCL v2.0 s6.13.11 - Atomic Functions.
 let MinVersion = CL20 in {
   def : Builtin<"atomic_work_item_fence", [Void, MemFenceFlags, MemoryOrder, 
MemoryScope]>;
@@ -1049,24 +1061,12 @@ let MinVersion = CL20 in {
   [AtomicFloat, Float], [AtomicDouble, Double]] in {
 def : Builtin<"atomic_init",
 [Void, PointerType, GenericAS>, 
TypePair[1]]>;
-def : Builtin<"atomic_store",
+defm : BuiltinAtomicExplicit<"atomic_store",
 [Void, PointerType, GenericAS>, 
TypePair[1]]>;
-def : Builtin<"atomic_store_explicit",
-[Void, PointerType, GenericAS>, TypePair[1], 
MemoryOrder]>;
-def : Builtin<"atomic_store_explicit",
-[Void, PointerType, GenericAS>, TypePair[1], 
MemoryOrder, MemoryScope]>;
-def : Builtin<"atomic_load",
+defm : BuiltinAtomicExplicit<"atomic_load",
 [TypePair[1], PointerType, GenericAS>]>;
-def : Builtin<"atomic_load_explicit",
-[TypePair[1], PointerType, GenericAS>, 
MemoryOrder]>;
-def : Builtin<"atomic_load_explicit",
-[TypePair[1], PointerType, GenericAS>, 
MemoryOrder, MemoryScope]>;
-def : Builtin<"atomic_exchange",
+defm : BuiltinAtomicExplicit<"atomic_exchange",
 [TypePair[1], PointerType, GenericAS>, 
TypePair[1]]>;
-def : Builtin<"atomic_exchange_explicit",
-[TypePair[1], PointerType, GenericAS>, 
TypePair[1], MemoryOrder]>;
-def : Builtin<"atomic_exchange_explicit",
-[TypePair[1], PointerType, GenericAS>, 
TypePair[1], MemoryOrder, MemoryScope]>;
 foreach Variant = ["weak", "strong"] in {
   def : Builtin<"atomic_compare_exchange_" # Variant,
   [Bool, PointerType, GenericAS>,
@@ -1084,249 +1084,125 @@ let MinVersion = CL20 in {
   [AtomicLong, Long, Long], [AtomicULong, ULong, ULong],
   [AtomicUIntPtr, UIntPtr, PtrDiff]] in {
 foreach ModOp = ["add", "sub"] in {
-  def : Builtin<"atomic_fetch_" # ModOp,
+  defm : BuiltinAtomicExplicit<"atomic_fetch_" # ModOp,
   [TypePair[1], PointerType, GenericAS>, 
TypePair[2]]>;
-  def : Builtin<"atomic_fetch_" # ModOp # "_explicit",
-  [TypePair[1], PointerType, GenericAS>, 
TypePair[2], MemoryOrder]>;
-  def : Builtin<"atomic_fetch_" # ModOp # "_explicit",
-  [TypePair[1], PointerType, GenericAS>, 
TypePair[2], MemoryOrder, MemoryScope]>;
 }
   }
   foreach TypePair = [[AtomicInt, Int, Int], [AtomicUInt, UInt, UInt],
   [AtomicLong, Long, Long], [AtomicULong, ULong, ULong]] 
in {
 foreach ModOp = ["or", "xor", "and", "min", "max"] in {
-  def : Builtin<"atomic_fetch_" # ModOp,
+  defm : BuiltinAtomicExplicit<"atomic_fetch_" # ModOp,
   [TypePair[1], PointerType, GenericAS>, 
TypePair[2]]>;
-  def : Builtin<"atomic_fetch_" # ModOp # "_explicit",
-  [TypePair[1], PointerType, GenericAS>, 
TypePair[2], MemoryOrder]>;
-  def : Builtin<"atomic_fetch_" # ModOp # "_explicit",
-  [TypePair[1], PointerType, GenericAS>, 
TypePair[2], MemoryOrder, MemoryScope]>;
 }
   }
 
-  def : Builtin<"atomic_flag_clear",
+  defm : BuiltinAtomicExplicit<"atomic_flag_clear",
   [Void, PointerType, GenericAS>]>;
-  def : Builtin<"atomic_flag_clear_explicit",
-  [Void, PointerType, GenericAS>, MemoryOrder]>;
-  def : Builtin<"atomic_flag_clear_explicit",
-  [Void, PointerType, GenericAS>, MemoryOrder, 
MemoryScope]>;
 
-  def : Builtin<"atomic_flag_test_and_set",
+  defm : BuiltinAtomicExplicit<"atomic_flag_test_and_set",
   [Bool, PointerType, GenericAS>]>;
-  def : Builtin<

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, a bunch of random notes but I'm getting a higher level picture.

I'm not sure about the split between LRAutomaton and LRTable. I suppose doing 
it in two stages simplifies the implementation, but I don't think the first 
stage particularly needs to be public.
Also I think the names are confusing: Automaton vs Table isn't a clear contrast 
as one describes behavior and one a data structure. And the "LRTable" seems 
more automaton-like to me!

I think my suggestion would be:

- call the final structure LRAutomaton
- call the LRAutomaton::build()::Builder structure `LRGraph` or something
- I'm not sure the LRAutomaton structure needs to exist exactly, seems like the 
current LRTable::build() could operate on `LRGraph` directly

but I'm going to ponder this more.




Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRAutomaton.h:32
+public:
+  Item(RuleID ID, uint8_t RuleLength, uint8_t DotPos)
+  : RID(ID), RuleLength(RuleLength), DotPos(DotPos) {}

accepting the grammar as a parameter rather than the rule length would let us 
ensure the length is correct locally rather than relying on the caller.
Essentially all the callers need to do a lookup anyway.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRAutomaton.h:45
+  }
+  const pseudo::Rule &rule(const Grammar &G) const { return G.lookupRule(RID); 
}
+  RuleID ruleID() const { return RID; }

nit: redundant pseudo::qualifier



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRAutomaton.h:47
+  RuleID ruleID() const { return RID; }
+  int dotPos() const { return DotPos; }
+  std::string dump(const Grammar &G) const;

nit: unsigned.

just dot()?
and similarly ruleID()->rule()?
Not sure these would really be unclear



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRAutomaton.h:79
+
+// A deterministic finite state automaton for LR parsing.
+//

Hmm, isn't the point of GLR that it's a nondeterministic automaton?

We've done the LR-style NFA->DFA conversion, but it's not complete (when we hit 
a conflict we continue rather than giving up)



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRBuilder.cpp:144
+   "augmented symbol rule must be _ := start_symbol.");
+auto StartState = closure({{{RID, Rule.size(), 0}}}, G);
+auto Result = Builder.insert(std::move(StartState));

nit: too many braces - please spell out some of the types
(and Auto should probably be State here?)



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRBuilder.cpp:215
+for (const Item &I : Automaton.states()[SID]) {
+  // If "_ := start_symbol ." in the State, set Action[State, eof] to
+  // accept.

These comments are echoing the code - saying what, not why.

"If we've just parsed the start symbol, we can accept the input".



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRBuilder.cpp:220
+Builder.addAction(SID, tokenSymbol(tok::eof),
+  LRTable::Action{LRTable::Action::Kind::Accept, {}});
+continue;

Seems like we should maybe have the accepted symbol as a payload on the Accept 
action?

Actually why do we need an Accept action, as opposed to just a reduce action 
and recognize it because it's the symbol we want and the stack is empty?



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRBuilder.cpp:224
+  if (!I.hasNext()) {
+// If a state i has an item "A := x.", set Action[i, a] to reduce
+// "A := x" for all a in FOLLOW(A).

"If we've reached the end of a rule A := ..., then we can reduce if the next 
token is in the follow set of A"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118196

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

[PATCH] D118608: [NFC] Increase initial size of FoldingSets used in ASTContext and CodeGenTypes

[Dawid Jurczak via Phabricator via cfe-commits]

yurai007 added a comment.

In D118608#3290086 , 
@serge-sans-paille wrote:

> Thanks, that's helpful. Any hint about why this particular values?

Sure, let me elaborate on this. Initial capacities were empirically chosen to 
reduce number of internal hashtable growths (rehashings) in FoldingSet. 
FoldingSet has initial size set to 6 by default which corresponds to 2^6 
buckets in internal hashtable. In most cases it's ok and doesn't really matter. 
However while building huge codebase (like Linux) I find 
PointerTypes/ElaboratedTypes/ParenTypes beeing hot and growing very often to 
256 (2^8) or 512 buckets (2^9). 
That would explains choosing GeneralTypesLog2InitSize and 
ConstantArrayTypesLog2InitSize as more appropriate initial values.
For FunctionProtoTypes initial 2^6 capacity is way too conservative as its 
ContextualFoldingSet easly grows to 2^10/2^11 buckets.
It's not only about "empirical proof". For example from frontend perspective 
FunctionProtoTypes keeps all found function prototypes in compiling module.
Since it's reasonable to assume that average non-trivial module has rather 
hundreds or thousands functions than 64 using 2^12 as initial capacity should 
be enough.
Also it's worth to mention that with such approach we don't pay much in terms 
of memory footprint because FoldingSet internally in hashtable stores just 
pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118608

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

[PATCH] D118700: Add support to --gcc-toolchain flag for GCC compiled with --enable-version-specific-runtime-libs.

[Raúl Peñacoba via Phabricator via cfe-commits]

rpenacob updated this revision to Diff 405567.

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

https://reviews.llvm.org/D118700

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/include/c++/.keep
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/lib64/.keep
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/32/crtbegin.o
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/include/c++/.keep
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/lib32/.keep
  
clang/test/Driver/Inputs/gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/lib64/.keep
  clang/test/Driver/gcc-toolchain-rt-libs-multi.cpp
  clang/test/Driver/gcc-toolchain-rt-libs.cpp


Index: clang/test/Driver/gcc-toolchain-rt-libs.cpp
===
--- /dev/null
+++ clang/test/Driver/gcc-toolchain-rt-libs.cpp
@@ -0,0 +1,12 @@
+// RUN: %clangxx %s -### -stdlib=libstdc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=STDCPLUS
+// RUN: %clangxx %s -### -stdlib=libc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=LIBCPLUS
+
+int main() {}
+
+// STDCPLUS: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// STDCPLUS: 
"-L{{.*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0"
+// STDCPLUS: 
"-L{{.*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/../lib64"
+
+// LIBCPLUS-NOT: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// LIBCPLUS: 
"-L{{.*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0"
+// LIBCPLUS: 
"-L{{.*}}gcc_version_parsing_rt_libs/lib/gcc/x86_64-redhat-linux/10.2.0/../lib64"
Index: clang/test/Driver/gcc-toolchain-rt-libs-multi.cpp
===
--- /dev/null
+++ clang/test/Driver/gcc-toolchain-rt-libs-multi.cpp
@@ -0,0 +1,22 @@
+// RUN: %clangxx %s -### -stdlib=libstdc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs_multilib 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=X64-STDCPLUS
+// RUN: %clangxx %s -### -stdlib=libc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs_multilib 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=X64-LIBCPLUS
+// RUN: %clangxx %s -m32 -### -stdlib=libstdc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs_multilib 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=X32-STDCPLUS
+// RUN: %clangxx %s -m32 -### -stdlib=libc++ 
--gcc-toolchain=%S/Inputs/gcc_version_parsing_rt_libs_multilib 
--target=x86_64-redhat-linux 2>&1 | FileCheck %s -check-prefix=X32-LIBCPLUS
+
+int main() {}
+
+// X64-STDCPLUS: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// X64-STDCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0"
+// X64-STDCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../lib64"
+
+// X64-LIBCPLUS-NOT: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// X64-LIBCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0"
+// X64-LIBCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../lib64"
+
+// X32-STDCPLUS: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// X32-STDCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/32"
+// X32-STDCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../lib32"
+
+// X32-LIBCPLUS-NOT: "-internal-isystem" "{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../../../gcc/x86_64-redhat-linux/10.2.0/include/c++/"
+// X32-LIBCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/32"
+// X32-LIBCPLUS: "-L{{[^ 
]*}}gcc_version_parsing_rt_libs_multilib/lib/gcc/x86_64-redhat-linux/10.2.0/../lib32"
Index: clang/lib/Driver/To

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam created this revision.
saiislam added reviewers: ABataev, jdoerfert, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
saiislam requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

OpenMP Spec 5.0 [2.12.5, Restrictions]: If a device clause in which the
ancestor device-modifier appears is present on the target construct,
then a requires directive with the reverse_offload clause must be
specified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118887

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_device_ancestor_messages.cpp
  clang/test/OpenMP/target_device_codegen.cpp


Index: clang/test/OpenMP/target_device_codegen.cpp
===
--- clang/test/OpenMP/target_device_codegen.cpp
+++ clang/test/OpenMP/target_device_codegen.cpp
@@ -40,11 +40,6 @@
   // CHECK:   [[END]]
   #pragma omp target device(device_num: n)
   ;
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  // CHECK:   call void @__omp_offloading_{{.+}}_l46()
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  #pragma omp target device(ancestor: n)
-  ;
 }
 
 #endif
Index: clang/test/OpenMP/target_device_ancestor_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_device_ancestor_messages.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp -fopenmp-targets=x86_64 -x 
c++ -fexceptions -fcxx-exceptions %s
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp-simd 
-fopenmp-targets=x86_64 -x c++ -fexceptions -fcxx-exceptions %s
+
+void bar() {
+#pragma omp target device(ancestor : 1) // expected-error {{Device clause with 
ancestor device-modifier used without specifying 'requires reverse_offload'}}
+  ;
+}
Index: clang/test/OpenMP/target_ast_print.cpp
===
--- clang/test/OpenMP/target_ast_print.cpp
+++ clang/test/OpenMP/target_ast_print.cpp
@@ -342,7 +342,7 @@
 // RUN: %clang_cc1 -DOMP5 -verify -fopenmp-simd -fopenmp-version=50 -ast-print 
%s | FileCheck %s --check-prefix OMP5
 // RUN: %clang_cc1 -DOMP5 -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 
-emit-pch -o %t %s
 // RUN: %clang_cc1 -DOMP5 -fopenmp-simd -fopenmp-version=50 -std=c++11 
-include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s 
--check-prefix OMP5
-
+#pragma omp requires reverse_offload
 typedef void **omp_allocator_handle_t;
 extern const omp_allocator_handle_t omp_null_allocator;
 extern const omp_allocator_handle_t omp_default_mem_alloc;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -18759,6 +18759,18 @@
   if (ErrorFound)
 return nullptr;
 
+  // OpenMP 5.0 [2.12.5, Restrictions]
+  // In case of ancestor device-modifier, a requires directive with
+  // the reverse_offload clause must be specified.
+  if (Modifier == OMPC_DEVICE_ancestor) {
+if (!DSAStack->hasRequiresDeclWithClause()) {
+  targetDiag(
+  StartLoc,
+  diag::err_omp_device_ancestor_without_requires_reverse_offload);
+  ErrorFound = true;
+}
+  }
+
   OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
   OpenMPDirectiveKind CaptureRegion =
   getOpenMPCaptureRegionForClause(DKind, OMPC_device, LangOpts.OpenMP);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10699,6 +10699,8 @@
   "'%0' region encountered before requires directive with '%1' clause">;
 def note_omp_requires_encountered_directive : Note <
   "'%0' previously encountered here">;
+def err_omp_device_ancestor_without_requires_reverse_offload : Error <
+  "Device clause with ancestor device-modifier used without specifying 
'requires reverse_offload'">;
 def err_omp_invalid_scope : Error <
   "'#pragma omp %0' directive must appear only in file scope">;
 def note_omp_invalid_length_on_this_ptr_mapping : Note <


Index: clang/test/OpenMP/target_device_codegen.cpp
===
--- clang/test/OpenMP/target_device_codegen.cpp
+++ clang/test/OpenMP/target_device_codegen.cpp
@@ -40,11 +40,6 @@
   // CHECK:   [[END]]
   #pragma omp target device(device_num: n)
   ;
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  // CHECK:   call void @__omp_offloading_{{.+}}_l46()
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  #pragma omp target device(ancestor: n)
-  ;
 }
 
 #endif
Index: clang/test/Open

[clang-tools-extra] c39969e - [clangd] NFC, remove an unused local varaiable.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-02-03T12:52:24+01:00
New Revision: c39969ef252e7b285a9ef87e9a14d701acdced7b

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

LOG: [clangd] NFC, remove an unused local varaiable.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 5a11a1ce44c07..dda5ad36e9b89 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -1034,7 +1034,6 @@ llvm::Optional getHover(ParsedAST &AST, 
Position Pos,
 // So our selection tree should be biased right. (Tested with VSCode).
 SelectionTree ST =
 SelectionTree::createRight(AST.getASTContext(), TB, Offset, Offset);
-std::vector Result;
 if (const SelectionTree::Node *N = ST.commonAncestor()) {
   // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
   auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias,



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

[clang-tools-extra] 2f8da95 - [clangd][nfc] cleanup of remaining clang-tidy findings

[Christian Kühnel via cfe-commits]

Author: Christian Kühnel
Date: 2022-02-03T12:05:25Z
New Revision: 2f8da95e070e0f6b8c3d3310de51b016176e840a

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

LOG: [clangd][nfc] cleanup of remaining clang-tidy findings

There were some left-overs (or new things) from the previous patches.

This will get us down to 0 open findings except:
clang-tidy is complaining in some files about
`warning: #includes are not sorted properly [llvm-include-order]`
however, clang-format does revert these changes.
It looks like clang-tidy and clang-format disagree there.

Not sure how we can fix that...

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 0c758b6d296d4..9015de26c91f4 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -349,8 +349,7 @@ class Parser {
 if (auto Scalar = scalarValue(N, Desc)) {
   if (auto Bool = llvm::yaml::parseBool(**Scalar))
 return Located(*Bool, Scalar->Range);
-  else
-warning(Desc + " should be a boolean", N);
+  warning(Desc + " should be a boolean", N);
 }
 return llvm::None;
   }

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index fea143e67824b..be1361f8bd278 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1928,9 +1928,9 @@ static QualType unwrapFindType(QualType T) {
   // FIXME: use HeuristicResolver to unwrap smart pointers?
 
   // Function type => return type.
-  if (auto FT = T->getAs())
+  if (auto *FT = T->getAs())
 return unwrapFindType(FT->getReturnType());
-  if (auto CRD = T->getAsCXXRecordDecl()) {
+  if (auto *CRD = T->getAsCXXRecordDecl()) {
 if (CRD->isLambda())
   return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType());
 // FIXME: more cases we'd prefer the return type of the call operator?

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index eee64d84fda63..2b67034da8d48 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -254,8 +254,9 @@ bool AddUsing::prepare(const Selection &Inputs) {
 if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
-  } else if (Node->Parent->ASTNode.get() ||
- Node->Parent->ASTNode.get()) {
+  }
+  if (Node->Parent->ASTNode.get() ||
+  Node->Parent->ASTNode.get()) {
 // Node is TypeLoc, but it's parent is either TypeLoc or
 // NestedNameSpecifier. In both cases, we want to go up, to find
 // the outermost TypeLoc.

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index b060935145999..7de3746d22511 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -42,7 +42,6 @@ using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
-using testing::UnorderedElementsAreArray;
 
 ::testing::Matcher withFix(::testing::Matcher FixMatcher) {
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));



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

[PATCH] D118698: [clangd][nfc] cleanup of remaining clang-tidy findings

[Christian Kühnel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f8da95e070e: [clangd][nfc] cleanup of remaining clang-tidy 
findings (authored by kuhnel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118698

Files:
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -42,7 +42,6 @@
 using ::testing::Pair;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
-using testing::UnorderedElementsAreArray;
 
 ::testing::Matcher withFix(::testing::Matcher FixMatcher) {
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -254,8 +254,9 @@
 if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
-  } else if (Node->Parent->ASTNode.get() ||
- Node->Parent->ASTNode.get()) {
+  }
+  if (Node->Parent->ASTNode.get() ||
+  Node->Parent->ASTNode.get()) {
 // Node is TypeLoc, but it's parent is either TypeLoc or
 // NestedNameSpecifier. In both cases, we want to go up, to find
 // the outermost TypeLoc.
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1928,9 +1928,9 @@
   // FIXME: use HeuristicResolver to unwrap smart pointers?
 
   // Function type => return type.
-  if (auto FT = T->getAs())
+  if (auto *FT = T->getAs())
 return unwrapFindType(FT->getReturnType());
-  if (auto CRD = T->getAsCXXRecordDecl()) {
+  if (auto *CRD = T->getAsCXXRecordDecl()) {
 if (CRD->isLambda())
   return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType());
 // FIXME: more cases we'd prefer the return type of the call operator?
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -349,8 +349,7 @@
 if (auto Scalar = scalarValue(N, Desc)) {
   if (auto Bool = llvm::yaml::parseBool(**Scalar))
 return Located(*Bool, Scalar->Range);
-  else
-warning(Desc + " should be a boolean", N);
+  warning(Desc + " should be a boolean", N);
 }
 return llvm::None;
   }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -42,7 +42,6 @@
 using ::testing::Pair;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
-using testing::UnorderedElementsAreArray;
 
 ::testing::Matcher withFix(::testing::Matcher FixMatcher) {
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -254,8 +254,9 @@
 if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
-  } else if (Node->Parent->ASTNode.get() ||
- Node->Parent->ASTNode.get()) {
+  }
+  if (Node->Parent->ASTNode.get() ||
+  Node->Parent->ASTNode.get()) {
 // Node is TypeLoc, but it's parent is either TypeLoc or
 // NestedNameSpecifier. In both cases, we want to go up, to find
 // the outermost TypeLoc.
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1928,9 +1928,9 @@
   // FIXME: use HeuristicResolver to unwrap smart pointers?
 
   // Function type => return type.
-  if (auto FT = T->getAs())
+  if (auto *FT = T->getAs())
 return unwrapFindType(FT->getReturnType());
-  if (auto CRD = T->getAsCXXRecordDecl()) {
+  if (auto *CRD = T->getAsCXXRecordDecl()) {
 if (CRD->isLambda())
   return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType());
 // FIXME: more cases we'd prefer the return type of the call operator?
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===

[clang] 9694332 - [clang-format] Add missing newline in -style help

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2022-02-03T12:12:29Z
New Revision: 9694332b81dd14a0a2ab5d9ea9977558fd5f53be

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

LOG: [clang-format] Add missing newline in -style help

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 1ee557079f6c..daa815106049 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3246,7 +3246,7 @@ const char *StyleOptionHelpDescription =
 ".clang-format file located in one of the parent\n"
 "directories of the source file (or current\n"
 "directory for stdin).\n"
-"Use -style=file: to explicitly specify"
+"Use -style=file: to explicitly specify\n"
 "the configuration file.\n"
 "Use -style=\"{key: value, ...}\" to set specific\n"
 "parameters, e.g.:\n"



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

[PATCH] D118879: [clang-format] Avoid merging macro definitions.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:1812
+  Style.BraceWrapping.AfterFunction = true;
+  // Test that a macro definition gets never merged with the following
+  // definition.





Comment at: clang/unittests/Format/FormatTest.cpp:1814
+  // definition.
+  // FIXME: The AAA macro definition should probably not be splitted into 3
+  // lines.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118879

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

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_device_codegen.cpp:43-47
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  // CHECK:   call void @__omp_offloading_{{.+}}_l46()
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  #pragma omp target device(ancestor: n)
-  ;

Do we have a codegen test for the ancestor modifier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118887

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

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 405581.
njames93 added a comment.
Herald added a subscriber: mgorny.

Add testing infrastructure to verify stack trace output


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118520

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.cpp
@@ -0,0 +1,17 @@
+// REQUIRES: plugins
+// RUN: not --crash clang-tidy %s -checks='-*,crash-test' -load   \
+// RUN:   %llvmshlibdir/CTCrashTestTrace%pluginext 2>&1 | FileCheck %s
+
+void foo() {
+  for (int I = 0; I < 5; ++I) {
+  }
+}
+// CHECK: Processing check crash-test
+// CHECK-NEXT: --- Bound Nodes Begin ---
+// CHECK-DAG: FS - { ForStmt : <{{.*}}/crash-trace.cpp:6:3, line:7:3> }
+// CHECK-DAG: DS - { DeclStmt : <{{.*}}/crash-trace.cpp:6:8, col:17> }
+// CHECK-DAG: VD - { VarDecl I : <{{.*}}/crash-trace.cpp:6:8, col:16> }
+// CHECK-DAG: IL - { IntegerLiteral : <{{.*}}/crash-trace.cpp:6:16> }
+// CHECK-DAG: QT - { QualType : int }
+// CHECK-DAG: T - { BuiltinType : int }
+// CHECK-NEXT: --- Bound Nodes End ---
Index: clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
@@ -0,0 +1,69 @@
+//=== CTCrashTestTrace.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+///  \file This file implements a clang-tidy plugin that registers one check
+///  designed to crash on any match. This is to test clang-tidys stack trace
+///  output for when a buggy check causes a crash.
+///
+//===--===//
+
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class CrashingCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(
+forStmt(
+hasLoopInit(
+declStmt(hasSingleDecl(varDecl(hasType(qualType().bind("QT")),
+   hasType(type().bind("T")),
+   hasInitializer(
+   integerLiteral().bind("IL")))
+   .bind("VD")))
+.bind("DS")))
+.bind("FS"),
+this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+LLVM_BUILTIN_TRAP;
+  }
+
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+class CTCrashTestTraceModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck("crash-test");
+  }
+};
+} // namespace
+
+namespace tidy {
+// Register the CTCrashTestTraceModule using this statically initialized
+// variable.
+static ClangTidyModuleRegistry::Add<::CTCrashTestTraceModule>
+X("crash-module", "Add a check which will trap on any match");
+} // namespace tidy
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTCrashTestTraceModule.
+volatile int CTCrashTestTraceAnchor = 0;
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -96,6 +96,22 @@
 )
   endif()
   endif()
+
+  llvm_add_library(
+  CTCrashTestTrace
+  MODULE clang-tidy/CTCrashTestTrace.cpp
+  PLUGIN_TOOL clang-tidy
+  DEPENDS clang-tidy-headers)
+
+  if(TARGET CTCrashTestTrace)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTCrashTestTrace LLVMHello)
+  target_include_directories(CTCrashTestTrace PUBLIC BEFOR

[PATCH] D118890: [clang][deps] Disable global module index

[Jan Svoboda via Phabricator via cfe-commits]

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

While scanning dependencies of a TU that depends on a PCH, the scanner 
basically performs mixed implicit/explicit modular compilation. (Explicit 
modules come from the PCH.) This seems to trip up the global module index.

This patch disables global module index in the dependency scanner.

TODO: Provide a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118890

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -177,6 +177,9 @@
 ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
 true;
 
+ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
+ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
+
 FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
 ScanInstance.setFileManager(FileMgr);
 ScanInstance.createSourceManager(*FileMgr);


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -177,6 +177,9 @@
 ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
 true;
 
+ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
+ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
+
 FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
 ScanInstance.setFileManager(FileMgr);
 ScanInstance.createSourceManager(*FileMgr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:100
+
+  llvm_add_library(
+  CTCrashTestTrace

Fairly certain llvm_add_library isnt the correct function to use as that will 
put the test module in the build/lib directory. However thats what the other 
plugin module is using so I've copied that for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118520

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

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM marked 3 inline comments as done.
MarkMurrayARM added inline comments.



Comment at: llvm/test/CodeGen/ARM/ifcvt-branch-weight.ll:22
+; CHECK: bb.1.bb2:
+; CHECK: successors: %bb.2(0x4000)
 

dmgreen wrote:
> I'm not sure this is still checking anything useful. How has the full output 
> changed? Is there not a block with two outputs anymore?
It is a failure caused by my change. 



Comment at: llvm/test/CodeGen/Thumb2/v8_IT_3.ll:4
 ; RUN: llc < %s -mtriple=thumbv8 -arm-atomic-cfg-tidy=0 -relocation-model=pic 
| FileCheck %s --check-prefix=CHECK-PIC
-; RUN: llc < %s -mtriple=thumbv7 -arm-atomic-cfg-tidy=0 -arm-restrict-it 
-relocation-model=pic | FileCheck %s --check-prefix=CHECK-PIC
+; RUN: llc < %s -mtriple=thumbv7 -arm-atomic-cfg-tidy=0 -arm-restrict-it 
-relocation-model=pic | FileCheck %s --check-prefix=CHECK-PIC-RESTRICT-IT 
--check-prefix=CHECK-RESTRICT-IT
 

dmgreen wrote:
> I'm not sure what this test is doing. Can you just remove -arm-restrict-it 
> and update the check lines?
It's checking restricted ID blocks in position-independent code. I don't see 
what your request will achieve?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118044

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

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp:41-47
+  const Decl *ParentDecl = ConstructParents.begin()->get();
+  if (!ParentDecl)
+return nullptr;
+  if (const auto *ParentVar = dyn_cast(ParentDecl))
+return ParentVar;
+  if (const auto *ParentField = dyn_cast(ParentDecl))
+return ParentField;





Comment at: 
clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h:40
+  /// in this class.
+  virtual SmartPtrClassMatcher getSmartPointerClassMatcher() const = 0;
+

LegalizeAdulthood wrote:
> There's only one implementation of this abstract method declared here.
> 
> Why the extra level in the class hierarchy if there's only one implementation?
The idea I think is so that this can be used to make a similar check for 
`unique_ptr`. However I almost don't see the need for this, and I'd argue that 
they should both be implemented in one check. So this will detect 
```lang=c++
std::shared_ptr X = new int[5];
std::unique_ptr Y = new int[5];
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added inline comments.



Comment at: clang/test/OpenMP/target_device_codegen.cpp:43-47
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  // CHECK:   call void @__omp_offloading_{{.+}}_l46()
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  #pragma omp target device(ancestor: n)
-  ;

ABataev wrote:
> Do we have a codegen test for the ancestor modifier?
We shouldn't have a test for ancestor modifier because it can only be used when 
requires reverse_offload is specified and Spec 5.2 says that if an 
implementation is not supporting a requirement (reverse offload in this case) 
then it should give compile-time error termination [1].

I am going to propose this change in a different phab review.

[1] [[ 
https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5-2.pdf | 
OpenMP API Specification 5.2 ]], Section 8.2.1, lines 12-13, page 212.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118887

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

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_device_codegen.cpp:43-47
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  // CHECK:   call void @__omp_offloading_{{.+}}_l46()
-  // CHECK-NOT:   call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}},
-  #pragma omp target device(ancestor: n)
-  ;

saiislam wrote:
> ABataev wrote:
> > Do we have a codegen test for the ancestor modifier?
> We shouldn't have a test for ancestor modifier because it can only be used 
> when requires reverse_offload is specified and Spec 5.2 says that if an 
> implementation is not supporting a requirement (reverse offload in this case) 
> then it should give compile-time error termination [1].
> 
> I am going to propose this change in a different phab review.
> 
> [1] [[ 
> https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5-2.pdf | 
> OpenMP API Specification 5.2 ]], Section 8.2.1, lines 12-13, page 212.
I'm not asking about runtime test but the unit test. We should check at least 
that using this modifier does not crash the compiler. Could you restore this 
test here and add `#pragma omp requires reverse_offload` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118887

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

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 405591.
saiislam marked an inline comment as done.
saiislam added a comment.

Restored device ancestor codegen unit test with requires reverese_offload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118887

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_device_ancestor_messages.cpp
  clang/test/OpenMP/target_device_codegen.cpp


Index: clang/test/OpenMP/target_device_codegen.cpp
===
--- clang/test/OpenMP/target_device_codegen.cpp
+++ clang/test/OpenMP/target_device_codegen.cpp
@@ -11,7 +11,7 @@
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
-
+#pragma omp requires reverse_offload
 void foo(int n) {
 
   // CHECK:   [[N:%.+]] = load i32, i32* [[N_ADDR:%.+]],
Index: clang/test/OpenMP/target_device_ancestor_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_device_ancestor_messages.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp -fopenmp-targets=x86_64 -x 
c++ -fexceptions -fcxx-exceptions %s
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp-simd 
-fopenmp-targets=x86_64 -x c++ -fexceptions -fcxx-exceptions %s
+
+void bar() {
+#pragma omp target device(ancestor : 1) // expected-error {{Device clause with 
ancestor device-modifier used without specifying 'requires reverse_offload'}}
+  ;
+}
Index: clang/test/OpenMP/target_ast_print.cpp
===
--- clang/test/OpenMP/target_ast_print.cpp
+++ clang/test/OpenMP/target_ast_print.cpp
@@ -342,7 +342,7 @@
 // RUN: %clang_cc1 -DOMP5 -verify -fopenmp-simd -fopenmp-version=50 -ast-print 
%s | FileCheck %s --check-prefix OMP5
 // RUN: %clang_cc1 -DOMP5 -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 
-emit-pch -o %t %s
 // RUN: %clang_cc1 -DOMP5 -fopenmp-simd -fopenmp-version=50 -std=c++11 
-include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s 
--check-prefix OMP5
-
+#pragma omp requires reverse_offload
 typedef void **omp_allocator_handle_t;
 extern const omp_allocator_handle_t omp_null_allocator;
 extern const omp_allocator_handle_t omp_default_mem_alloc;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -18759,6 +18759,18 @@
   if (ErrorFound)
 return nullptr;
 
+  // OpenMP 5.0 [2.12.5, Restrictions]
+  // In case of ancestor device-modifier, a requires directive with
+  // the reverse_offload clause must be specified.
+  if (Modifier == OMPC_DEVICE_ancestor) {
+if (!DSAStack->hasRequiresDeclWithClause()) {
+  targetDiag(
+  StartLoc,
+  diag::err_omp_device_ancestor_without_requires_reverse_offload);
+  ErrorFound = true;
+}
+  }
+
   OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
   OpenMPDirectiveKind CaptureRegion =
   getOpenMPCaptureRegionForClause(DKind, OMPC_device, LangOpts.OpenMP);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10699,6 +10699,8 @@
   "'%0' region encountered before requires directive with '%1' clause">;
 def note_omp_requires_encountered_directive : Note <
   "'%0' previously encountered here">;
+def err_omp_device_ancestor_without_requires_reverse_offload : Error <
+  "Device clause with ancestor device-modifier used without specifying 
'requires reverse_offload'">;
 def err_omp_invalid_scope : Error <
   "'#pragma omp %0' directive must appear only in file scope">;
 def note_omp_invalid_length_on_this_ptr_mapping : Note <


Index: clang/test/OpenMP/target_device_codegen.cpp
===
--- clang/test/OpenMP/target_device_codegen.cpp
+++ clang/test/OpenMP/target_device_codegen.cpp
@@ -11,7 +11,7 @@
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
-
+#pragma omp requires reverse_offload
 void foo(int n) {
 
   // CHECK:   [[N:%.+]] = load i32, i32* [[N_ADDR:%.+]],
Index: clang/test/OpenMP/target_device_ancestor_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_device_ancestor_messages.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp -fopenmp-targets=x86_64 -x c++ -fexceptions -fcxx-exceptions %s
+// RUN: %clang_cc1 -triple=x86_64 -verify -fopenmp-simd -fopenmp-targets=x86_64 -x c++ -fexceptions -fcxx-exceptions %s
+
+void bar() {
+#pragma omp target device(ancestor : 1) // expected-error {{Device clause with ancestor device-modifie

[PATCH] D118887: [OpenMP][Clang] Allow ancestor device modifier only with reverse offloading

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118887

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

[PATCH] D118882: [clangd] IncludeCleaner: Decrease API dependency on clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 405598.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Resolve review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118882

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h

Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -25,6 +25,7 @@
 #include "ParsedAST.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include 
 
 namespace clang {
@@ -41,15 +42,17 @@
 ///   associated locations. These may be macro expansions, and are not resolved
 ///   to their spelling or expansion location. These locations are later used to
 ///   determine which headers should be marked as "used" and "directly used".
-/// - We also examine all identifier tokens in the file in case they reference
-///   macros.
-///
+/// - If \p Tokens is not nullptr, we also examine all identifier tokens in the
+///   file in case they reference macros macros.
 /// We use this to compute unused headers, so we:
 ///
 /// - cover the whole file in a single traversal for efficiency
 /// - don't attempt to describe where symbols were referenced from in
 ///   ambiguous cases (e.g. implicitly used symbols, multiple declarations)
 /// - err on the side of reporting all possible locations
+ReferencedLocations findReferencedLocations(const SourceManager &SM,
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer *Tokens);
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
 struct ReferencedFiles {
@@ -60,6 +63,12 @@
 /// Retrieves IDs of all files containing SourceLocations from \p Locs.
 /// The output only includes things SourceManager sees as files (not macro IDs).
 /// This can include ,  etc that are not true files.
+/// \p HeaderResponsible returns the public header that should be included given
+/// symbols from a file with the given FileID (example: public headers should be
+/// preferred to non self-contained and private headers).
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+llvm::function_ref HeaderResponsible);
 ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
 const IncludeStructure &Includes,
 const SourceManager &SM);
@@ -81,8 +90,8 @@
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code);
 
-/// Affects whether standard library includes should be considered for removal.
-/// This is off by default for now due to implementation limitations:
+/// Affects whether standard library includes should be considered for
+/// removal. This is off by default for now due to implementation limitations:
 /// - macros are not tracked
 /// - symbol names without a unique associated header are not tracked
 /// - references to std-namespaced C types are not properly tracked:
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -14,6 +14,7 @@
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
@@ -21,6 +22,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
@@ -207,10 +209,10 @@
 
 // Finds locations of macros referenced from within the main file. That includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
-void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
+  const syntax::TokenBuffer *Tokens,
+  ReferencedLocations &Result) {
   trace::Span Tracer("IncludeCleaner::findReferencedMacros");
-  auto &SM = AST.getSourceManager();
-  auto &PP = AST.getPreprocessor();
   // FIXME(kirillbobyrev): The macros from the main file are collected in
   // ParsedAST's MainFileMacros. However, we can't use it here because it
   // doesn't handle macro references that were not expanded, e.g. in macro
@@ -220,8 +222,7 @@
   // this mechanism (as opposed to iterating through all tokens) will improve
   // the

[clang-tools-extra] 089d9c5 - [clangd] IncludeCleaner: Decrease API dependency on clangd

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2022-02-03T14:40:53+01:00
New Revision: 089d9c50b29e8e0eb18884edf17451e11a84a80f

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

LOG: [clangd] IncludeCleaner: Decrease API dependency on clangd

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 85ba59519e8a..04449904ecc4 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -14,6 +14,7 @@
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
@@ -21,6 +22,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
@@ -207,10 +209,10 @@ clangd::Range getDiagnosticRange(llvm::StringRef Code, 
unsigned HashOffset) {
 
 // Finds locations of macros referenced from within the main file. That 
includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
-void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
+  const syntax::TokenBuffer *Tokens,
+  ReferencedLocations &Result) {
   trace::Span Tracer("IncludeCleaner::findReferencedMacros");
-  auto &SM = AST.getSourceManager();
-  auto &PP = AST.getPreprocessor();
   // FIXME(kirillbobyrev): The macros from the main file are collected in
   // ParsedAST's MainFileMacros. However, we can't use it here because it
   // doesn't handle macro references that were not expanded, e.g. in macro
@@ -220,8 +222,7 @@ void findReferencedMacros(ParsedAST &AST, 
ReferencedLocations &Result) {
   // this mechanism (as opposed to iterating through all tokens) will improve
   // the performance of findReferencedMacros and also improve other features
   // relying on MainFileMacros.
-  for (const syntax::Token &Tok :
-   AST.getTokens().spelledTokens(SM.getMainFileID())) {
+  for (const syntax::Token &Tok : Tokens->spelledTokens(SM.getMainFileID())) {
 auto Macro = locateMacroAt(Tok, PP);
 if (!Macro)
   continue;
@@ -287,18 +288,26 @@ FileID headerResponsible(FileID ID, const SourceManager 
&SM,
 
 } // namespace
 
-ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+ReferencedLocations findReferencedLocations(const SourceManager &SM,
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer *Tokens) 
{
   trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
-  ReferencedLocationCrawler Crawler(Result, AST.getSourceManager());
-  Crawler.TraverseAST(AST.getASTContext());
-  findReferencedMacros(AST, Result);
+  ReferencedLocationCrawler Crawler(Result, SM);
+  Crawler.TraverseAST(Ctx);
+  if (Tokens)
+findReferencedMacros(SM, PP, Tokens, Result);
   return Result;
 }
 
-ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
-const IncludeStructure &Includes,
-const SourceManager &SM) {
+ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  return findReferencedLocations(AST.getSourceManager(), AST.getASTContext(),
+ AST.getPreprocessor(), &AST.getTokens());
+}
+
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+llvm::function_ref HeaderResponsible) {
   std::vector Sorted{Locs.User.begin(), Locs.User.end()};
   llvm::sort(Sorted); // Group by FileID.
   ReferencedFilesBuilder Builder(SM);
@@ -318,7 +327,7 @@ ReferencedFiles findReferencedFiles(const 
ReferencedLocations &Locs,
   // HeaderIDs, as each inclusion of a non-self-contained file is distinct.
   llvm::DenseSet UserFiles;
   for (FileID ID : Builder.Files)
-UserFiles.insert(headerResponsible(ID, SM, Includes));
+UserFiles.insert(HeaderResponsible(ID));
 
   llvm::DenseSet StdlibFiles;
   for (const auto &Symbol : Locs.Stdlib)
@@ -328,6 +337,14 @@ ReferencedFiles findReferencedFiles(const 
ReferencedLocations &Locs,
   return {std::move(UserFiles), std::move(StdlibFiles)};
 }
 
+ReferencedFiles findReferencedFiles(const Refere

[PATCH] D118882: [clangd] IncludeCleaner: Decrease API dependency on clangd

[Kirill Bobyrev 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 rG089d9c50b29e: [clangd] IncludeCleaner: Decrease API 
dependency on clangd (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118882

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h

Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -25,6 +25,7 @@
 #include "ParsedAST.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include 
 
 namespace clang {
@@ -41,15 +42,17 @@
 ///   associated locations. These may be macro expansions, and are not resolved
 ///   to their spelling or expansion location. These locations are later used to
 ///   determine which headers should be marked as "used" and "directly used".
-/// - We also examine all identifier tokens in the file in case they reference
-///   macros.
-///
+/// - If \p Tokens is not nullptr, we also examine all identifier tokens in the
+///   file in case they reference macros macros.
 /// We use this to compute unused headers, so we:
 ///
 /// - cover the whole file in a single traversal for efficiency
 /// - don't attempt to describe where symbols were referenced from in
 ///   ambiguous cases (e.g. implicitly used symbols, multiple declarations)
 /// - err on the side of reporting all possible locations
+ReferencedLocations findReferencedLocations(const SourceManager &SM,
+ASTContext &Ctx, Preprocessor &PP,
+const syntax::TokenBuffer *Tokens);
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
 struct ReferencedFiles {
@@ -60,6 +63,12 @@
 /// Retrieves IDs of all files containing SourceLocations from \p Locs.
 /// The output only includes things SourceManager sees as files (not macro IDs).
 /// This can include ,  etc that are not true files.
+/// \p HeaderResponsible returns the public header that should be included given
+/// symbols from a file with the given FileID (example: public headers should be
+/// preferred to non self-contained and private headers).
+ReferencedFiles
+findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM,
+llvm::function_ref HeaderResponsible);
 ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
 const IncludeStructure &Includes,
 const SourceManager &SM);
@@ -81,8 +90,8 @@
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code);
 
-/// Affects whether standard library includes should be considered for removal.
-/// This is off by default for now due to implementation limitations:
+/// Affects whether standard library includes should be considered for
+/// removal. This is off by default for now due to implementation limitations:
 /// - macros are not tracked
 /// - symbol names without a unique associated header are not tracked
 /// - references to std-namespaced C types are not properly tracked:
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -14,6 +14,7 @@
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
@@ -21,6 +22,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
@@ -207,10 +209,10 @@
 
 // Finds locations of macros referenced from within the main file. That includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
-void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
+  const syntax::TokenBuffer *Tokens,
+  ReferencedLocations &Result) {
   trace::Span Tracer("IncludeCleaner::findReferencedMacros");
-  auto &SM = AST.getSourceManager();
-  auto &PP = AST.getPreprocessor();
   // FIXME(kirillbobyrev): The macros from the main file are collected in
   // ParsedAST's MainFileMacros. However, we can't use it here because it
   // doesn't handle macro references that were not expanded, e.g. in macro
@@ -

[PATCH] D112774: [RISCV] Support k-ext clang intrinsics

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence updated this revision to Diff 405605.
achieveartificialintelligence added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112774

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkc.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkx.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkc.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkx.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv32-zksh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknd.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zkne.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zknh.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksed.c
  clang/test/CodeGen/RISCV/rvk-intrinsics/riscv64-zksh.c
  clang/test/Driver/riscv-arch.c
  clang/test/Preprocessor/riscv-target-features.c

Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -31,6 +31,16 @@
 // CHECK-NOT: __riscv_zfh
 // CHECK-NOT: __riscv_v
 // CHECK-NOT: __riscv_vector
+// CHECK-NOT: __riscv_zbkc
+// CHECK-NOT: __riscv_zbkx
+// CHECK-NOT: __riscv_zbkb
+// CHECK-NOT: __riscv_zkne
+// CHECK-NOT: __riscv_zknd
+// CHECK-NOT: __riscv_zknh
+// CHECK-NOT: __riscv_zksh
+// CHECK-NOT: __riscv_zksed
+// CHECK-NOT: __riscv_zkr
+// CHECK-NOT: __riscv_zk
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32im -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-M-EXT %s
@@ -343,3 +353,53 @@
 // CHECK-ZVE32X-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32X-EXT: __riscv_vector 1
 // CHECK-ZVE32X-EXT: __riscv_zve32x 100{{$}}
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izbkc1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZBKC-EXT %s
+// CHECK-ZBKC-EXT: __riscv_zbkc
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izbkx1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZBKX-EXT %s
+// CHECK-ZBKX-EXT: __riscv_zbkx
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izbkb1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZBKB-EXT %s
+// CHECK-ZBKB-EXT: __riscv_zbkb
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izknd1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKND-EXT %s
+// CHECK-ZKND-EXT: __riscv_zknd
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izkne1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKNE-EXT %s
+// CHECK-ZKNE-EXT: __riscv_zkne
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izknh1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKNH-EXT %s
+// CHECK-ZKNH-EXT: __riscv_zknh
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izksh1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKSH-EXT %s
+// CHECK-ZKSH-EXT: __riscv_zksh
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izksed1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKSED-EXT %s
+// CHECK-ZKSED-EXT: __riscv_zksed
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izkr1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZKR-EXT %s
+// CHECK-ZKR-EXT: __riscv_zkr
+
+// RUN: %clang -target riscv64-unknown-linux-gnu \
+// RUN: -march=rv64izk1p0 -x c -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZK-EXT %s
+// CHECK-ZK-EXT: __riscv_zk
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -414,3 +414,15 @@
 // RUN: %clang -target riscv32-unknown-elf -march=rv32iv1p0_zvl32b1p0 -### %s -c 2>&1 | \
 // RUN:   FileCheck -check-prefix=RV32-ZVL-GOODVERS %s
 // RV32-ZVL-GOODVERS: "-target-feature" "+zvl32b"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32izbkc1p0 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZBKC %s
+// RV32-ZBKC: "-target-feature" "+zbkc"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32izbkx1p0 -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZBKX %s
+// RV32-ZBKX: "-t

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Carl Peto via Phabricator via cfe-commits]

carlos4242 added a comment.
Herald added a subscriber: dexonsmith.

Can I just add a +1 on something like this. My swift compiler build just broke 
at runtime because of this.

Would it be possible to change the default so it reads the address space from 
the target data layout so it's all automatic in most cases? I know there's a 
bit of complexity because you have to work out whether to read the data or 
function address space, is that visible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78491

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions and initializer lists in annotate attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this is heading in the right direction! I've got a few more comments 
and questions though.




Comment at: clang/include/clang/Basic/Attr.td:545
+  // Set to true if this attribute accepts parameter pack expansion 
expressions.
+  bit AcceptsExprPack = 0;
   // Lists language options, one of which is required to be true for the

You should probably add a release note about this new feature.



Comment at: clang/include/clang/Basic/Attr.td:789
   let PragmaAttributeSupport = 1;
+  let AcceptsExprPack = 1;
   let Documentation = [Undocumented];

You should definitely add a release note about this new behavior for annotate.



Comment at: clang/include/clang/Basic/Attr.td:790
+  let AcceptsExprPack = 1;
   let Documentation = [Undocumented];
 }

Le sigh. We should update the documentation for this new feature, but we have 
no documentation for the feature *to* update. (This is not your problem to 
solve in this patch.)



Comment at: clang/lib/Parse/ParseDecl.cpp:421-424
+  // Parse variadic identifier arg. This can either consume identifiers or
+  // expressions.
+  // FIXME: Variadic identifier args do not currently support parameter
+  //packs.

steffenlarsen wrote:
> aaron.ballman wrote:
> > (Might need to re-flow comments to 80 col.) I don't think this is a FIXME 
> > so much as a "this just doesn't work like that" situation.
> I think it makes sense to have it be a FIXME because in theory it could be 
> possible to have expression parameter packs expanded in an identifier list as 
> it accepts expressions. I have reworded it slightly. Do you think this is 
> better?
Maybe we're thinking about identifier lists differently. We only have two 
attributes that use those (`cpu_specific` and `cpu_dispatch`) and in both cases 
(and all cases I would expect), what's being received is effectively a list of 
enumerators (not enumerators in the C or C++ sense) that could not be mixed 
with expressions. Expressions would go through sema and do all the usual lookup 
work to turn them into a value, but these are not real objects and so the 
lookup would fail for them. e.g., we're not going to be able to support 
something like: `[[clang::cpu_specific(generic, pentium, Ts..., atom)]]`. So I 
don't think there's anything here to be fixed (I prefer my comment formulation 
as that makes it more clear).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4179-4182
+  if (AllArgs.size() < 1) {
+Diag(CI.getLoc(), diag::err_attribute_too_few_arguments) << CI << 1;
+return;
+  }

Please double-check that the call to `checkCommonAttributeFeatures()` from 
`ProcessDeclAttribute()` doesn't already handle this for you. If it does, then 
I think this can be replaced with `assert(!AllArgs.empty() && "expected at 
least one argument");`



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4203
+  if (AllArgs.size() && AllArgs[0]->isValueDependent()) {
+auto *Attr = AnnotateAttr::CreateWithDelayedArgs(
+S.getASTContext(), AllArgs.data(), AllArgs.size(), AL);

erichkeane wrote:
> I would like @aaron.ballman to comment on this, but I think we probably want 
> this case to be covered in the top of `HandleDeclAttr`, which would mean in 
> the 'not all values filled' case, we skip the 'handleAnnotateAttr'.  
> 
> WDYT Aaron?  The downside is that this function couldn't check a 'partially 
> filled in' attribute, but it would make us that much closer to this flag 
> being a very simple thing to opt into.
Do you mean `ProcessDeclAttribute()`? I don't think we should have 
attribute-specific logic in there, but are you thinking of something more 
general than that (I'm not seeing how the suggestion makes the flag easier to 
opt into)?



Comment at: clang/test/SemaTemplate/attributes.cpp:231-240
+// CHECK:  FunctionTemplateDecl {{.*}} RedeclaredAnnotatedFunc
+// CHECK:AnnotateAttr {{.*}} Inherited "ANNOTATE_FAR"
+// CHECK:AnnotateAttr {{.*}} Inherited "ANNOTATE_FIZ"
+// CHECK:  ConstantExpr {{.*}} 'int'
+// CHECK-NEXT:   value: Int 4
+// CHECK:  ConstantExpr {{.*}} 'int'
+// CHECK-NEXT:   value: Int 5

I was thrown by the CHECK and CHECK-NEXT mixtures here because I couldn't see 
that the inherited attributes were or were not also getting the expanded pack 
values. You should make sure to include all of the lines with CHECK-NEXT so we 
see a full picture of the AST dump (this file is not super consistent about it).


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

https://reviews.llvm.org/D114439

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions and initializer lists in annotate attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4203
+  if (AllArgs.size() && AllArgs[0]->isValueDependent()) {
+auto *Attr = AnnotateAttr::CreateWithDelayedArgs(
+S.getASTContext(), AllArgs.data(), AllArgs.size(), AL);

aaron.ballman wrote:
> erichkeane wrote:
> > I would like @aaron.ballman to comment on this, but I think we probably 
> > want this case to be covered in the top of `HandleDeclAttr`, which would 
> > mean in the 'not all values filled' case, we skip the 'handleAnnotateAttr'. 
> >  
> > 
> > WDYT Aaron?  The downside is that this function couldn't check a 'partially 
> > filled in' attribute, but it would make us that much closer to this flag 
> > being a very simple thing to opt into.
> Do you mean `ProcessDeclAttribute()`? I don't think we should have 
> attribute-specific logic in there, but are you thinking of something more 
> general than that (I'm not seeing how the suggestion makes the flag easier to 
> opt into)?
Ah, yes, thats what I mean.  The question for ME is whether there should be a 
generic "this supports variadic pack, so check to see if all the named non-expr 
arguments are fill-in-able.  If not, do the 'delayed' version.

This would mean that HandleAnnotateAttr NEVER sees the "CreateWithDelayedArgs" 
case.


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

https://reviews.llvm.org/D114439

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

[clang] 6f2c956 - [clang][docs] Regenerate ASTMatchers documentation

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-02-03T14:11:30Z
New Revision: 6f2c95657b083063039065cc9729d0a428e44140

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

LOG: [clang][docs] Regenerate ASTMatchers documentation

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index 4c3916c0325c9..a3f57996a6fb2 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -2745,6 +2745,18 @@ Node Matchers
 
 
 
+MatcherType>usingTypeMatcherUsingType>...
+Matches types specified 
through a using declaration.
+
+Given
+  namespace a { struct S {}; }
+  using a::S;
+  S s;
+
+usingType() matches the type of the variable declaration of s.
+
+
+
 MatcherType>variableArrayTypeMatcherVariableArrayType>...
 Matches C arrays 
with a specified size that is not an
 integer-constant-expression.
@@ -4201,8 +4213,8 @@ Narrowing Matchers
 
 
 
-MatcherFunctionDecl>isConsteval
-Matches consteval 
function declarations and if consteval/if ! consteval
+MatcherFunctionDecl>isConsteval
+Matches consteval 
function declarations and if consteval/if ! consteval
 statements.
 
 Given:
@@ -4489,8 +4501,8 @@ Narrowing Matchers
 
 
 
-MatcherIfStmt>isConsteval
-Matches consteval 
function declarations and if consteval/if ! consteval
+MatcherIfStmt>isConsteval
+Matches consteval 
function declarations and if consteval/if ! consteval
 statements.
 
 Given:
@@ -5633,7 +5645,7 @@ Narrowing Matchers
 
 Given:
   constinit int foo = 42;
-  constinit const char* bar = "baz";
+  constinit const char* bar = "bar";
   int baz = 42;
   [[clang::require_constant_initialization]] int xyz = 42;
 varDecl(isConstinit())
@@ -7549,19 +7561,24 @@ AST Traversal Matchers
 
 
 
-MatcherDeclRefExpr>throughUsingDeclMatcherUsingShadowDecl>
 InnerMatcher
-Matches a 
DeclRefExpr that refers to a declaration through a
-specific using shadow declaration.
+MatcherDeclRefExpr>throughUsingDeclMatcherUsingShadowDecl>
 Inner
+Matches if a node 
refers to a declaration through a specific
+using shadow declaration.
 
-Given
-  namespace a { void f() {} }
+Examples:
+  namespace a { int f(); }
   using a::f;
-  void g() {
-f(); // Matches this ..
-a::f();  // .. but not this.
-  }
+  int x = f();
 declRefExpr(throughUsingDecl(anything()))
-  matches f()
+  matches f
+
+  namespace a { class X{}; }
+  using a::X;
+  X x;
+typeLoc(loc(usingType(throughUsingDecl(anything()
+  matches X
+
+Usable as: MatcherDeclRefExpr>,
 MatcherUsingType>
 
 
 
@@ -7651,7 +7668,7 @@ AST Traversal Matchers
 
 
 MatcherDecltypeType>hasUnderlyingTypeMatcherType>
-Matches 
DecltypeType nodes to find out the underlying type.
+Matches 
DecltypeType or UsingType nodes to find the underlying type.
 
 Given
   decltype(1) a = 1;
@@ -7659,7 +7676,7 @@ AST Traversal Matchers
 decltypeType(hasUnderlyingType(isInteger()))
   matches the type of "a"
 
-Usable as: MatcherDecltypeType>
+Usable as: MatcherDecltypeType>,
 MatcherUsingType>
 
 
 
@@ -9658,6 +9675,40 @@ AST Traversal Matchers
   matches using X::b but not using X::a 
 
 
+MatcherUsingType>hasUnderlyingTypeMatcherType>
+Matches 
DecltypeType or UsingType nodes to find the underlying type.
+
+Given
+  decltype(1) a = 1;
+  decltype(2.0) b = 2.0;
+decltypeType(hasUnderlyingType(isInteger()))
+  matches the type of "a"
+
+Usable as: Matcher

[PATCH] D114439: [Annotation] Allow parameter pack expansions and initializer lists in annotate attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4203
+  if (AllArgs.size() && AllArgs[0]->isValueDependent()) {
+auto *Attr = AnnotateAttr::CreateWithDelayedArgs(
+S.getASTContext(), AllArgs.data(), AllArgs.size(), AL);

erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > I would like @aaron.ballman to comment on this, but I think we probably 
> > > want this case to be covered in the top of `HandleDeclAttr`, which would 
> > > mean in the 'not all values filled' case, we skip the 
> > > 'handleAnnotateAttr'.  
> > > 
> > > WDYT Aaron?  The downside is that this function couldn't check a 
> > > 'partially filled in' attribute, but it would make us that much closer to 
> > > this flag being a very simple thing to opt into.
> > Do you mean `ProcessDeclAttribute()`? I don't think we should have 
> > attribute-specific logic in there, but are you thinking of something more 
> > general than that (I'm not seeing how the suggestion makes the flag easier 
> > to opt into)?
> Ah, yes, thats what I mean.  The question for ME is whether there should be a 
> generic "this supports variadic pack, so check to see if all the named 
> non-expr arguments are fill-in-able.  If not, do the 'delayed' version.
> 
> This would mean that HandleAnnotateAttr NEVER sees the 
> "CreateWithDelayedArgs" case.
Thanks for clarifying -- yes, I think that would be preferable if it works out 
in a clean, generic way. I'd be fine with tablegen emitting something else (if 
necessary) to help generalize it.


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

https://reviews.llvm.org/D114439

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

[PATCH] D118900: [ASTMatchers] Expand isInline matcher to VarDecl

[Nathan James via Phabricator via cfe-commits]

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

Add support to the `isInline` matcher for C++17's inline variables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118900

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -197,6 +197,8 @@
   functionDecl(isInline(), hasName("f";
   EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
   namespaceDecl(isInline(), hasName("m";
+  EXPECT_TRUE(matches("inline int Foo = 5;",
+  varDecl(isInline(), hasName("Foo")), {Lang_CXX17}));
 }
 
 // FIXME: Figure out how to specify paths so the following tests pass on
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7683,18 +7683,22 @@
 ///   namespace n {
 ///   inline namespace m {}
 ///   }
+///   inline int Foo = 5;
 /// \endcode
 /// functionDecl(isInline()) will match ::f().
 /// namespaceDecl(isInline()) will match n::m.
-AST_POLYMORPHIC_MATCHER(isInline,
-AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
-FunctionDecl)) {
+/// varDecl(isInline()) will match Foo;
+AST_POLYMORPHIC_MATCHER(isInline, 
AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
+  FunctionDecl,
+  VarDecl)) {
   // This is required because the spelling of the function used to determine
   // whether inline is specified or not differs between the polymorphic types.
   if (const auto *FD = dyn_cast(&Node))
 return FD->isInlineSpecified();
-  else if (const auto *NSD = dyn_cast(&Node))
+  if (const auto *NSD = dyn_cast(&Node))
 return NSD->isInline();
+  if (const auto *VD = dyn_cast(&Node))
+return VD->isInline();
   llvm_unreachable("Not a valid polymorphic type");
 }
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4331,8 +4331,10 @@
   namespace n {
   inline namespace m {}
   }
+  inline int Foo = 5;
 functionDecl(isInline()) will match ::f().
 namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
 
 
 
@@ -4706,8 +4708,10 @@
   namespace n {
   inline namespace m {}
   }
+  inline int Foo = 5;
 functionDecl(isInline()) will match ::f().
 namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
 
 
 
@@ -5728,6 +5732,23 @@
 
 
 
+MatcherVarDecl>isInline
+Matches function and 
namespace declarations that are marked with
+the inline keyword.
+
+Given
+  inline void f();
+  void g();
+  namespace n {
+  inline namespace m {}
+  }
+  inline int Foo = 5;
+functionDecl(isInline()) will match ::f().
+namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
+
+
+
 MatcherVarDecl>isStaticLocal
 Matches a static 
variable with local scope.
 


Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -197,6 +197,8 @@
   functionDecl(isInline(), hasName("f";
   EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
   namespaceDecl(isInline(), hasName("m";
+  EXPECT_TRUE(matches("inline int Foo = 5;",
+  varDecl(isInline(), hasName("Foo")), {Lang_CXX17}));
 }
 
 // FIXME: Figure out how to specify paths so the following tests pass on
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7683,18 +7683,22 @@
 ///   namespace n {
 ///   inline namespace m {}
 ///   }
+///   inline int Foo = 5;
 /// \endcode
 /// functionDecl(isInline()) will match ::f().
 /// namespaceDecl(isInline()) will match n::m.
-AST_POLYMORPHIC_MATCHER(isInline,
-AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceD

[PATCH] D117929: [XRay] Add support for RISCV

[Ashwin Poduval via Phabricator via cfe-commits]

ashwin98 updated this revision to Diff 405621.
ashwin98 added a comment.

Removed extra triples from the test.


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

https://reviews.llvm.org/D117929

Files:
  clang/lib/Driver/XRayArgs.cpp
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,56 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,11 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
@@ -65,11 +68,18 @@
 return LowerRISCVMachineOperandToMCOperand(MO, MCOp, *this);
   }
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 };
 }
 
@@ -92,6 +102,30 @@
   if (emitPseudoExpansionLowering(*OutStreamer, MI))
 return;
 
+  switch (MI->getOpcode()) {
+  case 

[PATCH] D118876: [HIPSPV] Fix literals are mapped to Generic address space

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118876

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

[PATCH] D118904: [clang][CodeGen] Use memory type representation in `va_arg`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: t.p.northover.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some types (e.g. `_Bool`) have different scalar and memory representations. 
CodeGen for `va_arg` didn't take this into account, leading to an assertion 
failures with different types.

This patch makes sure we use memory representation for `va_arg`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118904

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm64-arguments.c


Index: clang/test/CodeGen/arm64-arguments.c
===
--- clang/test/CodeGen/arm64-arguments.c
+++ clang/test/CodeGen/arm64-arguments.c
@@ -196,6 +196,15 @@
 __builtin_va_end(ap);
 return ll;
 }
+_Bool t3(int i, ...) {
+  // CHECK: t3
+  __builtin_va_list ap;
+  __builtin_va_start(ap, i);
+  // TODO: Add proper checks here.
+  _Bool b = __builtin_va_arg(ap, _Bool);
+  __builtin_va_end(ap);
+  return b;
+}
 
 #include 
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -709,7 +709,8 @@
"Unexpected CoerceToType seen in arginfo in generic VAArg 
emitter!");
 
 Address Temp = CGF.CreateMemTemp(Ty, "varet");
-Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(), 
CGF.ConvertType(Ty));
+Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(),
+  CGF.ConvertTypeForMem(Ty));
 CGF.Builder.CreateStore(Val, Temp);
 return Temp;
   }


Index: clang/test/CodeGen/arm64-arguments.c
===
--- clang/test/CodeGen/arm64-arguments.c
+++ clang/test/CodeGen/arm64-arguments.c
@@ -196,6 +196,15 @@
 __builtin_va_end(ap);
 return ll;
 }
+_Bool t3(int i, ...) {
+  // CHECK: t3
+  __builtin_va_list ap;
+  __builtin_va_start(ap, i);
+  // TODO: Add proper checks here.
+  _Bool b = __builtin_va_arg(ap, _Bool);
+  __builtin_va_end(ap);
+  return b;
+}
 
 #include 
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -709,7 +709,8 @@
"Unexpected CoerceToType seen in arginfo in generic VAArg emitter!");
 
 Address Temp = CGF.CreateMemTemp(Ty, "varet");
-Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(), CGF.ConvertType(Ty));
+Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(),
+  CGF.ConvertTypeForMem(Ty));
 CGF.Builder.CreateStore(Val, Temp);
 return Temp;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118904: [clang][CodeGen] Use memory type representation in `va_arg`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Should we add tests for other targets besides arm64?




Comment at: clang/test/CodeGen/arm64-arguments.c:203
+  __builtin_va_start(ap, i);
+  // TODO: Add proper checks here.
+  _Bool b = __builtin_va_arg(ap, _Bool);

Not sure what to check here. Currently, we're just verifying the compiler 
doesn't crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118904

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

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 405636.
njames93 edited the summary of this revision.
njames93 added a comment.

Ignore the CTCrashTestTrace implementation file in the lit tests, Fixes 'Test 
has no 'RUN'' error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118520

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
  clang-tools-extra/test/lit.cfg.py

Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -48,7 +48,7 @@
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
 # from test suites; there won't be any lit tests within them.
-config.excludes = ['Inputs']
+config.excludes = ['Inputs', 'CTCrashTestTrace.cpp']
 
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
Index: clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
@@ -0,0 +1,17 @@
+// REQUIRES: plugins, backtraces, crash-recovery
+// RUN: not --crash clang-tidy %s -checks='-*,crash-test' -load   \
+// RUN:   %llvmshlibdir/CTCrashTestTrace%pluginext 2>&1 | FileCheck %s
+
+void foo() {
+  for (int I = 0; I < 5; ++I) {
+  }
+}
+// CHECK: Processing check crash-test
+// CHECK-NEXT: --- Bound Nodes Begin ---
+// CHECK-DAG: FS - { ForStmt : <{{.*}}/crash-trace.c:6:3, line:7:3> }
+// CHECK-DAG: DS - { DeclStmt : <{{.*}}/crash-trace.c:6:8, col:17> }
+// CHECK-DAG: VD - { VarDecl I : <{{.*}}/crash-trace.c:6:8, col:16> }
+// CHECK-DAG: IL - { IntegerLiteral : <{{.*}}/crash-trace.c:6:16> }
+// CHECK-DAG: QT - { QualType : int }
+// CHECK-DAG: T - { BuiltinType : int }
+// CHECK-NEXT: --- Bound Nodes End ---
Index: clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
@@ -0,0 +1,69 @@
+//=== CTCrashTestTrace.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+///  \file This file implements a clang-tidy plugin that registers one check
+///  designed to crash on any match. This is to test clang-tidys stack trace
+///  output for when a buggy check causes a crash.
+///
+//===--===//
+
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class CrashingCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(
+forStmt(
+hasLoopInit(
+declStmt(hasSingleDecl(varDecl(hasType(qualType().bind("QT")),
+   hasType(type().bind("T")),
+   hasInitializer(
+   integerLiteral().bind("IL")))
+   .bind("VD")))
+.bind("DS")))
+.bind("FS"),
+this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+LLVM_BUILTIN_TRAP;
+  }
+
+  llvm::Optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+class CTCrashTestTraceModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck("crash-test");
+  }
+};
+} // namespace
+
+namespace tidy {
+// Register the CTCrashTestTraceModule using this statically initialized
+// variable.
+static ClangTidyModuleRegistry::Add<::CTCrashTestTraceModule>
+X("crash-module", "Add a check which will trap on any match");
+} // namespace tidy
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus regis

[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added inline comments.



Comment at: clang/lib/AST/Expr.cpp:1949-1950
 
 if (E->getCastKind() == CK_ConstructorConversion)
   return cast(SubExpr)->getConstructor();
 

kimgr wrote:
> davrec wrote:
> > I think the errors prove we should fall back to the most conservative 
> > possible fix: remove all the other changes and just change these 2 lines to:
> > ```
> > if (E->getCastKind() == CK_ConstructorConversion) {
> >   if (auto *CE = dyn_cast(SubExpr)
> > SubExpr = CE->getSubExpr();
> >   return cast(SubExpr)->getConstructor();
> > }
> > ```
> > I'm can't quite remember but I believe that would solve the bug as narrowly 
> > as possible.  @kimgr can you give it a try if and see if it avoids the 
> > errors?
> > (If it doesn't, I'm stumped...)
> Yes, it does. I needed to add the same `ConstantExpr` skipping to the 
> user-defined conversion handling below, but once those two were in place the 
> new unittests succeed and the existing lit tests also.
> 
> It does feel a little awkward, though... I wish I had a clearer mental model 
> of how the implicit-skipping is supposed to work. My intuition is telling me 
> `skipImplicitTemporary` should probably disappear and we should use the 
> `IgnoreExpr.h` facilities directly instead, but it's all a little fuzzy to me 
> at this point.
> 
> I'll run the full `check-clang` suite overnight with this alternative patch, 
> I have no reason to think there will be problems, but I'll make sure in the 
> morning.
> 
> Thanks!
I can confirm that full `check-clang` also works with the narrower fix. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

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

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, I think I understand it now! Sorry for so many comments, happy to chat 
offline about which ones to address/where to start.




Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:39
+public:
+  struct Action {
+enum class Kind : uint8_t {

API polish: this class is a mix of accessors and direct member state - it can 
be hard to know how to best interact with such a thing.

I'd suggest:
 - public factories `static Action Action::reduce(RuleID)` etc, private data & 
constructor
 - expose `Kind kind()`, but not the individual is() methods: they don't buy 
much and this pattern encourages switch where appropriate
 - make `Kind` enum rather than `enum class` - `Action::Shift` is sufficiently 
scoped already
 - shift() & goTo() -> `targetState()` which asserts either condition. Apart 
from anything else, this makes naming easier: "shift" is a verb so `shift()` is 
a confusing name.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:39
+public:
+  struct Action {
+enum class Kind : uint8_t {

sammccall wrote:
> API polish: this class is a mix of accessors and direct member state - it can 
> be hard to know how to best interact with such a thing.
> 
> I'd suggest:
>  - public factories `static Action Action::reduce(RuleID)` etc, private data 
> & constructor
>  - expose `Kind kind()`, but not the individual is() methods: they don't buy 
> much and this pattern encourages switch where appropriate
>  - make `Kind` enum rather than `enum class` - `Action::Shift` is 
> sufficiently scoped already
>  - shift() & goTo() -> `targetState()` which asserts either condition. Apart 
> from anything else, this makes naming easier: "shift" is a verb so `shift()` 
> is a confusing name.
mention that this combines the Action and Go-to tables from LR literature?



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:41
+enum class Kind : uint8_t {
+  // Action table
+  Error = 0,

I think it's worth documenting what each of these actions means.

Referring to enum values as "action table" and "goto table" seems confusing - 
they're part of the same table! The main reference point when reading the code 
needs to be the code, rather than the textbook.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:67
+
+bool operator==(const Action &L) const {
+  if (K != L.K)

if you make the data members private the union could just be a uint16 and save 
yourself some trouble here :-) Up to you



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:84
+
+Kind K = Kind::Error;
+// Value

This action struct can be squeezed to 16 bits.

Kind only needs 3 bits (I suspect it can be 2: Error and Accept aren't heavily 
used).
RuleID is 12 bits, StateID would fit within 11 and 12 should be safe.

Combined with the optimization suggested below, this should get the whole table 
down to  335KB.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:102
+  // Returns the transisted state for the state From on a nonterminal.
+  StateID goTo(StateID From, SymbolID NonTerminal) const {
+assert(pseudo::isNonterminal(NonTerminal));

nit: please be consistent with `Nonterminal` (and "nonterminal") vs 
`NonTerminal` (and "non-terminal"). I think `Nonterminal` is used elsewhere in 
the code.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:105
+auto Goto = find({From, NonTerminal});
+assert(Goto.size() == 1 && Goto.front().isGoTo());
+return Goto.front().goTo();

Why is this assertion valid? Is it a precondition of the function that some 
item in From can advance over NonTerminal?



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:121
+private:
+  struct IndexKey {
+StateID From;

I think there's an even more compact representation:
Basically sort the index keys as symbol-major, but then don't store the actual 
symbol there but rather store the range for the symbol in an external array:

```
// index is lookahead tok::TokenKind. Values are indices into Index: the range 
for tok is StateIdx[TokenIdx[tok]...TokenIdx[tok+1]]
vector TokenIdx;

// index is nonterminal SymbolID. Values are  indices into Index: the range for 
sym is StateIdx[NontermIdx[sym]...NontermIdx[sym+1]].
vector NontermIdx;

// parallel to actions, value is the from state. Only subranges are sorted.
vector StateIdx;

vector Actions;
```

This should be 4 * (392 + 245) + (2 + 4) * 83k ~= 500k. And the one extra 
indirection should save 5-10 steps of binary search.

This is equivalent to 2 * `vector` + `vector>` 
but keeping the searched index compact is probably a good thing.



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRBuilder.

[clang] b5787a0 - [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-02-03T16:37:52+01:00
New Revision: b5787a0c6cc4da47b7d7b218e23f780076ad2f5f

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

LOG: [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

The WebAssembly toolchain currently supports only -stdlib=libc++
and implicitly assumes the c++ stdlib to be libc++. Change this to also
support libstdc++.

Differential Revision: https://reviews.llvm.org/D117888#3290628

Added: 


Modified: 
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/lib/Driver/ToolChains/WebAssembly.h
clang/test/Driver/wasm-toolchain.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 3614272a5f747..5f64792b45014 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,15 +8,17 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "Gnu.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Option/ArgList.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -371,7 +373,11 @@ ToolChain::CXXStdlibType
 WebAssembly::GetCXXStdlibType(const ArgList &Args) const {
   if (Arg *A = Args.getLastArg(options::OPT_stdlib_EQ)) {
 StringRef Value = A->getValue();
-if (Value != "libc++")
+if (Value == "libc++")
+  return ToolChain::CST_Libcxx;
+else if (Value == "libstdc++")
+  return ToolChain::CST_Libstdcxx;
+else
   getDriver().Diag(diag::err_drv_invalid_stdlib_name)
   << A->getAsString(Args);
   }
@@ -417,17 +423,20 @@ void WebAssembly::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
 
 void WebAssembly::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {
-  if (!DriverArgs.hasArg(options::OPT_nostdlibinc) &&
-  !DriverArgs.hasArg(options::OPT_nostdincxx)) {
-if (getTriple().getOS() != llvm::Triple::UnknownOS) {
-  const std::string MultiarchTriple =
-  getMultiarchTriple(getDriver(), getTriple(), getDriver().SysRoot);
-  addSystemInclude(DriverArgs, CC1Args,
-   getDriver().SysRoot + "/include/" + MultiarchTriple +
-   "/c++/v1");
-}
-addSystemInclude(DriverArgs, CC1Args,
- getDriver().SysRoot + "/include/c++/v1");
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
+  DriverArgs.hasArg(options::OPT_nostdincxx))
+return;
+
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx:
+addLibCxxIncludePaths(DriverArgs, CC1Args);
+break;
+  case ToolChain::CST_Libstdcxx:
+addLibStdCXXIncludePaths(DriverArgs, CC1Args);
+break;
+  default:
+llvm_unreachable("Unknown stdlib type");
   }
 }
 
@@ -440,6 +449,9 @@ void WebAssembly::AddCXXStdlibLibArgs(const 
llvm::opt::ArgList &Args,
 CmdArgs.push_back("-lc++abi");
 break;
   case ToolChain::CST_Libstdcxx:
+CmdArgs.push_back("-lstdc++");
+break;
+  default:
 llvm_unreachable("invalid stdlib name");
   }
 }
@@ -455,3 +467,75 @@ SanitizerMask WebAssembly::getSupportedSanitizers() const {
 Tool *WebAssembly::buildLinker() const {
   return new tools::wasm::Linker(*this);
 }
+
+void WebAssembly::addLibCxxIncludePaths(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+  std::string SysRoot = computeSysRoot();
+  std::string LibPath = SysRoot + "/include";
+  const std::string MultiarchTriple =
+  getMultiarchTriple(D, getTriple(), SysRoot);
+  bool IsKnownOs = (getTriple().getOS() != llvm::Triple::UnknownOS);
+
+  std::string Version = detectLibcxxVersion(LibPath);
+  if (Version.empty())
+return;
+
+  // First add the per-target include path if the OS is known.
+  if (IsKnownOs) {
+std::string TargetDir = LibPath + "/" + MultiarchTriple + "/c++/" + 
Version;
+addSystemInclude(DriverArgs, CC1Args, TargetDir);
+  }
+
+  // Second add the generic one.
+  addSystemInclude(DriverArgs, CC1Args, LibPath + "/c++/" + Version);
+}
+
+void WebAssembly::addLibStdCXXIncludePaths(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  // We cannot use GCCInstallationDetector here as the sysroot usually does
+  // not contain a full GCC installation.
+  // Instead, we search the given sysroot for /usr

[PATCH] D117888: [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

[Timm Bäder 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 rGb5787a0c6cc4: [clang][driver][wasm] Support 
-stdlib=libstdc++ for WebAssembly (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117888

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/wasm-toolchain.cpp

Index: clang/test/Driver/wasm-toolchain.cpp
===
--- clang/test/Driver/wasm-toolchain.cpp
+++ clang/test/Driver/wasm-toolchain.cpp
@@ -19,6 +19,11 @@
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=libstdc++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_STDCXX %s
+// LINK_STDCXX: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_STDCXX: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lstdc++" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C++ link command-line with optimization with unknown OS.
 
 // RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=libc++ 2>&1 \
@@ -26,6 +31,11 @@
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=libstdc++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_STDCXX %s
+// LINK_OPT_STDCXX: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_STDCXX: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lstdc++" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C++ link command-line with known OS.
 
 // RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=libc++ %s 2>&1 \
@@ -33,6 +43,11 @@
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=libstdc++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN_STDCXX %s
+// LINK_KNOWN_STDCXX: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_KNOWN_STDCXX: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lstdc++" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C++ link command-line with optimization with known OS.
 
 // RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=libc++ 2>&1 \
@@ -40,15 +55,33 @@
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=libstdc++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN_STDCXX %s
+// LINK_OPT_KNOWN_STDCXX: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN_STDCXX: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lstdc++" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=libc++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --stdlib=libc++ %s 2>&1 \
+// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree/usr \
 // RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1"
 // COMPILE: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"
-// COMPILE: "-isysroot" "/foo"
-// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1"
-// COMPILE: "-internal-isystem" "/foo/include/c++/v1"
+// COMPILE: "-isysroot" "[[SYSROOT:[^"]+]]"
+// COMPILE: "-internal-isystem" "[[SYSROOT:[^"]+]]/include/wasm32-wasi/c++/v1"
+// COMPILE: "-internal-isystem" "[[SYSROOT:[^"]+]]/include/c++/v1"
 // COMPILE: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
-// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi"
-// COMPILE: "-internal-isystem" "/foo/include"
+// COMPILE: "-internal-isystem" "[[SYSROOT:[^"]+]]/include/wasm32-wasi"
+// COMPILE: "-internal-isystem" "[[SYSROOT:[^"]+]]/include"
+
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --stdlib=libstdc++ %s 2>&1 \
+// RUN:  

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Is there any background information anywhere what this pseudo parser is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt:3
+
+add_clang_library(clangSyntaxPseudo
+  Grammar.cpp

(i think the usual name would be "clangToolingSyntaxPseudo")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D118904: [clang][CodeGen] Use memory type representation in `va_arg`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 405644.
jansvoboda11 added a comment.

Add proper check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118904

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm64-arguments.c


Index: clang/test/CodeGen/arm64-arguments.c
===
--- clang/test/CodeGen/arm64-arguments.c
+++ clang/test/CodeGen/arm64-arguments.c
@@ -196,6 +196,15 @@
 __builtin_va_end(ap);
 return ll;
 }
+_Bool t3(int i, ...) {
+  // CHECK: t3
+  __builtin_va_list ap;
+  __builtin_va_start(ap, i);
+  // CHECK: va_arg {{.*}}* %ap, i8
+  _Bool b = __builtin_va_arg(ap, _Bool);
+  __builtin_va_end(ap);
+  return b;
+}
 
 #include 
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -709,7 +709,8 @@
"Unexpected CoerceToType seen in arginfo in generic VAArg 
emitter!");
 
 Address Temp = CGF.CreateMemTemp(Ty, "varet");
-Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(), 
CGF.ConvertType(Ty));
+Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(),
+  CGF.ConvertTypeForMem(Ty));
 CGF.Builder.CreateStore(Val, Temp);
 return Temp;
   }


Index: clang/test/CodeGen/arm64-arguments.c
===
--- clang/test/CodeGen/arm64-arguments.c
+++ clang/test/CodeGen/arm64-arguments.c
@@ -196,6 +196,15 @@
 __builtin_va_end(ap);
 return ll;
 }
+_Bool t3(int i, ...) {
+  // CHECK: t3
+  __builtin_va_list ap;
+  __builtin_va_start(ap, i);
+  // CHECK: va_arg {{.*}}* %ap, i8
+  _Bool b = __builtin_va_arg(ap, _Bool);
+  __builtin_va_end(ap);
+  return b;
+}
 
 #include 
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -709,7 +709,8 @@
"Unexpected CoerceToType seen in arginfo in generic VAArg emitter!");
 
 Address Temp = CGF.CreateMemTemp(Ty, "varet");
-Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(), CGF.ConvertType(Ty));
+Val = CGF.Builder.CreateVAArg(VAListAddr.getPointer(),
+  CGF.ConvertTypeForMem(Ty));
 CGF.Builder.CreateStore(Val, Temp);
 return Temp;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2dd35e9 - [clang][driver][wasm] Remove unneeded default labels

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-02-03T16:52:41+01:00
New Revision: 2dd35e98d3ffa1327df8261077958b47b2bdbb15

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

LOG: [clang][driver][wasm] Remove unneeded default labels

Fix build fallout from b5787a0c6cc4da47b7d7b218e23f780076ad2f5f

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 5f64792b45014..4fdfb7e1762e0 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -435,8 +435,6 @@ void WebAssembly::AddClangCXXStdlibIncludeArgs(const 
ArgList &DriverArgs,
   case ToolChain::CST_Libstdcxx:
 addLibStdCXXIncludePaths(DriverArgs, CC1Args);
 break;
-  default:
-llvm_unreachable("Unknown stdlib type");
   }
 }
 
@@ -451,8 +449,6 @@ void WebAssembly::AddCXXStdlibLibArgs(const 
llvm::opt::ArgList &Args,
   case ToolChain::CST_Libstdcxx:
 CmdArgs.push_back("-lstdc++");
 break;
-  default:
-llvm_unreachable("invalid stdlib name");
   }
 }
 



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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D114790#3293764 , @thakis wrote:

> Is there any background information anywhere what this pseudo parser is?

Design doc: 
https://docs.google.com/document/d/1eGkTOsFja63wsv8v0vd5JdoTonj-NlN3ujGF0T7xDbM/edit
RFC: https://discourse.llvm.org/t/rfc-a-c-pseudo-parser-for-tooling/59217


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt:6
+add_clang_unittest(ClangPseudoTests
+  GrammarTests.cpp
+)

```
thakis@thakis:~/src/llvm-project$ ls clang/unittests/**/*Test.cpp | wc -l
127
thakis@thakis:~/src/llvm-project$ ls clang/unittests/**/*Tests.cpp | wc -l
1
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D114790#3293777 , @sammccall wrote:

> In D114790#3293764 , @thakis wrote:
>
>> Is there any background information anywhere what this pseudo parser is?
>
> Design doc: 
> https://docs.google.com/document/d/1eGkTOsFja63wsv8v0vd5JdoTonj-NlN3ujGF0T7xDbM/edit
> RFC: https://discourse.llvm.org/t/rfc-a-c-pseudo-parser-for-tooling/59217

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 405645.
tianshilei1992 added a comment.

Fix template instantiation and add one test.
More tests are on the way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116637

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -5,6 +5,8 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify=omp51 -fopenmp -fopenmp-version=51 -ast-print %s | FileCheck --check-prefixes=CHECK,CHECK-51 %s
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -12,6 +14,7 @@
 
 template 
 T foo(T argc) {
+  T c = T();
   T b = T();
   T a = T();
 #pragma omp atomic
@@ -29,6 +32,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic seq_cst
   a++;
 #pragma omp atomic read seq_cst
@@ -44,6 +55,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare seq_cst // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic seq_cst compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare seq_cst // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read
@@ -59,6 +78,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare acq_rel // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic acq_rel compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare acq_rel // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic
   a++;
 #pragma omp atomic read acquire
@@ -74,6 +101,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare acquire // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic acquire compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare acquire // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic release
   a++;
 #pragma omp atomic read
@@ -89,6 +124,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare release // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic release compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a < b ? b : a; }
+#pragma omp atomic compare release // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a == b ? c : a; }
+#endif
 #pragma omp atomic relaxed
   a++;
 #pragma omp atomic read
@@ -104,6 +147,14 @@
 a = b;
 b++;
   }
+#if defined(_OPENMP) && _OPENMP >= 202011
+#pragma omp atomic compare relaxed // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic compare is not supported for now}}
+  { a = a > b ? b : a; }
+#pragma omp atomic relaxed compare // omp51-error {{atomic compare is not supported for now}} omp51-error {{atomic com

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D114790#3293783 , @thakis wrote:

> In D114790#3293777 , @sammccall 
> wrote:
>
>> In D114790#3293764 , @thakis wrote:
>>
>>> Is there any background information anywhere what this pseudo parser is?
>>
>> Design doc: 
>> https://docs.google.com/document/d/1eGkTOsFja63wsv8v0vd5JdoTonj-NlN3ujGF0T7xDbM/edit
>> RFC: https://discourse.llvm.org/t/rfc-a-c-pseudo-parser-for-tooling/59217
>
> Thanks!

Sure - I'll add a readme/links in this directory too, just forgot this was the 
first commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D118911: [clang-format] regression from clang-format v13

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/53567

The following source

  namespace A {
  
  template  struct Foo {
void foo() { std::cout << "Bar"; }
  }; // namespace A

is incorrectly formatted as:

  namespace A {
  
  template  struct Foo{void foo(){std::cout << "Bar";
  }
  }
  ; // namespace A

This looks to be caused by 
https://github.com/llvm/llvm-project/commit/5c2e7c9ca043d92bed75b08e653fb47c384edd13


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118911

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23638,6 +23638,7 @@
   verifyFormat("struct Y<[] { return 0; }> {};", Style);
 
   verifyFormat("struct Z : X {};", Style);
+  verifyFormat("template  struct Foo {};", Style);
 }
 
 TEST_F(FormatTest, RemoveBraces) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3080,7 +3080,8 @@
   }
   if (FormatTok->is(tok::l_square)) {
 FormatToken *Previous = FormatTok->Previous;
-if (!Previous || Previous->isNot(tok::r_paren)) {
+if (!Previous || (Previous->isNot(tok::r_paren) &&
+  !Previous->isTypeOrIdentifier())) {
   // Don't try parsing a lambda if we had a closing parenthesis before,
   // it was probably a pointer to an array: int (*)[].
   if (!tryToParseLambda())


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23638,6 +23638,7 @@
   verifyFormat("struct Y<[] { return 0; }> {};", Style);
 
   verifyFormat("struct Z : X {};", Style);
+  verifyFormat("template  struct Foo {};", Style);
 }
 
 TEST_F(FormatTest, RemoveBraces) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3080,7 +3080,8 @@
   }
   if (FormatTok->is(tok::l_square)) {
 FormatToken *Previous = FormatTok->Previous;
-if (!Previous || Previous->isNot(tok::r_paren)) {
+if (!Previous || (Previous->isNot(tok::r_paren) &&
+  !Previous->isTypeOrIdentifier())) {
   // Don't try parsing a lambda if we had a closing parenthesis before,
   // it was probably a pointer to an array: int (*)[].
   if (!tryToParseLambda())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117888: [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

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

tbaeder added a comment.

In D117888#3290628 , @sbc100 wrote:

> Great!   Thanks for working on this.I'm curious what toolchain you are 
> working that uses GNU libstdc++?

I'm looking at libstdc++ enablement in a wasm/emscripten toolchain, so nothing 
existing/public.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117888

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

[PATCH] D118815: [Clang][Docs] Add documention for new OpenMP offloading driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 405654.
jhuber6 added a comment.

Some additions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118815

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ClangLinkerWrapper.rst
  clang/docs/OffloadingDesign.rst
  clang/docs/OpenMPSupport.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -83,6 +83,7 @@
ClangFormatStyleOptions
ClangFormattedStatus
ClangNvlinkWrapper
+   ClangLinkerWrapper
ClangOffloadBundler
ClangOffloadWrapper
 
@@ -94,6 +95,7 @@
 
InternalsManual
DriverInternals
+   OffloadingDesign
PCHInternals
ItaniumMangleAbiTags
HardwareAssistedAddressSanitizerDesign.rst
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -256,8 +256,18 @@
 OpenMP Support in Clang
 ---
 
-- ``clang-nvlink-wrapper`` tool introduced to support linking of cubin files archived in an archive. See :doc:`ClangNvlinkWrapper`.
-- ``clang-linker-wrapper`` tool introduced to support linking using a new OpenMP target offloading method. See :doc:`ClangLinkerWrapper`.
+- ``clang-nvlink-wrapper`` tool introduced to support linking of cubin files
+  archived in an archive. See :doc:`ClangNvlinkWrapper`.
+- ``clang-linker-wrapper`` tool introduced to support linking using a new OpenMP
+  target offloading method. See :doc:`ClangLinkerWrapper`.
+- Support for a new driver for OpenMP target offloading has been added as an
+  opt-in feature. The new driver can be selected using ``-fopenmp-new-driver``
+  with clang. Device-side LTO can also be enabled using the new driver by
+  passing ``-foffload-lto=`` as well. The new driver supports the following
+  features:
+  - Linking AMDGPU and NVPTX offloading targets.
+  - Static linking using archive files.
+  - Device-side LTO.
 
 CUDA Support in Clang
 -
Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -95,7 +95,8 @@
 
 - Nested parallelism: inner parallel regions are executed sequentially.
 
-- Static linking of libraries containing device code is not supported yet.
+- Static linking of libraries containing device code is not supported without
+  explicitly using ``-fopenmp-new-driver``.
 
 - Automatic translation of math functions in target regions to device-specific
   math functions is not implemented yet.
Index: clang/docs/OffloadingDesign.rst
===
--- /dev/null
+++ clang/docs/OffloadingDesign.rst
@@ -0,0 +1,452 @@
+=
+Offloading Design & Internals
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+This document describes the Clang driver and code generation steps for creating
+offloading applications. Clang supports offloading to various architectures
+using programming models like CUDA, HIP, and OpenMP. The purpose of this
+document is to illustrate the steps necessary to create an offloading
+application using Clang.
+
+OpenMP Offloading
+=
+
+.. note::
+   This documentation describes Clang's behavior using the new offloading driver
+   which. This currently must be enabled manually using ``-fopenmp-new-driver``.
+
+Clang supports OpenMP target offloading to several different architectures such
+as NVPTX, AMDGPU, X86_64. ARM, and PowerPC. Offloading code is generated by
+Clang and then executed using the ``libomptarget`` runtime and the associated
+plugin for the target architecture, e.g. ``libomptarget.rtl.cuda``. This section
+describes the steps necessary to create a functioning device image that can be
+loaded by the OpenMP runtime.  More information on the OpenMP runtimes can be
+found at the `OpenMP documentation `__.
+
+Offloading Overview
+---
+
+The goal of offloading compilation is to create an executable device image that
+can be run on the target device. OpenMP offloading creates executable images by
+compiling the input file for both the host and the target device. The output
+from the device phase then needs to be embedded into the host to create a fat
+object. A special tool then needs to extract the device code from the fat
+objects, run the device linking step, and embed the final device image in a
+symbol the host can use to register the library.
+
+Compilation Process
+^^^
+
+The compiler performs the following high-level actions to generate offloading
+code:
+
+* Compile the input file for the host to produce a bitcode file. Lower ``#pragma
+  omp target`

[PATCH] D118815: [Clang][Docs] Add documention for new OpenMP offloading driver

[Alexey Bataev via Phabricator via cfe-commits]

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

LG with nits




Comment at: clang/docs/OffloadingDesign.rst:25
+Clang supports OpenMP target offloading to several different architectures such
+as NVPTX, AMDGPU, X86_64. ARM, and PowerPC. Offloading code is generated by
+Clang and then executed using the ``libomptarget`` runtime and the associated

`.`->`,` after X86_64


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118815

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

[PATCH] D118913: [clang-tidy] Fix LLVM include order check policy

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: carlosgalvezp, xazax.hun.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Clang-format LLVM style has a custom include category for gtest/ and
gmock/ headers between regular includes and angled includes. Do the same here.

Fixes https://github.com/llvm/llvm-project/issues/53525.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118913

Files:
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,4 +1,7 @@
+#include "ClangTidyOptions.h"
 #include "ClangTidyTest.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
@@ -9,11 +12,15 @@
 namespace tidy {
 namespace test {
 
-static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-   Optional ExpectedWarning) {
+template 
+static std::string runCheck(StringRef Code, const Twine &Filename,
+Optional ExpectedWarning,
+std::map PathsToContent =
+std::map()) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, &Errors, Filename, std::string("-xc++-header"));
+  std::string Result = test::runCheckOnCode(
+  Code, &Errors, Filename, std::string("-xc++-header"), ClangTidyOptions{},
+  std::move(PathsToContent));
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -22,27 +29,36 @@
   return Result;
 }
 
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
+   Optional ExpectedWarning) {
+  return runCheck(Code, Filename,
+std::move(ExpectedWarning));
+}
+
+static std::string
+runIncludeOrderCheck(StringRef Code, const Twine &Filename,
+ Optional ExpectedWarning,
+ llvm::ArrayRef Includes) {
+  std::map PathsToContent;
+  for (auto Include : Includes)
+PathsToContent.emplace(Include, "");
+  return runCheck(Code, Filename, std::move(ExpectedWarning),
+ PathsToContent);
+}
+
 namespace {
 struct WithEndifComment : public LLVMHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
   : LLVMHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
-} // namespace
 
 static std::string
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
  Optional ExpectedWarning) {
-  std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, &Errors, Filename, std::string("-xc++-header"));
-  if (Errors.size() != (size_t)ExpectedWarning.hasValue())
-return "invalid error count";
-  if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
-return "expected: '" + ExpectedWarning->str() + "', saw: '" +
-   Errors.back().Message.Message + "'";
-  return Result;
+  return runCheck(Code, Filename, std::move(ExpectedWarning));
 }
+} // namespace
 
 TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
@@ -270,6 +286,23 @@
 #endif
 }
 
+TEST(IncludeOrderCheck, GTestHeaders) {
+  EXPECT_EQ(
+  R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include "gtest/foo.h"
+  #include )cpp",
+  runIncludeOrderCheck(
+  R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include 
+  #include "gtest/foo.h")cpp",
+  "foo.cc", StringRef("#includes are not sorted properly"),
+  {"foo.h", "algorithm", "gtest/foo.h", "llvm/foo.h"}));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
@@ -6,6 +6,7 @@
 #include "gmock/foo.h"
 #include "i.h"
 #include 
+#include 
 #include "llvm/a.h"
 #include "clang/b.h"
 #include "clang-c/c.h" // hi
@@ -19,6 +20,7 @@
 // CHECK-FIXES-NEXT: #include "llvm/a.h"
 // CHECK-FIXES-NEXT: #include "gmock/foo.h"
 // CHECK-FIXES-NEXT: #include "gtest/foo.h"
+// CHECK-

[PATCH] D118913: [clang-tidy] Fix LLVM include order check policy

[Sam McCall via Phabricator via cfe-commits]

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

Thank you!
Seems strange to me that we have both forms of test here, but I'm sure there's 
a reason...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118913

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

[clang-tools-extra] 0447ec2 - [clang-tidy] Fix LLVM include order check policy

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-02-03T17:32:43+01:00
New Revision: 0447ec2fb050eb37ed1f5991a1562dea6e228f9e

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

LOG: [clang-tidy] Fix LLVM include order check policy

Clang-format LLVM style has a custom include category for gtest/ and
gmock/ headers between regular includes and angled includes. Do the same here.

Fixes https://github.com/llvm/llvm-project/issues/53525.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp 
b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
index c962fb3bc25b2..ab75e3100d85d 100644
--- a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -66,11 +66,15 @@ static int getPriority(StringRef Filename, bool IsAngled, 
bool IsMainModule) {
   Filename.startswith("clang/") || Filename.startswith("clang-c/"))
 return 2;
 
-  // System headers are sorted to the end.
-  if (IsAngled || Filename.startswith("gtest/") ||
-  Filename.startswith("gmock/"))
+  // Put these between system and llvm headers to be consistent with LLVM
+  // clang-format style.
+  if (Filename.startswith("gtest/") || Filename.startswith("gmock/"))
 return 3;
 
+  // System headers are sorted to the end.
+  if (IsAngled)
+return 4;
+
   // Other headers are inserted between the main module header and LLVM 
headers.
   return 1;
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
index 6dd3d6ace4581..d4da826ced386 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
@@ -6,6 +6,7 @@
 #include "gmock/foo.h"
 #include "i.h"
 #include 
+#include 
 #include "llvm/a.h"
 #include "clang/b.h"
 #include "clang-c/c.h" // hi
@@ -19,6 +20,7 @@
 // CHECK-FIXES-NEXT: #include "llvm/a.h"
 // CHECK-FIXES-NEXT: #include "gmock/foo.h"
 // CHECK-FIXES-NEXT: #include "gtest/foo.h"
+// CHECK-FIXES-NEXT: #include 
 // CHECK-FIXES-NEXT: #include 
 
 #include "b.h"

diff  --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
index 2a3fae05f585f..46a1600e2bd6d 100644
--- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,4 +1,7 @@
+#include "ClangTidyOptions.h"
 #include "ClangTidyTest.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
@@ -9,11 +12,15 @@ namespace clang {
 namespace tidy {
 namespace test {
 
-static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-   Optional ExpectedWarning) {
+template 
+static std::string runCheck(StringRef Code, const Twine &Filename,
+Optional ExpectedWarning,
+std::map PathsToContent =
+std::map()) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, &Errors, Filename, std::string("-xc++-header"));
+  std::string Result = test::runCheckOnCode(
+  Code, &Errors, Filename, std::string("-xc++-header"), ClangTidyOptions{},
+  std::move(PathsToContent));
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -22,27 +29,36 @@ static std::string runHeaderGuardCheck(StringRef Code, 
const Twine &Filename,
   return Result;
 }
 
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
+   Optional ExpectedWarning) {
+  return runCheck(Code, Filename,
+std::move(ExpectedWarning));
+}
+
+static std::string
+runIncludeOrderCheck(StringRef Code, const Twine &Filename,
+ Optional ExpectedWarning,
+ llvm::ArrayRef Includes) {
+  std::map PathsToContent;
+  for (auto Include : Includes)
+PathsToContent.emplace(Include, "");
+  return runCheck(Code, Filename, 
std::move(ExpectedWarning),
+ PathsToContent);
+}
+
 namespace {
 struct WithEndifComment : public LLVMHeaderGuardCheck {
   WithEndifComment(StringRef Name, Cl

[PATCH] D118913: [clang-tidy] Fix LLVM include order check policy

[Kadir Cetinkaya 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 rG0447ec2fb050: [clang-tidy] Fix LLVM include order check 
policy (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118913

Files:
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,4 +1,7 @@
+#include "ClangTidyOptions.h"
 #include "ClangTidyTest.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
@@ -9,11 +12,15 @@
 namespace tidy {
 namespace test {
 
-static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-   Optional ExpectedWarning) {
+template 
+static std::string runCheck(StringRef Code, const Twine &Filename,
+Optional ExpectedWarning,
+std::map PathsToContent =
+std::map()) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, &Errors, Filename, std::string("-xc++-header"));
+  std::string Result = test::runCheckOnCode(
+  Code, &Errors, Filename, std::string("-xc++-header"), ClangTidyOptions{},
+  std::move(PathsToContent));
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -22,27 +29,36 @@
   return Result;
 }
 
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
+   Optional ExpectedWarning) {
+  return runCheck(Code, Filename,
+std::move(ExpectedWarning));
+}
+
+static std::string
+runIncludeOrderCheck(StringRef Code, const Twine &Filename,
+ Optional ExpectedWarning,
+ llvm::ArrayRef Includes) {
+  std::map PathsToContent;
+  for (auto Include : Includes)
+PathsToContent.emplace(Include, "");
+  return runCheck(Code, Filename, std::move(ExpectedWarning),
+ PathsToContent);
+}
+
 namespace {
 struct WithEndifComment : public LLVMHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
   : LLVMHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
-} // namespace
 
 static std::string
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
  Optional ExpectedWarning) {
-  std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, &Errors, Filename, std::string("-xc++-header"));
-  if (Errors.size() != (size_t)ExpectedWarning.hasValue())
-return "invalid error count";
-  if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
-return "expected: '" + ExpectedWarning->str() + "', saw: '" +
-   Errors.back().Message.Message + "'";
-  return Result;
+  return runCheck(Code, Filename, std::move(ExpectedWarning));
 }
+} // namespace
 
 TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
@@ -270,6 +286,23 @@
 #endif
 }
 
+TEST(IncludeOrderCheck, GTestHeaders) {
+  EXPECT_EQ(
+  R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include "gtest/foo.h"
+  #include )cpp",
+  runIncludeOrderCheck(
+  R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include 
+  #include "gtest/foo.h")cpp",
+  "foo.cc", StringRef("#includes are not sorted properly"),
+  {"foo.h", "algorithm", "gtest/foo.h", "llvm/foo.h"}));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
@@ -6,6 +6,7 @@
 #include "gmock/foo.h"
 #include "i.h"
 #include 
+#include 
 #include "llvm/a.h"
 #include "clang/b.h"
 #include "clang-c/c.h" // hi
@@ -19,6 +20,7 @@
 // CHECK-FIXES-NEXT: #include "llvm/a.h"
 // CHECK-FIXES-NEXT: #include "gmock/foo.h"
 // CHECK-FIXES-NEXT: #include "gtest/foo.h"
+// CHECK-FIXES-NEXT: #include 
 // CHECK-FIXES-NEXT: #include 
 
 #include "b.h"
Index: clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
==

[PATCH] D118911: [clang-format] regression from clang-format v13

[Marek Kurdej via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3084
+if (!Previous || (Previous->isNot(tok::r_paren) &&
+  !Previous->isTypeOrIdentifier())) {
   // Don't try parsing a lambda if we had a closing parenthesis before,

Maybe it would be easier to read if written `!(is(tok::r_paren) || 
isTypeOrIdentifier())`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118911

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

[PATCH] D118915: [clang][deps] Generate '-fmodule-file=' only for direct dependencies

[Jan Svoboda via Phabricator via cfe-commits]

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

The `clang-scan-deps` tool currently generates `-fmodule-file=` command-line 
arguments for the whole transitive closure of modular dependencies. This is not 
necessary, we only need to provide the direct dependencies on the command line. 
Information about transitive dependencies is stored within the `.pcm` files of 
direct dependencies. This makes the command lines shorter, but should be a NFC 
otherwise (unless there are bugs in the loading mechanism for explicit modules).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118915

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/ClangScanDeps/modules-pch.c
  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
@@ -296,14 +296,11 @@
   Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
 }
 
-ID.AdditionalCommandLine =
-GenerateModulesPathArgs
-? FD.getAdditionalArgs(
-  [&](ModuleID MID) { return lookupPCMPath(MID); },
-  [&](ModuleID MID) -> const ModuleDeps & {
-return lookupModuleDeps(MID);
-  })
-: FD.getAdditionalArgsWithoutModulePaths();
+ID.AdditionalCommandLine = GenerateModulesPathArgs
+   ? FD.getAdditionalArgs([&](ModuleID MID) {
+   return lookupPCMPath(MID);
+ })
+   : FD.getAdditionalArgsWithoutModulePaths();
 
 Inputs.push_back(std::move(ID));
   }
@@ -337,10 +334,7 @@
   {"command-line",
GenerateModulesPathArgs
? MD.getCanonicalCommandLine(
- [&](ModuleID MID) { return lookupPCMPath(MID); },
- [&](ModuleID MID) -> const ModuleDeps & {
-   return lookupModuleDeps(MID);
- })
+ [&](ModuleID MID) { return lookupPCMPath(MID); })
: MD.getCanonicalCommandLineWithoutModulePaths()},
   };
   OutModules.push_back(std::move(O));
@@ -370,12 +364,16 @@
   StringRef lookupPCMPath(ModuleID MID) {
 auto PCMPath = PCMPaths.insert({MID, ""});
 if (PCMPath.second)
-  PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID));
+  PCMPath.first->second = constructPCMPath(MID);
 return PCMPath.first->second;
   }
 
   /// Construct a path for the explicitly built PCM.
-  std::string constructPCMPath(const ModuleDeps &MD) const {
+  std::string constructPCMPath(ModuleID MID) const {
+auto MDIt = Modules.find(IndexedModuleID{MID, 0});
+assert(MDIt != Modules.end());
+const ModuleDeps &MD = MDIt->second;
+
 StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath);
 
 SmallString<256> ExplicitPCMPath(
@@ -386,12 +384,6 @@
 return std::string(ExplicitPCMPath);
   }
 
-  const ModuleDeps &lookupModuleDeps(ModuleID MID) {
-auto I = Modules.find(IndexedModuleID{MID, 0});
-assert(I != Modules.end());
-return I->second;
-  };
-
   struct IndexedModuleID {
 ModuleID ID;
 mutable size_t InputIndex;
Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -94,7 +94,6 @@
 // CHECK-PCH-NEXT: "-fno-implicit-modules",
 // CHECK-PCH-NEXT: "-fno-implicit-module-maps",
 // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
-// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm",
 // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm"
 // CHECK-PCH-NEXT:   ],
 // CHECK-PCH-NEXT:   "file-deps": [
Index: clang/test/ClangScanDeps/modules-full.cpp
===
--- clang/test/ClangScanDeps/modules-full.cpp
+++ clang/test/ClangScanDeps/modules-full.cpp
@@ -166,9 +166,7 @@
 // CHECK-NEXT: "-fno-implicit-modules"
 // CHECK-NEXT: "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
-// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-

[PATCH] D118890: [clang][deps] Disable global module index

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I'd like to add a test case, but actually creating a minimal one is proving to 
be difficult. This is most likely caused by the interaction between the global 
module index and explicit+implicit modules. Since we want to move off of the 
implicit build in dependency scanner, I don't think having a test case for this 
is crucial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118890

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff abandoned this revision.
sepavloff added a comment.

@rsmith, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

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

[PATCH] D115523: [OpenCL] Set external linkage for block enqueue kernels

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D115523#3284446 , @yaxunl wrote:

> In D115523#3266584 , @Anastasia 
> wrote:
>
>> In D115523#3240870 , @yaxunl wrote:
>>
>>> In D115523#3237857 , @Anastasia 
>>> wrote:
>>>
 In D115523#3237410 , @yaxunl 
 wrote:

> It is possible that block kernels are defined and invoked in static 
> functions, therefore two block kernels in different TU's may have the 
> same name. Making such kernels external may cause duplicate symbols.

 Potentially we should append the name of the translation unit to all 
 kernel wrapper names for the enqueued blocks to resolve this? For example, 
 global constructors stubs are using such a similar naming scheme taken 
 from the translation unit.

 But the kernel function in OpenCL has to be globally visible and many 
 tools have been built with this assumption. Additionally, some toolchains 
 might require the enqueued kernels to be globally visible as well in order 
 to access them as an execution entry point.
>>>
>>> If we have to externalize such block kernels whose names are derived from 
>>> variables with internal linkage, we may need a way to make the block kernel 
>>> names unique.
>>>
>>> One approach would be use MD5 hash of the file path and compile options, 
>>> similar to 
>>> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Driver.cpp#L2623
>>
>> Ok, however it might be enough to do what C++ handling does with global 
>> ctors. Should we file a bug for this for now?
>
> Do you mean the global ctors like this? 
> https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenCXX/global-init.cpp#L201
>
> They do not have unique names across TU's. They work because they have 
> internal linkage. However OpenCL kernels have external linkage.
>
> I suggest opening a bug to track this. Thanks.

Sure: https://github.com/llvm/llvm-project/issues/53572. Feel free to add 
anything relevant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115523

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

The latest revision allows me to boot a kernel in QEMU now but that same kernel 
does not boot on bare metal. I'll see if I can narrow down the problematic 
translation unit with Nick's `subdir-ccflags-y` trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D112774: [RISCV] Support k-ext clang intrinsics

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:74
 
+// zbkb extension
+TARGET_BUILTIN(__builtin_riscv_brev8, "ZiZi", "nc", "zbkb")

Capital Z



Comment at: clang/lib/Basic/Targets/RISCV.cpp:198
+  ISAInfo->hasExtension("zk"))
+Builder.defineMacro("__riscv_crypto");
 }

This define doesn't seem very useful. It just tells you that you have some 
crypto instructions, but not which ones.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1
+
+// zbkx
+case RISCV::BI__builtin_riscv_xperm8:

Capital Z


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112774

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

[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Sam McCall via Phabricator via cfe-commits]

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

The second space in `void foo() &` is always produced by clang-format,
and isn't evidence of any particular style.

Before this patch, it was considered evidence of PAS_Right, because
there is a space before a pointerlike ampersand.

This caused the following code to have "unstable" pointer alignment:

  void a() &;
  void b() &;
  int *x;

PAS_Left, Derive=false would produce 'int* x' with other lines unchanged.
But subsequent formatting with Derive=true would produce 'int *x' again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118921

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,25 @@
AlignLeftBreakTemplate);
 
   verifyFormat("void (*foopt)(int) = &func;");
+
+  FormatStyle DerivePointerAlignment = getLLVMStyle();
+  DerivePointerAlignment.DerivePointerAlignment = true;
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+   "void b() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // Same if the function is an overloaded operator instead.
+  Prefix = "void operator()() &;\n"
+   "void operator()() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // However a space between cv-qualifiers and ref-qualifiers *is* evidence.
+  Prefix = "void a() const &;\n"
+   "void b() const &;\n";
+  EXPECT_EQ(Prefix + "int *x;",
+format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -18,6 +18,7 @@
 #include "ContinuationIndenter.h"
 #include "DefinitionBlockSeparator.h"
 #include "FormatInternal.h"
+#include "FormatToken.h"
 #include "FormatTokenLexer.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "QualifierAlignmentFixer.h"
@@ -1945,6 +1946,17 @@
   for (FormatToken *Tok = Line->First; Tok && Tok->Next; Tok = Tok->Next) {
 if (!Tok->is(TT_PointerOrReference))
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen) {
+if (const auto *Func =
+Prev->MatchingParen->getPreviousNonComment()) {
+  if (Func->isOneOf(TT_FunctionDeclarationName, TT_StartOfName,
+TT_OverloadedOperator))
+continue;
+}
+  }
+}
 bool SpaceBefore = Tok->hasWhitespaceBefore();
 bool SpaceAfter = Tok->Next->hasWhitespaceBefore();
 if (SpaceBefore && !SpaceAfter)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,25 @@
AlignLeftBreakTemplate);
 
   verifyFormat("void (*foopt)(int) = &func;");
+
+  FormatStyle DerivePointerAlignment = getLLVMStyle();
+  DerivePointerAlignment.DerivePointerAlignment = true;
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+   "void b() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // Same if the function is an overloaded operator instead.
+  Prefix = "void operator()() &;\n"
+   "void operator()() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // However a space between cv-qualifiers and ref-qualifiers *is* evidence.
+  Prefix = "void a() const &;\n"
+   "void b() const &;\n";
+  EXPECT_EQ(Prefix + "int *x;",
+format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -18,6 +18,7 @@
 #include "ContinuationIndenter.h"
 #include "DefinitionBlockSeparator.h"
 #includ

[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Sam McCall via Phabricator via cfe-commits]

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

clean up formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118921

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,25 @@
AlignLeftBreakTemplate);
 
   verifyFormat("void (*foopt)(int) = &func;");
+
+  FormatStyle DerivePointerAlignment = getLLVMStyle();
+  DerivePointerAlignment.DerivePointerAlignment = true;
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+   "void b() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // Same if the function is an overloaded operator instead.
+  Prefix = "void operator()() &;\n"
+   "void operator()() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // However a space between cv-qualifiers and ref-qualifiers *is* evidence.
+  Prefix = "void a() const &;\n"
+   "void b() const &;\n";
+  EXPECT_EQ(Prefix + "int *x;",
+format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -18,6 +18,7 @@
 #include "ContinuationIndenter.h"
 #include "DefinitionBlockSeparator.h"
 #include "FormatInternal.h"
+#include "FormatToken.h"
 #include "FormatTokenLexer.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "QualifierAlignmentFixer.h"
@@ -1945,6 +1946,14 @@
   for (FormatToken *Tok = Line->First; Tok && Tok->Next; Tok = Tok->Next) {
 if (!Tok->is(TT_PointerOrReference))
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+if (const auto *Func = 
Prev->MatchingParen->getPreviousNonComment())
+  if (Func->isOneOf(TT_FunctionDeclarationName, TT_StartOfName,
+TT_OverloadedOperator))
+continue;
+}
 bool SpaceBefore = Tok->hasWhitespaceBefore();
 bool SpaceAfter = Tok->Next->hasWhitespaceBefore();
 if (SpaceBefore && !SpaceAfter)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9683,6 +9683,25 @@
AlignLeftBreakTemplate);
 
   verifyFormat("void (*foopt)(int) = &func;");
+
+  FormatStyle DerivePointerAlignment = getLLVMStyle();
+  DerivePointerAlignment.DerivePointerAlignment = true;
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+   "void b() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // Same if the function is an overloaded operator instead.
+  Prefix = "void operator()() &;\n"
+   "void operator()() &;\n";
+  verifyFormat(Prefix + "int* x;", DerivePointerAlignment);
+  verifyFormat(Prefix + "int *x;", DerivePointerAlignment);
+  // However a space between cv-qualifiers and ref-qualifiers *is* evidence.
+  Prefix = "void a() const &;\n"
+   "void b() const &;\n";
+  EXPECT_EQ(Prefix + "int *x;",
+format(Prefix + "int* x;", DerivePointerAlignment));
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -18,6 +18,7 @@
 #include "ContinuationIndenter.h"
 #include "DefinitionBlockSeparator.h"
 #include "FormatInternal.h"
+#include "FormatToken.h"
 #include "FormatTokenLexer.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "QualifierAlignmentFixer.h"
@@ -1945,6 +1946,14 @@
   for (FormatToken *Tok = Line->First; Tok && Tok->Next; Tok = Tok->Next) {
 if (!Tok->is(TT_PointerOrReference))
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+if (const auto *Func = Prev->MatchingParen

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

[Evgeny Shulgin via Phabricator via cfe-commits]

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

Inline variables is a feature from C++17. It makes sense
to extend the `isInline` matcher in order to support corresponding varDecls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118922

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1455,6 +1455,23 @@
   "int main() { int x = 3; auto f = [vd=x]() { return vd; }; }", M));
 }
 
+TEST_P(ASTMatchersTest, IsInline) {
+  if (!GetParam().isCXX() && !GetParam().isC99OrLater())
+return;
+  EXPECT_TRUE(matches("void g(); inline void f();",
+  functionDecl(isInline(), hasName("f";
+
+  if (!GetParam().isCXX())
+return;
+  EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
+  namespaceDecl(isInline(), hasName("m";
+
+  if (!GetParam().isCXX17OrLater())
+return;
+  EXPECT_TRUE(matches("inline constexpr int i = 1;",
+  varDecl(isInline(), hasName("i";
+}
+
 TEST_P(ASTMatchersTest, StorageDuration) {
   StringRef T =
   "void f() { int x; static int y; } int a;static int b;extern int c;";
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -192,13 +192,6 @@
   llvm::ValueIs(TK_IgnoreUnlessSpelledInSource));
 }
 
-TEST(IsInlineMatcher, IsInline) {
-  EXPECT_TRUE(matches("void g(); inline void f();",
-  functionDecl(isInline(), hasName("f";
-  EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
-  namespaceDecl(isInline(), hasName("m";
-}
-
 // FIXME: Figure out how to specify paths so the following tests pass on
 // Windows.
 #ifndef _WIN32
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7673,25 +7673,29 @@
   return InnerMatcher.matches(*ES.getExpr(), Finder, Builder);
 }
 
-/// Matches function and namespace declarations that are marked with
+/// Matches variable, function and namespace declarations that are marked with
 /// the inline keyword.
 ///
 /// Given
 /// \code
+///   inline constexpr int i = 1;
 ///   inline void f();
 ///   void g();
 ///   namespace n {
 ///   inline namespace m {}
 ///   }
 /// \endcode
+/// varDecl(isInline()) will match ::i.
 /// functionDecl(isInline()) will match ::f().
 /// namespaceDecl(isInline()) will match n::m.
 AST_POLYMORPHIC_MATCHER(isInline,
-AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
-FunctionDecl)) {
+AST_POLYMORPHIC_SUPPORTED_TYPES(VarDecl, FunctionDecl,
+NamespaceDecl)) {
   // This is required because the spelling of the function used to determine
   // whether inline is specified or not differs between the polymorphic types.
-  if (const auto *FD = dyn_cast(&Node))
+  if (const auto *VD = dyn_cast(&Node))
+return VD->isInline();
+  else if (const auto *FD = dyn_cast(&Node))
 return FD->isInlineSpecified();
   else if (const auto *NSD = dyn_cast(&Node))
 return NSD->isInline();
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -135,6 +135,8 @@
 AST Matchers
 
 
+- The ``isInline`` matcher now accepts inline variable declarations.
+
 clang-format
 
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4310,15 +4310,17 @@
 
 
 MatcherFunctionDecl>isInline
-Matches function and namespace declarations that are marked with
+Matches variable, function and namespace declarations that are marked with
 the inline keyword.
 
 Given
+  inline constexpr int i = 1;
   inline void f();
   void g();
   namespace n {
   inline namespace m {}
   }
+varDecl(isInline()) will match ::i.
 functionDecl(isInline()) w

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

I am planning to reuse this matcher in my new checker 
(https://reviews.llvm.org/D118743, under development now), removing this line:

  AST_MATCHER(VarDecl, isVarInline) { return Node.isInline(); }

//P.S. If this review is eventually approved, kindly please merge the commit on 
my behalf =) As I don't have merge access. My name is Evgeny Shulgin and email 
is izaronpl...@gmail.com. Sorry for inconvenience!//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118922

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

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 requested changes to this revision.
xbolva00 added inline comments.
This revision now requires changes to proceed.



Comment at: clang/test/CodeGen/alloc-fns-alignment.c:37
-
-void *aligned_alloc_large_constant_test(size_t n) {
-  return aligned_alloc(4096, n);

These tests for should stay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118804

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

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG, with some things to address before the merge though.

Didn't we have a pass to expand shared memory (and such)?




Comment at: clang/lib/Basic/TargetInfo.cpp:155
+
+  if (Triple.getVendor() == llvm::Triple::OpenMP_VGPU)
+AddrSpaceMap = &llvm::omp::OpenMPVGPUAddrSpaceMap;

use isOpenMPVGPU



Comment at: clang/lib/Basic/Targets/X86.h:395
+return llvm::omp::VirtualGpuGridValues;
+  }
 };

Do we need the changes in this file at all? I couldn't see why.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1125
+llvm::GlobalValue::LinkageTypes Linkage) {
+  if (CGM.getTarget().getTriple().getVendor() == llvm::Triple::OpenMP_VGPU)
+return CGOpenMPRuntime::createOffloadEntry(ID, Addr, Size, Flags, Linkage);

isOpenMPVGPU



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:252
   default:
-if (LangOpts.OpenMPSimd)
+if (getTriple().getVendor() == llvm::Triple::OpenMP_VGPU)
+  OpenMPRuntime.reset(new CGOpenMPRuntimeGPU(*this));

isOpenMPVGPU



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3076
+
+  if (getTriple().getVendor() == llvm::Triple::OpenMP_VGPU) {
+std::string BitcodeSuffix = getTripleString() + "-openmp_vgpu";

isOpenMPVGPU



Comment at: openmp/libomptarget/DeviceRTL/src/Synchronization.cpp:323
+constexpr uint32_t UNSET = 0;
+constexpr uint32_t SET = 1;
+

Remove these. Also the TODO below (copied from somewhere)



Comment at: openmp/libomptarget/plugins/vgpu/src/ThreadEnvironmentImpl.cpp:85
+ CTAEnvironmentTy *CTAE)
+: ThreadIdInWarp(Idx++ % WE->getNumThreads()),
+  ThreadIdInBlock(WE->getWarpId() * WE->getNumThreads() + ThreadIdInWarp),

This is racy, I think. Can we use atomic_add for all these Idx updates or pass 
the Id from the outside?



Comment at: openmp/libomptarget/plugins/vgpu/src/ThreadEnvironmentImpl.h:118
+
+  // FIXME: This is wrong
+  LaneMaskTy getActiveMask() const;

at least add more information what the problem and potential solutions are.



Comment at: openmp/libomptarget/plugins/vgpu/src/rtl.cpp:271
+ ThreadIdx++) {
+  Threads.emplace_back([this, GlobalThreadIdx, CTAEnv, WarpEnv]() {
+ThreadEnvironment = new ThreadEnvironmentTy(WarpEnv, CTAEnv);

Move the lambda into a helper function. indention of 12 is too much.



Comment at: openmp/libomptarget/plugins/vgpu/src/rtl.cpp:313
+  });
+  GlobalThreadIdx = (GlobalThreadIdx + 1) % NumThreads;
+}

When do we have more threads than NumThreads? 



Comment at: openmp/libomptarget/plugins/vgpu/src/rtl.cpp:554
+
+int32_t __tgt_rtl_data_delete(int32_t device_id, void *tgt_ptr) {
+  free(tgt_ptr);

if we need for submit/retrieve, I'd assume to wait here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359

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

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp:195-201
-TEST(IsInlineMatcher, IsInline) {
-  EXPECT_TRUE(matches("void g(); inline void f();",
-  functionDecl(isInline(), hasName("f";
-  EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
-  namespaceDecl(isInline(), hasName("m";
-}
-

Isn't this test equivalent to the new one I wrote in the other file? The 
difference between `TEST` and `TEST_P` I see is that the latter one allows you 
to check C++17 code, while the former does not. So I decided to have all checks 
in one place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118922

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

[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/Format.cpp:1949
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {

Do we also need to worry about `T && foo();` style declarations?




Comment at: clang/lib/Format/Format.cpp:1951
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+if (const auto *Func = 
Prev->MatchingParen->getPreviousNonComment())

If this is parsing a general function declaration, I think it's not safe to 
assume that the token immediately before the ref-spec is the close-paren.

It looks like `Prev` can be a few other things, including at least:

* `tok::kw_const`
* `tok::kw_volatile`
* `tok::kw_throw`
* `tok::kw_noexcept`

and probably C++ attribute(s) as well:

> noptr-declarator ( parameter-list ) cv(optional) ref(optional) 
> except(optional) attr(optional)

* https://en.cppreference.com/w/cpp/language/function
* https://en.cppreference.com/w/cpp/language/declarations

I *think* they can also be in any order, so the ref-spec could come at the very 
end after cv-qualifier, an expection specification, and C++ attributes.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118921

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

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Not sure if it's good to merge such a large patch. We could potentially split 
the patch to three independent patches: tool chain, device runtime, and the 
OpenMPOpt pass to support expansion of shared variable (which for some reason 
is not included in this patch. That is actually very important component 
otherwise the backend will complain about it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359

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

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

We can merge runtime first, build it in isolation, then libomptarget host 
runtime, then clang.

Also make sure to adjust the commit messages


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359

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

[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

https://reviews.llvm.org/D118862
https://reviews.llvm.org/D118875


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117924

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

[clang] 529aa4b - [clang-format] Avoid adding space after the name of a function-like macro when the name is a keyword.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-03T18:45:51+01:00
New Revision: 529aa4b011c4ae808d658022ef643c44dd9b2c9c

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

LOG: [clang-format] Avoid adding space after the name of a function-like macro 
when the name is a keyword.

Fixes https://github.com/llvm/llvm-project/issues/31086.

Before the code:
```
#define if(x)
```

was erroneously formatted to:
```
#define if (x)
```

Reviewed By: HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index c43c8da6f3984..37fad4d9f49de 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1007,6 +1007,12 @@ void UnwrappedLineParser::parsePPDefine() {
 }
   }
 
+  // In the context of a define, even keywords should be treated as normal
+  // identifiers. Setting the kind to identifier is not enough, because we need
+  // to treat additional keywords like __except as well, which are already
+  // identifiers.
+  FormatTok->Tok.setKind(tok::identifier);
+  FormatTok->Tok.setIdentifierInfo(nullptr);
   nextToken();
   if (FormatTok->Tok.getKind() == tok::l_paren &&
   !FormatTok->hasWhitespaceBefore())

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 720c6e9b6b2f1..3d9e38616450c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1795,6 +1795,18 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, UnderstandsMacros) {
+  verifyFormat("#define A (parentheses)");
+  verifyFormat("#define true ((int)1)");
+  verifyFormat("#define and(x)");
+  verifyFormat("#define if(x) x");
+  verifyFormat("#define return(x) (x)");
+  verifyFormat("#define while(x) for (; x;)");
+  verifyFormat("#define xor(x) (^(x))");
+  verifyFormat("#define __except(x)");
+  verifyFormat("#define __try(x)");
+}
+
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;



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

[PATCH] D118844: [clang-format] Avoid adding space after the name of a function-like macro when the name is a keyword.

[Marek Kurdej 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 rG529aa4b011c4: [clang-format] Avoid adding space after the 
name of a function-like macro when… (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118844

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1795,6 +1795,18 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, UnderstandsMacros) {
+  verifyFormat("#define A (parentheses)");
+  verifyFormat("#define true ((int)1)");
+  verifyFormat("#define and(x)");
+  verifyFormat("#define if(x) x");
+  verifyFormat("#define return(x) (x)");
+  verifyFormat("#define while(x) for (; x;)");
+  verifyFormat("#define xor(x) (^(x))");
+  verifyFormat("#define __except(x)");
+  verifyFormat("#define __try(x)");
+}
+
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1007,6 +1007,12 @@
 }
   }
 
+  // In the context of a define, even keywords should be treated as normal
+  // identifiers. Setting the kind to identifier is not enough, because we need
+  // to treat additional keywords like __except as well, which are already
+  // identifiers.
+  FormatTok->Tok.setKind(tok::identifier);
+  FormatTok->Tok.setIdentifierInfo(nullptr);
   nextToken();
   if (FormatTok->Tok.getKind() == tok::l_paren &&
   !FormatTok->hasWhitespaceBefore())


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1795,6 +1795,18 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, UnderstandsMacros) {
+  verifyFormat("#define A (parentheses)");
+  verifyFormat("#define true ((int)1)");
+  verifyFormat("#define and(x)");
+  verifyFormat("#define if(x) x");
+  verifyFormat("#define return(x) (x)");
+  verifyFormat("#define while(x) for (; x;)");
+  verifyFormat("#define xor(x) (^(x))");
+  verifyFormat("#define __except(x)");
+  verifyFormat("#define __try(x)");
+}
+
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1007,6 +1007,12 @@
 }
   }
 
+  // In the context of a define, even keywords should be treated as normal
+  // identifiers. Setting the kind to identifier is not enough, because we need
+  // to treat additional keywords like __except as well, which are already
+  // identifiers.
+  FormatTok->Tok.setKind(tok::identifier);
+  FormatTok->Tok.setIdentifierInfo(nullptr);
   nextToken();
   if (FormatTok->Tok.getKind() == tok::l_paren &&
   !FormatTok->hasWhitespaceBefore())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits