[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

looks almost good.




Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:197
+}
+
+TEST(ArgStripperTest, Spellings) {

sammccall wrote:
> hokein wrote:
> > add tests for stripping the diagnostic flags, `-Wfoo` etc.
> Those aren't actually separate flags: "-W" is a Joined flag and foo is an arg.
> Do you want a test specifically for such a string anyway?
> Or do you want special behavior for them? (Like interaction between -Wfoo and 
> -Wno-foo)
I was a bit unclear about how the stripper strips the "-W/-D"-like flag, would 
be nice to have them in tests as I think these are important cases.

> Those aren't actually separate flags: "-W" is a Joined flag and foo is an arg.

oh, if I understand correctly:

- `strip("unused", "clang -Wunused foo.cc")` => `clang foo.cc` ?
- `strip("-W", "clang -Wunused -Wextra foo.cc")` => `clang foo.cc` // remove 
all -W flags ?
- `strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
foo.cc")`  =>  `clang -Wunused -Werror=date-time foo.cc`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958



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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

It would be a good idea to emit notes to help explain the problem code, I'm 
thinking something along the lines of:

  warning: exit-handler potentially calls a jump function. Handlers should 
terminate by returning [cert-env32-c]
  note: exit-handler declared here
  note: call to a jump function declared here

The second note would require you to track the call site along with the 
FunctionDecl. 
I wouldn't go as far as trying to track the entire call stack when functions in 
exit handlers call functions that exit. While its possible, it could end up 
looking messy.




Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:26
+namespace {
+constexpr StringRef EF__Exit = "_Exit";
+constexpr StringRef EF_exit = "exit";

Listen to the clang-tidy warnings.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:72
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  // clang-format off
+  Finder->addMatcher(

Is there a reason  for disabling format here? How messy is the matcher code 
when formatted?



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:78
+  anyOf(
+hasName(RF_atexit),
+hasName(RF_at_quick_exit)

Use `hasAnyName`



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:120-122
+if (SeenFunctions.count(Current))
+  continue;
+SeenFunctions.insert(Current);

nit:
```
if (!SeenFunctions.insert(Current).second)
  continue;
```



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:151
+// Add called functions to the worklist.
+std::copy(Collector.begin(), Collector.end(),
+  std::back_inserter(CalledFunctions));

Use `llvm::copy` instead of `std::copy`



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.h:18
+
+///
+/// Checker for SEI CERT rule ENV32-C

Unnecessary empty line



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
+  ` check.
+  Finds functions registered by ``atexit`` and ``at_quick_exit`` those are 
calling
+  exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.

s/those/that



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:114
+
+This compliant solution does not call longjmp()but instead returns from the 
exit handler normally:
+

Space after `longjmp() `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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


[PATCH] D83705: [clangd] Config: CompileFlags.Remove

2020-07-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:126
   void compile(Fragment::CompileFlagsBlock &&F) {
+if (!F.Remove.empty()) {
+  auto Remove = std::make_shared();

the order of processing "Remove" and "Add" matters, if they are conflict, the 
`Add` will take precedence, I think this is intended? Maybe worth clarifying in 
the CompileFlagsBlock comments.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:136
+/// - If the value is a recognized clang flag (like "-I") then it will be
+///   removed along with any arguments. Synonyms like --include-dir= wil
+///   also be removed.

nit: wil => will



Comment at: clang-tools-extra/clangd/ConfigFragment.h:141
+/// - Otherwise any argument exactly matching the value is removed.
+///
+/// In all cases, -Xclang is also removed where needed.

nit: I'd prefer to give a few concrete examples (e.g. examples I mentioned in 
another patch), it would be helpful for readers to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83705



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


[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a subscriber: vrnithinkumar.
vsavchenko added a comment.

In D83660#2148834 , @NoQ wrote:

> Looks like a copy-paste error indeed.


@vrnithinkumar @NoQ looks like a good opportunity for `SmartPtr` checker and 
`llvm::Optional`!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660



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


[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-14 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 277683.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Remove llvm::Optional from character fields.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange &SR) {
@@ -35,6 +39,20 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto &R : FoldingRanges) {
+NextRange.start.line = R.startLine;
+NextRange.start.character = R.startCharacter;
+NextRange.end.line = R.endLine;
+NextRange.end.character = R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +136,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +199,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+[[int global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -676,6 +684,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST &AST, Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> getFoldingRanges(ParsedAST &AST);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Semantic

[clang-tools-extra] 7a514c9 - [clangd] Implement textDocument/foldingRange

2020-07-14 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2020-07-14T09:28:42+02:00
New Revision: 7a514c9bf8f2513b57aee685879dd2c104381d99

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

LOG: [clangd] Implement textDocument/foldingRange

Summary:
This patch introduces basic textDocument/foldingRange support. It relies on
textDocument/documentSymbols to collect all symbols and uses takes ranges
to create folds.

The next steps for textDocument/foldingRange support would be:

* Implementing FoldingRangeClientCapabilities and respecting respect client
  preferences
* Specifying folding range kind
* Migrating from DocumentSymbol implementation to custom RecursiveASTVisitor 
flow that will allow more flexibility
* Supporting more folding range types: comments, PP conditional regions, 
includes and other code regions (e.g. public/private/protected sections of 
classes, control flow statement bodies)

Tested: (Neo)Vim (coc-clangd) and VSCode.

Related issue: https://github.com/clangd/clangd/issues/310

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: nridge, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, 
usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/SemanticSelection.cpp
clang-tools-extra/clangd/SemanticSelection.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index b0aba886edbe..0408b0498488 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -637,6 +637,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
 ->insert(
 {"semanticHighlighting",
  llvm::json::Object{{"scopes", 
buildHighlightScopeLookupTable()}}});
+  if (ClangdServerOpts.FoldingRanges)
+Result.getObject("capabilities")->insert({"foldingRangeProvider", true});
   Reply(std::move(Result));
 }
 
@@ -929,7 +931,6 @@ void ClangdLSPServer::onDocumentFormatting(
 static std::vector
 flattenSymbolHierarchy(llvm::ArrayRef Symbols,
const URIForFile &FileURI) {
-
   std::vector Results;
   std::function Process =
   [&](const DocumentSymbol &S, llvm::Optional ParentName) 
{
@@ -968,6 +969,12 @@ void ClangdLSPServer::onDocumentSymbol(const 
DocumentSymbolParams &Params,
   });
 }
 
+void ClangdLSPServer::onFoldingRange(
+const FoldingRangeParams &Params,
+Callback> Reply) {
+  Server->foldingRanges(Params.textDocument.uri.file(), std::move(Reply));
+}
+
 static llvm::Optional asCommand(const CodeAction &Action) {
   Command Cmd;
   if (Action.command && Action.edit)
@@ -1395,6 +1402,8 @@ ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/documentLink", 
&ClangdLSPServer::onDocumentLink);
   MsgHandler->bind("textDocument/semanticTokens/full", 
&ClangdLSPServer::onSemanticTokens);
   MsgHandler->bind("textDocument/semanticTokens/full/delta", 
&ClangdLSPServer::onSemanticTokensDelta);
+  if (Opts.FoldingRanges)
+MsgHandler->bind("textDocument/foldingRange", 
&ClangdLSPServer::onFoldingRange);
   // clang-format on
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index a779e9036c4a..d0c0e814c641 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -87,6 +87,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   // otherwise.
   void onDocumentSymbol(const DocumentSymbolParams &,
 Callback);
+  void onFoldingRange(const FoldingRangeParams &,
+  Callback>);
   void onCodeAction(const CodeActionParams &, Callback);
   void onCompletion(const CompletionParams &, Callback);
   void onSignatureHelp(const TextDocumentPositionParams &,

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5d99104dadaf..c33cdffcb0ca 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -674,6 +674,18 @@ void ClangdServer::documentSymbols(llvm::StringRef File,
TUScheduler::InvalidateOnUpdate);
 }
 
+void ClangdServer::foldingRanges(llvm::StringRef File,
+ Callback> CB) {
+  auto Action =
+  [CB = std::move(CB)](llvm:

[PATCH] D82436: [clangd] Implement textDocument/foldingRange

2020-07-14 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a514c9bf8f2: [clangd] Implement textDocument/foldingRange 
(authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82436

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -17,15 +17,19 @@
 #include "TestTU.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+
 namespace clang {
 namespace clangd {
 namespace {
+
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 // front() is SR.range, back() is outermost range.
 std::vector gatherRanges(const SelectionRange &SR) {
@@ -35,6 +39,20 @@
   return Ranges;
 }
 
+std::vector
+gatherFoldingRanges(llvm::ArrayRef FoldingRanges) {
+  std::vector Ranges;
+  Range NextRange;
+  for (const auto &R : FoldingRanges) {
+NextRange.start.line = R.startLine;
+NextRange.start.character = R.startCharacter;
+NextRange.end.line = R.endLine;
+NextRange.end.character = R.endCharacter;
+Ranges.push_back(NextRange);
+  }
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
   R"cpp( // Single statement in a function body.
@@ -118,16 +136,16 @@
   )cpp",
   R"cpp( // Inside struct.
 struct A { static int a(); };
-[[struct B { 
+[[struct B {
   [[static int b() [[{
 [[return 1^1]] + 2;
   }
 }]];
   )cpp",
   // Namespaces.
-  R"cpp( 
-[[namespace nsa { 
-  [[namespace nsb { 
+  R"cpp(
+[[namespace nsa {
+  [[namespace nsb {
 static int ccc();
 [[void func() [[{
   // int x = nsa::nsb::ccc();
@@ -181,6 +199,41 @@
   EXPECT_THAT(gatherRanges(Ranges->back()),
   ElementsAre(SourceAnnotations.range("empty")));
 }
+
+TEST(FoldingRanges, All) {
+  const char *Tests[] = {
+  R"cpp(
+[[int global_variable]];
+
+[[void func() {
+  int v = 100;
+}]]
+  )cpp",
+  R"cpp(
+[[class Foo {
+public:
+  [[Foo() {
+int X = 1;
+  }]]
+
+private:
+  [[int getBar() {
+return 42;
+  }]]
+
+  [[void getFooBar() { }]]
+}]];
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
+UnorderedElementsAreArray(T.ranges()))
+<< Test;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -296,6 +296,14 @@
 Hidden,
 };
 
+opt FoldingRanges{
+"folding-ranges",
+cat(Features),
+desc("Enable preview of FoldingRanges feature"),
+init(false),
+Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -676,6 +684,7 @@
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
+  Opts.FoldingRanges = FoldingRanges;
 
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -25,6 +25,10 @@
 /// If pos is not in any interesting range, return [Pos, Pos).
 llvm::Expected getSemanticRanges(ParsedAST &AST, Position Pos);
 
+/// Returns a list of ranges whose contents might be collapsible in an editor.
+/// This should include large scopes, preprocessor blocks etc.
+llvm::Expected> getFoldingRanges(ParsedAST &AST);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/c

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

I do like this change even apart from the `clang-tidy` integration epic.  I 
strongly believe that decoupling (when done right) makes things easier and this 
change seems like a good first step.


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

https://reviews.llvm.org/D67420



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


[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-07-14 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie added a comment.

> do you accidentally have a test case to reproduce the crash

@NoQ I am now working with the reporter of this bug to make a simple test case 
to trigger the crash.

> looks like a good opportunity for `SmartPtr` checker and `llvm::Optional`!

@vsavchenko It seems not difficult to write a checker specifically for 
`llvm::Optional`. However, the root reason for this crash is an assertion 
failure. It seems to be better to write a checker to report assertion failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660



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


[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-07-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D83660#2148834 , @NoQ wrote:

> Looks like a copy-paste error indeed.
>
> @OikawaKirie do you accidentally have a test case to reproduce the crash, so 
> that we could add it to our regression test suite? 'Cause traditionally we 
> add a test to every patch.
>
> Note that SMT-backed constraint managers aren't an actual supported mode; 
> it's an experimental project that isn't yet supposed to be used anywhere and 
> probably won't be ever officially supported, unless reworked dramatically.


I cannot agree more. Although I would appreciate some progress improving Z3 
constraint manager. Might in the future I would spend some time on it - we will 
see.

BTW nice catch & fix @OikawaKirie!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660



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


[PATCH] D83286: [analyzer][solver] Track symbol disequalities

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

In D83286#2148787 , @NoQ wrote:

> Data 👍


Thanks 😊 These tables are already generated by a script and I plan to make it 
into **a phabricator reporter**, so it is easier to put all the data we need 
here.

> Maybe spend a few minutes remeasuring `libsoundio` more carefully, just in 
> case?

Sure, will do!

> Also i really wish we had per-TU data of that kind, to see if any particular 
> TUs have regressed significantly.

I think we even store all this information about the run, but it needs a bit of 
refactoring in `CmpRuns.py` to get it in a nice way.  I think it's a good 
feature to put into my plan for the testing system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83286



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


[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-07-14 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie added a comment.

> Might in the future I would spend some time on it - we will see.

@steakhal My boss always asks me about how to improve the performance for SMT 
solver based constraint solving, on both the engine side and the SMT solver 
side. If there is anything that our research group can do, you are free to 
contact us.

> BTW nice catch & fix.

Thank you. This crash was discovered by one of our team members when we were 
testing our CSA based tool. If the patch gets merged in the future, I'd like to 
use his name and email for the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660



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


[clang] 089a0ad - [analyzer][tests] Add 5 more projects for testing

2020-07-14 Thread Valeriy Savchenko via cfe-commits

Author: Valeriy Savchenko
Date: 2020-07-14T11:42:46+03:00
New Revision: 089a0ad8bc993923817d7957f08bd67dbecd56af

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

LOG: [analyzer][tests] Add 5 more projects for testing

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

Added: 
clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh
clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh
clang/utils/analyzer/projects/harfbuzz/run_static_analyzer.cmd
clang/utils/analyzer/projects/tmux/cleanup_run_static_analyzer.sh
clang/utils/analyzer/projects/tmux/run_static_analyzer.cmd

Modified: 
clang/utils/analyzer/Dockerfile
clang/utils/analyzer/entrypoint.py
clang/utils/analyzer/projects/projects.json

Removed: 




diff  --git a/clang/utils/analyzer/Dockerfile b/clang/utils/analyzer/Dockerfile
index 21906011c7dc..f74ff8aa95c2 100644
--- a/clang/utils/analyzer/Dockerfile
+++ b/clang/utils/analyzer/Dockerfile
@@ -42,6 +42,16 @@ RUN apt-get install -y \
 libjsonrpccpp-dev=0.7.0-1build2 \
 uuid-dev=2.31.1-0.4ubuntu3.6
 
+# tmux dependencies
+RUN apt-get install -y \
+autotools-dev=20180224.1 \
+automake=1:1.15.1-3ubuntu2 \
+libncurses5-dev=6.1-1ubuntu1.18.04 \
+libevent-dev=2.1.8-stable-4build1 \
+pkg-config=0.29.1-0ubuntu2 \
+flex=2.6.4-6 \
+bison=2:3.0.4.dfsg-1build1
+
 RUN update-alternatives --install /usr/bin/python python /usr/bin/python3 1
 
 VOLUME /analyzer

diff  --git a/clang/utils/analyzer/entrypoint.py 
b/clang/utils/analyzer/entrypoint.py
index b440e776b57c..9c84431da548 100644
--- a/clang/utils/analyzer/entrypoint.py
+++ b/clang/utils/analyzer/entrypoint.py
@@ -50,7 +50,7 @@ def is_cmake_needed():
 
 CMAKE_COMMAND = "cmake -G Ninja -DCMAKE_BUILD_TYPE=Release " \
 "-DCMAKE_INSTALL_PREFIX=/analyzer -DLLVM_TARGETS_TO_BUILD=X86 " \
-"-DLLVM_ENABLE_PROJECTS=clang -DLLVM_BUILD_RUNTIME=OFF " \
+"-DLLVM_ENABLE_PROJECTS=\"clang;openmp\" -DLLVM_BUILD_RUNTIME=OFF " \
 "-DLLVM_ENABLE_TERMINFO=OFF -DCLANG_ENABLE_ARCMT=OFF " \
 "-DCLANG_ENABLE_STATIC_ANALYZER=ON"
 

diff  --git 
a/clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh 
b/clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh
new file mode 100755
index ..e14c423280ec
--- /dev/null
+++ b/clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+rm -rf ./build

diff  --git a/clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd 
b/clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
new file mode 100644
index ..6678fe635db3
--- /dev/null
+++ b/clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+cmake . -DCMAKE_BUILD_TYPE=Debug -Bbuild -GNinja
+cmake --build build

diff  --git 
a/clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh 
b/clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
new file mode 100755
index ..e14c423280ec
--- /dev/null
+++ b/clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+rm -rf ./build

diff  --git a/clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd 
b/clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
new file mode 100644
index ..72cb7f7677e6
--- /dev/null
+++ b/clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+cmake . -DCMAKE_BUILD_TYPE=Debug -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON -Bbuild 
-GNinja
+cmake --build build

diff  --git 
a/clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh 
b/clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
new file mode 100755
index ..efcd16e590a1
--- /dev/null
+++ b/clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+make clean

diff  --git a/clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd 
b/clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
new file mode 100644
index ..877fa2aa389b
--- /dev/null
+++ b/clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+./configure --without-cuda
+make

diff  --git 
a/clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh 
b/clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh
new file mode 100755
index ..e14c423280ec
--- /dev/null
+++ 

[PATCH] D83701: [analyzer][tests] Add 5 more projects for testing

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG089a0ad8bc99: [analyzer][tests] Add 5 more projects for 
testing (authored by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83701

Files:
  clang/utils/analyzer/Dockerfile
  clang/utils/analyzer/entrypoint.py
  clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh
  clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
  clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
  clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
  clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
  clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
  clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh
  clang/utils/analyzer/projects/harfbuzz/run_static_analyzer.cmd
  clang/utils/analyzer/projects/projects.json
  clang/utils/analyzer/projects/tmux/cleanup_run_static_analyzer.sh
  clang/utils/analyzer/projects/tmux/run_static_analyzer.cmd

Index: clang/utils/analyzer/projects/tmux/run_static_analyzer.cmd
===
--- /dev/null
+++ clang/utils/analyzer/projects/tmux/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+./autogen.sh
+./configure && make
Index: clang/utils/analyzer/projects/tmux/cleanup_run_static_analyzer.sh
===
--- /dev/null
+++ clang/utils/analyzer/projects/tmux/cleanup_run_static_analyzer.sh
@@ -0,0 +1,2 @@
+make distclean
+exit 0
Index: clang/utils/analyzer/projects/projects.json
===
--- clang/utils/analyzer/projects/projects.json
+++ clang/utils/analyzer/projects/projects.json
@@ -103,5 +103,40 @@
 "source": "git",
 "origin": "https://github.com/google/re2.git";,
 "commit": "2b25567"
+  },
+  {
+"name": "cppcheck",
+"mode": 1,
+"source": "git",
+"origin": "https://github.com/danmar/cppcheck.git";,
+"commit": "5fa3d53"
+  },
+  {
+"name": "harfbuzz",
+"mode": 1,
+"source": "git",
+"origin": "https://github.com/harfbuzz/harfbuzz.git";,
+"commit": "f8d345e"
+  },
+  {
+"name": "capnproto",
+"mode": 1,
+"source": "git",
+"origin": "https://github.com/capnproto/capnproto.git";,
+"commit": "8be1c9f"
+  },
+  {
+"name": "tmux",
+"mode": 1,
+"source": "git",
+"origin": "https://github.com/tmux/tmux.git";,
+"commit": "a5f99e1"
+  },
+  {
+"name": "faiss",
+"mode": 1,
+"source": "git",
+"origin": "https://github.com/facebookresearch/faiss.git";,
+"commit": "9e5d5b7"
   }
 ]
Index: clang/utils/analyzer/projects/harfbuzz/run_static_analyzer.cmd
===
--- /dev/null
+++ clang/utils/analyzer/projects/harfbuzz/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+cmake . -DCMAKE_BUILD_TYPE=Debug -Bbuild -GNinja
+cmake --build build
Index: clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh
===
--- /dev/null
+++ clang/utils/analyzer/projects/harfbuzz/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+rm -rf ./build
Index: clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
===
--- /dev/null
+++ clang/utils/analyzer/projects/faiss/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+./configure --without-cuda
+make
Index: clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
===
--- /dev/null
+++ clang/utils/analyzer/projects/faiss/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+make clean
Index: clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
===
--- /dev/null
+++ clang/utils/analyzer/projects/cppcheck/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+cmake . -DCMAKE_BUILD_TYPE=Debug -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON -Bbuild -GNinja
+cmake --build build
Index: clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
===
--- /dev/null
+++ clang/utils/analyzer/projects/cppcheck/cleanup_run_static_analyzer.sh
@@ -0,0 +1 @@
+rm -rf ./build
Index: clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
===
--- /dev/null
+++ clang/utils/analyzer/projects/capnproto/run_static_analyzer.cmd
@@ -0,0 +1,2 @@
+cmake . -DCMAKE_BUILD_TYPE=Debug -Bbuild -GNinja
+cmake --build build
Index: clang/utils/analyzer/projects/capnproto/cleanup_run_static_analyzer.sh
===
--- /dev/null
+++ clang/utils/analyzer/projects/capnproto/cleanup_run_stati

[clang] 5b4f143 - [analyzer][tests] Introduce analyzer benchmarking framework

2020-07-14 Thread Valeriy Savchenko via cfe-commits

Author: Valeriy Savchenko
Date: 2020-07-14T11:42:46+03:00
New Revision: 5b4f143564502664a9d1197d6909047eab49530e

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

LOG: [analyzer][tests] Introduce analyzer benchmarking framework

Summary:
This commit includes a couple of changes:
  * Benchmark selected projects by analyzing them multiple times
  * Compare two benchmarking results and visualizing them on one chart
  * Organize project build logging, so we can use the same code
in benchmarks

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

Added: 
clang/utils/analyzer/SATestBenchmark.py

Modified: 
clang/utils/analyzer/SATest.py
clang/utils/analyzer/SATestBuild.py
clang/utils/analyzer/SATestUpdateDiffs.py
clang/utils/analyzer/requirements.txt

Removed: 




diff  --git a/clang/utils/analyzer/SATest.py b/clang/utils/analyzer/SATest.py
index 16f1dce0c584..46e636ad2895 100755
--- a/clang/utils/analyzer/SATest.py
+++ b/clang/utils/analyzer/SATest.py
@@ -34,29 +34,10 @@ def add(parser, args):
 
 def build(parser, args):
 import SATestBuild
-from ProjectMap import ProjectMap
 
 SATestBuild.VERBOSE = args.verbose
 
-project_map = ProjectMap()
-projects = project_map.projects
-
-if args.projects:
-projects_arg = args.projects.split(",")
-available_projects = [project.name
-  for project in projects]
-
-# validate that given projects are present in the project map file
-for manual_project in projects_arg:
-if manual_project not in available_projects:
-parser.error("Project '{project}' is not found in "
- "the project map file. Available projects are "
- "{all}.".format(project=manual_project,
- all=available_projects))
-
-projects = [project.with_fields(enabled=project.name in projects_arg)
-for project in projects]
-
+projects = get_projects(parser, args.projects)
 tester = SATestBuild.RegressionTester(args.jobs,
   projects,
   args.override_compiler,
@@ -100,6 +81,44 @@ def update(parser, args):
 SATestUpdateDiffs.update_reference_results(project)
 
 
+def benchmark(parser, args):
+from SATestBenchmark import Benchmark
+
+projects = get_projects(parser, args.projects)
+benchmark = Benchmark(projects, args.iterations, args.output)
+benchmark.run()
+
+
+def benchmark_compare(parser, args):
+import SATestBenchmark
+SATestBenchmark.compare(args.old, args.new, args.output)
+
+
+def get_projects(parser, projects_str):
+from ProjectMap import ProjectMap
+
+project_map = ProjectMap()
+projects = project_map.projects
+
+if projects_str:
+projects_arg = projects_str.split(",")
+available_projects = [project.name
+  for project in projects]
+
+# validate that given projects are present in the project map file
+for manual_project in projects_arg:
+if manual_project not in available_projects:
+parser.error("Project '{project}' is not found in "
+ "the project map file. Available projects are "
+ "{all}.".format(project=manual_project,
+ all=available_projects))
+
+projects = [project.with_fields(enabled=project.name in projects_arg)
+for project in projects]
+
+return projects
+
+
 def docker(parser, args):
 if len(args.rest) > 0:
 if args.rest[0] != "--":
@@ -284,6 +303,36 @@ def main():
  "to the docker's entrypoint.")
 dock_parser.set_defaults(func=docker)
 
+# benchmark subcommand
+bench_parser = subparsers.add_parser(
+"benchmark",
+help="Run benchmarks by building a set of projects multiple times.")
+
+bench_parser.add_argument("-i", "--iterations", action="store",
+  type=int, default=20,
+  help="Number of iterations for building each "
+  "project.")
+bench_parser.add_argument("-o", "--output", action="store",
+  default="benchmark.csv",
+  help="Output csv file for the benchmark results")
+bench_parser.add_argument("--projects", action="store", default="",
+  help="Comma-separated list of projects to test")
+bench_parser.set_defaults(func=benchmark)
+
+bench_subparsers = bench_parser.add_subparsers()
+bench_compare_parser = be

[PATCH] D83539: [analyzer][tests] Introduce analyzer benchmarking framework

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
vsavchenko marked an inline comment as done.
Closed by commit rG5b4f14356450: [analyzer][tests] Introduce analyzer 
benchmarking framework (authored by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83539

Files:
  clang/utils/analyzer/SATest.py
  clang/utils/analyzer/SATestBenchmark.py
  clang/utils/analyzer/SATestBuild.py
  clang/utils/analyzer/SATestUpdateDiffs.py
  clang/utils/analyzer/requirements.txt

Index: clang/utils/analyzer/requirements.txt
===
--- clang/utils/analyzer/requirements.txt
+++ clang/utils/analyzer/requirements.txt
@@ -1,4 +1,6 @@
 graphviz
 humanize
 matplotlib
+pandas
 psutil
+seaborn
Index: clang/utils/analyzer/SATestUpdateDiffs.py
===
--- clang/utils/analyzer/SATestUpdateDiffs.py
+++ clang/utils/analyzer/SATestUpdateDiffs.py
@@ -21,10 +21,10 @@
 project_dir = tester.get_project_dir()
 
 tester.is_reference_build = True
-ref_results_path = os.path.join(project_dir, tester.get_output_dir())
+ref_results_path = tester.get_output_dir()
 
 tester.is_reference_build = False
-created_results_path = os.path.join(project_dir, tester.get_output_dir())
+created_results_path = tester.get_output_dir()
 
 if not os.path.exists(created_results_path):
 print("New results not found, was SATestBuild.py previously run?",
Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -87,10 +87,18 @@
 return 0
 
 
-Logger = logging.getLogger("main")
 LOCAL = threading.local()
-LOCAL.stdout = StreamToLogger(Logger, logging.INFO)
-LOCAL.stderr = StreamToLogger(Logger, logging.ERROR)
+
+
+def init_logger(name: str):
+# TODO: use debug levels for VERBOSE messages
+logger = logging.getLogger(name)
+logger.setLevel(logging.DEBUG)
+LOCAL.stdout = StreamToLogger(logger, logging.INFO)
+LOCAL.stderr = StreamToLogger(logger, logging.ERROR)
+
+
+init_logger("main")
 
 
 def stderr(message: str):
@@ -102,7 +110,6 @@
 
 
 logging.basicConfig(
-level=logging.DEBUG,
 format='%(asctime)s:%(levelname)s:%(name)s: %(message)s')
 
 
@@ -298,12 +305,13 @@
 """
 A component aggregating testing for one project.
 """
-def __init__(self, test_info: TestInfo):
+def __init__(self, test_info: TestInfo, silent: bool = False):
 self.project = test_info.project
 self.override_compiler = test_info.override_compiler
 self.extra_analyzer_config = test_info.extra_analyzer_config
 self.is_reference_build = test_info.is_reference_build
 self.strictness = test_info.strictness
+self.silent = silent
 
 def test(self) -> bool:
 """
@@ -312,20 +320,19 @@
 to the :param strictness: criteria.
 """
 if not self.project.enabled:
-stdout(f" \n\n--- Skipping disabled project {self.project.name}\n")
+self.out(
+f" \n\n--- Skipping disabled project {self.project.name}\n")
 return True
 
-stdout(f" \n\n--- Building project {self.project.name}\n")
+self.out(f" \n\n--- Building project {self.project.name}\n")
 
 start_time = time.time()
 
 project_dir = self.get_project_dir()
-if VERBOSE >= 1:
-stdout(f"  Build directory: {project_dir}.\n")
+self.vout(f"  Build directory: {project_dir}.\n")
 
 # Set the build results directory.
 output_dir = self.get_output_dir()
-output_dir = os.path.join(project_dir, output_dir)
 
 self.build(project_dir, output_dir)
 check_build(output_dir)
@@ -336,8 +343,8 @@
 else:
 passed = run_cmp_results(project_dir, self.strictness)
 
-stdout(f"Completed tests for project {self.project.name} "
-   f"(time: {time.time() - start_time:.2f}).\n")
+self.out(f"Completed tests for project {self.project.name} "
+ f"(time: {time.time() - start_time:.2f}).\n")
 
 return passed
 
@@ -346,22 +353,23 @@
 
 def get_output_dir(self) -> str:
 if self.is_reference_build:
-return REF_PREFIX + OUTPUT_DIR_NAME
+dirname = REF_PREFIX + OUTPUT_DIR_NAME
 else:
-return OUTPUT_DIR_NAME
+dirname = OUTPUT_DIR_NAME
+
+return os.path.join(self.get_project_dir(), dirname)
 
-def build(self, directory: str, output_dir: str):
+def build(self, directory: str, output_dir: str) -> Tuple[float, int]:
 build_log_path = get_build_log_path(output_dir)
 
-stdout(f"Log file: {build_log_path}\n")
-stdout(f"Output directory: {output_dir}\n")
+  

[PATCH] D83701: [analyzer][tests] Add 5 more projects for testing

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Post-commit LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83701



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


[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

2020-07-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D83660#2149509 , @OikawaKirie wrote:

> > Might in the future I would spend some time on it - we will see.
>
> [...] If there is anything that our research group can do, you are free to 
> contact us.


I would use the official LLVM/clang-static-analyzer 
 channel for lightweight discussions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83660



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


[PATCH] D83755: [clangd] Cache config files for 5 seconds, without revalidating with stat.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This is motivated by:

- code completion: nice to do no i/o on the request path
- background index: deciding whether to enqueue each file would stat the config 
file thousands of times in quick succession.

Currently it's applied uniformly to all requests though.

This gives up on performing stat() outside the lock, all this achieves is
letting multiple threads stat concurrently (and thus finish without contention
for nonexistent files).
The ability to finish without IO (just mutex lock + integer check) should
outweigh this, and is less sensitive to platform IO characteristics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83755

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/ConfigProvider.h
  clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -10,10 +10,11 @@
 #include "ConfigProvider.h"
 #include "ConfigTesting.h"
 #include "TestFS.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include "llvm/Support/SourceMgr.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -150,6 +151,43 @@
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
 }
 
+TEST(ProviderTest, Staleness) {
+  MockFS FS;
+
+  auto StartTime = std::chrono::steady_clock::now();
+  Params StaleOK;
+  StaleOK.FreshTime = StartTime;
+  Params MustBeFresh;
+  MustBeFresh.FreshTime = StartTime + std::chrono::hours(1);
+  CapturedDiags Diags;
+  auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS);
+
+  // Initial query always reads, regardless of policy.
+  FS.Files["foo.yaml"] = AddFooWithErr;
+  auto Cfg = P->getConfig(StaleOK, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+  Diags.Diagnostics.clear();
+
+  // Stale value reused by policy.
+  FS.Files["foo.yaml"] = AddBarBaz;
+  Cfg = P->getConfig(StaleOK, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached, not re-parsed";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+
+  // Cache revalidated by policy.
+  Cfg = P->getConfig(MustBeFresh, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+
+  // Cache revalidated by (default) policy.
+  FS.Files.erase("foo.yaml");
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/ConfigProvider.h
===
--- clang-tools-extra/clangd/ConfigProvider.h
+++ clang-tools-extra/clangd/ConfigProvider.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include 
 #include 
 #include 
 
@@ -34,6 +35,10 @@
   /// Absolute path to a source file we're applying the config to. Unix slashes.
   /// Empty if not configuring a particular file.
   llvm::StringRef Path;
+  /// Hint that stale data is OK to improve performance (e.g. avoid IO).
+  /// FreshTime sets a bound for how old the data can be.
+  /// If not set, providers should validate caches against the data source.
+  llvm::Optional FreshTime;
 };
 
 /// Used to report problems in parsing or interpreting a config.
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -11,32 +11,31 @@
 #include "ConfigFragment.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
+#include 
 #include 
 
 namespace clang {
 namespace clangd {
 namespace config {
 
+//
+
 // Threadsafe cache around reading a YAML config file from disk.
 class FileConfigCache {
   std::mutex Mu;
+  std::chrono::steady_clock::time_point ValidTime = {};
   llvm::SmallVector CachedValue;
   llvm::sys::TimePoint<> MTime = {};
   unsigned Size = -1;
 
-  void updateCacheLocked(const llvm::vfs::Status &Stat,
- llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
-if (Size == Stat.getSize() && MTime == Stat.getLastModificationTime())
-  retur

[PATCH] D83539: [analyzer][tests] Introduce analyzer benchmarking framework

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/utils/analyzer/SATestBenchmark.py:139-140
+import matplotlib
+import seaborn as sns
+from matplotlib import pyplot as plt
+

NoQ wrote:
> Why shorten? It's not that we're saving a lot of typing, and i think it's 
> easier to read when package names are recognizable.
I agree with that, but python world has a few very widespread shortenings:

```
numpy as np
matplotlib.pyplot as plt
seaborn as sns
pandas as pd
```

All the tutorials, official documentation and so on include these.  So they 
might be even more recognizable as shortenings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83539



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


[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:197
+}
+
+TEST(ArgStripperTest, Spellings) {

hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > add tests for stripping the diagnostic flags, `-Wfoo` etc.
> > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > arg.
> > Do you want a test specifically for such a string anyway?
> > Or do you want special behavior for them? (Like interaction between -Wfoo 
> > and -Wno-foo)
> I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> would be nice to have them in tests as I think these are important cases.
> 
> > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > arg.
> 
> oh, if I understand correctly:
> 
> - `strip("unused", "clang -Wunused foo.cc")` => `clang foo.cc` ?
> - `strip("-W", "clang -Wunused -Wextra foo.cc")` => `clang foo.cc` // remove 
> all -W flags ?
> - `strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> foo.cc")`  =>  `clang -Wunused -Werror=date-time foo.cc`?
> 
> I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> would be nice to have them in tests as I think these are important cases.

Added.

> strip("unused", "clang -Wunused foo.cc") => clang foo.cc ?

No, we only strip clang args or flag names, not flag args (confusing 
terminology).
-W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang arg). 
-W* will strip -Wunused (clang arg). but "unused" doesn't match - it's not the 
name of a flag, and it's not the arg either.

> strip("-W", "clang -Wunused -Wextra foo.cc") => clang foo.cc // remove all -W 
> flags ?

Yes

> strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> foo.cc") => clang -Wunused -Werror=date-time foo.cc?

No, again we don't strip flag args.
(It's not clear how useful/easy to use this would be, maybe we can re-examine 
later?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958



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


[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 277728.
sammccall added a comment.

Add more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958

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

Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -207,6 +207,166 @@
   EXPECT_THAT(Cmd, ElementsAre(_, "FOO.CC", "--hello", "-fsyntax-only"));
 }
 
+static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
+  llvm::SmallVector Parts;
+  llvm::SplitString(Argv, Parts);
+  std::vector Args = {Parts.begin(), Parts.end()};
+  ArgStripper S;
+  S.strip(Arg);
+  S.process(Args);
+  return llvm::join(Args, " ");
+}
+
+TEST(ArgStripperTest, Spellings) {
+  // May use alternate prefixes.
+  EXPECT_EQ(strip("-pedantic", "clang -pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-pedantic", "clang --pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--pedantic", "clang -pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--pedantic", "clang --pedantic foo.cc"), "clang foo.cc");
+  // May use alternate names.
+  EXPECT_EQ(strip("-x", "clang -x c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-x", "clang --language=c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--language=", "clang -x c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--language=", "clang --language=c++ foo.cc"),
+"clang foo.cc");
+}
+
+TEST(ArgStripperTest, UnknownFlag) {
+  EXPECT_EQ(strip("-xyzzy", "clang -xyzzy foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-xyz*", "clang -xyzzy foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-xyzzy", "clang -Xclang -xyzzy foo.cc"), "clang foo.cc");
+}
+
+TEST(ArgStripperTest, Xclang) {
+  // Flags may be -Xclang escaped.
+  EXPECT_EQ(strip("-ast-dump", "clang -Xclang -ast-dump foo.cc"),
+"clang foo.cc");
+  // Args may be -Xclang escaped.
+  EXPECT_EQ(strip("-add-plugin", "clang -Xclang -add-plugin -Xclang z foo.cc"),
+"clang foo.cc");
+}
+
+TEST(ArgStripperTest, ClangCL) {
+  // /I is a synonym for -I in clang-cl mode only.
+  // Not stripped by default.
+  EXPECT_EQ(strip("-I", "clang -I /usr/inc /Interesting/file.cc"),
+"clang /Interesting/file.cc");
+  // Stripped when invoked as clang-cl.
+  EXPECT_EQ(strip("-I", "clang-cl -I /usr/inc /Interesting/file.cc"),
+"clang-cl");
+  // Stripped when invoked as CL.EXE
+  EXPECT_EQ(strip("-I", "CL.EXE -I /usr/inc /Interesting/file.cc"), "CL.EXE");
+  // Stripped when passed --driver-mode=cl.
+  EXPECT_EQ(strip("-I", "cc -I /usr/inc /Interesting/file.cc --driver-mode=cl"),
+"cc --driver-mode=cl");
+}
+
+TEST(ArgStripperTest, ArgStyles) {
+  // Flag
+  EXPECT_EQ(strip("-Qn", "clang -Qn foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-Qn", "clang -QnZ foo.cc"), "clang -QnZ foo.cc");
+  // Joined
+  EXPECT_EQ(strip("-std=", "clang -std= foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-std=", "clang -std=c++11 foo.cc"), "clang foo.cc");
+  // Separate
+  EXPECT_EQ(strip("-mllvm", "clang -mllvm X foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-mllvm", "clang -mllvmX foo.cc"), "clang -mllvmX foo.cc");
+  // RemainingArgsJoined
+  EXPECT_EQ(strip("/link", "clang-cl /link b c d foo.cc"), "clang-cl");
+  EXPECT_EQ(strip("/link", "clang-cl /linka b c d foo.cc"), "clang-cl");
+  // CommaJoined
+  EXPECT_EQ(strip("-Wl,", "clang -Wl,x,y foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-Wl,", "clang -Wl, foo.cc"), "clang foo.cc");
+  // MultiArg
+  EXPECT_EQ(strip("-segaddr", "clang -segaddr a b foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-segaddr", "clang -segaddra b foo.cc"),
+"clang -segaddra b foo.cc");
+  // JoinedOrSeparate
+  EXPECT_EQ(strip("-G", "clang -GX foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-G", "clang -G X foo.cc"), "clang foo.cc");
+  // JoinedAndSeparate
+  EXPECT_EQ(strip("-plugin-arg-", "clang -cc1 -plugin-arg-X Y foo.cc"),
+"clang -cc1 foo.cc");
+  EXPECT_EQ(strip("-plugin-arg-", "clang -cc1 -plugin-arg- Y foo.cc"),
+"clang -cc1 foo.cc");
+}
+
+TEST(ArgStripperTest, EndOfList) {
+  // When we hit the end-of-args prematurely, we don't crash.
+  // We consume the incomplete args if we've matched the target option.
+  EXPECT_EQ(strip("-I", "clang -Xclang"), "clang -Xclang");
+  EXPECT_EQ(strip("-I", "clang -Xclang -I"), "clang");
+  EXPECT_EQ(strip("-I", "clang -I -Xclang"), "clang");
+  EXPECT_EQ(strip("-I", "clang -I"), "clang");
+}
+
+TEST(ArgStripperTest, Multiple) {
+  ArgStripper S;
+  S.strip("-o");
+  S.strip("-c");
+  std::vector Args = {"clang", "-o", "foo.o", "foo.cc", "-c"};
+  S.process(Args);
+  EXPECT_THAT(Args

[PATCH] D83286: [analyzer][solver] Track symbol disequalities

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

>> Maybe spend a few minutes remeasuring `libsoundio` more carefully, just in 
>> case?
> 
> Sure, will do!

It turns out that **libsoundio** has a problem similar to **re2** and varies a 
lot in terms of the analysis time (probably we can even try to figure out some 
reasons behind it sometime in the future).
Here is the chart comparing 50 runs before and after:
F12337181: libsoundio50.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83286



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


[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

2020-07-14 Thread Daniel Krupp via Phabricator via cfe-commits
dkrupp requested changes to this revision.
dkrupp added a comment.
This revision now requires changes to proceed.

Since the analyzer cannot cannot model the size of the containers just yet (as 
I believe this is the case now), what we are saying with this checker is to 
"always check the return value of the erase() function before use (for 
increment/decrement etc.) whether it is not past-end" .
Adam, are you sure that the user would like to enforce such a generic coding 
rule? Depending on the actual code analyzed, this would require this clumsy 
check at potentially too many places.

Wouldn't it be better to wait for the container size modeling patch? Then the 
user would only get a warning when we are sure that we are erasing the last 
element of the container.

In general I think we should keep the number of config parameters to a minimum 
as it is very hard to explain to the users which one to configure to what value 
and why.


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

https://reviews.llvm.org/D77150



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


[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I've seen you resurrecting the thread, I'll just take a bit of time to remember 
what happened in the previous episodes!


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

https://reviews.llvm.org/D67420



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


[PATCH] D83592: [Parser] Add comment to skipped regions

2020-07-14 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

> The comments in those coverage tests are used for FileCheck, like `//CHECK:`. 
> So, we can't remove those ones.

Oh, I didn't think about that :-)

It's a bit unusual and annoying that the test expectations end up affecting the 
test output. For the comments that are not really part of the test, maybe we 
could do something to exclude them, like running 'sed' before compiling, or 
wrapping them in #if 0 blocks or something.

But figuring that out seems less important than figuring out how to implement 
the feature, and I think the current approach looks promising.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592



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


[PATCH] D78189: [analyzer] StdLibraryFunctionsChecker: Add statistics

2020-07-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal marked an inline comment as done.
steakhal added a comment.

Besides @balazske's comments, looks good to me.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:504-505
 
-if (NewState && NewState != State)
+if (NewState && NewState != State) {
+  ++NumCaseApplied;
   C.addTransition(NewState);

At first, it was strange to check `NewState != State` since the `addTransition` 
will do this check regardless.
Then I recognized the statistic increment.
It makes sense to count only the new state transitions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78189



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


[PATCH] D83723: [OpenMP] Generalize CGOpenMPRuntimeNVPTX as CGOpenMPRuntimeGPU

2020-07-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp:39
+  // return constant compile-time target-specific warp size
+  unsigned WarpSize = CGF.getTarget().getGridValue(llvm::omp::GV_Warp_Size);
+  return Bld.getInt32(WarpSize);

This is new functionality, better to move it in a separate patch, and this one 
mark as NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83723



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


[PATCH] D83759: [clangd] Port lit tests to Windows

2020-07-14 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Changes:

- `background-index.test` Add Windows support.
- `dependency-output.test` Split into two tests (Windows and Posix).
- `did-change-configuration-params.test` Split into two tests (Windows and 
Posix).
- `test-uri-windows.test` This test did not run on Windows displite `REQUIRES: 
windows-gnu || windows-msvc` (replacement: `UNSUPPORTED: !(windows-gnu || 
windows-msvc)`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83759

Files:
  clang-tools-extra/clangd/test/background-index.test
  clang-tools-extra/clangd/test/dependency-output-posix.test
  clang-tools-extra/clangd/test/dependency-output-windows.test
  clang-tools-extra/clangd/test/dependency-output.test
  clang-tools-extra/clangd/test/did-change-configuration-params-posix.test
  clang-tools-extra/clangd/test/did-change-configuration-params-windows.test
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/test/test-uri-windows.test


Index: clang-tools-extra/clangd/test/test-uri-windows.test
===
--- clang-tools-extra/clangd/test/test-uri-windows.test
+++ clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: 
clang-tools-extra/clangd/test/did-change-configuration-params-windows.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params-windows.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params-windows.test
@@ -1,9 +1,9 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
-# UNSUPPORTED: windows-gnu,windows-msvc
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
-{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c":
 {"workingDirectory":"/clangd-test", "compilationCommand": ["clang", "-c", 
"foo.c"]}
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"C:\\clangd-test\\foo.c":
 {"workingDirectory":"C:\\clangd-test", "compilationCommand": ["clang", "-c", 
"foo.c"]}
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int
 main() { int i; return i; }"}}}
 #  CHECK:  "method": "textDocument/publishDiagnostics",
@@ -21,7 +21,7 @@
 # CHECK-NEXT:"version": 0
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c":
 {"workingDirectory":"/clangd-test2", "compilationCommand": ["clang", "-c", 
"foo.c", "-Wall", "-Werror"]}
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"C:\\clangd-test\\foo.c":
 {"workingDirectory":"C:\\clangd-test2", "compilationCommand": ["clang", "-c", 
"foo.c", "-Wall", "-Werror"]}
 #  CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
@@ -53,5 +53,3 @@
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
-
-
Index: clang-tools-extra/clangd/test/did-change-configuration-params-posix.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params-posix.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params-posix.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
 # UNSUPPORTED: windows-gnu,windows-msvc
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
@@ -53,5 +53,3 @@
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
-
-
Index: clang-tools-extra/clangd/test/dependency-output-windows.test
===
--- clang-tools-extra/clangd/test/dependency-output-windows.test
+++ clang-tools-extra/clangd/test/dependency-output-wind

[PATCH] D83497: [PowerPC][Power10] Fix VINS* (vector insert byte/half/word) instructions to have i32 arguments.

2020-07-14 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.

LGTM aside from a minor nit regarding the description.




Comment at: clang/include/clang/Basic/BuiltinsPPC.def:324
 // P10 Vector Insert built-ins.
-BUILTIN(__builtin_altivec_vinsblx, "V16UcV16UcULLiULLi", "")
-BUILTIN(__builtin_altivec_vinsbrx, "V16UcV16UcULLiULLi", "")
-BUILTIN(__builtin_altivec_vinshlx, "V8UsV8UsULLiULLi", "")
-BUILTIN(__builtin_altivec_vinshrx, "V8UsV8UsULLiULLi", "")
-BUILTIN(__builtin_altivec_vinswlx, "V4UiV4UiULLiULLi", "")
-BUILTIN(__builtin_altivec_vinswrx, "V4UiV4UiULLiULLi", "")
+BUILTIN(__builtin_altivec_vinsblx, "V16UcV16UcUiUi", "")
+BUILTIN(__builtin_altivec_vinsbrx, "V16UcV16UcUiUi", "")

The description of this review mentions the second argument but you are 
changing the second and third argument. Please fix the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83497



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


[PATCH] D83497: [PowerPC][Power10] Fix VINS* (vector insert byte/half/word) instructions to have i32 arguments.

2020-07-14 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

@rzurob This cannot proceed without your approval since you requested changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83497



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


[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

2020-07-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

Could you add some tests demonstrating the refined diagnostics of the checker?
It would help to validate the quality of the emitted diagnostics.




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNo;
   static const ArgNo Ret;

Shouldn't we use `using` instead?



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323
+std::string("Function argument constraint is not satisfied, ") +
+VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo());
 if (!BT_InvalidArg)

balazske wrote:
> baloghadamsoftware wrote:
> > Instead of `std::string` we usually use `llvm::SmallString` and an 
> > `llvm::raw_svector_ostream` to print into it.
> This would look better?
> "Function argument constraint is not satisfied, constraint: , ArgN: 
> "
Maybe `llvm::Twine` would be a better choice to `llvm::raw_svector_ostream` for 
string concatenations.
docs:
> Twine - A lightweight data structure for efficiently representing the 
> concatenation of temporary values as strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431



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


[PATCH] D83286: [analyzer][solver] Track symbol disequalities

2020-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

@xazax.hun You were interested in performance ⏫

These results here compare this patch together with D82445 
 against **master**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83286



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


[PATCH] D83226: [analyzer] Add system header simulator a symmetric random access iterator operator+

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

Yay!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83226



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


[PATCH] D83053: [clang-tidy] OptionsView::store specialized on bool

2020-07-14 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 28.
njames93 added a comment.

Removed no longer needed change to ProBoundsConstantArrayIndexCheck


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83053

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -18,13 +18,13 @@
 // RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
 // CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
 // CHECK-CHILD4: - key: llvm-qualified-auto.AddConstToQualified
-// CHECK-CHILD4-NEXT: value: '1'
+// CHECK-CHILD4-NEXT: value: 'true'
 // CHECK-CHILD4: - key: modernize-loop-convert.MaxCopySize
 // CHECK-CHILD4-NEXT: value: '20'
 // CHECK-CHILD4: - key: modernize-loop-convert.MinConfidence
 // CHECK-CHILD4-NEXT: value: reasonable
 // CHECK-CHILD4: - key: modernize-use-using.IgnoreMacros
-// CHECK-CHILD4-NEXT: value: '0'
+// CHECK-CHILD4-NEXT: value: 'false'
 
 // RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
 // CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}44{{[/\\]}}.clang-tidy.
@@ -42,7 +42,7 @@
 // CHECK-CHILD5: - key: modernize-loop-convert.MinConfidence
 // CHECK-CHILD5-NEXT: value: reasonable
 // CHECK-CHILD5: - key: modernize-use-using.IgnoreMacros
-// CHECK-CHILD5-NEXT: value: '0'
+// CHECK-CHILD5-NEXT: value: 'false'
 
 // RUN: clang-tidy -dump-config \
 // RUN: --config='{InheritParentConfig: false, \
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -405,9 +405,13 @@
StringRef Value) const;
 
 /// Stores an option with the check-local name \p LocalName with
-/// ``int64_t`` value \p Value to \p Options.
-void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
-   int64_t Value) const;
+/// integer value \p Value to \p Options.
+template 
+std::enable_if_t::value>
+store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+  T Value) const {
+  storeInt(Options, LocalName, Value);
+}
 
 /// Stores an option with the check-local name \p LocalName as the string
 /// representation of the Enum \p Value to \p Options.
@@ -448,6 +452,9 @@
   return Result;
 }
 
+void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+  int64_t Value) const;
+
 static void logErrToStdErr(llvm::Error &&Err);
 
 std::string NamePrefix;
@@ -509,6 +516,13 @@
 bool ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName,
  bool Default) const;
 
+/// Stores an option with the check-local name \p LocalName with
+/// bool value \p Value to \p Options.
+template <>
+void ClangTidyCheck::OptionsView::store(
+ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+bool Value) const;
+
 } // namespace tidy
 } // namespace clang
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -155,12 +155,19 @@
   Options[NamePrefix + LocalName.str()] = Value;
 }
 
