[PATCH] D60552: [X86] Enable intrinsics of AVX512_BF16, which are supported for BFLOAT16 in Cooper Lake

[Tianle Liu via Phabricator via cfe-commits]

liutianle added a comment.

@RKSimon @craig.topper , could you please review again?


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

https://reviews.llvm.org/D60552



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Sam McCall via Phabricator via cfe-commits]

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

Zap test/Unit directory, put lit.cfg in unittests directly.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61187

Files:
  CMakeLists.txt
  clangd/CMakeLists.txt
  clangd/test/CMakeLists.txt
  clangd/test/Inputs/BenchmarkHeader.h
  clangd/test/Inputs/BenchmarkSource.cpp
  clangd/test/Inputs/background-index/compile_commands.json
  clangd/test/Inputs/background-index/definition.jsonrpc
  clangd/test/Inputs/background-index/foo.cpp
  clangd/test/Inputs/background-index/foo.h
  clangd/test/Inputs/requests.json
  clangd/test/Inputs/symbols.test.yaml
  clangd/test/background-index.test
  clangd/test/compile-commands-path-in-initialize.test
  clangd/test/completion-auto-trigger.test
  clangd/test/completion-snippets.test
  clangd/test/completion.test
  clangd/test/crash-non-added-files.test
  clangd/test/delimited-input-comment-at-the-end.test
  clangd/test/diagnostic-category.test
  clangd/test/diagnostics-notes.test
  clangd/test/diagnostics.test
  clangd/test/did-change-configuration-params.test
  clangd/test/execute-command.test
  clangd/test/exit-with-shutdown.test
  clangd/test/exit-without-shutdown.test
  clangd/test/filestatus.test
  clangd/test/fixits-codeaction.test
  clangd/test/fixits-command.test
  clangd/test/fixits-embed-in-diagnostic.test
  clangd/test/formatting.test
  clangd/test/hover.test
  clangd/test/index-tools.test
  clangd/test/initialize-params-invalid.test
  clangd/test/initialize-params.test
  clangd/test/initialize-sequence.test
  clangd/test/input-mirror.test
  clangd/test/lit.cfg.in
  clangd/test/lit.local.cfg
  clangd/test/protocol.test
  clangd/test/references.test
  clangd/test/rename.test
  clangd/test/signature-help.test
  clangd/test/spaces-in-delimited-input.test
  clangd/test/symbol-info.test
  clangd/test/symbols.test
  clangd/test/test-uri-posix.test
  clangd/test/test-uri-windows.test
  clangd/test/textdocument-didchange-fail.test
  clangd/test/too_large.test
  clangd/test/trace.test
  clangd/test/tweaks-format.test
  clangd/test/type-hierarchy.test
  clangd/test/unsupported-method.test
  clangd/test/utf8.test
  clangd/test/xpc/initialize.test
  clangd/test/xrefs.test
  clangd/unittests/Annotations.cpp
  clangd/unittests/Annotations.h
  clangd/unittests/BackgroundIndexTests.cpp
  clangd/unittests/CMakeLists.txt
  clangd/unittests/CancellationTests.cpp
  clangd/unittests/ClangdTests.cpp
  clangd/unittests/ClangdUnitTests.cpp
  clangd/unittests/CodeCompleteTests.cpp
  clangd/unittests/CodeCompletionStringsTests.cpp
  clangd/unittests/ContextTests.cpp
  clangd/unittests/DexTests.cpp
  clangd/unittests/DiagnosticsTests.cpp
  clangd/unittests/DraftStoreTests.cpp
  clangd/unittests/ExpectedTypeTest.cpp
  clangd/unittests/FSTests.cpp
  clangd/unittests/FileDistanceTests.cpp
  clangd/unittests/FileIndexTests.cpp
  clangd/unittests/FindSymbolsTests.cpp
  clangd/unittests/FunctionTests.cpp
  clangd/unittests/FuzzyMatchTests.cpp
  clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clangd/unittests/HeadersTests.cpp
  clangd/unittests/IndexActionTests.cpp
  clangd/unittests/IndexTests.cpp
  clangd/unittests/JSONTransportTests.cpp
  clangd/unittests/Matchers.h
  clangd/unittests/PrintASTTests.cpp
  clangd/unittests/QualityTests.cpp
  clangd/unittests/RIFFTests.cpp
  clangd/unittests/SelectionTests.cpp
  clangd/unittests/SerializationTests.cpp
  clangd/unittests/SourceCodeTests.cpp
  clangd/unittests/SymbolCollectorTests.cpp
  clangd/unittests/SymbolInfoTests.cpp
  clangd/unittests/SyncAPI.cpp
  clangd/unittests/SyncAPI.h
  clangd/unittests/TUSchedulerTests.cpp
  clangd/unittests/TestFS.cpp
  clangd/unittests/TestFS.h
  clangd/unittests/TestIndex.cpp
  clangd/unittests/TestIndex.h
  clangd/unittests/TestScheme.h
  clangd/unittests/TestTU.cpp
  clangd/unittests/TestTU.h
  clangd/unittests/ThreadingTests.cpp
  clangd/unittests/TraceTests.cpp
  clangd/unittests/TweakTests.cpp
  clangd/unittests/TypeHierarchyTests.cpp
  clangd/unittests/URITests.cpp
  clangd/unittests/XRefsTests.cpp
  clangd/unittests/lit.cfg.in
  clangd/unittests/xpc/CMakeLists.txt
  clangd/unittests/xpc/ConversionTests.cpp
  test/CMakeLists.txt
  test/clangd/Inputs/BenchmarkHeader.h
  test/clangd/Inputs/BenchmarkSource.cpp
  test/clangd/Inputs/background-index/compile_commands.json
  test/clangd/Inputs/background-index/definition.jsonrpc
  test/clangd/Inputs/background-index/foo.cpp
  test/clangd/Inputs/background-index/foo.h
  test/clangd/Inputs/requests.json
  test/clangd/Inputs/symbols.test.yaml
  test/clangd/Unit/lit.site.cfg.py.in
  test/clangd/background-index.test
  test/clangd/compile-commands-path-in-initialize.test
  test/clangd/completion-auto-trigger.test
  test/clangd/completion-snippets.test
  test/clangd/completion.test
  test/clangd/crash-non-added-files.test
  test/clangd/delimited-input-comment-at-the-end.test
  test/clangd/diagnostic-categor

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Also for the record, none of the buildbots in zorg mention check-clang-tools.

Bots run tests by enabling clang-tools-extra and then `check-all`, so this 
patch shouldn't change their behavior.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61187



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

For the record: the tests pass in the shared build configuration.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61187



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This looks nice and minimal, thanks!
Happy to LGTM this, unless @sammccall want to take another look (e.g. to 
account for related information patch)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:115
+  const SourceManager &SM = Info.getSourceManager();
+  std::vector IncludeStack;
+  auto GetIncludeLoc = [&SM](SourceLocation SLoc) {

replace `vector` with `SourceLocation` now that we only need 
one of the locations.



Comment at: unittests/clangd/DiagnosticsTests.cpp:748
+  UnorderedElementsAre(
+  Diag(Main.range(), "Error in include c.h: C++ requires a "
+ "type specifier for all declarations")));

NIT: maybe quote the name of the header for better readability: `error in 
include 'c.h' ...`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall marked an inline comment as done.
sammccall added a comment.
This revision is now accepted and ready to land.

Rest is details only.




Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:59
+  explicit Token(const clang::Token &T);
+
+  tok::TokenKind kind() const { return Kind; }

