date:20200908

[PATCH] D76604: [Analyzer] Model `size()` member function of containers

2020-09-08 Thread Balogh , Ádám via Phabricator via cfe-commits

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

Fixes.


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

https://reviews.llvm.org/D76604

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.h
  clang/test/Analysis/container-modeling.cpp

Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -96,6 +96,40 @@
   // expected-note@-3   {{FALSE}}
 }
 
+// size()
+
+void size0(const std::vector& V) {
+  assert(V.size() == 0); // expected-note{{'?' condition is true}}
+ // expected-note@-1{{Assuming the condition is true}}
+
+  clang_analyzer_eval(clang_analyzer_container_end(V) ==
+  clang_analyzer_container_begin(V));
+  // expected-warning@-2{{TRUE}}
+  // expected-note@-3   {{TRUE}}
+}
+
+void size1(const std::vector& V) {
+  assert(V.size() == 1); // expected-note{{'?' condition is true}}
+ // expected-note@-1{{Assuming the condition is true}}
+
+  clang_analyzer_eval(clang_analyzer_container_end(V) ==
+  clang_analyzer_container_begin(V) + 1);
+  // expected-warning@-2{{TRUE}}
+  // expected-note@-3   {{TRUE}}
+}
+
+void size12(std::vector& V) {
+  assert(V.size() == 1); // expected-note{{'?' condition is true}}
+ // expected-note@-1{{Assuming the condition is true}}
+
+  V.push_back(1);
+
+  clang_analyzer_eval(clang_analyzer_container_end(V) ==
+  clang_analyzer_container_begin(V) + 2);
+  // expected-warning@-2{{TRUE}}
+  // expected-note@-3   {{TRUE}}
+}
+
 
 ///
 /// C O N T A I N E R   M O D I F I E R S
Index: clang/lib/StaticAnalyzer/Checkers/Iterator.h
===
--- clang/lib/StaticAnalyzer/Checkers/Iterator.h
+++ clang/lib/StaticAnalyzer/Checkers/Iterator.h
@@ -187,6 +187,8 @@
 assumeComparison(ProgramStateRef State, SymbolRef Sym1, SymbolRef Sym2,
  DefinedSVal RetVal, OverloadedOperatorKind Op);
 
+ProgramStateRef relateSymbols(ProgramStateRef State, SymbolRef Sym1,
+  SymbolRef Sym2, bool Equal);
 bool compare(ProgramStateRef State, SymbolRef Sym1, SymbolRef Sym2,
  BinaryOperator::Opcode Opc);
 bool compare(ProgramStateRef State, NonLoc NL1, NonLoc NL2,
Index: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
+++ clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
@@ -18,9 +18,6 @@
 namespace ento {
 namespace iterator {
 
-ProgramStateRef relateSymbols(ProgramStateRef State, SymbolRef Sym1,
-  SymbolRef Sym2, bool Equal);
-
 bool isIteratorType(const QualType &Type) {
   if (Type->isPointerType())
 return true;
Index: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -47,6 +47,8 @@
  SVal Cont, SVal RetVal) const;
   void handleEmpty(CheckerContext &C, const Expr *CE, const Expr *ContE,
SVal Cont, SVal RetVal) const;
+  void handleSize(CheckerContext &C, const Expr *CE, const Expr *ContE,
+  SVal Cont, SVal RetVal) const;
   void handleAssign(CheckerContext &C, const Expr *CE, const Expr *ContE,
 SVal Cont, SVal RetVal) const;
   void handleClear(CheckerContext &C, const Expr *CE, const Expr *ContE,
@@ -102,6 +104,7 @@
 
 // Capacity
 {{0, "empty", 0}, &ContainerModeling::handleEmpty},
+{{0, "size", 0}, &ContainerModeling::handleSize},
 
 // Modifiers
 {{0, "clear", 0}, &ContainerModeling::handleClear},
@@ -170,6 +173,8 @@
 SymbolRef rebaseSymbol(ProgramStateRef State, SValBuilder &SVB, SymbolRef Expr,
 SymbolRef OldSym, SymbolRef NewSym);
 bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
+const llvm::APSInt* calculateSize(ProgramStateRef State, SymbolRef Begin,
+  SymbolRef End);
 
 } // namespace
 
@@ -446,6 +451,72 @@
   }
 }
 
+void ContainerModeling::handleSize(CheckerContext &C, const Expr *CE,
+   const Expr *, SVal Cont, SVal RetVal) const {
+  const auto *ContReg = Cont.getAsRegion();
+  if (!ContReg)
+return;
+
+  ContReg = ContReg->getMostDerivedObjectRegion();
+
+  auto State = C.getState();
+
+  State = createContainerBegin(State, ContReg, CE, C.getASTContext().LongTy,
+   C.

[PATCH] D76604: [Analyzer] Model `size()` member function of containers

2020-09-08 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:456
+   SVal RetVal) const {
+  const auto *ContReg = Cont.getAsRegion();
+  if (!ContReg)

Szelethus wrote:
> martong wrote:
> > Just out of curiosity: How do we handle containers that do not have a 
> > contiguous memory region? Balanced trees, bucketed hash-maps, etc.
> I suspect that this is referring to the memory address of the container 
> object, not the storage of the elements.
Yes. The region just serves to identify the container. It is not necessarily 
the region where the data is stored.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:482-483
+// of the container (the difference between its `begin()` and `end()` to
+// this size. Function `relateSymbols()` returns null if it contradits
+// the current size.
+const auto CalcEnd =

martong wrote:
> How? I don't see how does it access the `size`.
As explained between the parenthesis, the actual size of the container is the 
difference between its `begin()` and its `end()`. If we have this difference, 
then we know the actual size. The other value we may have is the return value 
of the `size()` function. We either have one of them, both or none. If we have 
one, then we adjust the other. If we have both, then we check for consistency, 
and generated a sink if they are inconsistent. If we have none, then we do 
nothing.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:492
+  } else {
+if (CalcSize) {
+  // If the current size is a concrete integer, bind this to the return

martong wrote:
> What if we have both `RetSize` and `CalcSize`? Should we check their values 
> for consistency? (And perhaps adding another sink node if we have 
> inconsistency?)
This is handled in the `if` branch: having `CalcSize` means that we know the 
difference between the `begin()` and the `end()`, thus inconsistency between 
`RetSize` and `CalcSize` is the same as inconstistency between `CalcEnd` and 
`EndSym`. The comment above explains that if there is such inconsistency, then 
`relateSymbols()` returns a null pointer which we assign to `State`. At the end 
of this functions we generate a sink whenever `State` is a null pointer.


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

https://reviews.llvm.org/D76604

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-08 Thread James Henderson via Phabricator via cfe-commits

jhenderson added a comment.

In D78938#2259342 , @BRevzin wrote:

> In D78938#2258557 , @jhenderson 
> wrote:
>
>> Not that I have anything particularly against this, but won't this likely 
>> rot fairly rapidly? It's not like LLVM is even on C++17 let alone C++20 yet, 
>> so trying to make it work like the latter when it's just going to break 
>> again seems a bit like wasted effort to me.
>
> People will want to write C++20 programs that use LLVM headers, so I think 
> it's important to help let them do that. Sure, it may rot, but incremental 
> fixes down the line will be smaller.

Makes sense, thanks.




Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817
+  return IsASCII ? "^" : (const char *)u8"\u2548";
 case LineChar::RangeMid:
-  return IsASCII ? "|" : u8"\u2503";
+  return IsASCII ? "|" : (const char *)u8"\u2503";
 case LineChar::RangeEnd:
-  return IsASCII ? "v" : u8"\u253b";
+  return IsASCII ? "v" : (const char *)u8"\u253b";
 case LineChar::LabelVert:
+  return IsASCII ? "|" : (const char *)u8"\u2502";

BRevzin wrote:
> jhenderson wrote:
> > This seems unrelated to comparison checking?
> > This seems unrelated to comparison checking?
> 
> It is unrelated. But In C++20, `u8` literals become their own type so this no 
> longer compiled and I wanted to ensure that I could actually run the tests. 
Could it be a pre-requisite patch then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D87225: [clangd] When finding refs for a template specialization, do not return refs to other specializations

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1150
 // DeclRelation::Underlying.
-DeclRelationSet Relations = DeclRelation::TemplatePattern |
+DeclRelationSet Relations = DeclRelation::TemplateInstantiation |
 DeclRelation::Alias | DeclRelation::Underlying;

IIRC, clangd's index doesn't distinguish refs of template 
specifications/primary templates, all refs are treated as primary templates -- 
if we query refs for a template specification from the index, we'll get no 
results, so we will end up with refs from the main file (which are from the 
AST) only.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1574
+   class Foo {};
void func([[Fo^o]]);
   )cpp",

hmm, the new behavior seems wired (at least in this case), I thought it is a 
bug at first glance, then I realized that this is xrefs for template 
specification. I'd expect to see  the primary template in refs in xrefs result.

however if there is an explicit template specification, excluding the primary 
template maybe reasonable.

```
template 
class Foo {};
template <> class [[Foo]] {};
void func([[Fo^o]]);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87225

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

[PATCH] D87256: [clangd] Avoid relations being overwritten in a header shard

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

+@kadircet, he is tracking on this -- we had some discussion internally last 
week, but we don't reply on that thread yet (sorry).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87256

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

[clang] 2168dbf - getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-08 Thread Hans Wennborg via cfe-commits


Author: Shivanshu Goyal
Date: 2020-09-08T10:21:18+02:00
New Revision: 2168dbf4cc766dfb552076d9b1e84b00122b7993

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

LOG: getClangStripDependencyFileAdjuster(): Do not remove -M args when using 
MSVC cl driver

MSVC's cl.exe has a few command line arguments which start with -M such
as "-MD", "-MDd", "-MT", "-MTd", "-MP".
These arguments are not dependency file generation related, and these
arguments were being removed by getClangStripDependencyFileAdjuster()
which was wrong.

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

Added: 


Modified: 
clang/lib/Tooling/ArgumentsAdjusters.cpp
clang/unittests/Tooling/ToolingTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/ArgumentsAdjusters.cpp 
b/clang/lib/Tooling/ArgumentsAdjusters.cpp
index a857b57fbf7b..bcfb5b39a077 100644
--- a/clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ b/clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,6 +21,16 @@
 namespace clang {
 namespace tooling {
 
+static StringRef getDriverMode(const CommandLineArguments &Args) {
+  for (const auto &Arg : Args) {
+StringRef ArgRef = Arg;
+if (ArgRef.consume_front("--driver-mode=")) {
+  return ArgRef;
+}
+  }
+  return StringRef();
+}
+
 /// Add -fsyntax-only option and drop options that triggers output generation.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
@@ -93,20 +103,28 @@ ArgumentsAdjuster 
getClangStripSerializeDiagnosticAdjuster() {
 
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+auto UsingClDriver = (getDriverMode(Args) == "cl");
+
 CommandLineArguments AdjustedArgs;
 for (size_t i = 0, e = Args.size(); i < e; ++i) {
   StringRef Arg = Args[i];
-  // All dependency-file options begin with -M. These include -MM,
-  // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-  !Arg.startswith("-showIncludes")) {
-AdjustedArgs.push_back(Args[i]);
+
+  // These flags take an argument: -MX foo. Skip the next argument also.
+  if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
+++i;
 continue;
   }
+  // When not using the cl driver mode, dependency file generation options
+  // begin with -M. These include -MM, -MF, -MG, -MP, -MT, -MQ, -MD, and
+  // -MMD.
+  if (!UsingClDriver && Arg.startswith("-M"))
+continue;
+  // Under MSVC's cl driver mode, dependency file generation is controlled
+  // using /showIncludes
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
+continue;
 
-  if (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")
-// These flags take an argument: -MX foo. Skip the next argument also.
-++i;
+  AdjustedArgs.push_back(Args[i]);
 }
 return AdjustedArgs;
   };

diff  --git a/clang/unittests/Tooling/ToolingTest.cpp 
b/clang/unittests/Tooling/ToolingTest.cpp
index cc6f453284d7..691a847d5a71 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -563,6 +563,40 @@ TEST(ClangToolTest, 
StripDependencyFileAdjusterShowIncludes) {
   EXPECT_TRUE(HasFlag("-c"));
 }
 
+// Check getClangStripDependencyFileAdjuster doesn't strip args when using the
+// MSVC cl.exe driver
+TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) {
+  FixedCompilationDatabase Compilations(
+  "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-O1", "-MTd", "-MP"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_TRUE(HasFlag("-MD"));
+  EXPECT_TRUE(HasFlag("-MDd"));
+  EXPECT_TRUE(HasFlag("-MT"));
+  EXPECT_TRUE(HasFlag("-O1"));
+  EXPECT_TRUE(HasFlag("-MTd"));
+  EXPECT_TRUE(HasFlag("-MP"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(



___
cfe-commi

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-08 Thread Hans Wennborg via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2168dbf4cc76: getClangStripDependencyFileAdjuster(): Do not 
remove -M args when using MSVC cl… (authored by shivanshu3, committed by hans).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

Files:
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -563,6 +563,40 @@
   EXPECT_TRUE(HasFlag("-c"));
 }
 
+// Check getClangStripDependencyFileAdjuster doesn't strip args when using the
+// MSVC cl.exe driver
+TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) {
+  FixedCompilationDatabase Compilations(
+  "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-O1", "-MTd", "-MP"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_TRUE(HasFlag("-MD"));
+  EXPECT_TRUE(HasFlag("-MDd"));
+  EXPECT_TRUE(HasFlag("-MT"));
+  EXPECT_TRUE(HasFlag("-O1"));
+  EXPECT_TRUE(HasFlag("-MTd"));
+  EXPECT_TRUE(HasFlag("-MP"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -21,6 +21,16 @@
 namespace clang {
 namespace tooling {
 
+static StringRef getDriverMode(const CommandLineArguments &Args) {
+  for (const auto &Arg : Args) {
+StringRef ArgRef = Arg;
+if (ArgRef.consume_front("--driver-mode=")) {
+  return ArgRef;
+}
+  }
+  return StringRef();
+}
+
 /// Add -fsyntax-only option and drop options that triggers output generation.
 ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
@@ -93,20 +103,28 @@
 
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+auto UsingClDriver = (getDriverMode(Args) == "cl");
+
 CommandLineArguments AdjustedArgs;
 for (size_t i = 0, e = Args.size(); i < e; ++i) {
   StringRef Arg = Args[i];
-  // All dependency-file options begin with -M. These include -MM,
-  // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-  !Arg.startswith("-showIncludes")) {
-AdjustedArgs.push_back(Args[i]);
+
+  // These flags take an argument: -MX foo. Skip the next argument also.
+  if (!UsingClDriver && (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")) {
+++i;
 continue;
   }
+  // When not using the cl driver mode, dependency file generation options
+  // begin with -M. These include -MM, -MF, -MG, -MP, -MT, -MQ, -MD, and
+  // -MMD.
+  if (!UsingClDriver && Arg.startswith("-M"))
+continue;
+  // Under MSVC's cl driver mode, dependency file generation is controlled
+  // using /showIncludes
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
+continue;
 
-  if (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")
-// These flags take an argument: -MX foo. Skip the next argument also.
-++i;
+  AdjustedArgs.push_back(Args[i]);
 }
 return AdjustedArgs;
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87257: [clang] Traverse init-captures while indexing

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

In D87257#2260022 , @nridge wrote:

> I did try to add a test to `clang/test/Index/Core/index-source.cpp`, however 
> the output of `c-index-test` does not seem to be changed by adding this 
> reference.

Could you check `clang/test/Index/cxx14-lambdas.cpp`, looks like that file is 
testing lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87257

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

2020-09-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}

I repeat, this is a false positive. Recursive mutexes can be locked twice, 
that's the whole point.


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

https://reviews.llvm.org/D85984

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

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

2020-09-08 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski updated this revision to Diff 290435.
awarzynski added a comment.

Add missing update in lit.cfg.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/unittests/Driver/SanitizerArgsTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/CMakeLists.txt
  flang/README.md
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/FrontendTool/Utils.h
  flang/lib/CMakeLists.txt
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/CMakeLists.txt
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/CMakeLists.txt
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/emit-obj.f90
  flang/test/Flang-Driver/missing-input.f90
  flang/test/lit.cfg.py
  flang/test/lit.site.cfg.py.in
  flang/tools/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/CMakeLists.txt
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/CompilerInstanceTest.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -610,8 +610,10 @@
   continue;
 
 unsigned Flags = getInfo(Id).Flags;
+
 if (FlagsToInclude && !(Flags & FlagsToInclude))
   continue;
+
 if (Flags & FlagsToExclude)
   continue;
 
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -48,7 +48,7 @@
 unsigned ID;
 unsigned char Kind;
 unsigned char Param;
-unsigned short Flags;
+unsigned int Flags;
 unsigned short GroupID;
 unsigned short AliasID;
 const char *AliasArgs;
@@ -225,7 +225,8 @@
   /// \param Usage - USAGE: Usage
   /// \param Title - OVERVIEW: Title
   /// \param FlagsToInclude - If non-zero, only include options with any
-  /// of these flags set.
+  /// of these flags set. Takes precedence over
+  /// FlagsToExclude.
   /// \param FlagsToExclude - Exclude options with any of these flags set.
   /// \param ShowAllAliases - If true, display all options including aliases
   /// that don't have help texts. By default, we display
Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -0,0 +1,52 @@
+//===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+using namespace llvm;
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  // 1. Set-up a basic DiagnosticConsumer
+  std::string diagnosticOutput;
+  llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  auto diagPrinter = std::make_unique(
+  diagnosticsOS, new clang::DiagnosticOptions());
+
+  // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
+  CompilerInstance compInst;
+
+  // 3. Set-up DiagnosticOptions
+  auto diagOpts = new clang::DiagnosticOptions();
+  // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
+  // ensures that a chained diagnostic consumer is created so that the test can
+

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2325d6b42f09: [SyntaxTree] Ignore implicit non-leaf 
`CXXConstructExpr` (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,137 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test(){
+  // FIXME: 's...' is a declarator and '{...}' is initializer
+  [[S s0{}]];
+  [[S s1{1}]];
+  [[S s2{1, 2.}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s0'
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s1'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s2'
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_EqualBrace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test() {
+  // FIXME: '= {...}' is initializer
+  [[S s0 = {}]];
+  [[S s1 = {1}]];
+  [[S s2 = {1, 2.}]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s0'
+  |-'='
+  `-UnknownExpression
+|-'{'
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s1'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-'}'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s2'
+  |-'='
+  `-UnknownExpression
+|-'{'
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-FloatingLiteralExpression
+| `-'2.' LiteralToken
+`-'}'
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
@@ -3851,15 +3958,134 @@
   R"cpp(
 struct S {
   S(int);
+  S(int, float);
 };
-[[S s(1);]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
 )cpp",
   {R"txt(
 SimpleDeclaration
 |-'S'
 |-SimpleDeclarator Declarator
 | `-UnknownExpression
-|   |-'s'
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+void TakeX(const X&);
+void test() {
+  [[TakeX(1)]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-Unqualifi

[clang] 46f4439 - [SyntaxTree] Ignore implicit leaf `CXXConstructExpr`

2020-09-08 Thread Eduardo Caldas via cfe-commits


Author: Eduardo Caldas
Date: 2020-09-08T09:44:23Z
New Revision: 46f4439dc9bf9b8cfee0001b6752c3d074c83b00

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

LOG: [SyntaxTree] Ignore implicit leaf `CXXConstructExpr`

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

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index e5389ae4eff4..72083eeefa31 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit calls to default constructors.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index fe89e0d7d1a2..00e18057d7be 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@ namespace n {
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@ SimpleDeclaration
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@ SimpleDeclaration
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@ SimpleDeclaration
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@ SimpleDeclaration
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 



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

[clang] 134455a - [SyntaxTree] Ignore implicit `CXXFunctionalCastExpr` wrapping constructor

2020-09-08 Thread Eduardo Caldas via cfe-commits


Author: Eduardo Caldas
Date: 2020-09-08T09:44:23Z
New Revision: 134455a07c1f1de4cff62a6afb4ccd98b98343ec

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

LOG: [SyntaxTree] Ignore implicit `CXXFunctionalCastExpr` wrapping constructor

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

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 72083eeefa31..bb2b1494793a 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -60,9 +61,25 @@ static Expr *IgnoreImplicitConstructorSingleStep(Expr *E) {
   return E;
 }
 
+// In:
+// struct X {
+//   X(int)
+// };
+// X x = X(1);
+// Ignores the implicit `CXXFunctionalCastExpr` that wraps
+// `CXXConstructExpr X(1)`.
+static Expr *IgnoreCXXFunctionalCastExprWrappingConstructor(Expr *E) {
+  if (auto *F = dyn_cast(E)) {
+if (F->getCastKind() == CK_ConstructorConversion)
+  return F->getSubExpr();
+  }
+  return E;
+}
+
 static Expr *IgnoreImplicit(Expr *E) {
   return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
- IgnoreImplicitConstructorSingleStep);
+ IgnoreImplicitConstructorSingleStep,
+ IgnoreCXXFunctionalCastExprWrappingConstructor);
 }
 
 LLVM_ATTRIBUTE_UNUSED

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index 00e18057d7be..7a106e9297b9 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -4069,7 +4069,6 @@ struct X {
   X(int);
 };
 X test() {
-  // FIXME: Remove `UnknownExpression` due to implicit `CXXFunctionalCastExpr`
   [[return X(1);]]
 }
 )cpp",
@@ -4077,12 +4076,11 @@ X test() {
 ReturnStatement Statement
 |-'return' IntroducerKeyword
 |-UnknownExpression ReturnValue
-| `-UnknownExpression
-|   |-'X'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'X'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
 `-';'
 )txt"}));
 }



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

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG46f4439dc9bf: [SyntaxTree] Ignore implicit leaf 
`CXXConstructExpr` (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86700

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit calls to default constructors.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -548,9 +548,6 @@
   struct S { };
 }
 void test() {
-  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
-  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
-  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -564,8 +561,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -575,8 +571,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
@@ -608,8 +603,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s1'
+  `-'s1'
 )txt",
R"txt(
 SimpleDeclaration
@@ -623,8 +617,7 @@
 | `-'::' ListDelimiter
 |-'S'
 `-SimpleDeclarator Declarator
-  `-UnknownExpression
-`-'s2'
+  `-'s2'
 )txt"}));
 }
 
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1132,6 +1132,14 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) {
+// Ignore the implicit calls to default constructors.
+if ((S->getNumArgs() == 0 || isa(S->getArg(0))) &&
+S->getParenOrBraceRange().isInvalid())
+  return true;
+return RecursiveASTVisitor::WalkUpFromCXXConstructExpr(S);
+  }
+
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
 // To construct a syntax tree of the same shape for calls to built-in and
 // user-defined operators, ignore the `DeclRefExpr` that refers to the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2325d6b - [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