-void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
-StringRef LocalName,
-int64_t Value) const {
+void ClangTidyCheck::OptionsView::storeInt(ClangTidyOptions::OptionMap &Options,
+   StringRef LocalName,
+   int64_t Value) const {
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+template <>
+void ClangTidyCheck::OptionsView::store(
+ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+bool Value) const {
+  store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
+}
+
 llvm::Expected
 ClangTidyCheck::OptionsView::getEnumInt(StringRef LocalName,
 ArrayRef Mapping,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82547: [Debugify] Expose debugify (original mode) as CC1 option

2020-07-14 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro updated this revision to Diff 277781.
djtodoro added a comment.
Herald added a subscriber: dang.

- Rebase


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

https://reviews.llvm.org/D82547

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debugify-each-original.c
  llvm/docs/HowToUpdateDebugInfo.rst

Index: llvm/docs/HowToUpdateDebugInfo.rst
===
--- llvm/docs/HowToUpdateDebugInfo.rst
+++ llvm/docs/HowToUpdateDebugInfo.rst
@@ -317,6 +317,16 @@
 
   $ llvm-debugify-original.py sample.json sample.html
 
+The `original` mode can be invoked from front-end level as follows:
+
+.. code-block:: bash
+
+  # Test each pass.
+  $ clang -Xclang -fenable-debugify-each-original -g -O2 sample.c
+
+  # Test each pass and export the issues report into the JSON file.
+  $ clang -Xclang -fenable-debugify-each-original -Xclang -fenable-debugify-original-export=sample.json -g -O2 sample.c
+
 Using ``debugify``
 ^^
 
Index: clang/test/Driver/debugify-each-original.c
===
--- /dev/null
+++ clang/test/Driver/debugify-each-original.c
@@ -0,0 +1,14 @@
+// We support the CC1 options for testing whether each LLVM pass preserves
+// original debug info.
+
+// RUN: %clang -g -Xclang -fenable-debugify-each-original -### %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEBUGIFY %s
+
+// DEBUGIFY: "-fenable-debugify-each-original"
+
+// RUN: %clang -g -Xclang -fenable-debugify-each-original \
+// RUN: -Xclang -fenable-debugify-original-export=%t.json -### %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEBUGIFY-JSON-EXPORT %s
+
+// DEBUGIFY-JSON-EXPORT: "-fenable-debugify-each-original"
+// DEBUGIFY-JSON-EXPORT: "-fenable-debugify-original-export={{.*}}"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -791,6 +791,16 @@
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
+  Opts.EnableDebugifyEachOriginal =
+  Args.hasArg(OPT_fenable_debugify_each_original);
+  // Ignore the option if the -fenable-debugify-each-original wasn't enabled.
+  if (Opts.EnableDebugifyEachOriginal &&
+  Args.hasArg(OPT_fenable_debugify_original_export)) {
+  Opts.DIBugsReportFilePath =
+  std::string(
+  Args.getLastArgValue(OPT_fenable_debugify_original_export));
+  }
+
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -76,6 +76,7 @@
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Utils.h"
 #include "llvm/Transforms/Utils/CanonicalizeAliases.h"
+#include "llvm/Transforms/Utils/Debugify.h"
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
@@ -867,7 +868,14 @@
   if (TM)
 TheModule->setDataLayout(TM->createDataLayout());
 
-  legacy::PassManager PerModulePasses;
+  DebugifyCustomPassManager PerModulePasses;
+  if (CodeGenOpts.EnableDebugifyEachOriginal) {
+PerModulePasses.enableDebugifyEachOriginal();
+
+if (!CodeGenOpts.DIBugsReportFilePath.empty())
+  PerModulePasses.setDebugifyOriginalBugsReportFilePath(
+  CodeGenOpts.DIBugsReportFilePath);
+  }
   PerModulePasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
 
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -389,6 +389,16 @@
 HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
 HelpText<"Disables debug printing for the new pass manager">;
+def fenable_debugify_each_original
+: Flag<["-"], "fenable-debugify-each-original">,
+  HelpText<"Enable Debug Info Metadata preservation testing in "
+   "optimizations.">;
+def fenable_debugify_original_export
+: Joined<["-"], "fenable-debugify-original-export=">,
+  MetaVarName<"">,
+  HelpText<"Export debugify (by testing original Debug Info) failures into "
+   "specified (JSON) file (should be abs path as we use "
+   "append mode to insert new JSON objects

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

2020-07-14 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

I am trying to measure the amount of extra false positive on real code (not 
artificial examples) of this option. The modeling of container size may reduce 
it, of course, but we have no other option to find this bug: even 
loop-unrolling or loop-widening is not be able to recognize the last before the 
last iteration of a loop. I cannot imagine any solution to recognize it, this 
really sounds like voice recognition in the 80's as @NoQ wrote. I do not expect 
such thing in the next 10 years. Refining and enhancing container modeling is 
expected to reduce false positives generally in the iterator checkers and will 
eventually result in moving them out of alpha state.


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

https://reviews.llvm.org/D77150



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


[PATCH] D83755: [clangd] Cache config files for 5 seconds, without revalidating with stat.

2020-07-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, LGTM




Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:24
 
+//
+

looks like an unwated artifact



Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:103
+MTime = Stat->getLastModificationTime();
+updateCacheLocked(*FS, DC);
   }

nit: the rest of this function is also updating the cache now, maybe inline the 
function call?



Comment at: clang-tools-extra/clangd/ConfigProvider.h:41
+  /// If not set, providers should validate caches against the data source.
+  llvm::Optional FreshTime;
 };

i would've suggested storing a duration here instead of time point, but as 
discussed offline this is only done once in the clangdserver and storing a time 
point might help with invalidating the cache based on filewatcher events easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83755



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


[PATCH] D83722: [PowerPC] Add options to control paired vector memops support

2020-07-14 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

Please re-upload this and provide the missing context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83722



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


[PATCH] D83286: [analyzer][solver] Track symbol disequalities

2020-07-14 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D83286#2149912 , @vsavchenko wrote:

> @xazax.hun You were interested in performance ⏫
>
> These results here compare this patch together with D82445 
>  against **master**.


This is really great news, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83286



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


[PATCH] D81442: [PowerPC] Add clang options to control MMA support

2020-07-14 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

Since clang will now add `+/-mma` to the TargetFeatures list, please add a test 
case that specifies `-mattr=+/-mma` to `llc` to show that `llc` accepts it.
Other than that, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81442



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


[PATCH] D67381: [analyzer] NFC: Move stack hints to a side map.

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.
Herald added subscribers: ASDenysPetrov, martong.

Aren't `StackHint`s basically ancient `NoteTag`s?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67381



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


Re: [clang] dcd76c0 - [Lexer] Fix missing coverage line after #endif

2020-07-14 Thread Hans Wennborg via cfe-commits
Nice!

On Fri, Jul 10, 2020 at 6:05 PM Zequan Wu via cfe-commits
 wrote:
>
>
> Author: Zequan Wu
> Date: 2020-07-10T09:05:20-07:00
> New Revision: dcd76c0c0716a4417110423718c7cae4b516b4d0
>
> URL: 
> https://github.com/llvm/llvm-project/commit/dcd76c0c0716a4417110423718c7cae4b516b4d0
> DIFF: 
> https://github.com/llvm/llvm-project/commit/dcd76c0c0716a4417110423718c7cae4b516b4d0.diff
>
> LOG: [Lexer] Fix missing coverage line after #endif
>
> Summary: bug reported here: https://bugs.llvm.org/show_bug.cgi?id=46660
>
> Reviewers: vsk, efriedma, arphaman
>
> Reviewed By: vsk
>
> Subscribers: dexonsmith, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D83514
>
> Added:
>
>
> Modified:
> clang-tools-extra/clangd/SemanticHighlighting.cpp
> clang/lib/Lex/PPDirectives.cpp
> clang/test/CoverageMapping/preprocessor.c
>
> Removed:
>
>
>
> 
> diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
> b/clang-tools-extra/clangd/SemanticHighlighting.cpp
> index d2470da60140..ed75ce80999c 100644
> --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
> +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
> @@ -224,7 +224,7 @@ class HighlightingsBuilder {
>// Create one token for each line in the skipped range, so it works
>// with line-based
> diff ing.
>assert(R.start.line <= R.end.line);
> -  for (int Line = R.start.line; Line < R.end.line; ++Line) {
> +  for (int Line = R.start.line; Line <= R.end.line; ++Line) {
>  // Don't bother computing the offset for the end of the line, just 
> use
>  // zero. The client will treat this highlighting kind specially, and
>  // highlight the entire line visually (i.e. not just to where the 
> text
>
> diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
> index 396ba529fc9a..053ef1d2dd18 100644
> --- a/clang/lib/Lex/PPDirectives.cpp
> +++ b/clang/lib/Lex/PPDirectives.cpp
> @@ -432,6 +432,7 @@ void 
> Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
>  // Skip to the next '#endif' / '#else' / '#elif'.
>  CurLexer->skipOver(*SkipLength);
>}
> +  SourceLocation endLoc;
>while (true) {
>  CurLexer->Lex(Tok);
>
> @@ -538,7 +539,7 @@ void 
> Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
>// Restore the value of LexingRawMode so that trailing comments
>// are handled correctly, if we've reached the outermost block.
>CurPPLexer->LexingRawMode = false;
> -  CheckEndOfDirective("endif");
> +  endLoc = CheckEndOfDirective("endif");
>CurPPLexer->LexingRawMode = true;
>if (Callbacks)
>  Callbacks->Endif(Tok.getLocation(), CondInfo.IfLoc);
> @@ -565,7 +566,7 @@ void 
> Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
>// Restore the value of LexingRawMode so that trailing comments
>// are handled correctly.
>CurPPLexer->LexingRawMode = false;
> -  CheckEndOfDirective("else");
> +  endLoc = CheckEndOfDirective("else");
>CurPPLexer->LexingRawMode = true;
>if (Callbacks)
>  Callbacks->Else(Tok.getLocation(), CondInfo.IfLoc);
> @@ -621,7 +622,9 @@ void 
> Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
>// by the end of the preamble; we'll resume parsing after the preamble.
>if (Callbacks && (Tok.isNot(tok::eof) || !isRecordingPreamble()))
>  Callbacks->SourceRangeSkipped(
> -SourceRange(HashTokenLoc, CurPPLexer->getSourceLocation()),
> +SourceRange(HashTokenLoc, endLoc.isValid()
> +  ? endLoc
> +  : CurPPLexer->getSourceLocation()),
>  Tok.getLocation());
>  }
>
>
> diff  --git a/clang/test/CoverageMapping/preprocessor.c 
> b/clang/test/CoverageMapping/preprocessor.c
> index b3ebc7bd4ec0..9225c9f162a2 100644
> --- a/clang/test/CoverageMapping/preprocessor.c
> +++ b/clang/test/CoverageMapping/preprocessor.c
> @@ -3,7 +3,7 @@
>   // CHECK: func
>  void func() {// CHECK: File 0, [[@LINE]]:13 -> [[@LINE+5]]:2 = #0
>int i = 0;
> -#ifdef MACRO // CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> [[@LINE+3]]:1 
> = 0
> +#ifdef MACRO // CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> [[@LINE+2]]:7 
> = 0
>int x = i;
>  #endif
>  }
> @@ -11,7 +11,7 @@ void func() {// CHECK: File 0, [[@LINE]]:13 -> 
> [[@LINE+5]]:2 = #0
>   // CHECK: main
>  int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
>int i = 0;
> -#  if 0// CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> 
> [[@LINE+5]]:1 = 0
> +#  if 0// CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> 
> [[@LINE+4]]:29 = 0
>if(i == 0) {
>  i = 1;
>}
> @@ -22,44 +2

[PATCH] D83723: [OpenMP] Generalize CGOpenMPRuntimeNVPTX as CGOpenMPRuntimeGPU

2020-07-14 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp:39
+  // return constant compile-time target-specific warp size
+  unsigned WarpSize = CGF.getTarget().getGridValue(llvm::omp::GV_Warp_Size);
+  return Bld.getInt32(WarpSize);

ABataev wrote:
> This is new functionality, better to move it in a separate patch, and this 
> one mark as NFC.
Works for me. This patch shows how the per-target parts are intended to be 
done. First patch being totally NFC seems good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83723



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


[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus marked 2 inline comments as done.
Szelethus added a comment.
This revision is now accepted and ready to land.

LGTM! Very well done!

In D67420#2149461 , @vsavchenko wrote:

> I strongly believe that decoupling (when done right) makes things easier and 
> this change seems like a good first step.


This patch I think categorizes, rather. We could do a much better job at that 
with these options, I agree, if we switched over to TableGen 
(D83475#inline-768468 ). The 
problem is, its hard to justify the engineering hours for an arguably 
negligible gain.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:47-48
+
+void printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
+  const PathPieces &Path);
+

Nice.



Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:39-42
-  const bool IncludePath = false;
-  const bool ShouldEmitAsError = false;
-  const bool ApplyFixIts = false;
-  const bool ShouldDisplayCheckerName = false;

Really nice!


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

https://reviews.llvm.org/D67420



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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Endre Fülöp via Phabricator via cfe-commits
gamesh411 updated this revision to Diff 277796.
gamesh411 added a comment.

extend with notes
apply minor fixes
tests are WIP until I figure out how to properly use file-check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp
  clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-env32-c.c
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/cert/BUILD.gn
@@ -19,6 +19,7 @@
 "CommandProcessorCheck.cpp",
 "DefaultOperatorNewAlignmentCheck.cpp",
 "DontModifyStdNamespaceCheck.cpp",
+"ExitHandlerCheck.cpp",
 "FloatLoopCounter.cpp",
 "LimitedRandomnessCheck.cpp",
 "MutatingCopyCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/cert-env32-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-env32-c.c
@@ -0,0 +1,324 @@
+// RUN: %check_clang_tidy %s cert-env32-c %t
+
+// --
+// EXIT FUNCTIONS
+// --
+
+// No handlers are invoked when _Exit is called.
+void _Exit(int __status);
+
+// Handlers registered by atexit are invoked in reverse order when exit is
+// called.
+void exit(int __status);
+
+// Handlers registered by at_quick_exit are invoked in reverse order when
+// quick_exit is called.
+void quick_exit(int __status);
+
+// 
+// HANDLER REGISTRATION
+// 
+
+// Register handlers to run when exit is called.
+int atexit(void (*__func)(void));
+
+// Register handlers to run when exit is called.
+int at_quick_exit(void (*__func)(void));
+
+// --
+// Setjmp/longjmp
+// --
+// C99 requires jmp_buf to be an array type.
+typedef int jmp_buf[1];
+int setjmp(jmp_buf);
+void longjmp(jmp_buf, int);
+
+// Compliant solutions
+
+void cleanup1() {
+  // do cleanup
+}
+
+void cleanup2() {
+  // do cleanup
+}
+
+void test_atexit_single_compliant() {
+  (void)atexit(cleanup1);
+}
+
+void test_atexit_multiple_compliant() {
+  (void)atexit(cleanup1);
+  (void)atexit(cleanup2);
+}
+
+void test_at_quick_exit_single_compliant() {
+  (void)at_quick_exit(cleanup1);
+}
+
+void test_at_quick_exit_multiple_compliant() {
+  (void)at_quick_exit(cleanup1);
+  (void)at_quick_exit(cleanup2);
+}
+
+// Non-compliant solutions calling _Exit
+
+// CHECK-NOTES: :[[@LINE+2]]:1: note: handler function declared here
+// CHECK-NOTES: :[[@LINE+1]]:1: note: handler function declared here
+void call__Exit() {
+  _Exit(0);
+  // CHECK-NOTES: :[[@LINE-1]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-3]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-4]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-5]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-6]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-7]]:3: note: exit function called here
+  // CHECK-NOTES: :[[@LINE-8]]:3: note: exit function called here
+}
+
+// CHECK-NOTES: :[[@LINE+2]]:1: note: handler function declared here
+// CHECK-NOTES: :[[@LINE+1]]:1: note: handler function declared here
+void call_call__Exit() {
+  call__Exit();
+}
+
+extern int unknown__Exit_flag;
+
+// CHECK-NOTES: :[[@LINE+2]]:1: note: handler function declared here
+// CHECK-NOTES: :[[@LINE+1]]:1: note: handler function declared here
+void call__Exit_conditionally() {
+  if (unknown__Exit_flag)
+call__Exit();
+}
+
+// CHECK-NOTES: :[[@LINE+2]]:1: note: handler function declared here
+// CHECK-NOTES: :[[@LINE+1]]:1: note: handler function declared here
+void call_call__Exit_conditionally() {
+  call__Exit_conditionally();
+}
+
+void test__Exit_called_directly() {
+  (void)atexit(call__Exit);
+  // CHECK-NOTES: :[[@LINE-1]]:9: warning: exit-handler potentially calls an exit function. Handlers should terminate by returning [cert-env32-c]
+  (void)at_quick_exit(call__Exit);
+  // CHECK-NOTES: :[[@LINE-1]]:9: warning: exit-handler potentially calls an exit function. Handlers should terminate by returning [cert-env32-c]
+};
+
+void test__Exit_called_indirectly() {
+  (void)atexit(call_call__Exit);
+  // CHECK-NOTES: :[[@LINE-1]]:9: warning: exit-handler potentially calls an exit function. Handlers should terminate by returning [cert-env32-

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Endre Fülöp via Phabricator via cfe-commits
gamesh411 marked 10 inline comments as done.
gamesh411 added a comment.

Thanks @njames93 :) I have extended the check with notes, but I have to figure 
out how to appease file-check, so its still WIP until I do.




Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:72
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  // clang-format off
+  Finder->addMatcher(

njames93 wrote:
> Is there a reason  for disabling format here? How messy is the matcher code 
> when formatted?
It is not that bad after all. Especially with the `hasAnyName` predicate.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:120-122
+if (SeenFunctions.count(Current))
+  continue;
+SeenFunctions.insert(Current);

njames93 wrote:
> nit:
> ```
> if (!SeenFunctions.insert(Current).second)
>   continue;
> ```
Neat!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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


[PATCH] D83497: [PowerPC][Power10] Fix VINS* (vector insert byte/half/word) instructions to have i32 arguments.

2020-07-14 Thread Rafik Zurob via Phabricator via cfe-commits
rzurob accepted this revision.
rzurob added a comment.
This revision is now accepted and ready to land.

LGTM.  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83497



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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

Unfortunately, I'm not qualified enough to have much to say.




Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:1
+//===--- Env32CCheck.cpp - clang-tidy 
-===//
+//

Env32CCheck.cpp -> ExitHandlerCheck.cpp



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:38
+bool isExitFunction(StringRef FnName) {
+  return FnName == EF__Exit || FnName == EF_exit || FnName == EF_quick_exit;
+}

If `EF__Exit`, `EF_exit` and `EF_quick_exit` referred only once, we could 
simply inline those,

Same for the `isJumpFunction`.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:60
+  /// Iteration over the collector is iteration over the found FunctionDecls.
+  auto begin() const -> decltype(CalledFunctions.begin()) {
+return CalledFunctions.begin();

IMO `-> decltype(CalledFunctions.begin())` is unnecessary.
Return type deduction does the right thing for this case.

Same for the `auto end() ...`



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:84
+  hasArgument(
+0, // the first argument is the handler function
+declRefExpr(

If you disabled clang-format for having inline comments, Then you could create 
smaller matchers and give names for them.

Something similar to this:
```lang=cpp
const auto DesciptiveName = hasArgument(0, 
declRefExpr(hasDeclaration(functionDecl().bind("handler_decl";
```



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:132
+   "terminate by returning");
+  break;
+}

Why don't we `return` here?
Same for the next `break`.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:148-154
+// Collect all the referenced FunctionDecls.
+Collector.TraverseStmt(CurrentDefWithBody->getBody());
+// Add called functions to the worklist.
+std::copy(Collector.begin(), Collector.end(),
+  std::back_inserter(CalledFunctions));
+// Reset the ASTVisitor instance results.
+Collector.clear();

nit: I would clear the `Collector` before the `TraverseStmt`.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.h:20
+/// Checker for SEI CERT rule ENV32-C
+/// All exit handlers must return normal.
+/// Exit handlers must terminate by returning. It is important and potentially

In the documentation you use the:
clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst
> All exit handlers must return normally.

You should be consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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


[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.

still lg.




Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:197
+}
+
+TEST(ArgStripperTest, Spellings) {

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > hokein wrote:
> > > > add tests for stripping the diagnostic flags, `-Wfoo` etc.
> > > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > > arg.
> > > Do you want a test specifically for such a string anyway?
> > > Or do you want special behavior for them? (Like interaction between -Wfoo 
> > > and -Wno-foo)
> > I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> > would be nice to have them in tests as I think these are important cases.
> > 
> > > Those aren't actually separate flags: "-W" is a Joined flag and foo is an 
> > > arg.
> > 
> > oh, if I understand correctly:
> > 
> > - `strip("unused", "clang -Wunused foo.cc")` => `clang foo.cc` ?
> > - `strip("-W", "clang -Wunused -Wextra foo.cc")` => `clang foo.cc` // 
> > remove all -W flags ?
> > - `strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> > foo.cc")`  =>  `clang -Wunused -Werror=date-time foo.cc`?
> > 
> > I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> > would be nice to have them in tests as I think these are important cases.
> 
> Added.
> 
> > strip("unused", "clang -Wunused foo.cc") => clang foo.cc ?
> 
> No, we only strip clang args or flag names, not flag args (confusing 
> terminology).
> -W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang arg). 
> -W* will strip -Wunused (clang arg). but "unused" doesn't match - it's not 
> the name of a flag, and it's not the arg either.
> 
> > strip("-W", "clang -Wunused -Wextra foo.cc") => clang foo.cc // remove all 
> > -W flags ?
> 
> Yes
> 
> > strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> > foo.cc") => clang -Wunused -Werror=date-time foo.cc?
> 
> No, again we don't strip flag args.
> (It's not clear how useful/easy to use this would be, maybe we can re-examine 
> later?)
> No, we only strip clang args or flag names, not flag args (confusing 
> terminology).
> -W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang arg). 
> -W* will strip -Wunused (clang arg). but "unused" doesn't match - it's not 
> the name of a flag, and it's not the arg either.

oh, I was confused by these terms :(

> No, again we don't strip flag args.
> (It's not clear how useful/easy to use this would be, maybe we can re-examine 
> later?)

we can still strip it by passing "-Werror-unused" arg, right? if so, I think it 
should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958



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


[clang] d083adb - Prohibit use of _ExtInt in atomic intrinsic

2020-07-14 Thread Erich Keane via cfe-commits

Author: Mott, Jeffrey T
Date: 2020-07-14T06:11:04-07:00
New Revision: d083adb068e781a2fc35aea8c6b7cccd566a735f

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

LOG: Prohibit use of _ExtInt in atomic intrinsic

The _ExtInt type allows custom width integers, but the atomic memory
access's operand must have a power-of-two size. _ExtInts with
non-power-of-two size should not be allowed for atomic intrinsic.

Before this change:

$ cat test.c

typedef unsigned _ExtInt(42) dtype;
void verify_binary_op_nand(dtype* pval1, dtype val2)
{__sync_nand_and_fetch(pval1, val2); }

$ clang test.c

clang-11:
/home/ubuntu/llvm_workspace/llvm/clang/lib/CodeGen/CGBuiltin.cpp:117:
llvm::Value*
EmitToInt(clang::CodeGen::CodeGenFunction&, llvm::Value*,
clang::QualType, llvm::IntegerType*): Assertion `V->getType() ==
IntType' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the
crash backtrace, preprocessed source, and associated run script.

After this change:

$ clang test.c

test.c:3:30: error: Atomic memory operand must have a power-of-two size
{__sync_nand_and_fetch(pval1, val2); }
^

List of the atomic intrinsics that have this
problem:

__sync_fetch_and_add
__sync_fetch_and_sub
__sync_fetch_and_or
__sync_fetch_and_and
__sync_fetch_and_xor
__sync_fetch_and_nand
__sync_nand_and_fetch
__sync_and_and_fetch
__sync_add_and_fetch
__sync_sub_and_fetch
__sync_or_and_fetch
__sync_xor_and_fetch
__sync_fetch_and_min
__sync_fetch_and_max
__sync_fetch_and_umin
__sync_fetch_and_umax
__sync_val_compare_and_swap
__sync_bool_compare_and_swap

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/builtins.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 71517edd6659..aa4de2812312 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7939,6 +7939,8 @@ def err_atomic_builtin_pointer_size : Error<
 def err_atomic_exclusive_builtin_pointer_size : Error<
   "address argument to load or store exclusive builtin must be a pointer to"
   " 1,2,4 or 8 byte type (%0 invalid)">;
+def err_atomic_builtin_ext_int_size : Error<
+  "Atomic memory operand must have a power-of-two size">;
 def err_atomic_op_needs_atomic : Error<
   "address argument to atomic operation must be a pointer to _Atomic "
   "type (%0 invalid)">;

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index efaf36a69306..509d88e25000 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5349,6 +5349,15 @@ Sema::SemaBuiltinAtomicOverloaded(ExprResult 
TheCallResult) {
   // gracefully.
   TheCall->setType(ResultType);
 
+  // Prohibit use of _ExtInt with atomic builtins.
+  // The arguments would have already been converted to the first argument's
+  // type, so only need to check the first argument.
+  const auto *ExtIntValType = ValType->getAs();
+  if (ExtIntValType && !llvm::isPowerOf2_64(ExtIntValType->getNumBits())) {
+Diag(FirstArg->getExprLoc(), diag::err_atomic_builtin_ext_int_size);
+return ExprError();
+  }
+
   return TheCallResult;
 }
 

diff  --git a/clang/test/Sema/builtins.c b/clang/test/Sema/builtins.c
index 1d41bcf9f086..90c033e47cd1 100644
--- a/clang/test/Sema/builtins.c
+++ b/clang/test/Sema/builtins.c
@@ -281,6 +281,42 @@ void test21(const int *ptr) {
   __atomic_fetch_add(ptr, 1, 0);  // expected-error {{address argument to 
atomic operation must be a pointer to non-const type ('const int *' invalid)}}
 }
 
+void test_ei_i42i(_ExtInt(42) *ptr, int value) {
+  __sync_fetch_and_add(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+}
+
+void test_ei_i64i(_ExtInt(64) *ptr, int value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
+void test_ei_ii42(int *ptr, _ExtInt(42) value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
+void test_ei_ii64(int *ptr, _ExtInt(64) value) {
+  __sync_fet

[PATCH] D83340: Prohibit use of _ExtInt in atomic intrinsic

2020-07-14 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd083adb068e7: Prohibit use of _ExtInt in atomic intrinsic 
(authored by jtmott-intel, committed by erichkeane).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83340

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtins.c


Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -281,6 +281,42 @@
   __atomic_fetch_add(ptr, 1, 0);  // expected-error {{address argument to 
atomic operation must be a pointer to non-const type ('const int *' invalid)}}
 }
 
+void test_ei_i42i(_ExtInt(42) *ptr, int value) {
+  __sync_fetch_and_add(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+}
+
+void test_ei_i64i(_ExtInt(64) *ptr, int value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
+void test_ei_ii42(int *ptr, _ExtInt(42) value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
+void test_ei_ii64(int *ptr, _ExtInt(64) value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
+void test_ei_i42i42(_ExtInt(42) *ptr, _ExtInt(42) value) {
+  __sync_fetch_and_add(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expected-error {{Atomic memory operand 
must have a power-of-two size}}
+}
+
+void test_ei_i64i64(_ExtInt(64) *ptr, _ExtInt(64) value) {
+  __sync_fetch_and_add(ptr, value); // expect success
+  // expected-warning@+1 {{the semantics of this intrinsic changed with GCC 
version 4.4 - the newer semantics are provided here}}
+  __sync_nand_and_fetch(ptr, value); // expect success
+}
+
 void test22(void) {
   (void)__builtin_signbit(); // expected-error{{too few arguments to function 
call, expected 1, have 0}}
   (void)__builtin_signbit(1.0, 2.0, 3.0); // expected-error{{too many 
arguments to function call, expected 1, have 3}}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5349,6 +5349,15 @@
   // gracefully.
   TheCall->setType(ResultType);
 
+  // Prohibit use of _ExtInt with atomic builtins.
+  // The arguments would have already been converted to the first argument's
+  // type, so only need to check the first argument.
+  const auto *ExtIntValType = ValType->getAs();
+  if (ExtIntValType && !llvm::isPowerOf2_64(ExtIntValType->getNumBits())) {
+Diag(FirstArg->getExprLoc(), diag::err_atomic_builtin_ext_int_size);
+return ExprError();
+  }
+
   return TheCallResult;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7939,6 +7939,8 @@
 def err_atomic_exclusive_builtin_pointer_size : Error<
   "address argument to load or store exclusive builtin must be a pointer to"
   " 1,2,4 or 8 byte type (%0 invalid)">;
+def err_atomic_builtin_ext_int_size : Error<
+  "Atomic memory operand must have a power-of-two size">;
 def err_atomic_op_needs_atomic : Error<
   "address argument to atomic operation must be a pointer to _Atomic "
   "type (%0 invalid)">;


Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -281,6 +281,42 @@
   __atomic_fetch_add(ptr, 1, 0);  // expected-error {{address argument to atomic operation must be a pointer to non-const type ('const int *' invalid)}}
 }
 
+void test_ei_i42i(_ExtInt(42) *ptr, int value) {
+  __sync_fetch_and_add(ptr, value); // expected-error {{Atomic

[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

2020-07-14 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb updated this revision to Diff 277806.
krisb edited the summary of this revision.
krisb added a comment.

Changed MSVC -> WIN32 check and simplified the warning fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80873

Files:
  clang/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -261,7 +261,12 @@
   if ( LLVM_USE_LINKER )
 message(FATAL_ERROR "LLVM_ENABLE_LLD and LLVM_USE_LINKER can't be set at 
the same time")
   endif()
-  set(LLVM_USE_LINKER "lld")
+  # In case of MSVC cmake always invokes the linker directly, so the linker
+  # should be specified by CMAKE_LINKER cmake variable instead of by -fuse-ld
+  # compiler option.
+  if ( NOT MSVC )
+set(LLVM_USE_LINKER "lld")
+  endif()
 endif()
 
 if( LLVM_USE_LINKER )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -745,6 +745,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified if the compiler is MSVC-like,
+  # otherwise it defaults the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(WIN32 AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG 
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -261,7 +261,12 @@
   if ( LLVM_USE_LINKER )
 message(FATAL_ERROR "LLVM_ENABLE_LLD and LLVM_USE_LINKER can't be set at the same time")
   endif()
-  set(LLVM_USE_LINKER "lld")
+  # In case of MSVC cmake always invokes the linker directly, so the linker
+  # should be specified by CMAKE_LINKER cmake variable instead of by -fuse-ld
+  # compiler option.
+  if ( NOT MSVC )
+set(LLVM_USE_LINKER "lld")
+  endif()
 endif()
 
 if( LLVM_USE_LINKER )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -745,6 +745,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified if the compiler is MSVC-like,
+  # otherwise it defaults the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(WIN32 AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER -DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.
Herald added subscribers: ASDenysPetrov, martong.



Comment at: clang/include/clang/Analysis/IssueHash.h:35
+///
 /// In case a new hash is introduced, the old one should still be maintained 
for
 /// a while. One should not introduce a new hash for every change, it is

gribozavr wrote:
> I don't understand what this paragraph is talking about at all. What does it 
> mean for a new hash to be introduced? As in, when this hashing algorithm 
> changes?
It might refer to the extreme headaches issue hash changes can cause. In 
D77866,a block of comments in `MallocChecker.cpp` talks about this, and some 
other discussion (D77866#2068394) in the patch as well.


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

https://reviews.llvm.org/D67421



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


[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.
Herald added subscribers: ASDenysPetrov, martong.

In D67422#1667079 , @NoQ wrote:

> Casually  rename 
> `ClangDiagPathDiagConsumer` to `TextDiagnostics` according to the local 
> naming tradition.


Yoink.  You could be facing a rebasing 
nightmare here :)


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

https://reviews.llvm.org/D67422



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


[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

We only support Build/Skip for now, but with 'Load' or similar as an
option for future (load existing shards but don't build new ones).

This requires creating the config for each TU on startup. In LLVM, this
is 4000 occurrences for a total of 800ms on my machine.
But together with caching from D83755  it is 
only 25ms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83768

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -98,6 +98,22 @@
   EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
 }
 
+TEST_F(ConfigCompileTests, Index) {
+  Frag.Index.Background.emplace("Skip");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+
+  Frag = {};
+  Frag.Index.Background.emplace("Foo");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build)
+  << "by default";
+  EXPECT_THAT(
+  Diags.Diagnostics,
+  ElementsAre(DiagMessage(
+  "Invalid Background value 'Foo'. Valid values are Build, Skip.")));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -113,7 +113,7 @@
   // Set up two identical TUs, foo and bar.
   // They define foo::one and bar::one.
   std::vector Cmds;
-  for (std::string Name : {"foo", "bar"}) {
+  for (std::string Name : {"foo", "bar", "baz"}) {
 std::string Filename = Name + ".cpp";
 std::string Header = Name + ".h";
 FS.Files[Filename] = "#include \"" + Header + "\"";
@@ -126,11 +126,14 @@
   }
   // Context provider that installs a configuration mutating foo's command.
   // This causes it to define foo::two instead of foo::one.
+  // It also disables indexing of baz entirely.
   auto ContextProvider = [](PathRef P) {
 Config C;
 if (P.endswith("foo.cpp"))
   C.CompileFlags.Edits.push_back(
   [](std::vector &Argv) { Argv.push_back("-Done=two"); });
+if (P.endswith("baz.cpp"))
+  C.Index.Background = Config::BackgroundPolicy::Skip;
 return Context::current().derive(Config::Key, std::move(C));
   };
   // Create the background index.
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -8,6 +8,7 @@
 
 #include "index/Background.h"
 #include "Compiler.h"
+#include "Config.h"
 #include "Headers.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
@@ -354,6 +355,14 @@
 // staleness.
 std::vector
 BackgroundIndex::loadProject(std::vector MainFiles) {
+  // Drop files where background indexing is disabled in config.
+  if (ContextProvider)
+llvm::erase_if(MainFiles, [&](const std::string &TU) {
+  // Load the config for each TU, as indexing may be selectively enabled.
+  WithContext WithProvidedContext(ContextProvider(TU));
+  return Config::current().Index.Background ==
+ Config::BackgroundPolicy::Skip;
+});
   Rebuilder.startLoading();
   // Load shards for all of the mainfiles.
   const std::vector Result =
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "ConfigFragment.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -66,6 +67,13 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::IndexBlock &F, Node &N) {
+DictParser Dict("Index", this);
+Dict.handle("Background",
+[&](Node &N) { F.Background = scalarValue(N, "Background"); });
+Dict.parse(N);
+  }
+
   // Helper for parsing mapping nodes (dictionaries).
   // We don't use YamlIO as we want to control over unknown keys.
   class Dic

[PATCH] D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:328-329
   struct Signature {
-const ArgTypes ArgTys;
-const QualType RetTy;
+ArgTypes ArgTys;
+QualType RetTy;
 Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {

martong wrote:
> Szelethus wrote:
> > Ah right, because we need to copy this. Shame that `Optional` can't just 
> > inplace construct the object with a copy constructor or something.
> The problem is rather with the copy assignment. I think, copy construction 
> could be implemented if we keep the `const`, but assignment is not.
Yes, but copy assignment happens because out optional isn't optimal (hehe) 
http://lists.llvm.org/pipermail/cfe-dev/2018-July/058427.html. Anyways, it is 
what it is. You could add a comment about this, but it might not be worthwhile. 



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1746
+if (StructSockaddrPtrRestrictTy && Socklen_tPtrRestrictTy) {
+  auto Accept = Summary(NoEvalCall)
+.ArgConstraint(ArgumentCondition(0, WithinRange,

martong wrote:
> Szelethus wrote:
> > `AcceptSummary`?
> I prefer simply `Accept` because right after the `=` sign we have the 
> `Summary` string :)
Fair enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83407



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


[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG8eb8c92eb469: [clangd] Add library to semantically strip 
flags by name. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D81958?vs=277728&id=277810#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958

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

Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -207,6 +207,166 @@
   EXPECT_THAT(Cmd, ElementsAre(_, "FOO.CC", "--hello", "-fsyntax-only"));
 }
 
+static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
+  llvm::SmallVector Parts;
+  llvm::SplitString(Argv, Parts);
+  std::vector Args = {Parts.begin(), Parts.end()};
+  ArgStripper S;
+  S.strip(Arg);
+  S.process(Args);
+  return llvm::join(Args, " ");
+}
+
+TEST(ArgStripperTest, Spellings) {
+  // May use alternate prefixes.
+  EXPECT_EQ(strip("-pedantic", "clang -pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-pedantic", "clang --pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--pedantic", "clang -pedantic foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--pedantic", "clang --pedantic foo.cc"), "clang foo.cc");
+  // May use alternate names.
+  EXPECT_EQ(strip("-x", "clang -x c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-x", "clang --language=c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--language=", "clang -x c++ foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("--language=", "clang --language=c++ foo.cc"),
+"clang foo.cc");
+}
+
+TEST(ArgStripperTest, UnknownFlag) {
+  EXPECT_EQ(strip("-xyzzy", "clang -xyzzy foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-xyz*", "clang -xyzzy foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-xyzzy", "clang -Xclang -xyzzy foo.cc"), "clang foo.cc");
+}
+
+TEST(ArgStripperTest, Xclang) {
+  // Flags may be -Xclang escaped.
+  EXPECT_EQ(strip("-ast-dump", "clang -Xclang -ast-dump foo.cc"),
+"clang foo.cc");
+  // Args may be -Xclang escaped.
+  EXPECT_EQ(strip("-add-plugin", "clang -Xclang -add-plugin -Xclang z foo.cc"),
+"clang foo.cc");
+}
+
+TEST(ArgStripperTest, ClangCL) {
+  // /I is a synonym for -I in clang-cl mode only.
+  // Not stripped by default.
+  EXPECT_EQ(strip("-I", "clang -I /usr/inc /Interesting/file.cc"),
+"clang /Interesting/file.cc");
+  // Stripped when invoked as clang-cl.
+  EXPECT_EQ(strip("-I", "clang-cl -I /usr/inc /Interesting/file.cc"),
+"clang-cl");
+  // Stripped when invoked as CL.EXE
+  EXPECT_EQ(strip("-I", "CL.EXE -I /usr/inc /Interesting/file.cc"), "CL.EXE");
+  // Stripped when passed --driver-mode=cl.
+  EXPECT_EQ(strip("-I", "cc -I /usr/inc /Interesting/file.cc --driver-mode=cl"),
+"cc --driver-mode=cl");
+}
+
+TEST(ArgStripperTest, ArgStyles) {
+  // Flag
+  EXPECT_EQ(strip("-Qn", "clang -Qn foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-Qn", "clang -QnZ foo.cc"), "clang -QnZ foo.cc");
+  // Joined
+  EXPECT_EQ(strip("-std=", "clang -std= foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-std=", "clang -std=c++11 foo.cc"), "clang foo.cc");
+  // Separate
+  EXPECT_EQ(strip("-mllvm", "clang -mllvm X foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-mllvm", "clang -mllvmX foo.cc"), "clang -mllvmX foo.cc");
+  // RemainingArgsJoined
+  EXPECT_EQ(strip("/link", "clang-cl /link b c d foo.cc"), "clang-cl");
+  EXPECT_EQ(strip("/link", "clang-cl /linka b c d foo.cc"), "clang-cl");
+  // CommaJoined
+  EXPECT_EQ(strip("-Wl,", "clang -Wl,x,y foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-Wl,", "clang -Wl, foo.cc"), "clang foo.cc");
+  // MultiArg
+  EXPECT_EQ(strip("-segaddr", "clang -segaddr a b foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-segaddr", "clang -segaddra b foo.cc"),
+"clang -segaddra b foo.cc");
+  // JoinedOrSeparate
+  EXPECT_EQ(strip("-G", "clang -GX foo.cc"), "clang foo.cc");
+  EXPECT_EQ(strip("-G", "clang -G X foo.cc"), "clang foo.cc");
+  // JoinedAndSeparate
+  EXPECT_EQ(strip("-plugin-arg-", "clang -cc1 -plugin-arg-X Y foo.cc"),
+"clang -cc1 foo.cc");
+  EXPECT_EQ(strip("-plugin-arg-", "clang -cc1 -plugin-arg- Y foo.cc"),
+"clang -cc1 foo.cc");
+}
+
+TEST(ArgStripperTest, EndOfList) {
+  // When we hit the end-of-args prematurely, we don't crash.
+  // We consume the incomplete args if we've matched the target option.
+  EXPECT_EQ(strip("-I", "clang -Xclang"), "clang -Xclang");
+  EXPECT_EQ(strip("-I", "clang -Xclang -I"), "clang");
+  EXPECT_EQ(strip("-I", "clang -I -Xclang"), "clang");
+  EXPECT_EQ(strip

[clang-tools-extra] 8eb8c92 - [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-14T15:41:46+02:00
New Revision: 8eb8c92eb46908ee9c64dfc4a2f49501b085f682

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

LOG: [clangd] Add library to semantically strip flags by name.

Summary:
This is designed for tweaking compile commands by specifying flags to add/remove
in a config file. Something like:
  CompileFlags: { Remove: -fcolor-diagnostics }

Having users tweak raw argv (e.g. with a regex) is going to end in tears: bugs
around clang-cl, xclang, aliases, joined-vs-separate args etc are inevitable.

This isn't in tooling because of the performance choices: build a big table
up-front to make subsequent actions fast. Maybe it should be though.

Reviewers: adamcz, hokein

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 4b6955576942..473122157cac 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -9,8 +9,12 @@
 #include "CompileCommands.h"
 #include "Config.h"
 #include "support/Logger.h"
+#include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -234,5 +238,269 @@ CommandMangler::operator 
clang::tooling::ArgumentsAdjuster() && {
   };
 }
 
+// ArgStripper implementation
+namespace {
+
+// Determine total number of args consumed by this option.
+// Return answers for {Exact, Prefix} match. 0 means not allowed.
+std::pair getArgCount(const llvm::opt::Option &Opt) {
+  constexpr static unsigned Rest = 1; // Should be all the rest!
+  // Reference is llvm::opt::Option::acceptInternal()
+  using llvm::opt::Option;
+  switch (Opt.getKind()) {
+  case Option::FlagClass:
+return {1, 0};
+  case Option::JoinedClass:
+  case Option::CommaJoinedClass:
+return {1, 1};
+  case Option::GroupClass:
+  case Option::InputClass:
+  case Option::UnknownClass:
+  case Option::ValuesClass:
+return {1, 0};
+  case Option::JoinedAndSeparateClass:
+return {2, 2};
+  case Option::SeparateClass:
+return {2, 0};
+  case Option::MultiArgClass:
+return {1 + Opt.getNumArgs(), 0};
+  case Option::JoinedOrSeparateClass:
+return {2, 1};
+  case Option::RemainingArgsClass:
+return {Rest, 0};
+  case Option::RemainingArgsJoinedClass:
+return {Rest, Rest};
+  }
+}
+
+// Flag-parsing mode, which affects which flags are available.
+enum DriverMode : unsigned char {
+  DM_None = 0,
+  DM_GCC = 1, // Default mode e.g. when invoked as 'clang'
+  DM_CL = 2,  // MS CL.exe compatible mode e.g. when invoked as 'clang-cl'
+  DM_CC1 = 4, // When invoked as 'clang -cc1' or after '-Xclang'
+  DM_All = 7
+};
+
+// Examine args list to determine if we're in GCC, CL-compatible, or cc1 mode.
+DriverMode getDriverMode(const std::vector &Args) {
+  DriverMode Mode = DM_GCC;
+  llvm::StringRef Argv0 = Args.front();
+  if (Argv0.endswith_lower(".exe"))
+Argv0 = Argv0.drop_back(strlen(".exe"));
+  if (Argv0.endswith_lower("cl"))
+Mode = DM_CL;
+  for (const llvm::StringRef Arg : Args) {
+if (Arg == "--driver-mode=cl") {
+  Mode = DM_CL;
+  break;
+}
+if (Arg == "-cc1") {
+  Mode = DM_CC1;
+  break;
+}
+  }
+  return Mode;
+}
+
+// Returns the set of DriverModes where an option may be used.
+unsigned char getModes(const llvm::opt::Option &Opt) {
+  // Why is this so complicated?!
+  // Reference is clang::driver::Driver::getIncludeExcludeOptionFlagMasks()
+  unsigned char Result = DM_None;
+  if (Opt.hasFlag(driver::options::CC1Option))
+Result |= DM_CC1;
+  if (!Opt.hasFlag(driver::options::NoDriverOption)) {
+if (Opt.hasFlag(driver::options::CLOption)) {
+  Result |= DM_CL;
+} else {
+  Result |= DM_GCC;
+  if (Opt.hasFlag(driver::options::CoreOption)) {
+Result |= DM_CL;
+  }
+}
+  }
+  return Result;
+};
+
+} // namespace
+
+llvm::ArrayRef ArgStripper::rulesFor(llvm::StringRef Arg) {
+  // All the hard work is done once in a static initializer.
+  // We compute a table containing strings to look for and #args to skip.
+  // e.g. "-x" => {-x 2 args, -x* 1 arg, --language 2 args, --language=* 1 arg}
+  using Tabl

[clang] 8978032 - Fix test for the hasExternalFormalLinkage matcher

2020-07-14 Thread Dmitri Gribenko via cfe-commits

Author: Dmitri Gribenko
Date: 2020-07-14T15:44:53+02:00
New Revision: 8978032a17cd0f1c3925ecb1752fdf59de7f7967

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

LOG: Fix test for the hasExternalFormalLinkage matcher

Summary:
Names of local variables have no linkage (see C++20 [basic.link] p8).

Names of variables in unnamed namespace have internal linkage (see C++20
[basic.link] p4).

Reviewers: aaron.ballman, rsmith, ymandel

Reviewed By: ymandel

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Removed: 




diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 687908043a8d..c249410201ba 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2534,19 +2534,16 @@ TEST(NullPointerConstants, Basic) {
 }
 
 TEST(HasExternalFormalLinkage, Basic) {
-  EXPECT_TRUE(matches("int a = 0;", namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(
-  notMatches("static int a = 0;", namedDecl(hasExternalFormalLinkage(;
+  EXPECT_TRUE(matches("int a = 0;",
+  namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("static int a = 0;",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
   EXPECT_TRUE(notMatches("static void f(void) { int a = 0; }",
- namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(matches("void f(void) { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
-
-  // Despite having internal semantic linkage, the anonymous namespace member
-  // has external linkage because the member has a unique name in all
-  // translation units.
-  EXPECT_TRUE(matches("namespace { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("void f(void) { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("namespace { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
 }
 
 TEST(HasDefaultArgument, Basic) {



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


[PATCH] D83700: Fix test for the hasExternalFormalLinkage matcher

2020-07-14 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8978032a17cd: Fix test for the hasExternalFormalLinkage 
matcher (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83700

Files:
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2534,19 +2534,16 @@
 }
 
 TEST(HasExternalFormalLinkage, Basic) {
-  EXPECT_TRUE(matches("int a = 0;", namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(
-  notMatches("static int a = 0;", namedDecl(hasExternalFormalLinkage(;
+  EXPECT_TRUE(matches("int a = 0;",
+  namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("static int a = 0;",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
   EXPECT_TRUE(notMatches("static void f(void) { int a = 0; }",
- namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(matches("void f(void) { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
-
-  // Despite having internal semantic linkage, the anonymous namespace member
-  // has external linkage because the member has a unique name in all
-  // translation units.
-  EXPECT_TRUE(matches("namespace { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("void f(void) { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("namespace { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
 }
 
 TEST(HasDefaultArgument, Basic) {


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2534,19 +2534,16 @@
 }
 
 TEST(HasExternalFormalLinkage, Basic) {
-  EXPECT_TRUE(matches("int a = 0;", namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(
-  notMatches("static int a = 0;", namedDecl(hasExternalFormalLinkage(;
+  EXPECT_TRUE(matches("int a = 0;",
+  namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("static int a = 0;",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
   EXPECT_TRUE(notMatches("static void f(void) { int a = 0; }",
- namedDecl(hasExternalFormalLinkage(;
-  EXPECT_TRUE(matches("void f(void) { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
-
-  // Despite having internal semantic linkage, the anonymous namespace member
-  // has external linkage because the member has a unique name in all
-  // translation units.
-  EXPECT_TRUE(matches("namespace { int a = 0; }",
-  namedDecl(hasExternalFormalLinkage(;
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("void f(void) { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
+  EXPECT_TRUE(notMatches("namespace { int a = 0; }",
+ namedDecl(hasName("a"), hasExternalFormalLinkage(;
 }
 
 TEST(HasDefaultArgument, Basic) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
+  ` check.
+  Finds functions registered by ``atexit`` and ``at_quick_exit`` that are 
calling
+  exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.

Please separate with empty line.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:6
+
+This check implements SEI CERT rule ENV32-C.
+Description source: 
``_

Please synchronize with statement in Release Notes.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:7
+This check implements SEI CERT rule ENV32-C.
+Description source: 
``_
+

Reference to original description usually placed at the end. 



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:12
+
+The C Standard provides three functions that cause an application to terminate 
normally: _Exit(), exit(), and quick_exit(). These are collectively called exit 
functions. When the exit() function is called, or control transfers out of the 
main() entry point function, functions registered with atexit() are called (but 
not at_quick_exit()). When the quick_exit() function is called, functions 
registered with at_quick_exit() (but not atexit()) are called. These functions 
are collectively called exit handlers.  When the _Exit() function is called, no 
exit handlers or signal handlers are called.
+

Please enclose all function names in double back-ticks. Same below.

Please don't use lines longer then 80 symbols.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:16
+
+A nested call to an exit function is undefined behavior. (See undefined 
behavior 182.) This behavior can occur only when an exit function is invoked 
from an exit handler or when an exit function is called from within a signal 
handler. (See SIG30-C. Call only asynchronous-safe functions within signal 
handlers.)
+

Is //SIG30-C// implemented in Clang-tidy? If not, reference should be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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


[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

2020-07-14 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb marked an inline comment as done.
krisb added inline comments.



Comment at: clang/CMakeLists.txt:751
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(MSVC AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)

phosek wrote:
> I don't understand the second part of this condition, can you elaborate? Why 
> not set `CMAKE_LINKER` to `lld-link.exe` even if `BOOTSTRAP_CMAKE_SYSTEM_NAME 
> STREQUAL "Windows"`?
The only reason that stopped me from adding `BOOTSTRAP_CMAKE_SYSTEM_NAME 
STREQUAL "Windows"` case here is that there is no way to keep it working in the 
future (or, at least, I don't know one). Moreover, the build which sets 
BOOTSTRAP_CMAKE_SYSTEM_NAME equal to "Windows" is currently broken.
Since the same issue is exposed if to build LLVM with 
`clang/cmake/caches/DistributionExample.cmake` on Windows, I included only the 
changes that contribute to making this configuration buildable. I wanted to 
avoid making the impression that some other configurations are supported 
(because they are not) and also avoid adding any dead code that nobody would 
use and that would easily be broken.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80873



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


[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

2020-07-14 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 277814.
dgoldman added a comment.

Minor lint fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83501

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -674,7 +674,57 @@
   enum class E { [[A]], B };
   E e = E::A^;
 };
-  )cpp"};
+  )cpp",
+
+  R"objc(
+@protocol Dog;
+@protocol $decl[[Dog]]
+- (void)bark;
+@end
+id getDoggo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@interface Cat
+@end
+@implementation Cat
+@end
+@interface $decl[[Cat]] (Exte^nsion)
+- (void)meow;
+@end
+@implementation $def[[Cat]] (Extension)
+- (void)meow {}
+@end
+  )objc",
+
+  R"objc(
+@class $decl[[Foo]];
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@class Foo;
+@interface $decl[[Foo]]
+@end
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc",
+
+  R"objc(
+@class Foo;
+@interface $decl[[Foo]]
+@end
+@implementation $def[[Foo]]
+@end
+Fo^o * getFoo() {
+  return 0;
+}
+  )objc"};
   for (const char *Test : Tests) {
 Annotations T(Test);
 llvm::Optional WantDecl;
@@ -692,6 +742,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto Results = locateSymbolAt(AST, T.point());
@@ -709,6 +760,131 @@
   }
 }
 
+TEST(LocateSymbol, AllMulti) {
+  // Ranges in tests:
+  //   $declN is the declaration location
+  //   $defN is the definition location (if absent, symbol has no definition)
+  //
+  // NOTE:
+  //   N starts at 0.
+  //   Due to limitations in clang's Annotations, you can't annotate the same
+  //   text with multiple ranges, e.g. `$def0$def1[[Foo]]` is invalid.
+  struct ExpectedRanges {
+Range WantDecl;
+llvm::Optional WantDef;
+  };
+  const char *Tests[] = {
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @implementation $def0[[Cat]]
+  @end
+  @interface $decl1[[Ca^t]] (Extension)
+  - (void)meow;
+  @end
+  @implementation $def1[[Cat]] (Extension)
+  - (void)meow {}
+  @end
+)objc",
+
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @implementation $def0[[Cat]]
+  @end
+  @interface $decl1[[Cat]] (Extension)
+  - (void)meow;
+  @end
+  @implementation $def1[[Ca^t]] (Extension)
+  - (void)meow {}
+  @end
+)objc",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+std::vector Ranges;
+for (int Idx = 0; true; Idx++) {
+  bool HasDecl = !T.ranges("decl" + std::to_string(Idx)).empty();
+  bool HasDef = !T.ranges("def" + std::to_string(Idx)).empty();
+  if (!HasDecl && !HasDef)
+break;
+  ExpectedRanges Range;
+  if (HasDecl)
+Range.WantDecl = T.range("decl" + std::to_string(Idx));
+  if (HasDef)
+Range.WantDef = T.range("def" + std::to_string(Idx));
+  Ranges.push_back(Range);
+}
+
+TestTU TU;
+TU.Code = std::string(T.code());
+TU.ExtraArgs.push_back("-xobjective-c++");
+
+auto AST = TU.build();
+auto Results = locateSymbolAt(AST, T.point());
+
+ASSERT_THAT(Results, ::testing::SizeIs(Ranges.size())) << Test;
+for (size_t Idx = 0; Idx < Ranges.size(); Idx++) {
+  EXPECT_EQ(Results[Idx].PreferredDeclaration.range, Ranges[Idx].WantDecl)
+  << "($decl" << Idx << ")" << Test;
+  llvm::Optional GotDef;
+  if (Results[Idx].Definition)
+GotDef = Results[Idx].Definition->range;
+  EXPECT_EQ(GotDef, Ranges[Idx].WantDef) << "($def" << Idx << ")" << Test;
+}
+  }
+}
+
+TEST(LocateSymbol, MultipleDeclsWithSameDefinition) {
+  // Ranges in tests:
+  //   $def is the shared definition location
+  //   $declN is one of the declaration locations
+  //
+  // NOTE:
+  //   N starts at 0.
+  const char *Tests[] = {
+  R"objc(
+  @interface $decl0[[Cat]]
+  @end
+  @interface $decl1[[Ca^t]] ()
+  - (void)meow;
+  @end
+  @implementation $def[[Cat]]
+  - (void)meow {}
+  @end
+)objc",
+  };

[PATCH] D83705: [clangd] Config: CompileFlags.Remove

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c16fbd0ac7b: [clangd] Config: CompileFlags.Remove (authored 
by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D83705?vs=277510&id=277819#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83705

Files:
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -91,10 +91,12 @@
 
 TEST_F(ConfigCompileTests, CompileCommands) {
   Frag.CompileFlags.Add.emplace_back("-foo");
-  std::vector Argv = {"clang", "a.cc"};
+  Frag.CompileFlags.Remove.emplace_back("--include-directory=");
+  std::vector Argv = {"clang", "-I", "bar/", "a.cc"};
   EXPECT_TRUE(compileAndApply());
-  EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(1));
-  Conf.CompileFlags.Edits.front()(Argv);
+  EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
+  for (auto &Edit : Conf.CompileFlags.Edits)
+Edit(Argv);
   EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
 }
 
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -63,6 +63,10 @@
   if (auto Values = scalarValues(N))
 F.Add = std::move(*Values);
 });