ilya-biryukov wrote:
> sammccall wrote:
> > Token should be copyable I think?
> Sure, they are copyable. Am I missing something?
No. I think I got confused by the explicit clang::Token constructor.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:62
+  SourceLocation location() const { return Location; }
+  SourceLocation endLocation() const {
+return Location.getLocWithOffset(Length);

ilya-biryukov wrote:
> sammccall wrote:
> > sadly, this would need documentation - it's the one-past-the-end location, 
> > not the last character in the token, not the location of the "next" token, 
> > or the location of the "next" character...
> > 
> > I do wonder whether this is actually the right function to expose...
> >  Do you ever care about end but not start? (The reverse seems likelier). 
> > Having two independent source location accessors obscures the invariant 
> > that they have the same file ID.
> > 
> > I think exposing a `FileRange` accessor instead might be better, but for 
> > now you could also make callers use 
> > `Tok.location().getLocWithOffset(Tok.length())` until we know it's the 
> > right pattern to encourage
> Added a comment. With the comment and an implementation being inline and 
> trivial, I don't think anyone would have trouble understanding what this 
> method does.
(this is ok. I do think a FileRange accessor would make client code more 
readable/less error-prone. Let's discuss offline)



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:78
+  /// For debugging purposes.
+  std::string str() const;
+

ilya-biryukov wrote:
> sammccall wrote:
> > (do we need to have two names for this version?)
> You mean to have distinct names for two different overloads?
> I wouldn't do it, they both have fairly similar outputs, could add a small 
> comment that the one with SourceManager should probably be preferred if you 
> have one.
> 
No sorry, I meant do we need both str() and operator<<



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:95
+  /// End offset (exclusive) in a corresponding file.
+  unsigned End = 0;
+};

ilya-biryukov wrote:
> sammccall wrote:
> > may want to offer `length()` and `StringRef text(const SourceManager&)`
> SG.
> WDYT of exposing the struct members via getters too?
> 
> This would mean uniform access for all things (beginOffset, endOffset, 
> length) and adding a ctor that checks invariants (Begin <= End, 
> File.isValid()) would also fit naturally.
> 
that sounds fine to me, though I don't feel strongly



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:75
+
+  /// For debugging purposes. More verbose than the other overload, but 
requries
+  /// a source manager.

there is no "other overload"



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:77
+
+std::string TokenBuffer::dumpForTests() const {
+  auto PrintToken = [this](const syntax::Token &T) -> std::string {

Nit: I'd suggest moving this all the way to the bottom or something? It's 
pretty big and seems a bit "in the way" here.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:130
+  OS << llvm::formatv(
+  "['{0}'_{1}, '{2}'_{3}) => ['{4}'_{5}, '{6}'_{7})\n",
+  PrintToken(File.SpelledTokens[M.BeginSpelled]), M.BeginSpelled,

As predicted :-) I think these `_` suffixes are a maintenance hazard.

In practice, the assertions are likely to add them by copy-pasting the test 
output.

They guard against a class of error that doesn't seem very likely, and in 
practice they don't even really prevent it (because of the copy/paste issue).

I'd suggest just dropping them, and living with the test assertions being 
slightly ambiguous.

Failing that, some slightly trickier formatting could give more context:

`A B C D E F` --> `A B  ... E F`

With special cases for smaller numbers of tokens. I don't like the irregularity 
of that approach though.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:156
+  // Find the first mapping that produced tokens after \p Expanded.
+  auto It = llvm::upper_bound(
+  File.Mappings, ExpandedIndex,

if you prefer, this is

```
auto It = llvm::bsearch(File.Mappings,
[&](const Mapping &M) { return M.BeginExpanded >= 
Expanded; });
```
I *think* this is clear enough that you can drop the comment, though I may be 
biased.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:94
+TokenBuffer::expandedToRaw(const synt

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Thank you for the quick fix!




Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155
+void __vector_size__LongTest() {
+  VectorSizeLong v;
+}

Could you also add tests where the vector member is being subscripted?  for 
example, `v.x[0] = 0;`



Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155
+void __vector_size__LongTest() {
+  VectorSizeLong v;
+}

gribozavr wrote:
> Could you also add tests where the vector member is being subscripted?  for 
> example, `v.x[0] = 0;`
Should there be a warning about the member being uninitialized?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61246



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

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155
+void __vector_size__LongTest() {
+  VectorSizeLong v;
+}

gribozavr wrote:
> gribozavr wrote:
> > Could you also add tests where the vector member is being subscripted?  for 
> > example, `v.x[0] = 0;`
> Should there be a warning about the member being uninitialized?
Just to be clear -- if there should be a warning, I don't think this patch has 
to necessarily fix everything in the checker and static analyzer core to 
support vector types, the fix for the crash is valuable by itself.  However, if 
there should be a warning, this test should have a fixme.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61246



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359424: [clangd] Move clangd tests to clangd directory. 
check-clangd is no longer part… (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61187?vs=197058&id=197065#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187

Files:
  clang-tools-extra/trunk/CMakeLists.txt
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/test/CMakeLists.txt
  clang-tools-extra/trunk/clangd/test/Inputs/BenchmarkHeader.h
  clang-tools-extra/trunk/clangd/test/Inputs/BenchmarkSource.cpp
  
clang-tools-extra/trunk/clangd/test/Inputs/background-index/compile_commands.json
  clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc
  clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp
  clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h
  clang-tools-extra/trunk/clangd/test/Inputs/requests.json
  clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml
  clang-tools-extra/trunk/clangd/test/background-index.test
  clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test
  clang-tools-extra/trunk/clangd/test/completion-auto-trigger.test
  clang-tools-extra/trunk/clangd/test/completion-snippets.test
  clang-tools-extra/trunk/clangd/test/completion.test
  clang-tools-extra/trunk/clangd/test/crash-non-added-files.test
  clang-tools-extra/trunk/clangd/test/delimited-input-comment-at-the-end.test
  clang-tools-extra/trunk/clangd/test/diagnostic-category.test
  clang-tools-extra/trunk/clangd/test/diagnostics-notes.test
  clang-tools-extra/trunk/clangd/test/diagnostics.test
  clang-tools-extra/trunk/clangd/test/did-change-configuration-params.test
  clang-tools-extra/trunk/clangd/test/execute-command.test
  clang-tools-extra/trunk/clangd/test/exit-with-shutdown.test
  clang-tools-extra/trunk/clangd/test/exit-without-shutdown.test
  clang-tools-extra/trunk/clangd/test/filestatus.test
  clang-tools-extra/trunk/clangd/test/fixits-codeaction.test
  clang-tools-extra/trunk/clangd/test/fixits-command.test
  clang-tools-extra/trunk/clangd/test/fixits-embed-in-diagnostic.test
  clang-tools-extra/trunk/clangd/test/formatting.test
  clang-tools-extra/trunk/clangd/test/hover.test
  clang-tools-extra/trunk/clangd/test/index-tools.test
  clang-tools-extra/trunk/clangd/test/initialize-params-invalid.test
  clang-tools-extra/trunk/clangd/test/initialize-params.test
  clang-tools-extra/trunk/clangd/test/initialize-sequence.test
  clang-tools-extra/trunk/clangd/test/input-mirror.test
  clang-tools-extra/trunk/clangd/test/lit.cfg.in
  clang-tools-extra/trunk/clangd/test/lit.local.cfg
  clang-tools-extra/trunk/clangd/test/protocol.test
  clang-tools-extra/trunk/clangd/test/references.test
  clang-tools-extra/trunk/clangd/test/rename.test
  clang-tools-extra/trunk/clangd/test/signature-help.test
  clang-tools-extra/trunk/clangd/test/spaces-in-delimited-input.test
  clang-tools-extra/trunk/clangd/test/symbol-info.test
  clang-tools-extra/trunk/clangd/test/symbols.test
  clang-tools-extra/trunk/clangd/test/test-uri-posix.test
  clang-tools-extra/trunk/clangd/test/test-uri-windows.test
  clang-tools-extra/trunk/clangd/test/textdocument-didchange-fail.test
  clang-tools-extra/trunk/clangd/test/too_large.test
  clang-tools-extra/trunk/clangd/test/trace.test
  clang-tools-extra/trunk/clangd/test/tweaks-format.test
  clang-tools-extra/trunk/clangd/test/type-hierarchy.test
  clang-tools-extra/trunk/clangd/test/unsupported-method.test
  clang-tools-extra/trunk/clangd/test/utf8.test
  clang-tools-extra/trunk/clangd/test/xpc/initialize.test
  clang-tools-extra/trunk/clangd/test/xrefs.test
  clang-tools-extra/trunk/clangd/unittests/Annotations.cpp
  clang-tools-extra/trunk/clangd/unittests/Annotations.h
  clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
  clang-tools-extra/trunk/clangd/unittests/CancellationTests.cpp
  clang-tools-extra/trunk/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
  clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp
  clang-tools-extra/trunk/clangd/unittests/ContextTests.cpp
  clang-tools-extra/trunk/clangd/unittests/DexTests.cpp
  clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/trunk/clangd/unittests/DraftStoreTests.cpp
  clang-tools-extra/trunk/clangd/unittests/ExpectedTypeTest.cpp
  clang-tools-extra/trunk/clangd/unittests/FSTests.cpp
  clang-tools-extra/trunk/clangd/unittests/FileDistanceTests.cpp
  clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/trunk/clangd/unittests/FindSymbolsTests.cpp
  clan

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

> Also, IIUC the test case that I deleted wasn't actually supposed to produce 
> any diagnostics and the fact that it did was a bug. We could keep it as a 
> regression test but I think it has a rather low value. WDYT?

What do you mean?  The issue that the test is trying to show is that a single 
//-line in a ///-comment breaks the doc comment and produces weird errors.  
Instead it should tell the user that it looks like there's an unintended 
//-line.


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

https://reviews.llvm.org/D61103



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 197068.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -18,8 +18,13 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
 #include "ClangdUnit.h"
+#include "Path.h"
 #include "index/Index.h"
+#include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -45,6 +50,9 @@
   std::string HeaderCode;
   std::string HeaderFilename = "TestTU.h";
 
+  // Name and contents of each file.
+  llvm::StringMap AdditionalFiles;
+
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -21,11 +21,17 @@
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
-  std::vector Cmd = {"clang", FullFilename.c_str()};
   // We want to implicitly include HeaderFilename without messing up offsets.
   // -include achieves this, but sometimes we want #import (to simulate a header
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
+
+  llvm::StringMap Files(AdditionalFiles);
+  Files[FullFilename] = Code;
+  Files[FullHeaderName] = HeaderCode;
+  Files[ImportThunk] = ThunkContents;
+
+  std::vector Cmd = {"clang", FullFilename.c_str()};
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
   if (!HeaderCode.empty()) {
@@ -39,9 +45,7 @@
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS({{FullFilename, Code},
-   {FullHeaderName, HeaderCode},
-   {ImportThunk, ThunkContents}});
+  Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Index = ExternalIndex;
Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,10 +9,11 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Diagnostics.h"
+#include "Path.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "TestIndex.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Basic/Diagnostic.h"
@@ -20,6 +21,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -61,7 +63,8 @@
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
   if (toJSON(arg) != toJSON(LSPDiag)) {
 *result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",
-  toJSON(LSPDiag), toJSON(arg)).str();
+  toJSON(LSPDiag), toJSON(arg))
+.str();
 return false;
   }
   return true;
@@ -83,7 +86,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -114,8 +116,7 @@
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
-DiagSource(Diag::Clang),
-DiagName("undeclared_var_use_suggest"),
+DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
 WithFix(
 Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
 // This is a pretty normal range.
@@ -301,7 +302,8 @@
   MainLSP.severity = getSeverity(DiagnosticsEngine::Error);
   MainLSP.code = "enum_class_reference";
   MainLSP.source = "clang";
-  MainLSP.message = R"(Something terrible happened (fix available)
+  MainLSP.message =
+  R"(Something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -354,9 +356,8 @@
   NoteInHeaderDRI.location.range = NoteInHeader.Range;
   NoteInHeaderDRI.location.uri = HeaderFile;
   MainLSP.relatedInformation = {NoteInMainDRI, NoteInHeade

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Sam McCall via Phabricator via cfe-commits]

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

Nice! Just readability/wording nits




Comment at: clangd/Diagnostics.cpp:463
+  // header.
+  auto ShouldAddDiag = [this](const Diag &D) {
+if (mentionsMainFile(D))

ilya-biryukov wrote:
> Could you inline `ShouldAddDiag` into its single callsite?
Does this improve readability over (inline) `D.Severity >= 
DiagnosticsEngine::Error`?



Comment at: clangd/Diagnostics.cpp:137
+  N.File = FE->getName();
+  N.Message = "Error origin";
+  N.Range = diagnosticRange(Info, LangOpts);

Clang tends to use a phrase ending in "here" for such notes.
Suggest "error occurred here"



Comment at: clangd/Diagnostics.cpp:137
+  N.File = FE->getName();
+  N.Message = "Error origin";
+  N.Range = diagnosticRange(Info, LangOpts);

sammccall wrote:
> Clang tends to use a phrase ending in "here" for such notes.
> Suggest "error occurred here"
message should not be capitalized (capitalization will be added later if needed)



Comment at: clangd/Diagnostics.cpp:141
+  // Update message to mention original file.
+  D.Message = (llvm::Twine("Error in include ",
+   llvm::sys::path::filename(FE->getName())) +

Include is not a noun. "included file"?



Comment at: clangd/Diagnostics.cpp:141
+  // Update message to mention original file.
+  D.Message = (llvm::Twine("Error in include ",
+   llvm::sys::path::filename(FE->getName())) +

sammccall wrote:
> Include is not a noun. "included file"?
"Error" is the severity, which appears elsewhere in LSP. (Other messages do not 
include it)



Comment at: clangd/Diagnostics.cpp:141
+  // Update message to mention original file.
+  D.Message = (llvm::Twine("Error in include ",
+   llvm::sys::path::filename(FE->getName())) +

sammccall wrote:
> sammccall wrote:
> > Include is not a noun. "included file"?
> "Error" is the severity, which appears elsewhere in LSP. (Other messages do 
> not include it)
message should not be capitalized



Comment at: clangd/Diagnostics.cpp:142
+  D.Message = (llvm::Twine("Error in include ",
+   llvm::sys::path::filename(FE->getName())) +
+   ": " + D.Message)

why are we including the filename here?

it obscures the error message, and will be available via the note.



Comment at: clangd/Diagnostics.h:141
   llvm::Optional LastDiag;
+  /// Stores lines of the includes containing a diagnostic.
+  llvm::DenseSet IncludeHasDiag;

nit: "containing an error" (and variable name)

IncludeLinesWithErrors might be a clearer name


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:584
+// terminating early.
+for (auto CIt = RawComments.begin(); CIt != RawComments.end(); ++CIt) {
+  RawComment *C = *CIt;

jkorous wrote:
> gribozavr wrote:
> > Scanning all comments for every decl?  Isn't that O(n^2)?
> > 
> > Also logic duplication below.  I was expecting a call to 
> > `getRawCommentForDeclNoCache`, with an appropriate flag to disable loading 
> > external comments (it is a low-level API, users generally don't call it).
> The important thing is that the expensive operation is source location 
> decomposition. That's why I cache everything - `GetCachedCommentBegin` etc.
> 
> So while you're right that iteration-wise it's O(iteration*c*d) it`s actually 
> O(decompose*c + decompose*d) because of the caching. The current code (which 
> is sorting all the comments) is at least O(decompose*c*ln(c)) once you have 
> more comments than `sqrt(300)` (==`MagicCacheSize` in 
> `SourceManager::getInBeforeInTUCache()`).
> 
> That being said - you're right that just not-loading external comments in 
> `getRawCommentForDeclNoCache` definitely has it's appeal. I'm running a test 
> now get some idea about performance of both approaches.
> 
> BTW in theory we could also do one of these:
> 1. Allow clients to transparently set `MagicCacheSize` in 
> `SourceManager::getInBeforeInTUCache()` which is used for SourceLocation 
> sorting (`BeforeThanCompare`) is currently hard-coded to 300 
> while we are comparing ~100k x ~100k locations.
> 2. Change caching strategies in `SourceManager::getFileID` and 
> `SourceManager::getLineNumber`.
> So while you're right that iteration-wise it's O(iteration*c*d) it`s actually 
> O(decompose*c + decompose*d) because of the caching. 

The cost of decomposing is non-trivial, but the cost of each iteration is still 
at least a hash table lookup.

> That being said - you're right that just not-loading external comments in 
> getRawCommentForDeclNoCache definitely has it's appeal. I'm running a test 
> now get some idea about performance of both approaches.

Reopening, waiting for the results.


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

https://reviews.llvm.org/D61103



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

[clang-tools-extra] r359428 - [clangd] Delete config.clangd_xpc_support from test/ to unbreak check-llvm-tools

[Fangrui Song via cfe-commits]

Author: maskray
Date: Mon Apr 29 02:36:54 2019
New Revision: 359428

URL: http://llvm.org/viewvc/llvm-project?rev=359428&view=rev
Log:
[clangd] Delete config.clangd_xpc_support from test/ to unbreak check-llvm-tools

D61187 didn't delete config.clangd_xpc_support from test/
CLANGD_BUILD_XPC is defined in clangd/CMakeLists.txt and not available in 
test/lit.site.cfg.py.in

Modified:
clang-tools-extra/trunk/test/lit.cfg.py
clang-tools-extra/trunk/test/lit.site.cfg.py.in

Modified: clang-tools-extra/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/lit.cfg.py?rev=359428&r1=359427&r2=359428&view=diff
==
--- clang-tools-extra/trunk/test/lit.cfg.py (original)
+++ clang-tools-extra/trunk/test/lit.cfg.py Mon Apr 29 02:36:54 2019
@@ -115,10 +115,6 @@ if not platform.system() in ['Windows']
 if platform.system() not in ['Windows']:
 config.available_features.add('ansi-escape-sequences')
 
-# XPC support for Clangd.
-if config.clangd_xpc_support:
-config.available_features.add('clangd-xpc-support')
-
 if config.clang_staticanalyzer:
 config.available_features.add('static-analyzer')
 

Modified: clang-tools-extra/trunk/test/lit.site.cfg.py.in
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/lit.site.cfg.py.in?rev=359428&r1=359427&r2=359428&view=diff
==
--- clang-tools-extra/trunk/test/lit.site.cfg.py.in (original)
+++ clang-tools-extra/trunk/test/lit.site.cfg.py.in Mon Apr 29 02:36:54 2019
@@ -11,7 +11,6 @@ config.clang_libs_dir = "@SHLIBDIR@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
-config.clangd_xpc_support = @CLANGD_BUILD_XPC@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.


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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 197073.
kadircet marked 8 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -18,8 +18,13 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
 #include "ClangdUnit.h"
+#include "Path.h"
 #include "index/Index.h"
+#include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -45,6 +50,9 @@
   std::string HeaderCode;
   std::string HeaderFilename = "TestTU.h";
 
+  // Name and contents of each file.
+  llvm::StringMap AdditionalFiles;
+
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -21,11 +21,17 @@
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
-  std::vector Cmd = {"clang", FullFilename.c_str()};
   // We want to implicitly include HeaderFilename without messing up offsets.
   // -include achieves this, but sometimes we want #import (to simulate a header
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
+
+  llvm::StringMap Files(AdditionalFiles);
+  Files[FullFilename] = Code;
+  Files[FullHeaderName] = HeaderCode;
+  Files[ImportThunk] = ThunkContents;
+
+  std::vector Cmd = {"clang", FullFilename.c_str()};
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
   if (!HeaderCode.empty()) {
@@ -39,9 +45,7 @@
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS({{FullFilename, Code},
-   {FullHeaderName, HeaderCode},
-   {ImportThunk, ThunkContents}});
+  Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Index = ExternalIndex;
Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,10 +9,11 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Diagnostics.h"
+#include "Path.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "TestIndex.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Basic/Diagnostic.h"
@@ -20,6 +21,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -61,7 +63,8 @@
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
   if (toJSON(arg) != toJSON(LSPDiag)) {
 *result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",
-  toJSON(LSPDiag), toJSON(arg)).str();
+  toJSON(LSPDiag), toJSON(arg))
+.str();
 return false;
   }
   return true;
@@ -83,7 +86,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -114,8 +116,7 @@
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
-DiagSource(Diag::Clang),
-DiagName("undeclared_var_use_suggest"),
+DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
 WithFix(
 Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
 // This is a pretty normal range.
@@ -301,7 +302,8 @@
   MainLSP.severity = getSeverity(DiagnosticsEngine::Error);
   MainLSP.code = "enum_class_reference";
   MainLSP.source = "clang";
-  MainLSP.message = R"(Something terrible happened (fix available)
+  MainLSP.message =
+  R"(Something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -354,9 +356,8 @@
   NoteInHeaderDRI.location.range = NoteInHeader.Range;
   NoteInHeaderDRI.location.uri = HeaderFile;
   MainLSP.relatedInformation = {NoteInMainDRI, NoteInHeade

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@arphaman @jkorous @thakis  I committed https://reviews.llvm.org/rL359428 to 
fix the following build error. Can one of you check if the test is still built? 
Thanks! (I don't have a Mac...)

  if config.clangd_xpc_support:
  AttributeError: TestingConfig instance has no attribute 'clangd_xpc_support'  
  
 
  FAILED: tools/clang/tools/extra/test/CMakeFiles/check-clang-tools


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/unittests/DiagnosticsTests.cpp
  clangd/unittests/TestTU.cpp
  clangd/unittests/TestTU.h

Index: clangd/unittests/TestTU.h
===
--- clangd/unittests/TestTU.h
+++ clangd/unittests/TestTU.h
@@ -18,8 +18,13 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
 #include "ClangdUnit.h"
+#include "Path.h"
 #include "index/Index.h"
+#include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -45,6 +50,9 @@
   std::string HeaderCode;
   std::string HeaderFilename = "TestTU.h";
 
+  // Name and contents of each file.
+  llvm::StringMap AdditionalFiles;
+
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
Index: clangd/unittests/TestTU.cpp
===
--- clangd/unittests/TestTU.cpp
+++ clangd/unittests/TestTU.cpp
@@ -21,11 +21,17 @@
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
-  std::vector Cmd = {"clang", FullFilename.c_str()};
   // We want to implicitly include HeaderFilename without messing up offsets.
   // -include achieves this, but sometimes we want #import (to simulate a header
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
+
+  llvm::StringMap Files(AdditionalFiles);
+  Files[FullFilename] = Code;
+  Files[FullHeaderName] = HeaderCode;
+  Files[ImportThunk] = ThunkContents;
+
+  std::vector Cmd = {"clang", FullFilename.c_str()};
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
   if (!HeaderCode.empty()) {
@@ -39,9 +45,7 @@
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS({{FullFilename, Code},
-   {FullHeaderName, HeaderCode},
-   {ImportThunk, ThunkContents}});
+  Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Index = ExternalIndex;
Index: clangd/unittests/DiagnosticsTests.cpp
===
--- clangd/unittests/DiagnosticsTests.cpp
+++ clangd/unittests/DiagnosticsTests.cpp
@@ -9,10 +9,11 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Diagnostics.h"
+#include "Path.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "TestIndex.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Basic/Diagnostic.h"
@@ -20,6 +21,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -61,7 +63,8 @@
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
   if (toJSON(arg) != toJSON(LSPDiag)) {
 *result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",
-  toJSON(LSPDiag), toJSON(arg)).str();
+  toJSON(LSPDiag), toJSON(arg))
+.str();
 return false;
   }
   return true;
@@ -83,7 +86,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -114,8 +116,7 @@
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
-DiagSource(Diag::Clang),
-DiagName("undeclared_var_use_suggest"),
+DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
 WithFix(
 Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
 // This is a pretty normal range.
@@ -301,7 +302,8 @@
   MainLSP.severity = getSeverity(DiagnosticsEngine::Error);
   MainLSP.code = "enum_class_reference";
   MainLSP.source = "clang";
-  MainLSP.message = R"(Something terrible happened (fix available)
+  MainLSP.message =
+  R"(Something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -354,9 +356,8 @@
   NoteInHeaderDRI.location.range = NoteInHeader.Range;
   NoteInHeaderDRI.location.uri = HeaderFile;
   MainLSP.relatedInformation = {NoteInMainDRI, NoteInHeaderDRI};
-  EXPECT_THAT(
-  LSPDiags,
-  Elemen

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359432: [clangd] Surface diagnostics from headers inside 
main file (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59302

Files:
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/clangd/Diagnostics.h
  clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
  clang-tools-extra/trunk/clangd/unittests/TestTU.h

Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -13,11 +13,18 @@
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/Basic/AllDiagnostics.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Signals.h"
 #include 
 
 namespace clang {
@@ -102,6 +109,39 @@
   return halfOpenToRange(M, R);
 }
 
+void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+  const LangOptions &LangOpts) {
+  const SourceLocation &DiagLoc = Info.getLocation();
+  const SourceManager &SM = Info.getSourceManager();
+  SourceLocation IncludeInMainFile;
+  auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
+return SM.getIncludeLoc(SM.getFileID(SLoc));
+  };
+  for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
+   IncludeLocation = GetIncludeLoc(IncludeLocation))
+IncludeInMainFile = IncludeLocation;
+  if (IncludeInMainFile.isInvalid())
+return;
+
+  // Update diag to point at include inside main file.
+  D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+  D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
+  D.Range.end = sourceLocToPosition(
+  SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+
+  // Add a note that will point to real diagnostic.
+  const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
+  D.Notes.emplace_back();
+  Note &N = D.Notes.back();
+  N.AbsFile = FE->tryGetRealPathName();
+  N.File = FE->getName();
+  N.Message = "error occurred here";
+  N.Range = diagnosticRange(Info, LangOpts);
+
+  // Update message to mention original file.
+  D.Message = llvm::Twine("in included file: ", D.Message).str();
+}
+
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
   return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
 }
@@ -378,7 +418,7 @@
 Msg.resize(Rest.size());
 };
 CleanMessage(Diag.Message);
-for (auto& Note : Diag.Notes)
+for (auto &Note : Diag.Notes)
   CleanMessage(Note.Message);
 continue;
   }
@@ -477,6 +517,7 @@
 LastDiag = Diag();
 LastDiag->ID = Info.getID();
 FillDiagBase(*LastDiag);
+adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
 
 if (!Info.getFixItHints().empty())
   AddFix(true /* try to invent a message instead of repeating the diag */);
@@ -511,11 +552,15 @@
 void StoreDiags::flushLastDiag() {
   if (!LastDiag)
 return;
-  if (mentionsMainFile(*LastDiag))
+  // Only keeps diagnostics inside main file or the first one coming from a
+  // header.
+  if (mentionsMainFile(*LastDiag) ||
+  (LastDiag->Severity >= DiagnosticsEngine::Level::Error &&
+   IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
 Output.push_back(std::move(*LastDiag));
-  else
-vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);
+  } else {
+vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
+  }
   LastDiag.reset();
 }
 
Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
@@ -9,10 +9,11 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Diagnostics.h"
+#include "Path.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "TestIndex.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Basic/Diagnostic.h"
@@ -20,6 +21,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -6

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.h:113
 
+  // Note that FileSymbols counts References by incrementing it per each file
+  // mentioning the symbol, including headers. This contradicts with the

kadircet wrote:
> ilya-biryukov wrote:
> > We should agree on and stick to a single behavior in both cases.
> > I suggest keeping the current behavior for now (translation units). Why 
> > can't we implement that?
> Because `FileSymbols` only knows about file names and has no idea about 
> whether a given file is TU or not. We might add some heuristics on the file 
> extensions, or change SymbolCollector to count number of files(this would 
> require maintaining a cache to de-duplicate references coming from the same 
> files but originating from a different TU).
Update from the offline conversation:
It should be possible to produce the same counts as static-indexer, will wait 
for an updated revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D61256: [clang-format][docs] Fix the Google C++ and Chromium style guide URLs

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx created this revision.
m4tx added reviewers: djasper, MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Google C++ and Chromium style guides are broken in the clang-format docs. 
This patch updates them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61256

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -138,10 +138,10 @@
 `_
   * ``Google``
 A style complying with `Google's C++ style guide
-`_
+`_
   * ``Chromium``
 A style complying with `Chromium's style guide
-`_
+
`_
   * ``Mozilla``
 A style complying with `Mozilla's style guide
 `_


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -138,10 +138,10 @@
 `_
   * ``Google``
 A style complying with `Google's C++ style guide
-`_
+`_
   * ``Chromium``
 A style complying with `Chromium's style guide
-`_
+`_
   * ``Mozilla``
 A style complying with `Mozilla's style guide
 `_
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r359432 - [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Apr 29 03:25:44 2019
New Revision: 359432

URL: http://llvm.org/viewvc/llvm-project?rev=359432&view=rev
Log:
[clangd] Surface diagnostics from headers inside main file

Reviewers: ioeric, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/Diagnostics.h
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/trunk/clangd/unittests/TestTU.cpp
clang-tools-extra/trunk/clangd/unittests/TestTU.h

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=359432&r1=359431&r2=359432&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Mon Apr 29 03:25:44 2019
@@ -13,11 +13,18 @@
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/Basic/AllDiagnostics.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Signals.h"
 #include 
 
 namespace clang {
@@ -102,6 +109,39 @@ Range diagnosticRange(const clang::Diagn
   return halfOpenToRange(M, R);
 }
 
+void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+  const LangOptions &LangOpts) {
+  const SourceLocation &DiagLoc = Info.getLocation();
+  const SourceManager &SM = Info.getSourceManager();
+  SourceLocation IncludeInMainFile;
+  auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
+return SM.getIncludeLoc(SM.getFileID(SLoc));
+  };
+  for (auto IncludeLocation = GetIncludeLoc(DiagLoc); 
IncludeLocation.isValid();
+   IncludeLocation = GetIncludeLoc(IncludeLocation))
+IncludeInMainFile = IncludeLocation;
+  if (IncludeInMainFile.isInvalid())
+return;
+
+  // Update diag to point at include inside main file.
+  D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+  D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
+  D.Range.end = sourceLocToPosition(
+  SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+
+  // Add a note that will point to real diagnostic.
+  const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
+  D.Notes.emplace_back();
+  Note &N = D.Notes.back();
+  N.AbsFile = FE->tryGetRealPathName();
+  N.File = FE->getName();
+  N.Message = "error occurred here";
+  N.Range = diagnosticRange(Info, LangOpts);
+
+  // Update message to mention original file.
+  D.Message = llvm::Twine("in included file: ", D.Message).str();
+}
+
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
   return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
 }
@@ -378,7 +418,7 @@ std::vector StoreDiags::take(const
 Msg.resize(Rest.size());
 };
 CleanMessage(Diag.Message);
-for (auto& Note : Diag.Notes)
+for (auto &Note : Diag.Notes)
   CleanMessage(Note.Message);
 continue;
   }
@@ -477,6 +517,7 @@ void StoreDiags::HandleDiagnostic(Diagno
 LastDiag = Diag();
 LastDiag->ID = Info.getID();
 FillDiagBase(*LastDiag);
+adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
 
 if (!Info.getFixItHints().empty())
   AddFix(true /* try to invent a message instead of repeating the diag */);
@@ -511,11 +552,15 @@ void StoreDiags::HandleDiagnostic(Diagno
 void StoreDiags::flushLastDiag() {
   if (!LastDiag)
 return;
-  if (mentionsMainFile(*LastDiag))
+  // Only keeps diagnostics inside main file or the first one coming from a
+  // header.
+  if (mentionsMainFile(*LastDiag) ||
+  (LastDiag->Severity >= DiagnosticsEngine::Level::Error &&
+   IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
 Output.push_back(std::move(*LastDiag));
-  else
-vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);
+  } else {
+vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
+  }
   LastDiag.reset();
 }
 

Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=359432&r1=359431&r2=359432&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.h Mon Apr 29 03:25:44 2019
@@ -14,6 +14,9 @@
 #include "clang/

[clang-tools-extra] r359434 - [clangd] Fix windows buildbot, remove stray file after r359424. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr 29 03:35:56 2019
New Revision: 359434

URL: http://llvm.org/viewvc/llvm-project?rev=359434&view=rev
Log:
[clangd] Fix windows buildbot, remove stray file after r359424. NFC

Removed:
clang-tools-extra/trunk/test/clangd/Unit/lit.site.cfg.py.in
Modified:
clang-tools-extra/trunk/clangd/test/lit.cfg.in

Modified: clang-tools-extra/trunk/clangd/test/lit.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/lit.cfg.in?rev=359434&r1=359433&r2=359434&view=diff
==
--- clang-tools-extra/trunk/clangd/test/lit.cfg.in (original)
+++ clang-tools-extra/trunk/clangd/test/lit.cfg.in Mon Apr 29 03:35:56 2019
@@ -4,6 +4,7 @@ import lit.llvm
 import lit.formats
 
 # Reuse clang configuration (PATH setup, etc).
+config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.clang_libs_dir = "@CLANG_LIBS_DIR@"

Removed: clang-tools-extra/trunk/test/clangd/Unit/lit.site.cfg.py.in
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Unit/lit.site.cfg.py.in?rev=359433&view=auto
==
--- clang-tools-extra/trunk/test/clangd/Unit/lit.site.cfg.py.in (original)
+++ clang-tools-extra/trunk/test/clangd/Unit/lit.site.cfg.py.in (removed)
@@ -1,9 +0,0 @@
-@LIT_SITE_CFG_IN_HEADER@
-
-config.extra_tools_obj_dir = "@CLANG_TOOLS_BINARY_DIR@/unittests"
-config.extra_tools_src_dir = "@CLANG_TOOLS_SOURCE_DIR@/unittests"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.shlibdir = "@SHLIBDIR@"
-config.target_triple = "@TARGET_TRIPLE@"
-
-lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg.py")


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

[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D61015#1478886 , @ymandel wrote:

> The problem is that validation can't* be done in the abstract.  It has to be 
> done with respect to a specific match result. Unfortunately, the server won't 
> be layered directly on top of the call to the TextGenerator -- the rewrite 
> rule is interposed between them.  That is, the client of the TG is the 
> rewriterule and that's where the validation has to happen, but the TG is 
> opaque to the rewrite rule, so it can't hardcode that validation logic. So, 
> we'd need to change `TextGenerator` to bundle a validation function and 
> string generator.  Is it still worth it in that case?
>
> *"can't" is a bit strong. I could imagine a design which allows full analysis 
> and validation of rewrite rules before they are executed, but it would be far 
> more sophisticated than the current design.


Could you provide more details into how the server app is layered? 
Specifically, what are the user inputs and how do they get translated into the 
transformer library calls?
I imagine their should be a syntax for textual representation of the generator, 
the representation should allow referring to the named match results. While 
parsing this representation, it should be possible to collect all matches of 
named nodes and make sure there are corresponding binding in the rewrite rule.

If the setup is different, I may be missing where the complexity comes from and 
I would love to learn about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gabor Marton via Phabricator via cfe-commits]

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

ASTImporter.cpp and ASTStructuralEquivalence.cpp looks good to me now. I am 
resigning as  a reviewer since I don't feel competent enough to review the rest 
of the change.


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

https://reviews.llvm.org/D60934



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

[clang-tools-extra] r359442 - [clangd] Fix unittests CMake rules

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr 29 04:47:52 2019
New Revision: 359442

URL: http://llvm.org/viewvc/llvm-project?rev=359442&view=rev
Log:
[clangd] Fix unittests CMake rules

Modified:
clang-tools-extra/trunk/clangd/test/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/CMakeLists.txt?rev=359442&r1=359441&r2=359442&view=diff
==
--- clang-tools-extra/trunk/clangd/test/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/test/CMakeLists.txt Mon Apr 29 04:47:52 2019
@@ -19,10 +19,7 @@ endforeach()
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.cfg)
-configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/../unittests/lit.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/../unittests/lit.cfg)
 
 add_lit_testsuite(check-clangd "Running the Clangd regression tests"
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit;${CMAKE_CURRENT_BINARY_DIR}
+  ${CMAKE_CURRENT_BINARY_DIR}/../unittests;${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANGD_TEST_DEPS})

Modified: clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt?rev=359442&r1=359441&r2=359442&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt Mon Apr 29 04:47:52 
2019
@@ -90,3 +90,7 @@ target_link_libraries(ClangdTests
 if (CLANGD_BUILD_XPC)
   add_subdirectory(xpc)
 endif ()
+
+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.cfg)


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

[PATCH] D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

Looks good, thanks!


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

https://reviews.llvm.org/D59485



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

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

@Szelethus I know the dependent patch D59464  
will move `examples/analyzer-plugin` to `test/Analysis/plugins/...`, but this 
patch still seems to affect `examples/`. Are you sure this is the right diff? 
Because you are adding brand new files and brand new "folders", I don't think 
that'll apply cleanly.




Comment at: test/Analysis/checker-plugins.c:35
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \

Why isn't here a space before the line break escape `\`, but in the other lines?


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

https://reviews.llvm.org/D59465



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

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

I think this is good. Patch still marked as //Needs review// for some reason. 😦 
Can we look up this `blocking review` thing? Perhaps this could be marked ready 
to roll once the dependency patch is ironed out.




Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:332
+  AnOpts.Config.insert({(Twine() + CheckerFullName + ":" + OptionName).str(),
+DefaultValStr});
 }

baloghadamsoftware wrote:
> `Twine(CheckerFullName) + ":" + OptionName`. However, since the constructor 
> `Twine(const StringRef &Str)` is implicit, `CheckerFullName + ":" + 
> OptionName` results the same and is more readable.
This comment is **Done**.


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

https://reviews.llvm.org/D57922



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

@dcoughlin How would removing the `USAGE` part of the dump and keeping only the 
list of options and their formatted help sound? That way, this option will not 
invite the user to directly call the analyzer.

In D57858#1432714 , @dcoughlin wrote:

> - The help text also recommends invoking -cc1 directly or through the driver 
> with -Xclang. Neither of these are supported end-user interfaces to the 
> analyzer.


Calling this option itself, at least based on the original commit's first line, 
is through `-cc1`, and thus using a "checker/SA developer interface". This 
seems more purely as a tooling help, which as expressed by @dkrupp earlier, is 
helpful for "wrangler tools".

@Szelethus From a CodeChecker guy's perspective, I am a bit scared about the 
size this dump can get assuming every option is given a description/help text 
nicely, but all in all I like the direction of this patch.




Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:584-592
+  Out << "USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config "
+
"\n\n";
+  Out << "   clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, "
+  "-analyzer-config OPTION2=VALUE, 
...\n\n";
+  Out << "   clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang"
+
"\n\n";
+  Out << "   clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang "

(I mean killing these lines.)


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

https://reviews.llvm.org/D57858



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

[PATCH] D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:625
+  "class shouldNotBeImported {};", Lang_CXX, "class realDecl {};", 
Lang_CXX,
+  "shouldNotBeImported", RedirectingImporter::Constructor);
+  auto *Imported = cast(To);

I just realized that maybe we could simplify and make this part more elegant:
If `ASTImporterTestBase` had a data member with the type `ImporterConstructor` 
then we could overwrite that data member in the constructor of `CustomImporter`.
(The `ASTImporter` is created lazily when the first `getImportedDecl` is 
called, but by that time the creator is already set, so this will work.)
This way we could elide the last argument here 
(`RedirectingImporter::Constructor`) and in all subsequent `TEST_P` tests, it 
would be enough to set up the creator per test fixture.



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

https://reviews.llvm.org/D59485



___
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 created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, lebedev.ri, 
JonasToth.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: gamesh411, rnkovacs.
Herald added a project: clang.

Accidentally taking the size of a struct-pointer type or a value of this type 
is more common than explicitly using the `&` operator for the value. This patch 
extends the check to include these cases.


Repository:
  rCTE Clang Tools Extra

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,13 @@
 Array10* ptr;
   };
   typedef const MyStruct TMyStruct;
+  typedef const MyStruct *PMyStruct;
 
   static TMyStruct kGlocalMyStruct = {};
   static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
 
   MyStruct S;
+  PMyStruct PS;
   Array10 A10;
 
   int sum = 0;
@@ -225,6 +227,12 @@
   // 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(&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,13 @@
 Array10* ptr;
   };
   typedef const MyStruct TMyStruct;
+  typedef const MyStruct *PMyStruct;
 
   static TMyStruct kGlocalMyStruct = {};
   static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
 
   MyStruct S;
+  PMyStruct PS;
   Array10 A10;
 
   int sum = 0;
@@ -225,6 +227,12 @@
   // 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(&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 PointerToSt

r359448 - [libclang] Restore old clang_Cursor_isAnonymous behaviour

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Mon Apr 29 06:44:07 2019
New Revision: 359448

URL: http://llvm.org/viewvc/llvm-project?rev=359448&view=rev
Log:
[libclang] Restore old clang_Cursor_isAnonymous behaviour

D54996 Changed the behaviour of clang_Cursor_isAnonymous, but there is no 
alternative available to get the old behaviour in some cases, which is 
essential for determining if a record is syntactically accessible, e.g.

struct {
  int x;
  int y;
} foo;

struct {
  struct {
int x;
int y;
  };
} bar;

void fun(struct { int x; int y; } *param);
The only 'anonymous' struct here is the one nested in bar, since there is
no way to reference the struct itself, only the fields within. Though the
anonymity applies to the instance itself, not the type.

To avoid confusion, I have added a new function called 
clang_Cursor_isAnonymousRecordDecl
which has the old behaviour of clang_Cursor_isAnonymous (and updated the doc
for the latter as well, which was seemingly forgotten).

Patch by Jorn Vernee.

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

Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/test/Index/print-type.c
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=359448&r1=359447&r2=359448&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Mon Apr 29 06:44:07 2019
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 55
+#define CINDEX_VERSION_MINOR 56
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -3921,10 +3921,16 @@ CINDEX_LINKAGE CXType clang_Type_getModi
 CINDEX_LINKAGE long long clang_Cursor_getOffsetOfField(CXCursor C);
 
 /**
+ * Determine whether the given cursor represents an anonymous
+ * tag or namespace
+ */
+CINDEX_LINKAGE unsigned clang_Cursor_isAnonymous(CXCursor C);
+
+/**
  * Determine whether the given cursor represents an anonymous record
  * declaration.
  */
-CINDEX_LINKAGE unsigned clang_Cursor_isAnonymous(CXCursor C);
+CINDEX_LINKAGE unsigned clang_Cursor_isAnonymousRecordDecl(CXCursor C);
 
 enum CXRefQualifierKind {
   /** No ref-qualifier was provided. */

Modified: cfe/trunk/test/Index/print-type.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-type.c?rev=359448&r1=359447&r2=359448&view=diff
==
--- cfe/trunk/test/Index/print-type.c (original)
+++ cfe/trunk/test/Index/print-type.c Mon Apr 29 06:44:07 2019
@@ -15,6 +15,20 @@ int f2(int incompletearray[]);
 enum Enum{i}; enum Enum elaboratedEnumType();
 struct Struct{}; struct Struct elaboratedStructType();
 
+struct {
+  int x;
+  int y;
+} foo;
+
+struct {
+  struct {
+int x;
+int y;
+  };
+} bar;
+
+void fun(struct { int x; int y; } *param);
+
 // RUN: c-index-test -test-print-type %s | FileCheck %s
 // CHECK: FunctionDecl=f:3:6 (Definition) [type=int *(int *, char *, FooType, 
int *, void (*)(int))] [typekind=FunctionProto] [canonicaltype=int *(int *, 
char *, int, int *, void (*)(int))] [canonicaltypekind=FunctionProto] 
[resulttype=int *] [resulttypekind=Pointer] [args= [int *] [Pointer] [char *] 
[Pointer] [FooType] [Typedef] [int [5]] [ConstantArray] [void (*)(int)] 
[Pointer]] [isPOD=0]
 // CHECK: ParmDecl=p:3:13 (Definition) [type=int *] [typekind=Pointer] 
[isPOD=1] [pointeetype=int] [pointeekind=Int]
@@ -53,3 +67,7 @@ struct Struct{}; struct Struct elaborate
 // CHECK: StructDecl=Struct:16:8 (Definition) [type=struct Struct] 
[typekind=Record] [isPOD=1]
 // CHECK: FunctionDecl=elaboratedStructType:16:32 [type=struct Struct ()] 
[typekind=FunctionNoProto] [canonicaltype=struct Struct ()] 
[canonicaltypekind=FunctionNoProto] [resulttype=struct Struct] 
[resulttypekind=Elaborated] [isPOD=0]
 // CHECK: TypeRef=struct Struct:16:8 [type=struct Struct] [typekind=Record] 
[isPOD=1]
+// CHECK: StructDecl=:18:1 (Definition) [type=struct (anonymous at 
{{.*}}print-type.c:18:1)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] 
[isAnonRecDecl=0]
+// CHECK: StructDecl=:23:1 (Definition) [type=struct (anonymous at 
{{.*}}print-type.c:23:1)] [typekind=Record] [isPOD=1] [nbFields=1] [isAnon=1] 
[isAnonRecDecl=0]
+// CHECK: StructDecl=:24:3 (Definition) [type=struct (anonymous at 
{{.*}}print-type.c:24:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] 
[isAnonRecDecl=1]
+// CHECK: StructDecl=:30:10 (Definition) [type=struct (anonymous at 
{{.*}}print-type.c:30:10)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] 
[isAnonRecDecl=0]