2020-09-08 Thread Eduardo Caldas via cfe-commits


Author: Eduardo Caldas
Date: 2020-09-08T09:44:23Z
New Revision: 2325d6b42f096bf93d2ab0bed7096759e5c96ce8

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

LOG: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

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

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index a9f326439a2a..e5389ae4eff4 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -44,8 +45,28 @@
 
 using namespace clang;
 
+// Ignores the implicit `CXXConstructExpr` for copy/move constructor calls
+// generated by the compiler, as well as in implicit conversions like the one
+// wrapping `1` in `X x = 1;`.
+static Expr *IgnoreImplicitConstructorSingleStep(Expr *E) {
+  if (auto *C = dyn_cast(E)) {
+auto NumArgs = C->getNumArgs();
+if (NumArgs == 1 || (NumArgs > 1 && isa(C->getArg(1 
{
+  Expr *A = C->getArg(0);
+  if (C->getParenOrBraceRange().isInvalid())
+return A;
+}
+  }
+  return E;
+}
+
+static Expr *IgnoreImplicit(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
+ IgnoreImplicitConstructorSingleStep);
+}
+
 LLVM_ATTRIBUTE_UNUSED
-static bool isImplicitExpr(Expr *E) { return E->IgnoreImplicit() != E; }
+static bool isImplicitExpr(Expr *E) { return IgnoreImplicit(E) != E; }
 
 namespace {
 /// Get start location of the Declarator from the TypeLoc.
@@ -740,7 +761,7 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
   for (auto *D : DS->decls())
 Builder.noticeDeclWithoutSemicolon(D);
 } else if (auto *E = dyn_cast_or_null(S)) {
-  return RecursiveASTVisitor::TraverseStmt(E->IgnoreImplicit());
+  return RecursiveASTVisitor::TraverseStmt(IgnoreImplicit(E));
 }
 return RecursiveASTVisitor::TraverseStmt(S);
   }
@@ -1579,7 +1600,7 @@ void syntax::TreeBuilder::markStmtChild(Stmt *Child, 
NodeRole Role) {
 void syntax::TreeBuilder::markExprChild(Expr *Child, NodeRole Role) {
   if (!Child)
 return;
-  Child = Child->IgnoreImplicit();
+  Child = IgnoreImplicit(Child);
 
   syntax::Tree *ChildNode = Mapping.find(Child);
   assert(ChildNode != nullptr);

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index aab20008a497..fe89e0d7d1a2 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@ TEST_P(SyntaxTreeTest, OverloadedOperator_Plus) {
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored 
already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,137 @@ TranslationUnit Detached
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { 
+  S();
+  S(int);
+  S(int, float);
+};
+void test(){
+  // FIXME: 's...' is a declarator and '{...}' is initializer
+  [[S s0{}]];
+  [[S s1{1}]];
+  [[S s2{1, 2.}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  `-UnknownExpression
+|-'s0'
+|-'{'
+`-'}'
+  )txt",

[PATCH] D87229: [SyntaxTree] Ignore implicit `CXXFunctionalCastExpr` wrapping constructor

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG134455a07c1f: [SyntaxTree] Ignore implicit 
`CXXFunctionalCastExpr` wrapping constructor (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87229

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -4069,7 +4069,6 @@
   X(int);
 };
 X test() {
-  // FIXME: Remove `UnknownExpression` due to implicit `CXXFunctionalCastExpr`
   [[return X(1);]]
 }
 )cpp",
@@ -4077,12 +4076,11 @@
 ReturnStatement Statement
 |-'return' IntroducerKeyword
 |-UnknownExpression ReturnValue
-| `-UnknownExpression
-|   |-'X'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'X'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
 `-';'
 )txt"}));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -60,9 +61,25 @@
   return E;
 }
 
+// In:
+// struct X {
+//   X(int)
+// };
+// X x = X(1);
+// Ignores the implicit `CXXFunctionalCastExpr` that wraps
+// `CXXConstructExpr X(1)`.
+static Expr *IgnoreCXXFunctionalCastExprWrappingConstructor(Expr *E) {
+  if (auto *F = dyn_cast(E)) {
+if (F->getCastKind() == CK_ConstructorConversion)
+  return F->getSubExpr();
+  }
+  return E;
+}
+
 static Expr *IgnoreImplicit(Expr *E) {
   return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
- IgnoreImplicitConstructorSingleStep);
+ IgnoreImplicitConstructorSingleStep,
+ IgnoreCXXFunctionalCastExprWrappingConstructor);
 }
 
 LLVM_ATTRIBUTE_UNUSED


Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -4069,7 +4069,6 @@
   X(int);
 };
 X test() {
-  // FIXME: Remove `UnknownExpression` due to implicit `CXXFunctionalCastExpr`
   [[return X(1);]]
 }
 )cpp",
@@ -4077,12 +4076,11 @@
 ReturnStatement Statement
 |-'return' IntroducerKeyword
 |-UnknownExpression ReturnValue
-| `-UnknownExpression
-|   |-'X'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'X'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
 `-';'
 )txt"}));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -60,9 +61,25 @@
   return E;
 }
 
+// In:
+// struct X {
+//   X(int)
+// };
+// X x = X(1);
+// Ignores the implicit `CXXFunctionalCastExpr` that wraps
+// `CXXConstructExpr X(1)`.
+static Expr *IgnoreCXXFunctionalCastExprWrappingConstructor(Expr *E) {
+  if (auto *F = dyn_cast(E)) {
+if (F->getCastKind() == CK_ConstructorConversion)
+  return F->getSubExpr();
+  }
+  return E;
+}
+
 static Expr *IgnoreImplicit(Expr *E) {
   return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
- IgnoreImplicitConstructorSingleStep);
+ IgnoreImplicitConstructorSingleStep,
+ IgnoreCXXFunctionalCastExprWrappingConstructor);
 }
 
 LLVM_ATTRIBUTE_UNUSED
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas updated this revision to Diff 290439.
eduucaldas added a comment.

