[PATCH] D60995: [clangd] Make sure include path does not contain any backslashes on Windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.
kadircet added a child revision: D60873: [clang][HeaderSuggestion] Handle the 
case of dotdot with an absolute path.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60995

Files:
  clang-tools-extra/clangd/Headers.cpp


Index: clang-tools-extra/clangd/Headers.cpp
===
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -193,8 +193,10 @@
   bool IsSystem = false;
   if (!HeaderSearchInfo)
 return "\"" + InsertedHeader.File + "\"";
-  std::string Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(
-  InsertedHeader.File, BuildDir, &IsSystem);
+  // Suggest path might contain back slashes on windows.
+  std::string Suggested = llvm::sys::path::convert_to_slash(
+  HeaderSearchInfo->suggestPathToFileForDiagnostics(InsertedHeader.File,
+BuildDir, &IsSystem));
   if (IsSystem)
 Suggested = "<" + Suggested + ">";
   else


Index: clang-tools-extra/clangd/Headers.cpp
===
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -193,8 +193,10 @@
   bool IsSystem = false;
   if (!HeaderSearchInfo)
 return "\"" + InsertedHeader.File + "\"";
-  std::string Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(
-  InsertedHeader.File, BuildDir, &IsSystem);
+  // Suggest path might contain back slashes on windows.
+  std::string Suggested = llvm::sys::path::convert_to_slash(
+  HeaderSearchInfo->suggestPathToFileForDiagnostics(InsertedHeader.File,
+BuildDir, &IsSystem));
   if (IsSystem)
 Suggested = "<" + Suggested + ">";
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Sent out D60995 , hopefully it should fix the 
issue. Will wait until that lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60873



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
+

This looks wrong



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:24
+  "ERR_CAST", "PTR_ERR_OR_ZERO"));
+  auto NonCheckingStmts = stmt(anyOf(compoundStmt(), labelStmt()));
+  Finder->addMatcher(

Are there any rules what kernel considers to be checking and not checking?
This i think e.g. will accept cast-to-void, assignment to a variable and then 
ignoring it, etc.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

r358951 - [Analyzer] Instead of recording comparisons in interator checkers do an eager state split

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Apr 23 00:15:55 2019
New Revision: 358951

URL: http://llvm.org/viewvc/llvm-project?rev=358951&view=rev
Log:
[Analyzer] Instead of recording comparisons in interator checkers do an eager 
state split

Currently iterator checkers record comparison of iterator positions
and process them for keeping track the distance between them (e.g.
whether a position is the same as the end position). However this
makes some processing unnecessarily complex and it is not needed at
all: we only need to keep track between the abstract symbols stored
in these iterator positions. This patch changes this and opens the
path to comparisons to the begin() and end() symbols between the
container (e.g. size, emptiness) which are stored as symbols, not
iterator positions. The functionality of the checker is unchanged.

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=358951&r1=358950&r2=358951&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Apr 23 
00:15:55 2019
@@ -133,8 +133,6 @@ public:
   }
 };
 
-typedef llvm::PointerUnion RegionOrSymbol;
-
 // Structure to record the symbolic begin and end position of a container
 struct ContainerData {
 private:
@@ -172,41 +170,21 @@ public:
   }
 };
 
-// Structure fo recording iterator comparisons. We needed to retrieve the
-// original comparison expression in assumptions.
-struct IteratorComparison {
-private:
-  RegionOrSymbol Left, Right;
-  bool Equality;
-
-public:
-  IteratorComparison(RegionOrSymbol L, RegionOrSymbol R, bool Eq)
-  : Left(L), Right(R), Equality(Eq) {}
-
-  RegionOrSymbol getLeft() const { return Left; }
-  RegionOrSymbol getRight() const { return Right; }
-  bool isEquality() const { return Equality; }
-  bool operator==(const IteratorComparison &X) const {
-return Left == X.Left && Right == X.Right && Equality == X.Equality;
-  }
-  bool operator!=(const IteratorComparison &X) const {
-return Left != X.Left || Right != X.Right || Equality != X.Equality;
-  }
-  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Equality); }
-};
-
 class IteratorChecker
 : public Checker, check::Bind,
- check::LiveSymbols, check::DeadSymbols,
- eval::Assume> {
+ check::LiveSymbols, check::DeadSymbols> {
 
   std::unique_ptr OutOfRangeBugType;
   std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
 
-  void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal 
&LVal,
-const SVal &RVal, OverloadedOperatorKind Op) const;
+  void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+const SVal &LVal, const SVal &RVal,
+OverloadedOperatorKind Op) const;
+  void processComparison(CheckerContext &C, ProgramStateRef State,
+ SymbolRef Sym1, SymbolRef Sym2, const SVal &RetVal,
+ OverloadedOperatorKind Op) const;
   void verifyAccess(CheckerContext &C, const SVal &Val) const;
   void verifyDereference(CheckerContext &C, const SVal &Val) const;
   void handleIncrement(CheckerContext &C, const SVal &RetVal, const SVal &Iter,
@@ -281,8 +259,6 @@ public:
  CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
-  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
- bool Assumption) const;
 };
 } // namespace
 
@@ -292,9 +268,6 @@ REGISTER_MAP_WITH_PROGRAMSTATE(IteratorR
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ContainerMap, const MemRegion *, ContainerData)
 
-REGISTER_MAP_WITH_PROGRAMSTATE(IteratorComparisonMap, const SymExpr *,
-   IteratorComparison)
-
 namespace {
 
 bool isIteratorType(const QualType &Type);
@@ -324,16 +297,6 @@ bool isRandomIncrOrDecrOperator(Overload
 bool hasSubscriptOperator(ProgramStateRef State, const MemRegion *Reg);
 bool frontModifiable(ProgramStateRef State, const MemRegion *Reg);
 bool backModifiable(ProgramStateRef State, const MemRegion *Reg);
-BinaryOperator::Opcode getOpcode(const SymExpr *SE);
-const RegionOrSymbol getRegionOrSymbol(const SVal &Val);
-const ProgramStateRef processComparison(ProgramStateRef State,
-RegionOrSymbol LVal,
-RegionOrSymbol RVal, bool Equal);
-const ProgramStateRef saveComparison(ProgramStateRef State,
- const S

[PATCH] D60995: [clangd] Make sure include path does not contain any backslashes on Windows

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks like a bug rather than a feature of HeaderSearch, shouldn't the 
slash conversion go there?

(It seems unlikely that diagnostics should suggest one thing but we insert 
another)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995



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

[PATCH] D53701: [Analyzer] Instead of recording comparisons in interator checkers do an eager state split

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358951: [Analyzer] Instead of recording comparisons in 
interator checkers do an eager… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53701?vs=192593&id=196190#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D53701

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -133,8 +133,6 @@
   }
 };
 
-typedef llvm::PointerUnion RegionOrSymbol;
-
 // Structure to record the symbolic begin and end position of a container
 struct ContainerData {
 private:
@@ -172,41 +170,21 @@
   }
 };
 
-// Structure fo recording iterator comparisons. We needed to retrieve the
-// original comparison expression in assumptions.
-struct IteratorComparison {
-private:
-  RegionOrSymbol Left, Right;
-  bool Equality;
-
-public:
-  IteratorComparison(RegionOrSymbol L, RegionOrSymbol R, bool Eq)
-  : Left(L), Right(R), Equality(Eq) {}
-
-  RegionOrSymbol getLeft() const { return Left; }
-  RegionOrSymbol getRight() const { return Right; }
-  bool isEquality() const { return Equality; }
-  bool operator==(const IteratorComparison &X) const {
-return Left == X.Left && Right == X.Right && Equality == X.Equality;
-  }
-  bool operator!=(const IteratorComparison &X) const {
-return Left != X.Left || Right != X.Right || Equality != X.Equality;
-  }
-  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Equality); }
-};
-
 class IteratorChecker
 : public Checker, check::Bind,
- check::LiveSymbols, check::DeadSymbols,
- eval::Assume> {
+ check::LiveSymbols, check::DeadSymbols> {
 
   std::unique_ptr OutOfRangeBugType;
   std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
 
-  void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal,
-const SVal &RVal, OverloadedOperatorKind Op) const;
+  void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+const SVal &LVal, const SVal &RVal,
+OverloadedOperatorKind Op) const;
+  void processComparison(CheckerContext &C, ProgramStateRef State,
+ SymbolRef Sym1, SymbolRef Sym2, const SVal &RetVal,
+ OverloadedOperatorKind Op) const;
   void verifyAccess(CheckerContext &C, const SVal &Val) const;
   void verifyDereference(CheckerContext &C, const SVal &Val) const;
   void handleIncrement(CheckerContext &C, const SVal &RetVal, const SVal &Iter,
@@ -281,8 +259,6 @@
  CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
-  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
- bool Assumption) const;
 };
 } // namespace
 
@@ -292,9 +268,6 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ContainerMap, const MemRegion *, ContainerData)
 
-REGISTER_MAP_WITH_PROGRAMSTATE(IteratorComparisonMap, const SymExpr *,
-   IteratorComparison)
-
 namespace {
 
 bool isIteratorType(const QualType &Type);
@@ -324,16 +297,6 @@
 bool hasSubscriptOperator(ProgramStateRef State, const MemRegion *Reg);
 bool frontModifiable(ProgramStateRef State, const MemRegion *Reg);
 bool backModifiable(ProgramStateRef State, const MemRegion *Reg);
-BinaryOperator::Opcode getOpcode(const SymExpr *SE);
-const RegionOrSymbol getRegionOrSymbol(const SVal &Val);
-const ProgramStateRef processComparison(ProgramStateRef State,
-RegionOrSymbol LVal,
-RegionOrSymbol RVal, bool Equal);
-const ProgramStateRef saveComparison(ProgramStateRef State,
- const SymExpr *Condition, const SVal &LVal,
- const SVal &RVal, bool Eq);
-const IteratorComparison *loadComparison(ProgramStateRef State,
- const SymExpr *Condition);
 SymbolRef getContainerBegin(ProgramStateRef State, const MemRegion *Cont);
 SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont);
 ProgramStateRef createContainerBegin(ProgramStateRef State,
@@ -343,21 +306,11 @@
const SymbolRef Sym);
 const IteratorPosition *getIteratorPosition(ProgramStateRef State,
 const SVal &Val);
-const IteratorPosition *getIteratorPosition(ProgramStateRef State,
-RegionOrSymbol RegOrSym);
 Progr

[PATCH] D60990: [Driver] Support priority for multilibs

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Can this have test coverage?




Comment at: clang/lib/Driver/Multilib.cpp:22
 #include 
+#include 
 #include 

Doesn't seem to be used?