Modified: cfe/trunk/tools/c-index-test/c-index-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/c-index-test.c?rev=

[PATCH] D61232: [libclang] Restore old clang_Cursor_isAnonymous behaviour

[Ivan Donchevskii via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359448: [libclang] Restore old clang_Cursor_isAnonymous 
behaviour (authored by yvvan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61232?vs=197038&id=197103#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61232

Files:
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/test/Index/print-type.c
  cfe/trunk/tools/c-index-test/c-index-test.c
  cfe/trunk/tools/libclang/CXType.cpp

Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 55
+#define CINDEX_VERSION_MINOR 56
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -3921,10 +3921,16 @@
 CINDEX_LINKAGE long long clang_Cursor_getOffsetOfField(CXCursor C);
 
 /**
+ * Determine whether the given cursor represents an anonymous
+ * tag or namespace
+ */
+CINDEX_LINKAGE unsigned clang_Cursor_isAnonymous(CXCursor C);
+
+/**
  * Determine whether the given cursor represents an anonymous record
  * declaration.
  */
-CINDEX_LINKAGE unsigned clang_Cursor_isAnonymous(CXCursor C);
+CINDEX_LINKAGE unsigned clang_Cursor_isAnonymousRecordDecl(CXCursor C);
 
 enum CXRefQualifierKind {
   /** No ref-qualifier was provided. */
Index: cfe/trunk/test/Index/print-type.c
===
--- cfe/trunk/test/Index/print-type.c
+++ cfe/trunk/test/Index/print-type.c
@@ -15,6 +15,20 @@
 enum Enum{i}; enum Enum elaboratedEnumType();
 struct Struct{}; struct Struct elaboratedStructType();
 
+struct {
+  int x;
+  int y;
+} foo;
+
+struct {
+  struct {
+int x;
+int y;
+  };
+} bar;
+
+void fun(struct { int x; int y; } *param);
+
 // RUN: c-index-test -test-print-type %s | FileCheck %s
 // CHECK: FunctionDecl=f:3:6 (Definition) [type=int *(int *, char *, FooType, int *, void (*)(int))] [typekind=FunctionProto] [canonicaltype=int *(int *, char *, int, int *, void (*)(int))] [canonicaltypekind=FunctionProto] [resulttype=int *] [resulttypekind=Pointer] [args= [int *] [Pointer] [char *] [Pointer] [FooType] [Typedef] [int [5]] [ConstantArray] [void (*)(int)] [Pointer]] [isPOD=0]
 // CHECK: ParmDecl=p:3:13 (Definition) [type=int *] [typekind=Pointer] [isPOD=1] [pointeetype=int] [pointeekind=Int]
@@ -53,3 +67,7 @@
 // CHECK: StructDecl=Struct:16:8 (Definition) [type=struct Struct] [typekind=Record] [isPOD=1]
 // CHECK: FunctionDecl=elaboratedStructType:16:32 [type=struct Struct ()] [typekind=FunctionNoProto] [canonicaltype=struct Struct ()] [canonicaltypekind=FunctionNoProto] [resulttype=struct Struct] [resulttypekind=Elaborated] [isPOD=0]
 // CHECK: TypeRef=struct Struct:16:8 [type=struct Struct] [typekind=Record] [isPOD=1]
+// CHECK: StructDecl=:18:1 (Definition) [type=struct (anonymous at {{.*}}print-type.c:18:1)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=0]
+// CHECK: StructDecl=:23:1 (Definition) [type=struct (anonymous at {{.*}}print-type.c:23:1)] [typekind=Record] [isPOD=1] [nbFields=1] [isAnon=1] [isAnonRecDecl=0]
+// CHECK: StructDecl=:24:3 (Definition) [type=struct (anonymous at {{.*}}print-type.c:24:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=1]
+// CHECK: StructDecl=:30:10 (Definition) [type=struct (anonymous at {{.*}}print-type.c:30:10)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1] [isAnonRecDecl=0]
Index: cfe/trunk/tools/libclang/CXType.cpp
===
--- cfe/trunk/tools/libclang/CXType.cpp
+++ cfe/trunk/tools/libclang/CXType.cpp
@@ -1253,6 +1253,16 @@
 
   return 0;
 }
+
+unsigned clang_Cursor_isAnonymousRecordDecl(CXCursor C){
+  if (!clang_isDeclaration(C.kind))
+return 0;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  if (const RecordDecl *FD = dyn_cast_or_null(D))
+return FD->isAnonymousStructOrUnion();
+  return 0;
+}
+
 CXType clang_Type_getNamedType(CXType CT){
   QualType T = GetQualType(CT);
   const Type *TP = T.getTypePtrOrNull();
Index: cfe/trunk/tools/c-index-test/c-index-test.c
===
--- cfe/trunk/tools/c-index-test/c-index-test.c
+++ cfe/trunk/tools/c-index-test/c-index-test.c
@@ -1665,6 +1665,12 @@
   }
 }
 
+/* Print if it is an anonymous record decl */
+{
+  unsigned isAnonRecDecl = clang_Cursor_isAnonymousRecordDecl(cursor);
+  printf(" [isAnonRecDecl=%d]", isAnonRecDecl);
+}
+
 printf("\n");
   }
   return CXChildVisit_Recurse;