Test init declarator with default arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87249

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2733,6 +2733,54 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, CallExpression_DefaultArguments) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+void f(int i = 1, char c = '2');
+void test() {
+  [[f()]];
+  [[f(1)]];
+  [[f(1, '2')]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| |-IntegerLiteralExpression ListElement
+| | `-'1' LiteralToken
+| |-',' ListDelimiter
+| `-CharacterLiteralExpression ListElement
+|   `-''2'' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -3986,6 +4034,56 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int i = 1, float = 2.);
+};
+[[S s0;]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-'s0'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
   if (!GetParam().isCXX()) {
 return;
@@ -4114,6 +4212,48 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, ConstructorCall_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int i = 1, char c = '2');
+};
+X test() {
+  auto x0 = [[X()]];
+  auto x1 = [[X(1)]];
+  auto x2 = [[X(1, '2')]];
+}
+)cpp",
+  {R"txt(
+UnknownExpression
+|-'X'
+|-'('
+`-')'
+)txt",
+   R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-')'
+)txt",
+   R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-CharacterLiteralExpression
+| `-''2'' LiteralToken
+`-')'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, TypeConversion_FunctionalNotation) {
   if (!GetParam().isCXX()) {
 return;
@@ -4375,6 +4515,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParametersAndQualifiers_InFreeFunctions_Default_One) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func1([[int a = 1]]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+`-SimpleDeclaration ListElement
+  |-'int'
+  `-SimpleDeclarator Declarator
+|-'a'
+|-'='
+`-IntegerLiteralExpression
+  `-'1' LiteralToken
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest,
+   ParametersAndQualifiers_InFreeFunctions_Default_Multiple) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func2([[int *ap, int a = 1, char c = '2']]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'*'
+|   `-'ap'
+|-',' ListDelimiter
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'a'
+|   |-'='
+|   `-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-',' ListDelimiter
+`-SimpleDeclaration ListElement
+  |-'char'
+  `-SimpleDeclarator Declarator
+|-'c'
+|-'='
+`-CharacterLiteralExpression
+  `-''2'' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest,
ParametersAndQualifiers_InVariadicFunctionTemplate_ParameterPack) {
   if (!GetPara

[clang] f5087d5 - [SyntaxTree] Fix crash on functions with default arguments.

2020-09-08 Thread Eduardo Caldas via cfe-commits


Author: Eduardo Caldas
Date: 2020-09-08T09:49:30Z
New Revision: f5087d5c7248104b6580c7b079ed5f227332c2ef

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

LOG: [SyntaxTree] Fix crash on functions with default arguments.

* Do not visit `CXXDefaultArgExpr`
* To build `CallArguments` nodes, just go through non-default arguments

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

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index bb2b1494793a..1942290b5abc 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -154,6 +154,13 @@ struct GetStartLoc : TypeLocVisitor {
 };
 } // namespace
 
+static CallExpr::arg_range dropDefaultArgs(CallExpr::arg_range Args) {
+  auto firstDefaultArg = std::find_if(Args.begin(), Args.end(), [](auto it) {
+return isa(it);
+  });
+  return llvm::make_range(Args.begin(), firstDefaultArg);
+}
+
 static syntax::NodeKind getOperatorNodeKind(const CXXOperatorCallExpr &E) {
   switch (E.getOperator()) {
   // Comparison
@@ -,7 +1118,11 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
-  syntax::CallArguments *buildCallArguments(CallExpr::arg_range Args) {
+  /// Builds `CallArguments` syntax node from arguments that appear in source
+  /// code, i.e. not default arguments.
+  syntax::CallArguments *
+  buildCallArguments(CallExpr::arg_range ArgsAndDefaultArgs) {
+auto Args = dropDefaultArgs(ArgsAndDefaultArgs);
 for (const auto &Arg : Args) {
   Builder.markExprChild(Arg, syntax::NodeRole::ListElement);
   const auto *DelimiterToken =
@@ -1233,6 +1244,8 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
 }
   }
 
+  bool WalkUpFromCXXDefaultArgExpr(CXXDefaultArgExpr *S) { return true; }
+
   bool WalkUpFromNamespaceDecl(NamespaceDecl *S) {
 auto Tokens = Builder.getDeclarationRange(S);
 if (Tokens.front().kind() == tok::coloncolon) {

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp 
b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index 7a106e9297b9..225885437267 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2733,6 +2733,54 @@ CallExpression Expression
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, CallExpression_DefaultArguments) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+void f(int i = 1, char c = '2');
+void test() {
+  [[f()]];
+  [[f(1)]];
+  [[f(1, '2')]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| |-IntegerLiteralExpression ListElement
+| | `-'1' LiteralToken
+| |-',' ListDelimiter
+| `-CharacterLiteralExpression ListElement
+|   `-''2'' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -3986,6 +4034,56 @@ SimpleDeclaration
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int i = 1, float = 2.);
+};
+[[S s0;]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-'s0'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
   if (!GetParam().isCXX()) {
 return;
@@ -4114,6 +4212,48 @@ ReturnStatement Statement
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, ConstructorCall_DefaultArguments) {
+  if (!GetParam().isCXX(

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5087d5c7248: [SyntaxTree] Fix crash on functions with 
default arguments. (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87249

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2733,6 +2733,54 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, CallExpression_DefaultArguments) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+void f(int i = 1, char c = '2');
+void test() {
+  [[f()]];
+  [[f(1)]];
+  [[f(1, '2')]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+  )txt",
+   R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| |-IntegerLiteralExpression ListElement
+| | `-'1' LiteralToken
+| |-',' ListDelimiter
+| `-CharacterLiteralExpression ListElement
+|   `-''2'' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -3986,6 +4034,56 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S {
+  S(int i = 1, float = 2.);
+};
+[[S s0;]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-'s0'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+   R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
   if (!GetParam().isCXX()) {
 return;
@@ -4114,6 +4212,48 @@
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, ConstructorCall_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int i = 1, char c = '2');
+};
+X test() {
+  auto x0 = [[X()]];
+  auto x1 = [[X(1)]];
+  auto x2 = [[X(1, '2')]];
+}
+)cpp",
+  {R"txt(
+UnknownExpression
+|-'X'
+|-'('
+`-')'
+)txt",
+   R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-')'
+)txt",
+   R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-CharacterLiteralExpression
+| `-''2'' LiteralToken
+`-')'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, TypeConversion_FunctionalNotation) {
   if (!GetParam().isCXX()) {
 return;
@@ -4375,6 +4515,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParametersAndQualifiers_InFreeFunctions_Default_One) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func1([[int a = 1]]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+`-SimpleDeclaration ListElement
+  |-'int'
+  `-SimpleDeclarator Declarator
+|-'a'
+|-'='
+`-IntegerLiteralExpression
+  `-'1' LiteralToken
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest,
+   ParametersAndQualifiers_InFreeFunctions_Default_Multiple) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+int func2([[int *ap, int a = 1, char c = '2']]);
+)cpp",
+  {R"txt(
+ParameterDeclarationList Parameters
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'*'
+|   `-'ap'
+|-',' ListDelimiter
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'a'
+|   |-'='
+|   `-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-',' ListDelimiter
+`-SimpleDeclaration ListElement
+  |-'char'
+  `-SimpleDeclarator Declarator
+|-'c'
+|-'='
+`-CharacterLiteralExpression
+  `-''2'' LiteralToken
+)tx

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 290445.
hokein added a comment.

fix a typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguments exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguments exceeded expression nesting 
"
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguments exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprRes

[PATCH] D87250: [OpenMP] Fix typo in CodeGenFunction::EmitOMPWorksharingLoop (PR46412)

2020-09-08 Thread Simon Pilgrim via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58970eb7d1dd: [OpenMP] Fix typo in 
CodeGenFunction::EmitOMPWorksharingLoop (PR46412) (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87250

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2982,7 +2982,7 @@
   ((ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
 ScheduleKind.Schedule == OMPC_SCHEDULE_unknown) &&
!(ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic ||
- ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
+ ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
   ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_monotonic ||
   ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_monotonic;
   if ((RT.isStaticNonchunked(ScheduleKind.Schedule,


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2982,7 +2982,7 @@
   ((ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
 ScheduleKind.Schedule == OMPC_SCHEDULE_unknown) &&
!(ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic ||
- ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
+ ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
   ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_monotonic ||
   ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_monotonic;
   if ((RT.isStaticNonchunked(ScheduleKind.Schedule,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 58970eb - [OpenMP] Fix typo in CodeGenFunction::EmitOMPWorksharingLoop (PR46412)

2020-09-08 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-09-08T11:59:38+01:00
New Revision: 58970eb7d1ddd067e98f49fdcfb04373086245bc

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

LOG: [OpenMP] Fix typo in CodeGenFunction::EmitOMPWorksharingLoop (PR46412)

Fixes issue noticed by static analysis where we have a copy+paste typo, testing 
ScheduleKind.M1 twice instead of ScheduleKind.M2.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index c1def6c88f0a..b9260892bd21 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2982,7 +2982,7 @@ bool CodeGenFunction::EmitOMPWorksharingLoop(
   ((ScheduleKind.Schedule == OMPC_SCHEDULE_static ||
 ScheduleKind.Schedule == OMPC_SCHEDULE_unknown) &&
!(ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic ||
- ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
+ ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_nonmonotonic)) ||
   ScheduleKind.M1 == OMPC_SCHEDULE_MODIFIER_monotonic ||
   ScheduleKind.M2 == OMPC_SCHEDULE_MODIFIER_monotonic;
   if ((RT.isStaticNonchunked(ScheduleKind.Schedule,



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

[PATCH] D87278: [Ignore Expressions] Fix performance regression by inlining `Ignore*SingleStep`

2020-09-08 Thread Eduardo Caldas via Phabricator via cfe-commits

eduucaldas created this revision.
eduucaldas added a reviewer: gribozavr2.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
eduucaldas requested review of this revision.

We also add a `const` versions of `IgnoreExprNodes`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87278

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/IgnoreExpr.cpp

Index: clang/lib/AST/IgnoreExpr.cpp
===
--- clang/lib/AST/IgnoreExpr.cpp
+++ /dev/null
@@ -1,129 +0,0 @@
-//===--- IgnoreExpr.cpp - Ignore intermediate Expressions -===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-// This file implements common functions to ignore intermediate expression nodes
-//
-//===--===//
-
-#include "clang/AST/IgnoreExpr.h"
-#include "clang/AST/Expr.h"
-#include "clang/AST/ExprCXX.h"
-
-using namespace clang;
-
-Expr *clang::IgnoreImplicitCastsSingleStep(Expr *E) {
-  if (auto *ICE = dyn_cast(E))
-return ICE->getSubExpr();
-
-  if (auto *FE = dyn_cast(E))
-return FE->getSubExpr();
-
-  return E;
-}
-
-Expr *clang::IgnoreImplicitCastsExtraSingleStep(Expr *E) {
-  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
-  // addition to what IgnoreImpCasts() skips to account for the current
-  // behaviour of IgnoreParenImpCasts().
-  Expr *SubE = IgnoreImplicitCastsSingleStep(E);
-  if (SubE != E)
-return SubE;
-
-  if (auto *MTE = dyn_cast(E))
-return MTE->getSubExpr();
-
-  if (auto *NTTP = dyn_cast(E))
-return NTTP->getReplacement();
-
-  return E;
-}
-
-Expr *clang::IgnoreCastsSingleStep(Expr *E) {
-  if (auto *CE = dyn_cast(E))
-return CE->getSubExpr();
-
-  if (auto *FE = dyn_cast(E))
-return FE->getSubExpr();
-
-  if (auto *MTE = dyn_cast(E))
-return MTE->getSubExpr();
-
-  if (auto *NTTP = dyn_cast(E))
-return NTTP->getReplacement();
-
-  return E;
-}
-
-Expr *clang::IgnoreLValueCastsSingleStep(Expr *E) {
-  // Skip what IgnoreCastsSingleStep skips, except that only
-  // lvalue-to-rvalue casts are skipped.
-  if (auto *CE = dyn_cast(E))
-if (CE->getCastKind() != CK_LValueToRValue)
-  return E;
-
-  return IgnoreCastsSingleStep(E);
-}
-
-Expr *clang::IgnoreBaseCastsSingleStep(Expr *E) {
-  if (auto *CE = dyn_cast(E))
-if (CE->getCastKind() == CK_DerivedToBase ||
-CE->getCastKind() == CK_UncheckedDerivedToBase ||
-CE->getCastKind() == CK_NoOp)
-  return CE->getSubExpr();
-
-  return E;
-}
-
-Expr *clang::IgnoreImplicitSingleStep(Expr *E) {
-  Expr *SubE = IgnoreImplicitCastsSingleStep(E);
-  if (SubE != E)
-return SubE;
-
-  if (auto *MTE = dyn_cast(E))
-return MTE->getSubExpr();
-
-  if (auto *BTE = dyn_cast(E))
-return BTE->getSubExpr();
-
-  return E;
-}
-
-Expr *clang::IgnoreImplicitAsWrittenSingleStep(Expr *E) {
-  if (auto *ICE = dyn_cast(E))
-return ICE->getSubExprAsWritten();
-
-  return IgnoreImplicitSingleStep(E);
-}
-
-Expr *clang::IgnoreParensOnlySingleStep(Expr *E) {
-  if (auto *PE = dyn_cast(E))
-return PE->getSubExpr();
-  return E;
-}
-
-Expr *clang::IgnoreParensSingleStep(Expr *E) {
-  if (auto *PE = dyn_cast(E))
-return PE->getSubExpr();
-
-  if (auto *UO = dyn_cast(E)) {
-if (UO->getOpcode() == UO_Extension)
-  return UO->getSubExpr();
-  }
-
-  else if (auto *GSE = dyn_cast(E)) {
-if (!GSE->isResultDependent())
-  return GSE->getResultExpr();
-  }
-
-  else if (auto *CE = dyn_cast(E)) {
-if (!CE->isConditionDependent())
-  return CE->getChosenSubExpr();
-  }
-
-  return E;
-}
Index: clang/lib/AST/CMakeLists.txt
===
--- clang/lib/AST/CMakeLists.txt
+++ clang/lib/AST/CMakeLists.txt
@@ -55,7 +55,6 @@
   ExternalASTMerger.cpp
   ExternalASTSource.cpp
   FormatString.cpp
-  IgnoreExpr.cpp
   InheritViz.cpp
   Interp/ByteCodeEmitter.cpp
   Interp/ByteCodeExprGen.cpp
Index: clang/include/clang/AST/IgnoreExpr.h
===
--- clang/include/clang/AST/IgnoreExpr.h
+++ clang/include/clang/AST/IgnoreExpr.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_AST_IGNOREEXPR_H
 
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 
 namespace clang {
 namespace detail {
@@ -38,23 +39,122 @@
   return E;
 }
 
-Expr *IgnoreImplicitCastsSingleStep(Expr *E);
+template 
+const Expr *IgnoreExprNodes(const Expr *E, FnTys &&...Fns) {
+  return const_cast(IgnoreExprNodes(E, std::forward(Fns)...));
+}
+
+inline Expr *IgnoreImplicitCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSu

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

2020-09-08 Thread Louis Dionne via Phabricator via cfe-commits

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: dexonsmith.

I love this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87189

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

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

2020-09-08 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 290453.
kbobyrev added a comment.

Also check if deinition comes from unwwanted file (e.g. omit forward 
declarations).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -256,6 +256,11 @@
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
   assert(ASTNode.OrigD);
+  auto &SM = ASTCtx->getSourceManager();
+  if (!SM.isWrittenInBuiltinFile(Loc) &&
+  (!shouldIndexFile(SM.getFileID(Loc)) ||
+   !shouldIndexFile(SM.getFileID(D->getLocation()
+return true;
   // Indexing API puts canonical decl into D, which might not have a valid
   // source location for implicit/built-in decls. Fallback to original decl in
   // such cases.
@@ -298,7 +303,6 @@
 
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
-  auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
   SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
@@ -405,6 +409,9 @@
 return true;
 
   const auto &SM = PP->getSourceManager();
+  if (!SM.isWrittenInBuiltinFile(Loc) && !shouldIndexFile(SM.getFileID(Loc)))
+return true;
+
   auto DefLoc = MI->getDefinitionLoc();
   // Also avoid storing predefined macros like __DBL_MIN__.
   if (SM.isWrittenInBuiltinFile(DefLoc))


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -256,6 +256,11 @@
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
   assert(ASTNode.OrigD);
+  auto &SM = ASTCtx->getSourceManager();
+  if (!SM.isWrittenInBuiltinFile(Loc) &&
+  (!shouldIndexFile(SM.getFileID(Loc)) ||
+   !shouldIndexFile(SM.getFileID(D->getLocation()
+return true;
   // Indexing API puts canonical decl into D, which might not have a valid
   // source location for implicit/built-in decls. Fallback to original decl in
   // such cases.
@@ -298,7 +303,6 @@
 
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
-  auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
   SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
@@ -405,6 +409,9 @@
 return true;
 
   const auto &SM = PP->getSourceManager();
+  if (!SM.isWrittenInBuiltinFile(Loc) && !shouldIndexFile(SM.getFileID(Loc)))
+return true;
+
   auto DefLoc = MI->getDefinitionLoc();
   // Also avoid storing predefined macros like __DBL_MIN__.
   if (SM.isWrittenInBuiltinFile(DefLoc))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

2020-09-08 Thread Kirill Bobyrev via Phabricator via cfe-commits

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

WIP. Need to get around background indexing tricks to make it work.

  Failed Tests (3):
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.Config
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.IndexTwoFiles
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.ShardStorageLoad


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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

[clang] ae85da8 - [Codegen][X86] Begin moving X86 specific codegen tests into X86 subfolder.

2020-09-08 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-09-08T13:01:24+01:00
New Revision: ae85da86ad8fbd022129650d0b2a6b615709a790

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

LOG: [Codegen][X86] Begin moving X86 specific codegen tests into X86 subfolder.

Discussed with @craig.topper and @spatel - this is to try and tidyup the 
codegen folder and move the x86 specific tests (as opposed to general tests 
that just happen to use x86 triples) into subfolders. Its up to other targets 
if they follow suit.

It also helps speed up test iterations as using wildcards on lit commands often 
misses some filenames.

Added: 
clang/test/CodeGen/X86/x86-64-inline-asm.c
clang/test/CodeGen/X86/x86-GCC-inline-asm-Y-constraints.c
clang/test/CodeGen/X86/x86-atomic-long_double.c
clang/test/CodeGen/X86/x86-bswap.c
clang/test/CodeGen/X86/x86-builtins-vector-width.c
clang/test/CodeGen/X86/x86-builtins.c
clang/test/CodeGen/X86/x86-cf-protection.c
clang/test/CodeGen/X86/x86-crc-builtins.c
clang/test/CodeGen/X86/x86-enqcmd-builtins.c
clang/test/CodeGen/X86/x86-inline-asm-min-vector-width.c
clang/test/CodeGen/X86/x86-inline-asm-v-constraint.c
clang/test/CodeGen/X86/x86-long-double.cpp
clang/test/CodeGen/X86/x86-nontemporal.c
clang/test/CodeGen/X86/x86-serialize-intrin.c
clang/test/CodeGen/X86/x86-soft-float.c
clang/test/CodeGen/X86/x86-tsxldtrk-builtins.c
clang/test/CodeGen/X86/x86-vec-i128.c
clang/test/CodeGen/X86/x86-vec-struct-packing.c
clang/test/CodeGen/X86/x86-vector-width.c
clang/test/CodeGen/X86/x86.c
clang/test/CodeGen/X86/x86_32-arguments-darwin.c
clang/test/CodeGen/X86/x86_32-arguments-iamcu.c
clang/test/CodeGen/X86/x86_32-arguments-linux.c
clang/test/CodeGen/X86/x86_32-arguments-nommx.c
clang/test/CodeGen/X86/x86_32-arguments-realign.c
clang/test/CodeGen/X86/x86_32-arguments-win32.c
clang/test/CodeGen/X86/x86_32-fpcc-struct-return.c
clang/test/CodeGen/X86/x86_32-inline-asm.c
clang/test/CodeGen/X86/x86_32-xsave.c
clang/test/CodeGen/X86/x86_64-PR42672.c
clang/test/CodeGen/X86/x86_64-arguments-darwin.c
clang/test/CodeGen/X86/x86_64-arguments-nacl.c
clang/test/CodeGen/X86/x86_64-arguments-win32.c
clang/test/CodeGen/X86/x86_64-arguments.c
clang/test/CodeGen/X86/x86_64-atomic-128.c
clang/test/CodeGen/X86/x86_64-floatvectors.c
clang/test/CodeGen/X86/x86_64-instrument-functions.c
clang/test/CodeGen/X86/x86_64-longdouble.c
clang/test/CodeGen/X86/x86_64-mno-sse.c
clang/test/CodeGen/X86/x86_64-mno-sse2.c
clang/test/CodeGen/X86/x86_64-profiling-keep-fp.c
clang/test/CodeGen/X86/x86_64-xsave.c
clang/test/CodeGen/X86/x86_inlineasm_curly_bracket_escape.c

Modified: 


Removed: 
clang/test/CodeGen/x86-64-inline-asm.c
clang/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c
clang/test/CodeGen/x86-atomic-long_double.c
clang/test/CodeGen/x86-bswap.c
clang/test/CodeGen/x86-builtins-vector-width.c
clang/test/CodeGen/x86-builtins.c
clang/test/CodeGen/x86-cf-protection.c
clang/test/CodeGen/x86-crc-builtins.c
clang/test/CodeGen/x86-enqcmd-builtins.c
clang/test/CodeGen/x86-inline-asm-min-vector-width.c
clang/test/CodeGen/x86-inline-asm-v-constraint.c
clang/test/CodeGen/x86-long-double.cpp
clang/test/CodeGen/x86-nontemporal.c
clang/test/CodeGen/x86-serialize-intrin.c
clang/test/CodeGen/x86-soft-float.c
clang/test/CodeGen/x86-tsxldtrk-builtins.c
clang/test/CodeGen/x86-vec-i128.c
clang/test/CodeGen/x86-vec-struct-packing.c
clang/test/CodeGen/x86-vector-width.c
clang/test/CodeGen/x86.c
clang/test/CodeGen/x86_32-arguments-darwin.c
clang/test/CodeGen/x86_32-arguments-iamcu.c
clang/test/CodeGen/x86_32-arguments-linux.c
clang/test/CodeGen/x86_32-arguments-nommx.c
clang/test/CodeGen/x86_32-arguments-realign.c
clang/test/CodeGen/x86_32-arguments-win32.c
clang/test/CodeGen/x86_32-fpcc-struct-return.c
clang/test/CodeGen/x86_32-inline-asm.c
clang/test/CodeGen/x86_32-xsave.c
clang/test/CodeGen/x86_64-PR42672.c
clang/test/CodeGen/x86_64-arguments-darwin.c
clang/test/CodeGen/x86_64-arguments-nacl.c
clang/test/CodeGen/x86_64-arguments-win32.c
clang/test/CodeGen/x86_64-arguments.c
clang/test/CodeGen/x86_64-atomic-128.c
clang/test/CodeGen/x86_64-floatvectors.c
clang/test/CodeGen/x86_64-instrument-functions.c
clang/test/CodeGen/x86_64-longdouble.c
clang/test/CodeGen/x86_64-mno-sse.c
clang/test/CodeGen/x86_64-mno-sse2.c
clang/test/CodeGen/x86_64-profiling-keep-fp.c
clang/test/CodeGen/x86_64-xsave.c
clang/test/CodeGen/x86_inlineasm_curly_bracket_escape.c



diff  --git a/clang/test/CodeGen/x86-64-inline-asm.c 
b/clang/test/CodeG

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

2020-09-08 Thread Raul Tambre via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG86bd8f82cc74: [CMake] Remove dead FindPythonInterp code 
(authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87189

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  libcxx/CMakeLists.txt
  lld/CMakeLists.txt
  llvm/CMakeLists.txt

Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -696,38 +696,19 @@
 
 include(HandleLLVMOptions)
 
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if( NOT PYTHONINTERP_FOUND )
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for builds and testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
+  # Treat python2 as python3
   add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 ##
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -57,38 +57,19 @@
   include(CheckAtomic)
 
   if(LLVM_INCLUDE_TESTS)
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if(NOT PYTHONINTERP_FOUND)
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if(${PYTHON_VERSION_STRING} VERSION_LESS 2.7)
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
-  add_executable(Python3::Interpeter IMPORTED)
+  # Treat python2 as python3
+  add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 # Check prebuilt llvm/utils.
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -41,33 +41,19 @@
 endif()
 
 if (LIBCXX_STANDALONE_BUILD)
-  if(CMAKE_VERSION VERSION_LESS 3.12)
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(WARNING "Failed to find python interpreter. "
-  "The libc++ test suite will be disabled.")
-  set(LLVM_INCLUDE_TESTS OFF)
-else()
-  add_executable(Python3::Interpreter IMPORTED)
-  set_target_properties(Python3::Interp

[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

2020-09-08 Thread Joachim Meyer via Phabricator via cfe-commits

fodinabor updated this revision to Diff 290460.
fodinabor marked 9 inline comments as done.
fodinabor added a comment.

Incorporating review comments:

- renaming option to -Wno-error=unknown and adding warning in description
- emit warnings instead of fully ignoring the issues

Documentation and unit tests will follow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

Files:
  .clang-format
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  llvm/include/llvm/Support/YAMLParser.h
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp

Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -48,6 +48,10 @@
   Ctxt = Context;
 }
 
+void IO::setAllowUnknownKeys(bool Allow) {
+  llvm_unreachable("Only supported for Input");
+}
+
 //===--===//
 //  Input
 //===--===//
@@ -197,8 +201,13 @@
 return;
   for (const auto &NN : MN->Mapping) {
 if (!is_contained(MN->ValidKeys, NN.first())) {
-  setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
-  break;
+  auto ReportNode = NN.second.get();
+  auto ReportMessage = Twine("unknown key '") + NN.first() + "'";
+  if (!AllowUnknownKeys) {
+setError(ReportNode, ReportMessage);
+break;
+  } else
+reportWarning(ReportNode, ReportMessage);
 }
   }
 }
@@ -370,6 +379,15 @@
   EC = make_error_code(errc::invalid_argument);
 }
 
+void Input::reportWarning(HNode *hnode, const Twine &message) {
+  assert(hnode && "HNode must not be NULL");
+  reportWarning(hnode->_node, message);
+}
+
+void Input::reportWarning(Node *node, const Twine &message) {
+  Strm->printError(node, message, SourceMgr::DK_Warning);
+}
+
 std::unique_ptr Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   if (ScalarNode *SN = dyn_cast(N)) {
@@ -428,6 +446,8 @@
   setError(CurrentNode, Message);
 }
 
+void Input::setAllowUnknownKeys(bool Allow) { AllowUnknownKeys = Allow; }
+
 bool Input::canElideEmptySequence() {
   return false;
 }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -1775,12 +1775,9 @@
 
 bool Stream::failed() { return scanner->failed(); }
 
-void Stream::printError(Node *N, const Twine &Msg) {
+void Stream::printError(Node *N, const Twine &Msg, SourceMgr::DiagKind Kind) {
   SMRange Range = N ? N->getSourceRange() : SMRange();
-  scanner->printError( Range.Start
- , SourceMgr::DK_Error
- , Msg
- , Range);
+  scanner->printError(Range.Start, Kind, Msg, Range);
 }
 
 document_iterator Stream::begin() {
Index: llvm/include/llvm/Support/YAMLTraits.h
===
--- llvm/include/llvm/Support/YAMLTraits.h
+++ llvm/include/llvm/Support/YAMLTraits.h
@@ -789,6 +789,7 @@
   virtual NodeKind getNodeKind() = 0;
 
   virtual void setError(const Twine &) = 0;
+  virtual void setAllowUnknownKeys(bool Allow);
 
   template 
   void enumCase(T &Val, const char* Str, const T ConstVal) {
@@ -1495,6 +1496,9 @@
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
 
+  void reportWarning(HNode *hnode, const Twine &message);
+  void reportWarning(Node *hnode, const Twine &message);
+
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
@@ -1504,6 +1508,8 @@
   /// Returns the current node that's being parsed by the YAML Parser.
   const Node *getCurrentNode() const;
 
+  void setAllowUnknownKeys(bool Allow) override;
+
 private:
   SourceMgr   SrcMgr; // must be before Strm
   std::unique_ptr Strm;
@@ -1514,6 +1520,7 @@
   std::vector   BitValuesUsed;
   HNode *CurrentNode = nullptr;
   boolScalarMatchFound = false;
+  bool AllowUnknownKeys = false;
 };
 
 ///
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -40,6 +40,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/SMLoc.h"
+#include "llvm/Support/SourceMgr.h"
 #include 
 #include 
 #include 
@@ -51,7 +52,6 @@
 namespace llvm {
 
 class MemoryBufferRef;
-class SourceMgr;
 class raw_ostream;
 class Twine;
 
@@ -100,7 +100,8 @@
 return !failed();
   }
 
-  void printError(N

[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

2020-09-08 Thread Joachim Meyer via Phabricator via cfe-commits

fodinabor updated this revision to Diff 290461.
fodinabor added a comment.

Remove test entry form .clang-format :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  llvm/include/llvm/Support/YAMLParser.h
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp

Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -48,6 +48,10 @@
   Ctxt = Context;
 }
 
+void IO::setAllowUnknownKeys(bool Allow) {
+  llvm_unreachable("Only supported for Input");
+}
+
 //===--===//
 //  Input
 //===--===//
@@ -197,8 +201,13 @@
 return;
   for (const auto &NN : MN->Mapping) {
 if (!is_contained(MN->ValidKeys, NN.first())) {
-  setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
-  break;
+  auto ReportNode = NN.second.get();
+  auto ReportMessage = Twine("unknown key '") + NN.first() + "'";
+  if (!AllowUnknownKeys) {
+setError(ReportNode, ReportMessage);
+break;
+  } else
+reportWarning(ReportNode, ReportMessage);
 }
   }
 }
@@ -370,6 +379,15 @@
   EC = make_error_code(errc::invalid_argument);
 }
 
+void Input::reportWarning(HNode *hnode, const Twine &message) {
+  assert(hnode && "HNode must not be NULL");
+  reportWarning(hnode->_node, message);
+}
+
+void Input::reportWarning(Node *node, const Twine &message) {
+  Strm->printError(node, message, SourceMgr::DK_Warning);
+}
+
 std::unique_ptr Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   if (ScalarNode *SN = dyn_cast(N)) {
@@ -428,6 +446,8 @@
   setError(CurrentNode, Message);
 }
 
+void Input::setAllowUnknownKeys(bool Allow) { AllowUnknownKeys = Allow; }
+
 bool Input::canElideEmptySequence() {
   return false;
 }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -1775,12 +1775,9 @@
 
 bool Stream::failed() { return scanner->failed(); }
 
-void Stream::printError(Node *N, const Twine &Msg) {
+void Stream::printError(Node *N, const Twine &Msg, SourceMgr::DiagKind Kind) {
   SMRange Range = N ? N->getSourceRange() : SMRange();
-  scanner->printError( Range.Start
- , SourceMgr::DK_Error
- , Msg
- , Range);
+  scanner->printError(Range.Start, Kind, Msg, Range);
 }
 
 document_iterator Stream::begin() {
Index: llvm/include/llvm/Support/YAMLTraits.h
===
--- llvm/include/llvm/Support/YAMLTraits.h
+++ llvm/include/llvm/Support/YAMLTraits.h
@@ -789,6 +789,7 @@
   virtual NodeKind getNodeKind() = 0;
 
   virtual void setError(const Twine &) = 0;
+  virtual void setAllowUnknownKeys(bool Allow);
 
   template 
   void enumCase(T &Val, const char* Str, const T ConstVal) {
@@ -1495,6 +1496,9 @@
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
 
+  void reportWarning(HNode *hnode, const Twine &message);
+  void reportWarning(Node *hnode, const Twine &message);
+
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
@@ -1504,6 +1508,8 @@
   /// Returns the current node that's being parsed by the YAML Parser.
   const Node *getCurrentNode() const;
 
+  void setAllowUnknownKeys(bool Allow) override;
+
 private:
   SourceMgr   SrcMgr; // must be before Strm
   std::unique_ptr Strm;
@@ -1514,6 +1520,7 @@
   std::vector   BitValuesUsed;
   HNode *CurrentNode = nullptr;
   boolScalarMatchFound = false;
+  bool AllowUnknownKeys = false;
 };
 
 ///
Index: llvm/include/llvm/Support/YAMLParser.h
===
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -40,6 +40,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/SMLoc.h"
+#include "llvm/Support/SourceMgr.h"
 #include 
 #include 
 #include 
@@ -51,7 +52,6 @@
 namespace llvm {
 
 class MemoryBufferRef;
-class SourceMgr;
 class raw_ostream;
 class Twine;
 
@@ -100,7 +100,8 @@
 return !failed();
   }
 
-  void printError(Node *N, const Twine &Msg);
+  void printError(Node *N, const Twine &Msg,
+  SourceMgr::DiagKind Kind = SourceMgr::DK_Error);
 
 private:
   friend class Document;
Index: clang/tools/clang-format/ClangForma

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

2020-09-08 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.



> But if the string is invalidated (or the new length is completely unknown), 
> **do not remove** the length from the state; instead, set it to 
> SymbolConjured.

Where exactly do you implement the above?

> When strlen(R) is used for the first time on a region R, produce 
> `$meta` as the answer, but *do not store* it in the string length 
> map.

And this?




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2379
+  // Overwrite the associated cstring length values of the invalidated regions.
+  for (const auto &Entry : Entries) {
+const MemRegion *MR = Entry.first;

It's just a tiny nit. But, perhaps we could come up with a more meaningful name 
instead of `Entry` and `Entries`. Maybe `Length` ?



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2384-2385
 if (SuperRegions.count(MR)) {
   Entries = F.remove(Entries, MR);
+  Entries = F.add(Entries, MR, UnknownVal());
   continue;

Umm, these two lines are quite disturbing after each other. Seems like first we 
remove `MR` then we add `MR` again, so, this must not be a noop, right? But 
then what is happening here exactly? Some comments around here in the code 
would help.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2429
 if (SymbolRef Sym = Len.getAsSymbol()) {
   if (SR.isDead(Sym))
+Entries = F.remove(Entries, MR);

steakhal wrote:
> NoQ wrote:
> > Ok, this doesn't look correct (looks like it never was). It's liveness of 
> > the region that's important to us, not liveness of the symbol. Because it's 
> > the liveness of the region that causes the program to be able to access the 
> > map entry.
> Let's say we have this:
> ```lang=C++
> void foo() {
>   char *p = malloc(12);
>   // strlen(p); // no-leak if strlen called, leak warning otherwise...
> } // expected-warning {{leak}}
> ```
> If we were marking the region live here, we would miss the `leak` warning. 
> That warning is only triggered if the region of the `p` becomes dead. Which 
> will never become dead if we have a cstring length symbol associated to that 
> region.
> I came to this conclusion after implementing your suggested edit above 
> (checking regions instead of symbols).
Is it possible to have two string length symbols that are associated to the 
same region? Could that cause any problem?
E.g. what will happen below? Will we remove `MR` twice? 
```
char *p = "asdf"
char *q = p + 1;
strlen(p); strlen(q);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445

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

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

2020-09-08 Thread Gabor Marton via Phabricator via cfe-commits

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2384-2385
 if (SuperRegions.count(MR)) {
   Entries = F.remove(Entries, MR);
+  Entries = F.add(Entries, MR, UnknownVal());
   continue;

martong wrote:
> Umm, these two lines are quite disturbing after each other. Seems like first 
> we remove `MR` then we add `MR` again, so, this must not be a noop, right? 
> But then what is happening here exactly? Some comments around here in the 
> code would help.
Ugh, giving it more thought, we just reset the Value associated to `MR`. Still, 
would be nice to comment this there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

2020-09-08 Thread Denys Petrov via Phabricator via cfe-commits

ASDenysPetrov added a comment.

@steakhal

> the readability of the reports should be improved.

Absolutely agree. Let's do this in the next patches :)

@NoQ 
You've recommended to extend PthreadLockChecker with STL mutexes. I think I've 
done. Could you make a quick look, please?




Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}

NoQ wrote:
> I repeat, this is a false positive. Recursive mutexes can be locked twice, 
> that's the whole point.
Yes, I remember. I think you've mixed up with my previous attempts of 
introducting a new checks for recursive mutexes.
This is not this case. This kind of checks already exists in PthreadLockChecker 
before.
I've just reused this check for all kind of STL mutexes.

If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can find 
the same case.


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

https://reviews.llvm.org/D85984

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

[PATCH] D87282: [SYCL] Assume SYCL device functions are convergent

2020-09-08 Thread Alexey Bader via Phabricator via cfe-commits

bader created this revision.
Herald added subscribers: cfe-commits, Anastasia, ebevhan, yaxunl.
Herald added a project: clang.
bader requested review of this revision.

SYCL device compiler (similar to other SPMD compilers) assumes that
functions are convergent by default to avoid invalid transformations.
This attribute can be removed if compiler can prove that function does
not have convergent operations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87282

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenSYCL/convergent.cpp


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2874,7 +2874,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2874,7 +2874,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87138: [analyzer][NFC] Introduce refactoring of PthreadLockChecker

2020-09-08 Thread Denys Petrov via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe67405141836: [analyzer] [NFC] Introduce refactoring of 
PthreadLockChecker (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87138

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -83,7 +83,7 @@
 private:
   typedef void (PthreadLockChecker::*FnCheck)(const CallEvent &Call,
   CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   CallDescriptionMap PThreadCallbacks = {
   // Init.
   {{"pthread_mutex_init", 2}, &PthreadLockChecker::InitAnyLock},
@@ -167,46 +167,49 @@
   ProgramStateRef resolvePossiblyDestroyedMutex(ProgramStateRef state,
 const MemRegion *lockR,
 const SymbolRef *sym) const;
-  void reportUseDestroyedBug(const CallEvent &Call, CheckerContext &C,
- unsigned ArgNo, CheckerKind checkKind) const;
+  void reportBug(CheckerContext &C, std::unique_ptr BT[],
+ const Expr *MtxExpr, CheckerKind CheckKind,
+ StringRef Desc) const;
 
   // Init.
   void InitAnyLock(const CallEvent &Call, CheckerContext &C,
-   CheckerKind checkkind) const;
-  void InitLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-   SVal Lock, CheckerKind checkkind) const;
+   CheckerKind CheckKind) const;
+  void InitLockAux(const CallEvent &Call, CheckerContext &C,
+   const Expr *MtxExpr, SVal MtxVal,
+   CheckerKind CheckKind) const;
 
   // Lock, Try-lock.
   void AcquirePthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void AcquireXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryPthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryFuchsiaLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryC11Lock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void AcquireLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal lock, bool isTryLock, LockingSemantics semantics,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void AcquireLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal, bool IsTryLock,
+  LockingSemantics Semantics, CheckerKind CheckKind) const;
 
   // Release.
   void ReleaseAnyLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void ReleaseLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal lock, CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void ReleaseLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal,
+  CheckerKind CheckKind) const;
 
   // Destroy.
   void DestroyPthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void DestroyXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void DestroyLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal Lock, LockingSemantics semantics,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void DestroyLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal,
+  LockingSemantics Semantics, CheckerKind CheckKind) const;
 
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
@@ -226,18 +229,18 @@
   muta

[clang] e674051 - [analyzer] [NFC] Introduce refactoring of PthreadLockChecker

2020-09-08 Thread Denys Petrov via cfe-commits


Author: Denys Petrov
Date: 2020-09-08T16:04:19+03:00
New Revision: e67405141836fcd88183863758eeb42f32e847a6

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

LOG: [analyzer] [NFC] Introduce refactoring of PthreadLockChecker

Change capitalization of some names due to LLVM naming rules.
Change names of some variables to make them more speaking.
Rework similar bug reports into one common function.

Prepare code for the next patches to reduce unrelated changes.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
index 285d2da104f1..88e80c481a5a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -83,7 +83,7 @@ class PthreadLockChecker : public Checker PThreadCallbacks = {
   // Init.
   {{"pthread_mutex_init", 2}, &PthreadLockChecker::InitAnyLock},
@@ -167,46 +167,49 @@ class PthreadLockChecker : public 
Checker BT[],
+ const Expr *MtxExpr, CheckerKind CheckKind,
+ StringRef Desc) const;
 
   // Init.
   void InitAnyLock(const CallEvent &Call, CheckerContext &C,
-   CheckerKind checkkind) const;
-  void InitLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-   SVal Lock, CheckerKind checkkind) const;
+   CheckerKind CheckKind) const;
+  void InitLockAux(const CallEvent &Call, CheckerContext &C,
+   const Expr *MtxExpr, SVal MtxVal,
+   CheckerKind CheckKind) const;
 
   // Lock, Try-lock.
   void AcquirePthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void AcquireXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryPthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryFuchsiaLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void TryC11Lock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void AcquireLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal lock, bool isTryLock, LockingSemantics semantics,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void AcquireLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal, bool IsTryLock,
+  LockingSemantics Semantics, CheckerKind CheckKind) const;
 
   // Release.
   void ReleaseAnyLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void ReleaseLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal lock, CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void ReleaseLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal,
+  CheckerKind CheckKind) const;
 
   // Destroy.
   void DestroyPthreadLock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
   void DestroyXNULock(const CallEvent &Call, CheckerContext &C,
-  CheckerKind checkkind) const;
-  void DestroyLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
-  SVal Lock, LockingSemantics semantics,
-  CheckerKind checkkind) const;
+  CheckerKind CheckKind) const;
+  void DestroyLockAux(const CallEvent &Call, CheckerContext &C,
+  const Expr *MtxExpr, SVal MtxVal,
+  LockingSemantics Semantics, CheckerKind CheckKind) const;
 
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
@@ -226,18 +229,18 @@ class PthreadLockChecker : public 
Checker BT_initlock[CK_NumCheckKinds];
   mutable std::unique_ptr BT_lor[CK_NumCheckKinds];
 
