[PATCH] D69841: Target Ivy bridge on macOS Mojave and later

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

@bob.wilson – Ping. While Ivy Bridge (or really, Sandy Bridge) has AVX1, which 
enables faster memcpy, memset, and even memcmp (for common scenarios). If there 
is something I'm missing? If you can't explain why, that's okay. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69841



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

[PATCH] D69921: [clang-format] refactor the use of the SMDiagnostics in replacement warnings

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 228826.
MyDeveloperDay added a comment.

Move adding the source file to the source mgr outside the replacements loop


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

https://reviews.llvm.org/D69921

Files:
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -292,56 +292,25 @@
 static bool
 emitReplacementWarnings(const Replacements &Replaces, StringRef 
AssumedFileName,
 const std::unique_ptr &Code) {
-  if (Replaces.empty()) {
+  if (Replaces.empty())
 return false;
-  }
-
-  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  DiagOpts->ShowColors = (ShowColors && !NoShowColors);
-
-  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
-  IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts));
-
-  IntrusiveRefCntPtr InMemoryFileSystem(
-  new llvm::vfs::InMemoryFileSystem);
-  FileManager Files(FileSystemOptions(), InMemoryFileSystem);
-  SourceManager Sources(*Diags, Files);
-  FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
- Files, InMemoryFileSystem.get());
-
-  FileManager &FileMgr = Sources.getFileManager();
-  llvm::ErrorOr FileEntryPtr =
-  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
   if (WarnFormat && !NoWarnFormat) {
 llvm::SourceMgr Mgr;
-for (const auto &R : Replaces) {
-  PresumedLoc PLoc = Sources.getPresumedLoc(
-  
Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()));
-
-  SourceLocation LineBegin =
-  Sources.translateFileLineCol(FileEntryPtr.get(), PLoc.getLine(), 1);
-  SourceLocation NextLineBegin = Sources.translateFileLineCol(
-  FileEntryPtr.get(), PLoc.getLine() + 1, 1);
+const char *StartBuf = Code->getBufferStart();
 
-  const char *StartBuf = Sources.getCharacterData(LineBegin);
-  const char *EndBuf = Sources.getCharacterData(NextLineBegin);
-
-  StringRef Line(StartBuf, (EndBuf - StartBuf) - 1);
-
-  SMDiagnostic Diag(
-  Mgr, SMLoc::getFromPointer(StartBuf), AssumedFileName,
-  PLoc.getLine(), PLoc.getColumn(),
+Mgr.AddNewSourceBuffer(
+MemoryBuffer::getMemBuffer(StartBuf, AssumedFileName), SMLoc());
+for (const auto &R : Replaces) {
+  SMDiagnostic Diag = Mgr.GetMessage(
+  SMLoc::getFromPointer(StartBuf + R.getOffset()),
   WarningsAsErrors ? SourceMgr::DiagKind::DK_Error
: SourceMgr::DiagKind::DK_Warning,
-  "code should be clang-formatted [-Wclang-format-violations]", Line,
-  ArrayRef>());
+  "code should be clang-formatted [-Wclang-format-violations]");
 
   Diag.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
-  Errors++;
-  if (ErrorLimit && Errors >= ErrorLimit)
+  if (ErrorLimit && ++Errors >= ErrorLimit)
 break;
 }
   }


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -292,56 +292,25 @@
 static bool
 emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
 const std::unique_ptr &Code) {
-  if (Replaces.empty()) {
+  if (Replaces.empty())
 return false;
-  }
-
-  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  DiagOpts->ShowColors = (ShowColors && !NoShowColors);
-
-  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
-  IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts));
-
-  IntrusiveRefCntPtr InMemoryFileSystem(
-  new llvm::vfs::InMemoryFileSystem);
-  FileManager Files(FileSystemOptions(), InMemoryFileSystem);
-  SourceManager Sources(*Diags, Files);
-  FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
- Files, InMemoryFileSystem.get());
-
-  FileManager &FileMgr = Sources.getFileManager();
-  llvm::ErrorOr FileEntryPtr =
-  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
   if (WarnFormat && !NoWarnFormat) {
 llvm::SourceMgr Mgr;
-for (const auto &R : Replaces) {
-  PresumedLoc PLoc = Sources.getPresumedLoc(
-  Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()));
-
-  SourceLocation LineBegin =
-  Sources.translateFileLineCol(FileEntryPtr.get(), PLoc.getLine(), 1);
-  SourceLocation NextLineBegin = Sources.translateFileLineCol(
-  FileEntryPtr.get(), PLoc.getLine() + 1, 1);
+const char *StartBuf = Code->getBufferStart();
 
-  const char *StartBuf = Sources.getCharacterData(LineBegin);
-  const char *EndBuf = Sources.getCharacterData(NextLineBegin);

[clang] a75f8d9 - [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[via cfe-commits]

Author: mydeveloperday
Date: 2019-11-12T09:25:00Z
New Revision: a75f8d98d7ac9e557b238a229a9a2647c71feed1

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

LOG: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for 
some operator functions

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

Addressing bug related to returning after return type not being honoured for 
some operator types.

```
$ bin/clang-format --style="{BasedOnStyle: llvm, AlwaysBreakAfterReturnType: 
TopLevelDefinitions}" /tmp/foo.cpp
class Foo {
public:
  bool operator!() const;
  bool operator<(Foo const &) const;
  bool operator*() const;
  bool operator->() const;
  bool operator+() const;
  bool operator-() const;
  bool f() const;
};

bool Foo::operator!() const { return true; }
bool
Foo::operator<(Foo const &) const {
  return true;
}
bool Foo::operator*() const { return true; }
bool Foo::operator->() const { return true; }
bool
Foo::operator+() const {
  return true;
}
bool
Foo::operator-() const {
  return true;
}
bool
Foo::f() const {
  return true;
}
```

Reviewers: mitchell-stellar, klimek, owenpan, sammccall, rianquinn

Reviewed By: sammccall

Subscribers: merge_guards_bot, cfe-commits

Tags: #clang-format, #clang-tools-extra, #clang

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 3bd415ebd699..d3c9dd56f97c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -950,9 +950,9 @@ class AnnotatingParser {
 if (CurrentToken->isOneOf(tok::star, tok::amp))
   CurrentToken->Type = TT_PointerOrReference;
 consumeToken();
-if (CurrentToken &&
-CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+if (CurrentToken && CurrentToken->Previous->isOneOf(
+TT_BinaryOperator, TT_UnaryOperator, 
tok::comma,
+tok::star, tok::arrow, tok::amp, tok::ampamp))
   CurrentToken->Previous->Type = TT_OverloadedOperator;
   }
   if (CurrentToken) {
@@ -1344,8 +1344,12 @@ class AnnotatingParser {
 Contexts.back().IsExpression = false;
 } else if (Current.is(tok::kw_new)) {
   Contexts.back().CanBeExpression = false;
-} else if (Current.isOneOf(tok::semi, tok::exclaim)) {
+} else if (Current.is(tok::semi) ||
+   (Current.is(tok::exclaim) && Current.Previous &&
+!Current.Previous->is(tok::kw_operator))) {
   // This should be the condition or increment in a for-loop.
+  // But not operator !() (can't use TT_OverloadedOperator here as its not
+  // been annotated yet).
   Contexts.back().IsExpression = true;
 }
   }
@@ -2155,11 +2159,22 @@ static bool isFunctionDeclarationName(const FormatToken 
&Current,
 continue;
   if (Next->isOneOf(tok::kw_new, tok::kw_delete)) {
 // For 'new[]' and 'delete[]'.
-if (Next->Next && Next->Next->is(tok::l_square) && Next->Next->Next &&
-Next->Next->Next->is(tok::r_square))
+if (Next->Next &&
+Next->Next->startsSequence(tok::l_square, tok::r_square))
   Next = Next->Next->Next;
 continue;
   }
+  if (Next->startsSequence(tok::l_square, tok::r_square)) {
+// For operator[]().
+Next = Next->Next;
+continue;
+  }
+  if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
+  Next->Next && Next->Next->isOneOf(tok::star, tok::amp, tok::ampamp)) 
{
+// For operator void*(), operator char*(), operator Foo*().
+Next = Next->Next;
+continue;
+  }
 
   break;
 }
@@ -2673,6 +2688,13 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
 tok::l_square));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
+  if (Right.isOneOf(tok::star, tok::amp, tok::ampamp) &&
+  (Left.is(tok::identifier) || Left.isSimpleTypeSpecifier()) &&
+  Left.Previous && Left.Previous->is(tok::kw_operator))
+// Space between the type and the *
+// operator void*(), operator char*(), operator Foo*() dependant
+// on PointerAlignment style.
+return (Style.PointerAlignment == FormatStyle::PAS_Right);
   const auto SpaceRequiredForArrayInitializerLSquare =
   [](const FormatToken &LSquareTok, const FormatStyle &Style) {
 return Style.SpacesInContainerLiterals ||

diff  --git a/clang/unittests/Format/FormatTest.cp

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa75f8d98d7ac: [clang-format] [PR36294] 
AlwaysBreakAfterReturnType works incorrectly for some… (authored by 
MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69573

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6134,7 +6134,48 @@
"void\n"
"A::operator>>() {}\n"
"void\n"
-   "A::operator+() {}\n",
+   "A::operator+() {}\n"
+   "void\n"
+   "A::operator*() {}\n"
+   "void\n"
+   "A::operator->() {}\n"
+   "void\n"
+   "A::operator void *() {}\n"
+   "void\n"
+   "A::operator void &() {}\n"
+   "void\n"
+   "A::operator void &&() {}\n"
+   "void\n"
+   "A::operator char *() {}\n"
+   "void\n"
+   "A::operator[]() {}\n"
+   "void\n"
+   "A::operator!() {}\n",
+   Style);
+  verifyFormat("constexpr auto\n"
+   "operator()() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator>>() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator+() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator*() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator->() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator++() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator void *() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator void &() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator void &&() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator char *() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator!() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator[]() const -> reference {}\n",
Style);
   verifyFormat("void *operator new(std::size_t s);", // No break here.
Style);
@@ -14755,6 +14796,53 @@
"#endif // while");
 }
 