___
cfe-commits mailing list

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

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



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

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?



Comment at: test/clang-tidy/bugprone-sizeof-expression.cpp:231
+  sum += sizeof(MyStruct*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PMyStruct);

Why is this printed at `sizeof(A*)`? Do we not print the name of the actual 
type used in the expression?


Repository:
  rCTE Clang Tools Extra

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] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

[Jorn Vernee via Phabricator via cfe-commits]

JornVernee added a comment.

@yvvan Mind taking a look here as well?


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

https://reviews.llvm.org/D61239



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

The current diff (196894) seems to have the same `std::setw` issue as the 
previous one. Here's what it's trying to compile:

  _MRTIMP2 _Smanip __cdecl setw(streamsize wide)
  { // manipulator to set width
return (_Smanip(&swfun, wide));
   }

Here's the definition for `_Smanip`:

template
  struct _Smanip
{   // store function pointer and argument value
  _Smanip(void (__cdecl *_Left)(ios_base&, _Arg), _Arg _Val)
: _Pfun(_Left), _Manarg(_Val)
{   // construct from function pointer and argument value
}
  
 void (__cdecl *_Pfun)(ios_base&, _Arg);// the function 
pointer
 _Arg _Manarg;  // the argument value
   };


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

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

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

richard.townsend.arm added a comment.

Apologies, I meant to write "here's what Clang is trying to call" in the 
previous comment. It's clear from looking at MSVC's output that MSVC expects to 
return indirectly via X0, implying that `_Smanip` is not aggregate by MSVC's 
definition.


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

r359453 - [libclang] Add missing export for clang_Cursor_isAnonymousRecordDecl

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Mon Apr 29 07:13:11 2019
New Revision: 359453

URL: http://llvm.org/viewvc/llvm-project?rev=359453&view=rev
Log:
[libclang] Add missing export for clang_Cursor_isAnonymousRecordDecl

Follow up for D61232 to fix build.

Modified:
cfe/trunk/tools/libclang/libclang.exports

Modified: cfe/trunk/tools/libclang/libclang.exports
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/libclang.exports?rev=359453&r1=359452&r2=359453&view=diff
==
--- cfe/trunk/tools/libclang/libclang.exports (original)
+++ cfe/trunk/tools/libclang/libclang.exports Mon Apr 29 07:13:11 2019
@@ -39,6 +39,7 @@ clang_Cursor_getSpellingNameRange
 clang_Cursor_getTranslationUnit
 clang_Cursor_getReceiverType
 clang_Cursor_isAnonymous
+clang_Cursor_isAnonymousRecordDecl
 clang_Cursor_isBitField
 clang_Cursor_isDynamicCall
 clang_Cursor_isExternalSymbol


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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10362
 
+void CGOpenMPRuntime::emitFunctionProlog(CodeGenFunction &CGF, const Decl *D){
+  if (const auto *FD = dyn_cast(D)) {

Bad formatting.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast(D)) {
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+  HasEmittedDeclareTargetRegion = true;

No need for the braces



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast(D)) {
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+  HasEmittedDeclareTargetRegion = true;

ABataev wrote:
> No need for the braces
What if `declare target` is used only for variabes but not for the functions?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568



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

[clang-tools-extra] r359455 - [clangd] Add separate unit tests for CanonicalIncludes. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr 29 07:36:26 2019
New Revision: 359455

URL: http://llvm.org/viewvc/llvm-project?rev=359455&view=rev
Log:
[clangd] Add separate unit tests for CanonicalIncludes. NFC

Added:
clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp
Modified:
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt?rev=359455&r1=359454&r2=359455&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt Mon Apr 29 07:36:26 
2019
@@ -25,6 +25,7 @@ add_unittest(ClangdUnitTests ClangdTests
   Annotations.cpp
   BackgroundIndexTests.cpp
   CancellationTests.cpp
+  CanonicalIncludesTests.cpp
   ClangdTests.cpp
   ClangdUnitTests.cpp
   CodeCompleteTests.cpp

Added: clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp?rev=359455&view=auto
==
--- clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp (added)
+++ clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp Mon Apr 
29 07:36:26 2019
@@ -0,0 +1,62 @@
+//===-- CanonicalIncludesTests.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 "index/CanonicalIncludes.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(CanonicalIncludesTest, CXXStandardLibrary) {
+  CanonicalIncludes CI;
+  addSystemHeadersMapping(&CI);
+
+  // Usual standard library symbols are mapped correctly.
+  EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
+  // std::move is ambiguous, currently mapped only based on path
+  EXPECT_EQ("", CI.mapHeader("libstdc++/bits/move.h", "std::move"));
+  EXPECT_EQ("path/utility.h", CI.mapHeader("path/utility.h", "std::move"));
+  // Unknown std symbols aren't mapped.
+  EXPECT_EQ("foo/bar.h", CI.mapHeader("foo/bar.h", "std::notathing"));
+  // iosfwd declares some symbols it doesn't own.
+  EXPECT_EQ("", CI.mapHeader("iosfwd", "std::ostream"));
+  // And (for now) we assume it owns the others.
+  EXPECT_EQ("", CI.mapHeader("iosfwd", "std::notwathing"));
+}
+
+TEST(CanonicalIncludesTest, PathMapping) {
+  // As used for IWYU pragmas.
+  CanonicalIncludes CI;
+  CI.addMapping("foo/bar", "");
+
+  EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol"));
+  EXPECT_EQ("bar/bar", CI.mapHeader("bar/bar", "some::symbol"));
+}
+
+TEST(CanonicalIncludesTest, SymbolMapping) {
+  // As used for standard library.
+  CanonicalIncludes CI;
+  CI.addSymbolMapping("some::symbol", "");
+
+  EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol"));
+  EXPECT_EQ("foo/bar", CI.mapHeader("foo/bar", "other::symbol"));
+}
+
+TEST(CanonicalIncludesTest, Precedence) {
+  CanonicalIncludes CI;
+  CI.addMapping("some/path", "");
+  CI.addSymbolMapping("some::symbol", "");
+
+  // Symbol mapping beats path mapping.
+  EXPECT_EQ("", CI.mapHeader("some/path", "some::symbol"));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

Modified: clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp?rev=359455&r1=359454&r2=359455&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp Mon Apr 
29 07:36:26 2019
@@ -931,47 +931,15 @@ TEST_F(SymbolCollectorTest, IncludeHeade
   UnorderedElementsAre(IncludeHeaderWithRef(TestHeaderURI, 1u)));
 }
 
-#ifndef _WIN32
 TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
   CollectorOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
   addSystemHeadersMapping(&Includes);
   CollectorOpts.Includes = &Includes;
-  // bits/basic_string.h$ should be mapped to 
-  TestHeaderName = "/nasty/bits/basic_string.h";
-  TestFileName = "/nasty/bits/basic_string.cpp";
-  TestHeaderURI = URI::create(TestHeaderName).toString();
-  runSymbolCollector("class string {};", /*Main=*/"");
-  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("string"),
-  DeclURI(TestHeaderURI),
-   

[PATCH] D61264: Fix inconsistency in calculating DIAG_START values

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop created this revision.
russell.gallop added reviewers: xazax.hun, rsmith.
russell.gallop added a project: clang.
Herald added a subscriber: rnkovacs.

The inconsistency was introduced at r313975. As DIAG_SIZE_CROSSTU and 
DIAG_SIZE_COMMENT are both 100 this should be NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61264

Files:
  clang/include/clang/Basic/DiagnosticIDs.h


Index: clang/include/clang/Basic/DiagnosticIDs.h
===
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -50,8 +50,8 @@
   DIAG_START_PARSE = DIAG_START_LEX   + DIAG_SIZE_LEX,
   DIAG_START_AST   = DIAG_START_PARSE + DIAG_SIZE_PARSE,
   DIAG_START_COMMENT   = DIAG_START_AST   + DIAG_SIZE_AST,
-  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_CROSSTU,
-  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_COMMENT,
+  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_COMMENT,
+  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_CROSSTU,
   DIAG_START_ANALYSIS  = DIAG_START_SEMA  + DIAG_SIZE_SEMA,
   DIAG_START_REFACTORING   = DIAG_START_ANALYSIS  + DIAG_SIZE_ANALYSIS,
   DIAG_UPPER_LIMIT = DIAG_START_REFACTORING   + 
DIAG_SIZE_REFACTORING


Index: clang/include/clang/Basic/DiagnosticIDs.h
===
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -50,8 +50,8 @@
   DIAG_START_PARSE = DIAG_START_LEX   + DIAG_SIZE_LEX,
   DIAG_START_AST   = DIAG_START_PARSE + DIAG_SIZE_PARSE,
   DIAG_START_COMMENT   = DIAG_START_AST   + DIAG_SIZE_AST,
-  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_CROSSTU,
-  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_COMMENT,
+  DIAG_START_CROSSTU   = DIAG_START_COMMENT   + DIAG_SIZE_COMMENT,
+  DIAG_START_SEMA  = DIAG_START_CROSSTU   + DIAG_SIZE_CROSSTU,
   DIAG_START_ANALYSIS  = DIAG_START_SEMA  + DIAG_SIZE_SEMA,
   DIAG_START_REFACTORING   = DIAG_START_ANALYSIS  + DIAG_SIZE_ANALYSIS,
   DIAG_UPPER_LIMIT = DIAG_START_REFACTORING   + DIAG_SIZE_REFACTORING
___
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 inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1080
+  if (const auto *Constructor = dyn_cast(RD))
+if (Constructor->isUserProvided())
+  return true;

So I think that the problem with this new check is that it doesn't check all of 
the constructors. I replaced it with 

for (auto it = RD->ctor_begin(); it != RD->ctor_end(); ++it) {
  if (it->isUserProvided())
return true;
}

And that seems to resolve the `setw` problem.


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

r359459 - [OPENMP]Fix PR41617: crash on template instantiation.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Apr 29 08:51:36 2019
New Revision: 359459

URL: http://llvm.org/viewvc/llvm-project?rev=359459&view=rev
Log:
[OPENMP]Fix PR41617: crash on template instantiation.

Fixed the crash on the template instantiation when trying to check the
data locality in the current instantiation scope.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/critical_ast_print.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=359459&r1=359458&r2=359459&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Apr 29 08:51:36 2019
@@ -1145,7 +1145,7 @@ bool DSAStackTy::isOpenMPLocal(VarDecl *
   return false;
 TopScope = I->CurScope ? I->CurScope->getParent() : nullptr;
 Scope *CurScope = getCurScope();
-while (CurScope != TopScope && !CurScope->isDeclScope(D))
+while (CurScope && CurScope != TopScope && !CurScope->isDeclScope(D))
   CurScope = CurScope->getParent();
 return CurScope != TopScope;
   }

Modified: cfe/trunk/test/OpenMP/critical_ast_print.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/critical_ast_print.cpp?rev=359459&r1=359458&r2=359459&view=diff
==
--- cfe/trunk/test/OpenMP/critical_ast_print.cpp (original)
+++ cfe/trunk/test/OpenMP/critical_ast_print.cpp Mon Apr 29 08:51:36 2019
@@ -15,28 +15,46 @@ void foo() {}
 // CHECK: template  int tmain(T argc, char **argv)
 // CHECK: static int a;
 // CHECK-NEXT: #pragma omp critical{{$}}
-// CHECK-NEXT: a = 2;
+// CHECK-NEXT: a = argv[0][0];
 // CHECK-NEXT: ++a;
+// CHECK-NEXT: #pragma omp critical{{$}}
+// CHECK-NEXT: {
+// CHECK-NEXT: int b = 10;
+// CHECK-NEXT: T c = 100;
+// CHECK-NEXT: a = b + c;
+// CHECK-NEXT: }
 // CHECK-NEXT: #pragma omp critical (the_name) hint(N){{$}}
 // CHECK-NEXT: foo();
 // CHECK-NEXT: return N;
 // CHECK: template<> int tmain(int argc, char **argv)
 template 
-int tmain (T argc, char **argv) {
+int tmain(T argc, char **argv) {
   T b = argc, c, d, e, f, g;
   static int a;
 // CHECK: static int a;
 #pragma omp critical
-  a=2;
-// CHECK-NEXT: #pragma omp critical
-// CHECK-NEXT: a = 2;
-// CHECK-NEXT: ++a;
+  a = argv[0][0];
   ++a;
-#pragma omp critical  (the_name) hint(N)
+  // CHECK-NEXT: #pragma omp critical
+  // CHECK-NEXT: a = argv[0][0];
+  // CHECK-NEXT: ++a;
+  // CHECK-NEXT: #pragma omp critical{{$}}
+  // CHECK-NEXT: {
+  // CHECK-NEXT: int b = 10;
+  // CHECK-NEXT: int c = 100;
+  // CHECK-NEXT: a = b + c;
+  // CHECK-NEXT: }
+#pragma omp critical
+  {
+int b = 10;
+T c = 100;
+a = b + c;
+  }
+#pragma omp critical(the_name) hint(N)
   foo();
-// CHECK-NEXT: #pragma omp critical (the_name) hint(4)
-// CHECK-NEXT: foo();
-// CHECK-NEXT: return 4;
+  // CHECK-NEXT: #pragma omp critical (the_name) hint(4)
+  // CHECK-NEXT: foo();
+  // CHECK-NEXT: return 4;
   return N;
 }
 


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

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

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

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

In D59465#1482302 , @whisperity wrote:

> @Szelethus I know the dependent patch D59464 
>  will move `examples/analyzer-plugin` to 
> `test/Analysis/plugins/...`, but this patch still seems to affect 
> `examples/`. Are you sure this is the right diff? Because you are adding 
> brand new files and brand new "folders", I don't think that'll apply cleanly.


Oh wow, you're totally right, no idea how did I mess this up.


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

https://reviews.llvm.org/D59465



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

r359468 - [LibTooling] Fix unneeded use of unique_ptr where shared_ptr is expected.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Mon Apr 29 09:57:40 2019
New Revision: 359468

URL: http://llvm.org/viewvc/llvm-project?rev=359468&view=rev
Log:
[LibTooling] Fix unneeded use of unique_ptr where shared_ptr is expected.

Summary: This fixes a few places in the Stencil implementation where a 
unique_ptr is created at a callsite that expects shared_ptr. Since the former 
implicitly converts to the latter, the code compiles and runs correctly as is.  
But, there's no reason to involve unique_ptr -- the current code was leftover 
from a previous version in which unique_ptr was the expected type.

Reviewers: sbenza

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp?rev=359468&r1=359467&r2=359468&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp Mon Apr 29 09:57:40 2019
@@ -16,6 +16,7 @@
 #include "clang/Tooling/Refactoring/SourceCode.h"
 #include "llvm/Support/Errc.h"
 #include 
+#include 
 #include 
 
 using namespace clang;
@@ -183,17 +184,17 @@ Stencil::eval(const MatchFinder::MatchRe
 }
 
 StencilPart stencil::text(StringRef Text) {
-  return StencilPart(llvm::make_unique(Text));
+  return StencilPart(std::make_shared(Text));
 }
 
 StencilPart stencil::node(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, 
SemiAssociation::Inferred));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Inferred));
 }
 
 StencilPart stencil::sNode(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, SemiAssociation::Always));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Always));
 }
 
 StencilPart stencil::dPrint(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id));
+  return StencilPart(std::make_shared(Id));
 }


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

[PATCH] D61005: [LibTooling] Fix unneeded use of unique_ptr where shared_ptr is expected.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359468: [LibTooling] Fix unneeded use of unique_ptr where 
shared_ptr is expected. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61005?vs=196227&id=197136#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61005

Files:
  cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp


Index: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
@@ -16,6 +16,7 @@
 #include "clang/Tooling/Refactoring/SourceCode.h"
 #include "llvm/Support/Errc.h"
 #include 
+#include 
 #include 
 
 using namespace clang;
@@ -183,17 +184,17 @@
 }
 
 StencilPart stencil::text(StringRef Text) {
-  return StencilPart(llvm::make_unique(Text));
+  return StencilPart(std::make_shared(Text));
 }
 
 StencilPart stencil::node(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, 
SemiAssociation::Inferred));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Inferred));
 }
 
 StencilPart stencil::sNode(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, SemiAssociation::Always));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Always));
 }
 
 StencilPart stencil::dPrint(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id));
+  return StencilPart(std::make_shared(Id));
 }


Index: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
@@ -16,6 +16,7 @@
 #include "clang/Tooling/Refactoring/SourceCode.h"
 #include "llvm/Support/Errc.h"
 #include 
+#include 
 #include 
 
 using namespace clang;
@@ -183,17 +184,17 @@
 }
 
 StencilPart stencil::text(StringRef Text) {
-  return StencilPart(llvm::make_unique(Text));
+  return StencilPart(std::make_shared(Text));
 }
 
 StencilPart stencil::node(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, SemiAssociation::Inferred));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Inferred));
 }
 
 StencilPart stencil::sNode(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id, SemiAssociation::Always));