-  void initBugType(CheckerKind checkKind) const {
-

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

2020-09-08 Thread Gabor Marton via Phabricator via cfe-commits

martong accepted this revision.
martong added a comment.

LGTM! Thanks for the clarification and the example you gave.
(I agree with @steakhal and I wish if we could get rid of the many lines 
not-descriptive plist stuff, but that is rather unrelated)


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

https://reviews.llvm.org/D86135

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

[PATCH] D76604: [Analyzer] Model `size()` member function of containers

2020-09-08 Thread Gabor Marton via Phabricator via cfe-commits

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

This basically looks good to me.




Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:482-483
+// of the container (the difference between its `begin()` and `end()` to
+// this size. Function `relateSymbols()` returns null if it contradits
+// the current size.
+const auto CalcEnd =

baloghadamsoftware wrote:
> martong wrote:
> > How? I don't see how does it access the `size`.
> As explained between the parenthesis, the actual size of the container is the 
> difference between its `begin()` and its `end()`. If we have this difference, 
> then we know the actual size. The other value we may have is the return value 
> of the `size()` function. We either have one of them, both or none. If we 
> have one, then we adjust the other. If we have both, then we check for 
> consistency, and generated a sink if they are inconsistent. If we have none, 
> then we do nothing.
Ok, makes sense.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:492
+  } else {
+if (CalcSize) {
+  // If the current size is a concrete integer, bind this to the return

baloghadamsoftware wrote:
> martong wrote:
> > What if we have both `RetSize` and `CalcSize`? Should we check their values 
> > for consistency? (And perhaps adding another sink node if we have 
> > inconsistency?)
> This is handled in the `if` branch: having `CalcSize` means that we know the 
> difference between the `begin()` and the `end()`, thus inconsistency between 
> `RetSize` and `CalcSize` is the same as inconstistency between `CalcEnd` and 
> `EndSym`. The comment above explains that if there is such inconsistency, 
> then `relateSymbols()` returns a null pointer which we assign to `State`. At 
> the end of this functions we generate a sink whenever `State` is a null 
> pointer.
Ok. Perhpas we could move the relevant comment just right about the call of 
`relateSymbols()`.


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

https://reviews.llvm.org/D76604

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

[clang-tools-extra] 156b127 - Add a new altera check for structure packing and alignment.

2020-09-08 Thread Aaron Ballman via cfe-commits


Author: Frank Derry Wanye
Date: 2020-09-08T09:35:14-04:00
New Revision: 156b127945a8c923d141e608b7380427da024376

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

LOG: Add a new altera check for structure packing and alignment.

The altera struct pack align lint check finds structs that are inefficiently
packed or aligned and recommends packing/aligning of the structs using the
packed and aligned attributes as needed in a warning.

Added: 
clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
clang-tools-extra/clang-tidy/altera/CMakeLists.txt
clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp
clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
clang-tools-extra/docs/clang-tidy/checks/altera-struct-pack-align.rst
clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp

Modified: 
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/index.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 02573534ccae..923976197ebe 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -46,6 +46,7 @@ endif()
 # If you add a check, also add it to ClangTidyForceLinker.h in this directory.
 add_subdirectory(android)
 add_subdirectory(abseil)
+add_subdirectory(altera)
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
@@ -71,6 +72,7 @@ add_subdirectory(zircon)
 set(ALL_CLANG_TIDY_CHECKS
   clangTidyAndroidModule
   clangTidyAbseilModule
+  clangTidyAlteraModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h 
b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
index 1d6bd2a4fd62..63e681f878db 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -20,6 +20,11 @@ extern volatile int AbseilModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
 AbseilModuleAnchorSource;
 
+// This anchor is used to force the linker to link the AlteraModule.
+extern volatile int AlteraModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AlteraModuleAnchorDestination =
+AlteraModuleAnchorSource;
+
 // This anchor is used to force the linker to link the AndroidModule.
 extern volatile int AndroidModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED AndroidModuleAnchorDestination =

diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp 
b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
new file mode 100644
index ..d91f67ac1485
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
@@ -0,0 +1,39 @@
+//===--- AlteraTidyModule.cpp - clang-tidy 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "StructPackAlignCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+class AlteraModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"altera-struct-pack-align");
+  }
+};
+
+} // namespace altera
+
+// Register the AlteraTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("altera-module", "Adds Altera FPGA OpenCL lint checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AlteraModule.
+volatile int AlteraModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
new file mode 100644
index ..45131c1809a2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -0,0 +1,15 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAlteraModule
+  AlteraTidyModule.cpp
+  StructPackAlignCheck.cpp
+
+  LINK_LIBS
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )

diff  --git a/clang-tools-extra/clang

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-09-08 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've committed on your behalf in 156b127945a8c923d141e608b7380427da024376 
. Thank 
you for the new check!


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

https://reviews.llvm.org/D66564

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

[clang] 9c9974c - [clang] Limit the maximum level of fold-expr expansion.

2020-09-08 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-09-08T15:40:09+02:00
New Revision: 9c9974c3ccb6468cc83f759240293538cf123fcd

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