+TEST_F(FormatTest, OperatorSpacing) {
+  FormatStyle Style = getLLVMStyle();
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("Foo::operator void *();", Style);
+  verifyFormat("Foo::operator()(void *);", Style);
+  verifyFormat("Foo::operator*(void *);", Style);
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("operator*(int (*)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("Foo::operator void &();", Style);
+  verifyFormat("Foo::operator()(void &);", Style);
+  verifyFormat("Foo::operator&(void &);", Style);
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("operator&(int (&)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("Foo::operator void &&();", Style);
+  verifyFormat("Foo::operator()(void &&);", Style);
+  verifyFormat("Foo::operator&&(void &&);", Style);
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("operator&&(int(&&)(), class Foo);", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("Foo::operator void*();", Style);
+  verifyFormat("Foo::operator()(void*);", Style);
+  verifyFormat("Foo::operator*(void*);", Style);
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("operator*(int (*)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("Foo::operator void&();", Style);
+  verifyFormat("Foo::operator()(void&);", Style);
+  verifyFormat("Foo::operator&(void&);", Style);
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("operator&(int (&)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("Foo::operator void&&();", Style);
+  verifyFormat("Foo::operator()(void&&);", Style);
+  verifyFormat("Foo::operator&&(void&&);", Style);
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("operator&&(int(&&)(), class Foo);", Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -950,9 +950,9 @@
 if (CurrentToken->isOn

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

NOTE: Add  foo() const override and foo() const override LLVM_READONLY test 
examples as these currently fail


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

https://reviews.llvm.org/D69764



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

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 228842.
MyDeveloperDay set the repository for this revision to rG LLVM Github Monorepo.
MyDeveloperDay added a comment.

Fix an issue with foo() const override/final
Be more specific about this being turned off in all the base styles


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -27,6 +27,19 @@
 
 FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
 
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)   \
+  verifyFormat(expect, actual, style, __LINE__)
+
+<<<
+===
+FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
+
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)   \
+  verifyFormat(expect, actual, style, __LINE__)
+
+>>>
 class FormatTest : public ::testing::Test {
 protected:
   enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
@@ -66,10 +79,24 @@
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+<<<
+===
+const FormatStyle &Style = getLLVMStyle(),
+int line = __LINE__) {
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable at " << __FILE__ << ":" << line;
+EXPECT_EQ(Expected.str(), format(Code, Style))
+<< " at " << __FILE__ << ":" << line;
+if (Style.Language == FormatStyle::LK_Cpp) {
+  // Objective-C++ is a superset of C++, so everything checked for C++
+  // needs to be checked for Objective-C++ as well.
+>>>
+const FormatStyle &Style = getLLVMStyle(),
+int line = __LINE__) {
 EXPECT_EQ(Expected.str(), format(Expected, Style))
-<< "Expected code is not stable";
-EXPECT_EQ(Expected.str(), format(Code, Style));
+<< "Expected code is not stable at " << __FILE__ << ":" << line;
+EXPECT_EQ(Expected.str(), format(Code, Style))
+<< " at " << __FILE__ << ":" << line;
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
@@ -80,8 +107,9 @@
   }
 
   void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+const FormatStyle &Style = getLLVMStyle(),
+int line = __LINE__) {
+verifyFormat(Code, test::messUp(Code), Style, line);
   }
 
   void verifyIncompleteFormat(llvm::StringRef Code,
@@ -12570,6 +12598,24 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -14843,6 +14889,346 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const 

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 228843.
MyDeveloperDay added a comment.

Make the patch file correctly.


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -27,6 +27,10 @@
 
 FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
 
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)   \
+  verifyFormat(expect, actual, style, __LINE__)
+
 class FormatTest : public ::testing::Test {
 protected:
   enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
@@ -66,10 +70,12 @@
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+const FormatStyle &Style = getLLVMStyle(),
+int line = __LINE__) {
 EXPECT_EQ(Expected.str(), format(Expected, Style))
-<< "Expected code is not stable";
-EXPECT_EQ(Expected.str(), format(Code, Style));
+<< "Expected code is not stable at " << __FILE__ << ":" << line;
+EXPECT_EQ(Expected.str(), format(Code, Style))
+<< " at " << __FILE__ << ":" << line;
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
@@ -80,8 +86,9 @@
   }
 
   void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+const FormatStyle &Style = getLLVMStyle(),
+int line = __LINE__) {
+verifyFormat(Code, test::messUp(Code), Style, line);
   }
 
   void verifyIncompleteFormat(llvm::StringRef Code,
@@ -12570,6 +12577,24 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -14843,6 +14868,194 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  VERIFYFORMAT("const int a;", Style);
+  VERIFYFORMAT("const int *a;", Style);
+  VERIFYFORMAT("const int &a;", Style);
+  VERIFYFORMAT("const int &&a;", Style);
+  VERIFYFORMAT("int const b;", Style);
+  VERIFYFORMAT("int const *b;", Style);
+  VERIFYFORMAT("int const &b;", Style);
+  VERIFYFORMAT("int const &&b;", Style);
+  VERIFYFORMAT("int const *b const;", Style);
+  VERIFYFORMAT("int *const c;", Style);
+
+  VERIFYFORMAT("const Foo a;", Style);
+  VERIFYFORMAT("const Foo *a;", Style);
+  VERIFYFORMAT("const Foo &a;", Style);
+  VERIFYFORMAT("const Foo &&a;", Style);
+  VERIFYFORMAT("Foo const b;", Style);
+  VERIFYFORMAT("Foo const *b;", Style);
+  VERIFYFORMAT("Foo const &b;", Style);
+  VERIFYFORMAT("Foo const &&b;", Style);
+  VERIFYFORMAT("Foo const *b const;", Style);
+
+  VERIFYFORMAT("LLVM_NODISCARD const int &Foo();", Style);
+  VERIFYFORMAT("LLVM_NODISCARD int const &Foo();", Style);
+
+  VERIFYFORMAT("volatile const int *restrict;", Style);
+  VERIFYFORMAT("const volatile int

[PATCH] D69935: [DeclCXX] Remove unknown external linkage specifications

[Ehud Katz via Phabricator via cfe-commits]

ekatz added a comment.

In D69935#1741268 , @dblaikie wrote:

> In D69935#1739235 , @dblaikie wrote:
>
> > +1 to this & then it seems fine (probably without a committed test, but I 
> > wouldn't mind a copy/paste of clang's behavior before/after this patch 
> > posted to this review to demonstrate what is being fixed)
>
>
> Waiting on this bit.


I described an expected behavior in D69935#1739871 
.


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

https://reviews.llvm.org/D69935



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Uncertainty over the handling of constant data between clang and libopenmp not 
withstanding, I think this is good to go.




Comment at: llvm/include/llvm/IR/OpenMPKinds.def:186
+///{
+
+#ifndef OMP_IDENT_FLAG

Meinersbur wrote:
> JonChesterfield wrote:
> > jdoerfert wrote:
> > > Meinersbur wrote:
> > > > jdoerfert wrote:
> > > > > JonChesterfield wrote:
> > > > > > Sharing constants between the compiler and the runtime is an 
> > > > > > interesting subproblem. I think the cleanest solution is the one 
> > > > > > used by libc, where the compiler generates header files containing 
> > > > > > the constants which the runtime library includes.
> > > > > I'd prefer not to tackle this right now but get this done first and 
> > > > > revisit the issue later. OK?
> > > > I don't think this is a good solution. It means that libomp cannot 
> > > > built built anymore without also building clang. Moreover, the values 
> > > > cannot be changed anyway since it would break the ABI.
> > > > 
> > > > I'd go the other route: The libomp defines what it's ABI is, the 
> > > > compiler generates code for it. 
> > > This patch doesn't change what we do, just where. The numbers are hard 
> > > coded in clang now. Let's start a discussion on the list and if we come 
> > > up with a different scheme we do it after this landed.
> > Revisit later sounds good.
> > 
> > @Meinersbur Do you know of an example of a non-llvm compiler using this 
> > libomp?
> > 
> > The usual order is build a compiler, then use it to build the runtime 
> > libraries, then the whole package can build other stuff. Provided the 
> > compiler doesn't need any of the runtime libraries (compiler-rt, maths 
> > libraries, libomp etc) itself the system bootstraps cleanly. Especially 
> > important when cross compiling and I suspect the gpu targets in openmp have 
> > similarly strict requirements on the first compiler.
> > 
> > Closely related to that, I tend to assume that the runtime libraries can be 
> > rewritten to best serve their only client - the associated compiler - so if 
> > libomp is used by out of tree compilers I'd like to know who we are at risk 
> > of breaking.
> > Do you know of an example of a non-llvm compiler using this libomp?
> 
> [[ 
> https://github.com/llvm-project/llvm-project/blob/master/openmp/runtime/src/kmp_gsupport.cpp
>  | gcc  ]] (using libomp's gomp compatibility layer), [[ 
> https://www.openmprtl.org/ | icc  ]] (as libomp was initially donated by 
> Intel).
> 
> I don't understand why it even matters if there are other compilers using 
> libomp. Every LLVM runtime library can be built stand-alone. 
> With constant values being determined during compiler bootstrapping, programs 
> built on one computer would be potentially ABI-incompatible with a runtime 
> library on another. Think about updating your compiler-rt/libomp/libc++ on 
> you computer causing all existing binaries on the system to crash because 
> constants changed in the updated compiler's bootstrapping process.
> 
> The only use case I know that does this is are operating system's syscall 
> tables. Linux's reference is [[ 
> https://github.com/torvalds/linux/blob/master/arch/sh/include/uapi/asm/unistd_64.h
>  | unistd.h ]] which is platform-specific and Windows generates the table 
> during its [[ https://j00ru.vexillium.org/syscalls/nt/64/ | build process ]]. 
> Therefore on Windows, system calls can only be done through ntdll. Even on 
> Linux one should use the system's libc instead of directly invoking a system 
> call.
Thanks. GCC and ICC would presumably be happier with the magic numbers stored 
with openmp then (though with the move to a monorepo that's a little less 
persuasive).

When constants that affect the ABI change, the result won't work with existing 
software regardless of whether the compiler or the library contains the change. 
Either the new compiler builds things that don't work with the old library, or 
the new library doesn't work with things built by the old compiler. The two 
have to agree on the ABI.

At present, openmp does the moral equivalent of #include OpenMPKinds.def from 
clang. Moving the constants to libomp means clang will do the equivalent of 
#include OpenMPKinds.def from openmp. Breaking that dependency means making a 
new subproject that just holds/generates the constants, that both depend on, 
which seems more hassle than it's worth. 

I'd like to generate this header as part of the clang build (though ultimately 
don't care that much if it's generated as part of the openmp build) because 
it's going to become increasingly challenging to read as non-nvptx 
architectures are introduced. Likewise it would be useful to generate the 
interface.h for deviceRTL (or equivalently a set of unit tests checking the 
function types) from the same source to ensure it matches and that's not 
economically feasible within the C preprocessor.


Repository:
  rG LLVM G

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

The follow-up patch, implementing this correctly, is D70116 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67409



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

[PATCH] D67787: Add 8548 CPU definition and attributes

[Vitaly Cheptsov via Phabricator via cfe-commits]

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

Actually thanks for adding C checks too. The new revision also looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67787



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

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59989 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



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

https://reviews.llvm.org/D69764



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

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


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

https://reviews.llvm.org/D69618



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

[PATCH] D69935: [DeclCXX] Remove unknown external linkage specifications

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D69935#1739235 , @dblaikie wrote:

> In D69935#1738273 , @uabelho wrote:
>
> > Don't you need to also remove
> >
> >   case LinkageSpecDecl::lang_cxx_11:
> >   case LinkageSpecDecl::lang_cxx_14:
> >   
> >
> > from VisitLinkageSpecDecl in clang-tools-extra/modularize/Modularize.cpp? 
> > (added in r372714 / e07376a320d to silence a clang warning).
> >  I can't see how that code would compile otherwise?
>
>
> +1 to this


Did you see the above?


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

https://reviews.llvm.org/D69935



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

[PATCH] D69760: [Clang][Driver] Don't pun -fuse-ld=lld as -fuse-ld=lld-link with msvc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D69760#1739044 , @Ericson2314 wrote:

> While I am fine making `-fuse-ld=lld` mean "use that lld driver most 
> appropriate for the clang driver", in principle the clang driver and lld 
> driver choices are orthogonal


In principle, but as ld.lld and lld-link uses different syntaxes that only 
really are handled correctly if passed through from the gcc and cl drivers 
respectively, I find it mostly theoretical to want to use them in the opposite 
combination forms. (Additionally, in practice, people using the cl driver in 
many cases call (lld-)link directly as it requires very little boilerplate 
options, contrary to unix style environments where one is supposed to pass in 
paths to crt2.o and crtbegin.o and the builtins library, etc. So if one wants 
to use a link style linker it's in most cases easiest to just call it directly 
instead of using the $CC frontend.)

> and I'd want to expose that with something like `-fuse-ld=ld.lld` to 
> compliment `-fuse-ld=lld-link`.

I don't mind adding these two cases as explicit overrides over the generic logic

> How does that sound? Should we support `-fuse-ld=ld.bfd` and 
> `-fuse-ld=ld64.lld` too?

I don't see where you'd need those? `-fuse-ld=bfd` should pick `ld.bfd` in most 
cases, right? In which case wouldn't it? The mapping between parameter name to 
`-fuse-ld` and system linker naming scheme varies a bit between the platforms, 
and I'd presume the other existing toolchains in clang does this right as it is 
right now. Or can you clarify and elaborate in which cases you'd want to use 
them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69760



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 228852.
awpandey added a comment.

Revised and address comments of @djtodoro


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll

Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,64 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -305,10 +305,11 @@
 
 DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,
 DIFile *File, unsigned LineNo,
-DIScope *Context) {
+DIScope *Context,
+uint32_t AlignInBits) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-LineNo, getNonCompileUnitScope(Context), Ty, 0, 0,
-0, None, DINode::FlagZero);
+LineNo, getNonCompileUnitScope(Context), Ty, 0,
+AlignInBits, 0, None, DINode::FlagZero);
 }
 
 DIDerivedType *DIBuilder::createFriend(DIType *Ty, DIType *FriendTy) {
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -800,6 +800,15 @@
   if (!Name.empty())
 addString(Buffer, dwarf::DW_AT_name, Name);
 
+  // If alignment is specified for a typedef , create and insert DW_AT_alignment
+  // attribute in DW_TAG_typedef DIE.
+  if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) {
+uint32_t AlignInBytes = DTy->getAlignInBytes();
+if (AlignInBytes > 0)
+  addUInt(Buffer, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
+

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

[Anastasia Stulova via Phabricator via cfe-commits]

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

- Added missing handling of lambda w/o param list.


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

https://reviews.llvm.org/D69938

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaOpenCLCXX/address-space-lambda.cl


Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (int) const __generic'
+auto glambda = [](auto a) { return a; };
+
+__kernel void foo() {
+  int i;
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  auto  llambda = [&]() {i++;};
+  llambda();
+  glambda(1);
+  // Test lambda with default parameters
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  [&] {i++;} ();
+  __constant auto err = [&]() {}; //expected-note-re{{candidate function not 
viable: address space mismatch in 'this' argument ('__constant (lambda at 
{{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  err(); //expected-error-re{{no matching function for call to object of type 
'__constant (lambda at {{.*}})'}}
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4916,7 +4916,9 @@
   .getScopeRep()
   ->getKind() == NestedNameSpecifier::TypeSpec) ||
  state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext;
+ DeclaratorContext::MemberContext ||
+ state.getDeclarator().getContext() ==
+ DeclaratorContext::LambdaExprContext;
 };
 
 if (state.getSema().getLangOpts().OpenCLCPlusPlus && IsClassMember()) {
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -887,6 +887,9 @@
 /*IsVariadic=*/false, /*IsCXXMethod=*/true));
 EPI.HasTrailingReturn = true;
 EPI.TypeQuals.addConst();
+if (getLangOpts().OpenCL)
+  EPI.TypeQuals.addAddressSpace(LangAS::opencl_generic);
+
 // C++1y [expr.prim.lambda]:
 //   The lambda return type is 'auto', which is replaced by the
 //   trailing-return type if provided and/or deduced from 'return'


Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (int) const __generic'
+auto glambda = [](auto a) { return a; };
+
+__kernel void foo() {
+  int i;
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  auto  llambda = [&]() {i++;};
+  llambda();
+  glambda(1);
+  // Test lambda with default parameters
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  [&] {i++;} ();
+  __constant auto err = [&]() {}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('__constant (lambda at {{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  err(); //expected-error-re{{no matching function for call to object of type '__constant (lambda at {{.*}})'}}
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4916,7 +4916,9 @@
   .getScopeRep()
   ->getKind() == NestedNameSpecifier::TypeSpec) ||
  state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext;
+ DeclaratorContext::MemberContext ||
+ state.getDeclarator().getContext() ==
+ DeclaratorContext::LambdaExprContext;
 };
 
 if (state.getSema().getLangOpts().OpenCLCPlusPlus && IsClassMember()) {
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -887,6 +887,9 @@
 /*IsVariadic=*/false, /*IsCXXMethod=*/true));
 EPI.HasTrailingReturn = true;
 EPI.TypeQuals.addConst();
+if (getLangOpts().OpenCL)
+  EPI.TypeQuals.addAddressSpace(LangAS::opencl_generic);
+
 // C++1y [expr.prim.lambda]:
 //   The lambda return type is 'auto', which is replaced by the
 //   trailing-return type if provided

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D69938#1741713 , @rjmccall wrote:

> In D69938#1741080 , @Anastasia wrote:
>
> > In D69938#1737196 , @rjmccall 
> > wrote:
> >
> > > It does make logical sense to be able to qualify the call operator of a 
> > > lambda.  The lambda always starts as a temporary, so presumably we want 
> > > the default address space qualifier on lambdas to be compatible with the 
> > > address space of temporaries.  However, that's probably also true of the 
> > > default address qualifier on methods in general or you wouldn't be able 
> > > to call them on temporaries and locals.  Thus, we should be able to use 
> > > the same default in both cases, which seems to be what you've implemented.
> > >
> > > It even makes sense to be able to qualify a lambda with an address space 
> > > that's not compatible with the address space of temporaries; that just 
> > > means that the lambda has to be copied elsewhere before it can be invoked.
> >
> >
> > Just to clarify... Do you mean we should be able to compile this example:
> >
> >   [&] __global {
> > i++;
> >   } ();
> >   
> >   
> >
> > Or should this result in a diagnostic about an addr space mismatch?
>
>
> It should result in a diagnostic on the call.  But if you assigned that 
> lambda into global memory (somehow) it should work.


Ok, makes sense so something like this should compile fine:

  __global auto glob = [&] __global { ... };

> 
> 
>> 3. Diagnose address space mismatch between variable decl and lambda expr 
>> qualifier Example: __private auto  llambda = [&]() __local {i++;}; // error 
>> no legal conversion b/w __private and __local
>> 
>>   I think 1 is covered by this patch. I would like to implement 2 as a 
>> separate patch though to be able to commit fix for 1 earlier and unblock 
>> some developers waiting for the fix. I think 3 already works and I will just 
>> update the diagnostic in a separate patch too.
> 
> I agree that 3 should just fall out.  And yeah, implementing 2 as a separate 
> patch should be fine.  Please make sure 3 is adequately tested in this patch.

Ok, since we can't qualify lambda expr with addr spaces yet there is not much I 
can test at this point. `__constant` lambda variable seems to be the only case 
with a diagnostic for OpenCL.


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

https://reviews.llvm.org/D69938



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Awanish Pandey via Phabricator via cfe-commits]

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

Thank you @djtodoro  for reviewing this.


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

https://reviews.llvm.org/D70111



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

[PATCH] D69564: Include the mangled name in -ast-dump=json

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I've split out the update script changes into D70119 
 to make this easier to review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69564



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

[PATCH] D69935: [DeclCXX] Remove unknown external linkage specifications

[Ehud Katz via Phabricator via cfe-commits]

ekatz updated this revision to Diff 228860.
ekatz added a comment.

In D69935#1741995 , @uabelho wrote:

> In D69935#1739235 , @dblaikie wrote:
>
> > In D69935#1738273 , @uabelho wrote:
> >
> > > Don't you need to also remove
> > >
> > >   case LinkageSpecDecl::lang_cxx_11:
> > >   case LinkageSpecDecl::lang_cxx_14:
> > >   
> > >
> > > from VisitLinkageSpecDecl in clang-tools-extra/modularize/Modularize.cpp? 
> > > (added in r372714 / e07376a320d to silence a clang warning).
> > >  I can't see how that code would compile otherwise?
> >
> >
> > +1 to this
>
>
> Did you see the above?


Sorry, I missed that (its from a separate commit). Added to the patch.


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

https://reviews.llvm.org/D69935

Files:
  clang-tools-extra/modularize/Modularize.cpp
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp

Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -31,8 +31,6 @@
 ExternCLoc = LSD->getBeginLoc();
   break;
 case LinkageSpecDecl::lang_cxx:
-case LinkageSpecDecl::lang_cxx_11:
-case LinkageSpecDecl::lang_cxx_14:
   break;
 }
 DC = LSD->getParent();
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -14205,10 +14205,6 @@
 Language = LinkageSpecDecl::lang_c;
   else if (Lang == "C++")
 Language = LinkageSpecDecl::lang_cxx;
-  else if (Lang == "C++11")
-Language = LinkageSpecDecl::lang_cxx_11;
-  else if (Lang == "C++14")
-Language = LinkageSpecDecl::lang_cxx_14;
   else {
 Diag(LangStr->getExprLoc(), diag::err_language_linkage_spec_unknown)
   << LangStr->getSourceRange();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5191,9 +5191,7 @@
 // EmitLinkageSpec - Emit all declarations in a linkage spec.
 void CodeGenModule::EmitLinkageSpec(const LinkageSpecDecl *LSD) {
   if (LSD->getLanguage() != LinkageSpecDecl::lang_c &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx_11 &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx_14) {
+  LSD->getLanguage() != LinkageSpecDecl::lang_cxx) {
 ErrorUnsupported(LSD, "linkage spec");
 return;
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1769,12 +1769,6 @@
   case LinkageSpecDecl::lang_cxx:
 OS << " C++";
 break;
-  case LinkageSpecDecl::lang_cxx_11:
-OS << " C++11";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-OS << " C++14";
-break;
   }
 }
 
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -874,12 +874,6 @@
   switch (LSD->getLanguage()) {
   case LinkageSpecDecl::lang_c: Lang = "C"; break;
   case LinkageSpecDecl::lang_cxx: Lang = "C++"; break;
-  case LinkageSpecDecl::lang_cxx_11:
-Lang = "C++11";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-Lang = "C++14";
-break;
   }
   JOS.attribute("language", Lang);
   attributeOnlyIfTrue("hasBraces", LSD->hasBraces());
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -1001,19 +1001,12 @@
 
 void DeclPrinter::VisitLinkageSpecDecl(LinkageSpecDecl *D) {
   const char *l;
-  switch (D->getLanguage()) {
-  case LinkageSpecDecl::lang_c:
+  if (D->getLanguage() == LinkageSpecDecl::lang_c)
 l = "C";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-l = "C++14";
-break;
-  case LinkageSpecDecl::lang_cxx_11:
-l = "C++11";
-break;
-  case LinkageSpecDecl::lang_cxx:
+  else {
+assert(D->getLanguage() == LinkageSpecDecl::lang_cxx &&
+   "unknown language in linkage specification");
 l = "C++";
-break;
   }
 
   Out << "extern \"" << l << "\" ";
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -42,7 +42,6 @@
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/iterator_range.h"
-#include "ll

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 228868.
bader added a subscriber: hfinkel.
bader added a comment.

Applied two remaining comments from Aaron.

- Split diagnostics for `sycl_kernel` attribute to provide more informative 
message.
- Moved attribute target check to TableGen file. I stole a workaround for a 
function template subject emulation from @hfinkel C++ JIT compiler prototype 
(https://github.com/hfinkel/llvm-project-cxxjit/blob/cxxjit/clang/include/clang/Basic/Attr.td#L121).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only template functions
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to template functions with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to template functions with at least two template parameters}}
+
+// Both first two template parameters must be a typenames
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of template functions with 'sycl_kernel' attribute must be typename}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of template functions with 'sycl_kernel' attribute must be typename}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{template function with 'sycl_kernel' attribute must return void type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{template function with 'sycl_kernel' attribute must return void type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{template function with 'sycl_kernel' attribute must have single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{template function with 'sycl_kernel' attribute must have single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -131,6 +131,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 //

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaSYCL/device-attributes.cpp:35
+template 
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' 
attribute only applies to template funtions with special prototype, please 
refer 'sycl_kernel' attribute documentation}}
+

Do we have to check each diagnostic message for both attribute spellings?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 228869.
usaxena95 marked 2 inline comments as done.
usaxena95 added a comment.
Herald added a subscriber: mgorny.

Added tests for CollectMacros.h
Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2050,6 +2050,32 @@
 } // namespace ns
 int main() { [[^ns]]::Foo foo; }
   )cpp",
+
+  R"cpp(// Macros
+#define [[FOO]](x,y) (x + y)
+// FIXME: No references for nested macros.
+#define BAR(x,y,z) (FOO(x, y) * FOO(y, z))
+int main() {
+  int x = [[FOO]](1, 2);
+  int y = [[FOO]]([[FO^O]](x, 1), [[FOO]](1, 1));
+  int z = BAR(1, 2, 3);
+}
+#define FOO(x) (x + 1)
+  )cpp",
+
+  R"cpp(// Macros: Cursor on definition.
+#define [[F^OO]](x,y) (x + y)
+int main() { int x = [[FOO]](1, 2); }
+  )cpp",
+
+  R"cpp( // Choose correct definiton in case of multiple definitions.
+#define abc 1
+#undef abc
+
+#define [[abc]] 2
+int main() { int a = [[abc]];}
+#undef [[ab^c]]
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -260,6 +260,15 @@
   // Includes macro expansions in arguments that are expressions
   ^assert(0 <= ^BAR);
 }
+
+#ifdef ^UNDEFINED
+#endif
+
+#define ^MULTIPLE_DEFINITION 1
+#undef ^MULTIPLE_DEFINITION
+
+#define ^MULTIPLE_DEFINITION 2
+#undef ^MULTIPLE_DEFINITION
   )cpp");
   auto TU = TestTU::withCode(TestCase.code());
   TU.HeaderCode = R"cpp(
@@ -274,7 +283,11 @@
   )cpp";
   ParsedAST AST = TU.build();
   std::vector MacroExpansionPositions;
