[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:3961
+  // linkage so that the linker preserves the symbol name.
+  llvm::GlobalValue::LinkageTypes LT =
+  Section.empty() || Section.startswith("__DATA")

Hmm, when would you have a metadata variable not in the `__DATA` segment?



Comment at: lib/CodeGen/CGObjCMac.cpp:7266
+ false,
+ CGM.getTriple().isOSBinFormatMachO()
+ ? llvm::GlobalValue::InternalLinkage

Is there a reason to not make this and the other instances `Internal` 
irrespective of the object file format?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61454



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks pretty nice to me! Sorry for the wait.
Adding @kadircet as hover-guru-in-waiting.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:814
+return Reply(llvm::None);
+  // FIXME: render as markdown if client supports it.
+  Hover R;

(I'd like to see that in this patch if possible, it shouldn't be much work)



Comment at: clang-tools-extra/clangd/ClangdServer.h:186
   void findHover(PathRef File, Position Pos,
- Callback> CB);
+ Callback> CB);
 

Not sure about switching from `Hover` to `FormattedString` as return type here: 
LSP hover struct contains a range field (what are the bounds of the hovered 
thing?) that we may want to populate that doesn't fit in FormattedString.
I'd suggest the function in XRefs.h should return `FormattedString` (and later 
maybe `pair`), and the `ClangdServer` version should 
continue to provide the rendered `Hover`.

(We'll eventually want ClangdServer to return some HoverInfo struct with 
structured information, as we want embedding clients to be able to render it 
other ways, and maybe use it to provide extension methods like YCM's `GetType`. 
But no need to touch that in this patch, we'll end up producing HoverInfo -> 
FormattedString -> LSP Hover, I guess)



Comment at: clang-tools-extra/clangd/FormattedString.cpp:41
+
+void FormattedString::appendCodeBlock(std::string Code) {
+  Chunk C;

you may want to take the language name, and default it to either cpp or nothing.
Including it in the triple-backtick block can trigger syntax highlighting, and 
editors are probably somewhat likely to actually implement this.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:63
+case ChunkKind::InlineCodeBlock:
+  R += " `";
+  R += escapeBackticks(C.Contents);

why do we add surrounding spaces if we don't do the same for text chunks?



Comment at: clang-tools-extra/clangd/FormattedString.h:1
+//===--- FormattedString.h --*- 
C++-*--===//
+//

This will need tests as you note, which shouldn't be hard.
It's not terribly complicated, but it's the sort of thing that may accumulate 
special cases.



Comment at: clang-tools-extra/clangd/FormattedString.h:27
+  /// Append plain text to the end of the string.
+  void appendText(std::string Text);
+  /// Append a block of C++ code. This translates to a ``` block in markdown.

I guess this will get an optional parameter to control the style (bold, italic 
etc)?



Comment at: clang-tools-extra/clangd/FormattedString.h:31
+  /// newlines.
+  void appendCodeBlock(std::string Code);
+  /// Append an inline block of C++ code. This translates to the ` block in

Having various methods that take strings may struggle if we want a lot of 
composability (e.g. a bullet list whose bullet whose third word is a 
hyperlink/bold).

(I'm not sure whether this is going to be a real problem, just thinking out 
loud here)

One alternative extensibility would be to make "containers" methods taking a 
lambda that renders the body.
e.g.

```
FormattedString S;
S << "std::";
S.bold([&} S << "swap" };
S.list(FormattedString::Numbered, [&] {
  S.item([&] { S << "sometimes you get the wrong overload"; });
  S.item([&] {
S << "watch out for ";
S.link("https://en.cppreference.com/w/cpp/language/adl";, [&] { S << "ADL"; 
});
  });
});
S.codeBlock([&] S << "Here is an example"; });
```

Not sure if this is really better, I just have it on my mind because I ended up 
using it for the `JSON.h` streaming output. We can probably bolt it on later if 
necessary.



Comment at: clang-tools-extra/clangd/FormattedString.h:39
+  std::string renderAsPlainText() const;
+
+private:

I think we want renderForDebugging() or so which would print the chunks 
explicitly, a la CodeCompletionString.

This is useful for actual debugging, and for testing methods that return 
FormattedString (as opposed to the FormattedString->markdown translation)



Comment at: clang-tools-extra/clangd/XRefs.cpp:528
+R.appendText("Declared in ");
+R.appendText(*NamedScope);
+R.appendText("\n");

should this be an inline code block?



Comment at: clang-tools-extra/unittests/clangd/XRefsTests.cpp:1157
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(H->renderAsPlainText(), Test.ExpectedHover.str()) << 
Test.Input;
 } else

When this lands for real, I think we should assert on the renderForDebugging() 
output or similar.


Repository:
  rG LLVM Github Monorepo

CHAN

r359864 - [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri May  3 00:19:46 2019
New Revision: 359864

URL: http://llvm.org/viewvc/llvm-project?rev=359864&view=rev
Log:
[Sema][ObjC] Disable -Wunused-parameter for ObjC methods

The warning isn't very useful when the function is an ObjC method.

rdar://problem/41561853

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

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaObjC/method-unused-attribute.m
cfe/trunk/test/SemaObjC/unused.m

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=359864&r1=359863&r2=359864&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri May  3 00:19:46 2019
@@ -13353,8 +13353,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  if (!MD->hasSkippedBody())
-DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 

Modified: cfe/trunk/test/SemaObjC/method-unused-attribute.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-unused-attribute.m?rev=359864&r1=359863&r2=359864&view=diff
==
--- cfe/trunk/test/SemaObjC/method-unused-attribute.m (original)
+++ cfe/trunk/test/SemaObjC/method-unused-attribute.m Fri May  3 00:19:46 2019
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify 
-Wno-objc-root-class %s
 
+// -Wunused-parameter ignores ObjC method parameters that are unused.
+
+// expected-no-diagnostics
+
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
 - (void) will_warn_unused_arg: (void *) notice : (id)warn_unused;
@@ -9,7 +13,7 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void *)  __attribute__((unused)) notice : 
(id) __attribute__((unused)) newarg{
 }
-- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}  // expected-warning {{unused parameter 'warn_unused'}}
-- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}  // 
expected-warning {{unused parameter 'will_warn'}}
+- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}
+- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}
 @end
 

Modified: cfe/trunk/test/SemaObjC/unused.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unused.m?rev=359864&r1=359863&r2=359864&view=diff
==
--- cfe/trunk/test/SemaObjC/unused.m (original)
+++ cfe/trunk/test/SemaObjC/unused.m Fri May  3 00:19:46 2019
@@ -33,7 +33,7 @@ void test2() {
   // expected-note {{introduce a parameter name to make 
'x' part of the selector}} \
   // expected-note {{or insert whitespace before ':' to 
use 'x' as parameter name and have an empty entry in the selector}}
 
-(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the 
name of the previous parameter rather than as part of the selector}} \
+(int)y:  // expected-warning {{'y' used as the name of the previous parameter 
rather than as part of the selector}} \
  // expected-note {{introduce a parameter name to make 'y' part of the 
selector}} \
  // expected-note {{or insert whitespace before ':' to use 'y' as 
parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }


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

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359864: [Sema][ObjC] Disable -Wunused-parameter for ObjC 
methods (authored by ahatanak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61147?vs=197469&id=197924#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61147

Files:
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/SemaObjC/method-unused-attribute.m
  cfe/trunk/test/SemaObjC/unused.m


Index: cfe/trunk/test/SemaObjC/unused.m
===
--- cfe/trunk/test/SemaObjC/unused.m
+++ cfe/trunk/test/SemaObjC/unused.m
@@ -33,7 +33,7 @@
   // expected-note {{introduce a parameter name to make 
'x' part of the selector}} \
   // expected-note {{or insert whitespace before ':' to 
use 'x' as parameter name and have an empty entry in the selector}}
 
-(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the 
name of the previous parameter rather than as part of the selector}} \
+(int)y:  // expected-warning {{'y' used as the name of the previous parameter 
rather than as part of the selector}} \
  // expected-note {{introduce a parameter name to make 'y' part of the 
selector}} \
  // expected-note {{or insert whitespace before ':' to use 'y' as 
parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }
Index: cfe/trunk/test/SemaObjC/method-unused-attribute.m
===
--- cfe/trunk/test/SemaObjC/method-unused-attribute.m
+++ cfe/trunk/test/SemaObjC/method-unused-attribute.m
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify 
-Wno-objc-root-class %s
 
+// -Wunused-parameter ignores ObjC method parameters that are unused.
+
+// expected-no-diagnostics
+
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
 - (void) will_warn_unused_arg: (void *) notice : (id)warn_unused;
@@ -9,7 +13,7 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void *)  __attribute__((unused)) notice : 
(id) __attribute__((unused)) newarg{
 }
-- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}  // expected-warning {{unused parameter 'warn_unused'}}
-- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}  // 
expected-warning {{unused parameter 'will_warn'}}
+- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}
+- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}
 @end
 
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -13353,8 +13353,6 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  if (!MD->hasSkippedBody())
-DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 


Index: cfe/trunk/test/SemaObjC/unused.m
===
--- cfe/trunk/test/SemaObjC/unused.m
+++ cfe/trunk/test/SemaObjC/unused.m
@@ -33,7 +33,7 @@
   // expected-note {{introduce a parameter name to make 'x' part of the selector}} \
   // expected-note {{or insert whitespace before ':' to use 'x' as parameter name and have an empty entry in the selector}}
 
-(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the name of the previous parameter rather than as part of the selector}} \
+(int)y:  // expected-warning {{'y' used as the name of the previous parameter rather than as part of the selector}} \
  // expected-note {{introduce a parameter name to make 'y' part of the selector}} \
  // expected-note {{or insert whitespace before ':' to use 'y' as parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }
Index: cfe/trunk/test/SemaObjC/method-unused-attribute.m
===
--- cfe/trunk/test/SemaObjC/method-unused-attribute.m
+++ cfe/trunk/test/SemaObjC/method-unused-attribute.m
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify -Wno-objc-root-class %s
 
+// -Wunused-parameter ignores ObjC method parameters that are unused.
+
+// expected-no-diagnostics
+
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
 - (void) will_warn_unused_arg: (void *) notice : (id)warn_unused;
@@ -9,7 +13,7 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void 

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak marked 2 inline comments as done.
ahatanak added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:3961
+  // linkage so that the linker preserves the symbol name.
+  llvm::GlobalValue::LinkageTypes LT =
+  Section.empty() || Section.startswith("__DATA")

compnerd wrote:
> Hmm, when would you have a metadata variable not in the `__DATA` segment?
There are several places where a section string that starts with `__OBJC` is 
passed. For example, `CGObjCMac::EmitModuleSymbols`.



Comment at: lib/CodeGen/CGObjCMac.cpp:7266
+ false,
+ CGM.getTriple().isOSBinFormatMachO()
+ ? llvm::GlobalValue::InternalLinkage

compnerd wrote:
> Is there a reason to not make this and the other instances `Internal` 
> irrespective of the object file format?
When the object file format isn't MachO, this variable doesn't go into a 
section that is in `__DATA`, so we want to keep the variable private to avoid 
needlessly preserving the symbol name.

The intent of the patch is to prevent the linker from removing the symbol names 
of symbols in `__DATA` so that tools can collect information about those 
symbols.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61454



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

[clang-tools-extra] r359866 - [clangd] Minor code style cleanups in Protocol.h. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri May  3 01:03:21 2019
New Revision: 359866

URL: http://llvm.org/viewvc/llvm-project?rev=359866&view=rev
Log:
[clangd] Minor code style cleanups in Protocol.h. NFC

- Remove a parameter name that was misspelled (OS used for non-stream
  parameter)
- Declare operator == (TextEdit, TextEdit) outside the struct, for
  consistency with other user-declared ops in our code.
- Fix naming style of a parameter.

Modified:
clang-tools-extra/trunk/clangd/Protocol.h

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=359866&r1=359865&r2=359866&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Fri May  3 01:03:21 2019
@@ -206,11 +206,10 @@ struct TextEdit {
   /// The string to be inserted. For delete operations use an
   /// empty string.
   std::string newText;
-
-  bool operator==(const TextEdit &rhs) const {
-return newText == rhs.newText && range == rhs.range;
-  }
 };
+inline bool operator==(const TextEdit &L, const TextEdit &R) {
+  return std::tie(L.newText, L.range) == std::tie(R.newText, R.range);
+}
 bool fromJSON(const llvm::json::Value &, TextEdit &);
 llvm::json::Value toJSON(const TextEdit &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TextEdit &);
@@ -294,7 +293,7 @@ using CompletionItemKindBitset = std::bi
 bool fromJSON(const llvm::json::Value &, CompletionItemKindBitset &);
 CompletionItemKind
 adjustKindToCapability(CompletionItemKind Kind,
-   CompletionItemKindBitset &supportedCompletionItemKinds);
+   CompletionItemKindBitset &SupportedCompletionItemKinds);
 
 /// A symbol kind.
 enum class SymbolKind {
@@ -352,7 +351,7 @@ enum class OffsetEncoding {
 };
 llvm::json::Value toJSON(const OffsetEncoding &);
 bool fromJSON(const llvm::json::Value &, OffsetEncoding &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, OffsetEncoding OS);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, OffsetEncoding);
 
 // This struct doesn't mirror LSP!
 // The protocol defines deeply nested structures for client capabilities.


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

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61480