LOG: [clang] Limit the maximum level of fold-expr expansion.

Introduce a new diagnostic, and respect the bracket-depth (256) by default.

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

Added: 
clang/test/SemaCXX/fold_expr_expansion_limit.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e1601da74b73..ec0c0fd9fa8c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@ def err_fold_expression_empty : Error<
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguments exceeded expression nesting 
"
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 4c8293f3bf4c..6457b192477e 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@ 
TreeTransform::TransformCXXFoldExpr(CXXFoldExpr *E) {
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());

diff  --git a/clang/test/SemaCXX/fold_expr_expansion_limit.cpp 
b/clang/test/SemaCXX/fold_expr_expansion_limit.cpp
new file mode 100644
index ..600278da7828
--- /dev/null
+++ b/clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguments exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}



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

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c9974c3ccb6: [clang] Limit the maximum level of fold-expr 
expansion. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguments exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguments exceeded expression nesting 
"
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguments exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_dep

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-08 Thread David Truby via Phabricator via cfe-commits

DavidTruby accepted this revision.
DavidTruby added a comment.

LGTM and seems to work from the flang side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[clang-tools-extra] 9933188 - StructPackAlignCheck: Fix a -Winconsistent-missing-override warning

2020-09-08 Thread Nico Weber via cfe-commits


Author: Nico Weber
Date: 2020-09-08T10:02:00-04:00
New Revision: 9933188c90615c9c264ebb69117f09726e909a25

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

LOG: StructPackAlignCheck: Fix a -Winconsistent-missing-override warning

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h 
b/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
index b903641247e3..510e03030590 100644
--- a/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
+++ b/clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
@@ -27,7 +27,7 @@ class StructPackAlignCheck : public ClangTidyCheck {
 MaxConfiguredAlignment(Options.get("MaxConfiguredAlignment", 128)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  void storeOptions(ClangTidyOptions::OptionMap &Opts);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
   const unsigned MaxConfiguredAlignment;



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

[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-08 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L added a comment.

In D86790#2255371 , @jyknight wrote:

> Do you have open questions on whether some callsites passing "false" here, 
> should be switched to true? Given what's here, I would say that it definitely 
> does not makes sense to add this parameter everywhere.

Basically, the places where I changed to `true /* NeedsPreferredAlignment */` 
are ones I'd like reviewers to see if the switch is correct.




Comment at: clang/lib/CodeGen/CGAtomic.cpp:814
+  std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(
+  AtomicTy, true /* NeedsPreferredAlignment */);
   uint64_t Size = sizeChars.getQuantity();

jyknight wrote:
> This is wrong.
Can you explain a bit why it's wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790

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

[clang] 2d9d270 - Revert 3e782bf809 "[Sema][MSVC] warn at dynamic_cast when /GR- is given"

2020-09-08 Thread Hans Wennborg via cfe-commits


Author: Hans Wennborg
Date: 2020-09-08T16:10:18+02:00
New Revision: 2d9d270e77918dfc19ad9b3150ee7d40eeb8ca79

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

LOG: Revert 3e782bf809 "[Sema][MSVC] warn at dynamic_cast when /GR- is given"

This caused more warnings than expected, see https://crbug.com/1126019

Also reverts the follow-up 7907e5516.

> Differential Revision: https://reviews.llvm.org/D86369

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/no-rtti.cpp

Removed: 
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index a9bd52b8afcd..6b4dcc850612 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1235,5 +1235,3 @@ in addition with the pragmas or -fmax-tokens flag to get 
any warnings.
 }
 
 def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
-
-def RTTI : DiagGroup<"rtti">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ec0c0fd9fa8c..46f7ffc97ce7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7441,12 +7441,6 @@ def err_no_typeid_with_fno_rtti : Error<
   "use of typeid requires -frtti">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
   "use of dynamic_cast requires -frtti">;
-def warn_no_dynamic_cast_with_rtti_disabled: Warning<
-  "dynamic_cast will not work since RTTI data is disabled by " 
-  "%select{-fno-rtti-data|/GR-}0">, InGroup;
-def warn_no_typeid_with_rtti_disabled: Warning<
-  "typeid will not work since RTTI data is disabled by "
-  "%select{-fno-rtti-data|/GR-}0">, InGroup;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index b213fb756a65..726900c59f20 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -890,18 +890,6 @@ void CastOperation::CheckDynamicCast() {
 return;
   }
 
-  // Warns when dynamic_cast is used with RTTI data disabled.
-  if (!Self.getLangOpts().RTTIData) {
-bool MicrosoftABI =
-Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
-bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() 
==
- DiagnosticOptions::MSVC;
-if (MicrosoftABI || !DestPointee->isVoidType())
-  Self.Diag(OpRange.getBegin(),
-diag::warn_no_dynamic_cast_with_rtti_disabled)
-  << isClangCL;
-  }
-
   // Done. Everything else is run-time checks.
   Kind = CK_Dynamic;
 }

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 8f8847e63804..d1fcdf354527 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -646,12 +646,6 @@ Sema::ActOnCXXTypeid(SourceLocation OpLoc, SourceLocation 
LParenLoc,
 return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
   }
 
-  // Warns when typeid is used with RTTI data disabled.
-  if (!getLangOpts().RTTIData)
-Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
-<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
-DiagnosticOptions::MSVC);
-
   QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
 
   if (isType) {

diff  --git a/clang/test/SemaCXX/ms_no_dynamic_cast.cpp 
b/clang/test/SemaCXX/ms_no_dynamic_cast.cpp
deleted file mode 100644
index d2c007fd8c29..
--- a/clang/test/SemaCXX/ms_no_dynamic_cast.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 %s -triple x86_64-windows -fdiagnostics-format msvc 
-fno-rtti-data -fsyntax-only -verify
-
-namespace std {
-struct type_info {};
-} // namespace std
-class B {
-public:
-  virtual ~B() = default;
-};
-
-class D1 : public B {
-public:
-  ~D1() = default;
-};
-
-void f() {
-  B* b = new D1();
-  auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not 
work since RTTI data is disabled by /GR-}}
-  void* v = dynamic_cast(b); // expected-warning{{dynamic_cast will 
not work since RTTI data is disabled by /GR-}}
-  (void)typeid(int);  // expected-warning{{typeid will not work 
since RTTI data is disabled by /GR-}}
-}

diff  --git a/clang/test/SemaCXX/no-rtti.cpp b/clang/test/SemaCXX/no-rtti.cpp
index f8487a0902dd..e0b57153c24c 100644
--- a/clang/test/SemaCXX/no-rtti.cpp
+++ b/clang/test/SemaCXX/no-rtti.cpp
@@ -1,4 +1,4

[clang-tools-extra] 32ae37b - [clang-tidy] Fix dynamic build failures after 156b127945a8c923d141e608b7380427da024376

2020-09-08 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-09-08T16:26:48+02:00
New Revision: 32ae37b038b16a1ff9c81428ae4f003377439a22

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

LOG: [clang-tidy] Fix dynamic build failures after 
156b127945a8c923d141e608b7380427da024376

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
index 45131c1809a2..878e718c6596 100644
--- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -5,11 +5,15 @@ add_clang_library(clangTidyAlteraModule
   StructPackAlignCheck.cpp
 
   LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyAlteraModule
+  PRIVATE
   clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
-  clangTidy
-  clangTidyUtils
   )



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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-09-08 Thread Douglas Chen via Phabricator via cfe-commits

dougpuob updated this revision to Diff 290483.
dougpuob added a comment.

This is a test with `arc diff master --update D86671` command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,266 @@
+typedef signed char int8_t; // NOLINT
+typedef short   int16_t;// NOLINT
+typedef longint32_t;// NOLINT
+typedef long long   int64_t;// NOLINT
+
+typedef unsigned char   uint8_t;// NOLINT
+typedef unsigned short  uint16_t;   // NOLINT
+typedef unsigned long   uint32_t;   // NOLINT
+typedef unsigned long long  uint64_t;   // NOLINT
+
+typedef unsigned intsize_t; // NOLINT
+typedef longintptr_t;   // NOLINT
+typedef unsigned long   uintptr_t;  // NOLINT
+typedef long intptrdiff_t;  // NOLINT
+
+typedef unsigned char   BYTE;   // NOLINT
+typedef unsigned short  WORD;   // NOLINT
+typedef unsigned long   DWORD;  // NOLINT
+
+typedef int BOOL;   // NOLINT
+typedef BYTEBOOLEAN;// NOLINT
+
+#define NULL(0) // NOLINT
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.MemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.VariableCase   , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ParameterCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase , value: CamelCase }, \
+// RUN:   ]}"
+
+class UnlistedClass { public: mutable int ValInt; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: invalid case style for member 'ValInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}class UnlistedClass { public: mutable int iValInt; };
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+struct DataBuffer {
+mutable size_t Size;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: invalid case style for member 'Size' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}mutable size_t nSize;
+
+int &RefValueIndex = iIndex;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global variable 'RefValueIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int &iRefValueIndex = Index;
+
+typedef void (*FUNC_PTR_HELLO)();
+FUNC_PTR_HELLO Hello = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'Hello' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}FUNC_PTR_HELLO fnHello = NULL;
+
+void *ValueVoidPtr = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global pointer 'ValueVoidPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pValueVoidPtr = NULL;
+
+ptrdiff_t PtrDiff = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global variable 'PtrDiff'

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

2020-09-08 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D86445#2260744 , @martong wrote:

>> But if the string is invalidated (or the new length is completely unknown), 
>> **do not remove** the length from the state; instead, set it to 
>> SymbolConjured.
>
> Where exactly do you implement the above?
>
>> When strlen(R) is used for the first time on a region R, produce 
>> `$meta` as the answer, but *do not store* it in the string length 
>> map.
>
> And this?

Hm yes. I missed them. I will extend the current implementation, rethinking the 
invalidation cases, when to store and what etc.
It will bloat this patch, but indeed - necessary. Thank you!

However, as I planned to clean the CStringChecker a little bit up, it seems 
reasonable to me to land those patches before I start working on this to save 
me from some rebasing :|

What do you think about these patches:

- D84316  [analyzer][NFC] Split CStringChecker 
to modeling and reporting
- D84979  [analyzer][NFC] Refine CStringLength 
modeling API

If you like that direction, I can rebase this on top of that. That would help 
me a lot, to know when & how a given metadata symbol would be used, and check 
if we indeed handle the invalidation, etc. in the right way.




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2379
+  // Overwrite the associated cstring length values of the invalidated regions.
+  for (const auto &Entry : Entries) {
+const MemRegion *MR = Entry.first;

martong wrote:
> It's just a tiny nit. But, perhaps we could come up with a more meaningful 
> name instead of `Entry` and `Entries`. Maybe `Length` ?
We will see. Thanks.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2384-2385
 if (SuperRegions.count(MR)) {
   Entries = F.remove(Entries, MR);
+  Entries = F.add(Entries, MR, UnknownVal());
   continue;

martong wrote:
> martong wrote:
> > Umm, these two lines are quite disturbing after each other. Seems like 
> > first we remove `MR` then we add `MR` again, so, this must not be a noop, 
> > right? But then what is happening here exactly? Some comments around here 
> > in the code would help.
> Ugh, giving it more thought, we just reset the Value associated to `MR`. 
> Still, would be nice to comment this there.
Exactly. I will do that, ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445

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

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:1970
+  but this behavior is changed by another option,
+  ``JavaStaticImportAfterImport``.
 

Can you add a test that shows if the sorting is still in the groups, i.e. I 
can't tell if

```
import com.test.a;
import static org.a;

import com.test.b;
import static org.b;
```

becomes

```
import static org.a;
import static org.b;
import com.test.a;

import com.test.b;
```

or

```
import static org.a;
import static org.b;

import com.test.a;

import com.test.b;
```

or

```
import static org.a;
import com.test.a;

import static org.b;
import com.test.b;

```








Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

Can we consider changing the name or the option to make it clearer what its for?

`SortJavaStaticImport`

and make it an enumeration rather than true/false

`Before,After,Never`

There need to be a "don't touch it option (.e.g. Never)" that does what it does 
today (and that should be the default, otherwise we end up causing clang-format 
to change ALL java code" which could be 100's of millions of lines+





Comment at: clang/lib/Format/Format.cpp:906
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;

We can't have a default that does will cause a change



Comment at: clang/unittests/Format/SortImportsTestJava.cpp:253
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;

please test all options

You are also missing a test for checking the PARSING of the options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-09-08 Thread Ties Stuij via Phabricator via cfe-commits

stuij commandeered this revision.
stuij added a reviewer: dnsampaio.
stuij added a comment.

Commandeering as I've made some changes to the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-09-08 Thread Ties Stuij via Phabricator via cfe-commits

stuij marked 6 inline comments as done.
stuij added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:396
+/// according to the field declaring type width.
+CODEGENOPT(ForceNoAAPCSBitfieldWidth, 1, 0)
+

ostannard wrote:
> dnsampaio wrote:
> > ostannard wrote:
> > > Why is this a negative option, when the one above is positive?
> > The enforcing of number of accesses would not be accepted if it was not an 
> > opt-in option. This one I expect it should be accepted with a single 
> > opt-out option.
> My problem is with the name of the option (adding an extra negative just 
> makes things more confusing), not with the default value. This could just be 
> called `AAPCSBitfieldWidth`, (unless you think the `Force` is adding 
> something), and default to true father than false.
done



Comment at: clang/include/clang/Driver/Options.td:2328
   HelpText<"Follows the AAPCS standard that all volatile bit-field write 
generates at least one load. (ARM only).">;
+def ForceNoAAPCSBitfieldWidth : Flag<["-"], "fno-AAPCSBitfieldWidth">, 
Group,
+  Flags<[DriverOption,CC1Option]>,

ostannard wrote:
> ostannard wrote:
> > Command-line options are in kebab-case, so this should be something like 
> > `fno-aapcs-bitfield-width`. This also applies to the `fAAPCSBitfieldLoad` 
> > option above, assuming it's not too late to change that.
> > 
> > Please also add a positive version of this option (i.e. 
> > `faapcs-bitfield-width`).
> This still needs a positive version.
done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-08 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff updated this revision to Diff 290487.
sepavloff added a comment.

Updated patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp

Index: clang/test/AST/ast-dump-fpfeatures.cpp
===
--- clang/test/AST/ast-dump-fpfeatures.cpp
+++ clang/test/AST/ast-dump-fpfeatures.cpp
@@ -34,4 +34,48 @@
 // CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT:   ReturnStmt
-// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
\ No newline at end of file
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
+
+int func_04(float x) {
+#pragma STDC FP_CONTRACT ON
+  return x;
+}
+
+// CHECK:  FunctionDecl {{.*}} func_04 'int (float)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   ImplicitCastExpr {{.*}} 'int'  FPContractMode=1
+
+float func_05(double x) {
+#pragma STDC FP_CONTRACT ON
+  return (float)x;
+}
+
+// CHECK:  FunctionDecl {{.*}} func_05 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CStyleCastExpr {{.*}} FPContractMode=1
+
+float func_06(double x) {
+#pragma STDC FP_CONTRACT ON
+  return float(x);
+}
+
+// CHECK:  FunctionDecl {{.*}} func_06 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CXXFunctionalCastExpr {{.*}} FPContractMode=1
+
+float func_07(double x) {
+#pragma STDC FP_CONTRACT ON
+  return static_cast(x);
+}
+
+// CHECK:  FunctionDecl {{.*}} func_07 'float (double)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'double'
+// CHECK-NEXT:   CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT:   CXXStaticCastExpr {{.*}} FPContractMode=1
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -946,12 +946,16 @@
 void ASTStmtWriter::VisitCastExpr(CastExpr *E) {
   VisitExpr(E);
   Record.push_back(E->path_size());
+  Record.push_back(E->hasStoredFPFeatures());
   Record.AddStmt(E->getSubExpr());
   Record.push_back(E->getCastKind()); // FIXME: stable encoding
 
   for (CastExpr::path_iterator
  PI = E->path_begin(), PE = E->path_end(); PI != PE; ++PI)
 Record.AddCXXBaseSpecifier(**PI);
+
+  if (E->hasStoredFPFeatures())
+Record.push_back(E->getFPFeatures().getAsOpaqueInt());
 }
 
 void ASTStmtWriter::VisitBinaryOperator(BinaryOperator *E) {
@@ -1003,7 +1007,7 @@
   VisitCastExpr(E);
   Record.push_back(E->isPartOfExplicitCast());
 
-  if (E->path_size() == 0)
+  if (E->path_size() == 0 && !E->hasStoredFPFeatures())
 AbbrevToUse = Writer.getExprImplicitCastAbbrev();
 
   Code = serialization::EXPR_IMPLICIT_CAST;
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2346,6 +2346,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   // CastExpr
   Abv->Add(BitCodeAbbrevOp(0)); // PathSize
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasFPFeatures
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 6)); // CastKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // PartOfExplicitCast
   // ImplicitCastExpr
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/AS

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

2020-09-08 Thread Jake Merdich via Phabricator via cfe-commits

JakeMerdichAMD added inline comments.



Comment at: clang/include/clang/Format/Format.h:1708
+  /// \endcode
+  bool JavaStaticImportAfterImport;
+

MyDeveloperDay wrote:
> Can we consider changing the name or the option to make it clearer what its 
> for?
> 
> `SortJavaStaticImport`
> 
> and make it an enumeration rather than true/false
> 
> `Before,After,Never`
> 
> There need to be a "don't touch it option (.e.g. Never)" that does what it 
> does today (and that should be the default, otherwise we end up causing 
> clang-format to change ALL java code" which could be 100's of millions of 
> lines+
> 
> 
I'm confused, there is not currently a never option... Right now the behavior 
is always 'before', there is no 'after' or 'never'.

Making it an enum would probably be more ergonomic though, even there is no 
never option.



Comment at: clang/lib/Format/Format.cpp:906
   LLVMStyle.JavaScriptWrapImports = true;
+  LLVMStyle.JavaStaticImportAfterImport = false;
   LLVMStyle.TabWidth = 8;

MyDeveloperDay wrote:
> We can't have a default that does will cause a change
Not a default change, see previous comment for discussion.



Comment at: clang/unittests/Format/SortImportsTestJava.cpp:253
 
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+  FmtStyle.JavaStaticImportAfterImport = true;

MyDeveloperDay wrote:
> please test all options
> 
> You are also missing a test for checking the PARSING of the options
Parsing test was already noted and done (unless this changes to an enum of 
course). But testing the 'false' case here makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87201

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

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

2020-09-08 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff marked 3 inline comments as done.
sepavloff added a comment.

In D85960#2259703 , @riccibruno wrote:

>> This change allow a CallExpr to have optional FPOptionsOverride object,
>
> Should this be `CastExpr` instead?

Yes, thank you for the catch.

>> stored in trailing storage. The implementaion is made similar to the way
>> used in CallExpr.
>
> Nit, but that's not really similar. In the `CallExpr` case the trailing 
> objects are directly managed by `CallExpr` without using `TrailingObjects` in 
> the sub-classes.

This implementation was obtained by applying changes similar to those made for 
CallExpr. But you are right, there is some difference in the implementations. I 
think this statement is not too useful so removed it.




Comment at: clang/include/clang/AST/Expr.h:3612
 
+  unsigned numTrailingObjects(OverloadToken) const {
+return path_size();

riccibruno wrote:
> Here and elsewhere: `numTrailingObjects` is not part of the public interface.
Moved these methods to private section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85960

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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-09-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

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

[PATCH] D79293: [clang-format] [PR45218] Fix an issue where < and > and >> in a for loop gets incorrectly interpreted at a TemplateOpener/Closer

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay abandoned this revision.
MyDeveloperDay added a comment.

Better solution here D86581: [clang-format] Handle shifts within conditions 



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

https://reviews.llvm.org/D79293

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

[PATCH] D86581: [clang-format] Handle shifts within conditions

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc81dd3d159ab: [clang-format] Handle shifts within conditions 
(authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86581

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7565,6 +7565,21 @@
   verifyFormat("static_assert(is_convertible::value, \"AAA\");");
   verifyFormat("Constructor(A... a) : a_(X{std::forward(a)}...) {}");
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
+  verifyFormat("some_templated_type");
+}
+
+TEST_F(FormatTest, UnderstandsShiftOperators) {
+  verifyFormat("if (i < x >> 1)");
+  verifyFormat("while (i < x >> 1)");
+  verifyFormat("for (unsigned i = 0; i < i; ++i, v = v >> 1)");
+  verifyFormat("for (unsigned i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat(
+  "for (std::vector::iterator i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat("Foo.call>()");
+  verifyFormat("if (Foo.call>() == 0)");
+  verifyFormat("for (std::vector>::iterator i = 0; i < x >> 1; "
+   "++i, v = v >> 1)");
+  verifyFormat("if (w>, 1>::t)");
 }
 
 TEST_F(FormatTest, BitshiftOperatorWidth) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -56,6 +56,13 @@
  Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr, tok::kw_catch);
+}
+
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -108,6 +115,12 @@
 
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
+// Try to do a better job at looking for ">>" within the condition of
+// a statement.
+if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
+Left->ParentBracket != tok::less &&
+isKeywordWithCondition(*Line.First))
+  return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // In TT_Proto, we must distignuish between:
@@ -2768,13 +2781,6 @@
   Right.ParameterCount > 0);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken &Tok) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