-  for (const auto &R : AST.getMacros().Ranges)
+  for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+for (const auto &R : SIDToRefs.second)
+  MacroExpansionPositions.push_back(R.start);
+  }
+  for (const auto &R : AST.getMacros().UnknownMacros)
 MacroExpansionPositions.push_back(R.start);
   EXPECT_THAT(MacroExpansionPositions,
   testing::UnorderedElementsAreArray(TestCase.points()));
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -0,0 +1,86 @@
+#include "Annotations.h"
+#include "CollectMacros.h"
+#include "Matchers.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::UnorderedElementsAreArray;
+
+// Assumes that the ranges are named from 1 to N for some N.
+Matcher>>
+AreMacroRefsFrom(const Annotations &Test) {
+  std::vector>> Expected;
+  for (int I = 1;; I++) {
+const auto Refs = Test.ranges(llvm::to_string(I));
+if (Refs.empty())
+  break;
+Expected.push_back(UnorderedElementsAreArray(Refs));
+  }
+  return UnorderedElementsAreArray(Expected);
+}
+
+std::vector> collecknownReferences(MainFileMacros M) {
+  std::vector> Result;
+  for (const auto &SIDToRef : M.MacroRefs) {
+Result.emplace_back(SIDToRef.second.begin(), SIDToRef.second.end());
+  }
+  return Result;
+}
+
+TEST(CollectMainFileMacros, SelectedMacros) {
+  // References of the same symbol must have the ranges with the same
+  // name(integer). If there are N different symbols then they must be named
+  // from 1 to N. Macros for which SymbolID cannot be computed must be named
+  // "Unknown".
+  const char *Tests[] = {
+  R"cpp(// Macros: Cursor on definition.
+#define $1[[FOO]](x,y) (x + y)
+int main() { int x = $1[[FOO]]($1[[FOO]](3, 4), $1[[FOO]](5, 6)); }
+  )cpp",
+  R"cpp( // Multiple definitions.
+#define $1[[abc]] 1
+int func1() { int a = $1[[abc]];}
+#undef $1[[abc]]
+
+#define $2[[abc]] 2
+int func2() { int a = $2[[abc]];}
+#undef $2[[abc]]
+  )cpp",
+  R"cpp(
+#ifdef $Unknown[[UNDEFINED]]
+#endif
+  )cpp",
+  R"cpp(
+

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 228870.
usaxena95 added a comment.

Fixed typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2050,6 +2050,32 @@
 } // namespace ns
 int main() { [[^ns]]::Foo foo; }
   )cpp",
+
+  R"cpp(// Macros
+#define [[FOO]](x,y) (x + y)
+// FIXME: No references for nested macros.
+#define BAR(x,y,z) (FOO(x, y) * FOO(y, z))
+int main() {
+  int x = [[FOO]](1, 2);
+  int y = [[FOO]]([[FO^O]](x, 1), [[FOO]](1, 1));
+  int z = BAR(1, 2, 3);
+}
+#define FOO(x) (x + 1)
+  )cpp",
+
+  R"cpp(// Macros: Cursor on definition.
+#define [[F^OO]](x,y) (x + y)
+int main() { int x = [[FOO]](1, 2); }
+  )cpp",
+
+  R"cpp( // Choose correct definiton in case of multiple definitions.
+#define abc 1
+#undef abc
+
+#define [[abc]] 2
+int main() { int a = [[abc]];}
+#undef [[ab^c]]
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -260,6 +260,15 @@
   // Includes macro expansions in arguments that are expressions
   ^assert(0 <= ^BAR);
 }
+
+#ifdef ^UNDEFINED
+#endif
+
+#define ^MULTIPLE_DEFINITION 1
+#undef ^MULTIPLE_DEFINITION
+
+#define ^MULTIPLE_DEFINITION 2
+#undef ^MULTIPLE_DEFINITION
   )cpp");
   auto TU = TestTU::withCode(TestCase.code());
   TU.HeaderCode = R"cpp(
@@ -274,7 +283,11 @@
   )cpp";
   ParsedAST AST = TU.build();
   std::vector MacroExpansionPositions;
-  for (const auto &R : AST.getMacros().Ranges)
+  for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+for (const auto &R : SIDToRefs.second)
+  MacroExpansionPositions.push_back(R.start);
+  }
+  for (const auto &R : AST.getMacros().UnknownMacros)
 MacroExpansionPositions.push_back(R.start);
   EXPECT_THAT(MacroExpansionPositions,
   testing::UnorderedElementsAreArray(TestCase.points()));
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -0,0 +1,86 @@
+#include "Annotations.h"
+#include "CollectMacros.h"
+#include "Matchers.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::UnorderedElementsAreArray;
+
+// Assumes that the ranges are named from 1 to N for some N.
+Matcher>>
+AreMacroRefsFrom(const Annotations &Test) {
+  std::vector>> Expected;
+  for (int I = 1;; I++) {
+const auto Refs = Test.ranges(llvm::to_string(I));
+if (Refs.empty())
+  break;
+Expected.push_back(UnorderedElementsAreArray(Refs));
+  }
+  return UnorderedElementsAreArray(Expected);
+}
+
+std::vector> collectKnownReferences(MainFileMacros M) {
+  std::vector> Result;
+  for (const auto &SIDToRef : M.MacroRefs) {
+Result.emplace_back(SIDToRef.second.begin(), SIDToRef.second.end());
+  }
+  return Result;
+}
+
+TEST(CollectMainFileMacros, SelectedMacros) {
+  // References of the same symbol must have the ranges with the same
+  // name(integer). If there are N different symbols then they must be named
+  // from 1 to N. Macros for which SymbolID cannot be computed must be named
+  // "Unknown".
+  const char *Tests[] = {
+  R"cpp(// Macros: Cursor on definition.
+#define $1[[FOO]](x,y) (x + y)
+int main() { int x = $1[[FOO]]($1[[FOO]](3, 4), $1[[FOO]](5, 6)); }
+  )cpp",
+  R"cpp(// Multiple definitions.
+#define $1[[abc]] 1
+int func1() { int a = $1[[abc]];}
+#undef $1[[abc]]
+
+#define $2[[abc]] 2
+int func2() { int a = $2[[abc]];}
+#undef $2[[abc]]
+  )cpp",
+  R"cpp(
+#ifdef $Unknown[[UNDEFINED]]
+#endif
+  )cpp",
+  R"cpp(
+// Macros from token concatenations not included.
+#define $1[[CONCAT]](X) X##A()
+#define $2[[PREPEND

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/CollectMacros.h:29
   std::vector Ranges;
+  llvm::DenseMap> MacroRefs;
 };

hokein wrote:
> I think the `Ranges` and `MacrosRefs` have a lot of duplications, it is 
> wasteful to store a same range twice. We don't need the MainFilePath, the 
> callers should know the corresponding file path for the AST. Something like 
> `llvm::DenseMap> Refs;` should be enough.
> 
> symbol id for macro generation is required the definition location, but only 
> defined macros have it, we may need an extra `vector` to store ranges for 
> undefined macros.
> 
> ```
> #ifndef abc // there is no macro info* for abc, so getSymbolID will fail, but 
> we still want it, e.g. in semantic highlighting.
> #endif
> ``` 
> 
> There may be more tricky cases, it would be nice to have in the test.
> 
> ```
> #define abc 1
> #undef abc
> 
> #define abc 2
> #undef abc
> ```
- using `llvm::DenseMap> Refs;` as de-duplication is 
easier.
- Iterating on all the ranges is a pain. Let me know if you want to provide a 
different method to return all the ranges (sad part: lot of copying).
- I had multiple definitions of a macro in the test case, but I have shifted to 
its separate test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008



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

[clang] 44e5879 - AArch64: add arm64_32 support to Clang.

[Tim Northover via cfe-commits]

Author: Tim Northover
Date: 2019-11-12T12:45:18Z
New Revision: 44e5879f0fb7c28b90e8042fde81bba30b4090a3

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

LOG: AArch64: add arm64_32 support to Clang.

Added: 
clang/test/CodeGen/arm64_32-vaarg.c
clang/test/CodeGen/arm64_32.c
clang/test/Driver/arm64_32-link.c
clang/test/Preprocessor/arm64_32.c

Modified: 
clang/lib/Basic/Targets.cpp
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CodeGen/builtins-arm64.c
clang/test/CodeGen/target-data.c
clang/test/CodeGenCXX/armv7k.cpp
clang/test/Driver/aarch64-cpus.c
clang/test/Preprocessor/aarch64-target-features.c
clang/test/Preprocessor/init-v7k-compat.c
clang/test/Preprocessor/stdint.c
clang/test/Sema/aarch64-neon-vector-types.c
clang/test/Sema/types.c

Removed: 




diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 63a64ed2931a..664260d184fc 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -122,6 +122,11 @@ TargetInfo *AllocateTarget(const llvm::Triple &Triple,
   case llvm::Triple::lanai:
 return new LanaiTargetInfo(Triple, Opts);
 
+  case llvm::Triple::aarch64_32:
+if (Triple.isOSDarwin())
+  return new DarwinAArch64TargetInfo(Triple, Opts);
+
+return nullptr;
   case llvm::Triple::aarch64:
 if (Triple.isOSDarwin())
   return new DarwinAArch64TargetInfo(Triple, Opts);

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index c86cc63e3d84..bdfb5700b46a 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -51,7 +51,11 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple 
&Triple,
   HasLegalHalfType = true;
   HasFloat16 = true;
 
-  LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
+  if (Triple.isArch64Bit())
+LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
+  else
+LongWidth = LongAlign = PointerWidth = PointerAlign = 32;
+
   MaxVectorAlign = 128;
   MaxAtomicInlineWidth = 128;
   MaxAtomicPromoteWidth = 128;
@@ -160,7 +164,7 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions 
&Opts,
 Builder.defineMacro("__ELF__");
 
   // Target properties.
-  if (!getTriple().isOSWindows()) {
+  if (!getTriple().isOSWindows() && getTriple().isArch64Bit()) {
 Builder.defineMacro("_LP64");
 Builder.defineMacro("__LP64__");
   }
@@ -506,14 +510,19 @@ int AArch64TargetInfo::getEHDataRegisterNumber(unsigned 
RegNo) const {
   return -1;
 }
 
+bool AArch64TargetInfo::hasInt128Type() const { return true; }
+
 AArch64leTargetInfo::AArch64leTargetInfo(const llvm::Triple &Triple,
  const TargetOptions &Opts)
 : AArch64TargetInfo(Triple, Opts) {}
 
 void AArch64leTargetInfo::setDataLayout() {
-  if (getTriple().isOSBinFormatMachO())
-resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128");
-  else
+  if (getTriple().isOSBinFormatMachO()) {
+if(getTriple().isArch32Bit())
+  resetDataLayout("e-m:o-p:32:32-i64:64-i128:128-n32:64-S128");
+else
+  resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128");
+  } else
 resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
 }
 
@@ -631,19 +640,34 @@ DarwinAArch64TargetInfo::DarwinAArch64TargetInfo(const 
llvm::Triple &Triple,
  const TargetOptions &Opts)
 : DarwinTargetInfo(Triple, Opts) {
   Int64Type = SignedLongLong;
+  if (getTriple().isArch32Bit())
+IntMaxType = SignedLongLong;
+
+  WCharType = SignedInt;
   UseSignedCharForObjCBool = false;
 
   LongDoubleWidth = LongDoubleAlign = SuitableAlign = 64;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();
 
-  TheCXXABI.set(TargetCXXABI::iOS64);
+  UseZeroLengthBitfieldAlignment = false;
+
+  if (getTriple().isArch32Bit()) {
+UseBitFieldTypeAlignment = false;
+ZeroLengthBitfieldBoundary = 32;
+UseZeroLengthBitfieldAlignment = true;
+TheCXXABI.set(TargetCXXABI::WatchOS);
+  } else
+TheCXXABI.set(TargetCXXABI::iOS64);
 }
 
 void DarwinAArch64TargetInfo::getOSDefines(const LangOptions &Opts,
const llvm::Triple &Triple,
MacroBuilder &Builder) const {
   Builder.defineMacro("__AARCH64_SIMD__");
-  Builder.defineMacro("__ARM64_ARCH_8__");
+  if (Triple.isArch32Bit())
+Builder.define

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59989 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008



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

[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

[Mateusz Furdyna via Phabricator via cfe-commits]

furdyna updated this revision to Diff 228872.
furdyna marked 2 inline comments as done.
furdyna added a comment.

Hi @MyDeveloperDay, I changed the code following your advice.

Thanks for cathing the operator==(), I was not sure if this omission was 
intentional or not - fixed both comparisons.


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

https://reviews.llvm.org/D67750

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -466,6 +466,57 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, LeavesMainHeaderFirstInAdditionalExtensions) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?|(Impl)?$";
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+
+  // .cpp extension is considered "main" by default
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.cpp"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.cpp"));
+
+  // Allow additional filenames / extensions
+  Style.IncludeIsMainSourceRegex = "(Impl\\.hpp)|(\\.xxx)$";
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+}
+
 TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12847,6 +12847,8 @@
   IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
   "abc$");