Comment at: clang/lib/Driver/Multilib.cpp:271
+  // Sort multilibs by priority and select the one with the highest priority.
+  std::sort(Filtered.begin(), Filtered.end(),
+[](const Multilib &a, const Multilib &b) -> bool {

`llvm::sort`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60990



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

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes PR 36119 .


Repository:
  rC Clang

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\r\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\r\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/W

[PATCH] D60995: [clangd] Make sure include path does not contain any backslashes on Windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I had just looked at the method name and thought it was the right place( 
`suggestPathToFileForDiagnostics` since it says "path to file".)

But looking at the comments,

  /// Suggest a path by which the specified file could be found, for
  /// use in diagnostics to suggest a #include.

HeaderSearch API itself seems like a better place, I suppose the confusion 
comes from the fact that paths and the include-directives are
the same on unix-like systems.

Updating the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995



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

r358953 - [PowerPC] Fix test with -fno-discard-value-names after rC358949

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Apr 23 00:39:23 2019
New Revision: 358953

URL: http://llvm.org/viewvc/llvm-project?rev=358953&view=rev
Log:
[PowerPC] Fix test with -fno-discard-value-names after rC358949

For the clang driver, -DLLVM_ENABLE_ASSERTIONS=off builds default to discard 
value names.

Modified:
cfe/trunk/test/CodeGen/ppc-mmintrin.c

Modified: cfe/trunk/test/CodeGen/ppc-mmintrin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ppc-mmintrin.c?rev=358953&r1=358952&r2=358953&view=diff
==
--- cfe/trunk/test/CodeGen/ppc-mmintrin.c (original)
+++ cfe/trunk/test/CodeGen/ppc-mmintrin.c Tue Apr 23 00:39:23 2019
@@ -1,9 +1,9 @@
 // REQUIRES: powerpc-registered-target
 
-// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target 
powerpc64-gnu-linux %s \
-// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s 
--check-prefixes=CHECK,CHECK-BE
-// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target 
powerpc64le-gnu-linux %s \
-// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s 
--check-prefixes=CHECK,CHECK-LE
+// RUN: %clang -S -emit-llvm -target powerpc64-gnu-linux -mcpu=pwr8 
-DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: %clang -S -emit-llvm -target powerpc64le-gnu-linux -mcpu=pwr8 
-DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 #include 
 


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

r358955 - [Analyzer] Fix for previous commit

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Apr 23 00:45:10 2019
New Revision: 358955

URL: http://llvm.org/viewvc/llvm-project?rev=358955&view=rev
Log:
[Analyzer] Fix for previous commit

A compilation warning was in my previous commit which broke the buildbot
because it is using `-Werror` for compilation. This patch fixes this
issue.


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=358955&r1=358954&r2=358955&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Apr 23 
00:45:10 2019
@@ -848,9 +848,9 @@ void IteratorChecker::processComparison(
 SymbolRef Sym2, const SVal &RetVal,
 OverloadedOperatorKind Op) const {
   if (const auto TruthVal = RetVal.getAs()) {
-if (State = relateSymbols(State, Sym1, Sym2,
+if ((State = relateSymbols(State, Sym1, Sym2,
   (Op == OO_EqualEqual) ==
-  (TruthVal->getValue() != 0))) {
+   (TruthVal->getValue() != 0 {
   C.addTransition(State);
 } else {
   C.generateSink(State, C.getPredecessor());


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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

You use `%clang` (clang driver) because the test needs `-internal-isystem 
$resource_dir/include/ppc_wrappers`. A default release build of llvm sets 
`LLVM_ENABLE_ASSERTIONS` to off, the clang driver defaults to 
`-fdiscard-value-names`. This caused the test to break. I have fixed that in 
rC358953 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D60997: Fix unquoted spaces in args in clang --verbose output

[Brad Moody via Phabricator via cfe-commits]

bmoody created this revision.
bmoody added a reviewer: hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The behaviour of not quoting spaces appears to have been introduced by mistake 
in revision b212b34f19472469c1d20ada3161c3523268d5be 



Repository:
  rC Clang

https://reviews.llvm.org/D60997

Files:
  clang/lib/Driver/Job.cpp
  clang/test/Driver/verbose-output-quoting.c


Index: clang/test/Driver/verbose-output-quoting.c
===
--- /dev/null
+++ clang/test/Driver/verbose-output-quoting.c
@@ -0,0 +1,9 @@
+// RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck 
-check-prefix=SPACE -strict-whitespace %s
+// RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck 
-check-prefix=QUOTES-strict-whitespace %s
+// RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck 
-check-prefix=BACKSLASH -strict-whitespace %s
+// RUN: %clang --verbose -DDOLLAR=\$-c %s 2>&1 | FileCheck 
-check-prefix=DOLLAR-strict-whitespace %s
+
+// SPACE: -cc1 {{.*}} -D "SPACE=a b"
+// QUOTES: -cc1 {{.*}} -D "QUOTES=\"\""
+// BACKSLASH: -cc1 {{.*}} -D "BACKSLASH=\\"
+// DOLLAR: -cc1 {{.*}} -D "DOLLAR=\$"
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -99,7 +99,7 @@
 }
 
 void Command::printArg(raw_ostream &OS, StringRef Arg, bool Quote) {
-  const bool Escape = Arg.find_first_of("\"\\$") != StringRef::npos;
+  const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
   if (!Quote && !Escape) {
 OS << Arg;


Index: clang/test/Driver/verbose-output-quoting.c
===
--- /dev/null
+++ clang/test/Driver/verbose-output-quoting.c
@@ -0,0 +1,9 @@
+// RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck -check-prefix=SPACE -strict-whitespace %s
+// RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck -check-prefix=QUOTES-strict-whitespace %s
+// RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck -check-prefix=BACKSLASH -strict-whitespace %s
+// RUN: %clang --verbose -DDOLLAR=\$-c %s 2>&1 | FileCheck -check-prefix=DOLLAR-strict-whitespace %s
+
+// SPACE: -cc1 {{.*}} -D "SPACE=a b"
+// QUOTES: -cc1 {{.*}} -D "QUOTES=\"\""
+// BACKSLASH: -cc1 {{.*}} -D "BACKSLASH=\\"
+// DOLLAR: -cc1 {{.*}} -D "DOLLAR=\$"
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -99,7 +99,7 @@
 }
 
 void Command::printArg(raw_ostream &OS, StringRef Arg, bool Quote) {
-  const bool Escape = Arg.find_first_of("\"\\$") != StringRef::npos;
+  const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
   if (!Quote && !Escape) {
 OS << Arg;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60995: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Fix bug in HeaderSearch instead of clangd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[MyDeveloperDay via Phabricator via cfe-commits]

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

This looks logical to me, seems to fit with what 
``WhitespaceManager::appendNewlineText`` is doing

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D60996



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

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

[Tianle Liu via Phabricator via cfe-commits]

liutianle updated this revision to Diff 196195.

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

https://reviews.llvm.org/D60552

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/avx512bf16intrin.h
  lib/Headers/avx512vlbf16intrin.h
  lib/Headers/cpuid.h
  lib/Headers/immintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/avx512bf16-builtins.c
  test/CodeGen/avx512vlbf16-builtins.c

Index: test/CodeGen/avx512vlbf16-builtins.c
===
--- /dev/null
+++ test/CodeGen/avx512vlbf16-builtins.c
@@ -0,0 +1,163 @@
+//  RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin \
+//  RUN:-target-feature +avx512bf16 -target-feature \
+//  RUN:+avx512vl -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+__m128bh test_mm_cvtne2ps2bf16(__m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_cvtne2ps_pbh(A, B);
+}
+
+__m128bh test_mm_maskz_cvtne2ps2bf16(__m128 A, __m128 B, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m128bh test_mm_mask_cvtne2ps2bf16(__m128bh C, __mmask8 U, __m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m256bh test_mm256_cvtne2ps2bf16(__m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: ret <16 x i16> %{{.*}}
+  return _mm256_cvtne2ps_pbh(A, B);
+}
+
+__m256bh test_mm256_maskz_cvtne2ps2bf16(__m256 A, __m256 B, __mmask16 U) {
+  // CHECK-LABEL: @test_mm256_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  // CHECK: ret <16 x i16> %{{.*}}
+  return _mm256_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m256bh test_mm256_mask_cvtne2ps2bf16(__m256bh C, __mmask16 U, __m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  // CHECK: ret <16 x i16> %{{.*}}
+  return _mm256_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m512bh test_mm512_cvtne2ps2bf16(__m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: ret <32 x i16> %{{.*}}
+  return _mm512_cvtne2ps_pbh(A, B);
+}
+
+__m512bh test_mm512_maskz_cvtne2ps2bf16(__m512 A, __m512 B, __mmask32 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  // CHECK: ret <32 x i16> %{{.*}}
+  return _mm512_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m512bh test_mm512_mask_cvtne2ps2bf16(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  // CHECK: ret <32 x i16> %{{.*}}
+  return _mm512_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m128bh test_mm_cvtneps2bf16(__m128 A) {
+  // CHECK-LABEL: @test_mm_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_cvtneps_pbh(A);
+}
+
+__m128bh test_mm_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m128 A) {
+  // CHECK-LABEL: @test_mm_mask_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_mask_cvtneps_pbh(C, U, A);
+}
+
+__m128bh test_mm_maskz_cvtneps2bf16(__m128 A, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm_maskz_cvtneps_pbh(U, A);
+}
+
+__m128bh test_mm256_cvtneps2bf16(__m256 A) {
+  // CHECK-LABEL: @test_mm256_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.256
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm256_cvtneps_pbh(A);
+}
+
+__m128bh test_mm256_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m256 A) {
+  // CHECK-LABEL: @test_mm256_mask_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.256
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  // CHECK: ret <8 x i16> %{{.*}}
+  return _mm256_mask_cv

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

[Tianle Liu via Phabricator via cfe-commits]

liutianle added a comment.

@RKSimon I add doxygen based descriptions in header files and return type check 
in test files. Please review again.


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

https://reviews.llvm.org/D60552



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

friendly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60760



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

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 196197.
owenpan added a comment.

Removes a no longer needed hack that worked around this bug when computing 
`StartOfLine` in the `BreakableBlockComment::adjustWhitespace` function.

Also updates the test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -472,7 +473,7 @@
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+

[PATCH] D53701: [Analyzer] Instead of recording comparisons in interator checkers do an eager state split

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:825
+  // not then create a new symbol for the offset.
+  SymbolRef Sym;
+  if (!LPos || !RPos) {

This is an uninitialized version of `Sym` that will be used on line 835 and 
line 839.
The `Sym` variable assigned on line 828 is local to the scope starting at line 
826.

Not really sure, but was perhaps the idea to use the `Sym` value from line 828 
on line 835 and 839.
Then I guess the you can rewrite this as:


```
  // At least one of the iterators have recorded positions. If one of them has
  // not then create a new symbol for the offset.
  if (!LPos || !RPos) {
auto &SymMgr = C.getSymbolManager();
auto Sym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
C.getASTContext().LongTy, C.blockCount());
State = assumeNoOverflow(State, Sym, 4);

if (!LPos) {
  State = setIteratorPosition(State, LVal,
  IteratorPosition::getPosition(Cont, Sym));
  LPos = getIteratorPosition(State, LVal);
} else if (!RPos) {
  State = setIteratorPosition(State, RVal,
  IteratorPosition::getPosition(Cont, Sym));
  RPos = getIteratorPosition(State, RVal);
}
  }
```

As it is right now I get complaint about using an uninitialized value for `Sym` 
(so this patch still doesn't build with -Werror after the earlier fixup).


Repository:
  rC Clang

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

https://reviews.llvm.org/D53701



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

[PATCH] D60997: Fix unquoted spaces in args in clang --verbose output

[Hans Wennborg via Phabricator via cfe-commits]

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

Thanks for fixing!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60997



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Leaving a summary of the offline discussion here.

@sammccall has pointed out maintaining and shipping more tools is work and we 
should probably avoid adding them upstream without careful design.
We ended up deciding to **not** land this particular upstream just yet, but 
anyone is free to patch this change on top of their LLVM repo in case they need 
something like this for debugging purposes.

Obviously no guarantees that this won't break.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG with a comment.




Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:110-111
+  Lexer::getSourceText(TextRange, Sources, getLangOpts())
+  .rtrim('{') // Drop the { itself.
+  .rtrim();   // Drop any whitespace before it.
   bool IsNested = NestedNamespaceName.contains(':');

This all seems rather hacky. I believe, this will fail miserably when there are 
comments before the `{`. Can you leave a FIXME to rewrite this logic in terms 
of tokens instead?


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

https://reviews.llvm.org/D59977



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

r358968 - [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via cfe-commits]

Author: r.stahl
Date: Tue Apr 23 04:04:41 2019
New Revision: 358968

URL: http://llvm.org/viewvc/llvm-project?rev=358968&view=rev
Log:
[analyzer][CrossTU] Extend CTU to VarDecls with initializer

Summary:
The existing CTU mechanism imports `FunctionDecl`s where the definition is 
available in another TU. This patch extends that to VarDecls, to bind more 
constants.

- Add VarDecl importing functionality to CrossTranslationUnitContext
- Import Decls while traversing them in AnalysisConsumer
- Add VarDecls to CTU external mappings generator
- Name changes from "external function map" to "external definition map"

Reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov, martong

Reviewed By: xazax.hun

Subscribers: Charusso, baloghadamsoftware, mikhail.ramalho, Szelethus, 
donat.nagy, dkrupp, george.karpenkov, mgorny, whisperity, szepet, rnkovacs, 
a.sidorin, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Analysis/redecl.c
Modified:
cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
cfe/trunk/test/Analysis/ctu-main.cpp
cfe/trunk/test/Analysis/func-mapping-test.cpp
cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Modified: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h?rev=358968&r1=358967&r2=358968&view=diff
==
--- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h (original)
+++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h Tue Apr 23 04:04:41 
2019
@@ -28,6 +28,7 @@ class ASTImporter;
 class ASTUnit;
 class DeclContext;
 class FunctionDecl;
+class VarDecl;
 class NamedDecl;
 class TranslationUnitDecl;
 
@@ -87,6 +88,9 @@ parseCrossTUIndex(StringRef IndexPath, S
 
 std::string createCrossTUIndexString(const llvm::StringMap 
&Index);
 
+// Returns true if the variable or any field of a record variable is const.
+bool containsConst(const VarDecl *VD, const ASTContext &ACtx);
+
 /// This class is used for tools that requires cross translation
 ///unit capability.
 ///
@@ -102,16 +106,16 @@ public:
   CrossTranslationUnitContext(CompilerInstance &CI);
   ~CrossTranslationUnitContext();
 
-  /// This function loads a function definition from an external AST
-  ///file and merge it into the original AST.
+  /// This function loads a function or variable definition from an
+  ///external AST file and merges it into the original AST.
   ///
-  /// This method should only be used on functions that have no definitions in
+  /// This method should only be used on functions that have no definitions or
+  /// variables that have no initializer in
   /// the current translation unit. A function definition with the same
   /// declaration will be looked up in the index file which should be in the
   /// \p CrossTUDir directory, called \p IndexName. In case the declaration is
   /// found in the index the corresponding AST file will be loaded and the
-  /// definition of the function will be merged into the original AST using
-  /// the AST Importer.
+  /// definition will be merged into the original AST using the AST Importer.
   ///
   /// \return The declaration with the definition will be returned.
   /// If no suitable definition is found in the index file or multiple
@@ -121,17 +125,19 @@ public:
   llvm::Expected
   getCrossTUDefinition(const FunctionDecl *FD, StringRef CrossTUDir,
StringRef IndexName, bool DisplayCTUProgress = false);
+  llvm::Expected
+  getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir,
+   StringRef IndexName, bool DisplayCTUProgress = false);
 
-  /// This function loads a function definition from an external AST
-  ///file.
+  /// This function loads a definition from an external AST file.
   ///
-  /// A function definition with the same declaration will be looked up in the
+  /// A definition with the same declaration will be looked up in the
   /// index file which should be in the \p CrossTUDir directory, called
   /// \p IndexName. In case the declaration is found in the index the
   /// corresponding AST file will be loaded.
   ///
   /// \return Returns a pointer to the ASTUnit that contains the definition of
-  /// the looked up function or an Error.
+  /// the looked up name or an Error.
   /// The returned pointer is never a nullptr.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
@@ -146,8 +152,9 @@ public:
   ///
   /// \return Returns the resulting definition or an error.
   llvm::Expected importDefinition(con

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358968: [analyzer][CrossTU] Extend CTU to VarDecls with 
initializer (authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46421?vs=196075&id=196207#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/Analysis/redecl.c
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -158,6 +158,27 @@
   return Result.str();
 }
 
+bool containsConst(const VarDecl *VD, const ASTContext &ACtx) {
+  CanQualType CT = ACtx.getCanonicalType(VD->getType());
+  if (!CT.isConstQualified()) {
+const RecordType *RTy = CT->getAs();
+if (!RTy || !RTy->hasConstFields())
+  return false;
+  }
+  return true;
+}
+
+static bool hasBodyOrInit(const FunctionDecl *D, const FunctionDecl *&DefD) {
+  return D->hasBody(DefD);
+}
+static bool hasBodyOrInit(const VarDecl *D, const VarDecl *&DefD) {
+  return D->getAnyInitializer(DefD);
+}
+template  static bool hasBodyOrInit(const T *D) {
+  const T *Unused;
+  return hasBodyOrInit(D, Unused);
+}
+
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
 : CI(CI), Context(CI.getASTContext()) {}
 
@@ -165,48 +186,50 @@
 
 std::string CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
   SmallString<128> DeclUSR;
-  bool Ret = index::generateUSRForDecl(ND, DeclUSR); (void)Ret;
+  bool Ret = index::generateUSRForDecl(ND, DeclUSR);
+  (void)Ret;
   assert(!Ret && "Unable to generate USR");
   return DeclUSR.str();
 }
 
-/// Recursively visits the function decls of a DeclContext, and looks up a
-/// function based on USRs.
-const FunctionDecl *
-CrossTranslationUnitContext::findFunctionInDeclContext(const DeclContext *DC,
-   StringRef LookupFnName) {
+/// Recursively visits the decls of a DeclContext, and returns one with the
+/// given USR.
+template 
+const T *
+CrossTranslationUnitContext::findDefInDeclContext(const DeclContext *DC,
+  StringRef LookupName) {
   assert(DC && "Declaration Context must not be null");
   for (const Decl *D : DC->decls()) {
 const auto *SubDC = dyn_cast(D);
 if (SubDC)
-  if (const auto *FD = findFunctionInDeclContext(SubDC, LookupFnName))
-return FD;
+  if (const auto *ND = findDefInDeclContext(SubDC, LookupName))
+return ND;
 
-const auto *ND = dyn_cast(D);
-const FunctionDecl *ResultDecl;
-if (!ND || !ND->hasBody(ResultDecl))
+const auto *ND = dyn_cast(D);
+const T *ResultDecl;
+if (!ND || !hasBodyOrInit(ND, ResultDecl))
   continue;
-if (getLookupName(ResultDecl) != LookupFnName)
+if (getLookupName(ResultDecl) != LookupName)
   continue;
 return ResultDecl;
   }
   return nullptr;
 }
 
-llvm::Expected
-CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
-  StringRef CrossTUDir,
-  StringRef IndexName,
-  bool DisplayCTUProgress) {
-  assert(FD && "FD is missing, bad call to this function!");
-  assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+template 
+llvm::Expected CrossTranslationUnitContext::getCrossTUDefinitionImpl(
+const T *D, StringRef CrossTUDir, StringRef IndexName,
+bool DisplayCTUProgress) {
+  assert(D && "D is missing, bad call to this function!");
+  assert(!hasBodyOrInit(D) &&
+ "D has a body or init in current translation unit!");
   ++NumGetCTUCalled;
-  const std::string LookupFnName = getLookupName(FD);
-  if (LookupFnName.empty())
+  const std::string LookupName = getLookupName(D);
+  if (LookupName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
   llvm::Expected ASTUnitOrError =
-  loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
+  loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -262,12 +285,29 @@
   }
 
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
-  if (const FunctionDecl *ResultDecl =
-  findFunctionInDeclContext(TU, Lo

r358971 - [Analyzer] Second fix for last commit for IteratorChecker

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Tue Apr 23 04:18:50 2019
New Revision: 358971

URL: http://llvm.org/viewvc/llvm-project?rev=358971&view=rev
Log:
[Analyzer] Second fix for last commit for IteratorChecker

A variable was redeclared instead of assigned in an internal
block, leaving the original uninitialized. This is fixed now.


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=358971&r1=358970&r2=358971&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Tue Apr 23 
04:18:50 2019
@@ -825,7 +825,7 @@ void IteratorChecker::handleComparison(C
   SymbolRef Sym;
   if (!LPos || !RPos) {
 auto &SymMgr = C.getSymbolManager();
-auto Sym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
+Sym = SymMgr.conjureSymbol(CE, C.getLocationContext(),
C.getASTContext().LongTy, C.blockCount());
 State = assumeNoOverflow(State, Sym, 4);
   }


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

Re: r358665 - [clang][CIndex] Use llvm::set_thread_priority

[Kadir Çetinkaya via cfe-commits]

Thanks for the fix Nico!

On Sun, Apr 21, 2019 at 9:17 PM Nico Weber  wrote:

> r358858 might help with this.
>
> On Sat, Apr 20, 2019 at 7:15 PM Nico Weber  wrote:
>
>> This breaks building with LLVM_ENABLE_THREADS=OFF. The call probably
>> needs to be behind a `#if LLVM_ENABLE_THREADS`.
>>
>> FAILED: bin/c-index-test
>> ...
>> lib/libclang.a(CIndex.cpp.o): In function `void llvm::function_ref> ()>::callback_fn(long)':
>> CIndex.cpp:(.text._ZN4llvm12function_refIFvvEE11callback_fnIZ25clang_saveTranslationUnitEUlvE_EEvl+0x5a):
>> undefined reference to `llvm::set_thread_priority(llvm::ThreadPriority)'
>> lib/libclang.a(CIndex.cpp.o): In function
>> `clang::setThreadBackgroundPriority()':
>> CIndex.cpp:(.text._ZN5clang27setThreadBackgroundPriorityEv+0x27):
>> undefined reference to `llvm::set_thread_priority(llvm::ThreadPriority)'
>> lib/libclang.a(CIndex.cpp.o): In function `clang_saveTranslationUnit':
>> CIndex.cpp:(.text.clang_saveTranslationUnit+0x316): undefined reference
>> to `llvm::set_thread_priority(llvm::ThreadPriority)'
>>
>>
>> On Thu, Apr 18, 2019 at 9:47 AM Kadir Cetinkaya via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: kadircet
>>> Date: Thu Apr 18 06:49:20 2019
>>> New Revision: 358665
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=358665&view=rev
>>> Log:
>>> [clang][CIndex] Use llvm::set_thread_priority
>>>
>>> Reviewers: jkorous, gribozavr
>>>
>>> Subscribers: dexonsmith, arphaman, cfe-commits
>>>
>>> Tags: #clang
>>>
>>> Differential Revision: https://reviews.llvm.org/D60867
>>>
>>> Modified:
>>> cfe/trunk/tools/libclang/CIndex.cpp
>>>
>>> Modified: cfe/trunk/tools/libclang/CIndex.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=358665&r1=358664&r2=358665&view=diff
>>>
>>> ==
>>> --- cfe/trunk/tools/libclang/CIndex.cpp (original)
>>> +++ cfe/trunk/tools/libclang/CIndex.cpp Thu Apr 18 06:49:20 2019
>>> @@ -8723,9 +8723,7 @@ void clang::setThreadBackgroundPriority(
>>>if (getenv("LIBCLANG_BGPRIO_DISABLE"))
>>>  return;
>>>
>>> -#ifdef USE_DARWIN_THREADS
>>> -  setpriority(PRIO_DARWIN_THREAD, 0, PRIO_DARWIN_BG);
>>> -#endif
>>> +  llvm::set_thread_priority(llvm::ThreadPriority::Background);
>>>  }
>>>
>>>  void cxindex::printDiagsToStderr(ASTUnit *Unit) {
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D60990: [Driver] Support priority for multilibs

[Jon Roelofs via cfe-commits]

LGTM

On Mon, Apr 22, 2019 at 8:34 PM Petr Hosek via Phabricator <
revi...@reviews.llvm.org> wrote:

> phosek created this revision.
> phosek added reviewers: jroelofs, bkramer.
> Herald added subscribers: cfe-commits, mgrang.
> Herald added a project: clang.
>
> When more than one multilib flag matches, try to select the best
> possible match based on priority. When two different multilibs with
> the same same priority match, we still throw an error matching the
> existing behavior.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D60990
>
> Files:
>   clang/include/clang/Driver/Multilib.h
>   clang/lib/Driver/Multilib.cpp
>
>
> Index: clang/lib/Driver/Multilib.cpp
> ===
> --- clang/lib/Driver/Multilib.cpp
> +++ clang/lib/Driver/Multilib.cpp
> @@ -19,6 +19,7 @@
>  #include "llvm/Support/raw_ostream.h"
>  #include 
>  #include 
> +#include 
>  #include 
>
>  using namespace clang;
> @@ -51,8 +52,9 @@
>  }
>
>  Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
> -   StringRef IncludeSuffix)
> -: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix),
> IncludeSuffix(IncludeSuffix) {
> +   StringRef IncludeSuffix, int Priority)
> +: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix),
> IncludeSuffix(IncludeSuffix),
> +  Priority(Priority) {
>normalizePathSegment(this->GCCSuffix);
>normalizePathSegment(this->OSSuffix);
>normalizePathSegment(this->IncludeSuffix);
> @@ -265,8 +267,19 @@
>  return true;
>}
>
> -  // TODO: pick the "best" multlib when more than one is suitable
> -  assert(false);
> +  // Sort multilibs by priority and select the one with the highest
> priority.
> +  std::sort(Filtered.begin(), Filtered.end(),
> +[](const Multilib &a, const Multilib &b) -> bool {
> +  return a.priority() > b.priority();
> +});
> +
> +  if (Filtered[0].priority() <= Filtered[1].priority()) {
> +M = Filtered[0];
> +return true;
> +  }
> +
> +  // TODO: We should consider returning llvm::Error rather than aborting.
> +  assert(false && "More than one multilib with the same priority");
>return false;
>  }
>
> Index: clang/include/clang/Driver/Multilib.h
> ===
> --- clang/include/clang/Driver/Multilib.h
> +++ clang/include/clang/Driver/Multilib.h
> @@ -34,10 +34,11 @@
>std::string OSSuffix;
>std::string IncludeSuffix;
>flags_list Flags;
> +  int Priority;
>
>  public:
>Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
> -   StringRef IncludeSuffix = {});
> +   StringRef IncludeSuffix = {}, int Priority = 0);
>
>/// Get the detected GCC installation path suffix for the multi-arch
>/// target variant. Always starts with a '/', unless empty
> @@ -77,6 +78,9 @@
>const flags_list &flags() const { return Flags; }
>flags_list &flags() { return Flags; }
>
> +  /// Returns the multilib priority.
> +  int priority() const { return Priority; }
> +
>/// Add a flag to the flags list
>/// \p Flag must be a flag accepted by the driver with its leading '-'
> removed,
>/// and replaced with either:
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358949 - [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Qiu Chaofan via cfe-commits]

Author: chaofan
Date: Mon Apr 22 22:50:24 2019
New Revision: 358949

URL: http://llvm.org/viewvc/llvm-project?rev=358949&view=rev
Log:
[PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

Port mmintrin.h which include x86 MMX intrinsics implementation to PowerPC 
platform (using Altivec).

To make the include process correct, PowerPC's toolchain class is overrided to 
insert new headers directory (named ppc_wrappers) into the path. Basic test 
cases for several intrinsic functions are added.

The header is mainly developed by Steven Munroe, with contributions from Paul 
Clarke, Bill Schmidt, Jinsong Ji and Zixuan Wu.

Reviewed By: Jinsong Ji

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

Added:
cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp
cfe/trunk/lib/Driver/ToolChains/PPCLinux.h
cfe/trunk/lib/Headers/ppc_wrappers/
cfe/trunk/lib/Headers/ppc_wrappers/mmintrin.h
cfe/trunk/test/CodeGen/ppc-mmintrin.c
cfe/trunk/test/Headers/ppc-intrinsics.c
Modified:
cfe/trunk/lib/Driver/CMakeLists.txt
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Headers/CMakeLists.txt

Modified: cfe/trunk/lib/Driver/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=358949&r1=358948&r2=358949&view=diff
==
--- cfe/trunk/lib/Driver/CMakeLists.txt (original)
+++ cfe/trunk/lib/Driver/CMakeLists.txt Mon Apr 22 22:50:24 2019
@@ -65,6 +65,7 @@ add_clang_library(clangDriver
   ToolChains/TCE.cpp
   ToolChains/WebAssembly.cpp
   ToolChains/XCore.cpp
+  ToolChains/PPCLinux.cpp
   Types.cpp
   XRayArgs.cpp
 

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=358949&r1=358948&r2=358949&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Mon Apr 22 22:50:24 2019
@@ -38,6 +38,7 @@
 #include "ToolChains/NetBSD.h"
 #include "ToolChains/OpenBSD.h"
 #include "ToolChains/PS4CPU.h"
+#include "ToolChains/PPCLinux.h"
 #include "ToolChains/RISCVToolchain.h"
 #include "ToolChains/Solaris.h"
 #include "ToolChains/TCE.h"
@@ -4576,6 +4577,11 @@ const ToolChain &Driver::getToolChain(co
!Target.hasEnvironment())
 TC = llvm::make_unique(*this, Target,
   Args);
+  else if (Target.getArch() == llvm::Triple::ppc ||
+   Target.getArch() == llvm::Triple::ppc64 ||
+   Target.getArch() == llvm::Triple::ppc64le)
+TC = llvm::make_unique(*this, Target,
+  Args);
   else
 TC = llvm::make_unique(*this, Target, Args);
   break;

Added: cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp?rev=358949&view=auto
==
--- cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp (added)
+++ cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp Mon Apr 22 22:50:24 2019
@@ -0,0 +1,31 @@
+//===-- PPCLinux.cpp - PowerPC ToolChain Implementations *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PPCLinux.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver::toolchains;
+using namespace llvm::opt;
+
+void PPCLinuxToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+  ArgStringList &CC1Args) 
const {
+  // PPC wrapper headers are implementation of x86 intrinsics on PowerPC, which
+  // is not supported on PPC32 platform.
+  if (getArch() != llvm::Triple::ppc &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+const Driver &D = getDriver();
+SmallString<128> P(D.ResourceDir);
+llvm::sys::path::append(P, "include", "ppc_wrappers");
+addSystemInclude(DriverArgs, CC1Args, P);
+  }
+
+  Linux::AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+}

Added: cfe/trunk/lib/Driver/ToolChains/PPCLinux.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PPCLinux.h?rev=358949&view=auto
==
--- cfe/trunk/lib/Driver/ToolChains/PPCLinux.h (added)
+++ cfe/trunk/lib/Driver/ToolChains/PPCLinux.h Mon Apr 22 22:50:24 2019
@@ -0,0 +1,33 @@
+//===--- PPCLinux.h - PowerPC ToolChain Implementations -*- C++ 
-*-===//
+//
+

r358973 - Fix "-Wimplicit-fallthrough" warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Apr 23 04:45:28 2019
New Revision: 358973

URL: http://llvm.org/viewvc/llvm-project?rev=358973&view=rev
Log:
Fix "-Wimplicit-fallthrough" warning. NFCI.

Modified:
cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Modified: cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp?rev=358973&r1=358972&r2=358973&view=diff
==
--- cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp (original)
+++ cfe/trunk/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp Tue Apr 23 
04:45:28 2019
@@ -90,6 +90,7 @@ void MapExtDefNamesConsumer::addIfInMain
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
   Index[LookupName] = CurrentFileName;
+break;
   default:
 break;
   }


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

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

> Is there some kind of testcase?

@aprantl Usage of the option is tested within following patches from the stack. 
I am not sure if we need some additional test here?


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

https://reviews.llvm.org/D58033



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

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes broken tests when doing an out-of-source build that picks up a 
random .clang-format on the file system due to the default "file" style.


Repository:
  rC Clang

https://reviews.llvm.org/D61001

Files:
  test/Format/adjust-indent.cpp
  test/Format/disable-include-sorting.cpp
  test/Format/language-detection.cpp
  test/Format/xmloutput.cpp


Index: test/Format/xmloutput.cpp
===
--- test/Format/xmloutput.cpp
+++ test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: test/Format/disable-include-sorting.cpp
===
--- test/Format/disable-include-sorting.cpp
+++ test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | 
FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s 
-check-prefix=NOT-SORTED
 
Index: test/Format/adjust-indent.cpp
===
--- test/Format/adjust-indent.cpp
+++ test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {


Index: test/Format/xmloutput.cpp
===
--- test/Format/xmloutput.cpp
+++ test/Format/xmloutput.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format -output-replacements-xml -sort-includes %s \
+// RUN: clang-format -style=LLVM -output-replacements-xml -sort-includes %s \
 // RUN:   | FileCheck -strict-whitespace %s
 
 // CHECK: >>= b;$}}
 // CHECK2: {{^a >> >= b;$}}
Index: test/Format/disable-include-sorting.cpp
===
--- test/Format/disable-include-sorting.cpp
+++ test/Format/disable-include-sorting.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-format %s | FileCheck %s
+// RUN: clang-format %s -style=LLVM | FileCheck %s
 // RUN: clang-format %s -sort-includes -style="{SortIncludes: false}" | FileCheck %s
 // RUN: clang-format %s -sort-includes=false | FileCheck %s -check-prefix=NOT-SORTED
 
Index: test/Format/adjust-indent.cpp
===
--- test/Format/adjust-indent.cpp
+++ test/Format/adjust-indent.cpp
@@ -1,4 +1,4 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=2:2 \
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -lines=2:2 \
 // RUN:   | FileCheck -strict-whitespace %s
 
 void  f() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.
Herald added a project: clang.

Missing break/fallthrough


Repository:
  rC Clang

https://reviews.llvm.org/D61002

Files:
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp


Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -90,6 +90,7 @@
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
   Index[LookupName] = CurrentFileName;
+break;
   default:
 break;
   }


Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -90,6 +90,7 @@
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
   Index[LookupName] = CurrentFileName;
+break;
   default:
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl abandoned this revision.
r.stahl added a comment.

Was already done: 
https://github.com/llvm-mirror/clang/commit/bacdda22396c39181aa0e641182e01a0b3cf43ea


Repository:
  rC Clang

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

https://reviews.llvm.org/D61002



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

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

Feel free to commit such trivial fixes without reviews. Alternatively, you 
could use LLVM_FALLTHROUGH, but I have no strong preference in this case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61002



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

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

@xazax.hun Yes, I planned to just commit. Set you as Subscriber not Reviewer in 
Arc. Was just a bit confused why it fails at first :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61002



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

May I ask why the mmintrin.h emulation layer must be added to the clang 
resource directory? Why can't it be provided as a standalone library like 
https://github.com/intel/ARM_NEON_2_x86_SSE ? Will you add SSE/AVX/AVX512 
emulation layer to the clang resource directory as follow-ups?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: sbenza.
Herald added a project: clang.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61005

Files:
  clang/lib/Tooling/Refactoring/Stencil.cpp


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


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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: lib/Headers/ppc_wrappers/mmintrin.h:3
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal

This is not Apache 2.



Comment at: lib/Headers/ppc_wrappers/mmintrin.h:24
+
+/* Implemented from the specification included in the Intel C++ Compiler
+   User Guide and Reference, version 9.0.  */

Is this considered a derivative work of the Intel C++ Compiler?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:39
+  if (MatchedCallExpr &&
+  MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) {
+diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")

Is the presence of the attribute actually important, or is it simply expected 
that the declarations will have the attribute and so this check is benign?



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:40
+  MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) {
+diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")
+<< MatchedCallExpr->getDirectCallee()->getNameAsString();

clang-tidy diagnostics are nongrammatical in the same way the compiler error 
messages are: don't capitalize the start of the sentence, no terminating 
punctuation, etc. I'd probably reword to `result from error function %0 is 
unused`



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:41
+diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")
+<< MatchedCallExpr->getDirectCallee()->getNameAsString();
+  }

You should pass the result from `getDirectCallee()` rather than converting to a 
string. The diagnostics engine can handle any `NamedDecl *` automatically, 
including proper quoting, etc.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:48
+diag(MatchedTransitiveCallExpr->getExprLoc(),
+ "Unused result from function %0, which returns an error value")
+<< MatchedTransitiveCallExpr->getDirectCallee()->getNameAsString();

Similar comments here. How about `result from function %0 is unused but 
represents an error value`?



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:49
+ "Unused result from function %0, which returns an error value")
+<< MatchedTransitiveCallExpr->getDirectCallee()->getNameAsString();
+  }

Similar issue here as above.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:51
+  }
+}
+

Is there a way for the user to explicitly disable this warning, or is there no 
such thing as a false positive as far as the kernel is concerned?

Should the diagnostic also produce a fix-it, or is there not a sufficiently 
common replacement pattern to be used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
Herald added a project: clang.

Changes the signature of the std::function to return an Expected
instead of std::string to allow for (non-fatal) failures.  Previously, we
expected that any failures would be expressed with assertions. However, that's
unfriendly to running the code in servers or other places that don't want their
library calls to crash the program.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61015

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


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -157,12 +157,16 @@
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
 if (auto Err = RangeOrErr.takeError())
   return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+auto &Range = *RangeOrErr;
+if (Range.isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range.getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())
+  return std::move(Err);
+Transformation T;
+T.Range = Range;
+T.Replacement = std::move(*ReplacementOrErr);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,18 @@
   Name,
 };
 
-using TextGenerator =
-std::function;
-
-/// Wraps a string as a TextGenerator.
+// \c TextGenerator may fail because it processes dynamically-bound match
+// results. For example, a typo in the name of a bound node can lead to a
+// failure in the string generation code. We prefer to return \c Expected 
rather
+// than assert on such failures to allow this code to be run in a variety of
+// settings (including servers).
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
+
+/// Wraps a string as a (trivially successful) TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+ -> Expected { return M; };
 }
 
 // Description of a source-code edit, expressed in terms of an AST node.


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -157,12 +157,16 @@
 Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
 if (auto Err = RangeOrErr.takeError())
   return std::move(Err);
-Transformation T;
-T.Range = *RangeOrErr;
-if (T.Range.isInvalid() ||
-isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+auto &Range = *RangeOrErr;
+if (Range.isInvalid() ||
+isOriginMacroBody(*Result.SourceManager, Range.getBegin()))
   return SmallVector();
-T.Replacement = Edit.Replacement(Result);
+auto ReplacementOrErr = Edit.Replacement(Result);
+if (auto Err = ReplacementOrErr.takeError())
+  return std::move(Err);
+Transformation T;
+T.Range = Range;
+T.Replacement = std::move(*ReplacementOrErr);
 Transformations.push_back(std::move(T));
   }
   return Transformations;
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,18 @@
   Name,
 };
 
-using TextGenerator =
-std::function;
-
-/// Wraps a string as a TextGenerator.
+// \c TextGenerator may fail because it processes dynamically-bound match
+// results. For example, a typo in the name of a bound node can lead to a
+// failure in the string generation code. We prefer to return \c Expected rather
+// than assert on such failures to allow this code to be run in a variety of
+// settings (including servers).
+using TextGenerator = std::function(
+const ast_matchers::MatchFinder::MatchResult &)>;
+
+/// Wraps a string as a (trivially successful) TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+ -> Expected { r

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Gabor Marton via Phabricator via cfe-commits]

martong resigned from this revision.
martong added a comment.

The changes in `ASTImporter.cpp` looks good to me!
And I have no competence about the rest of the change, thus I resign as a 
reviewer.


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

https://reviews.llvm.org/D56571



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3126
+auto Imp =
+importSeq(FromConstructor->getExplicitSpecifier().getPointer());
+if (!Imp)

Why is it needed to import the explicit specifier here again?
You already imported that above. Perhaps this hunk is not needed here at all?



Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:965
 auto *Conversion2 = cast(Method2);
-if (Conversion1->isExplicit() != Conversion2->isExplicit())
+if 
(!Conversion1->isEquivalentExplicit(Conversion2->getExplicitSpecifier()))
   return false;

Would it work if `Method1` has a different `ASTContext` than `Method2?
That is exactly the case when we import an AST into another with ASTImporter.


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

https://reviews.llvm.org/D60934



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

[PATCH] D43357: [Concepts] Function trailing requires clauses

[Gabor Marton via Phabricator via cfe-commits]

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

From the `ASTImporter` point of view it looks good, thanks for the changes!


Repository:
  rC Clang

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

https://reviews.llvm.org/D43357



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

In D59924#1475363 , @MaskRay wrote:

> May I ask why the mmintrin.h emulation layer must be added to the clang 
> resource directory? Why can't it be provided as a standalone library like 
> https://github.com/intel/ARM_NEON_2_x86_SSE ?


The intention is to lower the porting effort for users,  so that user don't 
need to modify their source or add their own additional headers wrapper layer.

> Will you add SSE/AVX/AVX512 emulation layer to the clang resource directory 
> as follow-ups?

Yes, more headers (eg: `xmmintrin.h`,  `emmintrin.h`) will follow.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ has(ignoringParenCasts(cxxThisExpr()));

Will this also match code like:
```
Frobble &Frobble::operator=(const Frobble &F) {
  if (&F == this->whatever())
return *this;
}
```
or, more insidiously: 
```
Frobble &Frobble::operator=(const Frobble &F) {
  if (&F == whatever()) // whatever is a member function of Frobble
return *this;
}
```




Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:54
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  hasAnyName("std::shared_ptr", "std::unique_ptr", "std::weak_ptr",
+ "std::auto_ptr"),

These should be using `::std::whatever` to ensure we only care about names in 
the global namespace named `std`.

Should this list be configurable? For instance, boost has a fair number of 
smart pointers.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:61
+  // pointer member, then trying to access the object pointed by the pointer, 
or
+  // memcpy overlapping arrays)
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(

Missing a full stop at the end of the comment.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:62-64
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+  has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+  hasType(arrayType(;

Hmm, while I understand why you're doing this, I'm worried that it will miss 
some pretty important cases. For instance, improper thread locking could result 
in deadlocks, improper releasing of non-memory resources could be problematic 
(such as network connections, file streams, etc), even simple integer 
assignments could be problematic in theory:
```
Yobble& Yobble::operator=(const Yobble &Y) {
  superSecretHashVal = 0; // Being secure!
  ... some code that may early return ...
  superSecretHashVal = Y.superSecretHashVal;
}
```
I wonder whether we want an option here to allow users to diagnose regardless 
of whether we think it's suspicious or not.

At the very least, this code should not be enabled for the CERT version of the 
check as it doesn't conform to the CERT requirements.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:82
+  diag(MatchedDecl->getLocation(),
+   "operator=() might not handle self-assignment properly");
+}

Hmm, the "might not" seems a bit flat to me. How about: `'operator=()' does not 
properly test for self-assignment`?

Also, do we want to have a fix-it to insert a check for self assignment at the 
start of the function?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:6
+
+`cert-oop54-cpp` redirects here as an alias for this check.
+

ztamas wrote:
> aaron.ballman wrote:
> > You should add a link to CERT's documentation somewhere around this text.
> In an earlier version of this patch, there was a link. I removed it because 
> of a reviewer comment. Now I add it back, I hope now are OK.
Thank you for adding the link!



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};

ztamas wrote:
> aaron.ballman wrote:
> > ztamas wrote:
> > > JonasToth wrote:
> > > > Please add tests with templated classes and self-assignment.
> > > I tested with template classes, but TemplateCopyAndSwap test case, for 
> > > example, was wrongly caught by the check. So I just changed the code to 
> > > ignore template classes. I did not find any template class related catch 
> > > either in LibreOffice or LLVM when I run the first version of this check, 
> > > so we don't lose too much with ignoring template classes, I think.
> > I am not keen on ignoring template classes for the check; that seems like a 
> > bug in the check that should be addressed. At a minimum, the test cases 
> > should be present with FIXME comments about the unwanted diagnostics.
> I don't think it's a good idea to change the check now to catch template 
> classes and produce false positives.
> 
> First of all the check does not work with template classes because the AST is 
> different. Check TemplateCopyAndSwap test case for example. It's expected 
> that the definition of operator= contains a CXXConstructExpr, but something 
> is different for templates. It might be a lower level problem, how t

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

[Gabor Marton via Phabricator via cfe-commits]

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

From the `ASTImporter` point of view it looks good to me, thanks for the 
changes!


Repository:
  rC Clang

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

https://reviews.llvm.org/D41910



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59802#1474300 , @hintonda wrote:

> @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, 
> and it seems to work fine.  However, it did miss this one:
>
> - if (V && isa(V) && (EntInst = cast(V)) && +   
>  if (isa_and_nonnull(V) && (EntInst = cast(V)) &&
>
>   It got the first, but not the second.  Not sure how to pick that one up.  
> Even ran it a second time on just that file, but still didn't pick it up.  
> Any ideas?


I don't think it's a critical case to cover for the check, but yeah, it looks 
like that code really wants to be `(EntInst = 
dyn_cast_or_null(V))`. I think that looking for a pattern to 
handle this case would be tricky though and given how infrequent it seems to 
show up in the code base, I am not too worried. Someone applying the check will 
still get a notice for the `Var && isa(Var)` part of the expression, so 
they can hopefully see there's more related cleanup to be done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1475596 , @aaron.ballman 
wrote:

> In D59802#1474300 , @hintonda wrote:
>
> > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, 
> > and it seems to work fine.  However, it did miss this one:
> >
> > - if (V && isa(V) && (EntInst = cast(V)) && + 
> >if (isa_and_nonnull(V) && (EntInst = cast(V)) 
> > &&
> >
> >   It got the first, but not the second.  Not sure how to pick that one up.  
> > Even ran it a second time on just that file, but still didn't pick it up.  
> > Any ideas?
>
>
> I don't think it's a critical case to cover for the check, but yeah, it looks 
> like that code really wants to be `(EntInst = 
> dyn_cast_or_null(V))`. I think that looking for a pattern to 
> handle this case would be tricky though and given how infrequent it seems to 
> show up in the code base, I am not too worried. Someone applying the check 
> will still get a notice for the `Var && isa(Var)` part of the 
> expression, so they can hopefully see there's more related cleanup to be done.


Okay, thanks for looking.  I'll go ahead and land this today, and if I think of 
a good way to handle this case, I'll update it later.

Thanks again for all your help...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Thanks @MaskRay for help fixing and review!




Comment at: lib/Headers/ppc_wrappers/mmintrin.h:3
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal

MaskRay wrote:
> This is not Apache 2.
Ah.. This was aligned with other headers .. but forgot to update to Apache 2.

@qiucf Please update it to Apache 2 as well. Thanks.



Comment at: lib/Headers/ppc_wrappers/mmintrin.h:24
+
+/* Implemented from the specification included in the Intel C++ Compiler
+   User Guide and Reference, version 9.0.  */

MaskRay wrote:
> Is this considered a derivative work of the Intel C++ Compiler?
My understanding is that the `User Guide and Reference ` is the `specification` 
, implementation based on public specification should NOT be considered as 
`derivative` work. 

Otherwise, all codes that referenced `User Guide and Reference ` need to be 
considered derivative work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10
+functions are marked with ``__attribute__((warn_unused_result))``, but
+the compiler warning for this attribute is not always enabled.
+

IIRC it is possible to pass through compiler warnings through ClangTidy... WDYT 
about that instead of reimplementing the warning?

However we would lose the ability to "infer" `warn_unused_result` on functions 
that return `ERR_PTR`.  However, since the analysis is not 
cross-translation-unit, IDK how much value there is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 4 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ has(ignoringParenCasts(cxxThisExpr()));

aaron.ballman wrote:
> Will this also match code like:
> ```
> Frobble &Frobble::operator=(const Frobble &F) {
>   if (&F == this->whatever())
> return *this;
> }
> ```
> or, more insidiously: 
> ```
> Frobble &Frobble::operator=(const Frobble &F) {
>   if (&F == whatever()) // whatever is a member function of Frobble
> return *this;
> }
> ```
> 
I'll check that case. The original hasLHS(...), hasRHS(...) might work with 
this use case too.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:54
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  hasAnyName("std::shared_ptr", "std::unique_ptr", "std::weak_ptr",
+ "std::auto_ptr"),

aaron.ballman wrote:
> These should be using `::std::whatever` to ensure we only care about names in 
> the global namespace named `std`.
> 
> Should this list be configurable? For instance, boost has a fair number of 
> smart pointers.
> Should this list be configurable? For instance, boost has a fair number of 
> smart pointers.

xazax.hun has the same suggestion. I think it's a good idea for a follow-up 
patch. I actually added a test case with the name CustomPtrField to document 
this future possibility.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:62-64
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+  has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+  hasType(arrayType(;

aaron.ballman wrote:
> Hmm, while I understand why you're doing this, I'm worried that it will miss 
> some pretty important cases. For instance, improper thread locking could 
> result in deadlocks, improper releasing of non-memory resources could be 
> problematic (such as network connections, file streams, etc), even simple 
> integer assignments could be problematic in theory:
> ```
> Yobble& Yobble::operator=(const Yobble &Y) {
>   superSecretHashVal = 0; // Being secure!
>   ... some code that may early return ...
>   superSecretHashVal = Y.superSecretHashVal;
> }
> ```
> I wonder whether we want an option here to allow users to diagnose regardless 
> of whether we think it's suspicious or not.
> 
> At the very least, this code should not be enabled for the CERT version of 
> the check as it doesn't conform to the CERT requirements.
It's starting to be too much for me.
First, the problem was false positives. If there are too many false positives 
then better to have it in the cert module.
Now your problem is that this is not fit with the CERT requirements, because of 
this matcher which reduces the false positives. Adding this check to the CERT 
module was not my idea in the first place. So I suggest to have it a simple 
bugprone check  (and remove the cert alias) and also we can mention that it is 
related to the corresponding cert rule (but it does not conform with it 
entirely).
This check allows the usage of copy-and-move pattern, which does not conform 
with the cert specification either where only the self-check and copy-and-swap 
is mentioned. So your next suggestion will be to not allow copy-and-move 
because it does not fit with the cert rule? So I think it's better to have this 
not a cert check then, but a bugprone check. I prefer to have a working check 
then implementing a theoretical documentation.

Apart from that cert thing, it actually seems a good idea to add a config 
option to allow the user to get all catches, not just the "suspicious ones".



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:82
+  diag(MatchedDecl->getLocation(),
+   "operator=() might not handle self-assignment properly");
+}

aaron.ballman wrote:
> Hmm, the "might not" seems a bit flat to me. How about: `'operator=()' does 
> not properly test for self-assignment`?
> 
> Also, do we want to have a fix-it to insert a check for self assignment at 
> the start of the function?
I don't think "test for self-assignment" will be good, because it's only one 
way to make the operator self assignment safe. In case of copy-and-swap and 
copy-and-move there is no testing of self assignment, but swaping/moving works 
in all cases without explicit self check.

A fix-it can be a good idea for a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: xur.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, eraman, inglorion, 
mehdi_amini.
Herald added projects: clang, LLVM.

The opt level was not being passed down to the ThinLTO backend when
invoked via clang (for distributed ThinLTO).

This exposed an issue where the new PM was asserting if the Thin or
regular LTO backend pipelines were invoked with -O0 (not a new issue,
could be provoked by invoking in-process *LTO backends via linker using
new PM and -O0). Fix this similar to the old PM where -O0 only does the
necessary lowering of type metadata (WPD and LowerTypeTest passes) and
then quits, rather than asserting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61022

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/thinlto-debug-pm.c
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/tools/gold/X86/opt-level.ll

Index: llvm/test/tools/gold/X86/opt-level.ll
===
--- llvm/test/tools/gold/X86/opt-level.ll
+++ llvm/test/tools/gold/X86/opt-level.ll
@@ -6,12 +6,25 @@
 ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \
 ; RUN:-m elf_x86_64 \
 ; RUN:-plugin-opt=O1 -r -o %t.o %t.bc
-; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 %s
+; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 --check-prefix=CHECK-O1-OLDPM %s
 ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \
 ; RUN:-m elf_x86_64 \
 ; RUN:-plugin-opt=O2 -r -o %t.o %t.bc
 ; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O2 %s
 
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \
+; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \
+; RUN:-plugin-opt=O0 -r -o %t.o %t.bc
+; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O0 %s
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \
+; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \
+; RUN:-plugin-opt=O1 -r -o %t.o %t.bc
+; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 --check-prefix=CHECK-O1-NEWPM %s
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \
+; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \
+; RUN:-plugin-opt=O2 -r -o %t.o %t.bc
+; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O2 %s
+
 ; CHECK-O0: define internal void @foo(
 ; CHECK-O1: define internal void @foo(
 ; CHECK-O2-NOT: define internal void @foo(
@@ -36,7 +49,9 @@
 
 end:
   ; CHECK-O0: phi
-  ; CHECK-O1: select
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi
   %r = phi i32 [ 1, %t ], [ 2, %f ]
   ret i32 %r
 }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -1046,10 +1046,16 @@
 //
 // Also, WPD has access to more precise information than ICP and can
 // devirtualize more effectively, so it should operate on the IR first.
+//
+// The WPD and LowerTypeTest passes need to run at -O0 to lower type
+// metadata and intrinsics.
 MPM.addPass(WholeProgramDevirtPass(nullptr, ImportSummary));
 MPM.addPass(LowerTypeTestsPass(nullptr, ImportSummary));
   }
 
+  if (Level == O0)
+return MPM;
+
   // Force any function attributes we want the rest of the pipeline to observe.
   MPM.addPass(ForceFunctionAttrsPass());
 
@@ -1075,9 +1081,16 @@
 ModulePassManager
 PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, bool DebugLogging,
  ModuleSummaryIndex *ExportSummary) {
-  assert(Level != O0 && "Must request optimizations for the default pipeline!");
   ModulePassManager MPM(DebugLogging);
 
+  if (Level == O0) {
+// The WPD and LowerTypeTest passes need to run at -O0 to lower type
+// metadata and intrinsics.
+MPM.addPass(WholeProgramDevirtPass(ExportSummary, nullptr));
+MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr));
+return MPM;
+  }
+
   if (PGOOpt && PGOOpt->Action == PGOOptions::SampleUse) {
 // Load sample profile before running the LTO optimization pipeline.
 MPM.addPass(SampleProfileLoaderPass(PGOOpt->ProfileFile,
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -39,12 +39,12 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi")

[PATCH] D61023: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews created this revision.
eandrews added reviewers: rnk, erichkeane.

Clang currently crashes for switch statements inside a template when the 
condition is non-integer and instantiation dependent. This is because 
contextual implicit conversion is skipped while acting on switch condition but 
this conversion is checked in an assert when acting on case statement.

This patch delays checks for dependent expressions till instantiation. Behavior 
now matches GCC.

Patch fixes Bug 40982.


https://reviews.llvm.org/D61023

Files:
  lib/Sema/SemaStmt.cpp
  test/SemaTemplate/non-integral-switch-cond.cpp


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of 
integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
+
+int main() {
+  foo instance;
+  instance.run(); // expected-note {{in instantiation of member function 
'foo::run' requested here}}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -404,7 +404,8 @@
 QualType CondType = CondExpr->getType();
 
 auto CheckAndFinish = [&](Expr *E) {
-  if (CondType->isDependentType() || E->isTypeDependent())
+  if (CondType->isDependentType() || CondExpr->isInstantiationDependent() 
||
+  E->isTypeDependent())
 return ExprResult(E);
 
   if (getLangOpts().CPlusPlus11) {
@@ -695,7 +696,8 @@
   Expr *CondExpr = Cond.get().second;
   assert((Cond.isInvalid() || CondExpr) && "switch with no condition");
 
-  if (CondExpr && !CondExpr->isTypeDependent()) {
+  if (CondExpr && !CondExpr->isTypeDependent() &&
+  !CondExpr->isInstantiationDependent()) {
 // We have already converted the expression to an integral or enumeration
 // type, when we parsed the switch condition. If we don't have an
 // appropriate type now, enter the switch scope but remember that it's


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
+
+int main() {
+  foo instance;
+  instance.run(); // expected-note {{in instantiation of member function 'foo::run' requested here}}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -404,7 +404,8 @@
 QualType CondType = CondExpr->getType();
 
 auto CheckAndFinish = [&](Expr *E) {
-  if (CondType->isDependentType() || E->isTypeDependent())
+  if (CondType->isDependentType() || CondExpr->isInstantiationDependent() ||
+  E->isTypeDependent())
 return ExprResult(E);
 
   if (getLangOpts().CPlusPlus11) {
@@ -695,7 +696,8 @@
   Expr *CondExpr = Cond.get().second;
   assert((Cond.isInvalid() || CondExpr) && "switch with no condition");
 
-  if (CondExpr && !CondExpr->isTypeDependent()) {
+  if (CondExpr && !CondExpr->isTypeDependent() &&
+  !CondExpr->isInstantiationDependent()) {
 // We have already converted the expression to an integral or enumeration
 // type, when we parsed the switch condition. If we don't have an
 // appropriate type now, enter the switch scope but remember that it's
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 196264.
riccibruno added a comment.

- Added a test (see `N_shadow)` for the behavior of `Wshadow`. This test showed 
that I forgot to change `CheckShadow(New, PrevDecl, R);` to `CheckShadow(New, 
PrevDecl->getUnderlyingDecl(), R);` to match change in the condition of the if 
statement.

- Added a test which exposes a new problem that this patch incidentally solves 
(see `N_conflicting_namespace_alias`). Because of the using directive `using 
namespace M;`, the namespace alias `i` for the namespace `Q` if found in the 
redeclaration lookup. Before this patch, since `getAsSingle` used internally 
`getUnderlyingDecl()`, `PrevDecl` was for the `NamespaceDecl` for `Q`, and not 
for the `VarDecl` for `Q::i`. Then the declaration for the enumerator `i` was 
mistakenly rejected since `Q` is in the same scope.

- Clarified some comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp

Index: test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp
===
--- test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp
+++ test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp
@@ -6,37 +6,30 @@
 // enumerator is declared in the scope of the enumeration. These names obey
 // the scope rules defined for all names in 6.3 and 6.4.
 
-// FIXME: Actually implement the redeclaration lookup properly.
 // FIXME: Improve the wording of the error messages (definition vs declaration).
-// FIXME: Only emit the Wshadow warning when the declaration of the
-//enumerator does not conflict with another declaration.
 
-// We also test for -Wshadow since the functionality is closely related,
-// and we don't want to mess up Wshadow by fixing the redeclaration lookup.
+// We also test for -Wshadow since the functionality is closely related.
 
 // Conflict with another declaration at class scope.
 struct S_function {
   void f(); // expected-note 2{{previous definition}}
-// FIXME: Don't emit shadow-note@-1 2{{previous declaration}}
   enum { f };   // expected-error {{redefinition of 'f'}}
-// FIXME: Don't emit shadow-warning@-1 {{declaration shadows}}
   enum E1 { f };// expected-error {{redefinition of 'f'}}
-// FIXME: Don't emit shadow-warning@-1 {{declaration shadows}}
   enum class E2 { f };  // ok
 };
 
 struct S_overloaded_function {
   void f();
-  void f(int);
-  enum { f };   // FIXME: Reject.
-  enum E1 { f };// FIXME: Reject.
+  void f(int);  // expected-note 2{{previous definition}}
+  enum { f };   // expected-error {{redefinition of 'f'}}
+  enum E1 { f };// expected-error {{redefinition of 'f'}}
   enum class E2 { f };  // ok
 };
 
 struct S_function_template {
-  template  void f();
-  enum { f };  // FIXME: Reject.
-  enum E1 { f };   // FIXME: Reject.
+  template  void f(); // expected-note 2{{previous definition}}
+  enum { f };  // expected-error {{redefinition of 'f'}}
+  enum E1 { f };   // expected-error {{redefinition of 'f'}}
   enum class E2 { f }; // ok
 };
 
@@ -52,12 +45,9 @@
 struct S_data_member {
   struct f;
   int f;  // expected-note {{previous definition}}
-  // FIXME: Don't emit shadow-note@-1 {{previous declaration}}
   static int g;   // expected-note {{previous definition}}
-  // FIXME: Don't emit shadow-note@-1 {{previous declaration}}
   enum { f, g };  // expected-error {{redefinition of 'f'}}
   // expected-error@-1 {{redefinition of 'g'}}
-  // FIXME: Don't emit shadow-warning@-2 2{{declaration shadows}}
 };
 
 namespace S_out_of_line_definition {
@@ -78,14 +68,13 @@
 namespace S_using_decl {
 
 struct Base {
-  int f; // FIXME: Don't emit shadow-note {{previous declaration}}
+  int f;
   int g;
 };
 struct OtherBase {};
 struct Der : Base, OtherBase {
-  using Base::f;
-  enum { f }; // FIXME: Reject.
-  // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}}
+  using Base::f; // expected-note {{previous definition}}
+  enum { f }; // expected-error {{redefinition of 'f'}}
   enum { g }; // ok
 
   using OtherBase::OtherBase;
@@ -103,8 +92,8 @@
 };
 
 template  struct Der : Base {
-  using Base::t;
-  enum { t }; // FIXME: Reject.
+  using Base::t; // expected-note {{previous definition}}
+  enum { t };   // expected-error {{redefinition of 't'}}
   // [namespace.udecl]p20:
   //   If a using-declarator uses the keyword typename and specifies a
   //   dependent name (17.6.2), the name introduced by the using-declaration
@@ -122,32 +111,27 @@
 // Conflict with another declaration at namespace scope
 namespace N_function {
   void f(); // expected-note 2{{previous definition}}

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDecl.cpp:16607
+  // Check for other kinds of shadowing not already handled.
+  if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) &&
+  !TheEnumDecl->isScoped())

aaron.ballman wrote:
> riccibruno wrote:
> > aaron.ballman wrote:
> > > Is the change to `PrevDecl->getUnderlyingDecl()` intended here?
> > Yes it is. Previously it was done inside `LookupResult::getAsSingle`. 
> > However with this patch `PrevDecl` at this point can be a `UsingShadowDecl` 
> > for a given using-declaration. We need to look for the underlying 
> > declaration since this is what `CheckShadow` expects.
> But when it's not a `UsingShadowDecl`, will the behavior now be incorrect? 
> e.g., if it was a `NamespaceAliasDecl`, won't this check whether you are 
> shadowing the aliased namespace as opposed to the alias name itself? Might be 
> worth some tests.
Do you mean in an example like the following ?

```
namespace Q {}
namespace M { namespace i = Q; }
using namespace M;

enum { i };
```
This is example is mistakenly rejected (I think!) because of the fact that 
currently `getAsSingle` will look through the `NamespaceAliasDecl` for `i`, and 
find the `NamespaceDecl` for `Q`. There are currently no shadow warning for 
such an example.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956



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

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> Added a test which exposes a new problem that this patch incidentally solves 
> (see `N_conflicting_namespace_alias`). Because of the using directive `using 
> namespace M;`, the namespace alias `i` for the namespace `Q` is found in the 
> redeclaration lookup. Before this patch, since `getAsSingle` used internally 
> `getUnderlyingDecl()`, `PrevDecl` was for the `NamespaceDecl` for `Q`, and 
> not for the `VarDecl` for `Q::i`. Then the declaration for the enumerator `i` 
> was mistakenly rejected since `Q` is in the same scope.

I meant in the above "[...] `PrevDecl` was the `NamespaceDecl` for `Q`, and not 
the `NamespaceAliasDecl` for `M::i`"


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956



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

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

This is intended? I'm surprised the two PMs don't have the same list of passes 
for a given opt level?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder marked 5 inline comments as done.
tmroeder added a comment.

Thanks to everyone for the comments. I've answered them as best I can, and I'm 
definitely open to changes or to scrapping this entirely.

I should have prefixed this patch with a discussion on the main list, perhaps. 
My main use case for this clang-tidy module is twofold: find problems in 
existing kernel code and checking incoming patches to (some of the) kernel 
mailing lists. I don't expect all (or even most) kernel developers to use this, 
just like I don't think most kernel developers use existing checkers (sparse 
and smatch) that have to be explicitly enabled by build-time options in Kbuild.

You might ask why I would want to implement these checks in clang-tidy at all 
if there are already static-analysis tools like sparse and smatch, though I 
expect that this list is generally in favor of expanding the scope of 
clang-tidy. The answer is that I think that having these checks directly in the 
compiler is the right way to go; clang-tidy (and clang-check, where I also 
would like to add some static analysis for the kernel) provide a principled 
basis for checking C code rather than using custom C parsers for the kernel. 
And I really like the AST matcher language and think it provides a powerful 
tool for writing these checks.




Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
+

lebedev.ri wrote:
> This looks wrong
Yeah, I'm not sure where that came from. I'll remove it.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:24
+  "ERR_CAST", "PTR_ERR_OR_ZERO"));
+  auto NonCheckingStmts = stmt(anyOf(compoundStmt(), labelStmt()));
+  Finder->addMatcher(

lebedev.ri wrote:
> Are there any rules what kernel considers to be checking and not checking?
> This i think e.g. will accept cast-to-void, assignment to a variable and then 
> ignoring it, etc.
> 
Yes, this check is extremely simplistic. My understanding of clang-tidy checks 
was that the most value comes from them catching obviously wrong behavior and 
not having any false positives. I didn't think they were supposed to catch all 
the ways in which something could be wrong.

But I've never written a clang-tidy check before. What is their expected 
purpose?



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:39
+  if (MatchedCallExpr &&
+  MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) {
+diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")

aaron.ballman wrote:
> Is the presence of the attribute actually important, or is it simply expected 
> that the declarations will have the attribute and so this check is benign?
The latter. But I think I could remove it; it seems unlikely that the attribute 
will be removed from these functions any time, though it could be disabled. It 
gets set in include/linux/compiler-gcc.h because clang sets the macros __GNUC__ 
and __GNUC_MINOR__ and __GNUC_PATCHLEVEL__ greater than 3, 4, and 0, 
respectively.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:51
+  }
+}
+

aaron.ballman wrote:
> Is there a way for the user to explicitly disable this warning, or is there 
> no such thing as a false positive as far as the kernel is concerned?
> 
> Should the diagnostic also produce a fix-it, or is there not a sufficiently 
> common replacement pattern to be used?
The basic case (are these error functions being used in any way at all?) is an 
error and should be fixed. As noted in response to a comment above, the check 
in that case is so naive that anything it catches is wrong.

With respect to not using the result from functions that return this value, I 
think the same argument applies: if code calls a function that returns an error 
like this and literally ignores the result, then clang-tidy should flag it. 
However, I don't know of a tool that currently checks the kernel for this kind 
of transitive property with respect to these functions, so it's possible that 
there are errors like that in the kernel (or idioms that I don't know about).

I thought that the way that you turn off clang-tidy checks is by specifying 
them at the command line with a minus sign in front of them: 
-checks=-linuxkernel-must-check-errs.

Or do you mean locally turning it off? In that case, you can just use the 
result of the function in any trivial way, like casting it to void.

With respect to the fixit; I thought about that, but I'm not sure I know what 
the right fixit should be. I'd like to leave it without a fixit for now and 
come back to it later if it becomes clear what to do here.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10
+functions are mar

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via Phabricator via cfe-commits]

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



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

mehdi_amini wrote:
> This is intended? I'm surprised the two PMs don't have the same list of 
> passes for a given opt level?
I'm really not sure. I did compare the post-link LTO pipelines of both PMs at 
O0/O1/O2 and confirmed that the old PM is doing many more passes than the new 
PM at O1. Probably a question for @chandlerc ? In any case, I didn't want to 
address it here since it is orthogonal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

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

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

richard.townsend.arm added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:5908
+// The checks below ensure that thare no user provided constructors.
+// For AArch64, we use the C++14 definition of an aggregate, so we also
+// check for:

I think these checks are in the wrong place - the aggregate restriction applies 
to return values, but it doesn't apply to arguments. Placing this logic here 
breaks argument passing between MSVC and Clang, because Clang (with this logic 
in place) will pass and expect arguments with an extra layer of indirection: 
for example, when passing 
https://cs.chromium.org/chromium/src/v8/include/v8.h?q=v8::Local&sq=package:chromium&g=0&l=183
 v8::Local, what you actually see in the register bank is supposed to be its 
only private member. Moving the logic to MicrosoftCXXABI and applying it only 
to the return value resolves this problem.



Comment at: lib/Sema/SemaDeclCXX.cpp:5916
+if (isAArch64) {
+  if (D->getAccess() == AS_private || D->getAccess() == AS_protected)
+return false;

efriedma wrote:
> D->getAccess() is the access specifier for the class itself (if the class is 
> nested inside another class), not the access specifier for any of its members.
> 
> I think you're looking for HasProtectedFields and HasPrivateFields.  (I think 
> there currently isn't any getter on CXXRecordDecl for them at the moment, but 
> you can add one.)
So the issue that I mentioned in 
https://bugs.llvm.org/show_bug.cgi?id=41135#c18 is resolved by checking 
HasProtectedFields/HasPrivateFields.


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

https://reviews.llvm.org/D60349



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

[PATCH] D60778: Make precompiled headers reproducible by switching OpenCL extension to std::map

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno commandeered this revision.
riccibruno edited reviewers, added: Anastasia; removed: riccibruno.
riccibruno added a comment.

Closing this since the issue was fixed in r358674.


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

https://reviews.llvm.org/D60778



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

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews created this revision.
eandrews added reviewers: rnk, riccibruno, erichkeane.

Clang currently crashes for switch statements inside a template when the 
condition is non-integer and instantiation dependent. This is because 
contextual implicit conversion is skipped while acting on switch condition but 
this conversion is checked in an assert when acting on case statement.

This patch delays checks for dependent expressions till instantiation. Behavior 
now matches GCC.

Patch fixes Bug 40982.


https://reviews.llvm.org/D61027

Files:
  lib/Sema/SemaStmt.cpp
  test/SemaTemplate/non-integral-switch-cond.cpp


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of 
integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
+
+int main() {
+  foo instance;
+  instance.run(); // expected-note {{in instantiation of member function 
'foo::run' requested here}}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -399,7 +399,8 @@
 QualType CondType = CondExpr->getType();
 
 auto CheckAndFinish = [&](Expr *E) {
-  if (CondType->isDependentType() || E->isTypeDependent())
+  if (CondType->isDependentType() || CondExpr->isInstantiationDependent() 
||
+  E->isTypeDependent())
 return ExprResult(E);
 
   if (getLangOpts().CPlusPlus11) {
@@ -690,7 +691,8 @@
   Expr *CondExpr = Cond.get().second;
   assert((Cond.isInvalid() || CondExpr) && "switch with no condition");
 
-  if (CondExpr && !CondExpr->isTypeDependent()) {
+  if (CondExpr && !CondExpr->isTypeDependent() &&
+  !CondExpr->isInstantiationDependent()) {
 // We have already converted the expression to an integral or enumeration
 // type, when we parsed the switch condition. If we don't have an
 // appropriate type now, enter the switch scope but remember that it's


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
+
+int main() {
+  foo instance;
+  instance.run(); // expected-note {{in instantiation of member function 'foo::run' requested here}}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -399,7 +399,8 @@
 QualType CondType = CondExpr->getType();
 
 auto CheckAndFinish = [&](Expr *E) {
-  if (CondType->isDependentType() || E->isTypeDependent())
+  if (CondType->isDependentType() || CondExpr->isInstantiationDependent() ||
+  E->isTypeDependent())
 return ExprResult(E);
 
   if (getLangOpts().CPlusPlus11) {
@@ -690,7 +691,8 @@
   Expr *CondExpr = Cond.get().second;
   assert((Cond.isInvalid() || CondExpr) && "switch with no condition");
 
-  if (CondExpr && !CondExpr->isTypeDependent()) {
+  if (CondExpr && !CondExpr->isTypeDependent() &&
+  !CondExpr->isInstantiationDependent()) {
 // We have already converted the expression to an integral or enumeration
 // type, when we parsed the switch condition. If we don't have an
 // appropriate type now, enter the switch scope but remember that it's
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r359009 - MS ABI: Support mangling op<=> now that MSVC 2019 has a mangling

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Apr 23 09:37:42 2019
New Revision: 359009

URL: http://llvm.org/viewvc/llvm-project?rev=359009&view=rev
Log:
MS ABI: Support mangling op<=> now that MSVC 2019 has a mangling

Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/test/CodeGenCXX/cxx2a-three-way-comparison.cpp

Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=359009&r1=359008&r2=359009&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Tue Apr 23 09:37:42 2019
@@ -1242,15 +1242,8 @@ void MicrosoftCXXNameMangler::mangleOper
   case OO_Array_Delete: Out << "?_V"; break;
   //  ::= ?__L # co_await
   case OO_Coawait: Out << "?__L"; break;
-
-  case OO_Spaceship: {
-// FIXME: Once MS picks a mangling, use it.
-DiagnosticsEngine &Diags = Context.getDiags();
-unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-  "cannot mangle this three-way comparison operator yet");
-Diags.Report(Loc, DiagID);
-break;
-  }
+  //  ::= ?__M # <=>
+  case OO_Spaceship: Out << "?__M"; break;
 
   case OO_Conditional: {
 DiagnosticsEngine &Diags = Context.getDiags();

Modified: cfe/trunk/test/CodeGenCXX/cxx2a-three-way-comparison.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx2a-three-way-comparison.cpp?rev=359009&r1=359008&r2=359009&view=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx2a-three-way-comparison.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx2a-three-way-comparison.cpp Tue Apr 23 
09:37:42 2019
@@ -1,24 +1,28 @@
 // RUN: %clang_cc1 -std=c++2a -emit-llvm %s -o - -triple %itanium_abi_triple | 
FileCheck %s --check-prefix=ITANIUM
-// RUN: not %clang_cc1 -std=c++2a -emit-llvm %s -o - -triple %ms_abi_triple 
2>&1 | FileCheck %s --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -o - -triple %ms_abi_triple 2>&1 | 
FileCheck %s --check-prefix=MSABI
 // RUN: not %clang_cc1 -std=c++2a -emit-llvm %s -o - -triple 
%itanium_abi_triple -DBUILTIN 2>&1 | FileCheck %s --check-prefix=BUILTIN
-// MSABI: cannot mangle this three-way comparison operator yet
 
 struct A {
   void operator<=>(int);
 };
 
 // ITANIUM: define {{.*}}@_ZN1AssEi(
+// MSABI: define {{.*}}@"??__MA@@QEAAXH@Z"(
 void A::operator<=>(int) {}
 
 // ITANIUM: define {{.*}}@_Zssi1A(
+// MSABI: define {{.*}}@"??__M@YAXHUA@@@Z"(
 void operator<=>(int, A) {}
 
 int operator<=>(A, A);
 
 // ITANIUM: define {{.*}}_Z1f1A(
+// MSABI: define {{.*}}@"?f@@YAHUA@@@Z"(
 int f(A a) {
   // ITANIUM: %[[RET:.*]] = call {{.*}}_Zss1AS_(
   // ITANIUM: ret i32 %[[RET]]
+  // MSABI: %[[RET:.*]] = call {{.*}}"??__M@YAHUA@@0@Z"(
+  // MSABI: ret i32 %[[RET]]
   return a <=> a;
 }
 


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

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 196261.
owenpan added a comment.

Removes a redundant test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,12 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -472,7 +473,7 @@
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,12 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManag

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via Phabricator via cfe-commits]

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



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

tejohnson wrote:
> mehdi_amini wrote:
> > This is intended? I'm surprised the two PMs don't have the same list of 
> > passes for a given opt level?
> I'm really not sure. I did compare the post-link LTO pipelines of both PMs at 
> O0/O1/O2 and confirmed that the old PM is doing many more passes than the new 
> PM at O1. Probably a question for @chandlerc ? In any case, I didn't want to 
> address it here since it is orthogonal.
Some more info:

This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses 
essentially the same as a normal opt pipeline so would be consistent at -O1.

The optimization missing from the new PM regular LTO post link pipeline that 
affects this test is SimplifyCFG. This and a few other optimizations are added 
in the old PM at O1 via PassManagerBuilder::addLateLTOOptimizationPasses. Note 
that PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 
(similar to where we exit early in the new PM for the regular LTO post link 
compilation). But the "late" LTO optimization passes are added unconditionally 
above -O0. Is this correct/desired? There isn't an equivalent in the new PM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

I apologize for the confusion. I was working on an incorrect branch earlier, 
and so abandoned that patch and uploaded a new revision.


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

https://reviews.llvm.org/D61027



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 196273.
tmroeder added a comment.

Remove an unnecessary header and fix the error text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.h
  clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c

Index: clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s linuxkernel-must-check-errs %t
+
+#define __must_check __attribute__((warn_unused_result))
+
+// Prototypes of the error functions.
+void * __must_check ERR_PTR(long error);
+long  __must_check PTR_ERR(const void *ptr);
+int  __must_check IS_ERR(const void *ptr);
+int  __must_check IS_ERR_OR_NULL(const void *ptr);
+void * __must_check ERR_CAST(const void *ptr);
+int  __must_check PTR_ERR_OR_ZERO(const void *ptr);
+
+void f() {
+  ERR_PTR(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'ERR_PTR' is unused
+  PTR_ERR((void *)0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'PTR_ERR' is unused
+  IS_ERR((void *)0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'IS_ERR' is unused
+  ERR_CAST((void *)0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'ERR_CAST' is unused
+out:
+  PTR_ERR_OR_ZERO((void *)0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'PTR_ERR_OR_ZERO' is unused
+}
+
+void *f1() {
+  return ERR_PTR(0);
+}
+
+long f2() {
+  if (IS_ERR((void *)0)) {
+return PTR_ERR((void *)0);
+  }
+  return -1;
+}
+
+void f3() {
+  f1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'f1' is unused but represents an error value
+  f2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'f2' is unused but represents an error value
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   linuxkernel-must-use-errs
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - linuxkernel-must-use-errs
+
+linuxkernel-must-use-errs
+=
+
+Checks for cases where the kernel error functions ``ERR_PTR``,
+``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
+``PTR_ERR_OR_ZERO`` are called but the results are not used. These
+functions are marked with ``__attribute__((warn_unused_result))``, but
+the compiler warning for this attribute is not always enabled.
+
+This also checks for unused values returned by functions that return
+``ERR_PTR``.
+
+Examples:
+
+.. code-block:: c
+
+  /* Trivial unused call to an ERR function */
+  PTR_ERR_OR_ZERO(some_function_call());
+
+  /* A function that returns ERR_PTR. */
+  void *fn() { ERR_PTR(-EINVAL); }
+
+  /* An invalid use of fn. */
+  fn();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -142,6 +142,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`linuxkernel-must-use-errs
+  ` check.
+
+  Checks Linux Kernel code to see if it uses the results from the functions in
+  linux/err.h. Also checks to see if code uses the results from functions that
+  directly return a value from one of these error functions.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMa

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};

aaron.ballman wrote:
> ztamas wrote:
> > aaron.ballman wrote:
> > > ztamas wrote:
> > > > JonasToth wrote:
> > > > > Please add tests with templated classes and self-assignment.
> > > > I tested with template classes, but TemplateCopyAndSwap test case, for 
> > > > example, was wrongly caught by the check. So I just changed the code to 
> > > > ignore template classes. I did not find any template class related 
> > > > catch either in LibreOffice or LLVM when I run the first version of 
> > > > this check, so we don't lose too much with ignoring template classes, I 
> > > > think.
> > > I am not keen on ignoring template classes for the check; that seems like 
> > > a bug in the check that should be addressed. At a minimum, the test cases 
> > > should be present with FIXME comments about the unwanted diagnostics.
> > I don't think it's a good idea to change the check now to catch template 
> > classes and produce false positives.
> > 
> > First of all the check does not work with template classes because the AST 
> > is different. Check TemplateCopyAndSwap test case for example. It's 
> > expected that the definition of operator= contains a CXXConstructExpr, but 
> > something is different for templates. It might be a lower level problem, 
> > how to detect a constructor call in a template class or templates just need 
> > different matcher. So implementing this check with correct template 
> > handling might be tricky and it might make the checker more complex. I'm 
> > not sure it worth the time, because as I mentioned I did not see any 
> > template related catch in the tested code bases. However, it might be a 
> > good idea to mention this limitation in the documentation.
> > 
> > About the false positives. This template related false positives are 
> > different from other false positives. In general, check doesn't warn, if 
> > the code uses one of the three patterns (self-check, copy-and-move, 
> > copy-and-swap). However, TemplateCopyAndSwap test case was wrongly caught 
> > by the check even though it uses copy-and-swap method. I would not 
> > introduce this kind of false positives. So the user of the check can expect 
> > that if he / she uses one of the three patterns, then there will be no 
> > warning in his / her code.
> > 
> > I already added five template related test cases. I think the comments are 
> > clear about which test case should be ignored by the check and which test 
> > cases are incorrectly ignored by now.
> > So implementing this check with correct template handling might be tricky 
> > and it might make the checker more complex. 
> 
> I would be surprised if it added that much complexity. You wouldn't be 
> checking the template declarations, but the template instantiations 
> themselves, which should have the same AST representation as similar 
> non-templated code.
> 
> >  I'm not sure it worth the time, because as I mentioned I did not see any 
> > template related catch in the tested code bases.
> 
> It's needed to conform to the CERT coding standard, which has no exceptions 
> for templates here.
> 
> > However, it might be a good idea to mention this limitation in the 
> > documentation.
> 
> My preference is to support it from the start, but if we don't support it, it 
> should definitely be mentioned in the documentation.
I added instatiation of template classes to the test code (locally):

```
template 
class TemplateCopyAndMove {
public:
  TemplateCopyAndMove &operator=(const TemplateCopyAndMove &object) {
TemplateCopyAndMove temp(object);
*this = std::move(temp);
return *this;
  }

private:
  T *p;
};

int instaniateTemplateCopyAndMove() {
TemplateCopyAndMove x;
(void) x;
}
```

However I don't see the expected AST neither in the ClassTemplateDecl or in the 
ClassTemplateSpecializationDecl. So how can I get that AST which is similar to 
non-template case?

```
|-ClassTemplateDecl 0x117ed20  line:342:7 
TemplateCopyAndMove
| |-TemplateTypeParmDecl 0x117ec08  col:17 referenced 
class depth 0 index 0 T
| |-CXXRecordDecl 0x117ec90  line:342:7 class 
TemplateCopyAndMove definition
| | |-DefinitionData standard_layout
| | | |-DefaultConstructor exists trivial needs_implicit
| | | |-CopyConstructor simple trivial has_const_param needs_implicit 
implicit_has_const_param
| | | |-MoveConstructor
| | | |-CopyAssignment non_trivial has_const_param user_declared 
implicit_has_const_param
| | | |-MoveAssignment
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x117ef70  col:7 implicit class 
TemplateCopyAndMove
| | |-AccessSpecDecl 0x117f000  col:1 public
| | |-CXXMethodDecl 0x117f9d0  line:344:27 operator= 
'TemplateCopyAndMove &(const TemplateCopyAndMove &)'
| | | |-ParmVarDecl 0x117f820  col:67 referenced object 'c

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359012: [APSInt][OpenMP] Fix isNegative, etc. for unsigned 
types (authored by jdenny, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59712?vs=195626&id=196274#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59712

Files:
  test/CodeGen/alloc-size.c
  test/OpenMP/distribute_collapse_messages.cpp
  test/OpenMP/distribute_parallel_for_collapse_messages.cpp
  test/OpenMP/distribute_parallel_for_simd_collapse_messages.cpp
  test/OpenMP/distribute_parallel_for_simd_safelen_messages.cpp
  test/OpenMP/distribute_parallel_for_simd_simdlen_messages.cpp
  test/OpenMP/distribute_simd_collapse_messages.cpp
  test/OpenMP/distribute_simd_safelen_messages.cpp
  test/OpenMP/distribute_simd_simdlen_messages.cpp
  test/OpenMP/for_collapse_messages.cpp
  test/OpenMP/for_ordered_clause.cpp
  test/OpenMP/for_simd_collapse_messages.cpp
  test/OpenMP/for_simd_safelen_messages.cpp
  test/OpenMP/for_simd_simdlen_messages.cpp
  test/OpenMP/parallel_for_collapse_messages.cpp
  test/OpenMP/parallel_for_ordered_messages.cpp
  test/OpenMP/parallel_for_simd_collapse_messages.cpp
  test/OpenMP/parallel_for_simd_safelen_messages.cpp
  test/OpenMP/parallel_for_simd_simdlen_messages.cpp
  test/OpenMP/simd_collapse_messages.cpp
  test/OpenMP/simd_safelen_messages.cpp
  test/OpenMP/simd_simdlen_messages.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/target_parallel_for_collapse_messages.cpp
  test/OpenMP/target_parallel_for_map_messages.cpp
  test/OpenMP/target_parallel_for_ordered_messages.cpp
  test/OpenMP/target_parallel_for_simd_collapse_messages.cpp
  test/OpenMP/target_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_parallel_for_simd_ordered_messages.cpp
  test/OpenMP/target_parallel_for_simd_safelen_messages.cpp
  test/OpenMP/target_parallel_for_simd_simdlen_messages.cpp
  test/OpenMP/target_parallel_map_messages.cpp
  test/OpenMP/target_simd_collapse_messages.cpp
  test/OpenMP/target_simd_safelen_messages.cpp
  test/OpenMP/target_simd_simdlen_messages.cpp
  test/OpenMP/target_teams_distribute_collapse_messages.cpp
  test/OpenMP/target_teams_distribute_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_collapse_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_collapse_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_safelen_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_simdlen_messages.cpp
  test/OpenMP/target_teams_distribute_simd_collapse_messages.cpp
  test/OpenMP/target_teams_distribute_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_simd_safelen_messages.cpp
  test/OpenMP/target_teams_distribute_simd_simdlen_messages.cpp
  test/OpenMP/target_teams_map_messages.cpp
  test/OpenMP/taskloop_collapse_messages.cpp
  test/OpenMP/taskloop_simd_collapse_messages.cpp
  test/OpenMP/taskloop_simd_safelen_messages.cpp
  test/OpenMP/taskloop_simd_simdlen_messages.cpp
  test/OpenMP/teams_distribute_collapse_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_collapse_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_safelen_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_simdlen_messages.cpp
  test/OpenMP/teams_distribute_simd_collapse_messages.cpp
  test/OpenMP/teams_distribute_simd_safelen_messages.cpp
  test/OpenMP/teams_distribute_simd_simdlen_messages.cpp
  test/Sema/shift.c
  test/SemaCXX/constexpr-unsigned-high-bit.cpp

Index: test/OpenMP/distribute_parallel_for_collapse_messages.cpp
===
--- test/OpenMP/distribute_parallel_for_collapse_messages.cpp
+++ test/OpenMP/distribute_parallel_for_collapse_messages.cpp
@@ -53,7 +53,7 @@
 #pragma omp distribute parallel for collapse ((ST > 0) ? 1 + ST : 2) // expected-note 2 {{as specified in 'collapse' clause}}
   for (int i = ST; i < N; i++) argv[0][i] = argv[0][i] - argv[0][i-ST]; // expected-error 2 {{expected 2 for loops after '#pragma omp distribute parallel for', but found only 1}}
   // expected-error@+8 2 {{directive '#pragma omp distribute parallel for' cannot contain more than one 'collapse' clause}}
-  // expected-error@+7 2 {{argument to 'collapse' clause must be a strictly positive integer value}}
+  // expected-error@+7 {{argument to 'collapse' clause must be a strictly positive integer value}}
   // expected-error@+6 2 {{expression is not an integral constant expression}}
 #if __cplusplus >= 201103L
   // expected-note@+4 2 {{non-constexpr function 'foobool' cannot be used in a constant expression}}
@@ -124,7 +124,7 @@
   // expected-note@+6{{non-constexpr function 'foobool' cannot be used 

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added inline comments.



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

tejohnson wrote:
> tejohnson wrote:
> > mehdi_amini wrote:
> > > This is intended? I'm surprised the two PMs don't have the same list of 
> > > passes for a given opt level?
> > I'm really not sure. I did compare the post-link LTO pipelines of both PMs 
> > at O0/O1/O2 and confirmed that the old PM is doing many more passes than 
> > the new PM at O1. Probably a question for @chandlerc ? In any case, I 
> > didn't want to address it here since it is orthogonal.
> Some more info:
> 
> This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses 
> essentially the same as a normal opt pipeline so would be consistent at -O1.
> 
> The optimization missing from the new PM regular LTO post link pipeline that 
> affects this test is SimplifyCFG. This and a few other optimizations are 
> added in the old PM at O1 via 
> PassManagerBuilder::addLateLTOOptimizationPasses. Note that 
> PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 (similar 
> to where we exit early in the new PM for the regular LTO post link 
> compilation). But the "late" LTO optimization passes are added 
> unconditionally above -O0. Is this correct/desired? There isn't an equivalent 
> in the new PM.
I don't think it was intentional, but I'm not sure I would directly replicate 
it if it requires adding an entirely new customization point. Is their some 
simpler way of getting equivalent results at O1?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: cfe/trunk/lib/Headers/CMakeLists.txt:168
 install(
-  FILES ${cuda_wrapper_files}
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include/cuda_wrappers)
+  DIRECTORY ${output_dir}
+  DESTINATION ${header_install_dir}

tstellar wrote:
> vzakhari wrote:
> > This change results in a use of CMAKE_CFG_INTDIR, which expands to 
> > $(Configuration) inside cmake_install.cmake, when using Visual Studio 
> > generator. CMake cannot reasonably expand $(Configuration), so Visual 
> > Studio builds are broken for me after this change-set.
> > 
> > Can we revert to installation from FILES relative to the current source 
> > dir?  This will require keeping a separate list of source files in 
> > copy_header_to_output_dir(), and using this list in the install() command.
> > 
> > I do understand that the intention was, probably, to copy headers files 
> > into output_dir along with creating some structure inside output_dir, and 
> > then installing the whole output_dir verbatim to the install dir.  It will 
> > be hard to achieve this with the change I suggest, but can we fix Visual 
> > Studio builds in any other way?
> > 
> > FWIW, vcproj invokes the installation with the following command: cmake 
> > -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
> > CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of 
> > $(Configuration) inside cmake_install.cmake.
> > This change results in a use of CMAKE_CFG_INTDIR, which expands to 
> > $(Configuration) inside cmake_install.cmake, when using Visual Studio 
> > generator. CMake cannot reasonably expand $(Configuration), so Visual 
> > Studio builds are broken for me after this change-set.
> 
> Prior to this change we were installing the arm generated files from 
> ${CMAKE_CURRENT_BINARY_DIR}.  Do we really need to use 
> ${LLVM_LIBRARY_OUTPUT_INTDIR} as the base for output_dir?  Could we use 
> ${CMAKE_CURRENT_BINARY_DIR} instead?  That seems like it would fix this issue.
> 
> > FWIW, vcproj invokes the installation with the following command: cmake 
> > -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
> CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of 
> $(Configuration) inside cmake_install.cmake.
> 
> Are the vc project files part of the LLVM source tree?
> 
> 
> 
> 
> 
As I understand, ${CMAKE_CURRENT_BINARY_DIR} is the same directory for all 
(Debug, Release) builds for the multiconfiguration builders.  I am not sure if 
it is possible to run Debug and Release builds in parallel, which would imply 
parallel access to the generated files.  Maybe it is a potential problem, but 
we have it even now inside clang_generate_header() function.

VC project files are generated by CMake, that is why I said CMake could have 
used ${BUILD_TYPE} in the cmake_install.cmake files instead of $(Configuration).

I guess just using ${CMAKE_CURRENT_BINARY_DIR} as the output_dir will solve the 
current problem.  It should work as long as Debug and Release versions of the 
header files are functionally equivalent, and noone runs Debug and Release 
builds in parallel.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



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

[PATCH] D61029: clang-cl: List valid values for /std: in /? output

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: hans.

https://reviews.llvm.org/D61029

Files:
  clang/include/clang/Driver/CLCompatOptions.td


Index: clang/include/clang/Driver/CLCompatOptions.td
===
--- clang/include/clang/Driver/CLCompatOptions.td
+++ clang/include/clang/Driver/CLCompatOptions.td
@@ -166,7 +166,7 @@
 def _SLASH_execution_charset : CLCompileJoined<"execution-charset:">,
   HelpText<"Runtime encoding, supports only UTF-8">, Alias;
 def _SLASH_std : CLCompileJoined<"std:">,
-  HelpText<"Language standard to compile for">;
+  HelpText<"Language standard to compile for (c++14,c++17,c++latest)">;
 def _SLASH_U : CLJoinedOrSeparate<"U">, HelpText<"Undefine macro">,
   MetaVarName<"">, Alias;
 def _SLASH_validate_charset : CLFlag<"validate-charset">,


Index: clang/include/clang/Driver/CLCompatOptions.td
===
--- clang/include/clang/Driver/CLCompatOptions.td
+++ clang/include/clang/Driver/CLCompatOptions.td
@@ -166,7 +166,7 @@
 def _SLASH_execution_charset : CLCompileJoined<"execution-charset:">,
   HelpText<"Runtime encoding, supports only UTF-8">, Alias;
 def _SLASH_std : CLCompileJoined<"std:">,
-  HelpText<"Language standard to compile for">;
+  HelpText<"Language standard to compile for (c++14,c++17,c++latest)">;
 def _SLASH_U : CLJoinedOrSeparate<"U">, HelpText<"Undefine macro">,
   MetaVarName<"">, Alias;
 def _SLASH_validate_charset : CLFlag<"validate-charset">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via Phabricator via cfe-commits]

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



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

chandlerc wrote:
> tejohnson wrote:
> > tejohnson wrote:
> > > mehdi_amini wrote:
> > > > This is intended? I'm surprised the two PMs don't have the same list of 
> > > > passes for a given opt level?
> > > I'm really not sure. I did compare the post-link LTO pipelines of both 
> > > PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes 
> > > than the new PM at O1. Probably a question for @chandlerc ? In any case, 
> > > I didn't want to address it here since it is orthogonal.
> > Some more info:
> > 
> > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses 
> > essentially the same as a normal opt pipeline so would be consistent at -O1.
> > 
> > The optimization missing from the new PM regular LTO post link pipeline 
> > that affects this test is SimplifyCFG. This and a few other optimizations 
> > are added in the old PM at O1 via 
> > PassManagerBuilder::addLateLTOOptimizationPasses. Note that 
> > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 
> > (similar to where we exit early in the new PM for the regular LTO post link 
> > compilation). But the "late" LTO optimization passes are added 
> > unconditionally above -O0. Is this correct/desired? There isn't an 
> > equivalent in the new PM.
> I don't think it was intentional, but I'm not sure I would directly replicate 
> it if it requires adding an entirely new customization point. Is their some 
> simpler way of getting equivalent results at O1?
Yeah we can presumably just add those optimizations to the -O1 early exit path 
in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would like to 
get this one in as it is a bugfix and orthogonal).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 196279.
ztamas added a comment.

Added missing fullstop
Added a new test cases making sure that HasNoSelfCheck works correctly
Added template instantiation to tests
Remove the alias from cert module and add the CERT link as a see also


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
@@ -0,0 +1,545 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template 
+void swap(T x, T y) {
+}
+
+template 
+T &&move(T x) {
+}
+
+template 
+class unique_ptr {
+};
+
+template 
+class shared_ptr {
+};
+
+template 
+class weak_ptr {
+};
+
+template 
+class auto_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+  PtrField &operator=(const PtrField &object);
+
+private:
+  int *p;
+};
+
+PtrField &PtrField::operator=(const PtrField &object) {
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+  // ...
+  return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+  InlineDefinition &operator=(const InlineDefinition &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class UniquePtrField {
+public:
+  UniquePtrField &operator=(const UniquePtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::unique_ptr p;
+};
+
+class SharedPtrField {
+public:
+  SharedPtrField &operator=(const SharedPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::shared_ptr p;
+};
+
+class WeakPtrField {
+public:
+  WeakPtrField &operator=(const WeakPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::weak_ptr p;
+};
+
+class AutoPtrField {
+public:
+  AutoPtrField &operator=(const AutoPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::auto_ptr p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+  CArrayField &operator=(const CArrayField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+  CopyConstruct &operator=(const CopyConstruct &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+WeakPtrField a;
+WeakPtrField b(a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+  AssignOperator &operator=(const AssignOperator &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+a.operator=(object.a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+  WeakPtrField a;
+};
+
+class NotSelfCheck {
+public:
+  NotSelfCheck &operator=(const NotSelfCheck &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignmen

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 5 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ has(ignoringParenCasts(cxxThisExpr()));

ztamas wrote:
> aaron.ballman wrote:
> > Will this also match code like:
> > ```
> > Frobble &Frobble::operator=(const Frobble &F) {
> >   if (&F == this->whatever())
> > return *this;
> > }
> > ```
> > or, more insidiously: 
> > ```
> > Frobble &Frobble::operator=(const Frobble &F) {
> >   if (&F == whatever()) // whatever is a member function of Frobble
> > return *this;
> > }
> > ```
> > 
> I'll check that case. The original hasLHS(...), hasRHS(...) might work with 
> this use case too.
I added NotSelfCheck test case for this, which works correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507



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

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Rong Xu via Phabricator via cfe-commits]

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

LGTM.  We need to Initialize the OptLevel no matter what.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp:107
   enum { typedef_type };// expected-error {{redefinition of 
'typedef_type'}}
 };
 

Note also that this is just my interpretation, and I am not entirely confident 
that this is correct.

 As a data point GCC rejects `enum { t }` in the template and accepts the other 
two enumerators, and then in the instantiation rejects `enum { t }` and `enum { 
typedef_type }` but accepts `enum { type }` (presumably because it delays the 
check for the hiding rules to the instantiation).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

Ok, so I removed the alias from cert module and added CERT rule link as a "see 
also".
So I think we solved the problem that things do not conform with the CERT 
requirements and can focus on the actual problem.

Summary, templates are still ignored. If there is any idea how to solve this 
for templates easily or there is any sample code for this, then I happy to 
implement template support.
I see a related comment in clang-tidy/modernize/PassByValueCheck.cpp:

  // Clang builds a CXXConstructExpr only when it knows which
  // constructor will be called. In dependent contexts a
  // ParenListExpr is generated instead of a CXXConstructExpr,
  // filtering out templates automatically for us.

So It's not only me who sees that template AST is different.

Also, two good ideas were mentioned about extra options during the review.

1. Allow finding catches not just pointers and arrays
2. Allow configuring list of suspicious field types / pointer types

I think these two options are good ideas for future improvements. For now, the 
check seems useful and produces not many false positives in the current form, I 
think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:54
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  hasAnyName("std::shared_ptr", "std::unique_ptr", "std::weak_ptr",
+ "std::auto_ptr"),

ztamas wrote:
> aaron.ballman wrote:
> > These should be using `::std::whatever` to ensure we only care about names 
> > in the global namespace named `std`.
> > 
> > Should this list be configurable? For instance, boost has a fair number of 
> > smart pointers.
> > Should this list be configurable? For instance, boost has a fair number of 
> > smart pointers.
> 
> xazax.hun has the same suggestion. I think it's a good idea for a follow-up 
> patch. I actually added a test case with the name CustomPtrField to document 
> this future possibility.
Yeah, I think it's fine to leave the list until a follow-up patch (though the 
names should still be corrected for this patch).



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:62-64
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+  has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+  hasType(arrayType(;

ztamas wrote:
> aaron.ballman wrote:
> > Hmm, while I understand why you're doing this, I'm worried that it will 
> > miss some pretty important cases. For instance, improper thread locking 
> > could result in deadlocks, improper releasing of non-memory resources could 
> > be problematic (such as network connections, file streams, etc), even 
> > simple integer assignments could be problematic in theory:
> > ```
> > Yobble& Yobble::operator=(const Yobble &Y) {
> >   superSecretHashVal = 0; // Being secure!
> >   ... some code that may early return ...
> >   superSecretHashVal = Y.superSecretHashVal;
> > }
> > ```
> > I wonder whether we want an option here to allow users to diagnose 
> > regardless of whether we think it's suspicious or not.
> > 
> > At the very least, this code should not be enabled for the CERT version of 
> > the check as it doesn't conform to the CERT requirements.
> It's starting to be too much for me.
> First, the problem was false positives. If there are too many false positives 
> then better to have it in the cert module.
> Now your problem is that this is not fit with the CERT requirements, because 
> of this matcher which reduces the false positives. Adding this check to the 
> CERT module was not my idea in the first place. So I suggest to have it a 
> simple bugprone check  (and remove the cert alias) and also we can mention 
> that it is related to the corresponding cert rule (but it does not conform 
> with it entirely).
> This check allows the usage of copy-and-move pattern, which does not conform 
> with the cert specification either where only the self-check and 
> copy-and-swap is mentioned. So your next suggestion will be to not allow 
> copy-and-move because it does not fit with the cert rule? So I think it's 
> better to have this not a cert check then, but a bugprone check. I prefer to 
> have a working check then implementing a theoretical documentation.
> 
> Apart from that cert thing, it actually seems a good idea to add a config 
> option to allow the user to get all catches, not just the "suspicious ones".
> It's starting to be too much for me.

It can be tricky to get this stuff right; I'm sorry the difficulties are 
aggravating.

> First, the problem was false positives. If there are too many false positives 
> then better to have it in the cert module.
> Now your problem is that this is not fit with the CERT requirements, because 
> of this matcher which reduces the false positives. 
> Adding this check to the CERT module was not my idea in the first place. So I 
> suggest to have it a simple bugprone check (and remove the cert alias) and 
> also we can mention that it is related to the corresponding cert rule (but it 
> does not conform with it entirely).

We typically ask authors to support the various coding standard modules when 
plausible because of the considerable amount of overlap between the 
functionalities. I don't particularly like the idea of ignoring the CERT coding 
standard here given that the solution is almost trivial to implement. However, 
if you want to remove it from the CERT module and not support it, that's your 
choice.

> This check allows the usage of copy-and-move pattern, which does not conform 
> with the cert specification either where only the self-check and 
> copy-and-swap is mentioned.

I don't get that from my reading of the CERT rule, where it says, "A 
user-provided copy assignment operator must prevent self-copy assignment from 
leaving the object in an indeterminate state. This can be accomplished by 
self-assignment tests, copy-and-swap, or other idiomatic design patterns.". 
Copy-and-

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi

tejohnson wrote:
> chandlerc wrote:
> > tejohnson wrote:
> > > tejohnson wrote:
> > > > mehdi_amini wrote:
> > > > > This is intended? I'm surprised the two PMs don't have the same list 
> > > > > of passes for a given opt level?
> > > > I'm really not sure. I did compare the post-link LTO pipelines of both 
> > > > PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes 
> > > > than the new PM at O1. Probably a question for @chandlerc ? In any 
> > > > case, I didn't want to address it here since it is orthogonal.
> > > Some more info:
> > > 
> > > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline 
> > > uses essentially the same as a normal opt pipeline so would be consistent 
> > > at -O1.
> > > 
> > > The optimization missing from the new PM regular LTO post link pipeline 
> > > that affects this test is SimplifyCFG. This and a few other optimizations 
> > > are added in the old PM at O1 via 
> > > PassManagerBuilder::addLateLTOOptimizationPasses. Note that 
> > > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 
> > > (similar to where we exit early in the new PM for the regular LTO post 
> > > link compilation). But the "late" LTO optimization passes are added 
> > > unconditionally above -O0. Is this correct/desired? There isn't an 
> > > equivalent in the new PM.
> > I don't think it was intentional, but I'm not sure I would directly 
> > replicate it if it requires adding an entirely new customization point. Is 
> > their some simpler way of getting equivalent results at O1?
> Yeah we can presumably just add those optimizations to the -O1 early exit 
> path in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would 
> like to get this one in as it is a bugfix and orthogonal).
(I fully agree it is orthogonal, I just spotted this as a separate bug indeed, 
thanks for fixing!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61022



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:62-64
+  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+  has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+  hasType(arrayType(;

aaron.ballman wrote:
> ztamas wrote:
> > aaron.ballman wrote:
> > > Hmm, while I understand why you're doing this, I'm worried that it will 
> > > miss some pretty important cases. For instance, improper thread locking 
> > > could result in deadlocks, improper releasing of non-memory resources 
> > > could be problematic (such as network connections, file streams, etc), 
> > > even simple integer assignments could be problematic in theory:
> > > ```
> > > Yobble& Yobble::operator=(const Yobble &Y) {
> > >   superSecretHashVal = 0; // Being secure!
> > >   ... some code that may early return ...
> > >   superSecretHashVal = Y.superSecretHashVal;
> > > }
> > > ```
> > > I wonder whether we want an option here to allow users to diagnose 
> > > regardless of whether we think it's suspicious or not.
> > > 
> > > At the very least, this code should not be enabled for the CERT version 
> > > of the check as it doesn't conform to the CERT requirements.
> > It's starting to be too much for me.
> > First, the problem was false positives. If there are too many false 
> > positives then better to have it in the cert module.
> > Now your problem is that this is not fit with the CERT requirements, 
> > because of this matcher which reduces the false positives. Adding this 
> > check to the CERT module was not my idea in the first place. So I suggest 
> > to have it a simple bugprone check  (and remove the cert alias) and also we 
> > can mention that it is related to the corresponding cert rule (but it does 
> > not conform with it entirely).
> > This check allows the usage of copy-and-move pattern, which does not 
> > conform with the cert specification either where only the self-check and 
> > copy-and-swap is mentioned. So your next suggestion will be to not allow 
> > copy-and-move because it does not fit with the cert rule? So I think it's 
> > better to have this not a cert check then, but a bugprone check. I prefer 
> > to have a working check then implementing a theoretical documentation.
> > 
> > Apart from that cert thing, it actually seems a good idea to add a config 
> > option to allow the user to get all catches, not just the "suspicious ones".
> > It's starting to be too much for me.
> 
> It can be tricky to get this stuff right; I'm sorry the difficulties are 
> aggravating.
> 
> > First, the problem was false positives. If there are too many false 
> > positives then better to have it in the cert module.
> > Now your problem is that this is not fit with the CERT requirements, 
> > because of this matcher which reduces the false positives. 
> > Adding this check to the CERT module was not my idea in the first place. So 
> > I suggest to have it a simple bugprone check (and remove the cert alias) 
> > and also we can mention that it is related to the corresponding cert rule 
> > (but it does not conform with it entirely).
> 
> We typically ask authors to support the various coding standard modules when 
> plausible because of the considerable amount of overlap between the 
> functionalities. I don't particularly like the idea of ignoring the CERT 
> coding standard here given that the solution is almost trivial to implement. 
> However, if you want to remove it from the CERT module and not support it, 
> that's your choice.
> 
> > This check allows the usage of copy-and-move pattern, which does not 
> > conform with the cert specification either where only the self-check and 
> > copy-and-swap is mentioned.
> 
> I don't get that from my reading of the CERT rule, where it says, "A 
> user-provided copy assignment operator must prevent self-copy assignment from 
> leaving the object in an indeterminate state. This can be accomplished by 
> self-assignment tests, copy-and-swap, or other idiomatic design patterns.". 
> Copy-and-move is another idiomatic design pattern for dealing with this, and 
> I'm glad your check incorporates it. (tbh, it would be nice for the CERT rule 
> to have a compliant solution demonstrating it -- I'll recommend it on their 
> rule.)
> 
> > So your next suggestion will be to not allow copy-and-move because it does 
> > not fit with the cert rule? So I think it's better to have this not a cert 
> > check then, but a bugprone check. I prefer to have a working check then 
> > implementing a theoretical documentation.
> 
> Theoretical documentation? The CERT standard is a published standard used in 
> industry that's supported by other analyzers as well as clang-tidy, so it's 
> not really theoretical.
> 
> > Apart from that cert thing, it actually seems a good idea to add a config 
> >

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It seems reasonable to me for target hooks to run after global hooks, but can I 
ask why AMDGPU specifically relies on this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60967



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

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: jfb.
Herald added a reviewer: EricWF.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

Before this patch, we would add both /usr/include/c++/v1
and /usr/include/c++/v1 to the header search paths for C++. The
toolchain path is added by the Driver and the /usr/include path
was added in CC1, a remnant of the non-refactoring of search paths
on Darwin. This patch removes /usr/include/c++/v1, since we should
be only looking for the libc++ headers beside the toolchain on Darwin.

The patch also adds basic tests validating that the right -internal-isystem
is passed from the Driver to CC1 for libc++. Once we refactor the rest of
the header search logic to be passed as -internal-isystem from the Driver
to CC1, we can add more tests to check that e.g. C headers are taken from
the SDK, etc..


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61032

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Driver/Inputs/basic_darwin_sdk/usr/bin/.keep
  clang/test/Driver/Inputs/basic_darwin_sdk/usr/include/.keep
  clang/test/Driver/Inputs/basic_darwin_sdk/usr/lib/.keep
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/bin/.keep
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/include/c++/v1/.keep
  clang/test/Driver/darwin-header-search.cpp


Index: clang/test/Driver/darwin-header-search.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-header-search.cpp
@@ -0,0 +1,23 @@
+// General tests that the header search paths detected by the driver and passed
+// to CC1 are sane on Darwin platforms.
+
+// Check a basic invocation (the headers alongside the toolchain should be 
used).
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck -DINPUTS=%S/Inputs 
--check-prefix=CHECK-BASIC-LIBCXX-ISYSTEM %s
+// CHECK-BASIC-LIBCXX-ISYSTEM: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-BASIC-LIBCXX-ISYSTEM: "-internal-isystem" 
"[[INPUTS]]/basic_darwin_toolchain/usr/bin/../include/c++/v1"
+
+// Check with a custom -isysroot (the headers in the toolchain should still be 
used).
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot=%S/Inputs/basic_darwin_sdk \
+// RUN:   | FileCheck -DINPUTS=%S/Inputs 
--check-prefix=CHECK-BASIC-LIBCXX-ISYSTEM-WITH-SYSROOT %s
+// CHECK-BASIC-LIBCXX-ISYSTEM-WITH-SYSROOT: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-BASIC-LIBCXX-ISYSTEM-WITH-SYSROOT: "-internal-isystem" 
"[[INPUTS]]/basic_darwin_toolchain/usr/bin/../include/c++/v1"
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -467,7 +467,10 @@
   if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
   HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
 if (HSOpts.UseLibcxx) {
-  AddPath("/usr/include/c++/v1", CXXSystem, false);
+  // On Darwin, all libc++ header search paths are handled in the driver.
+  if (!triple.isOSDarwin()) {
+AddPath("/usr/include/c++/v1", CXXSystem, false);
+  }
 } else {
   AddDefaultCPlusPlusIncludePaths(Lang, triple, HSOpts);
 }
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1778,7 +1778,7 @@
 llvm::StringRef InstallDir = getDriver().getInstalledDir();
 if (InstallDir.empty())
   break;
-// On Darwin, libc++ may be installed alongside the compiler in
+// On Darwin, libc++ is installed alongside the compiler in
 // include/c++/v1.
 // Get from 'foo/bin' to 'foo/include/c++/v1'.
 SmallString<128> P = InstallDir;


Index: clang/test/Driver/darwin-header-search.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-header-search.cpp
@@ -0,0 +1,23 @@
+// General tests that the header search paths detected by the driver and passed
+// to CC1 are sane on Darwin platforms.
+
+// Check a basic invocation (the headers alongside the toolchain should be used).
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I will build a significant codebase with the change and report on the results, 
since we lack good tests for the removal of `/usr/include/c++/v1`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61032



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 196291.
plotfi added a comment.
Herald added a subscriber: mgrang.

I've added an output format. The YAML coming out of -emit-ifso can now run 
through yaml2obj to produce a striped .o file. Those .o files can be handled by 
an existing linker.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/IFSO/foo-inline.h
  clang/test/IFSO/foo.cpp

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -25,6 +25,8 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +160,186 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND,
+  std::vector &MangledNames, int RDO) {
+if (!isa(ND))
+  return false;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+std::string MangledName = CGNameGen.getName(ND);
+MangledNames.push_back(MangledName);
+// For now, lets just dump the -fdelayed-template-parsing decls until we
+// decide how to handle them.
+if (IsRDOLate) {
+  llvm::errs() << "LATE DECL:\n";
+  ND->dump();
+}
+return true;
+  }
+
+  template 
+  bool HandleSomeDecl(const NamedDecl *ND,
+  std::vector &MangledNames, int RDO) {
+if (!isa(ND))
+  return false;
+for (auto *I : cast(ND)->decls())
+  HandleNamedDecl(dyn_cast(I), Mangled

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1474994 , @compnerd wrote:

> Note that I am not advocating that this change go in as is - I think that 
> this is far from something which would be useful, and until it can produce 
> something meaningful, this shouldn't really be merged (i.e. generate 
> something which obj2yaml or some other tool can consume to generate the 
> barest ELF stub).


Neither was I, as I did not have an output format (but I've added something 
now). I just want feedback to iterate on.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10
+functions are marked with ``__attribute__((warn_unused_result))``, but
+the compiler warning for this attribute is not always enabled.
+

tmroeder wrote:
> gribozavr wrote:
> > IIRC it is possible to pass through compiler warnings through ClangTidy... 
> > WDYT about that instead of reimplementing the warning?
> > 
> > However we would lose the ability to "infer" `warn_unused_result` on 
> > functions that return `ERR_PTR`.  However, since the analysis is not 
> > cross-translation-unit, IDK how much value there is.
> I don't know exactly how to pass the warnings through, but I'd be interested 
> in learning how to do that. I agree that that would be cleaner than this 
> (partial) reimplementation.
> 
> Note that my code above does do something like that: I currently check that 
> the unused-result attribute is set on the return from the call.
> I don't know exactly how to pass the warnings through, but I'd be interested 
> in learning how to do that. I agree that that would be cleaner than this 
> (partial) reimplementation.

It seems like it is done by default, and `-Wunused-result` is enabled by 
default:

```
$ cat /tmp/example.cpp
int __attribute__((warn_unused_result)) foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp
1 warning generated.
/tmp/example.cpp:4:3: warning: ignoring return value of function declared with 
'warn_unused_result' attribute [clang-diagnostic-unused-result]
  foo();
  ^
```

If it does not work for you, you can enable it on the command line:

```
clang-tidy -extra-arg=-Wunused-result -checks=clang-diagnostic-unused-result 
/tmp/example.cpp
```

> Note that my code above does do something like that: I currently check that 
> the unused-result attribute is set on the return from the call.

Yes, I was saying that we would lose the ability to do that.  However, is it 
that valuable?  The analysis in ClangTidy does not cross translation unit 
boundaries, so unless the caller and the callee are in the same file, this 
inference does not buy us much.

You could also use an existing check, `bugprone-unused-return-value`, without 
even requiring the function to be annotated with the attribute:

```
$ cat /tmp/example.cpp
int foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp -config="{Checks: 
'bugprone-unused-return-value', CheckOptions: [{key: 
'bugprone-unused-return-value.CheckedFunctions', value: 'foo'}]}"
1 warning generated.
/tmp/example.cpp:4:3: warning: the value returned by this function should be 
used [bugprone-unused-return-value]
  foo();
  ^
/tmp/example.cpp:4:3: note: cast the expression to void to silence this warning
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 196295.
ztamas added a comment.

Make the check to work with templates too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
@@ -0,0 +1,521 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template 
+void swap(T x, T y) {
+}
+
+template 
+T &&move(T x) {
+}
+
+template 
+class unique_ptr {
+};
+
+template 
+class shared_ptr {
+};
+
+template 
+class weak_ptr {
+};
+
+template 
+class auto_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+  PtrField &operator=(const PtrField &object);
+
+private:
+  int *p;
+};
+
+PtrField &PtrField::operator=(const PtrField &object) {
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+  // ...
+  return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+  InlineDefinition &operator=(const InlineDefinition &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class UniquePtrField {
+public:
+  UniquePtrField &operator=(const UniquePtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::unique_ptr p;
+};
+
+class SharedPtrField {
+public:
+  SharedPtrField &operator=(const SharedPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::shared_ptr p;
+};
+
+class WeakPtrField {
+public:
+  WeakPtrField &operator=(const WeakPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::weak_ptr p;
+};
+
+class AutoPtrField {
+public:
+  AutoPtrField &operator=(const AutoPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::auto_ptr p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+  CArrayField &operator=(const CArrayField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+  CopyConstruct &operator=(const CopyConstruct &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+WeakPtrField a;
+WeakPtrField b(a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+  AssignOperator &operator=(const AssignOperator &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+a.operator=(object.a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+  WeakPtrField a;
+};
+
+class NotSelfCheck {
+public:
+  NotSelfCheck &operator=(const NotSelfCheck &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+if (&object == this->doSomething()) {
+  // ...
+}
+return *this;
+  }
+
+  void *doSomething()

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

BTW, to clarify how this is intended to be used: the collection of the public 
interfaces defines the *public* interfaces of the module.  This information can 
be used to generate a **minimally** viable ELF module that the linker can use 
to link against - no `.text` segment or any other metadata (IIRC, you just need 
`.symtab`, `.shstrtab`, `.strtab`, `.dynsym` and the ELF header, but that is 
off the top of my head, and may be incorrect).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1474777 , @jakehehrlich 
wrote:

> In D60974#1474751 , @plotfi wrote:
>
> > There are a few that I have in mind.
> >
> > 1. -emit-ifso could be added to a build to produce .so files as part of a 
> > device SDK (where we don't want to ship the runnable bits in the SDK, we 
> > ship those on the device updates).
>
>
> This would require creating a linker for these files. Is this what you intend 
> to do?


No, I do not intend to write any sort of linker in the classical sense. At most 
I'd be handling and merging symbol names from yaml files or possibly going from 
yaml -> elf .o -> .so. Weighing options.

>> 2. -emit-ifso could be added to whatever the existing set of build flags are 
>> to be run as a pre-build step to produce tapi-like .so files that break up 
>> build dependencies.
> 
> How would this work? To be clear, this would happen at the same time that .o 
> files are produced correct? The build graph would basically look the same but 
> your new linking step would replace the original link step and there would be 
> a pendent node for the original link step. This would not break up the build 
> graph much at all save for the fact that we might imagine that this new link 
> step would be much faster.

I was thinking that since -emit-ifso doesn't invoke the backend, it could be 
run as a pre-build step. Then during the build, linking is done against the 
ifso stubs that are already generated instead of the actual .so files.

> 
> 
>> 3. -emit-ifso -fvisibility=hidden could added to the invocation to disallow 
>> usage of non-public apis.
> 
> Can you be more specific? This is already how the actual modules do this 
> today so what is the advantage here.

I believe modules produce much more data than what I am doing here. What I am 
doing is trying to use visibility semantics to produce the bare minimum ELF and 
bare minimum symbols while still being able to link with the resulting .so file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I also reformatted the code with clang-format.

So now the templates are handled, however, it's still not fit with the cert 
rule because we not catch all classes, but only those who have suspicious 
fields. I think this one can be added in a follow-up patch and then this check 
can be added to the cert module as an alias.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:326
+(*OS) << "  - Name:.text\n";
+(*OS) << "Type:STT_SECTION\n";
+(*OS) << "Section: .text\n";

This is wrong, this marks the type to be a section symbol, which is not correct.



Comment at: clang/lib/Frontend/FrontendActions.cpp:332
+<< "Section: .text\n"
+<< "Binding: STB_GLOBAL\n";
+(*OS) << "...\n";

Hmm, you need to classify the data symbols properly and mark them 
appropriately, not as `STT_FUNC`.  This is particularly important on ARM where 
the ISA selection bit may alter the address.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359025: [ThinLTO] Pass down opt level to LTO backend and 
handle -O0 LTO in new PM (authored by tejohnson, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61022?vs=196263&id=196296#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61022

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/test/CodeGen/thinlto-debug-pm.c
  cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  llvm/trunk/lib/Passes/PassBuilder.cpp
  llvm/trunk/test/tools/gold/X86/opt-level.ll

Index: cfe/trunk/test/CodeGen/thinlto-debug-pm.c
===
--- cfe/trunk/test/CodeGen/thinlto-debug-pm.c
+++ cfe/trunk/test/CodeGen/thinlto-debug-pm.c
@@ -1,10 +1,17 @@
-// Test to ensure -fdebug-pass-manager works when invoking the
-// ThinLTO backend path with the new PM.
+// Test to ensure the opt level is passed down to the ThinLTO backend.
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -o %t.o -flto=thin -fexperimental-new-pass-manager -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s
 // RUN: llvm-lto -thinlto -o %t %t.o
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s
-// CHECK: Running pass:
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O2-NEWPM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O0-NEWPM
+// O2-NEWPM: Running pass: LoopVectorizePass
+// O0-NEWPM-NOT: Running pass: LoopVectorizePass
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | FileCheck %s --check-prefix=O2-OLDPM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | FileCheck %s --check-prefix=O0-OLDPM
+// O2-OLDPM: Loop Vectorization
+// O0-OLDPM-NOT: Loop Vectorization
 
 void foo() {
 }
Index: cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -39,12 +39,12 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi") ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
 ; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
 
 ; Check that backend does not fail generating native code.
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
 ; RUN:   -o %t.native.o -x ir %t.o
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -1325,6 +1325,7 @@
   Conf.MAttrs = TOpts.Features;
   Conf.RelocModel = CGOpts.RelocationModel;
   Conf.CGOptLevel = getCGOptLevel(CGOpts);
+  Conf.OptLevel = CGOpts.OptimizationLevel;
   initTargetOptions(Conf.Options, CGOpts, TOpts, LOpts, HeaderOpts);
   Conf.SampleProfile = std::move(SampleProfile);
 
Index: llvm/trunk/lib/Passes/PassBuilder.cpp
===
--- llvm/trunk/lib/Passes/PassBuilder.cpp
+++ llvm/trunk/lib/Passes/PassBuilder.cpp
@@ -1046,10 +1046,16 @@
 //
 // Also, WPD has access to more precise information than ICP and can
 // devirtualize more effectively, so it should operate on the IR first.
+//
+// The WPD and LowerTypeTest passes need to run at -O0 to lower type
+// metadata and intrinsics.
 MPM.addPass(WholeProgramDevirtPass(nullptr, ImportSummary));
 MPM.addPass(LowerTypeTestsPass(nullptr, ImportSummary));
   }
 
+  if (Level == O0)
+return MPM;
+
   // Force any function attributes we want the rest of the pipeline to observe.
   MPM.addPass(ForceFunctionAttrsPass());
 
@@ -1075,9 +1081,16 @@
 ModulePassManager
 PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, bool DebugL

r359025 - [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Tue Apr 23 11:56:19 2019
New Revision: 359025

URL: http://llvm.org/viewvc/llvm-project?rev=359025&view=rev
Log:
[ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

Summary:
The opt level was not being passed down to the ThinLTO backend when
invoked via clang (for distributed ThinLTO).

This exposed an issue where the new PM was asserting if the Thin or
regular LTO backend pipelines were invoked with -O0 (not a new issue,
could be provoked by invoking in-process *LTO backends via linker using
new PM and -O0). Fix this similar to the old PM where -O0 only does the
necessary lowering of type metadata (WPD and LowerTypeTest passes) and
then quits, rather than asserting.

Reviewers: xur

Subscribers: mehdi_amini, inglorion, eraman, hiraditya, steven_wu, dexonsmith, 
cfe-commits, llvm-commits, pcc

Tags: #clang, #llvm

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

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/thinlto-debug-pm.c
cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=359025&r1=359024&r2=359025&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Apr 23 11:56:19 2019
@@ -1325,6 +1325,7 @@ static void runThinLTOBackend(ModuleSumm
   Conf.MAttrs = TOpts.Features;
   Conf.RelocModel = CGOpts.RelocationModel;
   Conf.CGOptLevel = getCGOptLevel(CGOpts);
+  Conf.OptLevel = CGOpts.OptimizationLevel;
   initTargetOptions(Conf.Options, CGOpts, TOpts, LOpts, HeaderOpts);
   Conf.SampleProfile = std::move(SampleProfile);
 

Modified: cfe/trunk/test/CodeGen/thinlto-debug-pm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-debug-pm.c?rev=359025&r1=359024&r2=359025&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto-debug-pm.c (original)
+++ cfe/trunk/test/CodeGen/thinlto-debug-pm.c Tue Apr 23 11:56:19 2019
@@ -1,10 +1,17 @@
-// Test to ensure -fdebug-pass-manager works when invoking the
-// ThinLTO backend path with the new PM.
+// Test to ensure the opt level is passed down to the ThinLTO backend.
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -o %t.o -flto=thin -fexperimental-new-pass-manager -triple 
x86_64-unknown-linux-gnu -emit-llvm-bc %s
 // RUN: llvm-lto -thinlto -o %t %t.o
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x 
ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager 
-fexperimental-new-pass-manager 2>&1 | FileCheck %s
-// CHECK: Running pass:
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x 
ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager 
-fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O2-NEWPM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x 
ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager 
-fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O0-NEWPM
+// O2-NEWPM: Running pass: LoopVectorizePass
+// O0-NEWPM-NOT: Running pass: LoopVectorizePass
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x 
ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | 
FileCheck %s --check-prefix=O2-OLDPM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x 
ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | 
FileCheck %s --check-prefix=O0-OLDPM
+// O2-OLDPM: Loop Vectorization
+// O0-OLDPM-NOT: Loop Vectorization
 
 void foo() {
 }

Modified: cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll?rev=359025&r1=359024&r2=359025&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll Tue Apr 23 
11:56:19 2019
@@ -39,12 +39,12 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: 
allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: 
branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: 
"_ZN1A1nEi") ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
 ; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
 
 ; Check that backend does not fail generating native code.
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thi

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 8 inline comments as done.
ztamas added a comment.

Mark some comments Done, which were handled some way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507



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

[PATCH] D60985: Fix compatability for cuda sm_75

[Chuan Qiu via Phabricator via cfe-commits]

eagleonhill added a comment.

That's great! will it be cherry-picked for clang8.1 if there's one?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60985



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

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In D60967#1475925 , @rjmccall wrote:

> It seems reasonable to me for target hooks to run after global hooks, but can 
> I ask why AMDGPU specifically relies on this?


We want to ensure certain symbols have a meaningful visibility. For example, 
kernel symbols must not have hidden visibility. It's reasonable for the user to 
arrange for a kernel symbol to have either protected or default visibility, 
though, so we want our hook to be run after the global hooks have already 
calculated the global visibility.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60967



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Javed Absar via Phabricator via cfe-commits]

javed.absar updated this revision to Diff 196297.
javed.absar marked an inline comment as done.
javed.absar added a comment.

Hi Tim:
Have made the changes as suggested. Please have a look.


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

https://reviews.llvm.org/D60485

Files:
  include/clang/Basic/BuiltinsAArch64.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/AArch64.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/arm_acle.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/arm64-mte.c
  test/Preprocessor/aarch64-target-features.c
  test/Sema/builtins-arm64-mte.c

Index: test/Sema/builtins-arm64-mte.c
===
--- /dev/null
+++ test/Sema/builtins-arm64-mte.c
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -triple arm64-arm-eabi %s -target-feature +mte -fsyntax-only -verify
+// RUN: %clang_cc1 -triple arm64-arm-eabi %s -target-feature +mte -x c++ -fsyntax-only -verify
+#include 
+#include 
+
+int  *create_tag1(int a, unsigned b) {
+  // expected-error@+1 {{first argument of MTE builtin function must be a pointer ('int' invalid)}}
+  return __arm_mte_create_random_tag(a,b);
+}
+
+int  *create_tag2(int *a, unsigned *b) {
+  // expected-error@+1 {{second argument of MTE builtin function must be an integer type ('unsigned int *' invalid)}}
+  return __arm_mte_create_random_tag(a,b);
+}
+
+int  *create_tag3(const int *a, unsigned b) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const int *'}}
+  return __arm_mte_create_random_tag(a,b);
+#else
+  // expected-warning@+1 {{returning 'const int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_create_random_tag(a,b);
+#endif
+}
+
+int  *create_tag4(volatile int *a, unsigned b) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'volatile int *'}}
+  return __arm_mte_create_random_tag(a,b);
+#else
+  // expected-warning@+1 {{returning 'volatile int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_create_random_tag(a,b);
+#endif
+}
+
+int  *increment_tag1(int *a, unsigned b) {
+  // expected-error@+1 {{argument to '__builtin_arm_addg' must be a constant integer}}
+  return __arm_mte_increment_tag(a,b);
+}
+
+int  *increment_tag2(int *a) {
+  // expected-error@+1 {{argument value 16 is outside the valid range [0, 15]}}
+  return __arm_mte_increment_tag(a,16);
+}
+
+int  *increment_tag3(int *a) {
+  // expected-error@+1 {{argument value -1 is outside the valid range [0, 15]}}
+  return __arm_mte_increment_tag(a,-1);
+}
+
+int  *increment_tag4(const int *a) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const int *'}}
+  return __arm_mte_increment_tag(a,5);
+#else
+  // expected-warning@+1 {{returning 'const int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_increment_tag(a,5);
+#endif
+}
+
+int *increment_tag5(const volatile int *a) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const volatile int *'}}
+  return __arm_mte_increment_tag(a,5);
+#else
+  // expected-warning@+1 {{returning 'const volatile int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_increment_tag(a,5);
+#endif
+}
+
+unsigned exclude_tag1(int *ptr, unsigned m) {
+   // expected-error@+1 {{first argument of MTE builtin function must be a pointer ('int' invalid)}}
+   return  __arm_mte_exclude_tag(*ptr, m);
+}
+
+unsigned exclude_tag2(int *ptr, int *m) {
+   // expected-error@+1 {{second argument of MTE builtin function must be an integer type ('int *' invalid)}}
+   return  __arm_mte_exclude_tag(ptr, m);
+}
+
+void get_tag1() {
+   // expected-error@+1 {{too few arguments to function call, expected 1, have 0}}
+   __arm_mte_get_tag();
+}
+
+int *get_tag2(int ptr) {
+   // expected-error@+1 {{first argument of MTE builtin function must be a pointer ('int' invalid)}}
+   return __arm_mte_get_tag(ptr);
+}
+
+int *get_tag3(const volatile int *ptr) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const volatile int *'}}
+  return __arm_mte_get_tag(ptr);
+#else
+  // expected-warning@+1 {{returning 'const volatile int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_get_tag(ptr);
+#endif
+}
+
+void set_tag1() {
+   // expected-error@+1 {{too few arguments to function call, expected 1, have 0}}
+   __arm_mte_set_tag();
+}
+
+void set_tag2(int ptr) {
+   // expected-error@+1 {{first argument of MTE builtin function must be a pointer ('int' invalid)}}
+   __arm_mte_set_tag(ptr);
+}
+
+ptrdiff_t subtract_pointers1(int a, int *b) {
+  // ex

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Shouldn't it be an error if the user tries to give it hidden visibility?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60967



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