+Dict.handle("Remove", [&](Node &N) {
+  if (auto Values = scalarValues(N))
+F.Remove = std::move(*Values);
+});
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -117,9 +117,39 @@
   };
   IfBlock If;
 
+  /// Conditions in the CompileFlags block affect how a file is parsed.
+  ///
+  /// clangd emulates how clang would interpret a file.
+  /// By default, it behaves roughly like `clang $FILENAME`, but real projects
+  /// usually require setting the include path (with the `-I` flag), defining
+  /// preprocessor symbols, configuring warnings etc.
+  /// Often, a compilation database specifies these compile commands. clangd
+  /// searches for compile_commands.json in parents of the source file.
+  ///
+  /// This section modifies how the compile command is constructed.
   struct CompileFlagsBlock {
+/// List of flags to append to the compile command.
 std::vector> Add;
-  } CompileFlags;
+/// List of flags to remove from the compile command.
+///
+/// - If the value is a recognized clang flag (like "-I") then it will be
+///   removed along with any arguments. Synonyms like --include-dir= will
+///   also be removed.
+/// - Otherwise, if the value ends in * (like "-DFOO=*") then any argument
+///   with the prefix will be removed.
+/// - Otherwise any argument exactly matching the value is removed.
+///
+/// In all cases, -Xclang is also removed where needed.
+///
+/// Example:
+///   Command: clang++ --include-directory=/usr/include -DFOO=42 foo.cc
+///   Remove: [-I, -DFOO=*]
+///   Result: clang++ foo.cc
+///
+/// Flags added by the same CompileFlags entry will not be removed.
+std::vector> Remove;
+  };
+  CompileFlagsBlock CompileFlags;
 };
 
 } // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -23,6 +23,7 @@
 //
 //===--===//
 