+  return StencilPart(std::make_shared(Id, SemiAssociation::Always));
 }
 
 StencilPart stencil::dPrint(StringRef Id) {
-  return StencilPart(llvm::make_unique(Id));
+  return StencilPart(std::make_shared(Id));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

This is also making our Mac builders fail on configuration, and looks like a 
directory wasn't included properly when a test moved to a subdirectory -- would 
you mind taking a look? The CMake command we're running is at the top of the 
attached log.

  CMake Error at 
/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/unittests/xpc/CMakeLists.txt:11
 (add_extra_unittest):
Unknown CMake command "add_extra_unittest".

Full Log: 
https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8914851701929934272/+/steps/clang/0/steps/configure/0/stdout


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

@MaskRay @juliehockett I'll take a look.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[clang-tools-extra] r359470 - [clangd] Fix serialization logic for Origin and Flags.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Apr 29 10:25:58 2019
New Revision: 359470

URL: http://llvm.org/viewvc/llvm-project?rev=359470&view=rev
Log:
[clangd] Fix serialization logic for Origin and Flags.

Modified:
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Serialization.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Serialization.cpp?rev=359470&r1=359469&r2=359470&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Mon Apr 29 10:25:58 
2019
@@ -314,8 +314,8 @@ Symbol readSymbol(Reader &Data, llvm::Ar
   Sym.Definition = readLocation(Data, Strings);
   Sym.CanonicalDeclaration = readLocation(Data, Strings);
   Sym.References = Data.consumeVar();
-  Sym.Flags = static_cast(Data.consumeVar());
-  Sym.Origin = static_cast(Data.consumeVar());
+  Sym.Flags = static_cast(Data.consume8());
+  Sym.Origin = static_cast(Data.consume8());
   Sym.Signature = Data.consumeString(Strings);
   Sym.CompletionSnippetSuffix = Data.consumeString(Strings);
   Sym.Documentation = Data.consumeString(Strings);

Modified: clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp?rev=359470&r1=359469&r2=359470&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SerializationTests.cpp Mon Apr 29 
10:25:58 2019
@@ -40,8 +40,8 @@ CanonicalDeclaration:
   End:
 Line: 1
 Column: 1
-Origin:4
-Flags:1
+Origin:128
+Flags:129
 Documentation:'Foo doc'
 ReturnType:'int'
 IncludeHeaders:
@@ -115,7 +115,8 @@ TEST(SerializationTest, YAMLConversions)
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), 
"file:///path/foo.h");
-  EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
+  EXPECT_EQ(Sym1.Origin, static_cast(1 << 7));
+  EXPECT_EQ(static_cast(Sym1.Flags), 129);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
   EXPECT_THAT(Sym1.IncludeHeaders,


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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Updated comment to more explicity describe motivation for new signature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61015

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -157,12 +157,16 @@
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
 if (auto Err = RangeOrErr.takeError())
   return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+auto &Range = *RangeOrErr;
+if (Range.isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range.getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())
+  return std::move(Err);
+Transformation T;
+T.Range = Range;
+T.Replacement = std::move(*ReplacementOrErr);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,21 @@
   Name,
 };
 
-using TextGenerator =
-std::function;
+// \c TextGenerator may fail, because it processes dynamically-bound match
+// results.  For example, a typo in the name of a bound node or a mismatch in
+// the node's type can lead to a failure in the string generation code.  We
+// allow the generator to return \c Expected, rather than assert on such
+// failures, so that the Transformer client can choose how to handle the error.
+// For example, if used in a UI (for example, clang-query or a web app), in
+// which the user specifies the rewrite rule, the backend might choose to 
return
+// a diagnostic error, rather than crash.
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
 
-/// Wraps a string as a TextGenerator.
+/// Wraps a string as a (trivially successful) TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+ -> Expected { return M; };
 }
 
 // Description of a source-code edit, expressed in terms of an AST node.


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -157,12 +157,16 @@
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
 if (auto Err = RangeOrErr.takeError())
   return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+auto &Range = *RangeOrErr;
+if (Range.isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range.getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())
+  return std::move(Err);
+Transformation T;
+T.Range = Range;
+T.Replacement = std::move(*ReplacementOrErr);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,21 @@
   Name,
 };
 
-using TextGenerator =
-std::function;
+// \c TextGenerator may fail, because it processes dynamically-bound match
+// results.  For example, a typo in the name of a bound node or a mismatch in
+// the node's type can lead to a failure in the string generation code.  We
+// allow the generator to return \c Expected, rather than assert on such
+// failures, so that the Transformer client can choose how to handle the error.
+// For example, if used in a UI (for example, clang-query or a web app), in
+// which the user specifies the rewrite rule, the backend might choose to return
+// a diagnostic error, rather than crash.
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
 
-/// Wraps a string as a TextGenerator.
+/// Wraps a str

[PATCH] D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 197149.
teemperor added a comment.

- Refactored test according to Gábor's feedback.


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

https://reviews.llvm.org/D59485

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -313,6 +313,17 @@
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
+public:
+  /// Allocates an ASTImporter (or one of its subclasses).
+  typedef std::function
+  ImporterConstructor;
+
+  // The lambda that constructs the ASTImporter we use in this test.
+  ImporterConstructor Creator;
+
+private:
   // Buffer for the To context, must live in the test scope.
   std::string ToCode;
 
@@ -325,22 +336,32 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+ImporterConstructor Creator;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImporterConstructor C = ImporterConstructor())
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport, LookupTable);
+};
 }
 
 void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
   assert(ToAST);
-  if (!Importer) {
-Importer.reset(
-new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
-Unit->getASTContext(), Unit->getFileManager(),
-false, &LookupTable));
-  }
+  if (!Importer)
+Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
+   Unit->getASTContext(), Unit->getFileManager(),
+   false, &LookupTable));
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
@@ -424,7 +445,7 @@
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -562,6 +583,74 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+namespace {
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+
+protected:
+  llvm::Expected ImportImpl(Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND || ND->getName() != "shouldNotBeImported")
+  return ASTImporter::ImportImpl(FromD);
+for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+  if (auto *ND = dyn_cast(D))
+if (ND->getName() == "realDecl") {
+  RegisterImportedDecl(FromD, ND);
+  return ND;
+}
+}
+return ASTImporter::ImportImpl(FromD);
+  }
+};
+
+} // namespace
+
+struct RedirectingImporterTest : ASTImporterOptionSpecificTestBase {
+  RedirectingImporterTest() {
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new RedirectingImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport,
+ LookupTable);
+};
+  }
+};
+
+// Test that an ASTImporter subclass can intercept an import call.
+TEST_P(RedirectingImporterTest, InterceptImport) {
+  Decl *From, *To;
+  std::tie(From, To) =
+  getImportedDecl("class shouldNotBeImported {};", Lang_CXX,
+  "class realDecl {};", Lang_CXX, "shouldNotBeImported");
+  auto *Imported = cast(To);
+  EXPECT_EQ(Imported->getQualifiedNameAsString(), "realDecl");
+
+  // Make sure our importer prevented t

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Patch with fix for XPC tests https://reviews.llvm.org/D61271


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187



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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D55229#1479712 , @jfb wrote:

> Looks like this might have broken bots:


I replied on Thursday, but it looks like it didn't make it to Phab:

> I thought I reverted it, did I not push that?

Looking now, it seems I did push the revert in rL359251 
. Sorry about that.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55229



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

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:176
 install(
-  DIRECTORY ${output_dir}
+  FILES ${install_files}
   DESTINATION ${header_install_dir}

vzakhari wrote:
> This is going to flatten the headers' install directory structure.  install() 
> will put the files just by their name.
> 
> I had a local fix that use install() with RENAME, and it worked.  I can try 
> to find it.  Basically, we need to keep two lists:
> 1. The same as this ${install_files}
> 2. New list install_as, which holds all ${src} values passed to 
> copy_header_to_output_dir()
> 
> Walking the two lists together we may use install(FILES 
> ${elt_from_install_files} RENAME ${elt_from_install_as} ...)
Correction.

> New list install_as, which holds all ${src} values passed to 
> copy_header_to_output_dir()

Should be
New list install_as, which holds all ${file} values passed to 
copy_header_to_output_dir()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



___
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

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 197158.

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

https://reviews.llvm.org/D60349

Files:
  include/clang/AST/DeclCXX.h
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeGen/arm64-microsoft-arguments.cpp
  test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -69,6 +69,11 @@
   int bb;
 };
 
+struct SmallWithPrivate {
+private:
+ int i;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:   (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:   i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* inalloca)
@@ -165,7 +170,7 @@
 // WIN64:   call void @"??1SmallWithDtor@@QEAA@XZ"
 // WIN64: }
 // WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i64 %s.coerce) {{.*}} {
-// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"
+// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA64: }
 
 // FIXME: MSVC incompatible!
@@ -173,6 +178,12 @@
 // WOA:   call arm_aapcs_vfpcc void @"??1SmallWithDtor@@QAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA: }
 
+
+// Test that the eligible non-aggregate is passed directly, but returned
+// indirectly on ARM64 Windows.
+// WOA64: define dso_local void @"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate* inreg noalias sret %agg.result, i64 %s.coerce) {{.*}} {
+SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }
Index: test/CodeGen/arm64-microsoft-arguments.cpp
===
--- test/CodeGen/arm64-microsoft-arguments.cpp
+++ test/CodeGen/arm64-microsoft-arguments.cpp
@@ -1,25 +1,88 @@
 // RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
 // RUN: -x c++ -o - %s | FileCheck %s
 
-struct pod { int a, b, c, d, e; };
+// Pass and return for type size <= 8 bytes.
+// CHECK: define {{.*}} i64 @{{.*}}f1{{.*}}()
+// CHECK: call i64 {{.*}}func1{{.*}}(i64 %3)
+struct S1 {
+  int a[2];
+};
+
+S1 func1(S1 x);
+S1 f1() {
+  S1 x;
+  return func1(x);
+}
 
-struct non_pod {
-  int a;
-  non_pod() {}
+// Pass and return type size <= 16 bytes.
+// CHECK: define {{.*}} [2 x i64] @{{.*}}f2{{.*}}()
+// CHECK: call [2 x i64] {{.*}}func2{{.*}}([2 x i64] %3)
+struct S2 {
+  int a[4];
 };
 
-struct pod s;
-struct non_pod t;
+S2 func2(S2 x);
+S2 f2() {
+  S2 x;
+  return func2(x);
+}
+
+// Pass and return for type size > 16 bytes.
+// CHECK: define {{.*}} void @{{.*}}f3{{.*}}(%struct.S3* noalias sret %agg.result)
+// CHECK: call void {{.*}}func3{{.*}}(%struct.S3* sret %agg.result, %struct.S3* %agg.tmp)
+struct S3 {
+  int a[5];
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x;
+  return func3(x);
+}
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Passed directly but returned indirectly.
+// CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret %agg.result, [2 x i64] %4)
+struct S4 {
+  int a[3];
+  ~S4();
+};
 
-struct pod bar() { return s; }
-struct non_pod foo() { return t; }
-// CHECK: define {{.*}} void @{{.*}}bar{{.*}}(%struct.pod* noalias sret %agg.result)
-// CHECK: define {{.*}} void @{{.*}}foo{{.*}}(%struct.non_pod* noalias %agg.result)
+S4 func4(S4 x);
+S4 f4() {
+  S4 x;
+  return func4(x);
+}
 
+// Pass and return from instance method called from instance method.
+// CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* %this, %class.P1* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* %ref.tmp, %class.P1* inreg sret %agg.result, i8 %0)
 
-// Check instance methods.
-struct pod2 { int x; };
-struct Baz { pod2 baz(); };
+class P1 {
+public:
+  P1 foo(P1 x);
+};
+
+class Q1 {
+public:
+  P1 bar();
+};
+
+P1 Q1::bar() {
+  P1 p1;
+  return P1().foo(p1);
+}
+
+// Pass and return from instance method called from free function.
+// CHECK: define {{.*}} void {{.*}}bar{{.*}}()
+// CHECK: call void {{.*}}foo@P2{{.*}}(%class.P2* %ref.tmp, %class.P2* inreg sret %retval, i8 %0)
+class P2 {
+public:
+  P2 foo(P2 x);
+};
 
-int qux() { return Baz().baz().x; }
-// CHECK: declare {{.*}} void @{{.*}}baz@Baz{{.*}}(%struct.Baz*, %struct.pod2*)
+P2 bar() {
+  P2 p2;
+  return P2().foo(p2);
+}
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5952,8 +5952,11 @@
 
 // Note: This permits small classes with nontrivial destructors to be
 // passed in re

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

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Thanks @richard.townsend.arm . I have updated the patch.


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

[PATCH] D61051: [analyzer] Treat functions without runtime branches as "small".

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D61051#1481976 , @Charusso wrote:

> Great patch! There is only a design problem:
>  You have negated every `isSmall()` condition looks like you could write 
> `isLarge()` instead but there is no connection between these functions.
>  Could you rename or redesign them? The following would be cool: `isLarge()` 
> iff `!isSmalll` and `isSmall()` iff `!isLarge()`.


We essentially classify functions into small/medium/large/huge. In this sense 
`!isSmall()` may mean medium or large or huge, while `isLarge()` may mean large 
or huge.
I guess i'll comment this up.


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

https://reviews.llvm.org/D61051



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: lib/Format/FormatTokenLexer.cpp:249
+  Identifier->ColumnWidth += Question->ColumnWidth;
+  Identifier->Type = Identifier->Type;
+  Tokens.erase(Tokens.end() - 1);

@MyDeveloperDay Should this be
```
Identifier->Type = Question->Type;
```
Reported in https://www.viva64.com/en/b/0629/


Repository:
  rC Clang

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

https://reviews.llvm.org/D58404



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

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[Scott Linder via Phabricator via cfe-commits]

scott.linder created this revision.
scott.linder added reviewers: Anastasia, tra, yaxunl, rjmccall.
Herald added subscribers: cfe-commits, tpr.
Herald added a project: clang.

For AMDGPU the visibility of these symbols (OpenCL kernels, CUDA `__global__` 
functions, and CUDA `__device__` variables) must not be hidden, as we rely on 
them being available in the dynamic symbol table in the final DSO.

This patch implements this by considering language attributes as a source of 
explicit visibility, but rather than attributing any one visibility to them 
they are simply coerced to be a non-hidden visibility. This allows for the 
optimization of using protected visibility when these symbols are known to be 
`dso_local`.

This patch also adds diagnostics for explicitly setting a hidden visibility on 
these symbols.

I imagine there are a number of issues with the patch in its current state, but 
I wanted to get something implemented before reaching out to OpenCL/CUDA 
maintainers to see if this is a reasonable change. @Anastasia and @tra I wasn't 
certain if you would be good candidates to discuss this change, so please let 
me know if I need to keep looking.


Repository:
  rC Clang

https://reviews.llvm.org/D61274

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/Visibility.h
  lib/AST/Decl.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCUDA/visibility-diagnostics.cu
  test/SemaOpenCL/visibility-diagnostics.cl

Index: test/SemaOpenCL/visibility-diagnostics.cl
===
--- /dev/null
+++ test/SemaOpenCL/visibility-diagnostics.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=cl2.0 -verify -pedantic -fsyntax-only %s
+
+__attribute__((visibility("hidden"))) kernel void kern_hidden() {} // expected-warning {{'hidden' visibility on function with incompatible language attribute will be ignored}}
+__attribute__((visibility("protected"))) kernel void kern_protected();
+__attribute__((visibility("default"))) kernel void kern_default();
+kernel void kern();
+
+__attribute__((visibility("hidden"))) extern kernel void ext_kern_hidden(); // expected-warning {{'hidden' visibility on function with incompatible language attribute will be ignored}}
+__attribute__((visibility("protected"))) extern kernel void ext_kern_protected();
+__attribute__((visibility("default"))) extern kernel void ext_kern_default();
+extern kernel void ext_kern();
Index: test/SemaCUDA/visibility-diagnostics.cu
===
--- /dev/null
+++ test/SemaCUDA/visibility-diagnostics.cu
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+#include "Inputs/cuda.h"
+
+__attribute__((visibility("hidden"))) __global__ void global_func_hidden() {} // expected-warning {{'hidden' visibility on function with incompatible language attribute will be ignored}}
+__attribute__((visibility("protected"))) __global__ void global_func_protected() {}
+__attribute__((visibility("default"))) __global__ void global_func_default() {}
+__global__ void global_func() {}
+
+__attribute__((visibility("hidden"))) __device__ int device_var_hidden; // expected-warning {{'hidden' visibility on variable with incompatible language attribute will be ignored}}
+__attribute__((visibility("protected"))) __device__ int device_var_protected;
+__attribute__((visibility("default"))) __device__ int device_var_default;
+__device__ int device_var;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7375,6 +7375,16 @@
 Diag(D->getLocation(), diag::err_designated_init_attr_non_init);
 D->dropAttr();
   }
+
+  if ((D->hasAttr() &&
+   D->getAttr()->getVisibility() ==
+   VisibilityAttr::Hidden) &&
+  (D->hasAttr() ||
+   (isa(D) && D->hasAttr()) ||
+   (isa(D) && D->hasAttr( {
+Diag(D->getLocation(), diag::warn_attribute_hidden_visibility)
+<< (isa(D) ? 0 : 1);
+  }
 }
 
 // Helper for delayed processing TransparentUnion attribute.
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7840,23 +7840,8 @@
 };
 }
 
-static bool requiresAMDGPUProtectedVisibility(const Decl *D,
-  llvm::GlobalValue *GV) {
-  if (GV->getVisibility() != llvm::GlobalValue::HiddenVisibility)
-return false;
-
-  return D->hasAttr() ||
- (isa(D) && D->hasAttr()) ||
- (isa(D) && D->hasAttr());
-}
-
 void AMDGPUTargetCodeGenInfo::setTargetAttributes(
 const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
-  if (requiresAMDGPUProtectedVisibility(D, GV)) {
-GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
-GV->setDSOLocal(true);
-  }
-
   if (GV->isDeclaration())
 return;
   const F

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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'd like to see a few more tests to cover the interesting cases that came up 
during development of this patch:

1. The user-provided constructor issue: there should be testcases with an 
explicit user-provided constructor, a non-trivial constructor that's explicitly 
defaulted, and a non-trivial constructor that's implicitly defaulted.
2. The difference between a user-defined and a non-trivial destructor: there 
should be a testcase with an implicit non-trivial destructor (which is 
non-trivial because a member has a non-trivial destructor).
3. A testcase with a trivial copy constructor, but a non-trivial 
copy-assignment operator.
4. A testcase with a struct with a base class.


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

[clang-tools-extra] r359489 - [clangd][xpc] Fix XPC unittests

[Jan Korous via cfe-commits]

Author: jkorous
Date: Mon Apr 29 12:41:30 2019
New Revision: 359489

URL: http://llvm.org/viewvc/llvm-project?rev=359489&view=rev
Log:
[clangd][xpc] Fix XPC unittests

Fix build after recent changes in clangd tests & add xpc unittests to
check-clangd target.

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

Modified:
clang-tools-extra/trunk/clangd/test/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/xpc/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/CMakeLists.txt?rev=359489&r1=359488&r2=359489&view=diff
==
--- clang-tools-extra/trunk/clangd/test/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/test/CMakeLists.txt Mon Apr 29 12:41:30 2019
@@ -8,6 +8,7 @@ set(CLANGD_TEST_DEPS
 
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_TEST_DEPS clangd-xpc-test-client)
+  list(APPEND CLANGD_TEST_DEPS ClangdXpcUnitTests)
 endif()
 
 foreach(dep FileCheck count not)

Modified: clang-tools-extra/trunk/clangd/unittests/xpc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/xpc/CMakeLists.txt?rev=359489&r1=359488&r2=359489&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/xpc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/unittests/xpc/CMakeLists.txt Mon Apr 29 
12:41:30 2019
@@ -8,7 +8,8 @@ include_directories(
   ${CLANGD_SOURCE_DIR}
   )
 
-add_extra_unittest(ClangdXpcTests
+add_custom_target(ClangdXpcUnitTests)
+add_unittest(ClangdXpcUnitTests ClangdXpcTests
   ConversionTests.cpp
   )
 


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

[PATCH] D61271: [clangd][xpc] Fix XPC unittests

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE359489: [clangd][xpc] Fix XPC unittests (authored by 
jkorous, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61271

Files:
  clangd/test/CMakeLists.txt
  clangd/unittests/xpc/CMakeLists.txt


Index: clangd/test/CMakeLists.txt
===
--- clangd/test/CMakeLists.txt
+++ clangd/test/CMakeLists.txt
@@ -8,6 +8,7 @@
 
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_TEST_DEPS clangd-xpc-test-client)
+  list(APPEND CLANGD_TEST_DEPS ClangdXpcUnitTests)
 endif()
 
 foreach(dep FileCheck count not)
Index: clangd/unittests/xpc/CMakeLists.txt
===
--- clangd/unittests/xpc/CMakeLists.txt
+++ clangd/unittests/xpc/CMakeLists.txt
@@ -8,7 +8,8 @@
   ${CLANGD_SOURCE_DIR}
   )
 
-add_extra_unittest(ClangdXpcTests
+add_custom_target(ClangdXpcUnitTests)
+add_unittest(ClangdXpcUnitTests ClangdXpcTests
   ConversionTests.cpp
   )
 


Index: clangd/test/CMakeLists.txt
===
--- clangd/test/CMakeLists.txt
+++ clangd/test/CMakeLists.txt
@@ -8,6 +8,7 @@
 
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_TEST_DEPS clangd-xpc-test-client)
+  list(APPEND CLANGD_TEST_DEPS ClangdXpcUnitTests)
 endif()
 
 foreach(dep FileCheck count not)
Index: clangd/unittests/xpc/CMakeLists.txt
===
--- clangd/unittests/xpc/CMakeLists.txt
+++ clangd/unittests/xpc/CMakeLists.txt
@@ -8,7 +8,8 @@
   ${CLANGD_SOURCE_DIR}
   )
 
-add_extra_unittest(ClangdXpcTests
+add_custom_target(ClangdXpcUnitTests)
+add_unittest(ClangdXpcUnitTests ClangdXpcTests
   ConversionTests.cpp
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI

[Eli Friedman via Phabricator via cfe-commits]

efriedma added subscribers: mgrang, rsmith.
efriedma added a comment.

I was going to ask if local variables are also supposed to be aligned this 
way... but I guess there's no way for standard C++ code to tell without 
explicitly making weird alignment assumptions, so let's not worry about that.

This should do the right thing, as far as I can tell.

It probably would make sense to refactor the arm64-specific code into a 
TargetInfo API... or at least the triple check.  Not sure what that would look 
like; maybe `virtual uint64_t getMinGlobalAlign(uint64_t TypeSize)`?  Or maybe 
should just be "bool useMicrosoftGlobalAlign()" and keep the main logic here, 
if the rule is the same for multiple Windows targets.

Needs a testcase in test/CodeGenCXX/ demonstrating the alignment of the emitted 
global in various cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61225



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

[PATCH] D60552: [X86] Enable intrinsics of AVX512_BF16, which are supported for BFLOAT16 in Cooper Lake

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

We seem to be missing test cases in test/Driver/x86-target-feature.c to test 
the command line options work. And test/Preprocessor/x86_target_features.c to 
make sure -mavx512bf16 enables avx512vl and avx512bw. Also need tests to make 
sure "-mavx512bf16 -mno-avx512bw" will leave bf16 disabled. Same for 
"-mavx512bf16 -mno-avx512vl"




Comment at: lib/Basic/Targets/X86.cpp:667
 if (Name == "avx512bw" && !Enabled)
   Features["avx512vbmi"] = Features["avx512vbmi2"] =
   Features["avx512bitalg"] = false;

Need to also disable avx512fp16 when we disable "avx512vbmi" and "avx512vbmi2". 

Need a new if to disable "avx512fp16" when "if (Name == "avx512vl" && !Enabled)"


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

https://reviews.llvm.org/D60552



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

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: klimek, sammccall, MyDeveloperDay, djasper, krasimir.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes PR41213 .


Repository:
  rC Clang

https://reviews.llvm.org/D61276

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,16 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" * 456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -380,6 +380,8 @@
   break;
 if (!Content[i].empty() && i + 1 != e && Decoration.startswith(Content[i]))
   continue;
+if (Content[i].startswith("*\t"))
+  continue;
 while (!Content[i].startswith(Decoration))
   Decoration = Decoration.substr(0, Decoration.size() - 1);
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,16 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" * 456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -380,6 +380,8 @@
   break;
 if (!Content[i].empty() && i + 1 != e && Decoration.startswith(Content[i]))
   continue;
+if (Content[i].startswith("*\t"))
+  continue;
 while (!Content[i].startswith(Decoration))
   Decoration = Decoration.substr(0, Decoration.size() - 1);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61225: [COFF, ARM64] Align global symbol by size for ARM64 MSVC ABI

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Since this is target-specific, is it right to put this here ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61225



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

So, @compnerd and I have discussed and we'd like to propose a yaml schema that 
can express what we need for ifsos for the Sections and the Symbols. Something 
like:

  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61217: Fix PCH skipping to handle all Lexers

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!


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

https://reviews.llvm.org/D61217



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

r359502 - [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Mon Apr 29 14:02:35 2019
New Revision: 359502

URL: http://llvm.org/viewvc/llvm-project?rev=359502&view=rev
Log:
[ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

Summary:
We are currently implementing support in LLDB that reconstructs the STL 
templates from
the target program in the expression evaluator. This reconstruction happens 
during the
import process from our debug info AST into the expression evaluation AST, 
which means
we need a way to intercept the ASTImporter import process.

This patch adds an protected ImportImpl method that we can overwrite in LLDB to 
implement
our special importing logic (which is essentially just looking into a C++ 
module that is attached to
the target context). Because ImportImpl has to call MapImported/AddToLookup for 
the decls it
creates, this patch also exposes those via a new unified method and checks that 
we call it when
importing decls.

Reviewers: martong, balazske, a.sidorin, shafik, a_sidorin

Reviewed By: martong, a_sidorin

Subscribers: rnkovacs, cfe-commits, lldb-commits, aprantl

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=359502&r1=359501&r2=359502&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Mon Apr 29 14:02:35 2019
@@ -142,6 +142,12 @@ class TypeSourceInfo;
 
 void AddToLookupTable(Decl *ToD);
 
+  protected:
+/// Can be overwritten by subclasses to implement their own import logic.
+/// The overwritten method should call this method if it didn't import the
+/// decl on its own.
+virtual Expected ImportImpl(Decl *From);
+
   public:
 
 /// \param ToContext The context we'll be importing into.
@@ -427,6 +433,8 @@ class TypeSourceInfo;
 /// \c To declaration mappings as they are imported.
 virtual void Imported(Decl *From, Decl *To) {}
 
+void RegisterImportedDecl(Decl *FromD, Decl *ToD);
+
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);
 

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=359502&r1=359501&r2=359502&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Apr 29 14:02:35 2019
@@ -255,8 +255,7 @@ namespace clang {
 return true; // Already imported.
   ToD = CreateFun(std::forward(args)...);
   // Keep track of imported Decls.
-  Importer.MapImported(FromD, ToD);
-  Importer.AddToLookupTable(ToD);
+  Importer.RegisterImportedDecl(FromD, ToD);
   InitializeImportedDecl(FromD, ToD);
   return false; // A new Decl is created.
 }
@@ -7656,6 +7655,17 @@ void ASTImporter::AddToLookupTable(Decl
   LookupTable->add(ToND);
 }
 
+Expected ASTImporter::ImportImpl(Decl *FromD) {
+  // Import the decl using ASTNodeImporter.
+  ASTNodeImporter Importer(*this);
+  return Importer.Visit(FromD);
+}
+
+void ASTImporter::RegisterImportedDecl(Decl *FromD, Decl *ToD) {
+  MapImported(FromD, ToD);
+  AddToLookupTable(ToD);
+}
+
 Expected ASTImporter::Import_New(QualType FromT) {
   if (FromT.isNull())
 return QualType{};
@@ -7749,7 +7759,6 @@ Expected ASTImporter::Import_New
   if (!FromD)
 return nullptr;
 
-  ASTNodeImporter Importer(*this);
 
   // Check whether we've already imported this declaration.
   Decl *ToD = GetAlreadyImportedOrNull(FromD);
@@ -7760,7 +7769,7 @@ Expected ASTImporter::Import_New
   }
 
   // Import the declaration.
-  ExpectedDecl ToDOrErr = Importer.Visit(FromD);
+  ExpectedDecl ToDOrErr = ImportImpl(FromD);
   if (!ToDOrErr)
 return ToDOrErr;
   ToD = *ToDOrErr;
@@ -7771,6 +7780,9 @@ Expected ASTImporter::Import_New
 return nullptr;
   }
 
+  // Make sure that ImportImpl registered the imported decl.
+  assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
+
   // Once the decl is connected to the existing declarations, i.e. when the
   // redecl chain is properly set then we populate the lookup again.
   // This way the primary context will be able to find all decls.

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=359502&r1=359501&r2=359502&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Apr 29 14:02:35 2019
@

[PATCH] D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

I'll land this as it seems only the tests are subject to change and I want to 
also land the dependencies of this patch. Please let me know if you want any 
other changes changes to the test and thanks for the review!


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

https://reviews.llvm.org/D59485



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

[PATCH] D61175: [MinGW] Don't let template instantiation declarations cover nested classes

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2687-2689
   if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment()) 
&&
+   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
+   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&

I think this can be simplified to "if Windows" at this point. But, I'm confused 
why we need this change to the general template instantiation machinery... 
Anyway, I'll send the simplification as a code review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61175



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

[PATCH] D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359502: [ASTImporter] Add an ImportImpl method to allow 
customizing Import behavior. (authored by teemperor, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59485?vs=197149&id=197180#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59485

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -142,6 +142,12 @@
 
 void AddToLookupTable(Decl *ToD);
 
+  protected:
+/// Can be overwritten by subclasses to implement their own import logic.
+/// The overwritten method should call this method if it didn't import the
+/// decl on its own.
+virtual Expected ImportImpl(Decl *From);
+
   public:
 
 /// \param ToContext The context we'll be importing into.
@@ -427,6 +433,8 @@
 /// \c To declaration mappings as they are imported.
 virtual void Imported(Decl *From, Decl *To) {}
 
+void RegisterImportedDecl(Decl *FromD, Decl *ToD);
+
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);
 
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -255,8 +255,7 @@
 return true; // Already imported.
   ToD = CreateFun(std::forward(args)...);
   // Keep track of imported Decls.
-  Importer.MapImported(FromD, ToD);
-  Importer.AddToLookupTable(ToD);
+  Importer.RegisterImportedDecl(FromD, ToD);
   InitializeImportedDecl(FromD, ToD);
   return false; // A new Decl is created.
 }
@@ -7656,6 +7655,17 @@
   LookupTable->add(ToND);
 }
 
+Expected ASTImporter::ImportImpl(Decl *FromD) {
+  // Import the decl using ASTNodeImporter.
+  ASTNodeImporter Importer(*this);
+  return Importer.Visit(FromD);
+}
+
+void ASTImporter::RegisterImportedDecl(Decl *FromD, Decl *ToD) {
+  MapImported(FromD, ToD);
+  AddToLookupTable(ToD);
+}
+
 Expected ASTImporter::Import_New(QualType FromT) {
   if (FromT.isNull())
 return QualType{};
@@ -7749,7 +7759,6 @@
   if (!FromD)
 return nullptr;
 
-  ASTNodeImporter Importer(*this);
 
   // Check whether we've already imported this declaration.
   Decl *ToD = GetAlreadyImportedOrNull(FromD);
@@ -7760,7 +7769,7 @@
   }
 
   // Import the declaration.
-  ExpectedDecl ToDOrErr = Importer.Visit(FromD);
+  ExpectedDecl ToDOrErr = ImportImpl(FromD);
   if (!ToDOrErr)
 return ToDOrErr;
   ToD = *ToDOrErr;
@@ -7771,6 +7780,9 @@
 return nullptr;
   }
 