Files:
  clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/DeclBase.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2031,6 +2031,52 @@
   EXPECT_TRUE(matches("namespace {}", namespaceDecl(isAnonymous(;
 }
 
+TEST(DeclarationMatcher, InStdNamespace) {
+  EXPECT_TRUE(notMatches("class vector {};"
+ "namespace foo {"
+ "  class vector {};"
+ "}",
+ cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+
+  EXPECT_TRUE(matches("namespace std {"
+  "  class vector {};"
+  "}",
+  cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+  EXPECT_TRUE(matches("namespace std {"
+  "  inline namespace __1 {"
+  "class vector {};"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+  EXPECT_TRUE(notMatches("namespace std {"
+ "  inline namespace __1 {"
+ "inline namespace __fs {"
+ "  namespace filesystem {"
+ "inline namespace v1 {"
+ "  class path {};"
+ "}"
+ "  }"
+ "}"
+ "  }"
+ "}",
+ cxxRecordDecl(hasName("path"), isInStdNamespace(;
+  EXPECT_TRUE(
+  matches("namespace std {"
+  "  inline namespace __1 {"
+  "inline namespace __fs {"
+  "  namespace filesystem {"
+  "inline namespace v1 {"
+  "  class path {};"
+  "}"
+  "  }"
+  "}"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("path"),
+hasAncestor(namespaceDecl(hasName("filesystem"),
+  isInStdNamespace());
+}
+
 TEST(EqualsBoundNodeMatcher, QualType) {
   EXPECT_TRUE(matches(
 "int i = 1;", varDecl(hasType(qualType().bind("type")),
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -354,7 +354,8 @@
 }
 
 bool Decl::isInStdNamespace() const {
-  return getDeclContext()->isStdNamespace();
+  const DeclContext *DC = getDeclContext();
+  return DC && DC->isStdNamespace();
 }
 
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6212,6 +6212,27 @@
   return Node.isAnonymousNamespace();
 }
 
+/// Matches anonymous namespace declarations.
+///
+/// Given
+/// \code
+///   class vector {}; // #1
+///   namespace foo {
+/// class vector {}; // #2
+///   }
+///   namespace std {
+/// inline namespace __1 {
+///   class vector {}; // #3
+///   namespace experimental {
+/// class vector {}; // #4
+///   }
+/// }
+///   }
+/// \endcode
+/// cxxRecordDecl(hasName("vector"), isInStdNamespace()) will match #3 but not
+/// #1, #2, or #4.
+AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
+
 /// If the given case statement does not use the GNU case range
 /// extension, matches the constant given in the statement.
 ///
Index: clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp
@@ -17,10 +17,6 @@
 namespace tidy {
 namespace bugprone {
 
-namespace {
-AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
-}
-
 void InaccurateEraseCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGObjCMac.cpp:7266
+ false,
+ CGM.getTriple().isOSBinFormatMachO()
+ ? llvm::GlobalValue::InternalLinkage

ahatanak wrote:
> compnerd wrote:
> > Is there a reason to not make this and the other instances `Internal` 
> > irrespective of the object file format?
> When the object file format isn't MachO, this variable doesn't go into a 
> section that is in `__DATA`, so we want to keep the variable private to avoid 
> needlessly preserving the symbol name.
> 
> The intent of the patch is to prevent the linker from removing the symbol 
> names of symbols in `__DATA` so that tools can collect information about 
> those symbols.
I realized that there are other places where I should check the object file 
format so that the linkage isn't changed to internal when it doesn't have to be 
changed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61454



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

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

You should also update Registry.cpp to add this to clang-query and you should 
also regenerate the documentation by running 
clang\docs\tools\dump_ast_matchers.py.




Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2079
+}
+
 TEST(EqualsBoundNodeMatcher, QualType) {

Can you also add a negative test case for:
```
namespace foo {
namespace std {
  void bar(); // isInStdNamespace() is false here, right?
}
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61480



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

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

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

Fix latest case (trailing comment outdented in vscode).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Format.cpp
  clangd/Format.h
  test/clangd/formatting.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/FormatTests.cpp

Index: unittests/clangd/FormatTests.cpp
===
--- /dev/null
+++ unittests/clangd/FormatTests.cpp
@@ -0,0 +1,294 @@
+//===-- FormatTests.cpp - Automatic code formatting tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Format.h"
+#include "Annotations.h"
+#include "SourceCode.h"
+#include "TestFS.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string afterTyped(llvm::StringRef CodeWithCursor,
+   llvm::StringRef Typed) {
+  Annotations Code(CodeWithCursor);
+  unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
+  auto Changes =
+  formatIncremental(Code.code(), Cursor, Typed,
+format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+  tooling::Replacements Merged;
+  for (const auto& R : Changes)
+if (llvm::Error E = Merged.add(R))
+  ADD_FAILURE() << llvm::toString(std::move(E));
+  auto NewCode = tooling::applyAllReplacements(Code.code(), Merged);
+  EXPECT_TRUE(bool(NewCode))
+  << "Bad replacements: " << llvm::toString(NewCode.takeError());
+  NewCode->insert(transformCursorPosition(Cursor, Changes), "^");
+  return *NewCode;
+}
+
+// We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
+void expectAfterNewline(const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+}
+void expectAfter(const char *Typed, const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+}
+
+TEST(FormatIncremental, SplitComment) {
+  expectAfterNewline(R"cpp(
+// this comment was
+^split
+)cpp",
+   R"cpp(
+// this comment was
+// ^split
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// trailing whitespace is not a split
+^   
+)cpp",
+   R"cpp(
+// trailing whitespace is not a split
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// splitting a
+^
+// multiline comment
+)cpp",
+ R"cpp(
+// splitting a
+// ^
+// multiline comment
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// extra   
+^ whitespace
+)cpp",
+   R"cpp(
+// extra
+// ^whitespace
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// triple
+^slash
+)cpp",
+   R"cpp(
+/// triple
+/// ^slash
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// editor continuation
+//^
+)cpp",
+   R"cpp(
+/// editor continuation
+/// ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// break before
+^ // slashes
+)cpp",
+   R"cpp(
+// break before
+^// slashes
+)cpp");
+
+
+  expectAfterNewline(R"cpp(
+int x;  // aligned
+^comment
+)cpp",
+   R"cpp(
+int x;  // aligned
+// ^comment
+)cpp");
+
+  // Fixed bug: the second line of the aligned comment shouldn't be "attached"
+  // to the cursor and outdented.
+  expectAfterNewline(R"cpp(
+void foo() {
+  if (x)
+return; // All spelled tokens are accounted for.
+// that takes two lines
+^
+}
+)cpp",
+   R"cpp(
+void foo() {
+  if (x)
+return;  // All spelled tokens are accounted for.
+ // that takes two lines
+  ^
+}
+)cpp");
+}
+
+TEST(FormatIncremental, Indentation) {
+  expectAfterNewline(R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp",
+   R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+  bar(baz(
+^
+)cpp",
+   R"cpp(
+void foo() {
+  bar(baz(
+  ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+^}
+)cpp",
+   R"cpp(
+void foo() {
+  ^
+}
+)cpp");
+
+  expectAfterNewline(R"cpp(
+class X {
+protected:
+^
+)cpp",
+   R"cpp(
+class X {
+ protected:
+  ^
+)cpp");
+
+// Mismatched brackets (1)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^}
+}
+)cpp");
+// Mismatched brackets (2)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^text}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^text}
+}
+)cpp");
+// Matched brackets
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^)
+}
+)cpp",
+   R"

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D60605#1488039 , @ilya-biryukov 
wrote:

> Maybe that's due to some extra logic applied in VSCode on top of what we do.
>  Let me know if this does not reproduce.


Aha, the missing piece was that vscode reindented the cursor to align with the 
comment (but didn't continue the comment).
This caused us to replace the cursor with an *indented* identifier, and 
clang-format thought the last line of the comment was attached to it.

It almost seems silly to keep patching the code rather than add a "break here" 
option to clang-format that doesn't have side-effects. However the fix here is 
pretty simple, so why not.

Do you think this is converging to something we should land? The last few have 
been pretty small tweaks and I'm happy to keep fixing those as incremental 
patches.
Conversely, if the basic design isn't right, then further polish probably isn't 
worthwhile.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[PATCH] D60925: [analyzer] Don't display implementation checkers under -analyzer-checker-help, but do under the new flag -analyzer-checker-help-hidden

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

Szelethus added a comment.

In D60925#1489034 , @o.gyorgy wrote:

> Great, at least the users will not enable the debug checkers by accident!
>  We will check on the CodeChecker side if any configuration needs to be 
> updated.
>
> LGTM


I committed after checking whether this works with `CodeChecker`. I listed the 
enabled-by-default list with `CodeChecker checkers --only-enabled`, and made 
sure that the list didn't change after this patch. I also made sure that 
`CodeChecker checkers --only-enabled` is consistent with `clang [blahblah] 
-analyzer-list-enabled-checkers`, meaning that what `CodeChecker` requested was 
actually enabled, and nothing else (except modeling checkers, of course).

I guess adding a strictly developer only option to list the hidden checkers as 
well might be desirable, if we need to disable certain modeling checkers to get 
rid of some crashes.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60925



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

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Please regenerate the HTML docs using clang/docs/tools/dump_ast_matchers.py.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61480



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

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre updated this revision to Diff 197932.
Pierre marked 39 inline comments as done.
Pierre added a comment.

Requested changes have been made in this diff.  Some comments/ TODO have not 
been done yet. They will be done in a next patch.  This first patch is meant to 
start introducing the feature in Clang.
This patch integrates the new command line option introduced in 
https://reviews.llvm.org/D60764


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

https://reviews.llvm.org/D60763

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/OpenCLBuiltins.td
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/builtin-new.cl
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -80,6 +80,7 @@
 void EmitTestPragmaAttributeSupportedAttributes(llvm::RecordKeeper &Records,
 llvm::raw_ostream &OS);
 
+void EmitClangOpenCLBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 } // end namespace clang
 
 #endif
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -61,7 +61,8 @@
   GenDiagDocs,
   GenOptDocs,
   GenDataCollectors,
-  GenTestPragmaAttributeSupportedAttributes
+  GenTestPragmaAttributeSupportedAttributes,
+  GenClangOpenCLBuiltins,
 };
 
 namespace {
@@ -163,7 +164,9 @@
 clEnumValN(GenTestPragmaAttributeSupportedAttributes,
"gen-clang-test-pragma-attribute-supported-attributes",
"Generate a list of attributes supported by #pragma clang "
-   "attribute for testing purposes")));
+   "attribute for testing purposes"),
+clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
+   "Generate OpenCL builtin handlers")));
 
 cl::opt
 ClangComponent("clang-component",
@@ -293,6 +296,9 @@
   case GenTestPragmaAttributeSupportedAttributes:
 EmitTestPragmaAttributeSupportedAttributes(Records, OS);
 break;
+  case GenClangOpenCLBuiltins:
+EmitClangOpenCLBuiltins(Records, OS);
+break;
   }
 
   return false;
Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -0,0 +1,328 @@
+//===- ClangOpenCLBuiltinEmitter.cpp - Generate Clang OpenCL Builtin handling
+//=-*- C++ -*--=//
+//
+// The LLVM Compiler Infrastructure
+//
+// 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 tablegen backend emits code allowing to check whether a function
+// belongs to OpenCL builtin functions. In the following case, all overloads
+// of this function are added to the LookupResult.
+// The code is generated in "OpenCLBuiltins.inc" and included by Clang
+// SemaLookup.cpp
+//
+// The resolution of a function name and its overload is follwing these steps:
+// for the function "cos", which has the overloads:
+//- float   cos(float)
+//- double  cos(double)
+//
+// 1-  = isOpenCLBuiltin("cos")
+// 2- OpenCLBuiltins[Index - Index + Len] contains the pairs
+//   of the overloads of "cos".
+// 3- OpenCLSignature[SignatureIndex, SignatureIndex + SignatureLen] contains
+//  one of the signaures of "cos". This OpenCLSignature table can be
+//  referenced by other functions, i.e. "sin", since multiple functions
+//  can have the same signature.
+//===--===//
+
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include 
+
+using namespace llvm;
+
+namespace {
+class BuiltinNameEmitter {
+public:
+  BuiltinNameEmitter(RecordKeeper &Records, raw_ostream &OS)
+  : Records(Records), OS(OS) {}
+
+  // Entrypoint to generate the functions/ structures allowing to check
+  

[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 197934.
Anastasia marked an inline comment as done.
Anastasia added a comment.

Removed redundant C linkage checks!


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

https://reviews.llvm.org/D60454

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl
  test/SemaOpenCLCXX/kernel_invalid.cl

Index: test/SemaOpenCLCXX/kernel_invalid.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/kernel_invalid.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+
+struct C {
+  kernel void m(); //expected-error{{kernel functions cannot be class members}}
+};
+
+template 
+kernel void templ(T par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+template 
+kernel void bar(int par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+kernel void foo(int); //expected-note{{previous declaration is here}}
+
+kernel void foo(float); //expected-error{{conflicting types for 'foo'}}
Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- test/CodeGenOpenCLCXX/local_addrspace_init.cl
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
 
 // Test that we don't initialize local address space objects.
-//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
-//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+//CHECK: @_ZZ4testE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testE2ii = internal addrspace(3) global %class.C undef
 class C {
   int i;
 };
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -82,7 +82,7 @@
 // EXPL-LABEL: @__cxx_global_var_init()
 // EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
-// COMMON-LABEL: @_Z12test__globalv()
+// COMMON-LABEL: @test__global()
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
@@ -149,19 +149,19 @@
 
 TEST(__local)
 
-// COMMON-LABEL: _Z11test__localv
+// COMMON-LABEL: @test__local
 
 // Test that we don't initialize an object in local address space.
-// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
-// COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
-// EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* @_ZZ11test__localE1c to %class.C addrspace(4)*))
 // IMPL:  [[C1VOID:%[0-9]+]] = bitcast %class.C* %c1 to i8*
-// IMPL:  call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C1VOID]], i8 addrspace(4)* {{.*}}addrspacecast (i8 addrspace(3)* bitcast (%class.C addrspace(3)* @_ZZ11test__localvE1c to i8 addrspace(3)*) to i8 addrspace(4)*), i32 4, i1 false)
+// IMPL:  call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C1VOID]], i8 addrspace(4)* {{.*}}addrspacecast (i8 addrspace(3)* bitcast (%class.C addrspace(3)* @_ZZ11test__localE1c to i8 addrspace(3)*) to i8 addrspace(4)*), i32 4, i1 false)
 
 // Test the address space of 'this' when invoking a constructor.
 // EXPL: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
@@ -177,7 +177,7 @@
 
 TEST(__private)
 
-// CHECK-LABEL: @_Z13test__privatev
+// CHECK-LABEL: @test__private
 
 // Test the address space of 'this' when invoking a constructor for an object in non-default address space
 // EXPL: [[

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Pierre via Phabricator via cfe-commits]

Pierre added inline comments.



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:65
+class QualType {
+  // How to get the QualType. Can be one of ("field", "func")
+  string AccessMethod = _AccessMethod;

Anastasia wrote:
> I don't think "field" and "func" are clear at this point.
This field wasn't necessary and has been removed



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:211-212
+  def float_t   : Type<"float", QualType<"field", "FloatTy">>;
+  def double_t  : Type<"double", QualType<"field", "DoubleTy">>;
+  def half_t: Type<"half", QualType<"field", "HalfTy">>;
+}

Anastasia wrote:
> Nicola wrote:
> > Can half and double types and builtins be made dependent on extensions or 
> > configurable? The half datatype needs the cl_khr_fp16 extension, while 
> > double needs CL_DEVICE_DOUBLE_FP_CONFIG or cl_khr_fp64
> half and double types are already activated by an extension in Clang. This 
> behavior isn't modified here.
> 
> As for builtins there is `Extension` field in Builtin, but I think it's not 
> used in conversion functions at the moment. I guess we should update that.
An "Extension" field will be added in the next patch, so it will be possible to 
retrieve an extension of a prototype from the types the prototype is using.
E.g.: In the definition of the prototype half cos(half), the prototype will 
have the extension "cl_khr_fp16" because it uses the "half" type.

This scheme is present for the "half" and "double" types, but also for the 
"image2d_depth_t" types (extension "image2d_depth_t") and others.



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:306
+"atan", "atanh", "atanpi"] in {
+  foreach type = [float_t, double_t, half_t] in {  // TODO halfx is not part 
of the OpenCL1.2 spec
+defm : Bif1;

I removed the TODO and let the functions with the half prototypes. The fact 
that prototypes using the "half" types are part of the "cl_khr_fp16" extension 
(same for the "double" type, and other types) .



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:326
+// example 'foo', to show using 'version'
+def : Builtin<"foo_version", [int_t, PointerType]>;
+let Version = CL20 in {

Anastasia wrote:
> I think we now need to use some real function here
I changed it to a real function, but this should be changed anyway in the next 
patch



Comment at: clang/include/clang/Basic/OpenCLBuiltins.td:331
+
+// Example showing 'Extension'
+let Extension = "cl_khr_subgroups" in {

Anastasia wrote:
> I think this is no longer an example so we can change this something like 
> Built-in Subroups Extension...
> 
> Also this should probably moved to the bottom.
I would like to let it as an example for now because there is only one function 
using the Extension field



Comment at: clang/test/SemaOpenCL/builtin-new.cl:30
+kernel void test5(write_only image2d_t img, int2 coord, float4 colour) {
+  write_imagef(img, coord, colour);
+}

Anastasia wrote:
> I think different overloads can be available in different CL version. @svenvh 
> can we already handle this?
I wrote a TODO to check this later



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:58
+  //<, 35>.
+  std::vector, unsigned>> SignatureSet;
+

Anastasia wrote:
> I think it's worth checking whether inner vector is a good candidate for 
> using SmallVector of llvm. We can probably just use it with size 3 by default 
> since size is normally between 1-3. Although we could as well add a TODO and 
> address it later if we encounter performance problems.
I added a TODO



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:61
+  // Map the name of a builtin function to its signatures.
+  // Each signature is registered as a pair of:
+  //  Why not to just refer to the data structure above or we can alternatively 
> just typedef the vector?
I explained a bit more the purpose of the two fields in the comments.
**SignatureSet **is storing a list of . 
Tablegen is storing lists of types as objects that we can reference. 
**OverloadInfo **is storing, for each function having the same name, a list of  
the following pair:


Thus, this "**Index**" value allows functions with different names to have the 
same signature. By signature I mean the list of types is uses (for float 
cos(float), the signature would be [float, float])

I am not sure I answered the question, but I don't think it is possible to 
merge the SignatureSet and the OverloadInfo  structures



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:182
+
+  std::vector> Signature;
+

Anastasia wrote:
> Is this even used?
Actually no


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

https://reviews.llvm.org/D60763



_

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 197933.
baloghadamsoftware added a comment.

Dot added to the end of the comment.


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

https://reviews.llvm.org/D61422

Files:
  clang-tidy/bugprone/SizeofExpressionCheck.cpp
  test/clang-tidy/bugprone-sizeof-expression.cpp

Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -231,6 +231,35 @@
   return sum;
 }
 
+int Test6() {
+  int sum = 0;
+
+  struct S A = AsStruct(), B = AsStruct();
+  struct S *P = &A, *Q = &B;
+  sum += sizeof(struct S) == P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) != P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += sizeof(S) < P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) <= P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(*P) >= P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += Q - P > 3 * sizeof(*P);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += sizeof(S) + (P - Q);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) - (P - Q);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += (P - Q) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += (P - Q) / sizeof(*Q);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+
+  return sum;
+}
+
 int ValidExpressions() {
   int A[] = {1, 2, 3, 4};
   static const char str[] = "hello";
Index: clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -84,8 +84,11 @@
   const auto IntegerCallExpr = expr(ignoringParenImpCasts(
   callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
unless(isInTemplateInstantiation();
-  const auto SizeOfExpr =
-  expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr();
+  const auto SizeOfExpr = expr(anyOf(
+  sizeOfExpr(
+  has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type",
+  sizeOfExpr(has(expr(hasType(
+  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
   const auto SizeOfZero = expr(
   sizeOfExpr(has(ignoringParenImpCasts(expr(integerLiteral(equals(0)));
 
@@ -209,6 +212,36 @@
hasSizeOfDescendant(8, expr(SizeOfExpr, unless(SizeOfZero
   .bind("sizeof-sizeof-expr"),
   this);
+
+  // Detect sizeof in pointer aritmetic like: N * sizeof(S) == P1 - P2 or
+  // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S.
+  const auto PtrDiffExpr = binaryOperator(
+  hasOperatorName("-"),
+  hasLHS(expr(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
+  hasUnqualifiedDesugaredType(type().bind("left-ptr-type",
+  hasRHS(expr(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
+  hasUnqualifiedDesugaredType(type().bind("right-ptr-type");
+
+  Finder->addMatcher(
+  binaryOperator(
+  anyOf(hasOperatorName("=="), hasOperatorName("!="),
+hasOperatorName("<"), hasOperatorName("<="),
+hasOperatorName(">"), hasOperatorName(">="),
+hasOperatorName("+"), hasOperatorName("-")),
+  hasEitherOperand(expr(anyOf(
+  ignoringParenImpCasts(SizeOfExpr),
+  ignoringParenImpCasts(binaryOperator(
+  hasOperatorName("*"),
+  hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))),
+  hasEitherOperand(ignoringParenImpCasts(PtrDiffExpr)))
+  .bind("sizeof-in-ptr-arithmetic-mul"),
+  this);
+
+  Finder->addMatcher(binaryOperator(hasOperatorName("/"),
+hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
+hasRHS(ignoringParenImpCasts(SizeOfExpr)))
+ .bind("sizeof-in-ptr-arithmetic-div"),
+ this);
 }
 
 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
@@ -275,6 +308,26 @@
   } else if (const auto *E =
  Result.Nodes.getNodeAs("sizeof-multiply-sizeof")) {
 diag(E->getBeginLoc(), "susp

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D61422#1488081 , @aaron.ballman 
wrote:

> Out of curiosity, have you run this over any large code bases to see what the 
> false positive and true positive rate is?


Neither false, nor true positives found so far. I ran it on several open-source 
projects.


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

https://reviews.llvm.org/D61422



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

[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D60454#1488348 , @rjmccall wrote:

> I think it would be more reasonable to just change `getDeclLanguageLinkage` 
> to check for a kernel function.


I tried to change `getDeclLanguageLinkage` only but Sema calls 
`isInExternCContext` separately while giving diagnostics. 
`getDeclLanguageLinkage` also calls it through `isFirstInExternCContext`. So it 
seems we just need to check in `isInExternCContext` and that is a common part 
for all various program paths. Do you agree?




Comment at: lib/AST/Decl.cpp:2940
+  if (hasAttr())
+return true;
   return isDeclExternC(*this);

rjmccall wrote:
> Both of these changes should be unnecessary because they ultimately defer to 
> `isInExternCContext`.
> 
> I assume that OpenCL bans making a class member function a kernel?
> I assume that OpenCL bans making a class member function a kernel?

Yep, that's right! I am adding a diagnostic in this patch.


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

https://reviews.llvm.org/D60454



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

[PATCH] D57464: Generalize method overloading on addr spaces to C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D57464#1462342 , @ebevhan wrote:

> In D57464#1438213 , @Anastasia wrote:
>
> > > I think I would lean towards the latter since it means less fudging 
> > > around with a whole bunch of unrelated methods. Do @rjmccall or @rsmith 
> > > have any further opinions on this?
> >
> > Ok, I can change the patch to prototype this approach. I might need some 
> > example test cases though.
>
>
> Alright!
>
> Just to make sure of something here - are you waiting for me to provide some 
> example cases? There hasn't been activity here in a while and I was just 
> wondering if it was because you were waiting for this.


Sorry for delay. Examples would be helpful. But I am not blocked on them, I 
just can't find extra bandwidth at the moment unfortunately and I am not sure I 
will be able to do this in the time frame of clang9 development. Feel free to 
pick it up if you have time and I will be happy to review your rework. 
Otherwise it would have to wait. :(


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

https://reviews.llvm.org/D57464



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

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

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

Szelethus added a comment.

  $ cat orig.cpp
  #define a(b) switch (b) {
  #define c(b) \
  b:
  int d;
  e() {
  a(d) c(2);
  c(3)

  $ debugBuild/bin/clang-tidy orig.cpp -checks=bugprone-branch-clone --
  clang-tidy: ../extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:200: 
clang::DiagnosticBuilder clang::tidy::ClangTidyContext::diag(llvm::StringRef, 
clang::SourceLocation, llvm::StringRef, DiagnosticIDs::Level): Assertion 
`Loc.isValid()' failed.
  #0 0x7f263c30ee79 llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
debugBuild/../llvm/lib/Support/Unix/Signals.inc:494:11 #1 0x7f263c30f029 
PrintStackTraceSignalHandler(void*) 
debugBuild/../llvm/lib/Support/Unix/Signals.inc:558:1
  #2 0x7f263c30d936 llvm::sys::RunSignalHandlers() 
debugBuild/../llvm/lib/Support/Signals.cpp:67:5 
  #3 0x7f263c30f6db SignalHandler(int) 
debugBuild/../llvm/lib/Support/Unix/Signals.inc:357:1 #4 0x7f263bd64890 
__restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
  #5 0x7f2637aefe97 gsignal 
/build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0 
  #6 0x7f2637af1801 abort 
/build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0 
  #7 0x7f2637ae139a __assert_fail_base 
/build/glibc-OTsEL5/glibc-2.27/assert/assert.c:89:0 
  #8 0x7f2637ae1412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412) 
  #9 0x7f263a64c623 clang::tidy::ClangTidyContext::diag(llvm::StringRef, 
clang::SourceLocation, llvm::StringRef, clang::DiagnosticIDs::Level) 
debugBuild/../extra/clang-tidy/Cl
  angTidyDiagnosticConsumer.cpp:201:17 #10 0x7f263a647baf 
clang::tidy::ClangTidyCheck::diag(clang::SourceLocation, llvm::StringRef, 
clang::DiagnosticIDs::Level) 
debugBuild/../extra/clang-tidy/ClangTidyCheck.cpp:23
  :3 #11 0x7f263a28671e 
clang::tidy::bugprone::BranchCloneCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) debugBuild/../extra/clang-tidy/bugprone/BranchCloneCheck.cpp:200:9 
  #12 0x7f263a647be9 
clang::tidy::ClangTidyCheck::run(clang::ast_matchers::MatchFinder::MatchResult 
const&) debugBuild/../extra/clang-tidy/ClangTidyCheck.cpp:31:1 #13 
0x7f263ae60174 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::MatchVisitor::visitMatch(clang::ast_matchers::BoundNodes
 const&) debugBuild/../clang
  /lib/ASTMatchers/ASTMatchFinder.cpp:742:7 
  #14 0x7f263af4e699 
clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches(clang::ast_matchers::internal::BoundNodesTreeBuilder::Visitor*)
 debugBuild/../clang/lib/
  ASTMatchers/ASTMatchersInternal.cpp:75:5 #15 0x7f263ae5fe66 
clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::matchWithFilter(clang::ast_type_traits::DynTypedNode
 const&) debugBuild/../clang/lib
  /ASTMatchers/ASTMatchFinder.cpp:574:7 
  #16 0x7f263ae601c5 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::matchDispatch(clang::Stmt const*) 
debugBuild/../clang/lib/ASTMatchers/ASTMatchFinder
  .cpp:597:5
  #17 0x7f263ae5fa7d void clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::match(clang::Stmt const&) 
debugBuild/../clang/lib/ASTMatchers/ASTMatchFinder.cpp:499:3
  #18 0x7f263ae6dedd clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::TraverseStmt(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo > > >*) 
debugBuild/../clang/lib/ASTMatchers/ASTMatchFinder.cpp:868:48
  #19 0x7f263ae904b5 
clang::RecursiveASTVisitor::TraverseCompoundStmt(clang::CompoundStmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo > > >*) 
debugBuild/../clang/include/clang/AST/RecursiveASTVisitor.h:2175:1
  #20 0x7f263ae806a2 
clang::RecursiveASTVisitor::dataTraverseNode(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo > > >*) 
debugBuild/tools/clang/include/clang/AST/StmtNodes.inc:73:1
  #21 0x7f263ae7c95e 
clang::RecursiveASTVisitor::TraverseStmt(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo > > >*) 
debugBuild/../clang/include/clang/AST/RecursiveASTVisitor.h:655:7
  #22 0x7f263ae6def9 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::TraverseStmt(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo > > >*) 
debugBuild/../clang/lib/ASTMatchers/ASTMatchFinder.cpp:868:3
  #23 0x7f263aeb71ca 
clang::RecursiveASTVisitor::TraverseFunctionHelper(clang::FunctionDecl*) 
debugBuild/../clang/include/clang/AST/RecursiveASTVisitor.h:2020:5
  #24 0x7f263ae677be 
clang::RecursiveASTVisitor::TraverseFunctionDecl(clang::FunctionDecl*) 
debugBuild/../clang/include/clang/AST/RecursiveASTVisitor.h:2025:1
  #25 0x7f263ae61df3 
clang::RecursiveASTVisitor::TraverseDecl(clang::Decl*) 
debugBuild/tools/clang/include/clang/AST/DeclNodes.inc:389:1
  #26 0x7f263ae61281 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::TraverseDecl(clang::Decl*) 
debugBuild/../clang/lib/ASTMatchers/ASTMatchFinder.cpp:860:3
  #27 0x7f263ae6dc58 
clang::Recursive

[PATCH] D61318: [Sema] Prevent binding references with mismatching address spaces to temporaries

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 197940.
Anastasia added a comment.

Updated comments!


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

https://reviews.llvm.org/D61318

Files:
  include/clang/AST/Type.h
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Initialization.h
  lib/Sema/SemaInit.cpp
  test/SemaOpenCLCXX/address-space-references.cl

Index: test/SemaOpenCLCXX/address-space-references.cl
===
--- test/SemaOpenCLCXX/address-space-references.cl
+++ test/SemaOpenCLCXX/address-space-references.cl
@@ -9,3 +9,7 @@
 void foo() {
   bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
+
+__global const int& f(__global float &ref) {
+  return ref; // expected-error{{reference of type 'const __global int &' cannot bind to a temporary object because of address space mismatch}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -3336,6 +3336,7 @@
   case FK_NonConstLValueReferenceBindingToVectorElement:
   case FK_NonConstLValueReferenceBindingToUnrelated:
   case FK_RValueReferenceBindingToLValue:
+  case FK_ReferenceAddrspaceMismatchTemporary:
   case FK_ReferenceInitDropsQualifiers:
   case FK_ReferenceInitFailed:
   case FK_ConversionFailed:
@@ -4831,9 +4832,16 @@
 
   Sequence.AddReferenceBindingStep(cv1T1IgnoreAS, /*bindingTemporary=*/true);
 
-  if (T1Quals.hasAddressSpace())
+  if (T1Quals.hasAddressSpace()) {
+if (!Qualifiers::isAddressSpaceSupersetOf(T1Quals.getAddressSpace(),
+  LangAS::Default)) {
+  Sequence.SetFailed(
+  InitializationSequence::FK_ReferenceAddrspaceMismatchTemporary);
+  return;
+}
 Sequence.AddQualificationConversionStep(cv1T1, isLValueRef ? VK_LValue
: VK_XValue);
+  }
 }
 
 /// Attempt character array initialization from a string literal
@@ -8497,6 +8505,11 @@
   << Args[0]->getSourceRange();
 break;
 
+  case FK_ReferenceAddrspaceMismatchTemporary:
+S.Diag(Kind.getLocation(), diag::err_reference_bind_temporary_addrspace)
+<< DestType << Args[0]->getSourceRange();
+break;
+
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = OnlyArg->getType();
 QualType NonRefType = DestType.getNonReferenceType();
@@ -8832,6 +8845,10 @@
   OS << "reference initialization drops qualifiers";
   break;
 
+case FK_ReferenceAddrspaceMismatchTemporary:
+  OS << "reference with mismatching address space bound to temporary";
+  break;
+
 case FK_ReferenceInitFailed:
   OS << "reference initialization failed";
   break;
Index: include/clang/Sema/Initialization.h
===
--- include/clang/Sema/Initialization.h
+++ include/clang/Sema/Initialization.h
@@ -1010,6 +1010,9 @@
 /// Reference binding drops qualifiers.
 FK_ReferenceInitDropsQualifiers,
 
+/// Reference with mismatching address space binding to temporary.
+FK_ReferenceAddrspaceMismatchTemporary,
+
 /// Reference binding failed.
 FK_ReferenceInitFailed,
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1840,6 +1840,9 @@
   "reference %diff{to %select{type|incomplete type}1 $ could not bind to an "
   "%select{rvalue|lvalue}2 of type $|could not bind to %select{rvalue|lvalue}2 of "
   "incompatible type}0,3">;
+def err_reference_bind_temporary_addrspace : Error<
+  "reference of type %0 cannot bind to a temporary object because of "
+  "address space mismatch">;
 def err_reference_bind_init_list : Error<
   "reference to type %0 cannot bind to an initializer list">;
 def err_init_list_bad_dest_type : Error<
Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@
   DIAG_SIZE_AST   =  150,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 3600,
   DIAG_SIZE_ANALYSIS  =  100,
   DIAG_SIZE_REFACTORING   = 1000,
 };
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -460,21 +460,25 @@
 Mask |= qs.Mask;
   }
 
-  /// Returns true if this address space is a superset of the other one.
+  /// Returns true if address space A is equal to or a superset of B.
   /// OpenCL v2.0 defines conversio

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

[Jonas Toth via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61475



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:186
   void findHover(PathRef File, Position Pos,
- Callback> CB);
+ Callback> CB);
 

sammccall wrote:
> Not sure about switching from `Hover` to `FormattedString` as return type 
> here: LSP hover struct contains a range field (what are the bounds of the 
> hovered thing?) that we may want to populate that doesn't fit in 
> FormattedString.
> I'd suggest the function in XRefs.h should return `FormattedString` (and 
> later maybe `pair`), and the `ClangdServer` version 
> should continue to provide the rendered `Hover`.
> 
> (We'll eventually want ClangdServer to return some HoverInfo struct with 
> structured information, as we want embedding clients to be able to render it 
> other ways, and maybe use it to provide extension methods like YCM's 
> `GetType`. But no need to touch that in this patch, we'll end up producing 
> HoverInfo -> FormattedString -> LSP Hover, I guess)
I agree with Sam on the layering. I was also working in a patch that was 
changing `ClangdServer`s output from `Hover` to `HoverInfo`(a structured 
version of `Hover`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 197948.
martong added a comment.

- Log and do not assert anywhere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/ASTMerge.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/ClangASTImporter.cpp
  lldb/source/Symbol/CxxModuleHandler.cpp

Index: lldb/source/Symbol/CxxModuleHandler.cpp
===
--- lldb/source/Symbol/CxxModuleHandler.cpp
+++ lldb/source/Symbol/CxxModuleHandler.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Symbol/CxxModuleHandler.h"
 
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Utility/Log.h"
 #include "clang/Sema/Lookup.h"
 #include "llvm/Support/Error.h"
 
@@ -214,12 +215,16 @@
   // Import the foreign template arguments.
   llvm::SmallVector imported_args;
 
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
   // If this logic is changed, also update templateArgsAreSupported.
   for (const TemplateArgument &arg : foreign_args.asArray()) {
 switch (arg.getKind()) {
 case TemplateArgument::Type: {
-  llvm::Expected type = m_importer->Import_New(arg.getAsType());
+  llvm::Expected type = m_importer->Import(arg.getAsType());
   if (!type) {
+if (log)
+  log->Printf("Couldn't import type!");
 llvm::consumeError(type.takeError());
 return {};
   }
@@ -229,8 +234,10 @@
 case TemplateArgument::Integral: {
   llvm::APSInt integral = arg.getAsIntegral();
   llvm::Expected type =
-  m_importer->Import_New(arg.getIntegralType());
+  m_importer->Import(arg.getIntegralType());
   if (!type) {
+if (log)
+  log->Printf("Couldn't import type!");
 llvm::consumeError(type.takeError());
 return {};
   }
Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -62,8 +62,16 @@
 
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, dst_ast);
 
-  if (delegate_sp)
-return delegate_sp->Import(type);
+  if (delegate_sp) {
+if (llvm::Expected ret_or_error = delegate_sp->Import(type))
+  return *ret_or_error;
+else {
+  if (Log *log =
+  lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS))
+log->Printf("Couldn't import type!");
+  llvm::consumeError(ret_or_error.takeError());
+}
+  }
 
   return QualType();
 }
@@ -106,7 +114,7 @@
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, dst_ast);
 
   if (delegate_sp) {
-clang::Decl *result = delegate_sp->Import(decl);
+llvm::Expected result = delegate_sp->Import(decl);
 
 if (!result) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
@@ -127,9 +135,13 @@
   "metadata 0x%" PRIx64,
   decl->getDeclKindName(), user_id);
   }
+
+  llvm::consumeError(result.takeError());
+
+  return nullptr;
 }
 
-return result;
+return *result;
   }
 
   return nullptr;
@@ -641,7 +653,11 @@
 TagDecl *origin_tag_decl = llvm::dyn_cast(decl_origin.decl);
 
 for (Decl *origin_child_decl : origin_tag_decl->decls()) {
-  delegate_sp->Import(origin_child_decl);
+  llvm::Expected imported_or_err =
+  delegate_sp->Import(origin_child_decl);
+  if (!imported_or_err)
+// FIXME return with false?
+consumeError(imported_or_err.takeError());
 }
 
 if (RecordDecl *record_decl = dyn_cast(origin_tag_decl)) {
@@ -666,7 +682,11 @@
   llvm::dyn_cast(decl_origin.decl);
 
   for (Decl *origin_child_decl : origin_interface_decl->decls()) {
-delegate_sp->Import(origin_child_decl);
+llvm::Expected imported_or_err =
+delegate_sp->Import(origin_child_decl);
+if (!imported_or_err)
+  // FIXME return with false?
+  consumeError(imported_or_err.takeError());
   }
 
   return true;
@@ -919,7 +939,15 @@
   to_cxx_record->startDefinition();
   */
 
-  ImportDefinition(from);
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
+  llvm::Error Err = ImportDefinition(from);
+  if (Err) {
+if (log)
+  log->Printf("[ClangASTImporter] Error during importing definition!");
+return;
+  }
+
 
   if (clang::TagDecl *to_tag = dyn_cast(to)) {
 if (clang::TagDecl *from_tag = dyn_cast(from)) {
@@ -949,13 +977,18 @@
   if 

[PATCH] D61485: Added an assert in `isConstantInitializer`: initializer lists must be in semantic form

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61485

Files:
  clang/lib/AST/Expr.cpp


Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2947,6 +2947,7 @@
   }
   case InitListExprClass: {
 const InitListExpr *ILE = cast(this);
+assert(ILE->isSemanticForm() && "InitListExpr must be in semantic form");
 if (ILE->getType()->isArrayType()) {
   unsigned numInits = ILE->getNumInits();
   for (unsigned i = 0; i < numInits; i++) {


Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2947,6 +2947,7 @@
   }
   case InitListExprClass: {
 const InitListExpr *ILE = cast(this);
+assert(ILE->isSemanticForm() && "InitListExpr must be in semantic form");
 if (ILE->getType()->isArrayType()) {
   unsigned numInits = ILE->getNumInits();
   for (unsigned i = 0; i < numInits; i++) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61486: [Basic] Introduce active dummy DiagnosticBuilder

[Nikolai Kosjar via Phabricator via cfe-commits]

nik created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...which does not emit anything.

This dummy is useful for filtering. Code expecting a DiagnosticBuilder
object can get a dummy and report further details, without
knowing/checking whether this is needed at all.


Repository:
  rC Clang

https://reviews.llvm.org/D61486

Files:
  include/clang/Basic/Diagnostic.h


Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -1055,6 +1055,9 @@
   // Emit() would end up with if we used that as our status variable.
   mutable bool IsActive = false;
 
+  /// Flag indicating an active dummy builder that will not emit anything.
+  mutable bool IsDummy = false;
+
   /// Flag indicating that this diagnostic is being emitted via a
   /// call to ForceEmit.
   mutable bool IsForceEmit = false;
@@ -1077,6 +1080,7 @@
   void Clear() const {
 DiagObj = nullptr;
 IsActive = false;
+IsDummy = false;
 IsForceEmit = false;
   }
 
@@ -1095,6 +1099,12 @@
 // (or by a subclass, as in SemaDiagnosticBuilder).
 if (!isActive()) return false;
 
+if (IsDummy) {
+  DiagObj->Clear();
+  Clear();
+  return false;
+}
+
 // When emitting diagnostics, we set the final argument count into
 // the DiagnosticsEngine object.
 FlushCounts();
@@ -1114,6 +1124,7 @@
   DiagnosticBuilder(const DiagnosticBuilder &D) {
 DiagObj = D.DiagObj;
 IsActive = D.IsActive;
+IsDummy = D.IsDummy;
 IsForceEmit = D.IsForceEmit;
 D.Clear();
 NumArgs = D.NumArgs;
@@ -1131,6 +1142,13 @@
 return {};
   }
 
+  /// Retrieve an active diagnostic builder that will not emit anything.
+  static DiagnosticBuilder getDummy(DiagnosticsEngine *diagObj) {
+DiagnosticBuilder D(diagObj);
+D.IsDummy = true;
+return D;
+  }
+
   /// Forces the diagnostic to be emitted.
   const DiagnosticBuilder &setForceEmit() const {
 IsForceEmit = true;


Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -1055,6 +1055,9 @@
   // Emit() would end up with if we used that as our status variable.
   mutable bool IsActive = false;
 
+  /// Flag indicating an active dummy builder that will not emit anything.
+  mutable bool IsDummy = false;
+
   /// Flag indicating that this diagnostic is being emitted via a
   /// call to ForceEmit.
   mutable bool IsForceEmit = false;
@@ -1077,6 +1080,7 @@
   void Clear() const {
 DiagObj = nullptr;
 IsActive = false;
+IsDummy = false;
 IsForceEmit = false;
   }
 
@@ -1095,6 +1099,12 @@
 // (or by a subclass, as in SemaDiagnosticBuilder).
 if (!isActive()) return false;
 
+if (IsDummy) {
+  DiagObj->Clear();
+  Clear();
+  return false;
+}
+
 // When emitting diagnostics, we set the final argument count into
 // the DiagnosticsEngine object.
 FlushCounts();
@@ -1114,6 +1124,7 @@
   DiagnosticBuilder(const DiagnosticBuilder &D) {
 DiagObj = D.DiagObj;
 IsActive = D.IsActive;
+IsDummy = D.IsDummy;
 IsForceEmit = D.IsForceEmit;
 D.Clear();
 NumArgs = D.NumArgs;
@@ -1131,6 +1142,13 @@
 return {};
   }
 
+  /// Retrieve an active diagnostic builder that will not emit anything.
+  static DiagnosticBuilder getDummy(DiagnosticsEngine *diagObj) {
+DiagnosticBuilder D(diagObj);
+D.IsDummy = true;
+return D;
+  }
+
   /// Forces the diagnostic to be emitted.
   const DiagnosticBuilder &setForceEmit() const {
 IsForceEmit = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked 2 inline comments as done.
gribozavr added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2079
+}
+
 TEST(EqualsBoundNodeMatcher, QualType) {

aaron.ballman wrote:
> Can you also add a negative test case for:
> ```
> namespace foo {
> namespace std {
>   void bar(); // isInStdNamespace() is false here, right?
> }
> }
> ```
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61480



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

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 197950.
gribozavr added a comment.

Regenerated documentation and added one more test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61480

Files:
  clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2031,6 +2031,57 @@
   EXPECT_TRUE(matches("namespace {}", namespaceDecl(isAnonymous(;
 }
 
+TEST(DeclarationMatcher, InStdNamespace) {
+  EXPECT_TRUE(notMatches("class vector {};"
+ "namespace foo {"
+ "  class vector {};"
+ "}"
+ "namespace foo {"
+ "  namespace std {"
+ "class vector {};"
+ "  }"
+ "}",
+ cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+
+  EXPECT_TRUE(matches("namespace std {"
+  "  class vector {};"
+  "}",
+  cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+  EXPECT_TRUE(matches("namespace std {"
+  "  inline namespace __1 {"
+  "class vector {};"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("vector"), isInStdNamespace(;
+  EXPECT_TRUE(notMatches("namespace std {"
+ "  inline namespace __1 {"
+ "inline namespace __fs {"
+ "  namespace filesystem {"
+ "inline namespace v1 {"
+ "  class path {};"
+ "}"
+ "  }"
+ "}"
+ "  }"
+ "}",
+ cxxRecordDecl(hasName("path"), isInStdNamespace(;
+  EXPECT_TRUE(
+  matches("namespace std {"
+  "  inline namespace __1 {"
+  "inline namespace __fs {"
+  "  namespace filesystem {"
+  "inline namespace v1 {"
+  "  class path {};"
+  "}"
+  "  }"
+  "}"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("path"),
+hasAncestor(namespaceDecl(hasName("filesystem"),
+  isInStdNamespace());
+}
+
 TEST(EqualsBoundNodeMatcher, QualType) {
   EXPECT_TRUE(matches(
 "int i = 1;", varDecl(hasType(qualType().bind("type")),
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -366,6 +366,7 @@
   REGISTER_MATCHER(isExternC);
   REGISTER_MATCHER(isFinal);
   REGISTER_MATCHER(isImplicit);
+  REGISTER_MATCHER(isInStdNamespace);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isInline);
   REGISTER_MATCHER(isInstanceMessage);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -354,7 +354,8 @@
 }
 
 bool Decl::isInStdNamespace() const {
-  return getDeclContext()->isStdNamespace();
+  const DeclContext *DC = getDeclContext();
+  return DC && DC->isStdNamespace();
 }
 
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6212,6 +6212,29 @@
   return Node.isAnonymousNamespace();
 }
 
+/// Matches declarations in the namespace `std`, but not in nested namespaces.
+///
+/// Given
+/// \code
+///   class vector {};
+///   namespace foo {
+/// class vector {};
+/// namespace std {
+///   class vector {};
+/// }
+///   }
+///   namespace std {
+/// inline namespace __1 {
+///   class vector {}; // #1
+///   namespace experimental {
+/// class vector {};
+///   }
+/// }
+///   }
+/// \endcode
+/// cxxRecordDecl(hasName("vector"), isInStdNamespace()) will match only #1.
+AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
+
 /// If the given case stateme

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

The clang-tidy standalone tool implements the NOLINT filtering in
ClangTidyDiagnosticConsumer::HandleDiagnostic. For the plugin case no
ClangTidyDiagnosticConsumer is set up as it would have side effects with
the already set up diagnostic consumer.

This change introduces filtering in ClangTidyContext::diag() and returns
an active dummy DiagnosticBuilder in case the check was NOLINT-ed, so
that no check needs to be adapted. The filtering in HandleDiagnostic
needs to stay there as it is still relevant for non-tidy diagnostics.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61487

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  test/clang-tidy/nolint-plugin.cpp

Index: test/clang-tidy/nolint-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,49 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i'
+  int j; // NOLINT
+  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -215,6 +215,8 @@
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+
+  bool LastErrorWasIgnored;
 };
 
 /// \brief A diagnostic consumer that turns each \c Diagnostic into a
@@ -255,7 +257,6 @@
   std::unique_ptr HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
-  bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -186,7 +186,8 @@
 bool AllowEnablingAnalyzerAlphaCheckers)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
   Profile(false),
-  AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
+  AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+  LastErrorWasIgnored(false) {
   // Before the first translation unit we can get errors related to command-line
   // parsing, use empty string for the file name in this case.
   setCurrentFile("");
@@ -194,12 +195,112 @@
 
 ClangTidyContext::~ClangTidyContext() = default;
 
+static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
+  unsigned DiagID, const ClangTidyContext &Context) {
+  const size_t NolintIndex = Line.find(NolintDirectiveText);
+  if (NolintIndex == StringRef::npos)
+return false;
+
+  size_t BracketIndex = NolintIndex + NolintDirectiveText.size();
+  // Check if the specific checks are specified in brackets.
+  if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+++BracketIndex;
+const size_t BracketEndIndex = Line.find(')', BracketIndex);
+if (BracketEndIndex != StringRef::npos) {
+  StringRef ChecksStr =
+  Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
+  // Allow disabling all the checks with "*".
+  if (ChecksStr != "*") {
+std::string CheckName = Context.getCheckName(DiagID);
+// Allow specif

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

This one depends on https://reviews.llvm.org/D61486


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Thanks!

mostly LGTM, only confused about changing symbolcollector to produce a unique 
header per symbol(where as it was(could) produce multiple headers before).
What is the rationale behind that one?




Comment at: clangd/index/SymbolCollector.h:130
+  // The final spelling is calculated in finish().
+  llvm::DenseMap IncludeFiles;
+  // Indexed macros, to be erased if they turned out to be include guards.

This is losing ability to store multiple header files. Is that intentional? 



Comment at: clangd/unittests/SymbolCollectorTests.cpp:1043
+int decl();
+#define MACRO
+

can you also add a case where we first see `MACRO` and then `decl`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61442



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Did you forget to push the change to c-index-test.c?

Otherwise fine with me.


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

https://reviews.llvm.org/D60678



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Apart from NOLINT handling there's more logic in 
`ClangTidyDiagnosticConsumer::HandleDiagnostic`, which isn't properly 
transferred to `ClangTidyContext::diag` in this patch. The logic that is 
transferred seems to change the behavior w.r.t. notes that can "unmute" ignored 
warnings (see https://reviews.llvm.org/D59135#1456108). I suspect that we're 
missing proper test coverage here. Another issue is that compiler diagnostics 
don't pass ClangTidyContext::diag in the non-plugin use case. Do all the 
existing tests pass with your patch?

A better way to implement diagnostic filtering in the plugin would be to make 
ClangTidyDiagnosticConsumer able to forward diagnostics to the external 
diagnostics engine. It will still need some sort of a buffering though to 
handle diagnostics and notes attached to them together.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: kpet, rjmccall.
Herald added subscribers: ebevhan, yaxunl.

We need to be able to enqueue internal function that initializes global 
constructors on the host side. Therefore it has to be converted to a kernel.

Note, supporting destruction would need some more work. However, it seems 
global destruction has little meaning without any dynamic resource allocation 
on the device and program scope variables are destroyed by the runtime when 
program is released.


https://reviews.llvm.org/D61488

Files:
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenOpenCLCXX/global_init.cl

Index: test/CodeGenOpenCLCXX/global_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/global_init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct S {
+  S() {}
+};
+
+S s;
+
+//CHECK: define internal spir_func void @_GLOBAL__sub_I_{{.*}}!kernel_arg_addr_space [[ARGMD:![0-9]+]] !kernel_arg_access_qual [[ARGMD]] !kernel_arg_type [[ARGMD]] !kernel_arg_base_type [[ARGMD]] !kernel_arg_type_qual [[ARGMD]]
+// Check that parameters are empty.
+//CHECK: [[ARGMD]] = !{}
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1315,6 +1315,19 @@
   llvm::Value *
   createOpenCLIntToSamplerConversion(const Expr *E, CodeGenFunction &CGF);
 
+  /// OpenCL v1.2 s5.6.4.6 allows the compiler to store kernel argument
+  /// information in the program executable. The argument information stored
+  /// includes the argument name, its type, the address and access qualifiers
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If kernel is generated
+  /// implicitly null value have to be passed to the last two parameter,
+  /// otherwise all parameters must have valid non-null values.
+  /// \param FN is a pointer to IR function being generated.
+  /// \param FD is a pointer to function declaration if any.
+  /// \param CGF is a pointer to CodeGenFunction that generates this function.
+  void GenOpenCLArgMetadata(llvm::Function *Fn, const FunctionDecl *FD=nullptr,  
+ CodeGenFunction *CGF=nullptr);
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1187,6 +1187,212 @@
   F->setCallingConv(static_cast(CallingConv));
 }
 
+static void removeImageAccessQualifier(std::string& TyName) {
+  std::string ReadOnlyQual("__read_only");
+  std::string::size_type ReadOnlyPos = TyName.find(ReadOnlyQual);
+  if (ReadOnlyPos != std::string::npos)
+// "+ 1" for the space after access qualifier.
+TyName.erase(ReadOnlyPos, ReadOnlyQual.size() + 1);
+  else {
+std::string WriteOnlyQual("__write_only");
+std::string::size_type WriteOnlyPos = TyName.find(WriteOnlyQual);
+if (WriteOnlyPos != std::string::npos)
+  TyName.erase(WriteOnlyPos, WriteOnlyQual.size() + 1);
+else {
+  std::string ReadWriteQual("__read_write");
+  std::string::size_type ReadWritePos = TyName.find(ReadWriteQual);
+  if (ReadWritePos != std::string::npos)
+TyName.erase(ReadWritePos, ReadWriteQual.size() + 1);
+}
+  }
+}
+
+// Returns the address space id that should be produced to the
+// kernel_arg_addr_space metadata. This is always fixed to the ids
+// as specified in the SPIR 2.0 specification in order to differentiate
+// for example in clGetKernelArgInfo() implementation between the address
+// spaces with targets without unique mapping to the OpenCL address spaces
+// (basically all single AS CPUs).
+static unsigned ArgInfoAddressSpace(LangAS AS) {
+  switch (AS) {
+  case LangAS::opencl_global:   return 1;
+  case LangAS::opencl_constant: return 2;
+  case LangAS::opencl_local:return 3;
+  case LangAS::opencl_generic:  return 4; // Not in SPIR 2.0 specs.
+  default:
+return 0; // Assume private.
+  }
+}
+
+void CodeGenModule::GenOpenCLArgMetadata(llvm::Function *Fn,
+ const FunctionDecl *FD,
+ CodeGenFunction *CGF) {
+  assert(((FD && CGF) || (!FD && !CGF)) &&
+ "Incorrect use - FD and CFG should either be both null or not!");
+  // Create MDNodes that represent the kernel arg metadata.
+  // Each MDNode is a list in the form of "key", N number of values which is
+  // the same number of values as their are kernel arguments.
+
+  const PrintingPolicy &Policy = Context.getPrin

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CodeGenModule.cpp:1231
+  assert(((FD && CGF) || (!FD && !CGF)) &&
+ "Incorrect use - FD and CFG should either be both null or not!");
+  // Create MDNodes that represent the kernel arg metadata.

CFG -> CGF


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

https://reviews.llvm.org/D61488



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

[clang-tools-extra] r359874 - [clangd] Also perform merging for symbol definitions

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Fri May  3 05:11:14 2019
New Revision: 359874

URL: http://llvm.org/viewvc/llvm-project?rev=359874&view=rev
Log:
[clangd] Also perform merging for symbol definitions

Summary:
clangd currently prefers declarations from codegen files. This patch
implements that behavior for definition locations. If we have definiton
locations both coming from AST and index, clangd will perform a merging to show
the codegen file if that's the case.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=359874&r1=359873&r2=359874&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri May  3 05:11:14 2019
@@ -346,24 +346,27 @@ std::vector locateSymbolA
 Index->lookup(QueryRequest, [&](const Symbol &Sym) {
   auto &R = Result[ResultIndex.lookup(Sym.ID)];
 
-  // Special case: if the AST yielded a definition, then it may not be
-  // the right *declaration*. Prefer the one from the index.
   if (R.Definition) { // from AST
+// Special case: if the AST yielded a definition, then it may not be
+// the right *declaration*. Prefer the one from the index.
 if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
   R.PreferredDeclaration = *Loc;
+
+// We might still prefer the definition from the index, e.g. for
+// generated symbols.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
+*MainFilePath))
+  R.Definition = *Loc;
   } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
 
-if (Sym.CanonicalDeclaration) {
-  // Use merge logic to choose AST or index declaration.
-  // We only do this for declarations as definitions from AST
-  // is generally preferred (e.g. definitions in main file).
-  if (auto Loc = toLSPLocation(
-  getPreferredLocation(R.PreferredDeclaration,
-   Sym.CanonicalDeclaration, Scratch),
-  *MainFilePath))
-R.PreferredDeclaration = *Loc;
-}
+// Use merge logic to choose AST or index declaration.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration, Scratch),
+*MainFilePath))
+  R.PreferredDeclaration = *Loc;
   }
 });
   }

Modified: clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp?rev=359874&r1=359873&r2=359874&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp Fri May  3 05:11:14 
2019
@@ -186,7 +186,8 @@ TEST(LocateSymbol, WithIndex) {
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,25 @@ TEST(LocateSymbol, WithIndexPreferredLoc
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Proto;
+void func() {}
+P$p^roto* create() {
+  fu$f^nc();
+  return nullptr;
+}
   )cpp");
 
   auto AST = TestTU::withCode(Test.code()).build();
-  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
-  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("p"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("p");
+EXPECT_THAT(Locs, ElementsAre(Sym("Proto", CodeGenLoc, CodeGenLoc)));
+  }
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("f"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("f");
+EXPECT_THAT(Locs, ElementsAre(Sym("func", CodeGenLoc, CodeGenLoc)));
+  }
 }
 
 TEST(LocateSymbol, All) {


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

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked 2 inline comments as done.
Closed by commit rL359874: [clangd] Also perform merging for symbol definitions 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61126?vs=197725&id=197963#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61126

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -346,24 +346,27 @@
 Index->lookup(QueryRequest, [&](const Symbol &Sym) {
   auto &R = Result[ResultIndex.lookup(Sym.ID)];
 
-  // Special case: if the AST yielded a definition, then it may not be
-  // the right *declaration*. Prefer the one from the index.
   if (R.Definition) { // from AST
+// Special case: if the AST yielded a definition, then it may not be
+// the right *declaration*. Prefer the one from the index.
 if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
   R.PreferredDeclaration = *Loc;
+
+// We might still prefer the definition from the index, e.g. for
+// generated symbols.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
+*MainFilePath))
+  R.Definition = *Loc;
   } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
 
-if (Sym.CanonicalDeclaration) {
-  // Use merge logic to choose AST or index declaration.
-  // We only do this for declarations as definitions from AST
-  // is generally preferred (e.g. definitions in main file).
-  if (auto Loc = toLSPLocation(
-  getPreferredLocation(R.PreferredDeclaration,
-   Sym.CanonicalDeclaration, Scratch),
-  *MainFilePath))
-R.PreferredDeclaration = *Loc;
-}
+// Use merge logic to choose AST or index declaration.
+if (auto Loc = toLSPLocation(
+getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration, Scratch),
+*MainFilePath))
+  R.PreferredDeclaration = *Loc;
   }
 });
   }
Index: clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp
@@ -186,7 +186,8 @@
 
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
-class $[[Proto]] {};
+class $p[[Proto]] {};
+void $f[[func]]() {};
   )cpp");
   TestTU TU;
   TU.HeaderCode = SymbolHeader.code();
@@ -195,13 +196,25 @@
 
   Annotations Test(R"cpp(// only declaration in AST.
 // Shift to make range different.
-class [[Proto]];
-P^roto* create();
+class Proto;
+void func() {}
+P$p^roto* create() {
+  fu$f^nc();
+  return nullptr;
+}
   )cpp");
 
   auto AST = TestTU::withCode(Test.code()).build();
-  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
-  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("p"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("p");
+EXPECT_THAT(Locs, ElementsAre(Sym("Proto", CodeGenLoc, CodeGenLoc)));
+  }
+  {
+auto Locs = clangd::locateSymbolAt(AST, Test.point("f"), Index.get());
+auto CodeGenLoc = SymbolHeader.range("f");
+EXPECT_THAT(Locs, ElementsAre(Sym("func", CodeGenLoc, CodeGenLoc)));
+  }
 }
 
 TEST(LocateSymbol, All) {


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -346,24 +346,27 @@
 Index->lookup(QueryRequest, [&](const Symbol &Sym) {
   auto &R = Result[ResultIndex.lookup(Sym.ID)];
 
-  // Special case: if the AST yielded a definition, then it may not be
-  // the right *declaration*. Prefer the one from the index.
   if (R.Definition) { // from AST
+// Special case: if the AST yielded a definition, then it may not be
+// the right *declaration*. Prefer the one from the index.
 if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
   R.PreferredDeclaration = *Loc;
+
+// We might still prefer the definition from the index, e

[PATCH] D61495: [clangd] Use more relaxed fuzzy-matching rules for local variables and class members.

[Sam McCall via Phabricator via cfe-commits]

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

I think the actual rules implemented here are probably *too* relaxed, and we
should require the first character to match the start of a word segment.
But I'm not sure and this was easier to write, will try it out.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61495

Files:
  clangd/CodeComplete.cpp
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h
  clangd/unittests/CodeCompleteTests.cpp
  clangd/unittests/FuzzyMatchTests.cpp

Index: clangd/unittests/FuzzyMatchTests.cpp
===
--- clangd/unittests/FuzzyMatchTests.cpp
+++ clangd/unittests/FuzzyMatchTests.cpp
@@ -45,9 +45,11 @@
 
 struct MatchesMatcher : public testing::MatcherInterface {
   ExpectedMatch Candidate;
+  FuzzyMatcher::MatchStyle Style;
   llvm::Optional Score;
-  MatchesMatcher(ExpectedMatch Candidate, llvm::Optional Score)
-  : Candidate(std::move(Candidate)), Score(Score) {}
+  MatchesMatcher(ExpectedMatch Candidate, FuzzyMatcher::MatchStyle Style,
+ llvm::Optional Score)
+  : Candidate(std::move(Candidate)), Style(Style), Score(Score) {}
 
   void DescribeTo(::std::ostream *OS) const override {
 llvm::raw_os_ostream(*OS) << "Matches " << Candidate;
@@ -61,7 +63,7 @@
 L->stream()
 ? (llvm::raw_ostream *)(new llvm::raw_os_ostream(*L->stream()))
 : new llvm::raw_null_ostream());
-FuzzyMatcher Matcher(Pattern);
+FuzzyMatcher Matcher(Pattern, Style);
 auto Result = Matcher.match(Candidate.Word);
 auto AnnotatedMatch = Matcher.dumpLast(*OS << "\n");
 return Result && Candidate.accepts(AnnotatedMatch) &&
@@ -71,9 +73,12 @@
 
 // Accepts patterns that match a given word, optionally requiring a score.
 // Dumps the debug tables on match failure.
-testing::Matcher matches(llvm::StringRef M,
-  llvm::Optional Score = {}) {
-  return testing::MakeMatcher(new MatchesMatcher(M, Score));
+testing::Matcher
+matches(llvm::StringRef M,
+FuzzyMatcher::MatchStyle Style = FuzzyMatcher::Strict,
+llvm::Optional Score = {}) {
+  return testing::MakeMatcher(
+  new MatchesMatcher(M, Style, Score));
 }
 
 TEST(FuzzyMatch, Matches) {
@@ -179,6 +184,15 @@
   EXPECT_THAT("std", Not(matches("pthread_condattr_setpshared")));
 }
 
+TEST(FuzzyMatch, MatchesLax) {
+  EXPECT_THAT("", matches("unique_ptr", FuzzyMatcher::Lax));
+  EXPECT_THAT("u_p", matches("[u]nique[_p]tr", FuzzyMatcher::Lax));
+  EXPECT_THAT("up", matches("[u]nique_[p]tr", FuzzyMatcher::Lax));
+  EXPECT_THAT("uq", matches("[u]ni[q]ue_ptr", FuzzyMatcher::Lax));
+  EXPECT_THAT("qp", matches("uni[q]ue_[p]tr", FuzzyMatcher::Lax));
+  EXPECT_THAT("log", matches("SVGFEMorpho[log]yElement", FuzzyMatcher::Lax));
+}
+
 struct RankMatcher : public testing::MatcherInterface {
   std::vector RankedStrings;
   RankMatcher(std::initializer_list RankedStrings)
@@ -272,16 +286,17 @@
 // Verify some bounds so we know scores fall in the right range.
 // Testing exact scores is fragile, so we prefer Ranking tests.
 TEST(FuzzyMatch, Scoring) {
-  EXPECT_THAT("abs", matches("[a]w[B]xYz[S]", 7.f / 12.f));
-  EXPECT_THAT("abs", matches("[abs]l", 1.f));
-  EXPECT_THAT("abs", matches("[abs]", 2.f));
-  EXPECT_THAT("Abs", matches("[abs]", 2.f));
+  EXPECT_THAT("abs",
+  matches("[a]w[B]xYz[S]", FuzzyMatcher::Strict, 7.f / 12.f));
+  EXPECT_THAT("abs", matches("[abs]l", FuzzyMatcher::Strict, 1.f));
+  EXPECT_THAT("abs", matches("[abs]", FuzzyMatcher::Strict, 2.f));
+  EXPECT_THAT("Abs", matches("[abs]", FuzzyMatcher::Strict, 2.f));
 }
 
 TEST(FuzzyMatch, InitialismAndPrefix) {
   // We want these scores to be roughly the same.
-  EXPECT_THAT("up", matches("[u]nique_[p]tr", 3.f / 4.f));
-  EXPECT_THAT("up", matches("[up]per_bound", 1.f));
+  EXPECT_THAT("up", matches("[u]nique_[p]tr", FuzzyMatcher::Strict, 3.f / 4.f));
+  EXPECT_THAT("up", matches("[up]per_bound", FuzzyMatcher::Strict, 1.f));
 }
 
 // Returns pretty-printed segmentation of Text.
Index: clangd/unittests/CodeCompleteTests.cpp
===
--- clangd/unittests/CodeCompleteTests.cpp
+++ clangd/unittests/CodeCompleteTests.cpp
@@ -202,6 +202,37 @@
   AllOf(Has("Car"), Not(Has("MotorCar";
 }
 
+TEST(CompletionTest, FilterStrictLax) {
+  // qry matches "query" in lax mode, but not in strict mode.
+  std::string Body = R"cpp(
+#define QUERY_MACRO
+int queryVariable;
+void queryFunction();
+struct QueryClass {
+  void queryBaseMethod();
+};
+class QueryEnclosingClass : QueryClass {
+  int QueryField = 42;
+  void queryMethod();
+  void queryEnclosingMethod() {
+int queryLocalVariable;
+qry^
+  }
+};
+  )cpp";
+  auto Results = completions(B

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61480



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

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D61458#1488970 , @jlebar wrote:

> Here's one for you:
>
>   __host__ float bar();
>   __device__ int bar();
>   __host__ __device__ auto foo() -> decltype(bar()) {}
>
>
> What is the return type of `foo`?  :)
>
> I don't believe the right answer is, "float when compiling for host, int when 
> compiling for device."
>
> I'd be happy if we said this was an error, so long as it's well-defined what 
> exactly we're disallowing.  But I bet @rsmith can come up with substantially 
> more evil testcases than this.


This patch is introduced to allow function or template function from `std` 
library to be used with device function. By allowing different-side candidates 
with a context only caring type inspection, we have new issue as there are 
extra beyond the regular rule for C++ overloadable resolution. We need an extra 
policy to figure out which is one the best candidate by considering CUDA 
attributes. Says the case you proposed, we may consider the following order to 
choose an overloadable candidate, e.g.

  SAME-SIDE (with the same CUDA attribute)
  NATIVE (without any CUDA attribute)
  WRONG-SIDE (with the opposite CUDA attribute)

or just

  SAME-SIDE
  NATIVE

It that a reasonable change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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

[PATCH] D61260: [clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-sizeof-expression.cpp:196
   typedef const MyStruct TMyStruct;
+  typedef const MyStruct *PMyStruct;
 

whisperity wrote:
> While I trust Clang and the matchers to unroll the type and still match, I'd 
> prefer also adding a test case for
> 
> ```
> typedef TMyStruct* PMyStruct2;
> ```
> 
> or somesuch.
> 
> And perhaps a "copy" of these cases where they come from template arguments, 
> in case the checker can also warn for that?
I added the new test, but templates are outside the scope of this particular 
patch. They should be tested (and fixed if the tests fail) in another patch.


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

https://reviews.llvm.org/D61260



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

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D61458#1488981 , @rjmccall wrote:

> In D61458#1488972 , @hfinkel wrote:
>
> > In D61458#1488970 , @jlebar wrote:
> >
> > > Here's one for you:
> > >
> > >   __host__ float bar();
> > >   __device__ int bar();
> > >   __host__ __device__ auto foo() -> decltype(bar()) {}
> > >
> > >
> > > What is the return type of `foo`?  :)
> > >
> > > I don't believe the right answer is, "float when compiling for host, int 
> > > when compiling for device."
> >
> >
> > So, actually, I wonder if that's not the right answer. We generally allow 
> > different overloads to have different return types.
>
>
> Only if they also differ in some other way.  C++ does not (generally) have 
> return-type-based overloading.  The two functions described would even mangle 
> the same way if CUDA didn't include host/device in the mangling.
>
> (Function templates can differ only by return type, but if both return types 
> successfully instantiate for a given set of (possibly inferred) template 
> arguments then the templates can only be distinguished when taking their 
> address, not when calling.)
>
> I think I've said before that adding this kind of overloading is not a good 
> idea, but since it's apparently already there, you should consult the 
> specification (or at least existing practice) to figure out what you're 
> supposed to do.


BTW, just check similar stuff with nvcc, with more than one candidates, it 
accepts the following code

  float bar(); // This line could be replaced by appendig `__host` or 
`__device__`, all of them are accepted.
  __host__ __device__ auto foo() -> decltype(bar()) {}

however, if there are more than one candidates differenct on the return type 
(without or with CUDA attibute difference), it could raise the error

  foo.cu(4): error: cannot overload functions distinguished by return type alone

it seems to me that that's also an acceptable policy to handle the issue after 
we allow different-side candidates in type-only context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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

[PATCH] D61260: [clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 197966.
baloghadamsoftware added a comment.

New tests added.


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

https://reviews.llvm.org/D61260

Files:
  clang-tidy/bugprone/SizeofExpressionCheck.cpp
  test/clang-tidy/bugprone-sizeof-expression.cpp


Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -193,11 +193,15 @@
 Array10* ptr;
   };
   typedef const MyStruct TMyStruct;
+  typedef const MyStruct *PMyStruct;
+  typedef TMyStruct *PMyStruct2;
 
   static TMyStruct kGlocalMyStruct = {};
   static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
 
   MyStruct S;
+  PMyStruct PS;
+  PMyStruct2 PS2;
   Array10 A10;
 
   int sum = 0;
@@ -225,6 +229,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(&S);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(MyStruct*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PMyStruct);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PS);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PS2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(&A10);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
 
Index: clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -142,10 +142,17 @@
   unaryOperator(hasOperatorName("&"),
 hasUnaryOperand(ignoringParenImpCasts(expr(
 hasType(qualType(hasCanonicalType(recordType(;
+  const auto PointerToStructType = type(hasUnqualifiedDesugaredType(
+  pointerType(pointee(recordType();
+  const auto PointerToStructExpr = expr(ignoringParenImpCasts(expr(
+  hasType(qualType(hasCanonicalType(PointerToStructType))),
+  unless(cxxThisExpr();
 
   Finder->addMatcher(
-  expr(sizeOfExpr(has(expr(ignoringParenImpCasts(
-   anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr))
+  expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(
+   anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr,
+ PointerToStructExpr),
+sizeOfExpr(has(PointerToStructType
   .bind("sizeof-pointer-to-aggregate"),
   this);
 


Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -193,11 +193,15 @@
 Array10* ptr;
   };
   typedef const MyStruct TMyStruct;
+  typedef const MyStruct *PMyStruct;
+  typedef TMyStruct *PMyStruct2;
 
   static TMyStruct kGlocalMyStruct = {};
   static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
 
   MyStruct S;
+  PMyStruct PS;
+  PMyStruct2 PS2;
   Array10 A10;
 
   int sum = 0;
@@ -225,6 +229,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
   sum += sizeof(&S);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(MyStruct*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PMyStruct);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PS);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PS2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
   sum += sizeof(&A10);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
 
Index: clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -142,10 +142,17 @@
   unaryOperator(hasOperatorName("&"),
 hasUnaryOperand(ignoringParenImpCasts(expr(
 hasType(qualType(hasCanonicalType(recordType(;
+  const auto PointerToStructType = type(hasUnqualifiedDesugaredType(
+  pointerType(pointee(record

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 197967.
sammccall marked 3 inline comments as done.
sammccall added a comment.

Extract setIncludeLocation() and call it from a more obvious point in the code.
Tweak test to make it harder.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61442

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/index/Symbol.cpp
  clangd/index/Symbol.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/unittests/CodeCompleteTests.cpp
  clangd/unittests/SymbolCollectorTests.cpp

Index: clangd/unittests/SymbolCollectorTests.cpp
===
--- clangd/unittests/SymbolCollectorTests.cpp
+++ clangd/unittests/SymbolCollectorTests.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "gmock/gmock-matchers.h"
 #include "gmock/gmock-more-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -34,9 +35,9 @@
 using testing::_;
 using testing::AllOf;
 using testing::Contains;
+using testing::Each;
 using testing::ElementsAre;
 using testing::Field;
-using testing::IsEmpty;
 using testing::Not;
 using testing::Pair;
 using testing::UnorderedElementsAre;
@@ -1028,6 +1029,26 @@
   Not(IncludeHeader();
 }
 
+// Features that depend on header-guards are fragile. Header guards are only
+// recognized when the file ends, so we have to defer checking for them.
+TEST_F(SymbolCollectorTest, HeaderGuardDetected) {
+  CollectorOpts.CollectIncludePath = true;
+  CollectorOpts.CollectMacro = true;
+  runSymbolCollector(R"cpp(
+#ifndef HEADER_GUARD_
+#define HEADER_GUARD_
+
+// Symbols are seen before the header guard is complete.
+#define MACRO
+int decl();
+
+#endif // Header guard is recognized here.
+  )cpp",
+ "");
+  EXPECT_THAT(Symbols, Not(Contains(QName("HEADER_GUARD_";
+  EXPECT_THAT(Symbols, Each(IncludeHeader()));
+}
+
 TEST_F(SymbolCollectorTest, NonModularHeader) {
   auto TU = TestTU::withHeaderCode("int x();");
   EXPECT_THAT(TU.headerSymbols(), ElementsAre(IncludeHeader()));
Index: clangd/unittests/CodeCompleteTests.cpp
===
--- clangd/unittests/CodeCompleteTests.cpp
+++ clangd/unittests/CodeCompleteTests.cpp
@@ -2073,19 +2073,28 @@
   UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
-TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+TEST(CompletionTest, MacroFromPreamble) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  std::string FooHeader = testPath("foo.h");
+  FS.Files[FooHeader] = "#define CLANGD_PREAMBLE_HEADER x\n";
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   auto Results = completions(
-  R"cpp(#define CLANGD_PREAMBLE x
+  R"cpp(#include "foo.h"
+  #define CLANGD_PREAMBLE_MAIN x
 
   int x = 0;
   #define CLANGD_MAIN x
   void f() { CLANGD_^ }
   )cpp",
   {func("CLANGD_INDEX")});
-  // Index is overriden in code completion options, so the preamble symbol is
-  // not seen.
-  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
-Named("CLANGD_INDEX")));
+  // We should get results from the main file, including the preamble section.
+  // However no results from included files (the index should cover them).
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(Named("CLANGD_PREAMBLE_MAIN"),
+   Named("CLANGD_MAIN"),
+   Named("CLANGD_INDEX")));
 }
 
 TEST(CompletionTest, DeprecatedResults) {
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -125,6 +125,12 @@
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
+  // File IDs for Symbol.IncludeHeaders.
+  // The final spelling is calculated in finish().
+  llvm::DenseMap IncludeFiles;
+  void setIncludeLocation(const Symbol &S, SourceLocation);
+  // Indexed macros, to be erased if they turned out to be include guards.
+  llvm::DenseSet IndexedMacros;
   // All refs collected from the AST.
   // Only symbols declared in preamble (from #include) and referenced from the
   // main file will be included.
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
+#

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/index/SymbolCollector.h:130
+  // The final spelling is calculated in finish().
+  llvm::DenseMap IncludeFiles;
+  // Indexed macros, to be erased if they turned out to be include guards.

kadircet wrote:
> This is losing ability to store multiple header files. Is that intentional? 
Careful reading of the code shows that ability never existed :-) 
`addDeclaration` always creates a new Symbol, sometimes populates its 
`IncludeHeaders`, and then replaces the existing symbol. We always find a 
single decl of the symbol we prefer in the TU (though sometimes it takes us a 
few attempts).

Of course, I never read the code that carefully - I wrote this as a 
`vector>` to "preserve" the ability to add multiple 
headers... and then the tests failed :-)



Comment at: clangd/unittests/SymbolCollectorTests.cpp:1043
+int decl();
+#define MACRO
+

kadircet wrote:
> can you also add a case where we first see `MACRO` and then `decl`.
IIUC the idea is that macros are "eager-er" than decls so more likely to break 
the caching. That makes sense.

WDYT about just putting the macro first, so we *only* test the "hard" case.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61442



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Thanks for the fast comments!

In D61487#1489308 , @alexfh wrote:

> I suspect that we're missing proper test coverage here.


True, ideally all the test scripts would also trigger the plugin case code path.

I've started with a modified copy of nolint.cpp as I wasn't able to have the 
two invocations (clang-tidy, c-index-test plugin) work with the same file. For 
example, the order of the diagnostics is different (seems solvable by appending 
using -DAG) and the c-index-test plugin case does not output "Suppressed 13 
warnings (13 NOLINT)" - is there a way to exclude the check for this output for 
the c-index-test case and still having all in the same file?

I haven't tested the plugin case with nolintnextline.cpp yet, but at least this 
one does not seem to contain the unmute case as far as I see.

> Another issue is that compiler diagnostics don't pass ClangTidyContext::diag 
> in the non-plugin use case.

Right, but how is this an issue?

> Do all the existing tests pass with your patch?

Yes.

> A better way to implement diagnostic filtering in the plugin would be to make 
> ClangTidyDiagnosticConsumer able to forward diagnostics to the external 
> diagnostics engine. It will still need some sort of a buffering though to 
> handle diagnostics and notes attached to them together.

Ahh, you suggest that the plugin should have a separate diagnostic engine and 
instantiate also a ClangTidyDiagnosticConsumer object, that forwards to the 
external one? Will check how this could work.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D61458#1488970 , @jlebar wrote:

> Here's one for you:
>
>   __host__ float bar();
>   __device__ int bar();
>   __host__ __device__ auto foo() -> decltype(bar()) {}
>
>
> What is the return type of `foo`?  :)
>
> I don't believe the right answer is, "float when compiling for host, int when 
> compiling for device."
>
> I'd be happy if we said this was an error, so long as it's well-defined what 
> exactly we're disallowing.  But I bet @rsmith can come up with substantially 
> more evil testcases than this.


At from CUDA 10, that's not acceptable as we are declaring two functions only 
differ from the return type. It seems CUDA attributes do not contribute to the 
function signature. clang is quite different here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/index/SymbolCollector.h:130
+  // The final spelling is calculated in finish().
+  llvm::DenseMap IncludeFiles;
+  // Indexed macros, to be erased if they turned out to be include guards.

sammccall wrote:
> kadircet wrote:
> > This is losing ability to store multiple header files. Is that intentional? 
> Careful reading of the code shows that ability never existed :-) 
> `addDeclaration` always creates a new Symbol, sometimes populates its 
> `IncludeHeaders`, and then replaces the existing symbol. We always find a 
> single decl of the symbol we prefer in the TU (though sometimes it takes us a 
> few attempts).
> 
> Of course, I never read the code that carefully - I wrote this as a 
> `vector>` to "preserve" the ability to add multiple 
> headers... and then the tests failed :-)
Ah, ok I see. Thanks for pointing this out!



Comment at: clangd/unittests/SymbolCollectorTests.cpp:1043
+int decl();
+#define MACRO
+

sammccall wrote:
> kadircet wrote:
> > can you also add a case where we first see `MACRO` and then `decl`.
> IIUC the idea is that macros are "eager-er" than decls so more likely to 
> break the caching. That makes sense.
> 
> WDYT about just putting the macro first, so we *only* test the "hard" case.
exactly, SGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61442



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

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
gribozavr marked an inline comment as done.
Closed by commit rCTE359876: Added an AST matcher for declarations that are in 
the `std` namespace (authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61480?vs=197950&id=197970#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61480

Files:
  clang-tidy/bugprone/InaccurateEraseCheck.cpp


Index: clang-tidy/bugprone/InaccurateEraseCheck.cpp
===
--- clang-tidy/bugprone/InaccurateEraseCheck.cpp
+++ clang-tidy/bugprone/InaccurateEraseCheck.cpp
@@ -17,10 +17,6 @@
 namespace tidy {
 namespace bugprone {
 
-namespace {
-AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
-}
-
 void InaccurateEraseCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.


Index: clang-tidy/bugprone/InaccurateEraseCheck.cpp
===
--- clang-tidy/bugprone/InaccurateEraseCheck.cpp
+++ clang-tidy/bugprone/InaccurateEraseCheck.cpp
@@ -17,10 +17,6 @@
 namespace tidy {
 namespace bugprone {
 
-namespace {
-AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
-}
-
 void InaccurateEraseCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

[Michael Liao via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Sema/Sema.h:10407-10409
   bool IsAllowedCUDACall(const FunctionDecl *Caller,
  const FunctionDecl *Callee) {
+if (llvm::any_of(ExprEvalContexts,

tra wrote:
> One more thing. The idea of this function is that we're checking if the 
> `Caller` is allowed to call the `Callee`.
> However here, you're checking the current context, which may not necessarily 
> be the same as the caller's. I.e. someone could potentially call it way after 
> the context is gone.
> 
> Currently all uses of this function obtain the caller from `CurContext`, but 
> if we start relying on other properties of the current context other than the 
> caller function, then we may neet to pass the context explicitly, or only 
> pass the Callee and check if it's callable from the current context.
> 
> ```
> 
as the expression within `decltype` may be quite complicated, the idea here is 
to relax that rule within `decltype` context, not only for a particular pair of 
caller/callee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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

[PATCH] D61396: [hip] Fix ambiguity from `>>>` of CUDA.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61396



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

r359876 - Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri May  3 05:50:00 2019
New Revision: 359876

URL: http://llvm.org/viewvc/llvm-project?rev=359876&view=rev
Log:
Added an AST matcher for declarations that are in the `std` namespace

Reviewers: alexfh

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=359876&r1=359875&r2=359876&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Fri May  3 05:50:00 2019
@@ -2827,6 +2827,29 @@ by the compiler (eg. implicit default/co
 
 
 
+MatcherDecl>isInStdNamespace
+Matches 
declarations in the namespace `std`, but not in nested namespaces.
+
+Given
+  class vector {};
+  namespace foo {
+class vector {};
+namespace std {
+  class vector {};
+}
+  }
+  namespace std {
+inline namespace __1 {
+  class vector {}; // #1
+  namespace experimental {
+class vector {};
+  }
+}
+  }
+cxxRecordDecl(hasName("vector"), isInStdNamespace()) will match only #1.
+
+
+
 MatcherDecl>isPrivate
 Matches private C++ 
declarations.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=359876&r1=359875&r2=359876&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Fri May  3 05:50:00 2019
@@ -6212,6 +6212,29 @@ AST_MATCHER(NamespaceDecl, isAnonymous)
   return Node.isAnonymousNamespace();
 }
 
+/// Matches declarations in the namespace `std`, but not in nested namespaces.
+///
+/// Given
+/// \code
+///   class vector {};
+///   namespace foo {
+/// class vector {};
+/// namespace std {
+///   class vector {};
+/// }
+///   }
+///   namespace std {
+/// inline namespace __1 {
+///   class vector {}; // #1
+///   namespace experimental {
+/// class vector {};
+///   }
+/// }
+///   }
+/// \endcode
+/// cxxRecordDecl(hasName("vector"), isInStdNamespace()) will match only #1.
+AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
+
 /// If the given case statement does not use the GNU case range
 /// extension, matches the constant given in the statement.
 ///

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=359876&r1=359875&r2=359876&view=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri May  3 05:50:00 2019
@@ -354,7 +354,8 @@ bool Decl::isInAnonymousNamespace() cons
 }
 
 bool Decl::isInStdNamespace() const {
-  return getDeclContext()->isStdNamespace();
+  const DeclContext *DC = getDeclContext();
+  return DC && DC->isStdNamespace();
 }
 
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=359876&r1=359875&r2=359876&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Fri May  3 05:50:00 2019
@@ -366,6 +366,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isExternC);
   REGISTER_MATCHER(isFinal);
   REGISTER_MATCHER(isImplicit);
+  REGISTER_MATCHER(isInStdNamespace);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isInline);
   REGISTER_MATCHER(isInstanceMessage);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=359876&r1=359875&r2=359876&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Fri May  3 
05:50:00 2019
@@ -2031,6 +2031,57 @@ TEST(NS, Anonymous) {
   EXPECT_TRUE(matches("namespace {}", namespaceDecl(isAnonymous(;
 }
 
+TEST(DeclarationMatcher, InStdNamespace) {
+  EXPECT_TRUE(notMatches("class vector {};"
+   

[clang-tools-extra] r359876 - Added an AST matcher for declarations that are in the `std` namespace

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri May  3 05:50:00 2019
New Revision: 359876

URL: http://llvm.org/viewvc/llvm-project?rev=359876&view=rev
Log:
Added an AST matcher for declarations that are in the `std` namespace

Reviewers: alexfh

Subscribers: cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/InaccurateEraseCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/InaccurateEraseCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/InaccurateEraseCheck.cpp?rev=359876&r1=359875&r2=359876&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/InaccurateEraseCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/InaccurateEraseCheck.cpp Fri 
May  3 05:50:00 2019
@@ -17,10 +17,6 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
-namespace {
-AST_MATCHER(Decl, isInStdNamespace) { return Node.isInStdNamespace(); }
-}
-
 void InaccurateEraseCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.


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

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: rnk, jlebar.
aganea added a project: clang.
Herald added a subscriber: mstorsjo.
aganea edited the summary of this revision.

The files below have been synced by Tortoise as CRLF, and it looks like they 
are like that in the depot as well (I'm still using the SVN setup)
Strangely `svn diff` shows them as LF.
The issue is that `grep 'something$'` doesn't match the newline inside the 
MINGW32 bash (mintty).


Repository:
  rC Clang

https://reviews.llvm.org/D61496

Files:
  clang/trunk/test/Preprocessor/indent_macro.c
  clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
  clang/trunk/test/Preprocessor/macro_not_define.c
  clang/trunk/test/Preprocessor/macro_rparen_scan.c


Index: clang/trunk/test/Preprocessor/macro_rparen_scan.c
===
--- clang/trunk/test/Preprocessor/macro_rparen_scan.c
+++ clang/trunk/test/Preprocessor/macro_rparen_scan.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^3 ;$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^3 ;$'
 
 /* Right paren scanning, hard case.  Should expand to 3. */
 #define i(x) 3 
Index: clang/trunk/test/Preprocessor/macro_not_define.c
===
--- clang/trunk/test/Preprocessor/macro_not_define.c
+++ clang/trunk/test/Preprocessor/macro_not_define.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^ # define X 3$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^ # define X 3$'
 
 #define H # 
  #define D define 
Index: clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
===
--- clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
+++ clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -E %s | grep '^a: x$'
-// RUN: %clang_cc1 -E %s | grep '^b: x y, z,h$'
-// RUN: %clang_cc1 -E %s | grep '^c: foo(x)$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^a: x$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^b: x y, z,h$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^c: foo(x)$'
 
 #define A(b, c...) b c
 a: A(x)
Index: clang/trunk/test/Preprocessor/indent_macro.c
===
--- clang/trunk/test/Preprocessor/indent_macro.c
+++ clang/trunk/test/Preprocessor/indent_macro.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^   zzap$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^   zzap$'
 
 // zzap is on a new line, should be indented.
 #define BLAH  zzap


Index: clang/trunk/test/Preprocessor/macro_rparen_scan.c
===
--- clang/trunk/test/Preprocessor/macro_rparen_scan.c
+++ clang/trunk/test/Preprocessor/macro_rparen_scan.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^3 ;$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^3 ;$'
 
 /* Right paren scanning, hard case.  Should expand to 3. */
 #define i(x) 3 
Index: clang/trunk/test/Preprocessor/macro_not_define.c
===
--- clang/trunk/test/Preprocessor/macro_not_define.c
+++ clang/trunk/test/Preprocessor/macro_not_define.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^ # define X 3$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^ # define X 3$'
 
 #define H # 
  #define D define 
Index: clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
===
--- clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
+++ clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -E %s | grep '^a: x$'
-// RUN: %clang_cc1 -E %s | grep '^b: x y, z,h$'
-// RUN: %clang_cc1 -E %s | grep '^c: foo(x)$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^a: x$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^b: x y, z,h$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^c: foo(x)$'
 
 #define A(b, c...) b c
 a: A(x)
Index: clang/trunk/test/Preprocessor/indent_macro.c
===
--- clang/trunk/test/Preprocessor/indent_macro.c
+++ clang/trunk/test/Preprocessor/indent_macro.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -E %s | grep '^   zzap$'
+// RUN: %clang_cc1 -E %s | dos2unix | grep '^   zzap$'
 
 // zzap is on a new line, should be indented.
 #define BLAH  zzap
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

[Aaron Ballman via Phabricator via cfe-commits]

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

In D61422#1489159 , 
@baloghadamsoftware wrote:

> In D61422#1488081 , @aaron.ballman 
> wrote:
>
> > Out of curiosity, have you run this over any large code bases to see what 
> > the false positive and true positive rate is?
>
>
> Neither false, nor true positives found so far. I ran it on several 
> open-source projects.


Hmm, I am a little bit skeptical about the utility of this compared to the 
expense of making the check marginally slower. However, I don't have any 
specific objections and so I'm not opposed to the patch. LG!


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

https://reviews.llvm.org/D61422



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

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

The tests should be fixed to use `FileCheck`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61496



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Change ClangdServer layer to output a structured response for Hover,
which can be rendered by client according to their needs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61497

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1185,7 +1185,8 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(render(*H).contents.value, Test.ExpectedHover.str())
+  << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -16,6 +16,8 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
 #include 
 
@@ -46,8 +48,24 @@
 std::vector findDocumentHighlights(ParsedAST &AST,
   Position Pos);
 
+struct HoverInfo {
+  LocatedSymbol Sym;
+  /// Name of the context containing the symbol.
+  std::string ContainerName;
+  index::SymbolInfo SI;
+  /// Includes all the qualifiers.
+  std::string Type;
+  /// Empty for non-functions. First element is the return type.
+  std::vector Signature;
+  std::string Documentation;
+  /// Line containing the definition of the symbol.
+  std::string Definition;
+  std::string TemplateArgs;
+};
+Hover render(const HoverInfo &HI);
+
 /// Get the hover information when hovering at \p Pos.
-llvm::Optional getHover(ParsedAST &AST, Position Pos);
+llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
 /// Returns reference locations of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -7,6 +7,7 @@
 //===--===//
 #include "XRefs.h"
 #include "AST.h"
+#include "CodeCompletionStrings.h"
 #include "FindSymbols.h"
 #include "Logger.h"
 #include "SourceCode.h"
@@ -14,14 +15,22 @@
 #include "index/Merge.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/None.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -241,17 +250,17 @@
   return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()};
 }
 
-Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-  SourceLocation LocEnd = Lexer::getLocForEndOfToken(
-  TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) {
+  const SourceManager &SourceMgr = AST.getSourceManager();
+  SourceLocation LocEnd =
+  Lexer::getLocForEndOfToken(TokLoc, 0, SourceMgr, AST.getLangOpts());
   return {sourceLocToPosition(SourceMgr, TokLoc),
   sourceLocToPosition(SourceMgr, LocEnd)};
 }
 
-llvm::Optional makeLocation(ParsedAST &AST, SourceLocation TokLoc,
+llvm::Optional makeLocation(ASTContext &AST, SourceLocation TokLoc,
   llvm::StringRef TUPath) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+  const SourceManager &SourceMgr = AST.getSourceManager();
   const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
   if (!F)
 return None;
@@ -299,8 +308,8 @@
   // As a consequence, there's no n

[clang-tools-extra] r359880 - [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri May  3 06:17:29 2019
New Revision: 359880

URL: http://llvm.org/viewvc/llvm-project?rev=359880&view=rev
Log:
[clangd] Fix header-guard check for include insertion, and don't index header 
guards.

Summary:
Both of these attempt to check whether a header guard exists while parsing the
file. However the file is only marked as guarded once clang finishes processing
it. We defer the checks and work until SymbolCollector::finish().

This is ugly and ad-hoc, deferring *all* work might be cleaner.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/Symbol.cpp
clang-tools-extra/trunk/clangd/index/Symbol.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Symbol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Symbol.cpp?rev=359880&r1=359879&r2=359880&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Symbol.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Symbol.cpp Fri May  3 06:17:29 2019
@@ -48,27 +48,23 @@ static void own(Symbol &S, llvm::UniqueS
 }
 
 void SymbolSlab::Builder::insert(const Symbol &S) {
-  auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
-  if (R.second) {
-Symbols.push_back(S);
-own(Symbols.back(), UniqueStrings);
-  } else {
-auto &Copy = Symbols[R.first->second] = S;
-own(Copy, UniqueStrings);
-  }
+  own(Symbols[S.ID] = S, UniqueStrings);
 }
 
 SymbolSlab SymbolSlab::Builder::build() && {
-  Symbols = {Symbols.begin(), Symbols.end()}; // Force shrink-to-fit.
-  // Sort symbols so the slab can binary search over them.
-  llvm::sort(Symbols,
+  // Sort symbols into vector so the slab can binary search over them.
+  std::vector SortedSymbols;
+  SortedSymbols.reserve(Symbols.size());
+  for (auto &Entry : Symbols)
+SortedSymbols.push_back(std::move(Entry.second));
+  llvm::sort(SortedSymbols,
  [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
   // We may have unused strings from overwritten symbols. Build a new arena.
   llvm::BumpPtrAllocator NewArena;
   llvm::UniqueStringSaver Strings(NewArena);
-  for (auto &S : Symbols)
+  for (auto &S : SortedSymbols)
 own(S, Strings);
-  return SymbolSlab(std::move(NewArena), std::move(Symbols));
+  return SymbolSlab(std::move(NewArena), std::move(SortedSymbols));
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/index/Symbol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Symbol.h?rev=359880&r1=359879&r2=359880&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Symbol.h (original)
+++ clang-tools-extra/trunk/clangd/index/Symbol.h Fri May  3 06:17:29 2019
@@ -204,10 +204,13 @@ public:
 /// This is a deep copy: underlying strings will be owned by the slab.
 void insert(const Symbol &S);
 
-/// Returns the symbol with an ID, if it exists. Valid until next insert().
+/// Removes the symbol with an ID, if it exists.
+void erase(const SymbolID &ID) { Symbols.erase(ID); }
+
+/// Returns the symbol with an ID, if it exists. Valid until insert/remove.
 const Symbol *find(const SymbolID &ID) {
-  auto I = SymbolIndex.find(ID);
-  return I == SymbolIndex.end() ? nullptr : &Symbols[I->second];
+  auto I = Symbols.find(ID);
+  return I == Symbols.end() ? nullptr : &I->second;
 }
 
 /// Consumes the builder to finalize the slab.
@@ -217,9 +220,8 @@ public:
 llvm::BumpPtrAllocator Arena;
 /// Intern table for strings. Contents are on the arena.
 llvm::UniqueStringSaver UniqueStrings;
-std::vector Symbols;
 /// Values are indices into Symbols vector.
-llvm::DenseMap SymbolIndex;
+llvm::DenseMap Symbols;
   };
 
 private:

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=359880&r1=359879&r2=359880&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri May  3 
06:17:29 2019
@@ -352,9 +352,8 @@ bool SymbolCollector::handleMacroOccuren
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
 
-  // Header guards are not interesting in index. Builtin macros don't have
-  // useful locations and are not needed for code completions.
-  if (MI->isUsedForHeaderGuard() || MI->isBuiltinMacro())
+  //

[PATCH] D61136: [Analyzer] Refactor begin and end symbol creation

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 197980.
baloghadamsoftware retitled this revision from "[Analyzer] IteratorChecker - 
Ensure end()>=begin() and refactor begin and end symbol creation" to 
"[Analyzer] Refactor begin and end symbol creation".
baloghadamsoftware edited the summary of this revision.
baloghadamsoftware added a comment.

`SymbolConjured *` written, feature removed, just refactoring.


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

https://reviews.llvm.org/D61136

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -300,10 +300,13 @@
 SymbolRef getContainerBegin(ProgramStateRef State, const MemRegion *Cont);
 SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont);
 ProgramStateRef createContainerBegin(ProgramStateRef State,
- const MemRegion *Cont,
- const SymbolRef Sym);
+ const MemRegion *Cont, const Expr *E,
+ QualType T, const LocationContext *LCtx,
+ unsigned BlockCount);
 ProgramStateRef createContainerEnd(ProgramStateRef State, const MemRegion *Cont,
-   const SymbolRef Sym);
+   const Expr *E, QualType T,
+   const LocationContext *LCtx,
+   unsigned BlockCount);
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 const SVal &Val);
 ProgramStateRef setIteratorPosition(ProgramStateRef State, const SVal &Val,
@@ -1142,11 +1145,9 @@
   auto State = C.getState();
   auto BeginSym = getContainerBegin(State, ContReg);
   if (!BeginSym) {
-auto &SymMgr = C.getSymbolManager();
-BeginSym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
-C.getASTContext().LongTy, C.blockCount());
-State = assumeNoOverflow(State, BeginSym, 4);
-State = createContainerBegin(State, ContReg, BeginSym);
+State = createContainerBegin(State, ContReg, CE, C.getASTContext().LongTy,
+ C.getLocationContext(), C.blockCount());
+BeginSym = getContainerBegin(State, ContReg);
   }
   State = setIteratorPosition(State, RetVal,
   IteratorPosition::getPosition(ContReg, BeginSym));
@@ -1166,11 +1167,9 @@
   auto State = C.getState();
   auto EndSym = getContainerEnd(State, ContReg);
   if (!EndSym) {
-auto &SymMgr = C.getSymbolManager();
-EndSym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
-  C.getASTContext().LongTy, C.blockCount());
-State = assumeNoOverflow(State, EndSym, 4);
-State = createContainerEnd(State, ContReg, EndSym);
+State = createContainerEnd(State, ContReg, CE, C.getASTContext().LongTy,
+   C.getLocationContext(), C.blockCount());
+EndSym = getContainerEnd(State, ContReg);
   }
   State = setIteratorPosition(State, RetVal,
   IteratorPosition::getPosition(ContReg, EndSym));
@@ -1934,32 +1933,47 @@
 }
 
 ProgramStateRef createContainerBegin(ProgramStateRef State,
- const MemRegion *Cont,
- const SymbolRef Sym) {
+ const MemRegion *Cont, const Expr *E,
+ QualType T, const LocationContext *LCtx,
+ unsigned BlockCount) {
   // Only create if it does not exist
   const auto *CDataPtr = getContainerData(State, Cont);
+  if (CDataPtr && CDataPtr->getBegin())
+return State;
+
+  auto &SymMgr = State->getSymbolManager();
+  const SymbolConjured *Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount,
+   "begin");
+  State = assumeNoOverflow(State, Sym, 4);
+
   if (CDataPtr) {
-if (CDataPtr->getBegin()) {
-  return State;
-}
 const auto CData = CDataPtr->newBegin(Sym);
 return setContainerData(State, Cont, CData);
   }
+
   const auto CData = ContainerData::fromBegin(Sym);
   return setContainerData(State, Cont, CData);
 }
 
 ProgramStateRef createContainerEnd(ProgramStateRef State, const MemRegion *Cont,
-   const SymbolRef Sym) {
+   const Expr *E, QualType T,
+   const LocationContext *LCtx,
+   unsigned BlockCount) {
   // Only create if it does not exist
   const auto *CDataPtr = getContainerData(State, Cont);
+  if (CDataPtr && CDataPtr->getEnd())
+return State;
+
+

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added inline comments.



Comment at: clang/test/Driver/arm-mfpu.c:112
+// CHECK-VFP3XD-NOT: "-target-feature" "+fp64"
+// CHECK-VFP3XD-NOT: "-target-feature" "+32"
 // CHECK-VFP3XD: "-target-feature" "+vfp3"

"+d32" ?



Comment at: llvm/lib/Target/ARM/ARMSubtarget.h:587
 
   bool hasVFP2() const { return HasVFPv2; }
   bool hasVFP3() const { return HasVFPv3; }

Are the old functions still used anywhere? If they are not used (much), I think 
it would be better to just have one set of functions for the base FPU version, 
and check hasFP64 and hasD32 where needed, to avoid the rick of using the wrong 
version of these functions.



Comment at: llvm/test/MC/ARM/armv8.3a-js.s:16
 // REQ-V83: error: instruction requires: armv8.3a
-// REQ-FP: error: instruction requires: FPARMv8
+// REQ-FP: error: invalid instruction

Do you know why this diagnostic got worse?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60691



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This is still WIP, just looking for feedbacks on `HoverInfo` struct and overall 
layering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE359880: [clangd] Fix header-guard check for include 
insertion, and don't index header… (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61442?vs=197967&id=197981#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61442

Files:
  clangd/index/Symbol.cpp
  clangd/index/Symbol.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/unittests/SymbolCollectorTests.cpp

Index: clangd/unittests/SymbolCollectorTests.cpp
===
--- clangd/unittests/SymbolCollectorTests.cpp
+++ clangd/unittests/SymbolCollectorTests.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "gmock/gmock-matchers.h"
 #include "gmock/gmock-more-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -34,9 +35,9 @@
 using testing::_;
 using testing::AllOf;
 using testing::Contains;
+using testing::Each;
 using testing::ElementsAre;
 using testing::Field;
-using testing::IsEmpty;
 using testing::Not;
 using testing::Pair;
 using testing::UnorderedElementsAre;
@@ -1028,6 +1029,26 @@
   Not(IncludeHeader();
 }
 
+// Features that depend on header-guards are fragile. Header guards are only
+// recognized when the file ends, so we have to defer checking for them.
+TEST_F(SymbolCollectorTest, HeaderGuardDetected) {
+  CollectorOpts.CollectIncludePath = true;
+  CollectorOpts.CollectMacro = true;
+  runSymbolCollector(R"cpp(
+#ifndef HEADER_GUARD_
+#define HEADER_GUARD_
+
+// Symbols are seen before the header guard is complete.
+#define MACRO
+int decl();
+
+#endif // Header guard is recognized here.
+  )cpp",
+ "");
+  EXPECT_THAT(Symbols, Not(Contains(QName("HEADER_GUARD_";
+  EXPECT_THAT(Symbols, Each(IncludeHeader()));
+}
+
 TEST_F(SymbolCollectorTest, NonModularHeader) {
   auto TU = TestTU::withHeaderCode("int x();");
   EXPECT_THAT(TU.headerSymbols(), ElementsAre(IncludeHeader()));
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -125,6 +125,12 @@
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
+  // File IDs for Symbol.IncludeHeaders.
+  // The final spelling is calculated in finish().
+  llvm::DenseMap IncludeFiles;
+  void setIncludeLocation(const Symbol &S, SourceLocation);
+  // Indexed macros, to be erased if they turned out to be include guards.
+  llvm::DenseSet IndexedMacros;
   // All refs collected from the AST.
   // Only symbols declared in preamble (from #include) and referenced from the
   // main file will be included.
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -352,9 +352,8 @@
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
 
-  // Header guards are not interesting in index. Builtin macros don't have
-  // useful locations and are not needed for code completions.
-  if (MI->isUsedForHeaderGuard() || MI->isBuiltinMacro())
+  // Builtin macros don't have useful locations and aren't needed in completion.
+  if (MI->isBuiltinMacro())
 return true;
 
   // Skip main-file symbols if we are not collecting them.
@@ -408,22 +407,25 @@
   std::string Signature;
   std::string SnippetSuffix;
   getSignature(*CCS, &Signature, &SnippetSuffix);
-
-  std::string Include;
-  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
-if (auto Header = getIncludeHeader(
-Name->getName(), SM.getDecomposedExpansionLoc(DefLoc).first))
-  Include = std::move(*Header);
-  }
   S.Signature = Signature;
   S.CompletionSnippetSuffix = SnippetSuffix;
-  if (!Include.empty())
-S.IncludeHeaders.emplace_back(Include, 1);
 
+  IndexedMacros.insert(Name);
+  setIncludeLocation(S, DefLoc);
   Symbols.insert(S);
   return true;
 }
 
+void SymbolCollector::setIncludeLocation(const Symbol &S,
+ SourceLocation Loc) {
+  if (Opts.CollectIncludePath)
+if (shouldCollectIncludePath(S.SymInfo.Kind))
+  // Use the expansion location to get the #include header since this is
+  // where the symbol is exposed.
+  IncludeFiles[S.ID] =
+  PP->getSourceManager().getDecomposedExpansionLoc(Loc).first;
+}
+
 void SymbolCollector::finish() {
   // At the end of the TU, add 1 to the refcount of all referenced symbols.
   auto IncRef = [this](const SymbolID &ID) {
@@ -440,6 +442,14 @@
   }
   if (Opts.CollectMacro) {
 assert(PP);

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Checked-in files should not have CRLF endings, in general (there are a very few 
exceptions, and these aren't among them).
If the checked-in files have LF endings but your tool checks them out with CRLF 
then that is a problem with your config, or with the tool.
Adding dos2unix to the RUN lines is the wrong fix, either way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61496



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

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D61422#1489451 , @aaron.ballman 
wrote:

> Hmm, I am a little bit skeptical about the utility of this compared to the 
> expense of making the check marginally slower. However, I don't have any 
> specific objections and so I'm not opposed to the patch. LG!


I think that the open-source projects I used for testing are mature enough for 
such amateur errors. However many beginner programmers make such errors.


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

https://reviews.llvm.org/D61422



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197986.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.

- Rebase
- Parse client capabilities, send markdown on hover if client supports it
- Use inline block for the scope
- Add a language for code blocks
- Add a FIXME for hover tests
- Fix escaping, add tests
- Update XRefs tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1172,7 +1172,8 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  // FIXME: add renderForTests() and use it here.
+  EXPECT_EQ(H->renderAsPlainText(), Test.ExpectedHover.str()) << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,158 @@
+//===-- FormattedStringTests.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
+//
+//===--===//
+#include "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+
+  S = FormattedString();
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), 

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:41
+
+void FormattedString::appendCodeBlock(std::string Code) {
+  Chunk C;

sammccall wrote:
> you may want to take the language name, and default it to either cpp or 
> nothing.
> Including it in the triple-backtick block can trigger syntax highlighting, 
> and editors are probably somewhat likely to actually implement this.
Done. Defaulting to 'cpp'.
VSCode actually does syntax highlighting there and it looks nice.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:63
+case ChunkKind::InlineCodeBlock:
+  R += " `";
+  R += escapeBackticks(C.Contents);

sammccall wrote:
> why do we add surrounding spaces if we don't do the same for text chunks?
a leftover from my first prototype.
They don't seem to be necessary, removed them.



Comment at: clang-tools-extra/clangd/FormattedString.h:31
+  /// newlines.
+  void appendCodeBlock(std::string Code);
+  /// Append an inline block of C++ code. This translates to the ` block in

sammccall wrote:
> Having various methods that take strings may struggle if we want a lot of 
> composability (e.g. a bullet list whose bullet whose third word is a 
> hyperlink/bold).
> 
> (I'm not sure whether this is going to be a real problem, just thinking out 
> loud here)
> 
> One alternative extensibility would be to make "containers" methods taking a 
> lambda that renders the body.
> e.g.
> 
> ```
> FormattedString S;
> S << "std::";
> S.bold([&} S << "swap" };
> S.list(FormattedString::Numbered, [&] {
>   S.item([&] { S << "sometimes you get the wrong overload"; });
>   S.item([&] {
> S << "watch out for ";
> S.link("https://en.cppreference.com/w/cpp/language/adl";, [&] { S << 
> "ADL"; });
>   });
> });
> S.codeBlock([&] S << "Here is an example"; });
> ```
> 
> Not sure if this is really better, I just have it on my mind because I ended 
> up using it for the `JSON.h` streaming output. We can probably bolt it on 
> later if necessary.
Agree that the current approach won't generalize to more complicated cases 
without some design work.
The lambda-heavy code is hard to read to my personal taste, but may actually be 
a good option in practice.
I'd also try playing around with some lambda-free alternatives to see if they 
lead to better syntax without compromising extensibility or making the 
implementation overly complex.

I also think we should bolt on this until we hit more use-cases with more 
extensive needs for formatting markdown. 



Comment at: clang-tools-extra/clangd/FormattedString.h:39
+  std::string renderAsPlainText() const;
+
+private:

sammccall wrote:
> I think we want renderForDebugging() or so which would print the chunks 
> explicitly, a la CodeCompletionString.
> 
> This is useful for actual debugging, and for testing methods that return 
> FormattedString (as opposed to the FormattedString->markdown translation)
That would be a useful addition. I've added a FIXME for now and testing 
plaintext in XRefs.
I'd suggest adding debug representation in a follow-up change, but also happy 
to do it now if you feel that's necessary.

That would need updates to all hover tests and that's a big enough to go into a 
separate change, from my perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:186
   void findHover(PathRef File, Position Pos,
- Callback> CB);
+ Callback> CB);
 

kadircet wrote:
> sammccall wrote:
> > Not sure about switching from `Hover` to `FormattedString` as return type 
> > here: LSP hover struct contains a range field (what are the bounds of the 
> > hovered thing?) that we may want to populate that doesn't fit in 
> > FormattedString.
> > I'd suggest the function in XRefs.h should return `FormattedString` (and 
> > later maybe `pair`), and the `ClangdServer` version 
> > should continue to provide the rendered `Hover`.
> > 
> > (We'll eventually want ClangdServer to return some HoverInfo struct with 
> > structured information, as we want embedding clients to be able to render 
> > it other ways, and maybe use it to provide extension methods like YCM's 
> > `GetType`. But no need to touch that in this patch, we'll end up producing 
> > HoverInfo -> FormattedString -> LSP Hover, I guess)
> I agree with Sam on the layering. I was also working in a patch that was 
> changing `ClangdServer`s output from `Hover` to `HoverInfo`(a structured 
> version of `Hover`).
`ClangdServer` does not know whether to render the `FormattedString` into 
markdown or plaintext and I believe it should stay that way.
Happy to return `HoverInfo`, though. I'll introduce one with `FormattedString` 
and `Range`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197989.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Return 'HoverInfo' instead of FormattedString
- Reformat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1172,7 +1172,9 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  // FIXME: add renderForTests() and use it here.
+  EXPECT_EQ(H->Content.renderAsPlainText(), Test.ExpectedHover.str())
+  << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,158 @@
+//===-- FormattedStringTests.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
+//
+//===--===//
+#include "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+
+  S = FormattedString();
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "#define FOO\n* foo !+ bar * baz\n"
+  "```\n");
+
+  // But we have t

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197991.
ilya-biryukov added a comment.

- Remove a FIXME that was fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1172,7 +1172,9 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  // FIXME: add renderForTests() and use it here.
+  EXPECT_EQ(H->Content.renderAsPlainText(), Test.ExpectedHover.str())
+  << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,158 @@
+//===-- FormattedStringTests.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
+//
+//===--===//
+#include "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+
+  S = FormattedString();
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "#define FOO\n* foo !+ bar * baz\n"
+  "```\n");
+
+  // But we have to escape the backticks.
+  S = FormattedString();
+  S.appendInlineCode("fo

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D61496#1489523 , @probinson wrote:

> Checked-in files should not have CRLF endings, in general (there are a very 
> few exceptions, and these aren't among them).
>  If the checked-in files have LF endings but your tool checks them out with 
> CRLF then that is a problem with your config, or with the tool.
>  Adding dos2unix to the RUN lines is the wrong fix, either way.


Yeah I've realized that :) This is caused by svn on Windows which sets 
`svn:eol-style=native` by default, which causes local files to be saved in the 
"native" format, which is CRLF on Windows (even though in upstream they are LF) 
When files are commited, normally svn should convert them back to LF (or at 
least that's the contract 
 
for `svn:eol-style=native`).

  native
  This causes the file to contain the EOL markers that are native to the 
operating system on which Subversion was run. In other words, if a user on a 
Windows machine checks out a working copy that contains a file with an 
svn:eol-style property set to native, that file will contain CRLF EOL markers. 
A Unix user checking out a working copy that contains the same file will see LF 
EOL markers in his copy of the file.
  
  Note that Subversion will actually store the file in the repository using 
normalized LF EOL markers regardless of the operating system. This is basically 
transparent to the user, though.

I'm using the MINGW32 bash to run the tests/`check-all` because it comes with 
all the Unix tools (grep, sed, ...) that Windows doesn't have.
The bug here is that the GNU grep only matches LF, not CRLF.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61496



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

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 197990.
aganea added a comment.

Changed to use `FileCheck` instead of `grep`


Repository:
  rC Clang

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

https://reviews.llvm.org/D61496

Files:
  clang/trunk/test/Preprocessor/indent_macro.c
  clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
  clang/trunk/test/Preprocessor/macro_not_define.c
  clang/trunk/test/Preprocessor/macro_rparen_scan.c


Index: clang/trunk/test/Preprocessor/macro_rparen_scan.c
===
--- clang/trunk/test/Preprocessor/macro_rparen_scan.c
+++ clang/trunk/test/Preprocessor/macro_rparen_scan.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^3 ;$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK:3 ;
 
 /* Right paren scanning, hard case.  Should expand to 3. */
 #define i(x) 3 
Index: clang/trunk/test/Preprocessor/macro_not_define.c
===
--- clang/trunk/test/Preprocessor/macro_not_define.c
+++ clang/trunk/test/Preprocessor/macro_not_define.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^ # define X 3$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK: # define X 3
 
 #define H # 
  #define D define 
Index: clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
===
--- clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
+++ clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -E %s | grep '^a: x$'
-// RUN: %clang_cc1 -E %s | grep '^b: x y, z,h$'
-// RUN: %clang_cc1 -E %s | grep '^c: foo(x)$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 1
+// 1:a: x
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 2
+// 2:b: x y, z,h
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace 
--check-prefix 3
+// 3:c: foo(x)
 
 #define A(b, c...) b c
 a: A(x)
Index: clang/trunk/test/Preprocessor/indent_macro.c
===
--- clang/trunk/test/Preprocessor/indent_macro.c
+++ clang/trunk/test/Preprocessor/indent_macro.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^   zzap$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK:   zzap
 
 // zzap is on a new line, should be indented.
 #define BLAH  zzap


Index: clang/trunk/test/Preprocessor/macro_rparen_scan.c
===
--- clang/trunk/test/Preprocessor/macro_rparen_scan.c
+++ clang/trunk/test/Preprocessor/macro_rparen_scan.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^3 ;$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK:3 ;
 
 /* Right paren scanning, hard case.  Should expand to 3. */
 #define i(x) 3 
Index: clang/trunk/test/Preprocessor/macro_not_define.c
===
--- clang/trunk/test/Preprocessor/macro_not_define.c
+++ clang/trunk/test/Preprocessor/macro_not_define.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^ # define X 3$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK: # define X 3
 
 #define H # 
  #define D define 
Index: clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
===
--- clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
+++ clang/trunk/test/Preprocessor/macro_fn_varargs_named.c
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -E %s | grep '^a: x$'
-// RUN: %clang_cc1 -E %s | grep '^b: x y, z,h$'
-// RUN: %clang_cc1 -E %s | grep '^c: foo(x)$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace --check-prefix 1
+// 1:a: x
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace --check-prefix 2
+// 2:b: x y, z,h
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace --check-prefix 3
+// 3:c: foo(x)
 
 #define A(b, c...) b c
 a: A(x)
Index: clang/trunk/test/Preprocessor/indent_macro.c
===
--- clang/trunk/test/Preprocessor/indent_macro.c
+++ clang/trunk/test/Preprocessor/indent_macro.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -E %s | grep '^   zzap$'
+// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace
+// CHECK:   zzap
 
 // zzap is on a new line, should be indented.
 #define BLAH  zzap
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FormattedString.h:27
+  /// Append plain text to the end of the string.
+  void appendText(std::string Text);
+  /// Append a block of C++ code. This translates to a ``` block in markdown.

sammccall wrote:
> I guess this will get an optional parameter to control the style (bold, 
> italic etc)?
Either that or a separate function to add bold/italic, etc.
I think we should keep it simple in the first revision.

This really tries to provide a vocabulary type to carry a string with formatted 
blocks around, not support full markdown from the start.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:186
   void findHover(PathRef File, Position Pos,
- Callback> CB);
+ Callback> CB);
 

ilya-biryukov wrote:
> kadircet wrote:
> > sammccall wrote:
> > > Not sure about switching from `Hover` to `FormattedString` as return type 
> > > here: LSP hover struct contains a range field (what are the bounds of the 
> > > hovered thing?) that we may want to populate that doesn't fit in 
> > > FormattedString.
> > > I'd suggest the function in XRefs.h should return `FormattedString` (and 
> > > later maybe `pair`), and the `ClangdServer` 
> > > version should continue to provide the rendered `Hover`.
> > > 
> > > (We'll eventually want ClangdServer to return some HoverInfo struct with 
> > > structured information, as we want embedding clients to be able to render 
> > > it other ways, and maybe use it to provide extension methods like YCM's 
> > > `GetType`. But no need to touch that in this patch, we'll end up 
> > > producing HoverInfo -> FormattedString -> LSP Hover, I guess)
> > I agree with Sam on the layering. I was also working in a patch that was 
> > changing `ClangdServer`s output from `Hover` to `HoverInfo`(a structured 
> > version of `Hover`).
> `ClangdServer` does not know whether to render the `FormattedString` into 
> markdown or plaintext and I believe it should stay that way.
> Happy to return `HoverInfo`, though. I'll introduce one with 
> `FormattedString` and `Range`
Returning `HoverInfo` now, with a `FormattedString` and a range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This is ready for another round of review. As mentioned in the comments, I 
think some of the comments are better addressed by a separate change, because 
they would touch lots of lines in the test.
Let me know if I missed anything else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 197994.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Remove dead test code. NFC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1172,7 +1172,9 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  // FIXME: add renderForTests() and use it here.
+  EXPECT_EQ(H->Content.renderAsPlainText(), Test.ExpectedHover.str())
+  << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,156 @@
+//===-- FormattedStringTests.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
+//
+//===--===//
+#include "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "#define FOO\n* foo !+ bar * baz\n"
+  "```\n");
+
+  // But we have to escape the backticks.
+  S = FormattedString();
+  S.app

[PATCH] D61506: [OpenCL] Switch to C++17

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: kpet.
Herald added subscribers: ebevhan, yaxunl.

Caused test update as frontend now produces better IR due to copy elision.


https://reviews.llvm.org/D61506

Files:
  include/clang/Frontend/LangStandards.def
  test/CodeGenOpenCLCXX/addrspace-of-this.cl


Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -109,15 +109,9 @@
 // IMPL: call void @llvm.memcpy.p4i8.p4i8.i32(i8 addrspace(4)* 
{{.*}}[[C2GENVOID]], i8 addrspace(4)* {{.*}}[[C1GENVOID]]
 
 // Test the address space of 'this' when invoking the operator+
-// COMMON: [[C3GEN:%[0-9]+]] = addrspacecast %class.C* %c3 to %class.C 
addrspace(4)*
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C 
addrspace(4)*
-// COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %ref.tmp, %class.C 
addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]])
-// COMMON: [[REFGEN:%[0-9]+]] = addrspacecast %class.C* %ref.tmp to %class.C 
addrspace(4)*
-// EXPL: call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* [[C3GEN]], 
%class.C addrspace(4)* dereferenceable(4) [[REFGEN]])
-// IMPL: [[C3VOID:%[0-9]+]] = bitcast %class.C* %c3 to i8*
-// IMPL: [[REFGENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[REFGEN]] to 
i8 addrspace(4)*
-// IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C3VOID]], i8 
addrspace(4)* {{.*}}[[REFGENVOID]]
+// COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %c3, %class.C 
addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]])
 
 // Test the address space of 'this' when invoking the move constructor
 // COMMON: [[C4GEN:%[0-9]+]] = addrspacecast %class.C* %c4 to %class.C 
addrspace(4)*
Index: include/clang/Frontend/LangStandards.def
===
--- include/clang/Frontend/LangStandards.def
+++ include/clang/Frontend/LangStandards.def
@@ -159,7 +159,7 @@
  LineComment | C99 | Digraphs | HexFloat | OpenCL)
 LANGSTANDARD(openclcpp, "c++",
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 
| Digraphs | OpenCL)
 
 LANGSTANDARD_ALIAS_DEPR(opencl10, "CL")
 LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1")


Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -109,15 +109,9 @@
 // IMPL: call void @llvm.memcpy.p4i8.p4i8.i32(i8 addrspace(4)* {{.*}}[[C2GENVOID]], i8 addrspace(4)* {{.*}}[[C1GENVOID]]
 
 // Test the address space of 'this' when invoking the operator+
-// COMMON: [[C3GEN:%[0-9]+]] = addrspacecast %class.C* %c3 to %class.C addrspace(4)*
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
 // COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
-// COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %ref.tmp, %class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]])
-// COMMON: [[REFGEN:%[0-9]+]] = addrspacecast %class.C* %ref.tmp to %class.C addrspace(4)*
-// EXPL: call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* [[C3GEN]], %class.C addrspace(4)* dereferenceable(4) [[REFGEN]])
-// IMPL: [[C3VOID:%[0-9]+]] = bitcast %class.C* %c3 to i8*
-// IMPL: [[REFGENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[REFGEN]] to i8 addrspace(4)*
-// IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C3VOID]], i8 addrspace(4)* {{.*}}[[REFGENVOID]]
+// COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %c3, %class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]])
 
 // Test the address space of 'this' when invoking the move constructor
 // COMMON: [[C4GEN:%[0-9]+]] = addrspacecast %class.C* %c4 to %class.C addrspace(4)*
Index: include/clang/Frontend/LangStandards.def
===
--- include/clang/Frontend/LangStandards.def
+++ include/clang/Frontend/LangStandards.def
@@ -159,7 +159,7 @@
  LineComment | C99 | Digraphs | HexFloat | OpenCL)
 LANGSTANDARD(openclcpp, "c++",
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 | Digraphs | OpenCL)
 
 LANGSTANDARD_ALIAS_DEPR(opencl10, "CL")
 LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.ll

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> As for naming, agreed, but does that concern drop away once we have only a 
> single RewriteRule definition?

Sure, that won't be an issue.

The use-cases make sense, thanks for the examples. Could you add one or two to 
the code? Would probably make sense to express them in code as matchers, rather 
than explaining what we're trying to do in a natural language.

Let's proceed with making `RewriteRule` the vocabulary type that we use for 
transformations like this if we both agree that's a good idea.
There are obviously multiple ways to tackle this, which one you had in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335



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

[PATCH] D61130: [llvm-mc] Add reportWarning() to MCContext

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

ping!  any comments or concerns about this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61130



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

[PATCH] D61495: [clangd] Use more relaxed fuzzy-matching rules for local variables and class members.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

With two modes of operation, could inconsistencies between the client-side and 
the server-side filtering become more/less of an issue?
Is it true that the clients (VSCode, etc) do something closer to `Lax` mode?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61495



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

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D61399#1488897 , @hfinkel wrote:

> In D61399#1488366 , @ABataev wrote:
>
> > In D61399#1488329 , @hfinkel wrote:
> >
> > > In D61399#1488309 , @ABataev 
> > > wrote:
> > >
> > > > In D61399#1488299 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D61399#1488262 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > I don't like this implementation. Seems to me, it breaks one of the 
> > > > > > OpenMP standard requirements: the program can be compiled without 
> > > > > > openmp support. I assume, that with this includes the program won't 
> > > > > > be able to be compiled without OpenMP support anymore because it 
> > > > > > may use some device-specific math functions explicitly.
> > > > > >  Instead, I would like to see some additional, device-scpecific 
> > > > > > math header file, that must be included explicitly to support some 
> > > > > > device-specific math functions. And we need to provide default 
> > > > > > implementations for those extra math functions for all the 
> > > > > > platforms we're going to support, including default host 
> > > > > > implementations.
> > > > >
> > > > >
> > > > > Can you provide an example of a conforming program that can't be 
> > > > > compiled without OpenMP support? Regardless of the use of any 
> > > > > device-specific functions (which isn't covered by the standard, of 
> > > > > course, but might be needed in practice), the code still needs to be 
> > > > > compilable by the host in order to generate the host-fallback 
> > > > > version. This doesn't change that. Thus, any program that uses 
> > > > > anything from this math.h, etc. needs to compile for the host, and 
> > > > > thus, likely compiles without OpenMP support. Maybe I'm missing your 
> > > > > point, however.
> > > >
> > > >
> > > > Assume we have something like this:
> > > >
> > > >   #pragma omp target if(cond)
> > > >   a = __nv_();
> > > >
> > > >
> > > > Instead of `__nv_xxx` you can try to use any Cuda-specific function, 
> > > > which is not the part of the standard `math.h`/`cmath` files. Will it 
> > > > be compilable even with OpenMP?
> > >
> > >
> > > I don't think that this changes that one way or the other. Your example 
> > > won't work, AFAIK, unless you do something like:
> > >
> > >   #pragma omp target if(cond)
> > >   #ifdef __NVPTX__
> > >   a = __nv_();
> > >   #else
> > >   a = something_on_the_host;
> > >   #endif
> > >   
> > >
> > > and anything from these headers that doesn't also have a host version 
> > > will suffer the same fate: if it won't also compile for the host (one way 
> > > or another), then it won't work.
> >
> >
> > The problem with this header file is that it allows to use those 
> > Cuda-specific functions unconditionally in some cases:
> >
> >   #pragma omp target
> >   a = __nv_();
> >
> >
> > It won't require any target-specific guards to compile this code (if we 
> > compile it only for Cuda-specific devices) and we're loosing the 
> > consistency here: in some cases target regions will require special device 
> > guards, in others, with the same function calls, it is not. And the worst 
> > thing, is that we implicitly allow to introduce this kind of incostistency 
> > into users code. That's why I would prefer to see a special kind of the 
> > include file, NVPTX specific, that must be included explicitly, so the user 
> > explictly commanded to use some target-specific math functions, if he 
> > really wants it. Plus, maybe, in this files we need force check of the 
> > platform and warn users that the functions from this header file must be 
> > used using device-specific checks. Or provide some kind of the default 
> > implementations for all the platforms, that do not support those math 
> > function natively.
>
>
> I believe that I understand your point, but two things:
>
> 1. I think that you're mistaken on the underlying premise. That code will not 
> meaningfully compile without ifdefs, even if only CUDA-specific devices are 
> the only ones selected. We *always* compile code for the host as well, not 
> for offloading proper, but for the fallback (for execution when the 
> offloading fails). If I emulate this situation by writing this:
>
>
>
>   #ifdef __NVPTX__
>   int __nv_floor();
>   #endif
>  
>   int main() {
>   #pragma omp target
>   __nv_floor();
>   }
>   
>
> and try to compile using Clang with -fopenmp -fopenmp-targets=nvptx64, the 
> compilation fails:
>
>   int1.cpp:8:1: error: use of undeclared identifier '__nv_floor'
>   
>
> and this is because, when we invoke the compilation for the host, there is no 
> declaration for that function. This is true even though nvptx64 is the only 
> target for which the code is being compiled (be

[PATCH] D61495: [clangd] Use more relaxed fuzzy-matching rules for local variables and class members.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Another fair question is: how much should we care about client-side and 
server-side filtering consistencty in the first place?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61495



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

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: alexfh, bkramer.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

A general use, missing header guard finder and fixer.
No style checks.  If there is already a working header guard, it is used.
If the header guard is missing or broken, it is created from the path name
using this method.

/foo.h ->

#ifndef FOO_H
 #define FOO_H

#endif

The motivation for this change is fixing a large, non llvm with many missing 
header guards.
As no style is checked, it should not conflict with llvm-header-guard.

The change is also in this dev branch
https://github.com/trixirt/clang-tools-extra/tree/misc-header-check


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61508

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/HeaderGuardCheck.cpp
  clang-tidy/misc/HeaderGuardCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-header-guard.rst
  test/clang-tidy/misc-header-guard.cpp

Index: test/clang-tidy/misc-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s misc-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: misc-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* -- 
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: docs/clang-tidy/checks/misc-header-guard.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-header-guard.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - misc-header-guard
+
+misc-header-guard
+=
+
+Finds and fixes missing header guards and does not enforce any style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. e.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -181,6 +181,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-twine-local
misc-definitions-in-headers
+   misc-header-guard
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`misc-header-guard
+  ` check.
+
+  Finds and fixes missing header guards and does not enforce any style.
+
 - New :doc:`objc-super-self ` check.
 
   Finds invocations of ``-self`` on super instances in initializers of
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "HeaderGuardCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
@@ -32,6 +33,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
+CheckFactories.registerCheck(
+"misc-header-guard");
 CheckFactories.registerCheck("misc-misplaced-const");
 CheckFactories.registerCheck(
 "misc-new-delete-overloads");
Index: clang-tidy/misc/HeaderGuardCheck.h
===
--- /dev/null
+++ clang-tidy/misc/HeaderGuardCheck.h
@@ -0,0 +1,33 @@
+//===--- HeaderGuardCheck.h - clang-tidy *- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERGUARDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERGUARDCHECK_H
+
+#include "../utils/HeaderGuard.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {

[PATCH] D61488: [OpenCL] Make global ctor init function a kernel

[Kévin Petit via Phabricator via cfe-commits]

kpet added inline comments.



Comment at: lib/CodeGen/CGDeclCXX.cpp:583
 
+  // In OpenCL global init function should be converted to the kernel to be
+  // able to initiate its execution from the host side.

functions
should be -> must be?
to the kernel -> to a kernel



Comment at: lib/CodeGen/CGDeclCXX.cpp:584
+  // In OpenCL global init function should be converted to the kernel to be
+  // able to initiate its execution from the host side.
+  // FIXME: Some more work might be needed to handle destructors correctly.

in order that they may be launched by the host?



Comment at: lib/CodeGen/CGDeclCXX.cpp:590
+  // dynamic resource allocation on the device and program scope variables are
+  // destroyed by the runtime when program is released.
+  if (getLangOpts().OpenCL) {

Agree that global destructors aren't that attractive a feature in most 
contexts. There are quite a few runtime issues with them too. We can think 
about this later.



Comment at: lib/CodeGen/CGDeclCXX.cpp:593
+GenOpenCLArgMetadata(Fn);
+Fn->setCallingConv(llvm::CallingConv::SPIR_FUNC);
+  }

Shouldn't the calling convention be SPIR_KERNEL if you want the function to be 
host-visible?



Comment at: lib/CodeGen/CodeGenModule.h:1322
+  /// used. This helper can be used to generate metadata for source code kernel
+  /// function as well as generated implicitly kernels. If kernel is generated
+  /// implicitly null value have to be passed to the last two parameter,

kernels generated implicitly
if a kernel | when a kernel


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

https://reviews.llvm.org/D61488



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

[PATCH] D61506: [OpenCL] Switch to C++17

[Kévin Petit via Phabricator via cfe-commits]

kpet added inline comments.



Comment at: include/clang/Frontend/LangStandards.def:162
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 
| Digraphs | OpenCL)
 

Suggest you add `HexFloat` as well. It is part of c++17 and all OpenCL versions.


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

https://reviews.llvm.org/D61506



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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added reviewers: ABataev, rnk, Eugene.Zelenko, akyrtzi, rsmith.
Herald added subscribers: jdoerfert, dexonsmith, guansong.
Herald added a project: clang.

Currently, an OpenMP AST node's recorded location starts at the `omp`
token after the `#pragma` token, and the `#pragma` location isn't
available anywhere that I have found.  However, the `#pragma` location
can be useful when, for example, rewriting a directive using Clang's
Rewrite facility.

This patch makes `#pragma` locations available in any `PragmaHandler`.
However, this patch is incomplete as it does not actually use those
locations.  I'd like to extend the OpenMP implementation to use it, 
but I see two possible approaches, and I need feedback to choose one:

1. Extend `PragmaOpenMPHandler` to set the location of 
`tok::annot_pragma_openmp` to the `#pragma` location so that the `#pragma` 
location then becomes the start location of the OpenMP AST node.  The drawback 
here is that locations in many tests, especially `-ast-dump` tests, will need 
to be updated, and it's not clear to me if any external AST user depends on the 
existing locations.

2. Extend `PragmaOpenMPHandler` to insert a new token, perhaps named 
`tok::annot_pragma_openmp_intro`, before `tok::annot_pragma_openmp` and store 
both their locations in each OpenMP AST node.  The new location would be 
accessed by new member functions so that users of the old location wouldn't see 
a change.  The drawback here is that a lot of the OpenMP AST node construction 
code will need to be updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61509

Files:
  clang/include/clang/Lex/Pragma.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParsePragma.cpp

Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -27,32 +27,32 @@
 struct PragmaAlignHandler : public PragmaHandler {
   explicit PragmaAlignHandler() : PragmaHandler("align") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaGCCVisibilityHandler : public PragmaHandler {
   explicit PragmaGCCVisibilityHandler() : PragmaHandler("visibility") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaOptionsHandler : public PragmaHandler {
   explicit PragmaOptionsHandler() : PragmaHandler("options") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaPackHandler : public PragmaHandler {
   explicit PragmaPackHandler() : PragmaHandler("pack") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaClangSectionHandler : public PragmaHandler {
   explicit PragmaClangSectionHandler(Sema &S)
  : PragmaHandler("section"), Actions(S) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 private:
   Sema &Actions;
 };
@@ -60,38 +60,38 @@
 struct PragmaMSStructHandler : public PragmaHandler {
   explicit PragmaMSStructHandler() : PragmaHandler("ms_struct") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaUnusedHandler : public PragmaHandler {
   PragmaUnusedHandler() : PragmaHandler("unused") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaWeakHandler : public PragmaHandler {
   explicit PragmaWeakHandler() : PragmaHandler("weak") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaRedefineExtnameHandler : public PragmaHandler {
   explicit PragmaRedefineExtnameHandler() : PragmaHandler("redefine_extname") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &FirstToken) override;
+SourceLocation Introducer

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

rjmccall wrote:
> Sorry I didn't catch this before, but I don't see why this test is expected 
> to work.  We can't actually pass a pointer in address space 10 to that 
> constructor.
Ok, I have created a bug to follow up on this issues:
https://bugs.llvm.org/show_bug.cgi?id=41730

It seems that the check is missing here for allowing the address space 
conversions implicitly, but I am afraid if I add it now addr spaces will become 
less usable because implicit conversions can't be setup by the target yet. And 
everyone is using no address space as some sort of `__generic` but 
unofficially. :(

I have some thoughts about adding something like `__generic` address space to 
C++ that can either be resolved by the compiler or supported by HW. I think it 
might help to implement those cases correctly without modifying too much of 
code base. I just struggled to find enough bandwidth to send an RFC but I will 
try to progress on this asap.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

If the patch is going to be accepted, then definitely it must be solution #1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

@ABataev this patch works for both C and C++ and for both math.h and cmath 
headers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61509#1489752 , @ABataev wrote:

> If the patch is going to be accepted, then definitely it must be solution #1.


I certainly have no objection to that and will be happy to implement it, but 
can you help me to understand the rationale?  (Thanks for your quick response!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D61399#1489757 , @gtbercea wrote:

> @ABataev this patch works for both C and C++ and for both math.h and cmath 
> headers.


Did you test it for the builtins? like `pow`, `powf` and `powl`? How are the 
builtins resolved with this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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

[PATCH] D60379: Make precompiled headers reproducible

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Can this be closed now due to commit r358674?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60379



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

Seems so.


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

https://reviews.llvm.org/D60678



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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D61509#1489761 , @jdenny wrote:

> In D61509#1489752 , @ABataev wrote:
>
> > If the patch is going to be accepted, then definitely it must be solution 
> > #1.
>
>
> I certainly have no objection to that and will be happy to implement it, but 
> can you help me to understand the rationale?  (Thanks for your quick 
> response!)


Another one annotation token may significantly change the parsing process. It 
will require a lot of rework in the parsing of OpenMP pragmas plus may lead to 
some unpredictable results like endless parsing in some cases etc. It is much 
easier to change the tests rather than modify the whole parsing procedure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

> At [nvcc] from CUDA 10, that's not acceptable as we are declaring two 
> functions only differ from the return type. It seems CUDA attributes do not 
> contribute to the function signature. clang is quite different here.

Yes, this is an intentional and more relaxed semantics in clang.  It's also 
sort of the linchpin of our mixed-mode compilation strategy, which is very 
different from nvcc's source-to-source splitting strategy.

Back in the day you could trick nvcc into allowing host/device overloading on 
same-signature functions by slapping a `template` on one or both of them.  
Checking just now it seems they fixed this, but I suspect there are still dark 
corners where nvcc relies on effectively the same behavior as we get in clang 
via true overloading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458



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

[PATCH] D61399: [OpenMP][Clang] Support for target math functions

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D61399#1489762 , @ABataev wrote:

> In D61399#1489757 , @gtbercea wrote:
>
> > @ABataev this patch works for both C and C++ and for both math.h and cmath 
> > headers.
>
>
> Did you test it for the builtins? like `pow`, `powf` and `powl`? How are the 
> builtins resolved with this patch?


I have. There are tests for this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61399



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

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61335#1489680 , @ilya-biryukov 
wrote:

> > As for naming, agreed, but does that concern drop away once we have only a 
> > single RewriteRule definition?
>
> Sure, that won't be an issue.
>
> The use-cases make sense, thanks for the examples. Could you add one or two 
> to the code? Would probably make sense to express them in code as matchers, 
> rather than explaining what we're trying to do in a natural language.


Sounds good, will do.

> Let's proceed with making `RewriteRule` the vocabulary type that we use for 
> transformations like this if we both agree that's a good idea.
>  There are obviously multiple ways to tackle this, which one you had in mind?

Something like this, although if you have a better name than `RewriteAction` 
I'm open to alternatives. e.g. `RewriteCase`?

  struct RewriteAction {
SmallVector Edits;
TextGenerator Explanation;
  };
  
  struct RewriteRule {
ast_matchers::internal::DynTypedMatcher Matcher;
std::vector Actions;
static constexpr llvm::StringLiteral RootId = "___root___";
  };

Or, nest the definition:

  struct RewriteRule {
struct Action {
  SmallVector Edits;
  TextGenerator Explanation;
};
  
ast_matchers::internal::DynTypedMatcher Matcher;
std::vector Actions;
  
static constexpr llvm::StringLiteral RootId = "___root___";
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335



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

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/misc/HeaderGuardCheck.cpp:21
+ StringRef OldGuard) {
+  if (OldGuard.size()) {
+return OldGuard;

Please use early return.



Comment at: docs/clang-tidy/checks/misc-header-guard.rst:14
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no

Please use single back-tick instead of quotes for option values. Same below.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D61518: [clangd] add CLANG_ENABLE_CLANGD option to build clangd. Require threads.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: gribozavr.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov, mgorny.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61518

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added a comment.

Just completed testing... everything seems to be working correctly. So long as 
the all the tests pass, let's commit! :D


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

https://reviews.llvm.org/D60349



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