+#include "CompileCommands.h"
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "support/Logger.h"
@@ -122,6 +123,19 @@
   }
 
   void compile(Fragment::CompileFlagsBlock &&F) {
+if (!F.Remove.empty()) {
+  auto Remove = std::make_shared();
+  for (auto &A : F.Remove)
+Remove->strip(*A);
+  Out.Apply.push_back([Remove(std::shared_ptr(
+  std::move(Remove)))](Config &C) {
+C.CompileFlags.Edits.push_back(
+[Remove](std::vector &Args) {
+  Remove->process(Args);
+});
+  });
+}
+
 if (!F.Add.empty()) {
   std::vector Add;
   for (auto &A : F.Add)
Index: clang-tools-extra/clangd/CompileCommands.h
===
--- clang-tools-extra/clangd/CompileCommands.h
+++ clang-tools-extra/clangd/CompileCommands.h
@@ -59,6 +59,

[clang-tools-extra] 6c16fbd - [clangd] Config: CompileFlags.Remove

2020-07-14 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-14T15:56:44+02:00
New Revision: 6c16fbd0ac7b18110891d0f180a2408d55fe47a8

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

LOG: [clangd] Config: CompileFlags.Remove

Summary: While here, add documentation to CompileFlags and CompileFlags.Add.

Reviewers: hokein

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.h 
b/clang-tools-extra/clangd/CompileCommands.h
index c9f2d668c365..84c4c2a26a87 100644
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -59,6 +59,12 @@ struct CommandMangler {
 // The table-building strategy may not make sense outside clangd.
 class ArgStripper {
 public:
+  ArgStripper() = default;
+  ArgStripper(ArgStripper &&) = default;
+  ArgStripper(const ArgStripper &) = delete;
+  ArgStripper &operator=(ArgStripper &&) = default;
+  ArgStripper &operator=(const ArgStripper &) = delete;
+
   // Adds the arg to the set which should be removed.
   //
   // Recognized clang flags are stripped semantically. When "-I" is stripped:

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 04c0df88bbf7..568e029b5c0a 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -23,6 +23,7 @@
 //
 
//===--===//
 
+#include "CompileCommands.h"
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "support/Logger.h"
@@ -122,6 +123,19 @@ struct FragmentCompiler {
   }
 
   void compile(Fragment::CompileFlagsBlock &&F) {
+if (!F.Remove.empty()) {
+  auto Remove = std::make_shared();
+  for (auto &A : F.Remove)
+Remove->strip(*A);
+  Out.Apply.push_back([Remove(std::shared_ptr(
+  std::move(Remove)))](Config &C) {
+C.CompileFlags.Edits.push_back(
+[Remove](std::vector &Args) {
+  Remove->process(Args);
+});
+  });
+}
+
 if (!F.Add.empty()) {
   std::vector Add;
   for (auto &A : F.Add)

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 42f9ec2edc72..5cc8749c5efa 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -117,9 +117,39 @@ struct Fragment {
   };
   IfBlock If;
 
+  /// Conditions in the CompileFlags block affect how a file is parsed.
+  ///
+  /// clangd emulates how clang would interpret a file.
+  /// By default, it behaves roughly like `clang $FILENAME`, but real projects
+  /// usually require setting the include path (with the `-I` flag), defining
+  /// preprocessor symbols, configuring warnings etc.
+  /// Often, a compilation database specifies these compile commands. clangd
+  /// searches for compile_commands.json in parents of the source file.
+  ///
+  /// This section modifies how the compile command is constructed.
   struct CompileFlagsBlock {
+/// List of flags to append to the compile command.
 std::vector> Add;
-  } CompileFlags;
+/// List of flags to remove from the compile command.
+///
+/// - If the value is a recognized clang flag (like "-I") then it will be
+///   removed along with any arguments. Synonyms like --include-dir= will
+///   also be removed.
+/// - Otherwise, if the value ends in * (like "-DFOO=*") then any argument
+///   with the prefix will be removed.
+/// - Otherwise any argument exactly matching the value is removed.
+///
+/// In all cases, -Xclang is also removed where needed.
+///
+/// Example:
+///   Command: clang++ --include-directory=/usr/include -DFOO=42 foo.cc
+///   Remove: [-I, -DFOO=*]
+///   Result: clang++ foo.cc
+///
+/// Flags added by the same CompileFlags entry will not be removed.
+std::vector> Remove;
+  };
+  CompileFlagsBlock CompileFlags;
 };
 
 } // namespace config

diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index ef6003b02439..7e86e3721248 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -63,6 +63,10 @@ class Parser {
   if (auto Values = scalarValues(N))
 F.Add = std::move(*Values);
 });
+  

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: Szelethus, martong, steakhal, dkrupp.
Szelethus added a comment.

@Eugene.Zelenko Thanks for cleaning up revisions -- same as D69560#1725399 
, we are working in the same office 
and have worked on some forms of static analysis for a while now. Adding us as 
reviewers helps this revision being more visible. Its clear that we don't have 
the authority to approve changes to clang-tidy, but we can confidently request 
changes or add to the discussion while still respecting that.

I'll re-add ourselves to keep reviewing well oiled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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


[PATCH] D81676: [MSP430] Align the toolchain definition with the TI's msp430-gcc v9.2.0

2020-07-14 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb accepted this revision.
krisb added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: dang.

Sorry for the delays in response, busy days.
LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81676



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


[PATCH] D83772: [Windows] Fix limit on command line size

2020-07-14 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision.
sepavloff added reviewers: rnk, zturner.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Documentation on CreateProcessW states that maximal size of command line
is 32767 characters including ternimation null character. In the
function llvm::sys::commandLineFitsWithinSystemLimits this limit was set
to 32768. As a result if command line was exactly 32768 characters long,
a response file was not created and CreateProcessW was called with
too long command line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83772

Files:
  llvm/lib/Support/Windows/Program.inc


Index: llvm/lib/Support/Windows/Program.inc
===
--- llvm/lib/Support/Windows/Program.inc
+++ llvm/lib/Support/Windows/Program.inc
@@ -532,8 +532,13 @@
 
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
   ArrayRef Args) {
-  // The documented max length of the command line passed to CreateProcess.
-  static const size_t MaxCommandStringLength = 32768;
+  // In the documentation on CreateProcessW:
+  // 
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
+  // it is stated that the argument lpCommandLine has restriction:
+  //
+  //The maximum length of this string is 32,767 characters, including the
+  //Unicode terminating null character.
+  static const size_t MaxCommandStringLength = 32767;
   SmallVector FullArgs;
   FullArgs.push_back(Program);
   FullArgs.append(Args.begin(), Args.end());


Index: llvm/lib/Support/Windows/Program.inc
===
--- llvm/lib/Support/Windows/Program.inc
+++ llvm/lib/Support/Windows/Program.inc
@@ -532,8 +532,13 @@
 
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
   ArrayRef Args) {
-  // The documented max length of the command line passed to CreateProcess.
-  static const size_t MaxCommandStringLength = 32768;
+  // In the documentation on CreateProcessW:
+  // https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
+  // it is stated that the argument lpCommandLine has restriction:
+  //
+  //The maximum length of this string is 32,767 characters, including the
+  //Unicode terminating null character.
+  static const size_t MaxCommandStringLength = 32767;
   SmallVector FullArgs;
   FullArgs.push_back(Program);
   FullArgs.append(Args.begin(), Args.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83702: [AIX]Generate debug info for static init related functions

2020-07-14 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 277831.
Xiangling_L marked 6 inline comments as done.
Xiangling_L added a comment.

Simplified the testcase;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83702

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp


Index: clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited  < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+struct X {
+  X();
+  ~X();
+};
+
+X v;
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR16:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XC1Ev(%struct.X* @v), !dbg ![[DBGVAR19:[0-9]+]]
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR19]]
+// CHECK:   ret void, !dbg ![[DBGVAR19]]
+// CHECK: }
+
+// CHECK: define internal void @__dtor_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR20:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XD1Ev(%struct.X* @v), !dbg ![[DBGVAR21:[0-9]+]]
+// CHECK:   ret void, !dbg ![[DBGVAR21]]
+// CHECK: }
+
+// CHECK: define internal void @__finalize_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR22:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR24:[0-9]+]]
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0, !dbg ![[DBGVAR24]]
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end, 
!dbg ![[DBGVAR24]]
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_v(), !dbg ![[DBGVAR24]]
+// CHECK:   br label %destruct.end, !dbg ![[DBGVAR24]]
+
+// CHECK: destruct.end:
+// CHECK:   ret void, !dbg ![[DBGVAR24]]
+// CHECK: }
+
+// CHECK: define void 
@__sinit8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR25:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init(), !dbg ![[DBGVAR26:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define void 
@__sterm8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR27:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__finalize_v(), !dbg ![[DBGVAR28:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: 
"__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: 
!{{[0-9]+}}, scopeLine: 14, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR19]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR16]])
+// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: 
!{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, 
spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, 
retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR21]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR20]])
+// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", 
scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 
14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR24]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR22]])
+// CHECK: ![[DBGVAR25]] = distinct !DISubprogram(linkageName: 
"__sinit8000_clang_c3236cbaa79f2bae3a15e6379a05f625", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: 
DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR26]] = !DILocation(line: 0, scope: ![[DBGVAR25]])
+// CHECK: ![[DBGVAR27]] = distinct !DISubprogram(linkageName: 
"__sterm8000_clang_c3236cbaa79f2bae3a15e6379a05f625", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: 
DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR28]] = !DILocation(line: 0, scope: ![[DBGVAR27]])
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4567,7 +4567,8 @@
   CodeGenFunction CGF(CGM);
 
   CGF.StartFunction(GlobalDecl(), CGM.getContext().VoidTy, StermFinalizer, FI,
-FunctionArgList());
+FunctionArgList(), D.getLocation(),
+D.getInit()->getExprLoc());
 
   // The unatexit subroutine