- tok::kw_constexpr, tok::kw_catch);
-}
-
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7565,6 +7565,21 @@
   verifyFormat("static_assert(is_convertible::value, \"AAA\");");
   verifyFormat("Constructor(A... a) : a_(X{std::forward(a)}...) {}");
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
+  verifyFormat("some_templated_type");
+}
+
+TEST_F(FormatTest, UnderstandsShiftOperators) {
+  verifyFormat("if (i < x >> 1)");
+  verifyFormat("while (i < x >> 1)");
+  verifyFormat("for (unsigned i = 0; i < i; ++i, v = v >> 1)");
+  verifyFormat("for (unsigned i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat(
+  "for (std::vector::iterator i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat("Foo.call>()");
+  verifyFormat("if (Foo.call>() == 0)");
+  verifyFormat("for (std::vector>::iterator i = 0; i < x >> 1; "
+   "++i, v = v >> 1)");
+  verifyFormat("if (w>, 1>::t)");
 }
 
 TEST_F(FormatTest, BitshiftOperatorWidth) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -56,6 +56,13 @@
  Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr, tok::k

[clang] c81dd3d - [clang-format] Handle shifts within conditions

2020-09-08 Thread via cfe-commits


Author: mydeveloperday
Date: 2020-09-08T16:40:04+01:00
New Revision: c81dd3d159ab03d46e4280c458d3c29e56648218

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

LOG: [clang-format] Handle shifts within conditions

In some situation shifts can be treated as a template, and is thus formatted as 
one. So, by doing a couple extra checks to assure that the condition doesn't 
contain a template, and is in fact a bit shift should solve this problem.

This is a fix for [[ https://bugs.llvm.org/show_bug.cgi?id=46969 | bug 46969 ]]

Reviewed By: MyDeveloperDay

Patch By: Saldivarcher

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 5dd6a7a9da40..841f0b41e9a7 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -56,6 +56,13 @@ static bool isLambdaParameterList(const FormatToken *Left) {
  Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr, tok::kw_catch);
+}
+
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -108,6 +115,12 @@ class AnnotatingParser {
 
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
+// Try to do a better job at looking for ">>" within the condition of
+// a statement.
+if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
+Left->ParentBracket != tok::less &&
+isKeywordWithCondition(*Line.First))
+  return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // In TT_Proto, we must distignuish between:
@@ -2768,13 +2781,6 @@ bool TokenAnnotator::spaceRequiredBeforeParens(const 
FormatToken &Right) const {
   Right.ParameterCount > 0);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken &Tok) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
- tok::kw_constexpr, tok::kw_catch);
-}
-
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index b198efa4af9e..98e002003159 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -7565,6 +7565,21 @@ TEST_F(FormatTest, UnderstandsTemplateParameters) {
   verifyFormat("static_assert(is_convertible::value, \"AAA\");");
   verifyFormat("Constructor(A... a) : a_(X{std::forward(a)}...) {}");
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
+  verifyFormat("some_templated_type");
+}
+
+TEST_F(FormatTest, UnderstandsShiftOperators) {
+  verifyFormat("if (i < x >> 1)");
+  verifyFormat("while (i < x >> 1)");
+  verifyFormat("for (unsigned i = 0; i < i; ++i, v = v >> 1)");
+  verifyFormat("for (unsigned i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat(
+  "for (std::vector::iterator i = 0; i < x >> 1; ++i, v = v >> 1)");
+  verifyFormat("Foo.call>()");
+  verifyFormat("if (Foo.call>() == 0)");
+  verifyFormat("for (std::vector>::iterator i = 0; i < x >> 1; "
+   "++i, v = v >> 1)");
+  verifyFormat("if (w>, 1>::t)");
 }
 
 TEST_F(FormatTest, BitshiftOperatorWidth) {



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

[PATCH] D86581: [clang-format] Handle shifts within conditions

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

@Saldivarcher this should be landed now, please validate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86581

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

[PATCH] D84362: [NFC] Add missing functions to PartialDiagnostic

2020-09-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

ping. this is needed by https://reviews.llvm.org/D84364


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

https://reviews.llvm.org/D84362

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

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: JakeMerdichAMD, krasimir, arichardson, curdeius.
MyDeveloperDay added projects: clang-format, clang.
MyDeveloperDay requested review of this revision.

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

The following change D80940: [clang-format] [PR46159] Linux kernel 'C' code 
uses 'try' as a variable name, allow clang-format to handle such cases 
 caused a regression in code which ifdef's 
around the try and catch block cause incorrect brace placement around the catch

  #ifdef NO_EXCEPTIONS
try
  #endif
{
}
  #ifdef NO_EXCEPTIONS
catch (...) {
  // This is not a small function
  bar = 1;
}
  #endif
  }

The brace after the catch will be placed on a newline


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87291

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2743,6 +2743,19 @@
   verifyFormat("int catch, size;");
   verifyFormat("catch = foo();");
   verifyFormat("if (catch < size) {\n  return true;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("#if NO_EX\n"
+   "try\n"
+   "#endif\n"
+   "{\n"
+   "}\n"
+   "#if NO_EX\n"
+   "catch (...) {\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatSEHTryCatch) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -401,7 +401,7 @@
   if (!Try->is(tok::kw_try))
 return false;
   auto &Next = *(Tokens.end() - 1);
-  if (Next->isOneOf(tok::l_brace, tok::colon))
+  if (Next->isOneOf(tok::l_brace, tok::colon, tok::hash))
 return false;
 
   if (Tokens.size() > 2) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2743,6 +2743,19 @@
   verifyFormat("int catch, size;");
   verifyFormat("catch = foo();");
   verifyFormat("if (catch < size) {\n  return true;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("#if NO_EX\n"
+   "try\n"
+   "#endif\n"
+   "{\n"
+   "}\n"
+   "#if NO_EX\n"
+   "catch (...) {\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatSEHTryCatch) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -401,7 +401,7 @@
   if (!Try->is(tok::kw_try))
 return false;
   auto &Next = *(Tokens.end() - 1);
-  if (Next->isOneOf(tok::l_brace, tok::colon))
+  if (Next->isOneOf(tok::l_brace, tok::colon, tok::hash))
 return false;
 
   if (Tokens.size() > 2) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

2020-09-08 Thread Bevin Hansson via Phabricator via cfe-commits

ebevhan updated this revision to Diff 290502.
ebevhan added a comment.

Updated method name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86632

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/Frontend/fixed_point_compound.c
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_conversions_half.c
  llvm/include/llvm/IR/FixedPointBuilder.h

Index: llvm/include/llvm/IR/FixedPointBuilder.h
===
--- llvm/include/llvm/IR/FixedPointBuilder.h
+++ llvm/include/llvm/IR/FixedPointBuilder.h
@@ -120,6 +120,17 @@
 C.isSigned(), C.isSaturated(), BothPadded);
   }
 
+  /// Given a floating point type and a fixed-point semantic, return a floating
+  /// point type which can accommodate the fixed-point semantic. This is either
+  /// \p Ty, or a floating point type with a larger exponent than Ty.
+  Type *getAccommodatingFloatType(Type *Ty, const FixedPointSemantics &Sema) {
+const fltSemantics *FloatSema = &Ty->getFltSemantics();
+while (!Sema.fitsInFloatSemantics(*FloatSema))
+  FloatSema = APFixedPoint::promoteFloatSemantics(FloatSema);
+// There's seemingly no way to convert fltSemantics to Type.
+return ConstantFP::get(Ty->getContext(), APFloat(*FloatSema))->getType();
+  }
+
 public:
   FixedPointBuilder(IRBuilderTy &Builder) : B(Builder) {}
 
@@ -159,6 +170,55 @@
DstSema, false);
   }
 