+  // Make sure that ImportImpl registered the imported decl.
+  assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
+
   // Once the decl is connected to the existing declarations, i.e. when the
   // redecl chain is properly set then we populate the lookup again.
   // This way the primary context will be able to find all decls.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -313,6 +313,17 @@
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
+public:
+  /// Allocates an ASTImporter (or one of its subclasses).
+  typedef std::function
+  ImporterConstructor;
+
+  // The lambda that constructs the ASTImporter we use in this test.
+  ImporterConstructor Creator;
+
+private:
   // Buffer for the To context, must live in the test scope.
   std::string ToCode;
 
@@ -325,22 +336,32 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+ImporterConstructor Creator;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImporterConstructor C = ImporterConstructor())
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+   

[PATCH] D61278: Simplify exclusion of nested classes from extern template instantiation, NFC

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: hans, mstorsjo.
Herald added a project: clang.

This simplifies three checks for MS ABI, Win Itanium, or Win GNU to just
"is Windows".

The question remains, however, if this is really the correct thing to
do. We could, for example, only not consider inner classes to be
externally available if the outer class has a dllexport annotation.
However, I will leave that as future work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61278

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp


Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2684,15 +2684,14 @@
 == TSK_ExplicitSpecialization)
 continue;
 
-  if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
-   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
+  if (Context.getTargetInfo().getTriple().isOSWindows() &&
   TSK == TSK_ExplicitInstantiationDeclaration) {
-// In MSVC and Windows Itanium mode, explicit instantiation decl of the
-// outer class doesn't affect the inner class.
-// In GNU mode, inner classes aren't dllexported. Don't let the
-// instantiation cover the inner class, to avoid undefined references
-// to inner classes that weren't exported.
+// On Windows, explicit instantiation decl of the outer class doesn't
+// affect the inner class. Typically extern template declarations are
+// used in combination with dll import/export annotations, but those
+// are not propagated from the outer class templates to inner classes.
+// Therefore, do not instantiate inner classes on this platform, so
+// that users don't end up with undefined symbols during linking.
 continue;
   }
 


Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2684,15 +2684,14 @@
 == TSK_ExplicitSpecialization)
 continue;
 
-  if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
-   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
+  if (Context.getTargetInfo().getTriple().isOSWindows() &&
   TSK == TSK_ExplicitInstantiationDeclaration) {
-// In MSVC and Windows Itanium mode, explicit instantiation decl of the
-// outer class doesn't affect the inner class.
-// In GNU mode, inner classes aren't dllexported. Don't let the
-// instantiation cover the inner class, to avoid undefined references
-// to inner classes that weren't exported.
+// On Windows, explicit instantiation decl of the outer class doesn't
+// affect the inner class. Typically extern template declarations are
+// used in combination with dll import/export annotations, but those
+// are not propagated from the outer class templates to inner classes.
+// Therefore, do not instantiate inner classes on this platform, so
+// that users don't end up with undefined symbols during linking.
 continue;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61175: [MinGW] Don't let template instantiation declarations cover nested classes

[Martin Storsjö via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2687-2689
   if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment()) 
&&
+   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
+   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&

rnk wrote:
> I think this can be simplified to "if Windows" at this point. But, I'm 
> confused why we need this change to the general template instantiation 
> machinery... Anyway, I'll send the simplification as a code review.
Yes, I believe so. Not sure about why this is needed though.

In case GCC actually does make a move on this matter, and chooses a different 
path for fixing it, we might want to follow that so in that case we might need 
to reintroduce some condition like this. But until that hypothetical case, 
let's simplify it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61175



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

[PATCH] D61278: Simplify exclusion of nested classes from extern template instantiation, NFC

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo accepted this revision.
mstorsjo 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/D61278/new/

https://reviews.llvm.org/D61278



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

[PATCH] D61281: [clang-format] Fixed self assignment

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

xbolva00 created this revision.
xbolva00 added a reviewer: MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D61281

Files:
  lib/Format/FormatTokenLexer.cpp


Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -246,7 +246,7 @@
   StringRef(Identifier->TokenText.begin(),
 Question->TokenText.end() - Identifier->TokenText.begin());
   Identifier->ColumnWidth += Question->ColumnWidth;
-  Identifier->Type = Identifier->Type;
+  Identifier->Type = Question->Type;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }


Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -246,7 +246,7 @@
   StringRef(Identifier->TokenText.begin(),
 Question->TokenText.end() - Identifier->TokenText.begin());
   Identifier->ColumnWidth += Question->ColumnWidth;
-  Identifier->Type = Identifier->Type;
+  Identifier->Type = Question->Type;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: glider, pcc, kcc.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

C guarantees that brace-init with fewer initializers than members in the
aggregate will initialize the rest of the aggregate as-if it were static
initialization. In turn static initialization guarantees that padding is
initialized to zero bits.

rdar://problem/50188861


Repository:
  rC Clang

https://reviews.llvm.org/D61280

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/padding-init.c