[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread David Tenty via Phabricator via cfe-commits
daltenty added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:15
 #include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/Allocator.h"

This breaks the powerpc64le bots. Looks like we aren't linking against Option:

```
CompileCommands.cpp:(.text._ZZN5clang6clangd11ArgStripper8rulesForEN4llvm9StringRefEENK3$_3clEv+0x514c):
 undefined reference to 
`llvm::opt::OptTable::getOption(llvm::opt::OptSpecifier) const'
```

http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/5697/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958



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


[PATCH] D83723: [OpenMP][NFC] Generalize CGOpenMPRuntimeNVPTX as CGOpenMPRuntimeGPU

2020-07-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

I meant to remove just the new virtual function and its new implementation for 
AMDGCN, you could keep the new class for AMD GPUs. But I'm fine with this too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83723



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


[PATCH] D83755: [clangd] Cache config files for 5 seconds, without revalidating with stat.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
sammccall marked 4 inline comments as done.
Closed by commit rGff616f74c3b4: [clangd] Cache config files for 5 seconds, 
without revalidating with stat. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D83755?vs=277723&id=277836#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83755

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/ConfigProvider.h
  clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -10,10 +10,11 @@
 #include "ConfigProvider.h"
 #include "ConfigTesting.h"
 #include "TestFS.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include "llvm/Support/SourceMgr.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -150,6 +151,43 @@
   EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
 }
 
+TEST(ProviderTest, Staleness) {
+  MockFS FS;
+
+  auto StartTime = std::chrono::steady_clock::now();
+  Params StaleOK;
+  StaleOK.FreshTime = StartTime;
+  Params MustBeFresh;
+  MustBeFresh.FreshTime = StartTime + std::chrono::hours(1);
+  CapturedDiags Diags;
+  auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS);
+
+  // Initial query always reads, regardless of policy.
+  FS.Files["foo.yaml"] = AddFooWithErr;
+  auto Cfg = P->getConfig(StaleOK, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+  Diags.Diagnostics.clear();
+
+  // Stale value reused by policy.
+  FS.Files["foo.yaml"] = AddBarBaz;
+  Cfg = P->getConfig(StaleOK, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached, not re-parsed";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+
+  // Cache revalidated by policy.
+  Cfg = P->getConfig(MustBeFresh, Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+
+  // Cache revalidated by (default) policy.
+  FS.Files.erase("foo.yaml");
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/ConfigProvider.h
===
--- clang-tools-extra/clangd/ConfigProvider.h
+++ clang-tools-extra/clangd/ConfigProvider.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include 
 #include 
 #include 
 
@@ -34,6 +35,10 @@
   /// Absolute path to a source file we're applying the config to. Unix slashes.
   /// Empty if not configuring a particular file.
   llvm::StringRef Path;
+  /// Hint that stale data is OK to improve performance (e.g. avoid IO).
+  /// FreshTime sets a bound for how old the data can be.
+  /// If not set, providers should validate caches against the data source.
+  llvm::Optional FreshTime;
 };
 
 /// Used to report problems in parsing or interpreting a config.
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -11,8 +11,10 @@
 #include "ConfigFragment.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
+#include 
 #include 
 
 namespace clang {
@@ -22,21 +24,18 @@
 // Threadsafe cache around reading a YAML config file from disk.
 class FileConfigCache {
   std::mutex Mu;
+  std::chrono::steady_clock::time_point ValidTime = {};
   llvm::SmallVector CachedValue;
   llvm::sys::TimePoint<> MTime = {};
   unsigned Size = -1;
 
-  void updateCacheLocked(const llvm::vfs::Status &Stat,
- llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
-if (Size == Stat.getSize() && MTime == Stat.getLastModificationTime())
-  return; // Already valid.
-
-Size = Stat.getSize();
-MTime = Stat.getLastModificationTime();
+  // Called once we are sure we want to read the file.
+  // REQUIRES: Cache keys are set. Mutex must be held.
+  void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
 CachedValue.clear();
 
 auto Buf = FS.getBufferForFile(Path);
-// If stat() succeeds but we failed to read, don

[clang-tools-extra] ff616f7 - [clangd] Cache config files for 5 seconds, without revalidating with stat.

2020-07-14 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-14T17:00:41+02:00
New Revision: ff616f74c3b45e0890b53d92fcfc6a9d18f4bfdd

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

LOG: [clangd] Cache config files for 5 seconds, without revalidating with stat.

Summary:
This is motivated by:
 - code completion: nice to do no i/o on the request path
 - background index: deciding whether to enqueue each file would stat the config
   file thousands of times in quick succession.

Currently it's applied uniformly to all requests though.

This gives up on performing stat() outside the lock, all this achieves is
letting multiple threads stat concurrently (and thus finish without contention
for nonexistent files).
The ability to finish without IO (just mutex lock + integer check) should
outweigh this, and is less sensitive to platform IO characteristics.

Reviewers: kadircet

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/ConfigProvider.h
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index c33cdffcb0ca..ec4855659501 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -48,6 +48,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -762,6 +763,9 @@ Context ClangdServer::createProcessingContext(PathRef File) 
const {
 return Context::current().clone();
 
   config::Params Params;
+  // Don't reread config files excessively often.
+  // FIXME: when we see a config file change event, use the event timestamp.
+  Params.FreshTime = std::chrono::steady_clock::now() - 
std::chrono::seconds(5);
   llvm::SmallString<256> PosixPath;
   if (!File.empty()) {
 assert(llvm::sys::path::is_absolute(File));

diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
index 4b466d53e293..1f0f727998e3 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -11,8 +11,10 @@
 #include "ConfigFragment.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
+#include 
 #include 
 
 namespace clang {
@@ -22,21 +24,18 @@ namespace config {
 // Threadsafe cache around reading a YAML config file from disk.
 class FileConfigCache {
   std::mutex Mu;
+  std::chrono::steady_clock::time_point ValidTime = {};
   llvm::SmallVector CachedValue;
   llvm::sys::TimePoint<> MTime = {};
   unsigned Size = -1;
 
-  void updateCacheLocked(const llvm::vfs::Status &Stat,
- llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
-if (Size == Stat.getSize() && MTime == Stat.getLastModificationTime())
-  return; // Already valid.
-
-Size = Stat.getSize();
-MTime = Stat.getLastModificationTime();
+  // Called once we are sure we want to read the file.
+  // REQUIRES: Cache keys are set. Mutex must be held.
+  void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
 CachedValue.clear();
 
 auto Buf = FS.getBufferForFile(Path);
-// If stat() succeeds but we failed to read, don't cache failure.
+// If we failed to read (but stat succeeded), don't cache failure.
 if (!Buf) {
   Size = -1;
   MTime = {};
@@ -68,19 +67,40 @@ class FileConfigCache {
   // - allow caches to be reused based on short elapsed walltime
   // - allow latency-sensitive operations to skip revalidating the cache
   void read(const ThreadsafeFS &TFS, DiagnosticCallback DC,
+llvm::Optional FreshTime,
 std::vector &Out) {
+std::lock_guard Lock(Mu);
+// We're going to update the cache and return whatever's in it.
+auto Return = llvm::make_scope_exit(
+[&] { llvm::copy(CachedValue, std::back_inserter(Out)); });
+
+// Return any sufficiently recent result without doing any further work.
+if (FreshTime && ValidTime >= FreshTime)
+  return;
+
+// Ensure we bump the ValidTime at the end to allow for reuse.
+auto MarkTime = llvm::make_scope_exit(
+[&] { ValidTime = std::chrono::steady_clock::now(); });
+
+// Stat is cheaper than opening the file, it's usually unchanged.
 assert(llvm::sys::path::is_absolute(Path));
 auto FS = TFS.view(/*CWD=*/llvm::None);
 auto Stat = FS->status(Path);
+// If there's no file, t

[clang-tools-extra] 50a5fa8 - [clangd] Add missing link dep after 8eb8c92eb46908e

2020-07-14 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-14T17:03:45+02:00
New Revision: 50a5fa8b9ba4b09433bf46f4228d4e4cae9ac486

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

LOG: [clangd] Add missing link dep after 8eb8c92eb46908e

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 9eb06941e4dd..b3002b1d5698 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -25,6 +25,7 @@ set(LLVM_LINK_COMPONENTS
   Support
   AllTargetsInfos
   FrontendOpenMP
+  Option
   )
 
 add_clang_library(clangDaemon



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


[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:15
 #include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/Allocator.h"

daltenty wrote:
> This breaks the powerpc64le bots. Looks like we aren't linking against Option:
> 
> ```
> CompileCommands.cpp:(.text._ZZN5clang6clangd11ArgStripper8rulesForEN4llvm9StringRefEENK3$_3clEv+0x514c):
>  undefined reference to 
> `llvm::opt::OptTable::getOption(llvm::opt::OptSpecifier) const'
> ```
> 
> http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/5697/
Thanks! 50a5fa8b9ba4b09433bf46f4228d4e4cae9ac486 will hopefully fix.

(Not sure why this didn't show up on x64 but linking is mostly a mystery to 
me...)



Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:197
+}
+
+TEST(ArgStripperTest, Spellings) {

hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > sammccall wrote:
> > > > hokein wrote:
> > > > > add tests for stripping the diagnostic flags, `-Wfoo` etc.
> > > > Those aren't actually separate flags: "-W" is a Joined flag and foo is 
> > > > an arg.
> > > > Do you want a test specifically for such a string anyway?
> > > > Or do you want special behavior for them? (Like interaction between 
> > > > -Wfoo and -Wno-foo)
> > > I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> > > would be nice to have them in tests as I think these are important cases.
> > > 
> > > > Those aren't actually separate flags: "-W" is a Joined flag and foo is 
> > > > an arg.
> > > 
> > > oh, if I understand correctly:
> > > 
> > > - `strip("unused", "clang -Wunused foo.cc")` => `clang foo.cc` ?
> > > - `strip("-W", "clang -Wunused -Wextra foo.cc")` => `clang foo.cc` // 
> > > remove all -W flags ?
> > > - `strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> > > foo.cc")`  =>  `clang -Wunused -Werror=date-time foo.cc`?
> > > 
> > > I was a bit unclear about how the stripper strips the "-W/-D"-like flag, 
> > > would be nice to have them in tests as I think these are important cases.
> > 
> > Added.
> > 
> > > strip("unused", "clang -Wunused foo.cc") => clang foo.cc ?
> > 
> > No, we only strip clang args or flag names, not flag args (confusing 
> > terminology).
> > -W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang 
> > arg). -W* will strip -Wunused (clang arg). but "unused" doesn't match - 
> > it's not the name of a flag, and it's not the arg either.
> > 
> > > strip("-W", "clang -Wunused -Wextra foo.cc") => clang foo.cc // remove 
> > > all -W flags ?
> > 
> > Yes
> > 
> > > strip("error=unused", "clang -Wunused -Werror=unused -Werror=date-time 
> > > foo.cc") => clang -Wunused -Werror=date-time foo.cc?
> > 
> > No, again we don't strip flag args.
> > (It's not clear how useful/easy to use this would be, maybe we can 
> > re-examine later?)
> > No, we only strip clang args or flag names, not flag args (confusing 
> > terminology).
> > -W will strip -Wunused (flag name). -Wunused will strip -Wunused (clang 
> > arg). -W* will strip -Wunused (clang arg). but "unused" doesn't match - 
> > it's not the name of a flag, and it's not the arg either.
> 
> oh, I was confused by these terms :(
> 
> > No, again we don't strip flag args.
> > (It's not clear how useful/easy to use this would be, maybe we can 
> > re-examine later?)
> 
> we can still strip it by passing "-Werror-unused" arg, right? if so, I think 
> it should be fine.
> oh, I was confused by these terms :(

They're confusing. Tried to clarify in the doc.

> we can still strip it by passing "-Werror-unused" arg, right? if so, I think 
> it should be fine.

Yeah. (-Werror=unused I guess, but in any case...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81958



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


[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

2020-07-14 Thread Gui Andrade via Phabricator via cfe-commits
guiand added a comment.
Herald added a subscriber: dang.

Is anything still pending here (besides the tests, of course)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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


[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:98
+  ValidValues.push_back(Name);
+  if (!Result && *Input == Name)
+Result = Value;

should we assert on multiple matches of the same name ?



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:196
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
+  constexpr static llvm::SourceMgr::DiagKind Warning =
+  llvm::SourceMgr::DK_Warning;

nit: I know this isn't what the patch is about, but what about making these 
lambdas/functions of Out.diag with first parameter bound to DK_Error or 
DK_Warning? (or having them at an anonymous namespace instead of a member)



Comment at: clang-tools-extra/clangd/ConfigFragment.h:124
+
+  /// Controls how clangd understands code outside the current file.
+  struct IndexBlock {

i think saying "outside the current file" might not be the best description. 
Maybe rather "Controls clangd's understanding for entities(symbols, refs, 
relations) in the codebase" ?



Comment at: clang-tools-extra/clangd/ConfigFragment.h:129
+/// Legal values are "Build" or "Skip".
+llvm::Optional> Background;
+  };

why not store the enum in here directly and generate diagnostics during the 
parsing stage?

we seem to be doing this for syntactic errors, e.g. `expected a dictionary` and 
getting an unexpected enum value sounds similar to that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83768



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


[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

2020-07-14 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

I'll address the nits. Unsure if that is all. I also don't get the one comment.




Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:433
 
-  Builder.SetInsertPoint(OuterFn->getEntryBlock().getFirstNonPHI());
-  AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr");
-  AllocaInst *ZeroAddr = Builder.CreateAlloca(Int32, nullptr, "zero.addr");
+  IRBuilder<> AllocaBuilder(AllocaIP.getBlock(), AllocaIP.getPoint());
+

fghanim wrote:
> Here and elsewhere: You seem to have forgotten to undo the changes introduced 
> by using a separate `AllocaBuilder`?
> 
> Also, Nit: before using `AllocaIP`, either add an assert that it is not 
> empty, or alternatively, add logic to just set it to entry block of `outerfn` 
> if it is
> Here and elsewhere: You seem to have forgotten to undo the changes introduced 
> by using a separate AllocaBuilder?

I don't get this. These changes are on purpose. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82470



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


[PATCH] D83722: [PowerPC] Add options to control paired vector memops support

2020-07-14 Thread Baptiste Saleil via Phabricator via cfe-commits
bsaleil updated this revision to Diff 277838.
bsaleil added a comment.

Re-upload with surrounding context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83722

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/ppc-dependent-options.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/lib/Target/PowerPC/PPCScheduleP9.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h

Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -146,6 +146,7 @@
   bool VectorsUseTwoUnits;
   bool UsePPCPreRASchedStrategy;
   bool UsePPCPostRASchedStrategy;
+  bool PairedVectorMemops;
   bool PredictableSelectIsExpensive;
 
   POPCNTDKind HasPOPCNTD;
@@ -266,6 +267,7 @@
   bool hasP10Vector() const { return HasP10Vector; }
   bool hasPrefixInstrs() const { return HasPrefixInstrs; }
   bool hasPCRelativeMemops() const { return HasPCRelativeMemops; }
+  bool pairedVectorMemops() const { return PairedVectorMemops; }
   bool hasMFOCRF() const { return HasMFOCRF; }
   bool hasISEL() const { return HasISEL; }
   bool hasBPERMD() const { return HasBPERMD; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -122,6 +122,7 @@
   VectorsUseTwoUnits = false;
   UsePPCPreRASchedStrategy = false;
   UsePPCPostRASchedStrategy = false;
+  PairedVectorMemops = false;
   PredictableSelectIsExpensive = false;
 
   HasPOPCNTD = POPCNTD_Unavailable;
Index: llvm/lib/Target/PowerPC/PPCScheduleP9.td
===
--- llvm/lib/Target/PowerPC/PPCScheduleP9.td
+++ llvm/lib/Target/PowerPC/PPCScheduleP9.td
@@ -43,8 +43,8 @@
   // Do not support QPX (Quad Processing eXtension), SPE (Signal Processing
   // Engine), prefixed instructions on Power 9, PC relative mem ops, or
   // instructions introduced in ISA 3.1.
-  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, PCRelativeMemops,
- IsISA3_1];
+  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, PairedVectorMemops,
+ PCRelativeMemops, IsISA3_1];
 
 }
 
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -427,6 +427,7 @@
 
 def PrefixInstrs : Predicate<"Subtarget->hasPrefixInstrs()">;
 def IsISA3_1 : Predicate<"Subtarget->isISA3_1()">;
+def PairedVectorMemops : Predicate<"PPCSubTarget->pairedVectorMemops()">;
 
 let Predicates = [PrefixInstrs] in {
   let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -237,6 +237,10 @@
   SubtargetFeature<"pcrelative-memops", "HasPCRelativeMemops", "true",
"Enable PC relative Memory Ops",
[FeatureISA3_0]>;
+def FeaturePairedVectorMemops:
+  SubtargetFeature<"paired-vector-memops", "PairedVectorMemops", "true",
+   "32Byte load and store instructions",
+   [FeatureISA3_0]>;
 
 def FeaturePredictableSelectIsExpensive :
   SubtargetFeature<"predictable-select-expensive",
@@ -342,7 +346,7 @@
   // still exist with the exception of those we know are Power9 specific.
   list P10AdditionalFeatures =
 [DirectivePwr10, FeatureISA3_1, FeaturePrefixInstrs,
- FeaturePCRelativeMemops, FeatureP10Vector];
+ FeaturePCRelativeMemops, FeatureP10Vector, FeaturePairedVectorMemops];
   list P10SpecificFeatures = [];
   list P10InheritableFeatures =
 !listconcat(P9InheritableFeatures, P10AdditionalFeatures);
Index: clang/test/Driver/ppc-dependent-options.cpp
===
--- clang/test/Driver/ppc-dependent-options.cpp
+++ clang/test/Driver/ppc-dependent-options.cpp
@@ -55,6 +55,10 @@
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-FLT128
 
 // RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power10 -std=c++11 -mno-vsx -mpaired-vector-memops %s 2>&1 | \
+// RUN: FileCheck %s -check-prefix=CHECK-NVSX-PAIRED-VEC-MEMOPS
+
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
 // RUN: -mcpu=power9 -std=c++11 -mno-vsx -mfloat128 -mpower9-vector %s 2>&1 | \
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-MULTI
 
@@ -96,6 +100,7 @@
 // CHECK-NVSX-P10V: error: option '-mpower10-vector' cannot be specified with '-mno-vsx'
 /

[PATCH] D71124: [RISCV] support clang driver to select cpu

2020-07-14 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment.

I've got one major issue (inline below), and I'm confused by some other 
behaviour:

When I run `clang --target=riscv64 -mcpu=?`, the list includes both 
`generic-rv32` and `generic-rv64`. It doesn't show only the 64-bit cpus. This 
is not changed by giving a full triple, or by additionally specifying 
`-march=rv64gc`. Should it be?

For instance, this output:

  $ clang --target=riscv64-unknown-elf -mcpu=\? -march=rv64gc
  clang version 11.0.0
  Target: riscv64-unknown-unknown-elf
  Thread model: posix
  InstalledDir: /home/selliott/llvm-project/./build/llvm_clang/all/bin
  Available CPUs for this target:
  
generic-rv32
generic-rv64
rocket-rv32
rocket-rv64
sifive-e31
sifive-u54
  
  Use -mcpu or -mtune to specify the target's processor.
  For example, clang --target=aarch64-unknown-linux-gui -mcpu=cortex-a35




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:599
+  return "lp64";
   }
 }

I now get a stacktrace here if I call `clang --target=riscv32 -march=unknown 
foo.c -c`. In clang-10, you get a nice error:
```
clang-10: error: invalid arch name 'wat', string must begin with rv32{i,e,g} or 
rv64{i,g}
```

The stacktrace is coming from the call to `getRISCVABI` in 
`findRISCVBareMetalMultilibs` (in `clang/lib/Driver/ToolChains/Gnu.cpp:1608`).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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


[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 277844.
sammccall marked 5 inline comments as done.
sammccall added a comment.

Address some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83768

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -98,6 +98,22 @@
   EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
 }
 
+TEST_F(ConfigCompileTests, Index) {
+  Frag.Index.Background.emplace("Skip");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+
+  Frag = {};
+  Frag.Index.Background.emplace("Foo");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build)
+  << "by default";
+  EXPECT_THAT(
+  Diags.Diagnostics,
+  ElementsAre(DiagMessage(
+  "Invalid Background value 'Foo'. Valid values are Build, Skip.")));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -113,7 +113,7 @@
   // Set up two identical TUs, foo and bar.
   // They define foo::one and bar::one.
   std::vector Cmds;
-  for (std::string Name : {"foo", "bar"}) {
+  for (std::string Name : {"foo", "bar", "baz"}) {
 std::string Filename = Name + ".cpp";
 std::string Header = Name + ".h";
 FS.Files[Filename] = "#include \"" + Header + "\"";
@@ -126,11 +126,14 @@
   }
   // Context provider that installs a configuration mutating foo's command.
   // This causes it to define foo::two instead of foo::one.
+  // It also disables indexing of baz entirely.
   auto ContextProvider = [](PathRef P) {
 Config C;
 if (P.endswith("foo.cpp"))
   C.CompileFlags.Edits.push_back(
   [](std::vector &Argv) { Argv.push_back("-Done=two"); });
+if (P.endswith("baz.cpp"))
+  C.Index.Background = Config::BackgroundPolicy::Skip;
 return Context::current().derive(Config::Key, std::move(C));
   };
   // Create the background index.
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -8,6 +8,7 @@
 
 #include "index/Background.h"
 #include "Compiler.h"
+#include "Config.h"
 #include "Headers.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
@@ -354,6 +355,14 @@
 // staleness.
 std::vector
 BackgroundIndex::loadProject(std::vector MainFiles) {
+  // Drop files where background indexing is disabled in config.
+  if (ContextProvider)
+llvm::erase_if(MainFiles, [&](const std::string &TU) {
+  // Load the config for each TU, as indexing may be selectively enabled.
+  WithContext WithProvidedContext(ContextProvider(TU));
+  return Config::current().Index.Background ==
+ Config::BackgroundPolicy::Skip;
+});
   Rebuilder.startLoading();
   // Load shards for all of the mainfiles.
   const std::vector Result =
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "ConfigFragment.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -66,6 +67,13 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::IndexBlock &F, Node &N) {
+DictParser Dict("Index", this);
+Dict.handle("Background",
+[&](Node &N) { F.Background = scalarValue(N, "Background"); });
+Dict.parse(N);
+  }
+
   // Helper for parsing mapping nodes (dictionaries).
   // We don't use YamlIO as we want to control over unknown keys.
   class DictParser {
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -120,6 +120,17 @@
   struct CompileFlagsBlock {
 std::vector> Add;
   } CompileFlags;
+
+  /// Controls how clangd und

[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:196
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
+  constexpr static llvm::SourceMgr::DiagKind Warning =
+  llvm::SourceMgr::DK_Warning;

kadircet wrote:
> nit: I know this isn't what the patch is about, but what about making 
> these lambdas/functions of Out.diag with first parameter bound to DK_Error or 
> DK_Warning? (or having them at an anonymous namespace instead of a member)
I'm not sure about the readability of lambdas here, if we wanted to do this I 
think the simplest thing would just be to make them methods?

Can we defer this question until after the branch cut?



Comment at: clang-tools-extra/clangd/ConfigFragment.h:124
+
+  /// Controls how clangd understands code outside the current file.
+  struct IndexBlock {

kadircet wrote:
> i think saying "outside the current file" might not be the best description. 
> Maybe rather "Controls clangd's understanding for entities(symbols, refs, 
> relations) in the codebase" ?
I don't really understand this - clangd's understanding for symbols, refs and, 
relations is all based on the AST, except when we're talking about outside the 
current file. (And refs and relations aren't terms that make much sense to 
users)

Expanded this a bit and gave an example.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:129
+/// Legal values are "Build" or "Skip".
+llvm::Optional> Background;
+  };

kadircet wrote:
> why not store the enum in here directly and generate diagnostics during the 
> parsing stage?
> 
> we seem to be doing this for syntactic errors, e.g. `expected a dictionary` 
> and getting an unexpected enum value sounds similar to that?
Because then you have to do it twice for JSON vs YAML. You're right this isn't 
really principled, and I think more data points will help us resolve it.
OK to defer this to after the branch cut?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83768



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


[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.

let's ship it!




Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:196
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
+  constexpr static llvm::SourceMgr::DiagKind Warning =
+  llvm::SourceMgr::DK_Warning;

sammccall wrote:
> kadircet wrote:
> > nit: I know this isn't what the patch is about, but what about making 
> > these lambdas/functions of Out.diag with first parameter bound to DK_Error 
> > or DK_Warning? (or having them at an anonymous namespace instead of a 
> > member)
> I'm not sure about the readability of lambdas here, if we wanted to do this I 
> think the simplest thing would just be to make them methods?
> 
> Can we defer this question until after the branch cut?
SG :+1:



Comment at: clang-tools-extra/clangd/ConfigFragment.h:129
+/// Legal values are "Build" or "Skip".
+llvm::Optional> Background;
+  };

sammccall wrote:
> kadircet wrote:
> > why not store the enum in here directly and generate diagnostics during the 
> > parsing stage?
> > 
> > we seem to be doing this for syntactic errors, e.g. `expected a dictionary` 
> > and getting an unexpected enum value sounds similar to that?
> Because then you have to do it twice for JSON vs YAML. You're right this 
> isn't really principled, and I think more data points will help us resolve it.
> OK to defer this to after the branch cut?
sure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83768



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


[PATCH] D83722: [PowerPC] Add options to control paired vector memops support

2020-07-14 Thread Baptiste Saleil via Phabricator via cfe-commits
bsaleil updated this revision to Diff 277847.
bsaleil added a comment.

Add test to check `paired-vector-memops` is supported by the targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83722

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/ppc-dependent-options.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/lib/Target/PowerPC/PPCScheduleP9.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/test/CodeGen/PowerPC/future-check-features.ll

Index: llvm/test/CodeGen/PowerPC/future-check-features.ll
===
--- llvm/test/CodeGen/PowerPC/future-check-features.ll
+++ llvm/test/CodeGen/PowerPC/future-check-features.ll
@@ -1,9 +1,9 @@
-; RUN: llc -mattr=pcrelative-memops,prefix-instrs -verify-machineinstrs \
-; RUN:   -mtriple=powerpc64le-unknown-unknown -ppc-asm-full-reg-names \
-; RUN:   %s -o - 2>&1 | FileCheck %s
-; RUN: llc -mattr=pcrelative-memops,prefix-instrs -verify-machineinstrs \
-; RUN:   -mtriple=powerpc64-unknown-unknown -ppc-asm-full-reg-names \
-; RUN:   %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops \
+; RUN:   -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -ppc-asm-full-reg-names %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops \
+; RUN:   -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:   -ppc-asm-full-reg-names %s -o - 2>&1 | FileCheck %s
 
 define dso_local signext i32 @f() {
 entry:
Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -146,6 +146,7 @@
   bool VectorsUseTwoUnits;
   bool UsePPCPreRASchedStrategy;
   bool UsePPCPostRASchedStrategy;
+  bool PairedVectorMemops;
   bool PredictableSelectIsExpensive;
 
   POPCNTDKind HasPOPCNTD;
@@ -266,6 +267,7 @@
   bool hasP10Vector() const { return HasP10Vector; }
   bool hasPrefixInstrs() const { return HasPrefixInstrs; }
   bool hasPCRelativeMemops() const { return HasPCRelativeMemops; }
+  bool pairedVectorMemops() const { return PairedVectorMemops; }
   bool hasMFOCRF() const { return HasMFOCRF; }
   bool hasISEL() const { return HasISEL; }
   bool hasBPERMD() const { return HasBPERMD; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -122,6 +122,7 @@
   VectorsUseTwoUnits = false;
   UsePPCPreRASchedStrategy = false;
   UsePPCPostRASchedStrategy = false;
+  PairedVectorMemops = false;
   PredictableSelectIsExpensive = false;
 
   HasPOPCNTD = POPCNTD_Unavailable;
Index: llvm/lib/Target/PowerPC/PPCScheduleP9.td
===
--- llvm/lib/Target/PowerPC/PPCScheduleP9.td
+++ llvm/lib/Target/PowerPC/PPCScheduleP9.td
@@ -43,8 +43,8 @@
   // Do not support QPX (Quad Processing eXtension), SPE (Signal Processing
   // Engine), prefixed instructions on Power 9, PC relative mem ops, or
   // instructions introduced in ISA 3.1.
-  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, PCRelativeMemops,
- IsISA3_1];
+  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, PairedVectorMemops,
+ PCRelativeMemops, IsISA3_1];
 
 }
 
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -427,6 +427,7 @@
 
 def PrefixInstrs : Predicate<"Subtarget->hasPrefixInstrs()">;
 def IsISA3_1 : Predicate<"Subtarget->isISA3_1()">;
+def PairedVectorMemops : Predicate<"PPCSubTarget->pairedVectorMemops()">;
 
 let Predicates = [PrefixInstrs] in {
   let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -237,6 +237,10 @@
   SubtargetFeature<"pcrelative-memops", "HasPCRelativeMemops", "true",
"Enable PC relative Memory Ops",
[FeatureISA3_0]>;
+def FeaturePairedVectorMemops:
+  SubtargetFeature<"paired-vector-memops", "PairedVectorMemops", "true",
+   "32Byte load and store instructions",
+   [FeatureISA3_0]>;
 
 def FeaturePredictableSelectIsExpensive :
   SubtargetFeature<"predictable-select-expensive",
@@ -342,7 +346,7 @@
   // still exist with the exception of those we know are Power9 specific.
   li

[PATCH] D81676: [MSP430] Align the toolchain definition with the TI's msp430-gcc v9.2.0

2020-07-14 Thread Anatoly Trosinenko via Phabricator via cfe-commits
atrosinenko added a comment.

Thank you! Will probably land it tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81676



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


[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

2020-07-14 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes updated this revision to Diff 277853.
c-rhodes added a comment.

Address @sdesmalen comments


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

https://reviews.llvm.org/D83550

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/arm-feature-sve-bits-macro.c
  clang/test/Sema/attr-arm-sve-vector-bits.c

Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- /dev/null
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=2048 -fallow-half-arguments-and-returns %s
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+
+// Define valid fixed-width SVE types
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint16_t fixed_int16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint64_t fixed_int64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svuint8_t fixed_uint8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint16_t fixed_uint16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint32_t fixed_uint32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint64_t fixed_uint64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svfloat16_t fixed_float16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat32_t fixed_float32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbfloat16_t fixed_bfloat16_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
+
+// Attribute must have a single argument
+typedef svint8_t no_argument __attribute__((arm_sve_vector_bits)); // expected-error {{'arm_sve_vector_bits' attribute takes one argument}}
+typedef svint8_t two_arguments __attribute__((arm_sve_vector_bits(2, 4))); // expected-error {{'arm_sve_vector_bits' attribute takes one argument}}
+
+// The number of SVE vector bits must be an integer constant expression
+typedef svint8_t non_int_size1 __attribute__((arm_sve_vector_bits(2.0)));   // expected-error {{'arm_sve_vector_bits' attribute requires an integer constant}}
+typedef svint8_t non_int_size2 __attribute__((arm_sve_vector_bits("256"))); // expected-error {{'arm_sve_vector_bits' attribute requires an integer constant}}
+
+// Attribute must be attached to a single SVE vector or predicate type.
+typedef void *badtype1 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'void *'}}
+typedef int badtype2 __attribute__((arm_sve_vector_bits(N)));   // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'int'}}
+typedef float badtype3 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'float'}}
+typedef svint8x2_t badtype4 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svint8x2_t' (aka '__clang_svint8x2_t')}}
+typedef svfloat32x3_t badtype5 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svfloat32x3_t' (aka '__clang_svfloat32x3_t')}}
Index: clang/test/Sema/arm-feature-sve-bits-macro.c
===
--- /dev/null
+++ clang/test/Sema/arm-feature-sve-bits-macro.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sve -fsyntax-only -verify -D__ARM_FEATURE_SVE_BITS=512 -fallow-half-arguments-and-returns %s
+
+#include 
+
+#define N 512
+
+// __ARM_FEATURE_SVE_BITS macro must be defined for attribute to have any effect
+#undef __ARM_FEATURE_SVE_BITS
+typedef svint8_t macro_undefined __attribute__((arm_sve_vector_bits(N))); // expected-error {{__ARM_FEATURE_SVE_BITS is not defined}}
+
+// __ARM_FEATURE_SVE_BITS macro must have a single argument
+#define __ARM_FEATURE_SVE_BITS
+

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

2020-07-14 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment.

In D83494#2148868 , @dokyungs wrote:

> Addressed Matt's comments.
>
> A major change in this round that needs explanation is introduction of 
> FuzzerPlatform.h. Previously I defined `strstr` and `strcasestr` with `extern 
> "C++"` to workaround conflicting definition errors resulting from including 
> . But since including it is not necessary when compiling this 
> interceptor module, this patch now separates out platform related macros from 
> FuzzerDef.h into FuzzerPlatform.h, and the module includes FuzzerPlatform.h, 
> not FuzzerDef.h.


What was the conflicting definition error?  Does string.h have inline 
definitions for those functions?




Comment at: compiler-rt/test/fuzzer/memcmp.test:8
+RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | 
FileCheck %s --check-prefix=CHECK2
+CHECK2: BINGO

CHECK1 and CHECK2 is unnecessary.  Let's use the same CHECK for both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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


[clang] 2c2a297 - [clang][NFC] Add 'override' keyword to virtual function overrides

2020-07-14 Thread Logan Smith via cfe-commits

Author: Logan Smith
Date: 2020-07-14T08:59:57-07:00
New Revision: 2c2a297bb6d1ce9752a69c8c18a58eacc6d3f961

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

LOG: [clang][NFC] Add 'override' keyword to virtual function overrides

This patch adds override to several overriding virtual functions that were 
missing the keyword within the clang/ directory. These were found by the new 
-Wsuggest-override.

Added: 


Modified: 
clang/include/clang/AST/DeclOpenMP.h
clang/lib/AST/Interp/InterpFrame.h
clang/lib/AST/OSLog.cpp
clang/lib/Basic/Targets/OSTargets.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclOpenMP.h 
b/clang/include/clang/AST/DeclOpenMP.h
index 437feaba28fb..154ecb977692 100644
--- a/clang/include/clang/AST/DeclOpenMP.h
+++ b/clang/include/clang/AST/DeclOpenMP.h
@@ -129,7 +129,7 @@ class OMPDeclareReductionDecl final : public ValueDecl, 
public DeclContext {
   /// the declare reduction construct is declared inside compound statement.
   LazyDeclPtr PrevDeclInScope;
 
-  virtual void anchor();
+  void anchor() override;
 
   OMPDeclareReductionDecl(Kind DK, DeclContext *DC, SourceLocation L,
   DeclarationName Name, QualType Ty,
@@ -228,7 +228,7 @@ class OMPDeclareMapperDecl final : public ValueDecl, public 
DeclContext {
 
   LazyDeclPtr PrevDeclInScope;
 
-  virtual void anchor();
+  void anchor() override;
 
   OMPDeclareMapperDecl(Kind DK, DeclContext *DC, SourceLocation L,
DeclarationName Name, QualType Ty,

diff  --git a/clang/lib/AST/Interp/InterpFrame.h 
b/clang/lib/AST/Interp/InterpFrame.h
index b8391b0bcf92..304e2ad66537 100644
--- a/clang/lib/AST/Interp/InterpFrame.h
+++ b/clang/lib/AST/Interp/InterpFrame.h
@@ -45,16 +45,16 @@ class InterpFrame final : public Frame {
   void popArgs();
 
   /// Describes the frame with arguments for diagnostic purposes.
-  void describe(llvm::raw_ostream &OS);
+  void describe(llvm::raw_ostream &OS) override;
 
   /// Returns the parent frame object.
-  Frame *getCaller() const;
+  Frame *getCaller() const override;
 
   /// Returns the location of the call to the frame.
-  SourceLocation getCallLocation() const;
+  SourceLocation getCallLocation() const override;
 
   /// Returns the caller.
-  const FunctionDecl *getCallee() const;
+  const FunctionDecl *getCallee() const override;
 
   /// Returns the current function.
   Function *getFunction() const { return Func; }

diff  --git a/clang/lib/AST/OSLog.cpp b/clang/lib/AST/OSLog.cpp
index df2f808728cf..094c0102854b 100644
--- a/clang/lib/AST/OSLog.cpp
+++ b/clang/lib/AST/OSLog.cpp
@@ -55,9 +55,9 @@ class OSLogFormatStringHandler
 ArgsData.reserve(Args.size());
   }
 
-  virtual bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
- const char *StartSpecifier,
- unsigned SpecifierLen) {
+  bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
+ const char *StartSpecifier,
+ unsigned SpecifierLen) override {
 if (!FS.consumesDataArgument() &&
 FS.getConversionSpecifier().getKind() !=
 clang::analyze_format_string::ConversionSpecifier::PrintErrno)

diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index af04b75392f5..cfa362bef1b1 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -821,7 +821,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyOSTargetInfo
 : public OSTargetInfo {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
-MacroBuilder &Builder) const {
+MacroBuilder &Builder) const override {
 // A common platform macro.
 if (Opts.POSIXThreads)
   Builder.defineMacro("_REENTRANT");

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 515a2e9690ed..22bf35dbd0cb 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1066,7 +1066,7 @@ static IsTupleLike isTupleLike(Sema &S, SourceLocation 
Loc, QualType T,
 TemplateArgumentListInfo &Args;
 ICEDiagnoser(LookupResult &R, TemplateArgumentListInfo &Args)
 : R(R), Args(Args) {}
-void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) {
+void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) override {
   S.Diag(Loc, diag::err_decomp_decl_std_tuple_size_not_constant)

[PATCH] D81442: [PowerPC] Add clang options to control MMA support

2020-07-14 Thread Baptiste Saleil via Phabricator via cfe-commits
bsaleil updated this revision to Diff 277858.
bsaleil added a comment.

Add test to check that the `mma` option is supported by the targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81442

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/ppc-dependent-options.cpp
  clang/test/Preprocessor/init-ppc64.c
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/lib/Target/PowerPC/PPCScheduleP9.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/test/CodeGen/PowerPC/future-check-features.ll

Index: llvm/test/CodeGen/PowerPC/future-check-features.ll
===
--- llvm/test/CodeGen/PowerPC/future-check-features.ll
+++ llvm/test/CodeGen/PowerPC/future-check-features.ll
@@ -1,7 +1,7 @@
-; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops \
+; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops,mma \
 ; RUN:   -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
 ; RUN:   -ppc-asm-full-reg-names %s -o - 2>&1 | FileCheck %s
-; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops \
+; RUN: llc -mattr=pcrelative-memops,prefix-instrs,paired-vector-memops,mma \
 ; RUN:   -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
 ; RUN:   -ppc-asm-full-reg-names %s -o - 2>&1 | FileCheck %s
 
Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -108,6 +108,7 @@
   bool HasP10Vector;
   bool HasPrefixInstrs;
   bool HasPCRelativeMemops;
+  bool HasMMA;
   bool HasFCPSGN;
   bool HasFSQRT;
   bool HasFRE, HasFRES, HasFRSQRTE, HasFRSQRTES;
@@ -267,6 +268,7 @@
   bool hasP10Vector() const { return HasP10Vector; }
   bool hasPrefixInstrs() const { return HasPrefixInstrs; }
   bool hasPCRelativeMemops() const { return HasPCRelativeMemops; }
+  bool hasMMA() const { return HasMMA; }
   bool pairedVectorMemops() const { return PairedVectorMemops; }
   bool hasMFOCRF() const { return HasMFOCRF; }
   bool hasISEL() const { return HasISEL; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -78,6 +78,7 @@
   HasP8Crypto = false;
   HasP9Vector = false;
   HasP9Altivec = false;
+  HasMMA = false;
   HasP10Vector = false;
   HasPrefixInstrs = false;
   HasPCRelativeMemops = false;
Index: llvm/lib/Target/PowerPC/PPCScheduleP9.td
===
--- llvm/lib/Target/PowerPC/PPCScheduleP9.td
+++ llvm/lib/Target/PowerPC/PPCScheduleP9.td
@@ -43,8 +43,8 @@
   // Do not support QPX (Quad Processing eXtension), SPE (Signal Processing
   // Engine), prefixed instructions on Power 9, PC relative mem ops, or
   // instructions introduced in ISA 3.1.
-  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, PairedVectorMemops,
- PCRelativeMemops, IsISA3_1];
+  let UnsupportedFeatures = [HasQPX, HasSPE, PrefixInstrs, MMA,
+ PairedVectorMemops, PCRelativeMemops, IsISA3_1];
 
 }
 
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -428,6 +428,7 @@
 def PrefixInstrs : Predicate<"Subtarget->hasPrefixInstrs()">;
 def IsISA3_1 : Predicate<"Subtarget->isISA3_1()">;
 def PairedVectorMemops : Predicate<"PPCSubTarget->pairedVectorMemops()">;
+def MMA : Predicate<"PPCSubTarget->hasMMA()">;
 
 let Predicates = [PrefixInstrs] in {
   let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -241,6 +241,10 @@
   SubtargetFeature<"paired-vector-memops", "PairedVectorMemops", "true",
"32Byte load and store instructions",
[FeatureISA3_0]>;
+def FeatureMMA : SubtargetFeature<"mma", "HasMMA", "true",
+  "Enable MMA instructions",
+  [FeatureP8Vector, FeatureP9Altivec,
+   FeaturePairedVectorMemops]>;
 
 def FeaturePredictableSelectIsExpensive :
   SubtargetFeature<"predictable-select-expensive",
@@ -346,7 +350,8 @@
   // still exist with the exception of those we know are Power9 specific.
   list P10AdditionalFeatures =
 [DirectivePwr10, FeatureISA3_1, FeaturePrefixInstrs,
- FeaturePCRelativeMemops, FeatureP10Vector, FeaturePairedV

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

2020-07-14 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 277861.
NoQ added a comment.

Remove the `ShouldDisplayPathNotes` option as it never was an `AnalyzerOption` 
to begin with and it only makes things worse.

F12338321: 488ny6.jpg 


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

https://reviews.llvm.org/D67420

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -150,7 +150,7 @@
   break;
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)\
   case PD_##NAME:  \
-CREATEFN(*Opts.get(), PathConsumers, OutDir, PP, CTU); \
+CREATEFN(Opts->getDiagOpts(), PathConsumers, OutDir, PP, CTU); \
 break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 default:
Index: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
@@ -34,20 +34,17 @@
 /// type to the standard error, or to to compliment many others. Emits detailed
 /// diagnostics in textual format for the 'text' output type.
 class TextDiagnostics : public PathDiagnosticConsumer {
+  PathDiagnosticConsumerOptions DiagOpts;
   DiagnosticsEngine &DiagEng;
   const LangOptions &LO;
-  const bool IncludePath = false;
-  const bool ShouldEmitAsError = false;
-  const bool ApplyFixIts = false;
-  const bool ShouldDisplayCheckerName = false;
+  bool ShouldDisplayPathNotes;
 
 public:
-  TextDiagnostics(DiagnosticsEngine &DiagEng, const LangOptions &LO,
-  bool ShouldIncludePath, const AnalyzerOptions &AnOpts)
-  : DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
-ShouldEmitAsError(AnOpts.AnalyzerWerror),
-ApplyFixIts(AnOpts.ShouldApplyFixIts),
-ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
+  TextDiagnostics(PathDiagnosticConsumerOptions DiagOpts,
+  DiagnosticsEngine &DiagEng, const LangOptions &LO,
+  bool ShouldDisplayPathNotes)
+  : DiagOpts(std::move(DiagOpts)), DiagEng(DiagEng), LO(LO),
+ShouldDisplayPathNotes(ShouldDisplayPathNotes) {}
   ~TextDiagnostics() override {}
 
   StringRef getName() const override { return "TextDiagnostics"; }
@@ -56,13 +53,13 @@
   bool supportsCrossFileDiagnostics() const override { return true; }
 
   PathGenerationScheme getGenerationScheme() const override {
-return IncludePath ? Minimal : None;
+return ShouldDisplayPathNotes ? Minimal : None;
   }
 
   void FlushDiagnosticsImpl(std::vector &Diags,
 FilesMade *filesMade) override {
 unsigned WarnID =
-ShouldEmitAsError
+DiagOpts.ShouldDisplayWarningsAsErrors
 ? DiagEng.getCustomDiagID(DiagnosticsEngine::Error, "%0")
 : DiagEng.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
 unsigned NoteID = DiagEng.getCustomDiagID(DiagnosticsEngine::Note, "%0");
@@ -72,7 +69,7 @@
 auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
ArrayRef Ranges,
ArrayRef Fixits) {
-  if (!ApplyFixIts) {
+  if (!DiagOpts.ShouldApplyFixIts) {
 DiagEng.Report(Loc, ID) << String << Ranges << Fixits;
 return;
   }
@@ -92,9 +89,10 @@
  E = Diags.end();
  I != E; ++I) {
   const PathDiagnostic *PD = *I;
-  std::string WarningMsg =
-  (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
-  .str();
+  std::string WarningMsg = (DiagOpts.ShouldDisplayDiagnosticName
+? " [" + PD->getCheckerName() + "]"
+: "")
+   .str();
 
   reportPiece(WarnID, PD->getLocation().asLocation(),
   (PD->getShortDescription() + WarningMsg).str(),
@@ -110,7 +108,7 @@
 Piece->getFixits());
   }
 
-  if (!IncludePath)
+  if (!ShouldDisplayPathNotes)
 continue;
 
   // Then, add the path notes if necessary.
@@ -125,7 +123,7 @@
   }
 }
 
-if (!ApplyFixIts || Repls.empty())
+if (Repls.empty())
   return;
 

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

2020-07-14 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as done.
balazske added inline comments.



Comment at: clang/test/Analysis/stream.c:274-284
 // Check that "location uniqueing" works.
 // This results in reporting only one occurence of resource leak for a stream.
 void check_leak_noreturn_2() {
   FILE *F1 = tmpfile();
   if (!F1)
 return;
   if (Test == 1) {

NoQ wrote:
> balazske wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > balazske wrote:
> > > > > NoQ wrote:
> > > > > > balazske wrote:
> > > > > > > Szelethus wrote:
> > > > > > > > Why did this change? Is there a sink in the return branch?
> > > > > > > The change is probably because D83115. Because the "uniqueing" 
> > > > > > > one resource leak is reported from the two possible, and the 
> > > > > > > order changes somehow (probably not the shortest is found first).
> > > > > > The shortest should still be found first. I strongly suggest 
> > > > > > debugging this. Looks like a bug in suppress-on-sink.
> > > > > There is no code that ensures that the shortest path is reported. In 
> > > > > this case one equivalence class is created with both bug reports. If 
> > > > > `SuppressOnSink` is false the last one is returned from the list, 
> > > > > otherwise the first one 
> > > > > (`PathSensitiveBugReporter::findReportInEquivalenceClass`), this 
> > > > > causes the difference (seems to be unrelated to D83115).
> > > > > There is no code that ensures that the shortest path is reported.
> > > > 
> > > > There absolutely should be -- See the summary of D65379 for more info, 
> > > > CTRL+F "shortest" helps quite a bit as well. For each bug report, we 
> > > > create a bug path (a path in the exploded graph from the root to the 
> > > > sepcific bug reports error node), and sort them by length.
> > > > 
> > > > It all feels super awkward -- 
> > > > `PathSensitiveBugReporter::findReportInEquivalenceClass` picks out a 
> > > > bug report from an equivalence class as you described, but that will 
> > > > only be reported if it is a `BasicBugReport` (as implemented by 
> > > > `PathSensitiveBugReporter::generateDiagnosticForConsumerMap`), 
> > > > otherwise it should go through the graph cutting process etc.
> > > > 
> > > > So at the end of the day, the shortest path should appear still? 
> > > > 
> > > @balazske I spent a lot of my GSoC rewriting some especially miserable 
> > > code in `BugReporter.cpp`, please hunt me down if you need any help there.
> > Can we say that the one path in this case is shorter than the other? The 
> > difference is only at the "taking true/false branch" at the `if` in line 
> > 280. Maybe both have equal length. The notes are taken always from the 
> > single picked report that is returned from `findReportInEquivalenceClass` 
> > and these notes can contain different source locations (reports in a single 
> > equivalence class can have different locations, really this makes the 
> > difference between them?).  
> > There is no code that ensures that the shortest path is reported.
> 
> We would have been soo screwed if this was so. In fact, grepping 
> for "shortest" in the entire clang sources immediately points you to the 
> right line of code.
> 
> > the last one is returned from the list, otherwise the first one
> 
> The example report is not actually used later for purposes other than 
> extracting information common to all reports in the path. The array of valid 
> reports is used instead, and it's supposed to be sorted.
> 
> > Can we say that the one path in this case is shorter than the other?
> 
> Dump the graph and see for yourself. I expect a call with an argument and an 
> implicit lvalue-to-rvalue conversion of that argument to take a lot more 
> nodes than an empty return statement.
I found the sorting code, it revealed that the problem has other reason: It 
happens only if //-analyzer-output text// is not passed to clang. It looks like 
that in this case the path in `PathDiagnostic` is not collected, so 
`BugReporter::FlushReport` will use the one report instance from the bug report 
class (that is different if `SuppressOnSink` is set or not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83120



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


[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

2020-07-14 Thread Fady Ghanim via Phabricator via cfe-commits
fghanim added a comment.

> I'll address the nits.

Thanks :)

> Unsure if that is all.

depends on the comment about `AllocaBuilder`




Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:433
 
-  Builder.SetInsertPoint(OuterFn->getEntryBlock().getFirstNonPHI());
-  AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr");
-  AllocaInst *ZeroAddr = Builder.CreateAlloca(Int32, nullptr, "zero.addr");
+  IRBuilder<> AllocaBuilder(AllocaIP.getBlock(), AllocaIP.getPoint());
+

jdoerfert wrote:
> fghanim wrote:
> > Here and elsewhere: You seem to have forgotten to undo the changes 
> > introduced by using a separate `AllocaBuilder`?
> > 
> > Also, Nit: before using `AllocaIP`, either add an assert that it is not 
> > empty, or alternatively, add logic to just set it to entry block of 
> > `outerfn` if it is
> > Here and elsewhere: You seem to have forgotten to undo the changes 
> > introduced by using a separate AllocaBuilder?
> 
> I don't get this. These changes are on purpose. 
Oh, my bad. I assumed that since we now pass `allocaip` to communicate where is 
the insertion point, using `builder` the way we used to is sufficient, and this 
was leftover code. So now what purpose does `AllocaBuilder` serve?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82470



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


[clang] fbb30c3 - [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

2020-07-14 Thread Logan Smith via cfe-commits

Author: Logan Smith
Date: 2020-07-14T09:36:43-07:00
New Revision: fbb30c31fefcf992ddb287087e8ca766eeddb59d

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

LOG: [clang] Add 'override' to virtual function overrides generated by 
ClangAttrEmitter

ClangAttrEmitter.cpp generates ParsedAttr derived classes with virtual 
overrides in them (which end up in AttrParsedAttrImpl.inc); this patch ensures 
these generated functions are marked override, and not (redundantly) virtual.

I hesitate to say NFC since this does of course affect the behavior of the 
generator code, but the generated code behaves the same as it did before, so 
it's NFC in that sense.

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

Added: 


Modified: 
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 1b9fd2d29bf9..bd20e447a950 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2012,10 +2012,10 @@ 
PragmaClangAttributeSupport::generateStrictConformsTo(const Record &Attr,
 return;
   // Generate a function that constructs a set of matching rules that describe
   // to which declarations the attribute should apply to.
-  OS << "virtual void getPragmaAttributeMatchRules("
+  OS << "void getPragmaAttributeMatchRules("
  << "llvm::SmallVectorImpl> &MatchRules, const LangOptions &LangOpts) const {\n";
+ << ", bool>> &MatchRules, const LangOptions &LangOpts) const override 
{\n";
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
   std::vector Subjects = 
SubjectObj->getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
@@ -3519,8 +3519,8 @@ static void GenerateAppertainsTo(const Record &Attr, 
raw_ostream &OS) {
   // at all (for instance because it was applied to a type), or that the caller
   // has determined that the check should fail (perhaps prior to the creation
   // of the declaration).
-  OS << "virtual bool diagAppertainsToDecl(Sema &S, ";
-  OS << "const ParsedAttr &Attr, const Decl *D) const {\n";
+  OS << "bool diagAppertainsToDecl(Sema &S, ";
+  OS << "const ParsedAttr &Attr, const Decl *D) const override {\n";
   OS << "  if (";
   for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
 // If the subject has custom code associated with it, use the generated
@@ -3594,8 +3594,8 @@ static void GenerateLangOptRequirements(const Record &R,
   if (LangOpts.empty())
 return;
 
-  OS << "virtual bool diagLangOpts(Sema &S, const ParsedAttr &Attr) ";
-  OS << "const {\n";
+  OS << "bool diagLangOpts(Sema &S, const ParsedAttr &Attr) ";
+  OS << "const override {\n";
   OS << "  auto &LangOpts = S.LangOpts;\n";
   OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
@@ -3639,7 +3639,7 @@ static void GenerateTargetRequirements(const Record &Attr,
   std::string Test;
   bool UsesT = GenerateTargetSpecificAttrChecks(R, Arches, Test, &FnName);
 
-  OS << "virtual bool existsInTarget(const TargetInfo &Target) const {\n";
+  OS << "bool existsInTarget(const TargetInfo &Target) const override {\n";
   if (UsesT)
 OS << "  const llvm::Triple &T = Target.getTriple(); (void)T;\n";
   OS << "  return " << Test << ";\n";
@@ -3664,8 +3664,8 @@ static void GenerateSpellingIndexToSemanticSpelling(const 
Record &Attr,
   std::string Enum = CreateSemanticSpellings(Spellings, 
SemanticToSyntacticMap);
   std::string Name = Attr.getName().str() + "AttrSpellingMap";
 
-  OS << "virtual unsigned spellingIndexToSemanticSpelling(";
-  OS << "const ParsedAttr &Attr) const {\n";
+  OS << "unsigned spellingIndexToSemanticSpelling(";
+  OS << "const ParsedAttr &Attr) const override {\n";
   OS << Enum;
   OS << "  unsigned Idx = Attr.getAttributeSpellingListIndex();\n";
   WriteSemanticSpellingSwitch("Idx", SemanticToSyntacticMap, OS);
@@ -3678,8 +3678,8 @@ static void GenerateHandleDeclAttribute(const Record 
&Attr, raw_ostream &OS) {
 return;
 
   // Generate a function which just converts from ParsedAttr to the Attr type.
-  OS << "virtual AttrHandling handleDeclAttribute(Sema &S, Decl *D,";
-  OS << "const ParsedAttr &Attr) const {\n";
+  OS << "AttrHandling handleDeclAttribute(Sema &S, Decl *D,";
+  OS << "const ParsedAttr &Attr) const override {\n";
   OS << "  D->addAttr(::new (S.Context) " << Attr.getName();
   OS << "Attr(S.Context, Attr));\n";
   OS << "  return AttributeApplied;\n";



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


[PATCH] D83616: [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

2020-07-14 Thread Logan Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfbb30c31fefc: [clang] Add 'override' to virtual 
function overrides generated by… (authored by logan-5).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83616

Files:
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2012,10 +2012,10 @@
 return;
   // Generate a function that constructs a set of matching rules that describe
   // to which declarations the attribute should apply to.
-  OS << "virtual void getPragmaAttributeMatchRules("
+  OS << "void getPragmaAttributeMatchRules("
  << "llvm::SmallVectorImpl> &MatchRules, const LangOptions &LangOpts) const {\n";
+ << ", bool>> &MatchRules, const LangOptions &LangOpts) const override 
{\n";
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
   std::vector Subjects = 
SubjectObj->getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
@@ -3519,8 +3519,8 @@
   // at all (for instance because it was applied to a type), or that the caller
   // has determined that the check should fail (perhaps prior to the creation
   // of the declaration).
-  OS << "virtual bool diagAppertainsToDecl(Sema &S, ";
-  OS << "const ParsedAttr &Attr, const Decl *D) const {\n";
+  OS << "bool diagAppertainsToDecl(Sema &S, ";
+  OS << "const ParsedAttr &Attr, const Decl *D) const override {\n";
   OS << "  if (";
   for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
 // If the subject has custom code associated with it, use the generated
@@ -3594,8 +3594,8 @@
   if (LangOpts.empty())
 return;
 
-  OS << "virtual bool diagLangOpts(Sema &S, const ParsedAttr &Attr) ";
-  OS << "const {\n";
+  OS << "bool diagLangOpts(Sema &S, const ParsedAttr &Attr) ";
+  OS << "const override {\n";
   OS << "  auto &LangOpts = S.LangOpts;\n";
   OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
@@ -3639,7 +3639,7 @@
   std::string Test;
   bool UsesT = GenerateTargetSpecificAttrChecks(R, Arches, Test, &FnName);
 
-  OS << "virtual bool existsInTarget(const TargetInfo &Target) const {\n";
+  OS << "bool existsInTarget(const TargetInfo &Target) const override {\n";
   if (UsesT)
 OS << "  const llvm::Triple &T = Target.getTriple(); (void)T;\n";
   OS << "  return " << Test << ";\n";
@@ -3664,8 +3664,8 @@
   std::string Enum = CreateSemanticSpellings(Spellings, 
SemanticToSyntacticMap);
   std::string Name = Attr.getName().str() + "AttrSpellingMap";
 
-  OS << "virtual unsigned spellingIndexToSemanticSpelling(";
-  OS << "const ParsedAttr &Attr) const {\n";
+  OS << "unsigned spellingIndexToSemanticSpelling(";
+  OS << "const ParsedAttr &Attr) const override {\n";
   OS << Enum;
   OS << "  unsigned Idx = Attr.getAttributeSpellingListIndex();\n";
   WriteSemanticSpellingSwitch("Idx", SemanticToSyntacticMap, OS);
@@ -3678,8 +3678,8 @@
 return;
 
   // Generate a function which just converts from ParsedAttr to the Attr type.
-  OS << "virtual AttrHandling handleDeclAttribute(Sema &S, Decl *D,";
-  OS << "const ParsedAttr &Attr) const {\n";
+  OS << "AttrHandling handleDeclAttribute(Sema &S, Decl *D,";
+  OS << "const ParsedAttr &Attr) const override {\n";
   OS << "  D->addAttr(::new (S.Context) " << Attr.getName();
   OS << "Attr(S.Context, Attr));\n";
   OS << "  return AttributeApplied;\n";


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2012,10 +2012,10 @@
 return;
   // Generate a function that constructs a set of matching rules that describe
   // to which declarations the attribute should apply to.
-  OS << "virtual void getPragmaAttributeMatchRules("
+  OS << "void getPragmaAttributeMatchRules("
  << "llvm::SmallVectorImpl> &MatchRules, const LangOptions &LangOpts) const {\n";
+ << ", bool>> &MatchRules, const LangOptions &LangOpts) const override {\n";
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
   std::vector Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
@@ -3519,8 +3519,8 @@
   // at all (for instance because it was applied to a type), or that the caller
   // has determined that the check should fail (perhaps prior to the creation
   // of the declaration).
-  OS << "virtual bool diagAppertainsToDecl(Sema &S, ";
-  OS << "const ParsedAttr &Attr, const Decl *D) const {\n";
+  OS << "bool diagAppertainsToDecl(Sema &S, ";
+  OS << "const ParsedAttr &Attr, const Decl *D) const override {\n";
   OS << "  if (";
   for (auto I = Subjects.begin(), E = Subjects.e

[PATCH] D83079: [clang][aarch64] Generate preprocessor macros for -march=armv8.6a+sve.

2020-07-14 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli updated this revision to Diff 277879.
fpetrogalli added a comment.

Removed extra tests that are not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83079

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -112,6 +112,24 @@
 // CHECK-SVE-F64MM: __ARM_FEATURE_SVE 1
 // CHECK-SVE-F64MM: __ARM_FEATURE_SVE_MATMUL_FP64 1
 
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.5-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE-8_5 %s
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_MATMUL_INT8 1
+// CHECK-SVE-8_5: __ARM_FEATURE_SVE 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.6-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE-8_6 %s
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_MATMUL_INT8 1
+
+// RUN: %clang -target aarch64-none-linux-gnu 
-march=armv8.6-a+sve+noi8mm+nobf16+nof32mm -x c -E -dM %s -o - | FileCheck 
--check-prefix=CHECK-SVE-8_6-NOFEATURES %s
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_MATMUL_INT8 1
+// CHECK-SVE-8_6-NOFEATURES: __ARM_FEATURE_SVE 1
+
 // The following tests may need to be revised in the future since
 // SVE2 is currently still part of Future Architecture Technologies
 // (https://developer.arm.com/docs/ddi0602/latest)
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -365,6 +365,10 @@
 }
   }
 
+  auto V8_6Pos = llvm::find(Features, "+v8.6a");
+  if (V8_6Pos != std::end(Features))
+V8_6Pos = Features.insert(std::next(V8_6Pos), {"+i8mm", "+bf16"});
+
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
 if (A->getOption().matches(options::OPT_mno_unaligned_access))


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -112,6 +112,24 @@
 // CHECK-SVE-F64MM: __ARM_FEATURE_SVE 1
 // CHECK-SVE-F64MM: __ARM_FEATURE_SVE_MATMUL_FP64 1
 
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.5-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-8_5 %s
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_5-NOT: __ARM_FEATURE_SVE_MATMUL_INT8 1
+// CHECK-SVE-8_5: __ARM_FEATURE_SVE 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.6-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-8_6 %s
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_6: __ARM_FEATURE_SVE_MATMUL_INT8 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.6-a+sve+noi8mm+nobf16+nof32mm -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-8_6-NOFEATURES %s
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_BF16 1
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_MATMUL_FP32 1
+// CHECK-SVE-8_6-NOFEATURES-NOT: __ARM_FEATURE_SVE_MATMUL_INT8 1
+// CHECK-SVE-8_6-NOFEATURES: __ARM_FEATURE_SVE 1
+
 // The following tests may need to be revised in the future since
 // SVE2 is currently still part of Future Architecture Technologies
 // (https://developer.arm.com/docs/ddi0602/latest)
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -365,6 +365,10 @@
 }
   }
 
+  auto V8_6Pos = llvm::find(Features, "+v8.6a");
+  if (V8_6Pos != std::end(Features))
+V8_6Pos = Features.insert(std::next(V8_6Pos), {"+i8mm", "+bf16"});
+
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
 if (A->getOption().matches(options::OPT_mno_unaligned_access))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83055: [clang][Driver] Fix tool path priority test failures

2020-07-14 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

I have reproduced the type 1 failure and I confirm that this patch fixes the 
issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83055



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


[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

2020-07-14 Thread Mitch Phillips via Phabricator via cfe-commits
hctim added inline comments.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:242
+bool SanitizerArgs::needsFuzzerInterceptors() const {
+  return needsFuzzer() && !needsAsanRt() && !needsHwasanRt() &&
+ !needsTsanRt() && !needsMsanRt();

HWASan doesn't have interceptors - we actually want fuzzer interceptors under 
`needsFuzzer() && needsHwasanRt()` (just remove the `!needsHwasanRt()`).



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:52
+// NOLINTNEXTLINE
+void __sanitizer_weak_hook_memcmp(void *, const void *, const void *, size_t,
+  int);

Why not `#include ` (and below)?



Comment at: compiler-rt/lib/fuzzer/FuzzerPlatform.h:1
-//===- FuzzerDefs.h - Internal header for the Fuzzer *- C++ -* 
===//
+//===-- FuzzerInterceptors.cpp 
===//
 //

Nit - name change



Comment at: compiler-rt/lib/fuzzer/FuzzerPlatform.h:8
 
//===--===//
-// Basic definitions.
+//
+// Common platform macros.

Nit - remove blank comment



Comment at: compiler-rt/test/fuzzer/memcmp.test:8
+RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | 
FileCheck %s --check-prefix=CHECK2
+CHECK2: BINGO

morehouse wrote:
> CHECK1 and CHECK2 is unnecessary.  Let's use the same CHECK for both.
No need to have different `CHECK`'s throughout this patchset (in both tests 
we're checking for the same thing.

```
UNSUPPORTED: freebsd
RUN: %cpp_compiler %S/MemcmpTest.cpp -o %t-MemcmpTest
RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | 
FileCheck %s

RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp 
-o %t-NoAsanMemcmpTest
RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | 
FileCheck %s

CHECK: BINGO
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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


[clang] bfd6433 - Fix merging of two arity-only pack deductions.

2020-07-14 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-07-14T10:02:07-07:00
New Revision: bfd643353e6b7ca7b89c0f983ff6a24c36aed276

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

LOG: Fix merging of two arity-only pack deductions.

If we deduced the arity of a pack in two different ways, but didn't
deduce an element of the pack in either of those deductions, we'd merge
that element to produce a null template argument, which we'd incorrectly
interpret as the merge having failed.

Testcase based on one supplied by Hubert Tong.

Added: 


Modified: 
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/test/SemaTemplate/deduction.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index f3641afbbf8a..5392be57a3aa 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -355,7 +355,7 @@ checkDeducedTemplateArguments(ASTContext &Context,
   TemplateArgument Merged = checkDeducedTemplateArguments(
   Context, DeducedTemplateArgument(*XA, X.wasDeducedFromArrayBound()),
   DeducedTemplateArgument(*YA, Y.wasDeducedFromArrayBound()));
-  if (Merged.isNull())
+  if (Merged.isNull() && !(XA->isNull() && YA->isNull()))
 return DeducedTemplateArgument();
   NewPack.push_back(Merged);
 }

diff  --git a/clang/test/SemaTemplate/deduction.cpp 
b/clang/test/SemaTemplate/deduction.cpp
index 5218543ab8a4..a068bcaea048 100644
--- a/clang/test/SemaTemplate/deduction.cpp
+++ b/clang/test/SemaTemplate/deduction.cpp
@@ -581,3 +581,19 @@ namespace PR44890 {
 return w.get<0>();
   }
 }
+
+namespace merge_size_only_deductions {
+#if __cplusplus >= 201703L
+  // Based on a testcase by Hubert Tong.
+  template struct X {};
+  template struct Y {};
+  template struct id { using Type = T; };
+
+  template
+int f(X, Y, X);
+
+  using size_t = __SIZE_TYPE__;
+  int a = f(X(), Y<(size_t)1, (size_t)2>(), X, 
id>());
+  int b = f(X(), Y<1, 2>(), X, id>());
+#endif
+}



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


[PATCH] D83768: [clangd] Config: Index.Background

2020-07-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdbf486c0de92: [clangd] Config: Index.Background (authored by 
sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D83768?vs=277844&id=277885#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83768

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -100,6 +100,22 @@
   EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
 }
 
+TEST_F(ConfigCompileTests, Index) {
+  Frag.Index.Background.emplace("Skip");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+
+  Frag = {};
+  Frag.Index.Background.emplace("Foo");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build)
+  << "by default";
+  EXPECT_THAT(
+  Diags.Diagnostics,
+  ElementsAre(DiagMessage(
+  "Invalid Background value 'Foo'. Valid values are Build, Skip.")));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -113,7 +113,7 @@
   // Set up two identical TUs, foo and bar.
   // They define foo::one and bar::one.
   std::vector Cmds;
-  for (std::string Name : {"foo", "bar"}) {
+  for (std::string Name : {"foo", "bar", "baz"}) {
 std::string Filename = Name + ".cpp";
 std::string Header = Name + ".h";
 FS.Files[Filename] = "#include \"" + Header + "\"";
@@ -126,11 +126,14 @@
   }
   // Context provider that installs a configuration mutating foo's command.
   // This causes it to define foo::two instead of foo::one.
+  // It also disables indexing of baz entirely.
   auto ContextProvider = [](PathRef P) {
 Config C;
 if (P.endswith("foo.cpp"))
   C.CompileFlags.Edits.push_back(
   [](std::vector &Argv) { Argv.push_back("-Done=two"); });
+if (P.endswith("baz.cpp"))
+  C.Index.Background = Config::BackgroundPolicy::Skip;
 return Context::current().derive(Config::Key, std::move(C));
   };
   // Create the background index.
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -8,6 +8,7 @@
 
 #include "index/Background.h"
 #include "Compiler.h"
+#include "Config.h"
 #include "Headers.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
@@ -354,6 +355,14 @@
 // staleness.
 std::vector
 BackgroundIndex::loadProject(std::vector MainFiles) {
+  // Drop files where background indexing is disabled in config.
+  if (ContextProvider)
+llvm::erase_if(MainFiles, [&](const std::string &TU) {
+  // Load the config for each TU, as indexing may be selectively enabled.
+  WithContext WithProvidedContext(ContextProvider(TU));
+  return Config::current().Index.Background ==
+ Config::BackgroundPolicy::Skip;
+});
   Rebuilder.startLoading();
   // Load shards for all of the mainfiles.
   const std::vector Result =
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "ConfigFragment.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -70,6 +71,13 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::IndexBlock &F, Node &N) {
+DictParser Dict("Index", this);
+Dict.handle("Background",
+[&](Node &N) { F.Background = scalarValue(N, "Background"); });
+Dict.parse(N);
+  }
+
   // Helper for parsing mapping nodes (dictionaries).
   // We don't use YamlIO as we want to control over unknown keys.
   class DictParser {
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -150,6 +150,

  1   2   3   >