+  Value *CreateFixedToFloating(Value *Src, const FixedPointSemantics &SrcSema,
+   Type *DstTy) {
+Value *Result;
+Type *OpTy = getAccommodatingFloatType(DstTy, SrcSema);
+// Convert the raw fixed-point value directly to floating point. If the
+// value is too large to fit, it will be rounded, not truncated.
+Result = SrcSema.isSigned() ? B.CreateSIToFP(Src, OpTy)
+: B.CreateUIToFP(Src, OpTy);
+// Rescale the integral-in-floating point by the scaling factor. This is
+// lossless, except for overflow to infinity which is unlikely.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, -(int)SrcSema.getScale(;
+if (OpTy != DstTy)
+  Result = B.CreateFPTrunc(Result, DstTy);
+return Result;
+  }
+
+  Value *CreateFloatingToFixed(Value *Src, const FixedPointSemantics &DstSema) {
+bool UseSigned = DstSema.isSigned() || DstSema.hasUnsignedPadding();
+Value *Result = Src;
+Type *OpTy = getAccommodatingFloatType(Src->getType(), DstSema);
+if (OpTy != Src->getType())
+  Result = B.CreateFPExt(Result, OpTy);
+// Rescale the floating point value so that its significant bits (for the
+// purposes of the conversion) are in the integral range.
+Result = B.CreateFMul(Result,
+ConstantFP::get(OpTy, std::pow(2, DstSema.getScale(;
+
+Type *ResultTy = B.getIntNTy(DstSema.getWidth());
+if (DstSema.isSaturated()) {
+  Intrinsic::ID IID =
+  UseSigned ? Intrinsic::fptosi_sat : Intrinsic::fptoui_sat;
+  Result = B.CreateIntrinsic(IID, {ResultTy, OpTy}, {Result});
+} else {
+  Result = UseSigned ? B.CreateFPToSI(Result, ResultTy)
+ : B.CreateFPToUI(Result, ResultTy);
+}
+
+// When saturating unsigned-with-padding using signed operations, we may
+// get negative values. Emit an extra clamp to zero.
+if (DstSema.isSaturated() && DstSema.hasUnsignedPadding()) {
+  Constant *Zero = Constant::getNullValue(Result->getType());
+  Result =
+  B.CreateSelect(B.CreateICmpSLT(Result, Zero), Zero, Result, "satmin");
+}
+
+return Result;
+  }
+
   /// Add two fixed-point values and return the result in their common semantic.
   /// \p LHS - The left hand side
   /// \p LHSSema - The semantic of the left hand side
Index: clang/test/Frontend/fixed_point_conversions_half.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_conversions_half.c
@@ -0,0 +1,309 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple arm64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
+short _Fract sf;
+long _Fract lf;
+
+short _Accum sa;
+long _Accum la;
+
+unsigned short _Accum usa;
+unsigned long _Accum ula;
+
+_Sat short _Fract sf_sat;
+_Sat long _Fract lf_sat;
+
+_Sat short _Accum sa_sat;
+_Sat long _Accum la_sat;
+
+_Sat unsigned short _Accum usa_sat;
+_Sat unsigned long _Accum ula_sat;
+
+_Float16 h;
+
+
+// CHECK-LABEL: @half_fix1(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load half, half* @h, align 2
+// CHECK-NEXT:[[TMP1:%.*]] = f

[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

2020-09-08 Thread Florian Hahn via Phabricator via cfe-commits

fhahn updated this revision to Diff 290504.
fhahn marked an inline comment as done.
fhahn added a comment.

Change to codegen option, adjust description for option, limit to trivially 
copyable types in C++, add corresponding test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/pass-by-value-noalias.c
  clang/test/CodeGenCXX/pass-by-value-noalias.cpp
  clang/test/CodeGenObjC/pass-by-value-noalias.m

Index: clang/test/CodeGenObjC/pass-by-value-noalias.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/pass-by-value-noalias.m
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns -fobjc-runtime-has-weak -fobjc-arc -fobjc-dispatch-method=mixed %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns -fobjc-runtime-has-weak -fobjc-arc -fobjc-dispatch-method=mixed %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A struct large enough so it is not passed in registers on ARM64.
+
+@interface Bar
+@property char value;
+@end
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  Bar *__weak f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}
Index: clang/test/CodeGenCXX/pass-by-value-noalias.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pass-by-value-noalias.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A trivial struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// Make sure noalias is added to indirect arguments with trivially copyable types
+// if -fpass-by-value-is-noalias is provided.
+
+// WITH_NOALIAS: define void @_Z4take3Foo(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @_Z4take3Foo(%struct.Foo* %arg)
+void take(Foo arg) {}
+
+int G;
+
+// NonTrivial is not trivially-copyable, because it has a non-trivial copy
+// constructor.
+struct NonTrivial {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+
+  NonTrivial(const NonTrivial &Other) {
+a = G + 10 + Other.a;
+  }
+};
+
+// Make sure noalias is not added to indirect arguments that are not trivially
+// copyable even if -fpass-by-value-is-noalias is provided.
+
+// WITH_NOALIAS: define void @_Z4take10NonTrivial(%struct.NonTrivial* %arg)
+// NO_NOALIAS:   define void @_Z4take10NonTrivial(%struct.NonTrivial* %arg)
+void take(NonTrivial arg) {}
Index: clang/test/CodeGen/pass-by-value-noalias.c
===
--- /dev/null
+++ clang/test/CodeGen/pass-by-value-noalias.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1453,6 +1453,8 @@
   std::string(Args.getLastArgValue(OPT_fsymbol_partition_EQ));
 
   Opts.ForceAAPCSBitfieldLoad = Args.hasArg(OPT_ForceAAPCSBitfieldLoad);
+
+  Opts.PassByValueIsNoAlias = Args.hasArg(OPT_fpass_by_value_is_noalias);
   return Success;
 }
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2201,6 +2201,13 @@
   if (AI.getIndirectByVal())
 Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
 
+  if (CodeGenOpts.PassByValueIsNoAlias &&
+  ParamType.isTriviallyCopyableType(getContext()) &&
+  !ParamType.isObjCGCWeak())
+// When calling the function, the pointer passed in will

[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

2020-09-08 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4287-4290
+def fpass_by_value_noalias: Flag<["-"], "fpass-by-value-noalias">,
+  HelpText<"Allows assuming no references to passed by value escape before "
+   "transferring execution to the called function. Note that this "
+   "does not hold for C++">;

rjmccall wrote:
> rsmith wrote:
> > This should be in `Group`.
> The "Note" clause seems to muddy more than it clarifies.  Maybe "has no 
> effect on non-trivially-copyable classes in C++"?  Or add proper 
> documentation somewhere instead of trying to jam this into the help text.
I've added the clarification, thanks!



Comment at: clang/lib/CodeGen/CGCall.cpp:2198
+// reference to the underlying object. Mark it accordingly.
+Attrs.addAttribute(llvm::Attribute::NoAlias);
+

rjmccall wrote:
> This definitely can't be added unconditionally to all types; you need to rule 
> out non-trivial C++ class types, as well as types with ObjC weak references.
Updated to rule out non trivially-copyable types.

I am not sure how to best handle types with weak references. I did not manage 
to find any helpers that traverse a whole type to check if it contains weak 
references. 

I added `clang/test/CodeGenObjC/pass-by-value-noalias.m` and IIUC `noalias` 
should not be added for that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

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

[clang-tools-extra] 71133e8 - [clang-tidy] Fix linking for FrontendOpenMP

2020-09-08 Thread Heejin Ahn via cfe-commits


Author: Heejin Ahn
Date: 2020-09-08T09:22:22-07:00
New Revision: 71133e8b5bceaf68a2cee59af371df570a1aed79

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

LOG: [clang-tidy] Fix linking for FrontendOpenMP

Without this, builds with `-DBUILD_SHARED_LIBS=ON` fail.

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
index 878e718c6596..ed28d9f4892d 100644
--- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -1,4 +1,7 @@
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  support
+  )
 
 add_clang_library(clangTidyAlteraModule
   AlteraTidyModule.cpp



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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Rahman Lavaee via Phabricator via cfe-commits

rahmanl updated this revision to Diff 290511.
rahmanl marked an inline comment as done.
rahmanl added a comment.

- Merge remote-tracking branch 'origin/master' into arcpatch-D85408


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad [

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

As discussed offline, this is trading off some accuracy between getting `-I` 
correct vs `-std` and it is unclear whether that's beneficial or harmful. It is 
easy to come up with examples for both and we were split 50/50 between each.
In the end all of this is a heuristic work and it is quite likely that we might 
break this for more people, while trying to fix this one specific case. So I 
don't think it's worth it.

As for suggestions(again from offline discussion) :

- We can implement a more sophisticated transfer logic in clangd layer, by 
making use of compile commands from a TU including a particular header
- When there's a mismatch in language, we can try to merge two commands. One 
with filename/path match and filetype match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87253

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Rahman Lavaee via Phabricator via cfe-commits

rahmanl added a comment.

Thanks for the review @MaskRay. Is this ready to land now?




Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

MaskRay wrote:
> rahmanl wrote:
> > MaskRay wrote:
> > > BBAddrMapSection is always non-null. Delete the if.
> > I believe we return null for non-ELF environment (Please refer to 
> > MCObjectFileInfo::getBBAddrMapSection).
> In that case emitBBAddrMapSection will not be called?
Fair enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D87278: [Ignore Expressions] Fix performance regression by inlining `Ignore*SingleStep`

2020-09-08 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno accepted this revision.
riccibruno added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87278

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

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

2020-09-08 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

Note to self, other things could cause the same issue

  try // comment
  {



  try /* comment */
  {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87291

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits

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

I like the simplification:) However, I still feel puzzled by the large chunk of 
assembly test in `clang/test/CodeGen/basic-block-sections.c`. I hope another 
reviewer or the code owner can say that this is ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

2020-09-08 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

On C++20 mode rotting: it won't if someone sets up a bot. If it rots, then it's 
easier to un-rot with Barry's patch.




Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817
+  return IsASCII ? "^" : (const char *)u8"\u2548";
 case LineChar::RangeMid:
-  return IsASCII ? "|" : u8"\u2503";
+  return IsASCII ? "|" : (const char *)u8"\u2503";
 case LineChar::RangeEnd:
-  return IsASCII ? "v" : u8"\u253b";
+  return IsASCII ? "v" : (const char *)u8"\u253b";
 case LineChar::LabelVert:
+  return IsASCII ? "|" : (const char *)u8"\u2502";

jhenderson wrote:
> BRevzin wrote:
> > jhenderson wrote:
> > > This seems unrelated to comparison checking?
> > > This seems unrelated to comparison checking?
> > 
> > It is unrelated. But In C++20, `u8` literals become their own type so this 
> > no longer compiled and I wanted to ensure that I could actually run the 
> > tests. 
> Could it be a pre-requisite patch then?
I'm fine with this if the patch title is changed to "make LLVM build in C++20 
mode", and description edited accordingly. Basically, it makes it easy to 
figure out which changes were done for C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

2020-09-08 Thread Jan Korous via Phabricator via cfe-commits

jkorous added subscribers: dexonsmith, akyrtzi.
jkorous added a comment.

Hi @usaxena95 and @sammccall,

I am wondering about couple high-level things.

Do you guys intend to open-source also the training part of the model pipeline 
or publish a model trained on generic-enough training set so it could be 
reasonably used on "any" codebase?

Do you still intend to support the heuristic that is currently powering clangd 
in the future?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83814

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

[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

2020-09-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

friendly ping @rsmith.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84637

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

[clang] e6bb4c8 - [X86] SSE4_A should only imply SSE3 not SSSE3 in the frontend.

2020-09-08 Thread Craig Topper via cfe-commits


Author: Craig Topper
Date: 2020-09-08T10:50:59-07:00
New Revision: e6bb4c8e7b3e27f214c9665763a2dd09aa96a5ac

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

LOG: [X86] SSE4_A should only imply SSE3 not SSSE3 in the frontend.

SSE4_1 and SSE4_2 due imply SSSE3. So I guess I got confused when
switching the code to being table based in D83273.

Fixes PR47464

Added: 


Modified: 
clang/test/Preprocessor/predefined-arch-macros.c
llvm/lib/Support/X86TargetParser.cpp

Removed: 




diff  --git a/clang/test/Preprocessor/predefined-arch-macros.c 
b/clang/test/Preprocessor/predefined-arch-macros.c
index 5326596fee93..3c369ace32d5 100644
--- a/clang/test/Preprocessor/predefined-arch-macros.c
+++ b/clang/test/Preprocessor/predefined-arch-macros.c
@@ -2525,6 +2525,7 @@
 // CHECK_AMDFAM10_M32: #define __SSE4A__ 1
 // CHECK_AMDFAM10_M32: #define __SSE_MATH__ 1
 // CHECK_AMDFAM10_M32: #define __SSE__ 1
+// CHECK_AMDFAM10_M32-NOT: #define __SSSE3__ 1
 // CHECK_AMDFAM10_M32: #define __amdfam10 1
 // CHECK_AMDFAM10_M32: #define __amdfam10__ 1
 // CHECK_AMDFAM10_M32: #define __i386 1
@@ -2547,6 +2548,7 @@
 // CHECK_AMDFAM10_M64: #define __SSE4A__ 1
 // CHECK_AMDFAM10_M64: #define __SSE_MATH__ 1
 // CHECK_AMDFAM10_M64: #define __SSE__ 1
+// CHECK_AMDFAM10_M64-NOT: #define __SSSE3__ 1
 // CHECK_AMDFAM10_M64: #define __amd64 1
 // CHECK_AMDFAM10_M64: #define __amd64__ 1
 // CHECK_AMDFAM10_M64: #define __amdfam10 1

diff  --git a/llvm/lib/Support/X86TargetParser.cpp 
b/llvm/lib/Support/X86TargetParser.cpp
index a5af98582452..b7d9bd4f865c 100644
--- a/llvm/lib/Support/X86TargetParser.cpp
+++ b/llvm/lib/Support/X86TargetParser.cpp
@@ -529,7 +529,7 @@ static constexpr FeatureBitset ImpliedFeaturesAVX5124FMAPS 
= {};
 static constexpr FeatureBitset ImpliedFeaturesAVX5124VNNIW = {};
 
 // SSE4_A->FMA4->XOP chain.
-static constexpr FeatureBitset ImpliedFeaturesSSE4_A = FeatureSSSE3;
+static constexpr FeatureBitset ImpliedFeaturesSSE4_A = FeatureSSE3;
 static constexpr FeatureBitset ImpliedFeaturesFMA4 = FeatureAVX | 
FeatureSSE4_A;
 static constexpr FeatureBitset ImpliedFeaturesXOP = FeatureFMA4;
 



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

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-08 Thread Jonas Devlieghere via Phabricator via cfe-commits

JDevlieghere added a comment.

You need to add `LLVM_ENABLE_WARNINGS` to `LLVMConfig.cmake.in` so that the 
standalone builds know what value was set in the LLVM build. I think with the 
current patch the other projects won't inherit the value and just default to 
`ON`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[clang] d95ef00 - Update clang/test/Driver/darwin-infer-simulator-sdkroot.c

2020-09-08 Thread Azharuddin Mohammed via cfe-commits


Author: Azharuddin Mohammed
Date: 2020-09-08T11:27:18-07:00
New Revision: d95ef009bd502a1c2c82952d4fa6fd1db836cef9

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

LOG: Update clang/test/Driver/darwin-infer-simulator-sdkroot.c

 - Fix it to work on Apple Silicon
 - Add testcases for simulators running on Apple Silicon

Added: 


Modified: 
clang/test/Driver/darwin-infer-simulator-sdkroot.c

Removed: 




diff  --git a/clang/test/Driver/darwin-infer-simulator-sdkroot.c 
b/clang/test/Driver/darwin-infer-simulator-sdkroot.c
index a084bf6346b6..7d4d4070b81a 100644
--- a/clang/test/Driver/darwin-infer-simulator-sdkroot.c
+++ b/clang/test/Driver/darwin-infer-simulator-sdkroot.c
@@ -17,7 +17,7 @@
 //
 // RUN: rm -rf %t/SDKs/iPhoneSimulator8.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneSimulator8.0.sdk
-// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang %s 
-mlinker-version=400 -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -arch x86_64 %s 
-mlinker-version=400 -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
 //
 // CHECK-SIMULATOR: clang
@@ -27,6 +27,18 @@
 // CHECK-SIMULATOR: "-ios_simulator_version_min" "8.0.0"
 //
 //
+// RUN: rm -rf %t/SDKs/iPhoneSimulator14.0.sdk
+// RUN: mkdir -p %t/SDKs/iPhoneSimulator14.0.sdk
+// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator14.0.sdk %clang -arch arm64 %s 
-mlinker-version=400 -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR-ARM64 %s
+//
+// CHECK-SIMULATOR-ARM64: clang
+// CHECK-SIMULATOR-ARM64: "-cc1"
+// CHECK-SIMULATOR-ARM64: -apple-ios14.0.0-simulator"
+// CHECK-SIMULATOR-ARM64: ld
+// CHECK-SIMULATOR-ARM64: "-ios_simulator_version_min" "14.0.0"
+//
+//
 // RUN: rm -rf %t/SDKs/WatchOS3.0.sdk
 // RUN: mkdir -p %t/SDKs/WatchOS3.0.sdk
 // RUN: env SDKROOT=%t/SDKs/WatchOS3.0.sdk %clang %s -mlinker-version=400 -### 
2>&1 \
@@ -43,7 +55,7 @@
 //
 // RUN: rm -rf %t/SDKs/WatchSimulator3.0.sdk
 // RUN: mkdir -p %t/SDKs/WatchSimulator3.0.sdk
-// RUN: env SDKROOT=%t/SDKs/WatchSimulator3.0.sdk %clang %s 
-mlinker-version=400 -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/WatchSimulator3.0.sdk %clang -arch x86_64 %s 
-mlinker-version=400 -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-WATCH-SIMULATOR %s
 //
 // CHECK-WATCH-SIMULATOR: clang
@@ -53,6 +65,18 @@
 // CHECK-WATCH-SIMULATOR: "-watchos_simulator_version_min" "3.0.0"
 //
 //
+// RUN: rm -rf %t/SDKs/WatchSimulator7.0.sdk
+// RUN: mkdir -p %t/SDKs/WatchSimulator7.0.sdk
+// RUN: env SDKROOT=%t/SDKs/WatchSimulator7.0.sdk %clang -arch arm64 %s 
-mlinker-version=400 -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-WATCH-SIMULATOR-ARM64 %s
+//
+// CHECK-WATCH-SIMULATOR-ARM64: clang
+// CHECK-WATCH-SIMULATOR-ARM64: "-cc1"
+// CHECK-WATCH-SIMULATOR-ARM64: -apple-watchos7.0.0-simulator"
+// CHECK-WATCH-SIMULATOR-ARM64: ld
+// CHECK-WATCH-SIMULATOR-ARM64: "-watchos_simulator_version_min" "7.0.0"
+//
+//
 // RUN: rm -rf %t/SDKs/AppleTVOS10.0.sdk
 // RUN: mkdir -p %t/SDKs/AppleTVOS10.0.sdk
 // RUN: env SDKROOT=%t/SDKs/AppleTVOS10.0.sdk %clang %s -mlinker-version=400 
-### 2>&1 \
@@ -67,7 +91,7 @@
 //
 // RUN: rm -rf %t/SDKs/AppleTVSimulator10.0.sdk
 // RUN: mkdir -p %t/SDKs/AppleTVSimulator10.0.sdk
-// RUN: env SDKROOT=%t/SDKs/AppleTVSimulator10.0.sdk %clang %s 
-mlinker-version=400 -### 2>&1 \
+// RUN: env SDKROOT=%t/SDKs/AppleTVSimulator10.0.sdk %clang -arch x86_64 %s 
-mlinker-version=400 -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-TV-SIMULATOR %s
 //
 // CHECK-TV-SIMULATOR: clang
@@ -75,3 +99,16 @@
 // CHECK-TV-SIMULATOR: -apple-tvos10.0.0-simulator"
 // CHECK-TV-SIMULATOR: ld
 // CHECK-TV-SIMULATOR: "-tvos_simulator_version_min" "10.0.0"
+//
+//
+// RUN: rm -rf %t/SDKs/AppleTVSimulator14.0.sdk
+// RUN: mkdir -p %t/SDKs/AppleTVSimulator14.0.sdk
+// RUN: env SDKROOT=%t/SDKs/AppleTVSimulator14.0.sdk %clang -arch arm64 %s 
-mlinker-version=400 -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-TV-SIMULATOR-ARM64 %s
+//
+// CHECK-TV-SIMULATOR-ARM64: clang
+// CHECK-TV-SIMULATOR-ARM64: "-cc1"
+// CHECK-TV-SIMULATOR-ARM64: -apple-tvos14.0.0-simulator"
+// CHECK-TV-SIMULATOR-ARM64: ld
+// CHECK-TV-SIMULATOR-ARM64: "-tvos_simulator_version_min" "14.0.0"
+



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

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-08 Thread Dave Lee via Phabricator via cfe-commits

kastiglione added a comment.

If an LLVM install disabled `LLVM_ENABLE_WARNINGS`, should other builds inherit 
that? I would think no, but is there's a precedent for that that to be the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D84962: [PowerPC] Correct cpsgn's behaviour on PowerPC to match that of the ABI

2020-09-08 Thread Baptiste Saleil via Phabricator via cfe-commits

bsaleil added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14298
 llvm::Function *F = CGM.getIntrinsic(ID, ResultType);
-return Builder.CreateCall(F, {X, Y});
+return Builder.CreateCall(F, {Y, X});
   }

Could you add a test case in `clang/test/CodeGen/builtins-ppc-vsx.c` showing 
that calls to the builtins and calls to `vec_cpsgn` are generated as calls to 
the `copysign` LLVM intrinsic with the arguments being inverted ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84962

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

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-09-08 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies added a comment.

In D66564#2260836 , @aaron.ballman 
wrote:

> I've committed on your behalf in 156b127945a8c923d141e608b7380427da024376 
> . Thank 
> you for the new check!

No problem! Thanks for the commit, and for all the feedback!


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

https://reviews.llvm.org/D66564

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

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: tra, MaskRay.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
yaxunl requested review of this revision.

gcc translates -gz=zlib to --compress-debug-options=zlib for both assembler and 
linker
but clang only does this for assembler.

The linker needs --compress-debug-options=zlib option to compress the debug 
sections
in the generated executable or shared library.

Due to this bug, -gz=zlib has no effect on the generated executable or shared 
library.

This patch fixes that.


https://reviews.llvm.org/D87321

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/gz.c
  clang/test/Driver/hip-gz-options.hip

Index: clang/test/Driver/hip-gz-options.hip
===
--- /dev/null
+++ clang/test/Driver/hip-gz-options.hip
@@ -0,0 +1,15 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \
+// RUN:   -ggdb -gz=zlib 2>&1 | FileCheck %s
+
+// RUN: %clang -### -fgpu-rdc --offload-arch=gfx906 %s -nogpulib -nogpuinc \
+// RUN:   -ggdb -gz=zlib 2>&1 | FileCheck %s
+
+// CHECK: {{".*clang.*" .* "--compress-debug-sections=zlib"}}
+// CHECK-DAG: {{".*lld.*" .* "--compress-debug-sections=zlib"}}
+// CHECK-DAG: {{".*clang.*" .* "--compress-debug-sections=zlib"}}
+// CHECK: {{".*ld.*" .* "--compress-debug-sections=zlib"}}
+
Index: clang/test/Driver/gz.c
===
--- /dev/null
+++ clang/test/Driver/gz.c
@@ -0,0 +1,23 @@
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -g -gz=none %s 2>&1 | \
+// RUN:FileCheck -check-prefix=NONE %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -nogpulib -g -gz=none %s 2>&1 | \
+// RUN:FileCheck -check-prefix=NONE %s
+
+// NONE: {{".*clang.*".* "--compress-debug-sections=none"}}
+// NONE: {{".*ld.*".* "--compress-debug-sections=none"}}
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -g -gz=zlib %s 2>&1 | \
+// RUN:FileCheck -check-prefix=ZLIB %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -nogpulib -g -gz=zlib %s 2>&1 | \
+// RUN:FileCheck -check-prefix=ZLIB %s
+
+// ZLIB: {{".*clang.*".* "--compress-debug-sections=zlib"}}
+// ZLIB: {{".*ld.*".* "--compress-debug-sections=zlib"}}
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -g -gz=zlib-gnu %s 2>&1 | \
+// RUN:FileCheck -check-prefix=ZLIB-GNU %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -nogpulib -g -gz=zlib-gnu %s 2>&1 | \
+// RUN:FileCheck -check-prefix=ZLIB-GNU %s
+
+// ZLIB-GNU: {{".*clang.*".* "--compress-debug-sections=zlib-gnu"}}
+// ZLIB-GNU: {{".*ld.*".* "--compress-debug-sections=zlib-gnu"}}
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -89,6 +89,8 @@
   if (C.getDriver().isSaveTempsEnabled())
 LldArgs.push_back("-save-temps");
 
+  addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
+
   LldArgs.append({"-o", Output.getFilename()});
   for (auto Input : Inputs)
 LldArgs.push_back(Input.getFilename());
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -556,6 +556,7 @@
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
+  addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   // The profile runtime also needs access to system libraries.
   getToolChain().addProfileRTLibs(Args, CmdArgs);
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -27,6 +27,10 @@
  const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs, const JobAction &JA);
 
+void addLinkerCompressDebugSectionsOption(const ToolChain &TC,
+  const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs);
+
 void claimNoWarnArgs(const llvm::opt::ArgList &Args);
 
 bool addSanitizerRuntimes(const ToolChain &TC, const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -214,6 +214,24 @@
   }
 }
 
+void tools::addLinkerCompres

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/test/Driver/gz.c:1
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -g -gz=none %s 2>&1 | \
+// RUN:FileCheck -check-prefix=NONE %s

This can be merged into `compress.c`
You may delete some `-c` from compress.c to exercise the linker stage.



Comment at: clang/test/Driver/hip-gz-options.hip:1
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target

REQUIRES values can be comma separated



Comment at: clang/test/Driver/hip-gz-options.hip:5
+
+// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \
+// RUN:   -ggdb -gz=zlib 2>&1 | FileCheck %s

Is `-target` (or `--target=`)  needed?



Comment at: clang/test/Driver/hip-gz-options.hip:15
+// CHECK: {{".*ld.*" .* "--compress-debug-sections=zlib"}}
+

Delete trailing empty lines


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

https://reviews.llvm.org/D87321

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

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/test/Driver/hip-gz-options.hip:6
+// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \
+// RUN:   -ggdb -gz=zlib 2>&1 | FileCheck %s
+

MaskRay wrote:
> -ggdb is unrelated
Sorry it is related.


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

https://reviews.llvm.org/D87321

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

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/test/Driver/hip-gz-options.hip:6
+// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \
+// RUN:   -ggdb -gz=zlib 2>&1 | FileCheck %s
+

-ggdb is unrelated


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

https://reviews.llvm.org/D87321

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

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-08 Thread Alain Mosnier via Phabricator via cfe-commits

amosnier added a comment.

In D87253#2261205 , @kadircet wrote:

> As discussed offline, this is trading off some accuracy between getting `-I` 
> correct vs `-std` and it is unclear whether that's beneficial or harmful. It 
> is easy to come up with examples for both and we were split 50/50 between 
> each.
> In the end all of this is a heuristic work and it is quite likely that we 
> might break this for more people, while trying to fix this one specific case. 
> So I don't think it's worth it.
>
> As for suggestions(again from offline discussion) :
>
> - We can implement a more sophisticated transfer logic in clangd layer, by 
> making use of compile commands from a TU including a particular header
> - When there's a mismatch in language, we can try to merge two commands. One 
> with filename/path match and filetype match.

I do not have the full picture, but I trust your judgment. I think I understand 
the first suggestion, which sounds very good. I would offer to help, but I know 
I won't have enough time. I can test. :-) How can I make sure I will receive 
updates on this issue (or any prolongation of it)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87253

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

[PATCH] D87253: [libTooling] Change CDB heuristic to look further for files in a given language.

2020-09-08 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

In D87253#2261837 , @amosnier wrote:

> How can I make sure I will receive updates on this issue (or any prolongation 
> of it)?

By commenting on this review, you're automatically subscribed to it, so should 
get an email notification for any future comments or patch updates.

(You should also receive notifications for comments on the issue you filed, 
https://github.com/clangd/clangd/issues/519.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87253

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

[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

2020-09-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:138-139
   SVal ThisVal = State->getSVal(ThisPtr);
+  if (Init->isBaseInitializer() || Init->isDelegatingInitializer())
+return ThisVal;
 

For base initializer you probably want the base class region. It may have a 
non-trivial offset and it also has the correct type and extent.


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

https://reviews.llvm.org/D85351

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

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-08 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added a comment.

Note that we now also have to compute the PostDominatorTree, which adds an 
extra bit of compile-time overhead. By adjusting the pipeline a bit more, we 
can re-use ADCE's PDTs in most cases, which gives a `-0.18%` geomean 
improvement for -O3 D87322  
http://llvm-compile-time-tracker.com/compare.php?from=15fdd6cd7c24c745df1bb419e72ff66fd138aa7e&to=481f494515fc89cb7caea8d862e40f2c910dc994&stat=instructions

Ideally we would preserve the PDT, but my first try with using DomTreeUpdater 
introduced a significant compile-time regression, so I put that on hold for now.

I don't think it is worth holding off flipping the default for those changes. 
They can be done as follow-ups I think and there probably will also be a few 
other small issues to iron out. The compile-time numbers shared are without the 
recent improvements to MemDepAnalysis (which sped up legacy DSE a bit), so the 
temporary change will be slightly bigger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163

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

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

2020-09-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2429
 if (SymbolRef Sym = Len.getAsSymbol()) {
   if (SR.isDead(Sym))
+Entries = F.remove(Entries, MR);

martong wrote:
> steakhal wrote:
> > NoQ wrote:
> > > Ok, this doesn't look correct (looks like it never was). It's liveness of 
> > > the region that's important to us, not liveness of the symbol. Because 
> > > it's the liveness of the region that causes the program to be able to 
> > > access the map entry.
> > Let's say we have this:
> > ```lang=C++
> > void foo() {
> >   char *p = malloc(12);
> >   // strlen(p); // no-leak if strlen called, leak warning otherwise...
> > } // expected-warning {{leak}}
> > ```
> > If we were marking the region live here, we would miss the `leak` warning. 
> > That warning is only triggered if the region of the `p` becomes dead. Which 
> > will never become dead if we have a cstring length symbol associated to 
> > that region.
> > I came to this conclusion after implementing your suggested edit above 
> > (checking regions instead of symbols).
> Is it possible to have two string length symbols that are associated to the 
> same region? Could that cause any problem?
> E.g. what will happen below? Will we remove `MR` twice? 
> ```
> char *p = "asdf"
> char *q = p + 1;
> strlen(p); strlen(q);
> ```
> Which will never become dead if we have a cstring length symbol associated to 
> that region.

That's not how it's supposed to work. Symbols don't keep their associated 
regions alive, only regions keep symbols associated with them alive.

Don't manipulate region liveness manually at all here. Regardless of 
`strlen()`, the heap region dies with `p` which is the last variable to refer 
to it. Rely on it to garbage-collect the symbol for `strlen(p)` but don't 
actively mutate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445

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

[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

2020-09-08 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4583
   if (!InitE)
 return getDefaultInitValue(VD->getType(), Val);
 

The initializer might also be null because the variable is type-dependent (eg, 
`X x;`), in which case assuming default-init is wrong. We 
should check for that and treat it like a value-dependent initializer.



Comment at: clang/lib/AST/ExprConstant.cpp:4961
 }
+if (IS->getCond()->isValueDependent())
+  return EvaluateDependentExpr(IS->getCond(), Info);

hokein wrote:
> The `if` stmt (the same to `while`, `for`) is tricky -- if the condition is 
> value-dependent, then we don't know which branch we should run into.
> 
> - returning a ESR_Succeeded may lead to a bogus diagnostic ("reach the end of 
> the function");
> - returning a ESR_Failed may lead to a bogus diagnostic ("never produce a 
> constexpr"); 
> 
> I guess what we want is to stop the evaluation, and indicate that we hit a 
> value-dependent expression and we don't know how to evaluate it, but still 
> treat the constexpr function as potential constexpr (but no extra diagnostics 
> being emitted), but the current `EvalStmtResult` is not sufficient, maybe we 
> need a new enum.
We should only produce the "never produce a constant expression" diagnostic if 
we also produce a CCEDiag/FFDiag, so I think returning ESR_Failed here should 
work.



Comment at: clang/lib/AST/ExprConstant.cpp:4961
 }
+if (IS->getCond()->isValueDependent())
+  return EvaluateDependentExpr(IS->getCond(), Info);

rsmith wrote:
> hokein wrote:
> > The `if` stmt (the same to `while`, `for`) is tricky -- if the condition is 
> > value-dependent, then we don't know which branch we should run into.
> > 
> > - returning a ESR_Succeeded may lead to a bogus diagnostic ("reach the end 
> > of the function");
> > - returning a ESR_Failed may lead to a bogus diagnostic ("never produce a 
> > constexpr"); 
> > 
> > I guess what we want is to stop the evaluation, and indicate that we hit a 
> > value-dependent expression and we don't know how to evaluate it, but still 
> > treat the constexpr function as potential constexpr (but no extra 
> > diagnostics being emitted), but the current `EvalStmtResult` is not 
> > sufficient, maybe we need a new enum.
> We should only produce the "never produce a constant expression" diagnostic 
> if we also produce a CCEDiag/FFDiag, so I think returning ESR_Failed here 
> should work.
Should this check live in EvaluateCond instead?



Comment at: clang/lib/AST/ExprConstant.cpp:5053
 FullExpressionRAII IncScope(Info);
 if (!EvaluateIgnoredValue(Info, FS->getInc()) || !IncScope.destroy())
   return ESR_Failed;

Missing value dependence check here.



Comment at: clang/lib/AST/ExprConstant.cpp:7896
   QualType Type = Inner->getType();
-
+  if (Inner->isValueDependent())
+return EvaluateDependentExpr(Inner, Info);

How does this happen? I would expect the dependence of the subexpression to be 
reflected in the MaterializeTemporaryExpr.



Comment at: clang/lib/AST/ExprConstant.cpp:8440
 } else {
+  if (SubExpr->isValueDependent())
+return EvaluateDependentExpr(SubExpr, Info);

How does this happen?



Comment at: clang/lib/AST/ExprConstant.cpp:9145
   } else if (Init) {
+if (Init->isValueDependent())
+  return EvaluateDependentExpr(Init, Info);

How does this happen? Do we not propagate value-dependence from initializers to 
new-expressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84637

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

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

2020-09-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2429
 if (SymbolRef Sym = Len.getAsSymbol()) {
   if (SR.isDead(Sym))
+Entries = F.remove(Entries, MR);

NoQ wrote:
> martong wrote:
> > steakhal wrote:
> > > NoQ wrote:
> > > > Ok, this doesn't look correct (looks like it never was). It's liveness 
> > > > of the region that's important to us, not liveness of the symbol. 
> > > > Because it's the liveness of the region that causes the program to be 
> > > > able to access the map entry.
> > > Let's say we have this:
> > > ```lang=C++
> > > void foo() {
> > >   char *p = malloc(12);
> > >   // strlen(p); // no-leak if strlen called, leak warning otherwise...
> > > } // expected-warning {{leak}}
> > > ```
> > > If we were marking the region live here, we would miss the `leak` 
> > > warning. That warning is only triggered if the region of the `p` becomes 
> > > dead. Which will never become dead if we have a cstring length symbol 
> > > associated to that region.
> > > I came to this conclusion after implementing your suggested edit above 
> > > (checking regions instead of symbols).
> > Is it possible to have two string length symbols that are associated to the 
> > same region? Could that cause any problem?
> > E.g. what will happen below? Will we remove `MR` twice? 
> > ```
> > char *p = "asdf"
> > char *q = p + 1;
> > strlen(p); strlen(q);
> > ```
> > Which will never become dead if we have a cstring length symbol associated 
> > to that region.
> 
> That's not how it's supposed to work. Symbols don't keep their associated 
> regions alive, only regions keep symbols associated with them alive.
> 
> Don't manipulate region liveness manually at all here. Regardless of 
> `strlen()`, the heap region dies with `p` which is the last variable to refer 
> to it. Rely on it to garbage-collect the symbol for `strlen(p)` but don't 
> actively mutate it.
> E.g. what will happen below?

These are different regions. Your `p` is `Element{0, "asdf", char}`, whereas 
your `q` is `Element{1, "asdf", char}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86445

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

[PATCH] D87324: [HIP] Add gfx1030 and gfx1031

2020-09-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.
yaxunl requested review of this revision.

https://reviews.llvm.org/D87324

Files:
  clang/lib/Basic/Cuda.cpp
  clang/test/Driver/hip-offload-arch.hip


Index: clang/test/Driver/hip-offload-arch.hip
===
--- /dev/null
+++ clang/test/Driver/hip-offload-arch.hip
@@ -0,0 +1,10 @@
+// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx1030 \
+// RUN:   --offload-arch=gfx1031 \
+// RUN:   -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1030"}}
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1031"}}
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -84,7 +84,7 @@
 GFX(810), // stoney
 GFX(900), // vega, instinct
 GFX(902), GFX(904), GFX(906), GFX(908), GFX(909),
-GFX(1010), GFX(1011), GFX(1012),
+GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031)
 // clang-format on
 };
 #undef SM


Index: clang/test/Driver/hip-offload-arch.hip
===
--- /dev/null
+++ clang/test/Driver/hip-offload-arch.hip
@@ -0,0 +1,10 @@
+// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx1030 \
+// RUN:   --offload-arch=gfx1031 \
+// RUN:   -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1030"}}
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1031"}}
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -84,7 +84,7 @@
 GFX(810), // stoney
 GFX(900), // vega, instinct
 GFX(902), GFX(904), GFX(906), GFX(908), GFX(909),
-GFX(1010), GFX(1011), GFX(1012),
+GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031)
 // clang-format on
 };
 #undef SM
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-08 Thread Saleem Abdulrasool via Phabricator via cfe-commits

compnerd added a comment.

The change itself is fine, but what about downstream projects which define the 
option?  Will that trigger a warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D87324: [HIP] Add gfx1030 and gfx1031

2020-09-08 Thread Aaron Enye Shi via Phabricator via cfe-commits

ashi1 accepted this revision.
ashi1 added a comment.

LGTM, looks like its already in clang/include/clang/Basic/Cuda.h and 
clang/lib/Basic/Targets/AMDGPU.cpp.


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

https://reviews.llvm.org/D87324

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

[clang] f4ac79a - Sema: extract a check for `isCFError` (NFC)

2020-09-08 Thread Saleem Abdulrasool via cfe-commits


Author: Saleem Abdulrasool
Date: 2020-09-08T20:07:47Z
New Revision: f4ac79a364f2de7270a3238b176e17b40b036305

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

LOG: Sema: extract a check for `isCFError` (NFC)

Extract a simple check to check if a `RecordDecl` is a `CFError` Decl.
This is a simple refactoring to prepare for an upcoming change.  NFC.

Patch is extracted from
https://github.com/llvm/llvm-project-staging/commit/8afaf3aad2af43cfedca7a24cd817848c4e95c0c.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 53d0285d3702..129ac0355c87 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12415,6 +12415,7 @@ class Sema final {
 
   /// The struct behind the CFErrorRef pointer.
   RecordDecl *CFError = nullptr;
+  bool isCFError(RecordDecl *D);
 
   /// Retrieve the identifier "NSError".
   IdentifierInfo *getNSErrorIdent();

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 03442fb03b3a..d8ea9c037259 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -4043,32 +4043,9 @@ classifyPointerDeclarator(Sema &S, QualType type, 
Declarator &declarator,
 if (auto recordType = type->getAs()) {
   RecordDecl *recordDecl = recordType->getDecl();
 
-  bool isCFError = false;
-  if (S.CFError) {
-// If we already know about CFError, test it directly.
-isCFError = (S.CFError == recordDecl);
-  } else {
-// Check whether this is CFError, which we identify based on its bridge
-// to NSError. CFErrorRef used to be declared with "objc_bridge" but is
-// now declared with "objc_bridge_mutable", so look for either one of
-// the two attributes.
-if (recordDecl->getTagKind() == TTK_Struct && numNormalPointers > 0) {
-  IdentifierInfo *bridgedType = nullptr;
-  if (auto bridgeAttr = recordDecl->getAttr())
-bridgedType = bridgeAttr->getBridgedType();
-  else if (auto bridgeAttr =
-   recordDecl->getAttr())
-bridgedType = bridgeAttr->getBridgedType();
-
-  if (bridgedType == S.getNSErrorIdent()) {
-S.CFError = recordDecl;
-isCFError = true;
-  }
-}
-  }
-
   // If this is CFErrorRef*, report it as such.
-  if (isCFError && numNormalPointers == 2 && numTypeSpecifierPointers < 2) 
{
+  if (numNormalPointers == 2 && numTypeSpecifierPointers < 2 &&
+  S.isCFError(recordDecl)) {
 return PointerDeclaratorKind::CFErrorRefPointer;
   }
   break;
@@ -4092,6 +4069,31 @@ classifyPointerDeclarator(Sema &S, QualType type, 
Declarator &declarator,
   }
 }
 
+bool Sema::isCFError(RecordDecl *RD) {
+  // If we already know about CFError, test it directly.
+  if (CFError)
+return CFError == RD;
+
+  // Check whether this is CFError, which we identify based on its bridge to
+  // NSError. CFErrorRef used to be declared with "objc_bridge" but is now
+  // declared with "objc_bridge_mutable", so look for either one of the two
+  // attributes.
+  if (RD->getTagKind() == TTK_Struct) {
+IdentifierInfo *bridgedType = nullptr;
+if (auto bridgeAttr = RD->getAttr())
+  bridgedType = bridgeAttr->getBridgedType();
+else if (auto bridgeAttr = RD->getAttr())
+  bridgedType = bridgeAttr->getBridgedType();
+
+if (bridgedType == getNSErrorIdent()) {
+  CFError = RD;
+  return true;
+}
+  }
+
+  return false;
+}
+
 static FileID getNullabilityCompletenessCheckFileID(Sema &S,
 SourceLocation loc) {
   // If we're anywhere in a function, method, or closure context, don't perform



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

[clang] 041da0d - [HIP] Add gfx1031 and gfx1030

2020-09-08 Thread Yaxun Liu via cfe-commits


Author: Yaxun (Sam) Liu
Date: 2020-09-08T16:38:34-04:00
New Revision: 041da0d828e39d849c99adf1391aaa9291f4310f

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

LOG: [HIP] Add gfx1031 and gfx1030

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

Added: 
clang/test/Driver/hip-offload-arch.hip

Modified: 
clang/lib/Basic/Cuda.cpp

Removed: 




diff  --git a/clang/lib/Basic/Cuda.cpp b/clang/lib/Basic/Cuda.cpp
index 709185707bd9..2abbe3e81e0a 100644
--- a/clang/lib/Basic/Cuda.cpp
+++ b/clang/lib/Basic/Cuda.cpp
@@ -84,7 +84,7 @@ CudaArchToStringMap arch_names[] = {
 GFX(810), // stoney
 GFX(900), // vega, instinct
 GFX(902), GFX(904), GFX(906), GFX(908), GFX(909),
-GFX(1010), GFX(1011), GFX(1012),
+GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031)
 // clang-format on
 };
 #undef SM

diff  --git a/clang/test/Driver/hip-offload-arch.hip 
b/clang/test/Driver/hip-offload-arch.hip
new file mode 100644
index ..4cd37b5815f7
--- /dev/null
+++ b/clang/test/Driver/hip-offload-arch.hip
@@ -0,0 +1,10 @@
+// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx1030 \
+// RUN:   --offload-arch=gfx1031 \
+// RUN:   -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1030"}}
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1031"}}



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

[PATCH] D87324: [HIP] Add gfx1030 and gfx1031

2020-09-08 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG041da0d828e3: [HIP] Add gfx1031 and gfx1030 (authored by 
yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87324

Files:
  clang/lib/Basic/Cuda.cpp
  clang/test/Driver/hip-offload-arch.hip


Index: clang/test/Driver/hip-offload-arch.hip
===
--- /dev/null
+++ clang/test/Driver/hip-offload-arch.hip
@@ -0,0 +1,10 @@
+// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx1030 \
+// RUN:   --offload-arch=gfx1031 \
+// RUN:   -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1030"}}
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1031"}}
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -84,7 +84,7 @@
 GFX(810), // stoney
 GFX(900), // vega, instinct
 GFX(902), GFX(904), GFX(906), GFX(908), GFX(909),
-GFX(1010), GFX(1011), GFX(1012),
+GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031)
 // clang-format on
 };
 #undef SM


Index: clang/test/Driver/hip-offload-arch.hip
===
--- /dev/null
+++ clang/test/Driver/hip-offload-arch.hip
@@ -0,0 +1,10 @@
+// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --offload-arch=gfx1030 \
+// RUN:   --offload-arch=gfx1031 \
+// RUN:   -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1030"}}
+// CHECK: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx1031"}}
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -84,7 +84,7 @@
 GFX(810), // stoney
 GFX(900), // vega, instinct
 GFX(902), GFX(904), GFX(906), GFX(908), GFX(909),
-GFX(1010), GFX(1011), GFX(1012),
+GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031)
 // clang-format on
 };
 #undef SM
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

2020-09-08 Thread Dave Lee via Phabricator via cfe-commits

kastiglione added a comment.

If another project defines `LLVM_ENABLE_WARNINGS` before loading 
`HandleLLVMOptions`, it seems correct to me that the first one is used. This 
change ensures the default value of ON is setup at the last possible 
opportunity, before `LLVM_ENABLE_WARNINGS` is read and acted on.

To answer the question, according to a small sample, there's no warning in the 
case of redundant `option()`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87243

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

2020-09-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp:32-34
+auto CStringChecker::createOutOfBoundErrorMsg(StringRef FunctionDescription,
+  AccessKind Access)
+-> ErrorMessage {

Why suddenly use arrow syntax here?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h:227
+#endif
\ No newline at end of file


No NeWlInE aT eNd Of FiLe



Comment at: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp:309
+
+// TODO: Is it useful?
+void CStringChecker::printState(raw_ostream &Out, ProgramStateRef State,

Yes it is. It gets invoked during exploded graph dumps and it's an invaluable 
debugging facility.


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

https://reviews.llvm.org/D84316

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

1 2 >

1 - 100 of 122 matches

Mail list logo