Index: test/CodeGen/padding-init.c
===
--- /dev/null
+++ test/CodeGen/padding-init.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown 
-ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero 
%s -emit-llvm -o - | FileCheck %s
+
+// C guarantees that brace-init with fewer initializers than members in the
+// aggregate will initialize the rest of the aggregate as-if it were static
+// initialization. In turn static initialization guarantees that padding is
+// initialized to zero bits.
+
+// CHECK: @__const.partial_init.s = private unnamed_addr constant { i8, [7 x 
i8], i64 } { i8 42, [7 x i8] zeroinitializer, i64 0 }, align 8
+
+// Technically, we could initialize this padding to non-zero because all of the
+// struct's members have initializers.
+
+// CHECK: @__const.init_all.s = private unnamed_addr constant { i8, [7 x i8], 
i64 } { i8 42, [7 x i8] zeroinitializer, i64 -2401053089374216531 }, align 8
+
+struct S {
+  char c;
+  long long l;
+};
+
+void use(struct S*);
+
+// CHECK-LABEL: @empty_braces(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memset{{.*}}(i8* align 8 %[[B]], i8 0,
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void empty_braces() {
+  struct S s = {};
+  return use(&s);
+}
+
+// CHECK-LABEL: @partial_init(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memcpy{{.*}}(i8* align 8 %[[B]], 
{{.*}}@__const.partial_init.s
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void partial_init() {
+  struct S s = { .c = 42 };
+  return use(&s);
+}
+
+// CHECK-LABEL: @init_all(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memcpy{{.*}}(i8* align 8 %[[B]], 
{{.*}}@__const.init_all.s
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void init_all() {
+  struct S s = { .c = 42, .l = 0xdeadbeefc0fedead };
+  return use(&s);
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1787,13 +1787,20 @@
   D.isUsableInConstantExpressions(getContext())) {
 assert(!capturedByInit && "constant init contains a capturing block?");
 constant = ConstantEmitter(*this).tryEmitAbstractForInitializer(D);
-if (constant && trivialAutoVarInit !=
-LangOptions::TrivialAutoVarInitKind::Uninitialized) {
+if (constant && !constant->isZeroValue() &&
+(trivialAutoVarInit !=
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)) {
   IsPattern isPattern =
   (trivialAutoVarInit == LangOptions::TrivialAutoVarInitKind::Pattern)
   ? IsPattern::Yes
   : IsPattern::No;
-  constant = constWithPadding(CGM, isPattern,
+  // C guarantees that brace-init with fewer initializers than members in
+  // the aggregate will initialize the rest of the aggregate as-if it were
+  // static initialization. In turn static initialization guarantees that
+  // padding is initialized to zero bits. We could instead pattern-init if 
D
+  // has any ImplicitValueInitExpr, but that seems to be unintuitive
+  // behavior.
+  constant = constWithPadding(CGM, IsPattern::No,
   replaceUndef(CGM, isPattern, constant));
 }
   }


Index: test/CodeGen/padding-init.c
===
--- /dev/null
+++ test/CodeGen/padding-init.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s
+
+// C guarantees that brace-init with fewer initializers than members in the
+// aggregate will initialize the rest of the aggregate as-if it were static
+// initialization. In turn static initialization guarantees that padding is
+// initialized to zero bits.
+
+// CHECK: @__const.partial_init.s = private unnamed_addr constant { i8, [7 x i8], i64 } { i8 42, [7 x i8] zeroinitializer, i64 0 }, align 8
+
+// Technically, we could initialize this pa

r359506 - When skipping code at the start of a file during PCH use, Preprocessor::Lex

[Mike Rice via cfe-commits]

Author: mikerice
Date: Mon Apr 29 14:21:17 2019
New Revision: 359506

URL: http://llvm.org/viewvc/llvm-project?rev=359506&view=rev
Log:
When skipping code at the start of a file during PCH use, Preprocessor::Lex
is not used since it consumes all preprocessor directives until it returns
a real token. Using the specific Lexer (i.e. CurLexer->Lex) makes it
possible to stop skipping after an #include or #pragma hdrstop. Previously
the skipping code was only handling CurLexer, now all will be handled
correctly.

Fixes: llvm.org/PR41585

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

Added:
cfe/trunk/test/PCH/Inputs/pch-through-macro.h   (with props)
cfe/trunk/test/PCH/pch-through4.cpp   (with props)
cfe/trunk/test/PCH/pch-through4a.cpp   (with props)
Modified:
cfe/trunk/lib/Lex/Preprocessor.cpp

Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=359506&r1=359505&r2=359506&view=diff
==
--- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
+++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Apr 29 14:21:17 2019
@@ -625,8 +625,23 @@ void Preprocessor::SkipTokensWhileUsingP
   bool UsingPragmaHdrStop = SkippingUntilPragmaHdrStop;
   Token Tok;
   while (true) {
-bool InPredefines = (CurLexer->getFileID() == getPredefinesFileID());
-CurLexer->Lex(Tok);
+bool InPredefines =
+(CurLexer && CurLexer->getFileID() == getPredefinesFileID());
+switch (CurLexerKind) {
+case CLK_Lexer:
+  CurLexer->Lex(Tok);
+ break;
+case CLK_TokenLexer:
+  CurTokenLexer->Lex(Tok);
+  break;
+case CLK_CachingLexer:
+  bool IsNewToken;
+  CachingLex(Tok, IsNewToken);
+  break;
+case CLK_LexAfterModuleImport:
+  LexAfterModuleImport(Tok);
+  break;
+}
 if (Tok.is(tok::eof) && !InPredefines) {
   ReachedMainFileEOF = true;
   break;

Added: cfe/trunk/test/PCH/Inputs/pch-through-macro.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/Inputs/pch-through-macro.h?rev=359506&view=auto
==
--- cfe/trunk/test/PCH/Inputs/pch-through-macro.h (added)
+++ cfe/trunk/test/PCH/Inputs/pch-through-macro.h Mon Apr 29 14:21:17 2019
@@ -0,0 +1,3 @@
+#pragma once
+#define Source(x,y)
+#define InOut(size) Source(InOut, (size))

Propchange: cfe/trunk/test/PCH/Inputs/pch-through-macro.h
--
svn:eol-style = native

Propchange: cfe/trunk/test/PCH/Inputs/pch-through-macro.h
--
svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/PCH/Inputs/pch-through-macro.h
--
svn:mime-type = text/plain

Added: cfe/trunk/test/PCH/pch-through4.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/pch-through4.cpp?rev=359506&view=auto
==
--- cfe/trunk/test/PCH/pch-through4.cpp (added)
+++ cfe/trunk/test/PCH/pch-through4.cpp Mon Apr 29 14:21:17 2019
@@ -0,0 +1,12 @@
+// expected-no-diagnostics
+// Create PCH with #pragma hdrstop processing.
+// RUN: %clang_cc1 -verify -I %S -emit-pch -pch-through-hdrstop-create \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-hdrstop-create -fms-extensions -o %t.obj -x c++ %s
+
+#pragma once
+#include "Inputs/pch-through-macro.h"
+void f(InOut(a) char *b, unsigned long a);

Propchange: cfe/trunk/test/PCH/pch-through4.cpp
--
svn:eol-style = native

Propchange: cfe/trunk/test/PCH/pch-through4.cpp
--
svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/PCH/pch-through4.cpp
--
svn:mime-type = text/plain

Added: cfe/trunk/test/PCH/pch-through4a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/pch-through4a.cpp?rev=359506&view=auto
==
--- cfe/trunk/test/PCH/pch-through4a.cpp (added)
+++ cfe/trunk/test/PCH/pch-through4a.cpp Mon Apr 29 14:21:17 2019
@@ -0,0 +1,16 @@
+// expected-no-diagnostics
+// Create PCH with a through header.
+// RUN: %clang_cc1 -verify -I %S -emit-pch \
+// RUN: -pch-through-header=Inputs/pch-through1.h \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-header=Inputs/pch-through1.h \
+// R

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

A kernel functions in CUDA is actually two different functions. One is the real 
kernel we compile for the GPU, another is a host-side stub that launches the 
device-side kernel.

On device side both clang and nvcc currently silently ignore `hidden` 
visibility and force the kernel to always be visible:
https://godbolt.org/z/xrPMGc
This is needed because the kernel must be externally visible in the device-side 
executable for the host-side code to execute it.
The device-side executable and its symbols are isolated from the host DSO they 
are encapsulated in, so whether the `hidden` attribute is ignored or not on 
device side is independent of the visibility the kernel symbol gets on the host 
side.

On the host side there's no particular reason to give kernel (or, rather, its 
host-side stub) a special treatment. Setting `hidden` on it should be fine, if 
someone needs it for whatever reason.
Most likely users who may have applied `hidden` to a kernel would do so in 
order to avoid exposing kernel symbols outside of a DSO and the attribute will 
do that job just fine.

I think the warning is not going to buy anything for CUDA. The hidden attribute 
effectively applies to the host side only, where it should work correctly and 
where it is potentially useful. I'd rather not impose restrictions that are not 
necessary, even if it's just a warning.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61274



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

[PATCH] D61217: Fix PCH skipping to handle all Lexers

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359506: When skipping code at the start of a file during PCH 
use, Preprocessor::Lex (authored by mikerice, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D61217

Files:
  lib/Lex/Preprocessor.cpp
  test/PCH/Inputs/pch-through-macro.h
  test/PCH/pch-through4.cpp
  test/PCH/pch-through4a.cpp


Index: test/PCH/pch-through4a.cpp
===
--- test/PCH/pch-through4a.cpp
+++ test/PCH/pch-through4a.cpp
@@ -0,0 +1,16 @@
+// expected-no-diagnostics
+// Create PCH with a through header.
+// RUN: %clang_cc1 -verify -I %S -emit-pch \
+// RUN: -pch-through-header=Inputs/pch-through1.h \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-header=Inputs/pch-through1.h \
+// RUN:   -fms-extensions -o %t.obj -x c++ %s
+
+#define Source(x,y)
+#define InOut(size) Source(InOut, (size))
+void f(InOut(a) char *b, unsigned long a);
+#include "Inputs/pch-through1.h"
+int other;
Index: test/PCH/Inputs/pch-through-macro.h
===
--- test/PCH/Inputs/pch-through-macro.h
+++ test/PCH/Inputs/pch-through-macro.h
@@ -0,0 +1,3 @@
+#pragma once
+#define Source(x,y)
+#define InOut(size) Source(InOut, (size))
Index: test/PCH/pch-through4.cpp
===
--- test/PCH/pch-through4.cpp
+++ test/PCH/pch-through4.cpp
@@ -0,0 +1,12 @@
+// expected-no-diagnostics
+// Create PCH with #pragma hdrstop processing.
+// RUN: %clang_cc1 -verify -I %S -emit-pch -pch-through-hdrstop-create \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-hdrstop-create -fms-extensions -o %t.obj -x c++ %s
+
+#pragma once
+#include "Inputs/pch-through-macro.h"
+void f(InOut(a) char *b, unsigned long a);
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -625,8 +625,23 @@
   bool UsingPragmaHdrStop = SkippingUntilPragmaHdrStop;
   Token Tok;
   while (true) {
-bool InPredefines = (CurLexer->getFileID() == getPredefinesFileID());
-CurLexer->Lex(Tok);
+bool InPredefines =
+(CurLexer && CurLexer->getFileID() == getPredefinesFileID());
+switch (CurLexerKind) {
+case CLK_Lexer:
+  CurLexer->Lex(Tok);
+ break;
+case CLK_TokenLexer:
+  CurTokenLexer->Lex(Tok);
+  break;
+case CLK_CachingLexer:
+  bool IsNewToken;
+  CachingLex(Tok, IsNewToken);
+  break;
+case CLK_LexAfterModuleImport:
+  LexAfterModuleImport(Tok);
+  break;
+}
 if (Tok.is(tok::eof) && !InPredefines) {
   ReachedMainFileEOF = true;
   break;


Index: test/PCH/pch-through4a.cpp
===
--- test/PCH/pch-through4a.cpp
+++ test/PCH/pch-through4a.cpp
@@ -0,0 +1,16 @@
+// expected-no-diagnostics
+// Create PCH with a through header.
+// RUN: %clang_cc1 -verify -I %S -emit-pch \
+// RUN: -pch-through-header=Inputs/pch-through1.h \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-header=Inputs/pch-through1.h \
+// RUN:   -fms-extensions -o %t.obj -x c++ %s
+
+#define Source(x,y)
+#define InOut(size) Source(InOut, (size))
+void f(InOut(a) char *b, unsigned long a);
+#include "Inputs/pch-through1.h"
+int other;
Index: test/PCH/Inputs/pch-through-macro.h
===
--- test/PCH/Inputs/pch-through-macro.h
+++ test/PCH/Inputs/pch-through-macro.h
@@ -0,0 +1,3 @@
+#pragma once
+#define Source(x,y)
+#define InOut(size) Source(InOut, (size))
Index: test/PCH/pch-through4.cpp
===
--- test/PCH/pch-through4.cpp
+++ test/PCH/pch-through4.cpp
@@ -0,0 +1,12 @@
+// expected-no-diagnostics
+// Create PCH with #pragma hdrstop processing.
+// RUN: %clang_cc1 -verify -I %S -emit-pch -pch-through-hdrstop-create \
+// RUN:   -fms-extensions -o %t.pch -x c++-header %s
+
+// Create the PCH object
+// RUN: %clang_cc1 -verify -I %S -emit-obj -include-pch %t.pch \
+// RUN:   -pch-through-hdrstop-create -fms-extensions -o %t.obj -x c++ %s
+
+#pragma once
+#include "Inputs/pch-through-macro.h"
+void f(InOut(a) char *b, unsigned long a);
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -625,8 +625,23 @@
   bool UsingPragmaHdrStop =

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> xbolva00 abandoned this revision.

Do you not want to pursue this any more?

This seems reasonable to me, and is in line with other cases where we have 
diagnostic flags as aliases to GCC's similar-but-not-quite-the-same flags (eg, 
GCC's `-Wnoexcept-type` doesn't fire in quite the same set of cases as clang's 
`-Wc++17-compat-mangling`, but we alias them; likewise for GCC 
`-Wsequence-point` versus Clang `-Wunsequenced`).


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

https://reviews.llvm.org/D58841



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

r359507 - Simplify exclusion of nested classes from extern template instantiation, NFC

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Apr 29 14:32:05 2019
New Revision: 359507

URL: http://llvm.org/viewvc/llvm-project?rev=359507&view=rev
Log:
Simplify exclusion of nested classes from extern template instantiation, NFC

Summary:
This simplifies three checks for MS ABI, Win Itanium, or Win GNU to just
"is Windows".

The question remains, however, if this is really the correct thing to
do. We could, for example, only not consider inner classes to be
externally available if the outer class has a dllexport annotation.
However, I will leave that as future work.

Reviewers: hans, mstorsjo

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=359507&r1=359506&r2=359507&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Mon Apr 29 14:32:05 2019
@@ -2684,15 +2684,14 @@ Sema::InstantiateClassMembers(SourceLoca
 == TSK_ExplicitSpecialization)
 continue;
 
-  if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
-   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
+  if (Context.getTargetInfo().getTriple().isOSWindows() &&
   TSK == TSK_ExplicitInstantiationDeclaration) {
-// In MSVC and Windows Itanium mode, explicit instantiation decl of the
-// outer class doesn't affect the inner class.
-// In GNU mode, inner classes aren't dllexported. Don't let the
-// instantiation cover the inner class, to avoid undefined references
-// to inner classes that weren't exported.
+// On Windows, explicit instantiation decl of the outer class doesn't
+// affect the inner class. Typically extern template declarations are
+// used in combination with dll import/export annotations, but those
+// are not propagated from the outer class templates to inner classes.
+// Therefore, do not instantiate inner classes on this platform, so
+// that users don't end up with undefined symbols during linking.
 continue;
   }
 


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

[PATCH] D61278: Simplify exclusion of nested classes from extern template instantiation, NFC

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359507: Simplify exclusion of nested classes from extern 
template instantiation, NFC (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61278?vs=197182&id=197191#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61278

Files:
  cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp


Index: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2684,15 +2684,14 @@
 == TSK_ExplicitSpecialization)
 continue;
 
-  if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
-   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
+  if (Context.getTargetInfo().getTriple().isOSWindows() &&
   TSK == TSK_ExplicitInstantiationDeclaration) {
-// In MSVC and Windows Itanium mode, explicit instantiation decl of the
-// outer class doesn't affect the inner class.
-// In GNU mode, inner classes aren't dllexported. Don't let the
-// instantiation cover the inner class, to avoid undefined references
-// to inner classes that weren't exported.
+// On Windows, explicit instantiation decl of the outer class doesn't
+// affect the inner class. Typically extern template declarations are
+// used in combination with dll import/export annotations, but those
+// are not propagated from the outer class templates to inner classes.
+// Therefore, do not instantiate inner classes on this platform, so
+// that users don't end up with undefined symbols during linking.
 continue;
   }
 


Index: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2684,15 +2684,14 @@
 == TSK_ExplicitSpecialization)
 continue;
 
-  if ((Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-   Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment() ||
-   Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
+  if (Context.getTargetInfo().getTriple().isOSWindows() &&
   TSK == TSK_ExplicitInstantiationDeclaration) {
-// In MSVC and Windows Itanium mode, explicit instantiation decl of the
-// outer class doesn't affect the inner class.
-// In GNU mode, inner classes aren't dllexported. Don't let the
-// instantiation cover the inner class, to avoid undefined references
-// to inner classes that weren't exported.
+// On Windows, explicit instantiation decl of the outer class doesn't
+// affect the inner class. Typically extern template declarations are
+// used in combination with dll import/export annotations, but those
+// are not propagated from the outer class templates to inner classes.
+// Therefore, do not instantiate inner classes on this platform, so
+// that users don't end up with undefined symbols during linking.
 continue;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

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

xbolva00 added a comment.

Ok, reclaiming patch.

Thanks for  the review! If the patch is fine, please approve it.


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

https://reviews.llvm.org/D58841



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

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

In D58841#1483128 , @xbolva00 wrote:

> Thanks for  the review! If the patch is fine, please approve it.


Sure thing! (Phab doesn't permit approving a patch in "abandoned" state.)


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

https://reviews.llvm.org/D58841



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

So, @compnerd and I have discussed and we'd like to propose a yaml schema that 
can express what we need for ifsos for the Sections and the Symbols.

For the Symbols we want:

  Symbols:
- Name:_dataA
  Type:STT_OBJECT
  Section: .data
  Binding: STB_GLOBAL
- Name:__ZN3qux3barEii
  Type:STT_FUNC
  Section: .text
  Binding: STB_GLOBAL

Because this expresses the bare minimum of the symbol name, whether its a 
function or an exported object, which section it belongs to, and if it is bound 
globally or locally or weakly.

For the sections, this yaml schema gives us what we need to set the name, type 
and flags properly:

  Sections:
- Name:.text
  Type:SHT_PROGBITS
  Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
- Name:.data
  Type:SHT_PROGBITS
  Flags:   [ SHF_WRITE, SHF_ALLOC ]

We also think that the existing FileHeader works for emitting ELF ifsos:

  
FileHeader:
Class:   ELFCLASS64
Data:ELFDATA2LSB
Type:ET_REL
Machine: EM_X86_64
  


I could see us compacting this (something like "Kind: ELF_64_LSB_X86-64") but I 
don't see why we'd want to deviate from what ELF already lays out.
The file type is important because ET_REL vs ET_DYN can be used to denote if 
this is an ifso artifact that is meant to be merged or if it is already merged. 
I think we could have one stage that does the merging of the ET_RELs and one 
that assembles the final binary from a merged ET_DYN ifso.

We looked at other formats like TBD, and the major thing we also want to add is:

  --- !ifso-elf-v1

at the beginning of the yaml to denote that this is an ifso specifically for 
ELF and to denote the versioning.
I think the versioning could be used to direct how we read the FileHeader.

So the yaml ifso file syntax:

  --- !ifso-elf-
  FileHeader:
Class:   
Data:
Type:
Machine: 
  Sections: 
# One or more of these:
- Name:
  Type:
  Flags:   [  ]
  Symbols:
# One or more of these:
- Name:
  Type:
  Section: 
  Binding: 

How does this sound?

PL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Thanks for doing this! I'm glad you were able to do it without needing to 
change the traverser class. That's a good indicator.

I think it would be great if there were somewhere to document the non-stability 
of the format and content here.




Comment at: include/clang/AST/DeclBase.h:1139
+  void dump(raw_ostream &Out, bool Deserialize = false,
+ASTDumpOutputFormat OutputFormat = ADOF_Default) const;
 

I think we've talked about this before, but I don't think growing interfaces 
like this is the best way forward. An enum is a less-good replacement for an 
object (ie making the user of the API responsible for creating the dumper they 
want to use).

I think that could be made more convenient in the future. What do you think?



Comment at: include/clang/AST/JSONNodeDumper.h:53
+  template  void AddChild(Fn DoAddChild) {
+return AddChild("inner", DoAddChild);
+  }

You have 

!Label.empty() ? Label : "inner";

below. This looks needlessly non-DRY?



Comment at: include/clang/AST/JSONNodeDumper.h:130
+// JSON and then dump to a file because the AST is an ordered collection of
+// nodes but our JSON objects are modelled as an unordered container. We can
+// use JSON facilities for constructing the terminal fields in a node because

I'm no expert in JSON, so I'm not certain if this has implications.

Do entries in a JSON object (ie not an Array-type) have order? Are you 'giving' 
them order here which is not part of how JSON is usually parsed (eg by third 
party tools)?

Can you create ordered JSON (arrays) where it is needed? I see that `inner` is 
already an array, so I'm probably missing the part that is 'modelled as an 
unordered container'. Can you point that out to me?



Comment at: include/clang/AST/JSONNodeDumper.h:161
+  OS << ",\n";
+OS << Indentation << "\"" << Key << "\": {\n";
+Indentation += "  ";

Is it possible this 'textual' JSON generation could produce invalid JSON? One 
of the things a library solution ensures is balanced braces and valid syntax 
etc. I'm not familiar enough with JSON to know if it has dark corners.


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

https://reviews.llvm.org/D60910



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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I see you've ended up implementing the intrinsics with inline assembly but are 
there any reasons not to do it with builtins like in this patch? The problem 
with inline assembly is that for some Apple platforms we require developers to 
emit bitcode. And inline assembly isn't really compatible with bitcode 
abstraction. Unfortunately, I have no experience with Intel intrinsics and I 
need your help in pursuing implementing them with builtins.


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

https://reviews.llvm.org/D44387



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

[PATCH] D58321: Support for relative vtables

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197196.
leonardchan marked an inline comment as done.
leonardchan retitled this revision from "[WIP] Support for relative vtables" to 
"Support for relative vtables".
leonardchan added a comment.

Ok. Formally requesting for code reviews now.

- Made the flag an `experimental` flag.
- Added changes to relevant AST Serialization code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58321

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/VTableBuilder.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGenCXX/debug-info-virtual-fn-relative.cpp
  clang/test/CodeGenCXX/vtable-relative-abi.cpp
  llvm/lib/IR/Constants.cpp

Index: llvm/lib/IR/Constants.cpp
===
--- llvm/lib/IR/Constants.cpp
+++ llvm/lib/IR/Constants.cpp
@@ -495,6 +495,35 @@
   return false;
 }
 
+/// Returns true if this constant mathes the relative referencing for virtual
+/// functions introduced in the relative ABI. This is the difference between a
+/// function and its corresponding vtable entry.
+///
+/// i64 sub (
+///   i64 ptrtoint (i8* obj to i64),
+//i64 ptrtoint (
+//  i32* getelementptr inbounds (%object_vtable, %object_vtable*
+//@_ZTV12derived_virt, i32 0, i32 0, i32 2) to i64)
+static bool MatchesRelativeOffset(const Constant *C) {
+  const auto *CE = dyn_cast(C);
+  if (!CE || CE->getOpcode() != Instruction::Sub)
+return false;
+
+  const auto *LHS = dyn_cast(CE->getOperand(0));
+  const auto *RHS = dyn_cast(CE->getOperand(1));
+  if (!LHS || !RHS || LHS->getOpcode() != Instruction::PtrToInt ||
+  RHS->getOpcode() != Instruction::PtrToInt)
+return false;
+
+  RHS = dyn_cast(RHS->getOperand(0));
+  if (!RHS)
+return false;
+
+  return (isa(LHS->getOperand(0)) &&
+  RHS->getOpcode() == Instruction::GetElementPtr &&
+  isa(RHS->getOperand(0)));
+}
+
 bool Constant::needsRelocation() const {
   if (isa(this))
 return true; // Global reference.
@@ -519,6 +548,10 @@
 return false;
 }
 
+  // This results in an R_PLT_PC reloc which can be computed at link time.
+  if (MatchesRelativeOffset(this))
+return false;
+
   bool Result = false;
   for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
 Result |= cast(getOperand(i))->needsRelocation();
Index: clang/test/CodeGenCXX/vtable-relative-abi.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/vtable-relative-abi.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -fexperimental-relative-c++-abi-vtables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-ITANIUM %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-windows-msvc -fexperimental-relative-c++-abi-vtables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-MS %s
+
+// CHECK-ITANIUM: @_ZTV1S = unnamed_addr constant { { i8*, i8*, i32, i32 } } { { i8*, i8*, i32, i32 } { i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1S to i8*), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @_ZN1S2f1Ev to i64), i64 ptrtoint (i32* getelementptr inbounds ({ { i8*, i8*, i32, i32 } }, { { i8*, i8*, i32, i32 } }* @_ZTV1S, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @_ZN1S2f2Ev to i64), i64 ptrtoint (i32* getelementptr inbounds ({ { i8*, i8*, i32, i32 } }, { { i8*, i8*, i32, i32 } }* @_ZTV1S, i32 0, i32 0, i32 2) to i64)) to i32) } }, align 8
+// CHECK-MS: @0 = private unnamed_addr constant { { i8*, i32, i32 } } { { i8*, i32, i32 } { i8* bitcast (%rtti.CompleteObjectLocator* @"??_R4S@@6B@" to i8*), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @"?f1@S@@UEAAXXZ" to i64), i64 ptrtoint (i32* getelementptr inbounds ({ { i8*, i32, i32 } }, { { i8*, i32, i32 } }* @0, i32 0, i32 0, i32 1) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @"?f2@S@@UEAAXXZ" to i64), i64 ptrtoint (i32* getelementptr inbounds ({ { i8*, i32, i32 } }, { { i8*, i32, i32 } }* @0, i32 0, i32 0, i32 1) to i64)) to i32) } }, comdat($"??_7S@@6B@")
+
+struct S {
+  S();
+  virtual void f1();
+  virtual void f2();
+};
+
+// CHECK-ITANIUM: @_ZTV1T = unnamed_addr constant { { i8*, i8*, i32 } } { { i8*, i8*, i32 } { i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1T to i8*), i3

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155
+void __vector_size__LongTest() {
+  VectorSizeLong v;
+}

gribozavr wrote:
> gribozavr wrote:
> > gribozavr wrote:
> > > Could you also add tests where the vector member is being subscripted?  
> > > for example, `v.x[0] = 0;`
> > Should there be a warning about the member being uninitialized?
> Just to be clear -- if there should be a warning, I don't think this patch 
> has to necessarily fix everything in the checker and static analyzer core to 
> support vector types, the fix for the crash is valuable by itself.  However, 
> if there should be a warning, this test should have a fixme.
Most of the code in the Static Analyzer that deals with vector types is exactly 
like this patch: "oh, there are also vector types, right, i've no idea what 
they are but let's suppress everything just in case". Fixing false positives 
should be fine, but introducing warnings that are specific to vector types 
would be hard.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61246



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

I think I know the format that I'd like *roughly*. I haven't worked out how to 
handle COMDAT and section groups yet. Roland seems to think that COMDAT and 
section groups will be an issue but I'm working out the details with him to 
make sure I understand it. I'll get back to you soon.

  Version: 1.0
  Arch: AArch64
  Symbols:
- Name: foo
  Type: Function
  Weak: true
- Name: bar
  Type: Function
  Defined: false
- Name: baz
  Type: Object
  Size: 256
  Alignment: 32

Only Arch is used other details from your header can be inferred as defaults 
but are not needed here. I think binding is not needed beyond Weak vs. Not 
Weak. I spoke with Roland about this and he pointed out STB_GNU_UNIQUE. This 
would motivate allowing Global, Weak, and GNU_UNIQUE as valid bindings. The 
only cases for viability that we care are protected and default. Internal and 
hidden will never appear in the public interface. We can treat protected and 
default as the same as they just mean that the symbol will appear in the public 
interface and their difference is related to how the linker resolves symbols 
relative to module itself not other external modules.

We don't have to worry about symbol version (and the TextAPI types don't 
support that yet either) but Warnings are another case to think about. Each 
symbol can have a warning string associated with it. This will be one of the 
last features (before symbol versions but after almost everyhing else) that 
support is added for in llvm-elfabi so I think we can ignore it here for now.

One thing we forgot to add in llvm-elfabi that we now know we need to add is 
Alignment. Alignment can be computed from the section and the offset within the 
section. This is the one bit of information that comes from the section itself 
and not the symbol table.

Absolute symbols probably aren't needed but if we ever need them they're easy 
enough to add via a new symbol type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1483164 , @plotfi wrote:

> So, @compnerd and I have discussed and we'd like to propose a yaml schema 
> that can express what we need for ifsos for the Sections and the Symbols.
>
> ...


Can you clarify is this is for the fully merged format or for the unmerged 
format? I *really* strongly believe that the fully merged format should go 
though llvm-elfabi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

To clarify my format is for the unmerged format. The merged format should be 
the ELFStub internal representation found in llvm's TextAPI. The plan is to 
have at minimum .tbe and ELF stubs. I'm actively working on getting the ELF 
stub output which can then be converted to the yaml2obj format since they will 
just be ELF files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483210 , @jakehehrlich 
wrote:

> In D60974#1483164 , @plotfi wrote:
>
> > So, @compnerd and I have discussed and we'd like to propose a yaml schema 
> > that can express what we need for ifsos for the Sections and the Symbols.
> >
> > ...
>
>
> Can you clarify is this is for the fully merged format or for the unmerged 
> format? I *really* strongly believe that the fully merged format should go 
> though llvm-elfabi.


Unmerged. The merged format could be elfabi or raw elf, but I want to agree on 
something that just lets us generate the intermediate unmerged artifacts for 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

The main reason that we used inline assembly is due to the fixed register 
allocation for these instructions. We would have had to write out the register 
rules in the backend as a special case as seen in the getInstrWFourImplicitOps 
in https://reviews.llvm.org/D44386 Since the register allocator had no freedom 
there was no performance advantage to an intrinsic. So we just did inline 
assembly to avoid needing to special case these instructions.


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

https://reviews.llvm.org/D44387



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483209 , @jakehehrlich 
wrote:

>   I think I know the format that I'd like *roughly*. I haven't worked out how 
> to handle COMDAT and section groups yet. Roland seems to think that COMDAT 
> and section groups will be an issue but I'm working out the details with him 
> to make sure I understand it. I'll get back to you soon.
>   
>
>   Version: 1.0
>   Arch: AArch64
>   Symbols:
> - Name: foo
>   Type: Function
>   Weak: true
> - Name: bar
>   Type: Function
>   Defined: false
> - Name: baz
>   Type: Object
>   Size: 256
>   Alignment: 32
>
>
>


Me and @compnerd had discussed a more abstracted format like this but decided 
it was best to just use the same names that are in the ELF already.
Is there any compelling reason not to?
As far as I understand, by having something like "Weak: true" is already 
harking back to ELF so why not stick to the same names?

I think the !tbd-elf-v1 versioning can help with any changes or alterations we 
want to make along the way too.
We did discuss the alignment field too.

> Only Arch is used other details from your header can be inferred as defaults 
> but are not needed here. I think binding is not needed beyond Weak vs. Not 
> Weak. I spoke with Roland about this and he pointed out STB_GNU_UNIQUE. This 
> would motivate allowing Global, Weak, and GNU_UNIQUE as valid bindings. The 
> only cases for viability that we care are protected and default. Internal and 
> hidden will never appear in the public interface. We can treat protected and 
> default as the same as they just mean that the symbol will appear in the 
> public interface and their difference is related to how the linker resolves 
> symbols relative to module itself not other external modules.
> 
> We don't have to worry about symbol version (and the TextAPI types don't 
> support that yet either) but Warnings are another case to think about. Each 
> symbol can have a warning string associated with it. This will be one of the 
> last features (before symbol versions but after almost everyhing else) that 
> support is added for in llvm-elfabi so I think we can ignore it here for now.
> 
> One thing we forgot to add in llvm-elfabi that we now know we need to add is 
> Alignment. Alignment can be computed from the section and the offset within 
> the section. This is the one bit of information that comes from the section 
> itself and not the symbol table.
> 
> Absolute symbols probably aren't needed but if we ever need them they're easy 
> enough to add via a new symbol type.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

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

Szelethus updated this revision to Diff 197209.
Szelethus added a comment.

Add `TODO:`s about missing warnings.


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

https://reviews.llvm.org/D61246

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  test/Analysis/cxx-uninitialized-object.cpp


Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -1132,7 +1132,7 @@
 }
 
 
//===--===//
-// _Atomic tests.
+// "Esoteric" primitive type tests.
 
//===--===//
 
 struct MyAtomicInt {
@@ -1142,6 +1142,17 @@
   MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
 };
 
-void entry() {
+void _AtomicTest() {
   MyAtomicInt b;
 }
+
+struct VectorSizeLong {
+  VectorSizeLong() {}
+  __attribute__((__vector_size__(16))) long x;
+};
+
+void __vector_size__LongTest() {
+  // TODO: Warn for v.x.
+  VectorSizeLong v;
+  v.x[0] = 0;
+}
Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -256,6 +256,29 @@
   CharPointerTest();
 }
 
+struct VectorSizePointer {
+  VectorSizePointer() {} // expected-warning{{1 uninitialized field}}
+  __attribute__((__vector_size__(8))) int *x; // expected-note{{uninitialized 
pointer 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+};
+
+void __vector_size__PointerTest() {
+  VectorSizePointer v;
+}
+
+struct VectorSizePointee {
+  using MyVectorType = __attribute__((__vector_size__(8))) int;
+  MyVectorType *x;
+
+  VectorSizePointee(decltype(x) x) : x(x) {}
+};
+
+void __vector_size__PointeeTest() {
+  VectorSizePointee::MyVectorType i;
+  // TODO: Report v.x's pointee.
+  VectorSizePointee v(&i);
+}
+
 struct CyclicPointerTest1 {
   int *ptr; // expected-note{{object references itself 'this->ptr'}}
   int dontGetFilteredByNonPedanticMode = 0;
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -324,7 +324,8 @@
 inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType() || T->isAtomicType();
+ T->isFunctionType() || T->isAtomicType() ||
+ T->isVectorType();
 }
 
 inline bool isDereferencableType(const QualType &T) {


Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -1132,7 +1132,7 @@
 }
 
 //===--===//
-// _Atomic tests.
+// "Esoteric" primitive type tests.
 //===--===//
 
 struct MyAtomicInt {
@@ -1142,6 +1142,17 @@
   MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
 };
 
-void entry() {
+void _AtomicTest() {
   MyAtomicInt b;
 }
+
+struct VectorSizeLong {
+  VectorSizeLong() {}
+  __attribute__((__vector_size__(16))) long x;
+};
+
+void __vector_size__LongTest() {
+  // TODO: Warn for v.x.
+  VectorSizeLong v;
+  v.x[0] = 0;
+}
Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -256,6 +256,29 @@
   CharPointerTest();
 }
 
+struct VectorSizePointer {
+  VectorSizePointer() {} // expected-warning{{1 uninitialized field}}
+  __attribute__((__vector_size__(8))) int *x; // expected-note{{uninitialized pointer 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+};
+
+void __vector_size__PointerTest() {
+  VectorSizePointer v;
+}
+
+struct VectorSizePointee {
+  using MyVectorType = __attribute__((__vector_size__(8))) int;
+  MyVectorType *x;
+
+  VectorSizePointee(decltype(x) x) : x(x) {}
+};
+
+void __vector_size__PointeeTest() {
+  VectorSizePointee::MyVectorType i;
+  // TODO: Report v.x's pointee.
+  VectorSizePointee v(&i);
+}
+
 struct CyclicPointerTest1 {
   int *ptr; // expected-note{{object references itself 'this->ptr'}}
   int dontGetFilteredByNonPedanticMode = 0;
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

[PATCH] D61136: [Analyzer] IteratorChecker - Ensure end()>=begin() and refactor begin and end symbol creation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D61136#1480023 , 
@baloghadamsoftware wrote:

> Abstract iterator positions are represented by symbolic expressions because 
> we must be able to do simple calculations with them. This has nothing to do 
> with iterator objects represented as symbols. Or what do you mean exactly by 
> "replacing position symbols with `SymbolMetadata`"?


I mean, use `SymbolMetadata` wherever you use `SymbolConjured`. Because 
metadata is attached to objects (i.e., it takes a region as part of its 
identity), it's problematic to use with iterators represented as symbols.

Though i'd also consider the opposite direction which seems to be even more 
promising: re-do `SymbolMetadata` so that it took a symbol instead of a region 
as part of its identity, and then attach "identity symbols" to all C++ objects, 
so that we didn't ever have to track iterators by their regions, but instead we 
could make `IteratorChecker` operate more like `SimpleStreamChecker`. This 
still requires re-doing all the conjuring, so keeping all the conjuring in one 
place is still a good idea.




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1929-1930
+
+  auto &SymMgr = State->getSymbolManager();
+  auto Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount, "end");
+  State = assumeNoOverflow(State, Sym, 4);

baloghadamsoftware wrote:
> NoQ wrote:
> > This is a bit more `auto` than allowed by [[ 
> > https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> >  | our coding standards ]] (it pretty much disallows `auto` unless it's 
> > some sort of `dyn_cast` (`getAs`) or an iterator.
> I can add the type, of course. However, until now I understood and it is also 
> in the coding standard that "other places where the type is already obvious 
> from the context". For me it is obvious that `conjureSymbol()` returns a 
> `SymbolRef`. Even more obvious is the `getSymbolManager()` returns a 
> `SymbolManager`.
[[ https://reviews.llvm.org/D33672?id=171506#inline-475812 | Here is a 
discussion on this matter that i had recently. ]] It was hard enough to 
convince people that the code below follows the coding standards:
```lang=c++
auto DV = V.getAs();
```

Also, `conjuredSymbol()` in fact returns a `const SymbolConjured *`, which is a 
more specialized type. This probably deserves at least a `const auto *`.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:2247
 
+ProgramStateRef ensureNonNegativeDiff(ProgramStateRef State, SymbolRef Sym1,
+  SymbolRef Sym2) {

baloghadamsoftware wrote:
> NoQ wrote:
> > This looks like a new feature. Is it testable?
> I am not sure I can test it alone. Maybe I should leave it for now and add it 
> in another patch together with modelling of `empty()` or `size()`. Then I 
> should also rename this patch which remains pure refactoring.
Yup, i think it's good to keep NFC commits and features apart.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61136



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

r359512 - Make test more robust by writing stdout/stderr to different files.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Mon Apr 29 15:57:15 2019
New Revision: 359512

URL: http://llvm.org/viewvc/llvm-project?rev=359512&view=rev
Log:
Make test more robust by writing stdout/stderr to different files.

Our internal build bots were failing this test randomly as the stderr
output was emitted to the file in the middle of the stdout output
line that the test was checking.

Modified:
cfe/trunk/test/Index/missing_vfs.c

Modified: cfe/trunk/test/Index/missing_vfs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/missing_vfs.c?rev=359512&r1=359511&r2=359512&view=diff
==
--- cfe/trunk/test/Index/missing_vfs.c (original)
+++ cfe/trunk/test/Index/missing_vfs.c Mon Apr 29 15:57:15 2019
@@ -1,6 +1,6 @@
-// RUN: c-index-test -test-load-source local %s -ivfsoverlay 
%t/does-not-exist.yaml &> %t.out
-// RUN: FileCheck -check-prefix=STDERR %s < %t.out
+// RUN: c-index-test -test-load-source local %s -ivfsoverlay 
%t/does-not-exist.yaml > %t.stdout 2> %t.stderr
+// RUN: FileCheck -check-prefix=STDERR %s < %t.stderr
 // STDERR: fatal error: virtual filesystem overlay file '{{.*}}' not found
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s < %t.stdout
 // CHECK: missing_vfs.c:[[@LINE+1]]:6: FunctionDecl=foo:[[@LINE+1]]:6
 void foo(void);


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