+  CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
+  IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
   Style.RawStringFormats.clear();
   std::vector ExpectedRawStringFormats = {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -75,9 +75,9 @@
 
 static cl::opt AssumeFileName(
 "assume-filename",
-cl::desc("When reading from stdin, clang-format assumes this\n"
- "filename to look for a style config file (with\n"
- "-style=file) and to determine the language."),
+cl::desc("Override filename used to determine the language.\n"
+ "When reading from stdin, clang-format assumes this\n"
+ "filename to determine the language."),
 cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt Inplace("i",
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -184,6 +184,10 @@
FileName.endswith(".cpp") || FileName.endswith(".c++") ||
FileName.endswith(".cxx") || FileName.endswith(".m") ||
FileName.endswith(".mm");
+  if (!Style.IncludeIsMainSourceRegex.empty()) {
+llvm::Regex MainFileRege

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59989 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

In D61634#1732884 , @tejohnson wrote:

> In D61634#1682660 , @gchatelet wrote:
>
> > In D61634#1679331 , @tejohnson 
> > wrote:
> >
> > > In D61634#1635595 , @tejohnson 
> > > wrote:
> > >
> > > > I had some time to work on this finally late last week. I decided the 
> > > > most straightforward thing was to implement the necessary interface 
> > > > changes to the TLI analysis to make it require a Function (without any 
> > > > changes yet to how that analysis operates). See D66428 
> > > >  that I just mailed for review. That 
> > > > takes care of the most widespread changes needed for this migration, 
> > > > and afterwards we can change the analysis to look at the function 
> > > > attributes and make a truly per-function TLI.
> > >
> > >
> > > D66428  went in a few weeks ago at 
> > > r371284, and I just mailed the follow on patch D67923 
> > >  which will adds the support into the 
> > > TLI analysis to use the Function to override the available builtins (with 
> > > some of the code stubbed out since we don't yet have those per-Function 
> > > attributes finalized).
> > >
> > > @gchatelet where are you at on finalizing this patch? Also, I mentioned 
> > > this offline but to follow up here: I think we will want an attribute to 
> > > represent -fno-builtins (so that it doesn't need to be expanded out into 
> > > the full list of individual no-builtin-{func} attributes, which would be 
> > > both more verbose and less efficient, as well as being less backward 
> > > compatible when new builtin funcs are added).
> >
> >
> > I'll break this patch in several pieces. The first one is to add the 
> > `no_builtin` attribute, see https://reviews.llvm.org/D61634.
>
>
> Are you planning to add a follow on patch that translates the various 
> -fno-builtin* options to these attributes? Once that is done I can refine 
> D67923  to actually set the TLI availability 
> from the attributes and remove the clang functionality that does this from 
> the options.


The `no-builtin` attribute is in as of 
rG98f3151a7dded8838fafcb5f46e6c8358def96b8 
. I've 
started working on getting the `-fno-builtin` flag propagate to `no-builtin` 
function attributes. I'll ping back when its ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

[MyDeveloperDay via Phabricator via cfe-commits]

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

Thanks for the patch,

This LGTM, in the future, just a small nit for the future, can you ensure the 
diff is a full context diff with something like `@git diff --cached -U99 > 
patch.diff` (I wanted to check you got the recent changes by @sylvestre.ledru 
in docs/ClangFormat.rst) but I couldn't scroll that far down, so you'll need to 
be sure you've rebased if you haven't done so already.


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

https://reviews.llvm.org/D67750



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

[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

[Mateusz Furdyna via Phabricator via cfe-commits]

furdyna added a comment.

@MyDeveloperDay Thanks for the review!

I did rebase it a few hours ago. I actually made sure to use the full-context 
diff (with -U99 option) - I even have this as a last command in my command 
history, and the patch file on my disk is full-context. So either I still 
missed something, or the tool is playing with us ;)

BTW, I don't have rights to commit to master / merge the pull request. Can you 
do this for me?


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

https://reviews.llvm.org/D67750



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

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.

Thanks for updating the patch.

I'm happy for this patch to land, but I would like you to wait for @luismarques 
to approve it as well before landing it.




Comment at: libunwind/src/Registers.hpp:3756
+inline double Registers_riscv::getFloatRegister(int regNum) const {
+#ifdef __riscv_float_abi_double
+  assert(validFloatRegister(regNum));

mhorne wrote:
> luismarques wrote:
> > luismarques wrote:
> > > lenary wrote:
> > > > mhorne wrote:
> > > > > lenary wrote:
> > > > > > Is this an ABI or an architecture issue? I'm not sure what other 
> > > > > > libunwind "backends" do for similar cases.
> > > > > > 
> > > > > > The difference is, if I compile libunwind with `-march=rv64g 
> > > > > > -mabi=lp64`, `__riscv_float_abi_double` is not defined (because 
> > > > > > you're using a soft-float ABI), but `__riscv_flen == 64` (because 
> > > > > > the machine does have hardware floating-point registers).
> > > > > > 
> > > > > > I'm not sure what the intended behaviour of libunwind is in this 
> > > > > > case.
> > > > > > 
> > > > > > `__riscv_float_abi_double` implies `__riscv_flen >= 64`.
> > > > > An ABI issue, in my opinion. The unwind frame will always contain 
> > > > > space for the float registers, but accessing them should be 
> > > > > disallowed for soft-float configurations as the intent of soft-float 
> > > > > is that the FPRs will not be used at all. I'd say there is precedent 
> > > > > for this in the MIPS implementation, since it checks for 
> > > > > `defined(__mips_hard_float) && __mips_fpr == 64`.
> > > > I had a discussion with @asb about this. The ISA vs ABI issue in RISC-V 
> > > > is complex. The TL;DR is we both think you need to be using 
> > > > `__riscv_flen == 64` here.
> > > > 
> > > > The reason for this is that if you compile with `-march=rv64imfd` but 
> > > > `-mabi=lp64`, the architecture still has floating point registers, it 
> > > > just does not use the floating-point calling convention. This means 
> > > > there are still `D`-extension instructions in the stream of 
> > > > instructions, just that "No floating-point registers, if present, are 
> > > > preserved across calls." (see [[ 
> > > > https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#integer-calling-convention
> > > >  | psABI Integer Calling Convention ]]) This effectively means that 
> > > > with this combination, `f0-f31` are treated exactly the same as 
> > > > `t0-t6`, and so should be able to be restored when unwinding. It is not 
> > > > necessarily the case that with a soft float ABI, `f0-f31` are not used 
> > > > at all. This is similar to ARM's `soft` vs `softfp` calling conventions.
> > > > 
> > > > The expectation is that if you are compiling your programs with a 
> > > > specific `-march`, then you should be compiling your runtime libraries 
> > > > with the same `-march`. Eventually there should be enough details in 
> > > > the ELF file to allow you to ensure both `-march` and `-mabi` match 
> > > > when linking programs, but support for this is not widespread.
> > > A soft-float *ABI* doesn't mean that FPRs aren't used at all, it means 
> > > that floating-point arguments aren't passed in the floating-point 
> > > registers. From a quick Google search I got the impression that 
> > > `__mips_hard_float` was used for a mips softfloat target (i.e. without 
> > > hardware floating-point support, not for a soft-float ABI), so that's 
> > > probably not a comparable example.
> > I just saw @lenary's reply. I agree with his more detailed analysis.
> Thanks Sam and Luis for the detailed replies.
> 
> I definitely agree with you that `__riscv_flen == 64` is the more appropriate 
> check. But now I'm reconsidering if a floating point check is needed at all. 
> By adding it are we not preventing access to the FPRs for cross/remote 
> unwinding?
Cross-compiling across RISC-V architectures is very complex. Sadly, using only 
the target triple is not enough, and nor is matching the ABI, because the 
architecture is so extensible.

In all of these cases, we expect end users to explicitly compile their required 
libraries with the correct `-march`/`-mabi` for the RISC-V platform they are 
using. If they are cross-compiling, then that means the whole sysroot, compiler 
runtime (libgcc or compiler-rt), and libc should be compiled with an 
explicitly-set `-march`/`-mabi`. If they do this, then there will be no issues 
with our code. Importantly, this should still largely work for multilib builds, 
where there are multiple march/mabi combinations that libraries are compiled 
for.

This is not optimal from the point-of-view of someone developing for lots of 
disparate RISC-V targets (like compiler developers), but should be ok for 
developers developing for single devices.


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

https://reviews.llvm.org/D68362



_

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

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

xbolva00 added a comment.

Should we reconsider this as a clang warning? I found that the newest GCC can 
diagnose it.

GCC 9+:

-Wdeprecated-copy, implied by -Wextra, warns about the C++11 deprecation of 
implicitly declared copy constructor and assignment operator if one of them is 
user-provided. -Wdeprecated-copy-dtor also warns if the destructor is 
user-provided, as specified in C++11.

It woule be nice to keep compatibility. -Wextra seems reasonable for me too.


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

https://reviews.llvm.org/D68185



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

[PATCH] D63131: arm64_32: implement the desired ABI for the ILP32 triple.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover marked 6 inline comments as done.
t.p.northover added a comment.

Thanks, I've updated for most of the suggestions and committed it. I'll make 
the AArch64 naming changes separately if we decide to.




Comment at: clang/lib/Basic/Targets/AArch64.cpp:167
   // Target properties.
-  if (!getTriple().isOSWindows()) {
+  if (!getTriple().isOSWindows() && getTriple().isArch64Bit()) {
 Builder.defineMacro("_LP64");

jfb wrote:
> This might affect odd non-Darwin targets? Seems unlikely, but just asking 
> since we have existence proof with Windows that stuff is weird. Admittedly 
> they're untested if it affects them, so I think this is fine.
isArch64Bit is taken directly from Triple::aarch64/Triple::aarch64_32, so I 
think someone would have to be intentionally bringing up an aarch64_32 platform 
to be affected, in which case they probably want to be.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5457
   // for AArch64, emit a warning and ignore the flag. Otherwise, add the
   // proper mllvm flags.
+  if (Triple.getArch() != llvm::Triple::aarch64 &&

jfb wrote:
> The comment isn't quite right anymore. Maybe don't say `AArch64` since the 
> code is obvious about what it checks?
AArch64 is the official name of the 64-bit execution mode of ARM processors so 
I think it's still correct to say aarch64_32 is AArch64. Or were you referring 
to some other aspect of the comment?



Comment at: clang/lib/Sema/SemaChecking.cpp:5512
+  bool IsAArch64 = (TT.getArch() == llvm::Triple::aarch64 ||
+TT.getArch() == llvm::Triple::aarch64_32);
   bool IsWindows = TT.isOSWindows();

jfb wrote:
> This is now a weird variable name, since it's aarch64 maybe 32 but not be. 
> Could you rename `IsAArch64`?
I think the same point about the comment above applies here. The Triple is the 
odd thing out here in distinguishing the two.



Comment at: clang/test/CodeGen/builtins-arm64.c:11
 
+#if __LP64__
 void *tp (void) {

jfb wrote:
> Why isn't this one supported?
It was an iOS version check, I've updated the triple instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63131



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

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@aaron.ballman just ping, could you let me know if you have any further 
comments? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks for sharing this!

I still need to take a proper look, but my first thought is this:

Instead of invoking main() (or a similar function like ClangDriverMain) as an 
alternative to spinning up the cc1 process, would it be possible to call 
ExecuteCC1Tool() directly? I guess it would be a little less general, but maybe 
it would be more straight-forward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

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

Szelethus accepted this revision.
Szelethus added a comment.

Whoo!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70047



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

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

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

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

In D69962#1741503 , @NoQ wrote:

> In D69962#1739440 , @NoQ wrote:
>
> > In D69962#1738618 , @Szelethus 
> > wrote:
> >
> > > Nice catch! Though, wouldn't the memory sanitizer buildbots break on this 
> > > reliably?
> >
> >
> > I kinda hope so; i'll take a look at home.
>
>
> Memory sanitizer at home: //*fails with 1700 uninitialized reads in 
> TableGen*//.


Try to add a non-sanitizer built tablegen: 
`-DCLANG_TABLEGEN=/path/to/non/sanitized/clang-tblgen` 
`-DLLVM_TABLEGEN=/path/to/non/sanitized/llvm-tblgen`

> I guess i could commit test first, then get buildbots to fail, then if they 
> do commit the code, otherwise commit the code and multiply the run-lines.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69962



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

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

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

Szelethus added a comment.

Accepted accidentally. Well, in any case, I trust your judgement on this! I'd 
prefer not to commit the test file as-is.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69962



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

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D69938#1742026 , @Anastasia wrote:

> In D69938#1741713 , @rjmccall wrote:
>
> > In D69938#1741080 , @Anastasia 
> > wrote:
> >
> > > In D69938#1737196 , @rjmccall 
> > > wrote:
> > >
> > > > It does make logical sense to be able to qualify the call operator of a 
> > > > lambda.  The lambda always starts as a temporary, so presumably we want 
> > > > the default address space qualifier on lambdas to be compatible with 
> > > > the address space of temporaries.  However, that's probably also true 
> > > > of the default address qualifier on methods in general or you wouldn't 
> > > > be able to call them on temporaries and locals.  Thus, we should be 
> > > > able to use the same default in both cases, which seems to be what 
> > > > you've implemented.
> > > >
> > > > It even makes sense to be able to qualify a lambda with an address 
> > > > space that's not compatible with the address space of temporaries; that 
> > > > just means that the lambda has to be copied elsewhere before it can be 
> > > > invoked.
> > >
> > >
> > > Just to clarify... Do you mean we should be able to compile this example:
> > >
> > >   [&] __global {
> > > i++;
> > >   } ();
> > >   
> > >   
> > >
> > > Or should this result in a diagnostic about an addr space mismatch?
> >
> >
> > It should result in a diagnostic on the call.  But if you assigned that 
> > lambda into global memory (somehow) it should work.
>
>
> Ok, makes sense so something like this should compile fine:
>
>   __global auto glob = [&] __global { ... };


Right, a call to `glob()` should work in this case.  I don't know if `__global` 
would parse here without `()`, though; in the grammar, it looks like attributes 
and so on have to follow an explicit parameter clause.

>>> 3. Diagnose address space mismatch between variable decl and lambda expr 
>>> qualifier Example: __private auto  llambda = [&]() __local {i++;}; // error 
>>> no legal conversion b/w __private and __local
>>> 
>>>   I think 1 is covered by this patch. I would like to implement 2 as a 
>>> separate patch though to be able to commit fix for 1 earlier and unblock 
>>> some developers waiting for the fix. I think 3 already works and I will 
>>> just update the diagnostic in a separate patch too.
>> 
>> I agree that 3 should just fall out.  And yeah, implementing 2 as a separate 
>> patch should be fine.  Please make sure 3 is adequately tested in this patch.
> 
> Ok, since we can't qualify lambda expr with addr spaces yet there is not much 
> I can test at this point. `__constant` lambda variable seems to be the only 
> case with a diagnostic for OpenCL.

You can certainly copy a lambda type into a different address space, or 
construct a pointer to a qualified lambda type, e.g. `(*(__constant 
decltype(somelambda) *) nullptr)()`.

John.


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

https://reviews.llvm.org/D69938



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

[PATCH] D69393: [RFC][DebugInfo] emit user specified address_space in dwarf

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

For what it's worth, in our downstream fork of Clang we have added the ability 
for function types to possess an address space.

Though technically, even in our fork it is not possible to actually declare 
functions/function pointers with an address space; the target-specified AS is 
implicitly applied to all functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69393



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

[PATCH] D70063: clang/Modules: Error if ReadASTBlock does not find the main module

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM with some minor comment below!




Comment at: clang/include/clang/Basic/DiagnosticSerializationKinds.td:78
+def err_module_file_missing_definition : Error<
+  "module file '%0' is missing the main module's definition">, DefaultFatal;
 

dexonsmith wrote:
> bruno wrote:
> > aprantl wrote:
> > > Should this be `AST file` like in the above error message?
> > +1
> I think this should be `module file`, because it only applies to modules.  
> Notice that err_module_file_not_found and err_module_file_out_of_date use 
> `%select{PCH|module|AST} file`; this is just a constant-folding of that.
Makes sense!



Comment at: clang/lib/Serialization/ASTReader.cpp:5503
 
+F.DidReadMainModule = true;
 CurrentModule->setASTFile(F.File);

dexonsmith wrote:
> bruno wrote:
> > Is this enough? Because this is done at the end of `SUBMODULE_DEFINITION`, 
> > could it be that only some of the submodules were read (top level or not) 
> > and this is still going to be true? I wonder if marking this `true` at the 
> > end of successful `SUBMODULE_BLOCK` instead wouldn't be a better option.
> I see three cases to distinguish between:
> 
> 1. An error is encountered somewhere in the submodule block.
> 2. The submodule block is read without error, but the main module is not 
> seen/added.
> 3. The submodule block is read without error, and the main module is 
> seen/added.
> 
> Setting a flag to `true` at the end of a successful `SUBMODULE_BLOCK` would 
> help to distinguish between (1) and (2 or 3), but the return status already 
> tells us that, and `ReadAST` already deletes all the just-loaded modules in 
> that case.
> 
> This patch as-is helps to distinguish between (2) and (3) once the early 
> return for (1) has been avoided.
> 
> Does that make sense?  Or maybe I didn't follow your suggestion.
Oh, I see now! Thanks for the explanation, perhaps add this to the commit 
message? 


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

https://reviews.llvm.org/D70063



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

[PATCH] D69841: Target Ivy bridge on macOS Mojave and later

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added reviewers: Gerolf, dexonsmith.
arphaman added a comment.

@bob.wilson no longer works on llvm-project. I added Gerolf and Duncan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69841



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

In D67160#1704840 , @rsmith wrote:

> If overload resolution can't distinguish between overloads that perform a lax 
> vector conversions and those that do not, we should fix that in overload 
> resolution rather than papering over it with a change of per-target default


I'm a bit late coming back to this, but //now// I've remembered why I wanted 
this change, and why clang's normal overload resolution doesn't do quite what 
the MVE polymorphic intrinsics want.

I'm in the middle of writing implementations of the `vsetq_lane` family of 
intrinsics. These take a vector as input, a scalar of the same type as the 
vector lane, and a lane index, and returns a modified vector with the specified 
lane replaced by the scalar input value. In other words, the overloaded 
versions consist of things like

  int8x16_t vsetq_lane(int8_t, int8x16_t, int);
  int16x8_t vsetq_lane(int16_t, int16x8_t, int);
  int32x4_t vsetq_lane(int32_t, int32x4_t, int);

... and so on for unsigned and floating types too.
Now if the user calls

  myvector = vsetq_lane(23, myvector, 1);

then I think they reasonably expect the choice of polymorphic intrinsic to be 
based on the type of the //vector// parameter, because the integer literal 23 
in the scalar parameter could have been intended to be any of those integer 
types. But the strict rules of C say that it has type `int` (which in this case 
matches `int32_t`). So if you do this with `myvector` being (say) an 
`int8x16_t`, you get an error message complaining that the call is ambiguous: 
the vector parameter matches one of those prototypes, but the scalar parameter 
matches another, and neither one matches both.

Ideally I'd like to be able to configure this particular family of overloaded 
functions to give strictly higher priority to the vector type than the scalar 
type, in resolving this ambiguity. Turning off lax vector conversions has that 
effect, but I do agree with you that it would be better not to do it that way.

So, do you have any thoughts on a better approach? The only one I've so far 
thought of is to add extra spurious overloaded declarations in the header file 
for integer types that things might have accidentally been promoted to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:179
+Builder.CreateCall(FnDecl, Ident, "omp_global_thread_num");
+if (Instruction *IdentI = dyn_cast(Ident))
+  Call->moveAfter(IdentI);

`auto *`



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

Maybe add an assert when the cancellation version is requested but the 
cancellation block is not set? Instead of the generating simple version of 
barrier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D69922#1741659 , @jdoerfert wrote:

> Use IRBuilder for cancel barriers


In general, it would better to implement cancellation barriers in the separate 
patch.  Same for OpenMPIrBuilder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[clang] 83dcb34 - clang/Modules: Error if ReadASTBlock does not find the main module

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2019-11-12T08:40:53-08:00
New Revision: 83dcb34b6bf4c175040b18d3e8c3c468418009fc

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

LOG: clang/Modules: Error if ReadASTBlock does not find the main module

If ReadASTBlock does not find its top-level submodule, there's something
wrong the with the PCM.  Error in that case, to avoid hitting problems
further from the source.

Note that the Swift compiler sometimes hits a case in
CompilerInstance::loadModule where the top-level submodule mysteriously
does not have Module::IsFromModuleFile set.  That will emit a confusing
warn_missing_submodule, which was never intended for the main module.
The recent audit of error-handling in ReadAST may have rooted out the
real problem.  If not, this commit will help to clarify the real
problem, and replace a confusing warning with an error pointing at the
malformed PCM file.

We're specifically sniffing out whether the top-level submodule was
found/processed, in case there is a malformed module file that is
missing it.  If there is an error encountered during ReadSubmoduleBlock
the return status should already propagate through.  It would be nice to
detect other missing submodules around here to catch other instances of
warn_missing_submodule closer to the source, but that's left as a future
exercise.

https://reviews.llvm.org/D70063

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSerializationKinds.td
clang/include/clang/Serialization/Module.h
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td 
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 757dbbeee3cc..f46470c3 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -74,6 +74,8 @@ def note_module_file_imported_by : Note<
   "imported by %select{|module '%2' in }1'%0'">;
 def err_module_file_not_module : Error<
   "AST file '%0' was not built as a module">, DefaultFatal;
+def err_module_file_missing_top_level_submodule : Error<
+  "module file '%0' is missing its top-level submodule">, DefaultFatal;
 
 def remark_module_import : Remark<
   "importing module '%0'%select{| into '%3'}2 from '%1'">,

diff  --git a/clang/include/clang/Serialization/Module.h 
b/clang/include/clang/Serialization/Module.h
index 1979c53a7133..b5afdf90c6af 100644
--- a/clang/include/clang/Serialization/Module.h
+++ b/clang/include/clang/Serialization/Module.h
@@ -159,6 +159,9 @@ class ModuleFile {
   /// Whether the PCH has a corresponding object file.
   bool PCHHasObjectFile = false;
 
+  /// Whether the top-level module has been read from the AST file.
+  bool DidReadTopLevelSubmodule = false;
+
   /// The file entry for the module file.
   const FileEntry *File = nullptr;
 

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f7e3805fd04c..9111b60a7179 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4217,6 +4217,12 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef 
FileName,
 if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
   return removeModulesAndReturn(Result);
 
+// The AST block should always have a definition for the main module.
+if (F.isModule() && !F.DidReadTopLevelSubmodule) {
+  Error(diag::err_module_file_missing_top_level_submodule, F.FileName);
+  return removeModulesAndReturn(Failure);
+}
+
 // Read the extension blocks.
 while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
   if (ASTReadResult Result = ReadExtensionBlock(F))
@@ -5489,6 +5495,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned 
ClientLoadCapabilities) {
   }
 }
 
+F.DidReadTopLevelSubmodule = true;
 CurrentModule->setASTFile(F.File);
 CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
   }



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

[PATCH] D70063: clang/Modules: Error if ReadASTBlock does not find the main module

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith marked 2 inline comments as done.
dexonsmith added a comment.

Committed 83dcb34b6bf4c175040b18d3e8c3c468418009fc 
.  Thanks 
for the reviews!




Comment at: clang/lib/Serialization/ASTReader.cpp:5503
 
+F.DidReadMainModule = true;
 CurrentModule->setASTFile(F.File);

bruno wrote:
> dexonsmith wrote:
> > bruno wrote:
> > > Is this enough? Because this is done at the end of 
> > > `SUBMODULE_DEFINITION`, could it be that only some of the submodules were 
> > > read (top level or not) and this is still going to be true? I wonder if 
> > > marking this `true` at the end of successful `SUBMODULE_BLOCK` instead 
> > > wouldn't be a better option.
> > I see three cases to distinguish between:
> > 
> > 1. An error is encountered somewhere in the submodule block.
> > 2. The submodule block is read without error, but the main module is not 
> > seen/added.
> > 3. The submodule block is read without error, and the main module is 
> > seen/added.
> > 
> > Setting a flag to `true` at the end of a successful `SUBMODULE_BLOCK` would 
> > help to distinguish between (1) and (2 or 3), but the return status already 
> > tells us that, and `ReadAST` already deletes all the just-loaded modules in 
> > that case.
> > 
> > This patch as-is helps to distinguish between (2) and (3) once the early 
> > return for (1) has been avoided.
> > 
> > Does that make sense?  Or maybe I didn't follow your suggestion.
> Oh, I see now! Thanks for the explanation, perhaps add this to the commit 
> message? 
Added a comment there, and also reworded variables / diagnostics from talking 
about "main module" to talking about "top-level submodule", since I think that 
might be more consistent (and maybe more clear).


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

https://reviews.llvm.org/D70063



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);

I believe this problem exists in the original code? If so, better to split this 
patch and commit this fix in the separate patch.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16393-16416
+static DefaultMapImplicitBehavior
+getImplicitBehaviorFromModifier(OpenMPDefaultmapClauseModifier M) {
+  switch (M) {
+  case OMPC_DEFAULTMAP_MODIFIER_alloc:
+return DMIB_alloc;
+  case OMPC_DEFAULTMAP_MODIFIER_to:
+return DMIB_to;

cchen wrote:
> ABataev wrote:
> > ABataev wrote:
> > > Do we need types `DefaultMapImplicitBehavior` and 
> > > `DefaultMapVariableCategory` if the map 1 to 1 to 
> > > `OpenMPDefaultmapClauseModifier` and `OpenMPDefaultmapClauseKind`?
> > What about this?
> The value of OpenMPDefaultmapClauseModifier does not start from one (due to 
> the design in parsing I guess) so I'm not able to do so.
No, I meant that the enumerics are almost the same. Can we reuse the original 
one rather than adding 2 new enums with almost the same members and the same 
meanings?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3013-3020
+  IsFirstprivate = IsFirstprivate ||
+   (((DMVC == DMVC_pointer &&
+  Stack->mustBeFirstprivate(DMVC_pointer)) ||
+ (DMVC == DMVC_scalar &&
+  Stack->mustBeFirstprivate(DMVC_scalar)) ||
+ (DMVC == DMVC_aggregate &&
+  Stack->mustBeFirstprivate(DMVC_aggregate))) &&

`Stack->mustBeFirstprivate(DMVC)`?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4448
+  
DSAChecker.getImplicitMap(static_cast(I));
+  for (const auto &M : ImplicitMap)
+ImplicitMaps[I].emplace_back(M);

`ImplicitMaps[I].append(ImplicitMap.begin(), ImplicitMap.end());`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-template-align.cpp:8
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align:
+

Do we need to hardcode the target here? Could we check for the specific align 
value?



Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);
 

This should be `Optional` AlignInBits. Even better perhaps 
`llvm::Align` but perhaps that's too restrictive.



Comment at: llvm/include/llvm/IR/DIBuilder.h:244
+ uint32_t AlignInBits = 0);
 
 /// Create debugging information entry for a 'friend'.

Do you need to update the C bindings as well?



Comment at: llvm/test/DebugInfo/X86/debug-info-template-align.ll:1
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s

`RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s`


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

https://reviews.llvm.org/D70111



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

(The draft implementations of `vsetq_lane` are now in D70133 
.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: ostannard, MarkMurrayARM, dmgreen.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
simon_tatham added a parent revision: D70088: [ARM,MVE] Add intrinsics for 
contiguous load/stores..

This batch of intrinsics includes lots of things that move vector data
around or change its type without really affecting its value very
much. It includes the `vreinterpretq` family (cast one vector type to
another); `vuninitializedq` (create a vector of a given type with
don't-care contents); `vcreateq` (make a 128-bit vector out of two
`uint64_t` halves); and the `vgetq_lane` and `vsetq_lane` families, to
read and write an individual lane of a vector.

These are all implemented using completely standard IR that's already
tested in existing LLVM unit tests, so I've just written a clang test
to check the IR is correct, and left it at that.

One of the new `vgetq_lane` intrinsics returns a `float16_t`, which
causes a compile error if `%clang_cc1` doesn't get the option
`-fallow-half-arguments-and-returns`. The driver passes that option to
cc1 already, but I've had to edit all the explicit cc1 command lines
in the existing MVE intrinsics tests.

I've also added some richer infrastructure to the MveEmitter Tablegen
backend, to make it specify the exact integer type of integer
arguments passed to IR construction functions, and wrap those
arguments in a `static_cast` in the autogenerated C++. That was
necessary to prevent an overloading ambiguity when passing the integer
literal `0` to `IRBuilder::CreateInsertElement`, because otherwise, it
could mean either a null pointer `llvm::Value *` or a zero `uint64_t`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70133

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/admin.c
  clang/test/CodeGen/arm-mve-intrinsics/load-store.c
  clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
  clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
  clang/test/CodeGen/arm-mve-intrinsics/vadc.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c
  clang/test/CodeGen/arm-mve-intrinsics/vldr.c
  clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
  clang/test/Sema/arm-mve-immediates.c
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -632,10 +632,10 @@
   StringRef CallPrefix;
   std::vector Args;
   std::set AddressArgs;
-  std::set IntConstantArgs;
+  std::map IntConstantArgs;
   IRBuilderResult(StringRef CallPrefix, std::vector Args,
   std::set AddressArgs,
-  std::set IntConstantArgs)
+  std::map IntConstantArgs)
 : CallPrefix(CallPrefix), Args(Args), AddressArgs(AddressArgs),
 IntConstantArgs(IntConstantArgs) {}
   void genCode(raw_ostream &OS,
@@ -644,11 +644,13 @@
 const char *Sep = "";
 for (unsigned i = 0, e = Args.size(); i < e; ++i) {
   Ptr Arg = Args[i];
-  if (IntConstantArgs.find(i) != IntConstantArgs.end()) {
+  auto it = IntConstantArgs.find(i);
+  if (it != IntConstantArgs.end()) {
 assert(Arg->hasIntegerConstantValue());
-OS << Sep
+OS << Sep << "static_cast<" << it->second << ">("
<< ParamAlloc.allocParam("unsigned",
-utostr(Arg->integerConstantValue()));
+utostr(Arg->integerConstantValue()))
+   << ")";
   } else {
 OS << Sep << Arg->varname();
   }
@@ -763,6 +765,14 @@
   // shares with at least one other intrinsic.
   std::string ShortName, FullName;
 
+  // A very small number of intrinsics _only_ have a polymorphic
+  // variant (vuninitializedq taking an unevaluated argument).
+  bool PolymorphicOnly;
+
+  // Another rarely-used flag indicating that the builtin doesn't
+  // evaluate its argument(s) at all.
+  bool NonEvaluating;
+
   const Type *ReturnType;
   std::vector ArgTypes;
   std::map ImmediateArgs;
@@ -796,6 +806,8 @@
 return false;
   }
   bool polymorphic() const { return ShortName != FullName; }
+  bool polymorphicOnly() const { return PolymorphicOnly; }
+  bool nonEvaluating() const { return NonEvaluating; }
 
   // External entry point for code generation, called from MveEmitter.
   void genCode(raw_ostream &OS, CodeGenParamAllocator &ParamAlloc,
@@ -1126,11 +1138,15 @@
   Args.push_back(getCodeForDagArg(D, i, Scope, Param));
 if (Op->isSubClassOf("IRBuilderBase")) {
   std::set AddressArgs;
-  for (unsigned i : Op->getValueAsListOfInts("address_params"))
-AddressArgs.insert(i);
-  std::set IntConstantArgs;
-  for (unsigned i : Op->getValueAsLis

[PATCH] D70110: [Driver][FreeBSD] Enable unwind tables on !amd64

[Dimitry Andric via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70110



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

@dblaikie do you need more time to investigate? fwiw I didn't uncover any new 
failures in a stage2 build with this patch applied.


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

https://reviews.llvm.org/D69970



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);

ABataev wrote:
> I believe this problem exists in the original code? If so, better to split 
> this patch and commit this fix in the separate patch.
Yes, this problem is from the original code, but `bool isDefaultmapModifier = 
(M != OMPC_DEFAULTMAP_MODIFIER_unknown);` at line 16437 relies on this change. 
Otherwise, I'll have to write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || (M == 
OMPC_DEFAULTMAP_MODIFIER_from) ||...`.
So do you think that I should just write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || 
(M == OMPC_DEFAULTMAP_MODIFIER_from) ||...` or `M > 
OMPC_DEFAULTMAP_MODIFIER_unknown` for this patch and fix them in another patch?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16393-16416
+static DefaultMapImplicitBehavior
+getImplicitBehaviorFromModifier(OpenMPDefaultmapClauseModifier M) {
+  switch (M) {
+  case OMPC_DEFAULTMAP_MODIFIER_alloc:
+return DMIB_alloc;
+  case OMPC_DEFAULTMAP_MODIFIER_to:
+return DMIB_to;

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > Do we need types `DefaultMapImplicitBehavior` and 
> > > > `DefaultMapVariableCategory` if the map 1 to 1 to 
> > > > `OpenMPDefaultmapClauseModifier` and `OpenMPDefaultmapClauseKind`?
> > > What about this?
> > The value of OpenMPDefaultmapClauseModifier does not start from one (due to 
> > the design in parsing I guess) so I'm not able to do so.
> No, I meant that the enumerics are almost the same. Can we reuse the original 
> one rather than adding 2 new enums with almost the same members and the same 
> meanings?
Got it, I'll use OpenMPDefaultmapClauseModifier and OpenMPDefaultmapClauseKind 
instead. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[clang] 3c676e3 - [OPENMP]Use copy constructors instead of assignment operators in declare

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-11-12T13:13:37-05:00
New Revision: 3c676e3891b962b859e7613781419ee0dacce7dd

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

LOG: [OPENMP]Use copy constructors instead of assignment operators in declare
reduction initializers.

Better to use copy constructor at the initialization of the declare
reduction construct rather than assignment operator.

Added: 


Modified: 
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/AST/dump.cpp
clang/test/OpenMP/declare_reduction_messages.cpp
clang/test/OpenMP/for_reduction_codegen_UDR.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 8430e72d3c88..49ba52897fe6 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -368,14 +368,8 @@ 
Parser::ParseOpenMPDeclareReductionDirective(AccessSpecifier AS) {
 // Check if initializer is omp_priv  or something else.
 if (Tok.is(tok::identifier) &&
 Tok.getIdentifierInfo()->isStr("omp_priv")) {
-  if (Actions.getLangOpts().CPlusPlus) {
-InitializerResult = Actions.ActOnFinishFullExpr(
-ParseAssignmentExpression().get(), D->getLocation(),
-/*DiscardedValue*/ false);
-  } else {
-ConsumeToken();
-ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
-  }
+  ConsumeToken();
+  ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
 } else {
   InitializerResult = Actions.ActOnFinishFullExpr(
   ParseAssignmentExpression().get(), D->getLocation(),
@@ -419,7 +413,8 @@ void Parser::ParseOpenMPReductionInitializerForDecl(VarDecl 
*OmpPrivParm) {
   return;
 }
 
-ExprResult Init(ParseInitializer());
+PreferredType.enterVariableInit(Tok.getLocation(), OmpPrivParm);
+ExprResult Init = ParseAssignmentExpression();
 
 if (Init.isInvalid()) {
   SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch);

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index cd8b95231aad..600265e2d852 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3118,7 +3118,12 @@ Decl 
*TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
 SubstInitializer =
 SemaRef.SubstExpr(D->getInitializer(), TemplateArgs).get();
   } else {
-IsCorrect = IsCorrect && OmpPrivParm->hasInit();
+auto *OldPrivParm =
+cast(cast(D->getInitPriv())->getDecl());
+IsCorrect = IsCorrect && OldPrivParm->hasInit();
+if (IsCorrect)
+  SemaRef.InstantiateVariableInitializer(OmpPrivParm, OldPrivParm,
+ TemplateArgs);
   }
   SemaRef.ActOnOpenMPDeclareReductionInitializerEnd(
   NewDRD, SubstInitializer, OmpPrivParm);

diff  --git a/clang/test/AST/dump.cpp b/clang/test/AST/dump.cpp
index 641abc5ea646..4d271514349f 100644
--- a/clang/test/AST/dump.cpp
+++ b/clang/test/AST/dump.cpp
@@ -33,13 +33,11 @@ int ga, gb;
 // CHECK-NEXT: | | |-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_out' 'float'
 // CHECK-NEXT: | | `-ImplicitCastExpr {{.+}}  'float' 
 // CHECK-NEXT: | |   `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_in' 'float'
-// CHECK-NEXT: | |-BinaryOperator {{.+}}  'float' lvalue '='
-// CHECK-NEXT: | | |-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_priv' 'float'
-// CHECK-NEXT: | | `-BinaryOperator {{.+}}  'float' '+'
-// CHECK-NEXT: | |   |-ImplicitCastExpr {{.+}}  'float' 

-// CHECK-NEXT: | |   | `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_orig' 'float'
-// CHECK-NEXT: | |   `-ImplicitCastExpr {{.+}}  'float' 

-// CHECK-NEXT: | | `-IntegerLiteral {{.+}}  'int' 15
+// CHECK-NEXT: | |-BinaryOperator {{.+}}  'float' '+'
+// CHECK-NEXT: | | |-ImplicitCastExpr {{.+}}  'float' 
+// CHECK-NEXT: | | | `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_orig' 'float'
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.+}}  'float' 

+// CHECK-NEXT: | |   `-IntegerLiteral {{.+}}  'int' 15
 
 struct S {
   int a, b;

diff  --git a/clang/test/OpenMP/declare_reduction_messages.cpp 
b/clang/test/OpenMP/declare_reduction_messages.cpp
index a674af7151da..1fc5ec6e683f 100644
--- a/clang/test/OpenMP/declare_reduction_messages.cpp
+++ b/clang/test/OpenMP/declare_reduction_messages.cpp
@@ -142,7 +142,7 @@ int main() {
 #if __cplusplus == 201103L
 struct A {
   A() {}
-  A& operator=(A&&) = default;
+  A(const A &) = default;
 };
 
 int A_TEST() {

diff  --git a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp 
b/clang/test/O

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+  ClangDriverMain = Driver::Main;

Why is this check necessary? Why not assuming that if `Driver::Main` is set, it 
will be the right one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);

cchen wrote:
> ABataev wrote:
> > I believe this problem exists in the original code? If so, better to split 
> > this patch and commit this fix in the separate patch.
> Yes, this problem is from the original code, but `bool isDefaultmapModifier = 
> (M != OMPC_DEFAULTMAP_MODIFIER_unknown);` at line 16437 relies on this 
> change. Otherwise, I'll have to write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || 
> (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...`.
> So do you think that I should just write `(M == OMPC_DEFAULTMAP_MODIFIER_to) 
> || (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...` or `M > 
> OMPC_DEFAULTMAP_MODIFIER_unknown` for this patch and fix them in another 
> patch?
You will just commit the second patch at first and then just rebase this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69832: [WebAssembly] -fwasm-exceptions enables reference-types

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69832



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

Does this change crash recovery semantics in any meaningful way?  Will we still 
be able to get stack traces on all platforms when the compiler crashes?




Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:207
+  // FIXME error: cannot compile this 'this' captured by SEH yet
+  CrashRecoveryContext *This = this;
   __try {

You can fix this by writing:

```
static bool wrap_function_call(function_ref Fn, bool 
EnableExceptionHandler, unsigned &RetCode)
{
   __try {
 Fn();
 return true;
  } __except (EnableExceptionHandler
  ? LLVMUnhandledExceptionFilter(GetExceptionInformation())
  : 1) {
RetCode = GetExceptionCode();
return false;
  }
}

bool CrashRecoveryContext::RunSafely(function_ref Fn) {
...
bool Result = wrap_function_call(EnableExceptionHandler, Fn, RetCode);
...
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69841: Target Ivy bridge on macOS Mojave and later

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

Hi Dave, thanks for checking in on this, but unfortunately we're not ready for 
this yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69841



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

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

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 228924.
hliao added a comment.

This patch is revived with more changes addressing the previous concerns.

Back to Justin's example:

  __host__ float bar();
  __device__ int bar();
  __host__ __device__ auto foo() -> decltype(bar()) { return bar(); }

Even without this patch, that example already passed the compilation without
either errors or warnings. Says

  clang -std=c++11 -x cuda -nocudainc -nocudalib --cuda-gpu-arch=sm_60 
--cuda-device-only -S -emit-llvm -O3 foo.cu

In c++14, that example could be even simplified without `decltype` but the same 
ambiguity.

  __host__ float bar();
  __device__ int bar();
  __host__ __device__ auto foo() { return bar(); }

Without any change, clang also compiles the code as well and uses different 
return types between host-side and device-side compilation.[^1]

[^1]: The first example has the same return type between host-side and 
device-side but that seems incorrect or unreasonable to me.

The ambiguity issue is in fact not introduced by relaxing `decltype`. That's an 
inherent one as we allow overloading over target attributes. Issuing warnings 
instead of errors seems more reasonable to me for such cases.

In this patch, besides relaxing the CUDA call rule under `decltype`, it also 
generates warning during function overloading if there are more than candidates 
with different return types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCUDA/function-overload.cu
  clang/test/Misc/warning-flags.c
  clang/test/SemaCUDA/function-overload.cu

Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -3,6 +3,8 @@
 
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
 
 #include "Inputs/cuda.h"
 
@@ -419,3 +421,30 @@
 int test_constexpr_overload(C2 &x, C2 &y) {
   return constexpr_overload(x, y);
 }
+
+#if defined(CHECK_DECLTYPE)
+#if defined(__CUDA_ARCH__)
+// expected-note@+6 {{other definition of 't0'}}
+// expected-note@+6 {{use this definition of 't0'}}
+#else
+// expected-note@+3 {{use this definition of 't0'}}
+// expected-note@+3 {{other definition of 't0'}}
+#endif
+__host__ float t0();
+__device__ int t0();
+
+__host__ __device__ void dt0() {
+  // expected-warning@+1 {{return type of 't0' in 'decltype' is ambiguous and may not be expected}}
+  decltype(t0()) ret;
+}
+
+__host__ float t1();
+
+__device__ void dt1() {
+  decltype(t1()) ret; // OK. `decltype` is relaxed.
+}
+
+__host__ __device__ void dt2() {
+  decltype(t1()) ret; // OK. `decltype` is relaxed.
+}
+#endif
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (75):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -47,6 +47,7 @@
 CHECK-NEXT:   warn_conv_to_base_not_used
 CHECK-NEXT:   warn_conv_to_self_not_used
 CHECK-NEXT:   warn_conv_to_void_not_used
+CHECK-NEXT:   warn_decltype_ambiguous_return_type
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/test/CodeGenCUDA/function-overload.cu
===
--- clang/test/CodeGenCUDA/function-overload.cu
+++ clang/test/CodeGenCUDA/function-overload.cu
@@ -8,6 +8,8 @@
 // RUN: | FileCheck -check-prefix=CHECK-BOTH -check-prefix=CHECK-HOST %s
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -emit-llvm -o - %s \
 // RUN: | FileCheck -check-prefix=CHECK-BOTH -check-prefix=CHECK-DEVICE %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple amdgcn -fcuda-is-device -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefix=CHECK-DECLTYPE %s
 
 #include "Inputs/cuda.h"
 
@@ -53,3 +55,14 @@
 // CHECK-BOTH: define linkonce_odr void @_ZN7s_cd_hdD2Ev(
 // CHECK-BOTH: store i32 32,
 // CHECK-BOTH: ret void
+
+#if defined(CHECK_DECLTYPE)
+int foo(float);
+// CHECK-DECLTYPE-LABEL: @_Z3barf
+// CHECK-DECLTYPE: fptosi
+// CHECK-DECLTYPE: sitofp
+__dev

[clang] 2149028 - [AST] Use an explicit copy in a range-based for

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2019-11-12T20:47:46+01:00
New Revision: 2149028c49f8af1f3d8a9d81b2081a2b302b2d9a

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

LOG: [AST] Use an explicit copy in a range-based for

The AssociationIteratorTy type will be copied in a range-based for loop.
Make the copy explicit to avoid the -Wrange-loop-analysis warning.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/StmtDataCollectors.td
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaPseudoObject.cpp
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 0bb2aad553fb..ed9fc14aba42 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -620,7 +620,7 @@ class ASTNodeTraverser
 Visit(E->getControllingExpr());
 Visit(E->getControllingExpr()->getType()); // FIXME: remove
 
-for (const auto &Assoc : E->associations()) {
+for (const auto Assoc : E->associations()) {
   Visit(Assoc);
 }
   }

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 86059842da65..9fea4980c89a 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2351,7 +2351,7 @@ bool RecursiveASTVisitor::TraverseInitListExpr(
 // generic associations).
 DEF_TRAVERSE_STMT(GenericSelectionExpr, {
   TRY_TO(TraverseStmt(S->getControllingExpr()));
-  for (const GenericSelectionExpr::Association &Assoc : S->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : S->associations()) {
 if (TypeSourceInfo *TSI = Assoc.getTypeSourceInfo())
   TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
 TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(Assoc.getAssociationExpr());

diff  --git a/clang/include/clang/AST/StmtDataCollectors.td 
b/clang/include/clang/AST/StmtDataCollectors.td
index a46d2714eb4b..7cb9f16fbce2 100644
--- a/clang/include/clang/AST/StmtDataCollectors.td
+++ b/clang/include/clang/AST/StmtDataCollectors.td
@@ -189,7 +189,7 @@ class CXXFoldExpr {
 }
 class GenericSelectionExpr {
   code Code = [{
-for (const GenericSelectionExpr::ConstAssociation &Assoc : 
S->associations()) {
+for (const GenericSelectionExpr::ConstAssociation Assoc : 
S->associations()) {
   addData(Assoc.getType());
 }
   }];

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 0f92d4c367e9..603ae5f9c48d 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -1304,7 +1304,7 @@ void 
StmtPrinter::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *Node){
 void StmtPrinter::VisitGenericSelectionExpr(GenericSelectionExpr *Node) {
   OS << "_Generic(";
   PrintExpr(Node->getControllingExpr());
-  for (const GenericSelectionExpr::Association &Assoc : Node->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : Node->associations()) {
 OS << ", ";
 QualType T = Assoc.getType();
 if (T.isNull())

diff  --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 6f266cf12949..9edd9e5d5856 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -1296,7 +1296,7 @@ void StmtProfiler::VisitBlockExpr(const BlockExpr *S) {
 
 void StmtProfiler::VisitGenericSelectionExpr(const GenericSelectionExpr *S) {
   VisitExpr(S);
-  for (const GenericSelectionExpr::ConstAssociation &Assoc :
+  for (const GenericSelectionExpr::ConstAssociation Assoc :
S->associations()) {
 QualType T = Assoc.getType();
 if (T.isNull())

diff  --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp
index 207812c8848b..fc27094c9a53 100644
--- a/clang/lib/Sema/SemaExprObjC.cpp
+++ b/clang/lib/Sema/SemaExprObjC.cpp
@@ -4353,7 +4353,7 @@ Expr *Sema::stripARCUnbridgedCast(Expr *e) {
 SmallVector subTypes;
 subExprs.reserve(n);
 subTypes.reserve(n);
-for (const GenericSelectionExpr::Association &assoc : gse->associations()) 
{
+for (const GenericSelectionExpr::Association assoc : gse->associations()) {
   subTypes.push_back(assoc.getTypeSourceInfo());
   Expr *sub = assoc.getAssociationExpr();
   if (assoc.isSelected())

diff  --git a/clang/lib/Sema/SemaPseudoObject.cpp 
b/clang/lib/Sema/SemaPseudoObject.cpp
index 602806968ced..5587e0d24c7f 100644
--- a/clang/lib/Sema/SemaPseudoObject.cpp
+++ b/clang/

[clang] 51abceb - [OpenMP] Use an explicit copy in a range-based for

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2019-11-12T20:50:38+01:00
New Revision: 51abcebbb6e5c8f8befaa523ae873adecf2d1012

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

LOG: [OpenMP] Use an explicit copy in a range-based for

The std::pair>
type will be copied in a range-based for loop. Make the copy explicit to
avoid the -Wrange-loop-analysis warning.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index ae881c3e2d7a..cd20cb83afb9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@ class MappableExprsHandler {
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@ class MappableExprsHandler {
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@ class MappableExprsHandler {
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto &L : MC->component_lists()) {
+  for (const auto L : MC->component_lists()) {
 InfoGen(L.first, L.second, MC->getMapType(), MC->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, MC->isImplicit());
   }
@@ -8288,7 +8288,7 @@ class MappableExprsHandler {
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->decl_component_lists(VD)) {
+  for (const auto L : C->decl_component_lists(VD)) {
 assert(L.first == VD &&
"We got information for the wrong declaration??");
 assert(!L.second.empty() &&
@@ -8441,7 +8441,7 @@ class MappableExprsHandler {
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 if (!L.first)
   continue;
 const auto *VD = dyn_cast(L.first);



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

[PATCH] D70046: [OpenMP] Use an explicit copy in a range-based for

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51abcebbb6e5: [OpenMP] Use an explicit copy in a range-based 
for (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70046

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto &L : MC->component_lists()) {
+  for (const auto L : MC->component_lists()) {
 InfoGen(L.first, L.second, MC->getMapType(), MC->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, MC->isImplicit());
   }
@@ -8288,7 +8288,7 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->decl_component_lists(VD)) {
+  for (const auto L : C->decl_component_lists(VD)) {
 assert(L.first == VD &&
"We got information for the wrong declaration??");
 assert(!L.second.empty() &&
@@ -8441,7 +8441,7 @@
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 if (!L.first)
   continue;
 const auto *VD = dyn_cast(L.first);


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto &L : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto &L : MC->component_lists()) {
+  for (const auto L : MC->compon

[PATCH] D70045: [AST] Use an explicit copy in a range-based for

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2149028c49f8: [AST] Use an explicit copy in a range-based 
for (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70045

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtDataCollectors.td
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9380,7 +9380,7 @@
 
   SmallVector AssocExprs;
   SmallVector AssocTypes;
-  for (const GenericSelectionExpr::Association &Assoc : E->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : E->associations()) {
 TypeSourceInfo *TSI = Assoc.getTypeSourceInfo();
 if (TSI) {
   TypeSourceInfo *AssocType = getDerived().TransformType(TSI);
Index: clang/lib/Sema/SemaPseudoObject.cpp
===
--- clang/lib/Sema/SemaPseudoObject.cpp
+++ clang/lib/Sema/SemaPseudoObject.cpp
@@ -145,7 +145,7 @@
 assocExprs.reserve(numAssocs);
 assocTypes.reserve(numAssocs);
 
-for (const GenericSelectionExpr::Association &assoc :
+for (const GenericSelectionExpr::Association assoc :
  gse->associations()) {
   Expr *assocExpr = assoc.getAssociationExpr();
   if (assoc.isSelected())
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -4353,7 +4353,7 @@
 SmallVector subTypes;
 subExprs.reserve(n);
 subTypes.reserve(n);
-for (const GenericSelectionExpr::Association &assoc : gse->associations()) {
+for (const GenericSelectionExpr::Association assoc : gse->associations()) {
   subTypes.push_back(assoc.getTypeSourceInfo());
   Expr *sub = assoc.getAssociationExpr();
   if (assoc.isSelected())
Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1296,7 +1296,7 @@
 
 void StmtProfiler::VisitGenericSelectionExpr(const GenericSelectionExpr *S) {
   VisitExpr(S);
-  for (const GenericSelectionExpr::ConstAssociation &Assoc :
+  for (const GenericSelectionExpr::ConstAssociation Assoc :
S->associations()) {
 QualType T = Assoc.getType();
 if (T.isNull())
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1304,7 +1304,7 @@
 void StmtPrinter::VisitGenericSelectionExpr(GenericSelectionExpr *Node) {
   OS << "_Generic(";
   PrintExpr(Node->getControllingExpr());
-  for (const GenericSelectionExpr::Association &Assoc : Node->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : Node->associations()) {
 OS << ", ";
 QualType T = Assoc.getType();
 if (T.isNull())
Index: clang/include/clang/AST/StmtDataCollectors.td
===
--- clang/include/clang/AST/StmtDataCollectors.td
+++ clang/include/clang/AST/StmtDataCollectors.td
@@ -189,7 +189,7 @@
 }
 class GenericSelectionExpr {
   code Code = [{
-for (const GenericSelectionExpr::ConstAssociation &Assoc : S->associations()) {
+for (const GenericSelectionExpr::ConstAssociation Assoc : S->associations()) {
   addData(Assoc.getType());
 }
   }];
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2351,7 +2351,7 @@
 // generic associations).
 DEF_TRAVERSE_STMT(GenericSelectionExpr, {
   TRY_TO(TraverseStmt(S->getControllingExpr()));
-  for (const GenericSelectionExpr::Association &Assoc : S->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : S->associations()) {
 if (TypeSourceInfo *TSI = Assoc.getTypeSourceInfo())
   TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
 TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(Assoc.getAssociationExpr());
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -620,7 +620,7 @@
 Visit(E->getControllingExpr());
 Visit(E->getControllingExpr()->getType()); // FIXME: remove
 
-for (const auto &Assoc : E->associations()) {
+for (const auto Assoc : E->asso

[clang] 9648428 - [Analyzer] Use a reference in a range-based for

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2019-11-12T20:53:08+01:00
New Revision: 964842861c8acd53b8df8799f7c3800c5528fb72

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

LOG: [Analyzer] Use a reference in a range-based for

Let the checkers use a reference instead of a copy in a range-based
for loop.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 38a9aaf72c27..45ab652d9e02 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -567,7 +567,7 @@ class CheckerManager {
 if (I == Events.end())
   return;
 const EventInfo &info = I->second;
-for (const auto Checker : info.Checkers)
+for (const auto &Checker : info.Checkers)
   Checker(&event);
   }
 

diff  --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp 
b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index f676bd895283..a9361837cf68 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -91,7 +91,7 @@ void CheckerManager::runCheckersOnASTDecl(const Decl *D, 
AnalysisManager& mgr,
   }
 
   assert(checkers);
-  for (const auto checker : *checkers)
+  for (const auto &checker : *checkers)
 checker(D, mgr, BR);
 }
 
@@ -99,7 +99,7 @@ void CheckerManager::runCheckersOnASTBody(const Decl *D, 
AnalysisManager& mgr,
   BugReporter &BR) {
   assert(D && D->hasBody());
 
-  for (const auto BodyChecker : BodyCheckers)
+  for (const auto &BodyChecker : BodyCheckers)
 BodyChecker(D, mgr, BR);
 }
 
@@ -402,7 +402,7 @@ void CheckerManager::runCheckersForBind(ExplodedNodeSet 
&Dst,
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph &G,
BugReporter &BR,
ExprEngine &Eng) {
-  for (const auto EndAnalysisChecker : EndAnalysisCheckers)
+  for (const auto &EndAnalysisChecker : EndAnalysisCheckers)
 EndAnalysisChecker(G, BR, Eng);
 }
 
@@ -455,7 +455,7 @@ void 
CheckerManager::runCheckersForEndFunction(NodeBuilderContext &BC,
   // creates a successor for Pred, we do not need to generate an
   // autotransition for it.
   NodeBuilder Bldr(Pred, Dst, BC);
-  for (const auto checkFn : EndFunctionCheckers) {
+  for (const auto &checkFn : EndFunctionCheckers) {
 const ProgramPoint &L =
 FunctionExitPoint(RS, Pred->getLocationContext(), checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
@@ -542,7 +542,7 @@ void CheckerManager::runCheckersForNewAllocator(
 /// Run checkers for live symbols.
 void CheckerManager::runCheckersForLiveSymbols(ProgramStateRef state,
SymbolReaper &SymReaper) {
-  for (const auto LiveSymbolsChecker : LiveSymbolsCheckers)
+  for (const auto &LiveSymbolsChecker : LiveSymbolsCheckers)
 LiveSymbolsChecker(state, SymReaper);
 }
 
@@ -599,7 +599,7 @@ CheckerManager::runCheckersForRegionChanges(ProgramStateRef 
state,
 ArrayRef 
Regions,
 const LocationContext *LCtx,
 const CallEvent *Call) {
-  for (const auto RegionChangesChecker : RegionChangesCheckers) {
+  for (const auto &RegionChangesChecker : RegionChangesCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -621,7 +621,7 @@ CheckerManager::runCheckersForPointerEscape(ProgramStateRef 
State,
   (Kind != PSK_DirectEscapeOnCall &&
Kind != PSK_IndirectEscapeOnCall)) &&
  "Call must not be NULL when escaping on call");
-  for (const auto PointerEscapeChecker : PointerEscapeCheckers) {
+  for (const auto &PointerEscapeChecker : PointerEscapeCheckers) {
 // If any checker declares the state infeasible (or if it starts that
 //  way), bail out.
 if (!State)
@@ -635,7 +635,7 @@ CheckerManager::runCheckersForPointerEscape(ProgramStateRef 
State,
 ProgramStateRef
 CheckerManager::runCheckersForEvalAssume(ProgramStateRef state,
  SVal Cond, bool Assumption) {
-  for (const auto EvalAssumeChecker : EvalAssumeCheckers) {
+  for (const auto &EvalAssumeChecker : EvalAssumeCheckers) {
 // If any checker declares the state infeasible (or if it starts t

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

@aaron.ballman good catch, I've fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70047



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added a comment.

In D69825#1742276 , @hans wrote:

> Instead of invoking main() (or a similar function like ClangDriverMain) as an 
> alternative to spinning up the cc1 process, would it be possible to call 
> ExecuteCC1Tool() directly? I guess it would be a little less general, but 
> maybe it would be more straight-forward?


Possibly, yes, I'll have to check. There might be a few edge cases where we 
re-enter `ClangDriverMain()` twice if I recall correctly - I'll check.

In D69825#1742621 , @zturner wrote:

> Does this change crash recovery semantics in any meaningful way?  Will we 
> still be able to get stack traces on all platforms when the compiler crashes?


Yes, the changes in `CrashRecoveryContext` allow the same side-effects as the 
global exception handler. You would see the callstack and minidump in the same 
way.

There's still a minimal risk of memory/heap/CRT corruption, even with 
`CrashRecoveryContext` (after a crash). But that would possibly affect 
`LLVMUnhandledExceptionFilter` as well, and right now (//without this patch//), 
that handler runs in the context of the **crashed process **(not the calling 
one) so it wouldn't be worse than what it is now.

I could write a follow-up patch to prepare //on startup// the cmd-line invoked 
by `Driver::generateCompilationDiagnostics()`, instead of preparing after a 
crash. We could also pre-allocate a few virtual pages on advance, and use that 
in a BumpAllocator, instead of allocating after the crash. We could also merge 
the feature of `llvm-symbolizer` into `clang.exe`, so that we could call-back 
into `clang.exe` to render the callstack on stdout, even if `llvm-symbolizer` 
is not present.

The only drawback I see now is that we don't get the coredump on Linux, because 
the program doesn't end through the kernel signal handler. However, given that 
@Meinersbur says he sees no improvement on his side, we could disable this 
optimization on non-win? (or hide it behind a disabled flag on non-win)




Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+  ClangDriverMain = Driver::Main;

Meinersbur wrote:
> Why is this check necessary? Why not assuming that if `Driver::Main` is set, 
> it will be the right one?
The driver builds phases that do not always call the cc1 process. Simply 
stating `clang a.cpp` would invoke `clang -cc1`, then the linker. In the later 
case, even if we have `Driver::Main` it doesn't mean we should use it. There 
are a number of other edge cases of the same kind, such as `/fallback` or build 
cuda files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG964842861c8a: [Analyzer] Use a reference in a range-based 
for (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D70047?vs=228582&id=228935#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70047

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -91,7 +91,7 @@
   }
 
   assert(checkers);
-  for (const auto checker : *checkers)
+  for (const auto &checker : *checkers)
 checker(D, mgr, BR);
 }
 
@@ -99,7 +99,7 @@
   BugReporter &BR) {
   assert(D && D->hasBody());
 
-  for (const auto BodyChecker : BodyCheckers)
+  for (const auto &BodyChecker : BodyCheckers)
 BodyChecker(D, mgr, BR);
 }
 
@@ -402,7 +402,7 @@
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph &G,
BugReporter &BR,
ExprEngine &Eng) {
-  for (const auto EndAnalysisChecker : EndAnalysisCheckers)
+  for (const auto &EndAnalysisChecker : EndAnalysisCheckers)
 EndAnalysisChecker(G, BR, Eng);
 }
 
@@ -455,7 +455,7 @@
   // creates a successor for Pred, we do not need to generate an
   // autotransition for it.
   NodeBuilder Bldr(Pred, Dst, BC);
-  for (const auto checkFn : EndFunctionCheckers) {
+  for (const auto &checkFn : EndFunctionCheckers) {
 const ProgramPoint &L =
 FunctionExitPoint(RS, Pred->getLocationContext(), checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
@@ -542,7 +542,7 @@
 /// Run checkers for live symbols.
 void CheckerManager::runCheckersForLiveSymbols(ProgramStateRef state,
SymbolReaper &SymReaper) {
-  for (const auto LiveSymbolsChecker : LiveSymbolsCheckers)
+  for (const auto &LiveSymbolsChecker : LiveSymbolsCheckers)
 LiveSymbolsChecker(state, SymReaper);
 }
 
@@ -599,7 +599,7 @@
 ArrayRef Regions,
 const LocationContext *LCtx,
 const CallEvent *Call) {
-  for (const auto RegionChangesChecker : RegionChangesCheckers) {
+  for (const auto &RegionChangesChecker : RegionChangesCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -621,7 +621,7 @@
   (Kind != PSK_DirectEscapeOnCall &&
Kind != PSK_IndirectEscapeOnCall)) &&
  "Call must not be NULL when escaping on call");
-  for (const auto PointerEscapeChecker : PointerEscapeCheckers) {
+  for (const auto &PointerEscapeChecker : PointerEscapeCheckers) {
 // If any checker declares the state infeasible (or if it starts that
 //  way), bail out.
 if (!State)
@@ -635,7 +635,7 @@
 ProgramStateRef
 CheckerManager::runCheckersForEvalAssume(ProgramStateRef state,
  SVal Cond, bool Assumption) {
-  for (const auto EvalAssumeChecker : EvalAssumeCheckers) {
+  for (const auto &EvalAssumeChecker : EvalAssumeCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -658,7 +658,7 @@
 NodeBuilder B(Pred, checkDst, Eng.getBuilderContext());
 
 // Check if any of the EvalCall callbacks can evaluate the call.
-for (const auto EvalCallChecker : EvalCallCheckers) {
+for (const auto &EvalCallChecker : EvalCallCheckers) {
   // TODO: Support the situation when the call doesn't correspond
   // to any Expr.
   ProgramPoint L = ProgramPoint::getProgramPoint(
@@ -697,7 +697,7 @@
   const TranslationUnitDecl *TU,
   AnalysisManager &mgr,
   BugReporter &BR) {
-  for (const auto EndOfTranslationUnitChecker : EndOfTranslationUnitCheckers)
+  for (const auto &EndOfTranslationUnitChecker : EndOfTranslationUnitCheckers)
 EndOfTranslationUnitChecker(TU, mgr, BR);
 }
 
@@ -904,6 +904,6 @@
 }
 
 CheckerManager::~CheckerManager() {
-  for (const auto CheckerDtor : CheckerDtors)
+  for (const auto &CheckerDtor : CheckerDtors)
 CheckerDtor();
 }
Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -567,7 +567,7 @@
 if (I == Events.end())
   retu

[PATCH] D69598: Work on cleaning up denormal mode handling

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.

Thanks. I understand your direction for denormal handling now, and I'm OK with 
this patch apart from the remaining references to subnormal that Sanjay 
mentioned.

In D69598#1739723 , @arsenm wrote:

> I do think the floating point environment bits should be a considered a 
> property of the calling convention, with attributes that override them. A 
> function which calls a function with a different mode would be responsible 
> for switching the mode before the call. This would require people actually 
> caring about getting this right to really implement


Do you mean the compiler should insert code to restore the FP environment on 
function transitions? Or do you mean that the function itself (i.e. the user's 
code) is responsible for switching the mode? I have some reservations about 
this, but I think the C standard specification for FENV_ACCESS is clear that 
the it is the programmer's responsibility to manage the floating point 
environment correctly. Yes, that's a sure recipe for broken code, but that's 
what it says. Obviously LLVM IR is not bound by the C standard and we could 
take a different approach, but I have concerns about the performance 
implications because in general the compiler won't know when the environment 
needs to be restored so it would have to take a conservative approach.

I've been meaning to write some documentation on the expected behavior at 
function boundaries of the FP environment. Perhaps we can continue this 
discussion there.


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

https://reviews.llvm.org/D69598



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

[PATCH] D6920: [clang-format] Add SpaceBeforeBrackets

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I realize this is an old diff, and you and I have just spoke about it on 
twitter https://twitter.com/NIV_Anteru/status/1193792307386081281?s=20 would 
you consider rebasing as the /brief style isn't used any more

It would help the approval process if you could highlight a public project with 
a style guide that uses this style.

I think this is a simple enough addition to consider by getting it upto date 
will help




Comment at: include/clang/Format/Format.h:415
+  bool SpaceBeforeSquareBrackets;
+
   /// \brief Supported language standards.

when you rebase you'll see examples of /codeblocks with before and after I 
think that would really help


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6920



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Yeah, I'm OK with warning on the deprecated cases (especially for symmetry with 
GCC's warning) - not sure why that didn't occur to me/why I didn't mention it 
in my previous comment...

Deprecation wording is, in C++17, 15.8.1 [class.copy.ctor] p6, and 15.8.2 
[class.copy.assign] p2.


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

https://reviews.llvm.org/D68185



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

[PATCH] D70139: [clangd] Add bool return type to Index::refs API.

[Haojian Wu via Phabricator via cfe-commits]

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

Similar to fuzzyFind, the bool indicates whether there are more xref
results.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70139

Files:
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2161,9 +2161,10 @@
 TEST(FindReferences, NoQueryForLocalSymbols) {
   struct RecordingIndex : public MemIndex {
 mutable Optional> RefIDs;
-void refs(const RefsRequest &Req,
+bool refs(const RefsRequest &Req,
   llvm::function_ref) const override {
   RefIDs = Req.IDs;
+  return false;
 }
   };
 
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -389,7 +389,8 @@
   RefsRequest Request;
   Request.IDs = {Foo.ID};
   RefSlab::Builder Results;
-  Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); });
+  EXPECT_FALSE(
+  Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); }));
   EXPECT_THAT(
   std::move(Results).build(),
   ElementsAre(Pair(
@@ -400,7 +401,8 @@
 
   Request.Limit = 1;
   RefSlab::Builder Results2;
-  Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); });
+  EXPECT_TRUE(
+  Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); }));
   EXPECT_THAT(std::move(Results2).build(),
   ElementsAre(Pair(
   _, ElementsAre(AnyOf(FileURI("unittest:///test.cc"),
Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -687,14 +687,18 @@
   Req.Filter = RefKind::Declaration | RefKind::Definition;
 
   std::vector Files;
-  Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
-  .refs(Req, [&](const Ref &R) { Files.push_back(R.Location.FileURI); });
+  EXPECT_FALSE(Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
+   .refs(Req, [&](const Ref &R) {
+ Files.push_back(R.Location.FileURI);
+   }));
   EXPECT_THAT(Files, UnorderedElementsAre("foo.h", "foo.cc"));
 
   Req.Limit = 1;
   Files.clear();
-  Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
-  .refs(Req, [&](const Ref &R) { Files.push_back(R.Location.FileURI); });
+  EXPECT_TRUE(Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
+  .refs(Req, [&](const Ref &R) {
+Files.push_back(R.Location.FileURI);
+  }));
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1193,8 +1193,10 @@
   void lookup(const LookupRequest &,
   llvm::function_ref) const override {}
 
-  void refs(const RefsRequest &,
-llvm::function_ref) const override {}
+  bool refs(const RefsRequest &,
+llvm::function_ref) const override {
+return false;
+  }
 
   void relations(const RelationsRequest &,
  llvm::function_ref)
Index: clang-tools-extra/clangd/index/dex/Dex.h
===
--- clang-tools-extra/clangd/index/dex/Dex.h
+++ clang-tools-extra/clangd/index/dex/Dex.h
@@ -77,7 +77,7 @@
   void lookup(const LookupRequest &Req,
   llvm::function_ref Callback) const override;
 
-  void refs(const RefsRequest &Req,
+  bool refs(const RefsRequest &Req,
 llvm::function_ref Callback) const override;
 
   void relations(const RelationsRequest &Req,
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -249,18 +249,26 @@
   }
 }
 

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

sorry searching through old issues, are you still interested in this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54628



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

[PATCH] D70139: [clangd] Add bool return type to Index::refs API.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59972 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70139



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

[PATCH] D69935: [DeclCXX] Remove unknown external linkage specifications

[David Blaikie via Phabricator via cfe-commits]

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

Looks good - thanks!


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

https://reviews.llvm.org/D69935



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

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

xbolva00 added a comment.

Ah, Clang has it under -Wdeprecated. 
https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated.

But combo -Wall -Wextra does not enable this warning, sadly.

This should be extracted to -Wdeprecated-copy and put into -Wextra. Seems fine 
for you?


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

https://reviews.llvm.org/D68185



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

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 228948.
hokein marked 4 inline comments as done.
hokein added a comment.

- make rename only work when the position is on the name range of the decl
- add more testsj


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -38,20 +38,20 @@
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
   // Rename function.
   R"cpp(
-void [[foo]]() {
+void [[foo^]]() {
   [[fo^o]]();
 }
   )cpp",
 
   // Rename type.
   R"cpp(
-struct [[foo]]{};
+struct [[foo^]]{};
 [[foo]] test() {
[[f^oo]] x;
return x;
@@ -66,18 +66,313 @@
   }
 }
   )cpp",
+
+  R"cpp(
+class [[F^oo]] {};
+template  void func() {}
+template  class Baz {};
+int main() {
+  func<[[F^oo]]>();
+  Baz<[[F^oo]]> obj;
+  return 0;
+}
+  )cpp",
+
+  // class simple rename
+  R"cpp(
+class [[F^oo]] {
+  void foo(int x);
+};
+void [[Foo]]::foo(int x) {}
+  )cpp",
+
+  // class constructor/destructor.
+  R"cpp(
+class [[^Foo]] {
+  [[F^oo]]();
+  ~[[Foo]]();
+};
+  )cpp",
+
+  // class overrides
+  R"cpp(
+struct A {
+ virtual void [[f^oo]]() {}
+};
+struct B : A {
+  void [[f^oo]]() override {}
+};
+struct C : B {
+  void [[f^oo]]() override {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  B().[[f^oo]]();
+  C().[[f^oo]]();
+}
+  )cpp",
+
+  // complicated class type
+  R"cpp(
+ // Forward declaration.
+class [[Fo^o]];
+class Baz {
+  virtual int getValue() const = 0;
+};
+
+class [[F^oo]] : public Baz  {
+public:
+  [[Foo]](int value = 0) : x(value) {}
+
+  [[Foo]] &operator++(int);
+
+  bool operator<([[Foo]] const &rhs);
+  int getValue() const;
+private:
+  int x;
+};
+
+void func() {
+  [[Foo]] *Pointer = 0;
+  [[Foo]] Variable = [[Foo]](10);
+  for ([[Foo]] it; it < Variable; it++);
+  const [[Foo]] *C = new [[Foo]]();
+  const_cast<[[Foo]] *>(C)->getValue();
+  [[Foo]] foo;
+  const Baz &BazReference = foo;
+  const Baz *BazPointer = &foo;
+  dynamic_cast(BazReference).getValue();
+  dynamic_cast(BazPointer)->getValue();
+  reinterpret_cast(BazPointer)->getValue();
+  static_cast(BazReference).getValue();
+  static_cast(BazPointer)->getValue();
+}
+  )cpp",
+
+  // class constructors
+  R"cpp(
+class [[^Foo]] {
+public:
+  [[Foo]]();
+};
+
+[[Foo]]::[[Fo^o]]() {}
+  )cpp",
+
+  // constructor initializer list
+  R"cpp(
+class Baz {};
+class Qux {
+  Baz [[F^oo]];
+public:
+  Qux();
+};
+// FIXME: selection tree on initializer list Foo below will returns
+// CXXConstructExpr node, which ends up with renaming class "Baze"
+// instead of "Foo".
+Qux::Qux() : [[Foo]]() {}
+  )cpp",
+
+  // DeclRefExpr
+  R"cpp(
+class C {
+public:
+  static int [[F^oo]];
+};
+
+int foo(int x) { return 0; }
+#define MACRO(a) foo(a)
+
+void func() {
+  C::[[F^oo]] = 1;
+  MACRO(C::[[Foo]]);
+  int y = C::[[F^oo]];
+}
+  )cpp",
+
+  // Forward declaration
+  R"cpp(
+class [[F^oo]];
+[[Foo]] *f();
+  )cpp",
+
+  // function marco
+  R"cpp(
+#define moo [[foo]]
+int [[fo^o]]() { return 42; }
+void boo(int value) {}
+
+void qoo() {
+  [[foo]]();
+  boo([[foo]]());
+  moo();
+  boo(moo());
+}
+  )cpp",
+
+  // MemberExpr in macro
+  R"cpp(
+class Baz {
+public:
+  int [[F^oo]];
+};
+int qux(int x) { return 0; }
+#define MACRO(a) qux(a)
+
+int main() {
+  Baz baz;
+  baz.[[Foo]] = 1;
+  MACR

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:326
+// class Foo, but the token under the cursor is not corresponding to the
+// "Foo" range, though the final result is correct.
 SourceLocation Loc = getBeginningOfIdentifier(

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > I would argue rename should not work in that case.
> > > Could we check that the cursor is actually on the name token of the 
> > > `NamedDecl` and not rename if it isn't?
> > you are probably right, we only allow rename which is triggered on the name 
> > token. Will update the patch.
> Definitely think we should do it before landing the patch.
> Otherwise we'll introduce regressions.
Done. 



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:225
   tooling::Replacements FilteredChanges;
-  for (const tooling::SymbolOccurrence &Rename :
-   findOccurrencesWithinFile(AST, RenameDecl)) {
-// Currently, we only support normal rename (one range) for C/C++.
-// FIXME: support multiple-range rename for objective-c methods.
-if (Rename.getNameRanges().size() > 1)
-  continue;
-// We shouldn't have conflicting replacements. If there are conflicts, it
-// means that we have bugs either in clangd or in Rename library, therefore
-// we refuse to perform the rename.
+  for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
+// We shouldn't have conflicting replacements or replacements from 
different

ilya-biryukov wrote:
> This seems to assume all occurrences are outside macros.
> Won't it break in presence of macros?
> Do we have tests when the renamed token is:
> - inside macro body
> - inside macro arguments
> ?
if Loc is a macro location, tooling::Replacement will use the spelling 
location, so if the spelling location is in the main file, the code is correct, 
we have test cases for it.

but we will fail on cases like below, we should fix this, note that this issue 
exists even before this patch. Updated the comment here.

```
// foo.h
#define MACRO foo

// foo.cc
void f() {
  int fo^o = 2;
  MACRO;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D68185#1742809 , @xbolva00 wrote:

> Ah, Clang has it under -Wdeprecated. 
> https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated.
>
> But combo -Wall -Wextra does not enable this warning, sadly.
>
> This should be extracted to -Wdeprecated-copy and put into -Wextra. Seems 
> fine for you?


Sounds plausible to me - though I'd expect @rsmith should probably have a final 
say before that's submitted.


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

https://reviews.llvm.org/D68185



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

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59990 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[clang] 335ac2e - Allow additional file suffixes/extensions considered as source in main include grouping

[via cfe-commits]

Author: mydeveloperday
Date: 2019-11-12T21:26:52Z
New Revision: 335ac2eb662ce5f1888e2a50310b01fba2d40d68

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

LOG: Allow additional file suffixes/extensions considered as source in main 
include grouping

Summary:
By additional regex match, grouping of main include can be enabled in files 
that are not normally considered as a C/C++ source code.
For example, this might be useful in templated code, where template 
implementations are being held in *Impl.hpp files.
On the occassion, 'assume-filename' option description was reworded as it was 
misleading. It has nothing to do with `style=file` option and it does not 
influence sourced style filename.

Reviewers: rsmith, ioeric, krasimir, sylvestre.ledru, MyDeveloperDay

Reviewed By: MyDeveloperDay

Subscribers: MyDeveloperDay, cfe-commits

Patch by:  furdyna

Tags: #clang

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

Added: 


Modified: 
clang/docs/ClangFormat.rst
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/include/clang/Tooling/Inclusions/IncludeStyle.h
clang/lib/Format/Format.cpp
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/tools/clang-format/ClangFormat.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortIncludesTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index 7fe050ec9e29..1138b2332670 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -31,9 +31,9 @@ to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.
   Clang-format options:
 
 --Werror   - If set, changes formatting warnings to errors
---assume-filename= - When reading from stdin, clang-format assumes 
this
- filename to look for a style config file (with
- -style=file) and to determine the language.
+--assume-filename= - Override filename used to determine the 
language.
+ When reading from stdin, clang-format assumes 
this
+ filename to determine the language.
 --cursor=- The position of the cursor when invoking
  clang-format from an editor integration
 --dry-run  - If set, do not actually make the formatting 
changes

diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index e0ad96532136..1a9c5b8395f4 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1581,6 +1581,26 @@ the configuration (without a prefix: ``Auto``).
   For example, if configured to "(_test)?$", then a header a.h would be seen
   as the "main" include in both a.cc and a_test.cc.
 
+**IncludeIsMainSourceRegex** (``std::string``)
+  Specify a regular expression for files being formatted
+  that are allowed to be considered "main" in the
+  file-to-main-include mapping.
+
+  By default, clang-format considers files as "main" only when they end
+  with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``, ``.m`` or ``.mm``
+  extensions.
+  For these files a guessing of "main" include takes place
+  (to assign category 0, see above). This config option allows for
+  additional suffixes and extensions for files to be considered as "main".
+
+  For example, if this option is configured to ``(Impl\.hpp)$``,
+  then a file ``ClassImpl.hpp`` is considered "main" (in addition to
+  ``Class.c``, ``Class.cc``, ``Class.cpp`` and so on) and "main
+  include file" logic will be executed (with *IncludeIsMainRegex* setting
+  also being respected in later phase). Without this option set,
+  ``ClassImpl.hpp`` would not have the main include file put on top
+  before any other include.
+
 **IndentCaseLabels** (``bool``)
   Indent case labels one level from the switch statement.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3db1603e0631..1139116ed101 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -292,6 +292,24 @@ clang-format
 - clang-format gets a new option called ``--dry-run`` or ``-n`` to emit a
   warning.
 
+- Option *IncludeIsMainSourceRegex* added to allow for additional
+  suffixes and file extensions to be considered as a source file
+  for execution of logic that looks for "main *include* file" to put
+  it on top.
+
+  By default, clang-format considers *source* files as "main" only when
+  they end with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``,
+  ``.m`` or ``.mm`` extensions. This config option allows to
+  extend this set of source files considered as "main".
+

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/OpenMPKinds.def:186
+///{
+
+#ifndef OMP_IDENT_FLAG

JonChesterfield wrote:
> Meinersbur wrote:
> > JonChesterfield wrote:
> > > jdoerfert wrote:
> > > > Meinersbur wrote:
> > > > > jdoerfert wrote:
> > > > > > JonChesterfield wrote:
> > > > > > > Sharing constants between the compiler and the runtime is an 
> > > > > > > interesting subproblem. I think the cleanest solution is the one 
> > > > > > > used by libc, where the compiler generates header files 
> > > > > > > containing the constants which the runtime library includes.
> > > > > > I'd prefer not to tackle this right now but get this done first and 
> > > > > > revisit the issue later. OK?
> > > > > I don't think this is a good solution. It means that libomp cannot 
> > > > > built built anymore without also building clang. Moreover, the values 
> > > > > cannot be changed anyway since it would break the ABI.
> > > > > 
> > > > > I'd go the other route: The libomp defines what it's ABI is, the 
> > > > > compiler generates code for it. 
> > > > This patch doesn't change what we do, just where. The numbers are hard 
> > > > coded in clang now. Let's start a discussion on the list and if we come 
> > > > up with a different scheme we do it after this landed.
> > > Revisit later sounds good.
> > > 
> > > @Meinersbur Do you know of an example of a non-llvm compiler using this 
> > > libomp?
> > > 
> > > The usual order is build a compiler, then use it to build the runtime 
> > > libraries, then the whole package can build other stuff. Provided the 
> > > compiler doesn't need any of the runtime libraries (compiler-rt, maths 
> > > libraries, libomp etc) itself the system bootstraps cleanly. Especially 
> > > important when cross compiling and I suspect the gpu targets in openmp 
> > > have similarly strict requirements on the first compiler.
> > > 
> > > Closely related to that, I tend to assume that the runtime libraries can 
> > > be rewritten to best serve their only client - the associated compiler - 
> > > so if libomp is used by out of tree compilers I'd like to know who we are 
> > > at risk of breaking.
> > > Do you know of an example of a non-llvm compiler using this libomp?
> > 
> > [[ 
> > https://github.com/llvm-project/llvm-project/blob/master/openmp/runtime/src/kmp_gsupport.cpp
> >  | gcc  ]] (using libomp's gomp compatibility layer), [[ 
> > https://www.openmprtl.org/ | icc  ]] (as libomp was initially donated by 
> > Intel).
> > 
> > I don't understand why it even matters if there are other compilers using 
> > libomp. Every LLVM runtime library can be built stand-alone. 
> > With constant values being determined during compiler bootstrapping, 
> > programs built on one computer would be potentially ABI-incompatible with a 
> > runtime library on another. Think about updating your 
> > compiler-rt/libomp/libc++ on you computer causing all existing binaries on 
> > the system to crash because constants changed in the updated compiler's 
> > bootstrapping process.
> > 
> > The only use case I know that does this is are operating system's syscall 
> > tables. Linux's reference is [[ 
> > https://github.com/torvalds/linux/blob/master/arch/sh/include/uapi/asm/unistd_64.h
> >  | unistd.h ]] which is platform-specific and Windows generates the table 
> > during its [[ https://j00ru.vexillium.org/syscalls/nt/64/ | build process 
> > ]]. Therefore on Windows, system calls can only be done through ntdll. Even 
> > on Linux one should use the system's libc instead of directly invoking a 
> > system call.
> Thanks. GCC and ICC would presumably be happier with the magic numbers stored 
> with openmp then (though with the move to a monorepo that's a little less 
> persuasive).
> 
> When constants that affect the ABI change, the result won't work with 
> existing software regardless of whether the compiler or the library contains 
> the change. Either the new compiler builds things that don't work with the 
> old library, or the new library doesn't work with things built by the old 
> compiler. The two have to agree on the ABI.
> 
> At present, openmp does the moral equivalent of #include OpenMPKinds.def from 
> clang. Moving the constants to libomp means clang will do the equivalent of 
> #include OpenMPKinds.def from openmp. Breaking that dependency means making a 
> new subproject that just holds/generates the constants, that both depend on, 
> which seems more hassle than it's worth. 
> 
> I'd like to generate this header as part of the clang build (though 
> ultimately don't care that much if it's generated as part of the openmp 
> build) because it's going to become increasingly challenging to read as 
> non-nvptx architectures are introduced. Likewise it would be useful to 
> generate the interface.h for deviceRTL (or equivalently a set of unit tests 
> checking the function types) from the same sour

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
cchen retitled this revision from "[OpenMP5.0] Fix a parsing issue for the 
extended defaultmap

This patch is for fixing a possible parsing issue in the OpenMP5.0 defaultmap 
patch.

In this case: "#pragma omp target defaultmap(scalar:", the implicit-behavior 
after the parsing will..." to "[OpenMP5.0] Fix a parsing issue for the extended 
defaultmap".
cchen edited the summary of this revision.
cchen removed a subscriber: guansong.

This patch is for fixing a possible parsing issue in the OpenMP5.0 defaultmap 
patch.
In this case:

  #pragma omp target defaultmap(scalar:

the implicit-behavior after the parsing will not be 
OMPC_DEFAULTMAP_MODIFIER_unknown.
So we need to either enforce the enum value of implicit behavior to be unknown 
or modify the logic inside getOpenMPSimpleClauseType.
And we now choose to just enforce the enum value of implicit behavior to be 
unknown.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70142

Files:
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2310,8 +2310,14 @@
   DelimLoc = ConsumeAnyToken();
   } else if (Kind == OMPC_defaultmap) {
 // Get a defaultmap modifier
-Arg.push_back(getOpenMPSimpleClauseType(
-Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok)));
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer (modifier should be either alloc, to, from, tofrom, 
firstprivate,
+// none, or default)
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);
 KLoc.push_back(Tok.getLocation());
 if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::comma) &&
 Tok.isNot(tok::annot_pragma_openmp_end))


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2310,8 +2310,14 @@
   DelimLoc = ConsumeAnyToken();
   } else if (Kind == OMPC_defaultmap) {
 // Get a defaultmap modifier
-Arg.push_back(getOpenMPSimpleClauseType(
-Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok)));
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer (modifier should be either alloc, to, from, tofrom, firstprivate,
+// none, or default)
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);
 KLoc.push_back(Tok.getLocation());
 if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::comma) &&
 Tok.isNot(tok::annot_pragma_openmp_end))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

[Dimitry Andric via Phabricator via cfe-commits]

dim created this revision.
dim added reviewers: xbolva00, spatel, jdoerfert, efriedma.
Herald added subscribers: cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

This fixes PR43081, where the transformation of `strchr(p, 0) -> p +
strlen(p)` can cause a segfault, if `-fno-builtin-strlen` is used.  In
that case, `emitStrLen` returns nullptr, which CreateGEP is not designed
to handle.  Also add the minimized code from the PR as a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70143

Files:
  clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp


Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -363,9 +363,11 @@
   // a string literal.  If so, we can constant fold.
   StringRef Str;
   if (!getConstantStringInfo(SrcStr, Str)) {
-if (CharC->isZero()) // strchr(p, 0) -> p + strlen(p)
-  return B.CreateGEP(B.getInt8Ty(), SrcStr, emitStrLen(SrcStr, B, DL, TLI),
- "strchr");
+if (CharC->isZero()) { // strchr(p, 0) -> p + strlen(p)
+  Value *StrLen = emitStrLen(SrcStr, B, DL, TLI);
+  return StrLen ? B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr")
+: nullptr;
+}
 return nullptr;
   }
 
Index: clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-- -S -O1 -fno-builtin-strlen %s -o - | 
FileCheck %s
+char *strchr(const char *, int);
+char *b(char *a) {
+  return strchr(a, '\0');
+// CHECK: jmp  strchr
+}


Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -363,9 +363,11 @@
   // a string literal.  If so, we can constant fold.
   StringRef Str;
   if (!getConstantStringInfo(SrcStr, Str)) {
-if (CharC->isZero()) // strchr(p, 0) -> p + strlen(p)
-  return B.CreateGEP(B.getInt8Ty(), SrcStr, emitStrLen(SrcStr, B, DL, TLI),
- "strchr");
+if (CharC->isZero()) { // strchr(p, 0) -> p + strlen(p)
+  Value *StrLen = emitStrLen(SrcStr, B, DL, TLI);
+  return StrLen ? B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr")
+: nullptr;
+}
 return nullptr;
   }
 
Index: clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-- -S -O1 -fno-builtin-strlen %s -o - | FileCheck %s
+char *strchr(const char *, int);
+char *b(char *a) {
+  return strchr(a, '\0');
+// CHECK: jmp	strchr
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70142



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

[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG335ac2eb662c: Allow additional file suffixes/extensions 
considered as source in main include… (authored by MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D67750?vs=228872&id=228953#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67750

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -466,6 +466,57 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, LeavesMainHeaderFirstInAdditionalExtensions) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?|(Impl)?$";
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+
+  // .cpp extension is considered "main" by default
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.cpp"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.cpp"));
+
+  // Allow additional filenames / extensions
+  Style.IncludeIsMainSourceRegex = "(Impl\\.hpp)|(\\.xxx)$";
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+}
+
 TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12847,6 +12847,8 @@
   IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
   "abc$");
+  CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
+  IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
   Style.RawStringFormats.clear();
   std::vector ExpectedRawStringFormats = {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -75,9 +75,9 @@
 
 static cl::opt AssumeFileName(
 "assume-filename",
-cl::desc("When reading from stdin, clang-format assumes this\n"
- "filename to look for a style config file (with\n"
- "-style=file) and to determine the language."),
+cl::desc("Override filename used to determine the language.\n"
+ "When reading from stdin, clang-format assumes this\n"
+ "filename to determine the language."),
 cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt Inplace("i",
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -184,6 +184,10 @@
FileName.endswith(".cpp") || FileName.endswith(".c++") ||
FileName.endswith(".cxx") || FileName.endswith(".m") ||
FileName.endswith(".mm");
+  if (!Style.IncludeIsMainSourceRe

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This should have a llvm ir test in `llvm/test/transforms/instcombine` i think, 
not a clang test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > Maybe add an assert when the cancellation version is requested but the 
> > cancellation block is not set? Instead of the generating simple version of 
> > barrier.
> The interface doesn't work that way as we do not know here if the 
> cancellation was requested except if the block was set. That is basically the 
> flag (and I expect it to continue to be that way).
Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if it 
set to true, always emit cancel barrier, otherwise always emit simple barrier? 
And add an assertion for non-set cancellation block or even accept it as a 
parameter here.

Also, what if we have inner exception handling in the region? Will you handle 
the cleanup correctly in case of the cancelation barrier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

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

xbolva00 added a comment.

Yes, i will prepare a new patch and add him as a reviewer.


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

https://reviews.llvm.org/D68185



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor added a comment.

A similar PR has been submitted to the `vscode-cpptools` repo to enable 
debugging CUDA files via `cuda-gdb` with the official Microsoft VSCode C++ 
plugin: https://github.com/microsoft/vscode-cpptools/pull/4585


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > Maybe add an assert when the cancellation version is requested but the 
> > > cancellation block is not set? Instead of the generating simple version 
> > > of barrier.
> > The interface doesn't work that way as we do not know here if the 
> > cancellation was requested except if the block was set. That is basically 
> > the flag (and I expect it to continue to be that way).
> Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if 
> it set to true, always emit cancel barrier, otherwise always emit simple 
> barrier? And add an assertion for non-set cancellation block or even accept 
> it as a parameter here.
> 
> Also, what if we have inner exception handling in the region? Will you handle 
> the cleanup correctly in case of the cancelation barrier?
> Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if it 
> set to true, always emit cancel barrier, otherwise always emit simple 
> barrier? And add an assertion for non-set cancellation block or even accept 
> it as a parameter here.

What is the difference in moving some of the boolean logic to the caller? Also, 
we have test to verify we get cancellation barriers if we need them, both unit 
tests and clang lit tests.


> Also, what if we have inner exception handling in the region? Will you handle 
> the cleanup correctly in case of the cancelation barrier?

I think so. Right now through the code in clang that does the set up of the 
cancellation block, later through callbacks but we only need that for regions 
where we actually go out of some scope, e.g., parallel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: malcolm.parsons, angelgarcia, aaron.ballman.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

`modernize-use-equals-default` replaces default constructors/destructors with 
`= default;`. When the optional semicolon after a member function 
 is present, this results in 
two consecutive semicolons.

This small patch checks to see if the next non-comment token after the code to 
be replaced is a semicolon, and if so offers a replacement of `= default` 
rather than `= default;`.

This patch adds trailing comments and semicolons to about 5 existing tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70144

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
   ~OL();
 };
 
-OL::OL() {}
+OL::OL() {};
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a 
trivial default constructor [modernize-use-equals-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
@@ -17,9 +17,9 @@
 // Inline definitions.
 class IL {
 public:
-  IL() {}
+  IL() {}   ; // Note embedded tab on this line
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: IL() = default;
+  // CHECK-FIXES: IL() = default; // Note embedded tab on this line
   ~IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
 // Default member initializer
 class DMI {
 public:
-  DMI() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: DMI() = default;
+  DMI() {} // Comment before semi-colon on next line
+  ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+  // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+  // CHECK-FIXES-NEXT:   ;
   int Field = 5;
 };
 
 // Class member
 class CM {
 public:
-  CM() {}
+  CM() {} /* Comments */ /* before */ /* semicolon */;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: CM() = default;
+  // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
   OL o;
 };
 
@@ -66,7 +68,7 @@
   Priv() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Priv() = default;
-  ~Priv() {}
+  ~Priv() {};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~Priv() = default;
 };
Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
 struct BF {
   BF() = default;
   BF(const BF &Other) : Field1(Other.Field1), Field2(Other.Field2), 
Field3(Other.Field3),
-Field4(Other.Field4) {}
+Field4(Other.Field4) {};
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   // CHECK-FIXES: BF(const BF &Other) {{$}}
   // CHECK-FIXES: = default;
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -302,9 +302,19 @@
   auto Diag = diag(Location, "use '= default' to define a trivial " +
  SpecialFunctionName);
 
-  if (ApplyFix)
-Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+  if (ApplyFix) {
+// Peek ahead to see if there's a semicolon after Body->getSourceRange()
+Optional Token;
+do {
+  Token = Lexer::findNextToken(
+  Body->getSourceRange().getEnd().getLocWithOffset(1),
+  Result.Context->getSourceManager(), Result.Context->getLangOpts());
+} while (Token && Token->is(tok::comment));
+StringRef Replacement =
+Token && Token->is(tok::semi) ? "= default" : "= default;";
+Diag << FixItHint::CreateReplacement(Body->getSourceRange(), Replacement)
  << RemoveInitializers;
+  }
 }
 
 } // namespace modernize


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
--- clang-tools-e

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

In D70143#1742885 , @lebedev.ri wrote:

> This should have a llvm ir test in `llvm/test/transforms/instcombine` i 
> think, not a clang test.


Yes, I thought so too, but I could not get it to work (i.e. crash) with LLVM 
IR.  I just don't understand how `opt` works, and it does not have a 
`-fno-builtin-strlen` option either.  Therefore, I made it work with clang, as 
having a working test is better than no test at all.  But I'm fine with leaving 
out the test, it was just for completeness' sake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > Maybe add an assert when the cancellation version is requested but the 
> > > > cancellation block is not set? Instead of the generating simple version 
> > > > of barrier.
> > > The interface doesn't work that way as we do not know here if the 
> > > cancellation was requested except if the block was set. That is basically 
> > > the flag (and I expect it to continue to be that way).
> > Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if 
> > it set to true, always emit cancel barrier, otherwise always emit simple 
> > barrier? And add an assertion for non-set cancellation block or even accept 
> > it as a parameter here.
> > 
> > Also, what if we have inner exception handling in the region? Will you 
> > handle the cleanup correctly in case of the cancelation barrier?
> > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if it 
> > set to true, always emit cancel barrier, otherwise always emit simple 
> > barrier? And add an assertion for non-set cancellation block or even accept 
> > it as a parameter here.
> 
> What is the difference in moving some of the boolean logic to the caller? 
> Also, we have test to verify we get cancellation barriers if we need them, 
> both unit tests and clang lit tests.
> 
> 
> > Also, what if we have inner exception handling in the region? Will you 
> > handle the cleanup correctly in case of the cancelation barrier?
> 
> I think so. Right now through the code in clang that does the set up of the 
> cancellation block, later through callbacks but we only need that for regions 
> where we actually go out of some scope, e.g., parallel.
1. I'm just thinking about future users of thus interface. It woild be good if 
we could provide safe interface for all the users, not only clang.
2. Exit out of the OpenMP region is not allowed. But you may have inner 
try...catch or just simple compound statement with local vars that require 
constructors/destructors. And the cancellation barrier may exit out of these 
regions. And you need to call all required destructors. You'd better to think 
about it now, not later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 228959.
nridge added a comment.

Add unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto &LineTokens : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -589,6 +589,33 @@
   R"cpp(
   void $Function[[foo]]();
   using ::$Function[[foo]];
+)cpp",
+  // Inactive code highlighting
+  R"cpp(
+  // FIXME: In the preamble, no inactive code highlightings are produced.
+  #ifdef $Macro[[test]]
+  #endif
+
+  // A declaration to cause the preamble to end.
+  int $Variable[[EndPreamble]];
+
+  // Code after the preamble.
+  // Inactive lines get an empty InactiveCode token at the beginning.
+  // Code inside inactive blocks does not get regular highlightings
+  // because it's not part of the AST.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive1;
+$InactiveCode[[]]  #endif
+
+  #ifndef $Macro[[test]]
+  int $Variable[[Active1]];
+  #endif
+
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive2;
+$InactiveCode[[]]  #else
+  int $Variable[[Active2]];
+  #endif
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
@@ -656,10 +683,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -44,7 +44,11 @@
   Primitive,
   Macro,
 
-  LastKind = Macro
+  // This one is different from the other kinds as it's a line style
+  // rather than a token st

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:143
+for (const SourceRange &R :
+ AST.getPreprocessor().getPreprocessingRecord()->getSkippedRanges()) {
+  if (isInsideMainFile(R.getBegin(), SM)) {

hokein wrote:
> I think the current implementation only collects the skipped preprocessing 
> ranges **after preamble** region in the main file.  We probably need to store 
> these ranges in PreambleData to make the ranges in the preamble region work, 
> no need to do it in this patch, but we'd better have a test reflecting this 
> fact. 
> 
> ```
> #ifdef ActiveCOde
> // inactive code here
> #endif
> 
> #include "foo.h"
> // preamble ends here
> 
> namespace ns {
> // ..
> }  
> ```
Your observation is correct: the current implementation only highlights 
inactive lines after the preamble. For now, I added a test case with a FIXME 
for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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