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

2019-04-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!
Can you document this in `HeaderSearch.h`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995



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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments.



Comment at: llvm/trunk/unittests/IR/ConstantRangeTest.cpp:398-401
+#if defined(__GNUC__) && __GNUC__ >= 7
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)&HaveInterrupt3;
+#endif

tstellar wrote:
> Same here, no compiler specific ifdefs.
Just doing an unconditional `(void)HaveInterrupt3;` should be fine here. This 
warning is likely not specific to GCC 7 anyway.


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

https://reviews.llvm.org/D61046



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


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

2019-04-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 196392.
kadircet added a comment.

- Update documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995

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


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will be valid for
+  /// include-directive.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will be valid for
+  /// include-directive.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search 
directory
   /// paths that are relative.


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will be valid for
+  /// include-directive.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will be valid for
+  /// include-directive.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search directory
   /// paths that are relative.
___
cfe-commi

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

2019-04-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:710
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will be valid for
+  /// include-directive.

I don't think this is specific enough - on windows, backslashes are valid (or 
at least accepted by clang).
I'd suggest specifically mentioning forward slashes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995



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


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

2019-04-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 196393.
kadircet added a comment.

- Update comments to focus on forward slashes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60995

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


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search 
directory
   /// paths that are relative.


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain forward
+  /// slashes as separators.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search directory
   /// paths tha

Re: r359048 - C++ DR2387: a variable template declared wtih (or instantiated with) a

2019-04-24 Thread Ilya Biryukov via cfe-commits
Hi Richard,

This seems to break libc++, found while doing an integrate. The errors are:

In file included from valarray:4:
.../include/c++/v1/valarray:1062:60: error: explicit instantiation
declaration of 'valarray<_Tp>' with internal linkage
_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t))
   ^
.../include/c++/v1/valarray:1063:60: error: explicit instantiation
declaration of '~valarray<_Tp>' with internal linkage
_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::~valarray())
   ^

I will likely revert the change to unbreak the integrate.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359075 - [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

2019-04-24 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Wed Apr 24 01:45:03 2019
New Revision: 359075

URL: http://llvm.org/viewvc/llvm-project?rev=359075&view=rev
Log:
[clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

Reviewers: sammccall

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

Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=359075&r1=359074&r2=359075&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Apr 24 01:45:03 2019
@@ -706,16 +706,18 @@ public:
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search 
directory
   /// paths that are relative.

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=359075&r1=359074&r2=359075&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Apr 24 01:45:03 2019
@@ -1720,5 +1720,5 @@ std::string HeaderSearch::suggestPathToF
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }

Modified: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/HeaderSearchTest.cpp?rev=359075&r1=359074&r2=359075&view=diff
==
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp (original)
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp Wed Apr 24 01:45:03 2019
@@ -91,5 +91,14 @@ TEST_F(HeaderSearchTest, Dots) {
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang


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


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

2019-04-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359075: [clang][HeaderSearch] Make sure there are no 
backslashes in suggestedPath (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60995?vs=196393&id=196396#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60995

Files:
  cfe/trunk/include/clang/Lex/HeaderSearch.h
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/unittests/Lex/HeaderSearchTest.cpp


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: cfe/trunk/include/clang/Lex/HeaderSearch.h
===
--- cfe/trunk/include/clang/Lex/HeaderSearch.h
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain 
forward
+  /// slashes as separators.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search 
directory
   /// paths that are relative.


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1720,5 +1720,5 @@
 
   if (IsSystem)
 *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -91,5 +91,14 @@
 "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/""),
+"z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang
Index: cfe/trunk/include/clang/Lex/HeaderSearch.h
===
--- cfe/trunk/include/clang/Lex/HeaderSearch.h
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h
@@ -706,16 +706,18 @@
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
   bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics 

r359076 - Revert r359048: C++ DR2387: a variable template declared wthi

2019-04-24 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Apr 24 01:50:24 2019
New Revision: 359076

URL: http://llvm.org/viewvc/llvm-project?rev=359076&view=rev
Log:
Revert r359048: C++ DR2387: a variable template declared wthi

The change breaks libc++ with the follwing error:

In file included from valarray:4:
.../include/c++/v1/valarray:1062:60: error: explicit instantiation declaration 
of 'valarray<_Tp>' with internal linkage
_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t))
   ^
.../include/c++/v1/valarray:1063:60: error: explicit instantiation declaration 
of '~valarray<_Tp>' with internal linkage
_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::~valarray())

Removed:
cfe/trunk/test/CXX/drs/dr23xx.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CXX/drs/dr0xx.cpp
cfe/trunk/test/CXX/drs/dr17xx.cpp
cfe/trunk/test/CXX/module/module.interface/p3.cpp
cfe/trunk/test/CXX/module/module.interface/p5.cpp
cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
cfe/trunk/test/SemaCXX/PR10177.cpp
cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359076&r1=359075&r2=359076&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 24 01:50:24 
2019
@@ -4384,8 +4384,6 @@ def err_explicit_instantiation_of_typede
   "explicit instantiation of typedef %0">;
 def err_explicit_instantiation_storage_class : Error<
   "explicit instantiation cannot have a storage class">;
-def err_explicit_instantiation_internal_linkage : Error<
-  "explicit instantiation declaration of %0 with internal linkage">;
 def err_explicit_instantiation_not_known : Error<
   "explicit instantiation of %0 does not refer to a function template, "
   "variable template, member function, member class, or static data member">;

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=359076&r1=359075&r2=359076&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Wed Apr 24 01:50:24 2019
@@ -610,18 +610,6 @@ static LinkageInfo getExternalLinkageFor
   return LinkageInfo::external();
 }
 
-static StorageClass getStorageClass(const Decl *D) {
-  if (auto *TD = dyn_cast(D))
-D = TD->getTemplatedDecl();
-  if (D) {
-if (auto *VD = dyn_cast(D))
-  return VD->getStorageClass();
-if (auto *FD = dyn_cast(D))
-  return FD->getStorageClass();
-  }
-  return SC_None;
-}
-
 LinkageInfo
 LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
 LVComputationKind computation,
@@ -633,28 +621,24 @@ LinkageComputer::getLVForNamespaceScopeD
   // C++ [basic.link]p3:
   //   A name having namespace scope (3.3.6) has internal linkage if it
   //   is the name of
-
-  if (getStorageClass(D->getCanonicalDecl()) == SC_Static) {
-// - a variable, variable template, function, or function template
-//   that is explicitly declared static; or
-// (This bullet corresponds to C99 6.2.2p3.)
-return getInternalLinkageFor(D);
-  }
-
+  // - an object, reference, function or function template that is
+  //   explicitly declared static; or,
+  // (This bullet corresponds to C99 6.2.2p3.)
   if (const auto *Var = dyn_cast(D)) {
-// - a non-template variable of non-volatile const-qualified type, unless
-//   - it is explicitly declared extern, or
-//   - it is inline or exported, or
-//   - it was previously declared and the prior declaration did not have
-// internal linkage
-// (There is no equivalent in C99.)
+// Explicitly declared static.
+if (Var->getStorageClass() == SC_Static)
+  return getInternalLinkageFor(Var);
+
+// - a non-inline, non-volatile object or reference that is explicitly
+//   declared const or constexpr and neither explicitly declared extern
+//   nor previously declared to have external linkage; or (there is no
+//   equivalent in C99)
+// The C++ modules TS adds "non-exported" to this list.
 if (Context.getLangOpts().CPlusPlus &&
 Var->getType().isConstQualified() &&
 !Var->getType().isVolatileQualified() &&
 !Var->isInline() &&
-!isExportedFromModuleInterfaceUnit(Var) &&
-!isa(Var) &&
-!Var->getDescribedVarTemplate()) {
+!isExportedFromModuleInterfaceUnit(Var)) {
   const VarDecl *PrevV

Re: r359048 - C++ DR2387: a variable template declared wtih (or instantiated with) a

2019-04-24 Thread Ilya Biryukov via cfe-commits
Reverted in r359076.

On Wed, Apr 24, 2019 at 10:28 AM Ilya Biryukov  wrote:

> Hi Richard,
>
> This seems to break libc++, found while doing an integrate. The errors are:
>
> In file included from valarray:4:
> .../include/c++/v1/valarray:1062:60: error: explicit instantiation
> declaration of 'valarray<_Tp>' with internal linkage
> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS
> valarray::valarray(size_t))
>^
> .../include/c++/v1/valarray:1063:60: error: explicit instantiation
> declaration of '~valarray<_Tp>' with internal linkage
> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::~valarray())
>^
>
> I will likely revert the change to unbreak the integrate.
>
>

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


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

2019-04-24 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm


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

https://reviews.llvm.org/D61029



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


r359077 - Fix unquoted spaces in args in clang --verbose output

2019-04-24 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Wed Apr 24 02:06:03 2019
New Revision: 359077

URL: http://llvm.org/viewvc/llvm-project?rev=359077&view=rev
Log:
Fix unquoted spaces in args in clang --verbose output

The behaviour of not quoting spaces appears to have been introduced by
mistake in r190620.

Patch by Brad Moody!

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

Added:
cfe/trunk/test/Driver/verbose-output-quoting.c
Modified:
cfe/trunk/lib/Driver/Job.cpp

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=359077&r1=359076&r2=359077&view=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Wed Apr 24 02:06:03 2019
@@ -99,7 +99,7 @@ static bool skipArgs(const char *Flag, b
 }
 
 void Command::printArg(raw_ostream &OS, StringRef Arg, bool Quote) {
-  const bool Escape = Arg.find_first_of("\"\\$") != StringRef::npos;
+  const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
   if (!Quote && !Escape) {
 OS << Arg;

Added: cfe/trunk/test/Driver/verbose-output-quoting.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/verbose-output-quoting.c?rev=359077&view=auto
==
--- cfe/trunk/test/Driver/verbose-output-quoting.c (added)
+++ cfe/trunk/test/Driver/verbose-output-quoting.c Wed Apr 24 02:06:03 2019
@@ -0,0 +1,9 @@
+// RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck 
-check-prefix=SPACE -strict-whitespace %s
+// RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck 
-check-prefix=QUOTES-strict-whitespace %s
+// RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck 
-check-prefix=BACKSLASH -strict-whitespace %s
+// RUN: %clang --verbose -DDOLLAR=\$-c %s 2>&1 | FileCheck 
-check-prefix=DOLLAR-strict-whitespace %s
+
+// SPACE: -cc1 {{.*}} -D "SPACE=a b"
+// QUOTES: -cc1 {{.*}} -D "QUOTES=\"\""
+// BACKSLASH: -cc1 {{.*}} -D "BACKSLASH=\\"
+// DOLLAR: -cc1 {{.*}} -D "DOLLAR=\$"


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


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

2019-04-24 Thread Hans Wennborg via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359077: Fix unquoted spaces in args in clang --verbose 
output (authored by hans, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60997?vs=196191&id=196398#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60997

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


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


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


r359078 - [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

2019-04-24 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Wed Apr 24 02:23:31 2019
New Revision: 359078

URL: http://llvm.org/viewvc/llvm-project?rev=359078&view=rev
Log:
[clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

Summary:
Include insertion in clangd was inserting absolute paths when the
include directory was an absolute path with a double dot. This patch makes sure
double dots are handled both with absolute and relative paths.

Reviewers: sammccall

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=359078&r1=359077&r2=359078&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Apr 24 02:23:31 2019
@@ -1685,11 +1685,10 @@ std::string HeaderSearch::suggestPathToF
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
 llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
-if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+if (!WorkingDir.empty() && !path::is_absolute(Dir))
   fs::make_absolute(WorkingDir, DirPath);
-  path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-  Dir = DirPath;
-}
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {

Modified: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/HeaderSearchTest.cpp?rev=359078&r1=359077&r2=359078&view=diff
==
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp (original)
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp Wed Apr 24 02:23:31 2019
@@ -100,5 +100,12 @@ TEST_F(HeaderSearchTest, BackSlash) {
 }
 #endif
 
+TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z",
+   /*WorkingDir=*/""),
+"z");
+}
+
 } // namespace
 } // namespace clang


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


[clang-tools-extra] r359078 - [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

2019-04-24 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Wed Apr 24 02:23:31 2019
New Revision: 359078

URL: http://llvm.org/viewvc/llvm-project?rev=359078&view=rev
Log:
[clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

Summary:
Include insertion in clangd was inserting absolute paths when the
include directory was an absolute path with a double dot. This patch makes sure
double dots are handled both with absolute and relative paths.

Reviewers: sammccall

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp?rev=359078&r1=359077&r2=359078&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp Wed Apr 24 
02:23:31 2019
@@ -213,6 +213,11 @@ TEST_F(HeadersTest, DoNotInsertIfInSameF
 TEST_F(HeadersTest, ShortenedInclude) {
   std::string BarHeader = testPath("sub/bar.h");
   EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
+
+  SearchDirArg = (llvm::Twine("-I") + Subdir + "/..").str();
+  CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+  BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }
 
 TEST_F(HeadersTest, NotShortenedInclude) {


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


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

2019-04-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359078: [clang][HeaderSuggestion] Handle the case of dotdot 
with an absolute path (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60873?vs=195751&id=196400#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60873

Files:
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
  clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
@@ -213,6 +213,11 @@
 TEST_F(HeadersTest, ShortenedInclude) {
   std::string BarHeader = testPath("sub/bar.h");
   EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
+
+  SearchDirArg = (llvm::Twine("-I") + Subdir + "/..").str();
+  CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+  BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }
 
 TEST_F(HeadersTest, NotShortenedInclude) {
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1685,11 +1685,10 @@
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
 llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
-if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+if (!WorkingDir.empty() && !path::is_absolute(Dir))
   fs::make_absolute(WorkingDir, DirPath);
-  path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-  Dir = DirPath;
-}
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -100,5 +100,12 @@
 }
 #endif
 
+TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z",
+   /*WorkingDir=*/""),
+"z");
+}
+
 } // namespace
 } // namespace clang


Index: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
@@ -213,6 +213,11 @@
 TEST_F(HeadersTest, ShortenedInclude) {
   std::string BarHeader = testPath("sub/bar.h");
   EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
+
+  SearchDirArg = (llvm::Twine("-I") + Subdir + "/..").str();
+  CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+  BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }
 
 TEST_F(HeadersTest, NotShortenedInclude) {
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1685,11 +1685,10 @@
 
 StringRef Dir = SearchDirs[I].getDir()->getName();
 llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
-if (!WorkingDir.empty() && !path::is_absolute(Dir)) {
+if (!WorkingDir.empty() && !path::is_absolute(Dir))
   fs::make_absolute(WorkingDir, DirPath);
-  path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-  Dir = DirPath;
-}
+path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -100,5 +100,12 @@
 }
 #endif
 
+TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z",
+   /*WorkingDir=*/""),
+"z");
+}
+
 } // namespace
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r359079 - [clangd] Fix handling of include paths in windows tests

2019-04-24 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Wed Apr 24 02:42:53 2019
New Revision: 359079

URL: http://llvm.org/viewvc/llvm-project?rev=359079&view=rev
Log:
[clangd] Fix handling of include paths in windows tests

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=359079&r1=359078&r2=359079&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Apr 24 
02:42:53 2019
@@ -22,6 +22,7 @@
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -699,11 +700,13 @@ TEST(CompletionTest, DynamicIndexInclude
   Server.addDocument(testPath("foo_impl.cpp"), FileContent);
   // Wait for the dynamic index being built.
   ASSERT_TRUE(Server.blockUntilIdleForTest());
-  EXPECT_THAT(
-  completions(Server, "Foo^ foo;").Completions,
-  ElementsAre(AllOf(Named("Foo"),
-HasInclude('"' + testPath("foo_header.h") + '"'),
-InsertInclude(;
+  EXPECT_THAT(completions(Server, "Foo^ foo;").Completions,
+  ElementsAre(AllOf(Named("Foo"),
+HasInclude('"' +
+   llvm::sys::path::convert_to_slash(
+   testPath("foo_header.h")) +
+   '"'),
+InsertInclude(;
 }
 
 TEST(CompletionTest, DynamicIndexMultiFile) {

Modified: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp?rev=359079&r1=359078&r2=359079&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp Wed Apr 24 
02:42:53 2019
@@ -15,6 +15,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -221,7 +222,8 @@ TEST_F(HeadersTest, ShortenedInclude) {
 }
 
 TEST_F(HeadersTest, NotShortenedInclude) {
-  std::string BarHeader = testPath("sub-2/bar.h");
+  std::string BarHeader =
+  llvm::sys::path::convert_to_slash(testPath("sub-2/bar.h"));
   EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\"");
 }
 
@@ -265,8 +267,7 @@ TEST(Headers, NoHeaderSearchInfo) {
   auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
   auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
 
-  EXPECT_EQ(Inserter.calculateIncludePath(Inserting),
-"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.calculateIncludePath(Inserting), "\"" + HeaderPath + 
"\"");
   EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Inserting), false);
 
   EXPECT_EQ(Inserter.calculateIncludePath(Verbatim), "");


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


[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

2019-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Found an issue today.

Input (break a line at ^):

  // here is my comment ^// another comment

Expected:

  // here is my comment
  ^// another comment

Actual (an extra comment marker is added):

  // here is my comment
  // // another comment


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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


r359081 - Add 'REQUIRES: shell' to verbose-output-quoting.c

2019-04-24 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Wed Apr 24 03:12:30 2019
New Revision: 359081

URL: http://llvm.org/viewvc/llvm-project?rev=359081&view=rev
Log:
Add 'REQUIRES: shell' to verbose-output-quoting.c

The lit shell couldn't handle these run lines.

Modified:
cfe/trunk/test/Driver/verbose-output-quoting.c

Modified: cfe/trunk/test/Driver/verbose-output-quoting.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/verbose-output-quoting.c?rev=359081&r1=359080&r2=359081&view=diff
==
--- cfe/trunk/test/Driver/verbose-output-quoting.c (original)
+++ cfe/trunk/test/Driver/verbose-output-quoting.c Wed Apr 24 03:12:30 2019
@@ -1,3 +1,4 @@
+// REQUIRES: shell
 // RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck 
-check-prefix=SPACE -strict-whitespace %s
 // RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck 
-check-prefix=QUOTES-strict-whitespace %s
 // RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck 
-check-prefix=BACKSLASH -strict-whitespace %s


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


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

2019-04-24 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover added inline comments.



Comment at: include/clang/Sema/Sema.h:10762
 bool AllowName);
+ bool SemaBuiltinARMMemoryTaggingCall(unsigned BuiltinID, CallExpr *TheCall);
 public:

Slightly misaligned.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

I think this should be fixed now.  It looks like technical debt from the fact 
that the instructions only fairly recently gained that feature after the 
intrinsics were implemented internally. There's no good way to justify the 
current semantics to someone unaware of that history.


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

https://reviews.llvm.org/D60485



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


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

2019-04-24 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas updated this revision to Diff 196410.
ztamas added a comment.

Remove outdated comment from docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60507

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

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

Re: [PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

2019-04-24 Thread Jon Roelofs via cfe-commits
On Tue, Apr 23, 2019 at 11:36 PM Petr Hosek via Phabricator <
revi...@reviews.llvm.org> wrote:

> phosek added inline comments.
>
>
> 
> Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:122
> +
> +/// \p Flag must be a flag accepted by the driver with its leading '-'
> removed,
> +// otherwise '-print-multi-lib' will not emit them correctly.
> 
> jroelofs wrote:
> > Can we enforce this precondition with an assert? The
> '-'-prefix-not-there part is easy, but what about the "it's a driver flag"
> part?
> We could, but we would need to pass in the reference to `Driver` so we can
> use `getOpts().ParseOneArg` to parse the flag. However, we're not doing
> that in `Multilib` either at the moment even though [the comment says it
> has to be a valid flag](
> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Multilib.h#L80).
> I'd prefer to keep this as a pure move and then add the additional checking
> both here and to `Multilib` as a separate change, would that be fine with
> you?


Sure, that works.

Jon


>
>
> Repository:
>   rC Clang
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D61040/new/
>
> https://reviews.llvm.org/D61040
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60907: [OpenMP][WIP] Add math functions support in OpenMP offloading

2019-04-24 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

In D60907#1473406 , @Hahnfeld wrote:

> So the scheme is: `pow` is defined in `__clang_openmp_math.h` to call 
> `__kmpc_pow`. This lives in `libomptarget-nvptx` (both bc and static lib) and 
> just calls `pow` which works because `nvcc` and Clang in CUDA mode make sure 
> that the call gets routed into `libdevice`?
>
> Did you test that something like `pow(d, 2)` is optimized by LLVM to `d * d`? 
> There's a pass doing so (can't recall the name) and from my previous attempts 
> it didn't work well if you hid the function name instead of the known `pow` 
> one.


The transformation was blocked because of a check in optimizePow() this was 
preventing pow(x,2) from becoming x*x. By adding the pow functions to the TLI 
the transformation now applies. This has now been fixed. SQRT is eliminated as 
per usual, no change for that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


r359098 - Use llvm::stable_sort

2019-04-24 Thread Fangrui Song via cfe-commits
Author: maskray
Date: Wed Apr 24 07:43:05 2019
New Revision: 359098

URL: http://llvm.org/viewvc/llvm-project?rev=359098&view=rev
Log:
Use llvm::stable_sort

Modified:
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/Analysis/CloneDetection.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/lib/Index/CommentToXML.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp

Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=359098&r1=359097&r2=359098&view=diff
==
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Wed Apr 24 07:43:05 2019
@@ -1641,7 +1641,7 @@ ObjCIvarDecl *ObjCInterfaceDecl::all_dec
 
   if (!layout.empty()) {
 // Order synthesized ivars by their size.
-std::stable_sort(layout.begin(), layout.end());
+llvm::stable_sort(layout);
 unsigned Ix = 0, EIx = layout.size();
 if (!data().IvarList) {
   data().IvarList = layout[0].Ivar; Ix++;

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=359098&r1=359097&r2=359098&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Wed Apr 24 07:43:05 2019
@@ -3287,10 +3287,10 @@ static void DumpRecordLayout(raw_ostream
 }
 
 // Sort nvbases by offset.
-std::stable_sort(Bases.begin(), Bases.end(),
- [&](const CXXRecordDecl *L, const CXXRecordDecl *R) {
-  return Layout.getBaseClassOffset(L) < Layout.getBaseClassOffset(R);
-});
+llvm::stable_sort(
+Bases, [&](const CXXRecordDecl *L, const CXXRecordDecl *R) {
+  return Layout.getBaseClassOffset(L) < Layout.getBaseClassOffset(R);
+});
 
 // Dump (non-virtual) bases
 for (const CXXRecordDecl *Base : Bases) {

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=359098&r1=359097&r2=359098&view=diff
==
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Wed Apr 24 07:43:05 2019
@@ -3188,8 +3188,8 @@ void VFTableBuilder::dumpLayout(raw_ostr
   const CXXMethodDecl *MD = MethodNameAndDecl.second;
 
   ThunkInfoVectorTy ThunksVector = Thunks[MD];
-  std::stable_sort(ThunksVector.begin(), ThunksVector.end(),
-   [](const ThunkInfo &LHS, const ThunkInfo &RHS) {
+  llvm::stable_sort(ThunksVector, [](const ThunkInfo &LHS,
+ const ThunkInfo &RHS) {
 // Keep different thunks with the same adjustments in the order they
 // were put into the vector.
 return std::tie(LHS.This, LHS.Return) < std::tie(RHS.This, RHS.Return);

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=359098&r1=359097&r2=359098&view=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Wed Apr 24 07:43:05 2019
@@ -367,11 +367,7 @@ void RecursiveCloneTypeIIHashConstraint:
 }
 
 // Sort hash_codes in StmtsByHash.
-std::stable_sort(StmtsByHash.begin(), StmtsByHash.end(),
- [](std::pair LHS,
-std::pair RHS) {
-   return LHS.first < RHS.first;
- });
+llvm::stable_sort(StmtsByHash, llvm::less_first());
 
 // Check for each StmtSequence if its successor has the same hash value.
 // We don't check the last StmtSequence as it has no successor.

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=359098&r1=359097&r2=359098&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Apr 24 07:4

[PATCH] D59725: Additions to creduce script

2019-04-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

I've stumbled into an issue with the script:
It got a line: `clang: 
/build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
{anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
`L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' 
failed.`
And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, *LI) && 
"LCSSA required to run indvars!"' t.log || exit 1`
But that doesn't work, escapes should be applied. it should be
`grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
indvars!\"' t.log || exit 1`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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


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

2019-04-24 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:3402
+CmdArgs.push_back("-femit-param-entry-values");
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);

If this is now a cc1-only option, this part goes away right?


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

https://reviews.llvm.org/D58033



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


[PATCH] D60907: [OpenMP][WIP] Add math functions support in OpenMP offloading

2019-04-24 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a subscriber: gregrodgers.
gtbercea added a comment.

@gregrodgers


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


r359107 - clang-cl: List valid values for /std: in /? output

2019-04-24 Thread Nico Weber via cfe-commits
Author: nico
Date: Wed Apr 24 08:31:13 2019
New Revision: 359107

URL: http://llvm.org/viewvc/llvm-project?rev=359107&view=rev
Log:
clang-cl: List valid values for /std: in /? output

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

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=359107&r1=359106&r2=359107&view=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Apr 24 08:31:13 2019
@@ -166,7 +166,7 @@ def _SLASH_source_charset : CLCompileJoi
 def _SLASH_execution_charset : CLCompileJoined<"execution-charset:">,
   HelpText<"Runtime encoding, supports only UTF-8">, Alias;
 def _SLASH_std : CLCompileJoined<"std:">,
-  HelpText<"Language standard to compile for">;
+  HelpText<"Language standard to compile for (c++14,c++17,c++latest)">;
 def _SLASH_U : CLJoinedOrSeparate<"U">, HelpText<"Undefine macro">,
   MetaVarName<"">, Alias;
 def _SLASH_validate_charset : CLFlag<"validate-charset">,


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


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

2019-04-24 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359107: clang-cl: List valid values for /std: in /? output 
(authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61029?vs=196277&id=196458#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61029

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


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


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


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

2019-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 196462.
ilya-biryukov marked 53 inline comments as done.
ilya-biryukov added a comment.

- Simplify rawByExpanded by using a helper function.
- Add a FIXME to add spelled-to-expanded mapping
- s/raw*/spelled*
- Split token collector and token buffer tests
- Rewrite dump function for use in tests
- Simplify tests by using a dumpForTests function
- Address other comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,590 @@
+//===- TokensTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using llvm::unittest::ValueIs;
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Not;
+using ::testing::StartsWith;
+
+namespace {
+// Checks the passed ArrayRef has the same begin() and end() iterators as the
+// argument.
+MATCHER_P(SameRange, A, "") {
+  return A.begin() == arg.begin() && A.end() == arg.end();
+}
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+
+class TokenCollectorTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+return llvm::make_unique();
+  }
+
+private:
+  TokenBuffer &Result;
+  llvm::Optional Collector;
+};
+
+constexpr const char *FileName = "./input.cpp";
+FS->addFile(FileName, time_t(), llvm::MemoryBuffer::getMemBufferCopy(""));
+// Prepare to run a compiler.
+s

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

2019-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



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

sammccall wrote:
> Token should be copyable I think?
Sure, they are copyable. Am I missing something?



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

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



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

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




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

sammccall wrote:
> may want to offer `length()` and `StringRef text(const SourceManager&)`
SG.
WDYT of exposing the struct members via getters too?

This would mean uniform access for all things (beginOffset, endOffset, length) 
and adding a ctor that checks invariants (Begin <= End, File.isValid()) would 
also fit naturally.




Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:111
+///   replacements,
+///2. Raw tokens: corresponding directly to the source code of a file 
before
+///   any macro replacements occurred.

sammccall wrote:
> I think "Spelled" would be a better name than "Raw" here. (Despite being 
> longer and grammatically awkward)
> 
> - The spelled/expanded dichotomy is well-established in clang-land, and this 
> will help understand how they relate to each other and why the difference is 
> important. It's true we'd be extending the meaning a bit (all PP activity, 
> not just macros), but that applies to Expanded too. I think consistency is 
> valuable here.
> - "Raw" regarding tokens has an established meaning (raw lexer mode, 
> raw_identifier etc) that AIUI you're not using here, but plausibly could be. 
> So I think this may cause some confusion.
> 
> There's like 100 occurrences of "raw" in this patch, happy to discuss first 
> to avoid churn.
Spelled looks fine. The only downside is that it intersects with 
SourceManager's definition of spelled (which is very similar, but not the same).
Still like it more than raw.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:160
+  llvm::Optional>
+  findRawByExpanded(llvm::ArrayRef Expanded) const;
+

sammccall wrote:
> sammccall wrote:
> > Not totally clear what the legal arguments are here:
> >  - any sequence of tokens?
> >  - a subrange of expandedTokens() (must point into that array) - most 
> > obvious, but in this case why does mapping an empty range always fail?
> >  - any subrange of expandedTokens(), compared by value? - I think this is 
> > what the implementation assumes
> > 
> I think `by` is a little weak semantically, I think it just means "the 
> parameter is expanded tokens".
> 
> `findRawForExpanded()` seems clearer.
> `rawTokensForExpanded()` would be clearer still I think.
Yes, `Expanded` should be a subrange of `expandedTokens()`. Added a comment.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:165
+  llvm::Optional
+  findOffsetsByExpanded(llvm::ArrayRef Expanded) const;
+

sammccall wrote:
> this still seems a bit non-orthogonal:
>  - it fails if and only if findRawByExpanded fails, but that isn't visible to 
> callers
>  - there's no way to avoid the redundant work of doing the mapping twice
>  - the name doesn't indicate that it maps to raw locations as well as 
> translating tokens to locations
>  - seems likely to lead to combinatorial explosion, e.g. if we want a pair of 
> 

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

2019-04-24 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> For NotPod, it is aggregate which is specific in the document

Yes, it's an aggregate which is returned in registers... but it's returned in 
integer registers, unlike Pod which is returned in floating-point registers.


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

https://reviews.llvm.org/D60349



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


[PATCH] D60764: Add clang cc1 option to generate OpenCL builtin functions

2019-04-24 Thread Pierre via Phabricator via cfe-commits
Pierre updated this revision to Diff 196474.
Pierre marked an inline comment as done.
Pierre added a comment.

The name of the command line option has been updated.


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

https://reviews.llvm.org/D60764

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -820,7 +820,7 @@
   }
 
   // Check if this is an OpenCL Builtin, and if so, insert the 
declarations.
-  if (S.getLangOpts().OpenCL) {
+  if (S.getLangOpts().OpenCL && S.getLangOpts().GenerateOpenCLBuiltin) {
 auto Index = isOpenCLBuiltin(II->getName());
 if (Index.first) {
   InsertBuiltinDeclarations(S, R, II, Index.first, Index.second);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2152,7 +2152,7 @@
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
 // Include default header file for OpenCL.
-if (Opts.IncludeDefaultHeader) {
+if (Opts.IncludeDefaultHeader && !Opts.AddOpenCLBuiltins) {
   PPOpts.Includes.push_back("opencl-c.h");
 }
   }
@@ -2355,6 +2355,7 @@
   }
 
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.AddOpenCLBuiltins = Args.hasArg(OPT_fadd_opencl_builtins);
 
   llvm::Triple T(TargetOpts.Triple);
   CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangStd);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -751,7 +751,9 @@
 def fdefault_calling_conv_EQ : Joined<["-"], "fdefault-calling-conv=">,
   HelpText<"Set default calling convention">, 
Values<"cdecl,fastcall,stdcall,vectorcall,regcall">;
 def finclude_default_header : Flag<["-"], "finclude-default-header">,
-  HelpText<"Include the default header file for OpenCL">;
+  HelpText<"Include default header file containing OpenCL builtin functions">;
+def fadd_opencl_builtins: Flag<["-"], "fadd-opencl-builtins">,
+  HelpText<"Add OpenCL builtin function declarations automatically">;
 def fpreserve_vec3_type : Flag<["-"], "fpreserve-vec3-type">,
   HelpText<"Preserve 3-component vector type">;
 def fwchar_type_EQ : Joined<["-"], "fwchar-type=">,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -254,7 +254,8 @@
 LANGOPT(CFProtectionBranch , 1, 0, "Control-Flow Branch Protection enabled")
 LANGOPT(FakeAddressSpaceMap , 1, 0, "OpenCL fake address space map")
 ENUM_LANGOPT(AddressSpaceMapMangling , AddrSpaceMapMangling, 2, ASMM_Target, 
"OpenCL address space map mangling mode")
-LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
+LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file containing 
OpenCL builtin functions")
+LANGOPT(AddOpenCLBuiltins, 1, 0, "Add OpenCL builtin function declarations 
automatically")
 BENIGN_LANGOPT(DelayedTemplateParsing , 1, 0, "delayed template parsing")
 LANGOPT(BlocksRuntimeOptional , 1, 0, "optional blocks runtime")
 LANGOPT(


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -820,7 +820,7 @@
   }
 
   // Check if this is an OpenCL Builtin, and if so, insert the declarations.
-  if (S.getLangOpts().OpenCL) {
+  if (S.getLangOpts().OpenCL && S.getLangOpts().GenerateOpenCLBuiltin) {
 auto Index = isOpenCLBuiltin(II->getName());
 if (Index.first) {
   InsertBuiltinDeclarations(S, R, II, Index.first, Index.second);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2152,7 +2152,7 @@
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
 // Include default header file for OpenCL.
-if (Opts.IncludeDefaultHeader) {
+if (Opts.IncludeDefaultHeader && !Opts.AddOpenCLBuiltins) {
   PPOpts.Includes.push_back("opencl-c.h");
 }
   }
@@ -2355,6 +2355,7 @@
   }
 
   Opts.IncludeDefaultHeader = Args.hasArg(OPT_finclude_default_header);
+  Opts.AddOpenCLBuiltins = Args.hasArg(OPT_fadd_opencl_builtins);
 
   llvm::Triple T(TargetOpts.Triple);
   CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangSt

[PATCH] D61054: lib/Header: Fix Visual Studio builds

2019-04-24 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM

This would break if someone does a debug and release build simultaneously, the 
copies occur simultaneously and the filesystem gets unhappy, That seems fairly 
contrived though, and this is also restoring the behavior before your previous 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61054



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


[clang-tools-extra] r359112 - [clangd] Fix broken helper deep in unit test. NFC

2019-04-24 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Wed Apr 24 10:00:38 2019
New Revision: 359112

URL: http://llvm.org/viewvc/llvm-project?rev=359112&view=rev
Log:
[clangd] Fix broken helper deep in unit test. NFC

Modified:
clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp?rev=359112&r1=359111&r2=359112&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp Wed Apr 24 10:00:38 
2019
@@ -29,15 +29,9 @@ Symbol symbol(llvm::StringRef QName) {
 
 static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle,
llvm::StringRef Repl) {
-  std::string Result;
-  llvm::raw_string_ostream OS(Result);
-  std::pair Split;
-  for (Split = Haystack.split(Needle); !Split.second.empty();
-   Split = Split.first.split(Needle))
-OS << Split.first << Repl;
-  Result += Split.first;
-  OS.flush();
-  return Result;
+  llvm::SmallVector Parts;
+  Haystack.split(Parts, Needle);
+  return llvm::join(Parts, Repl);
 }
 
 // Helpers to produce fake index symbols for memIndex() or completions().


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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

aganea wrote:
> Fixes
> ```
> [2097/2979] Building CXX object 
> tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
>  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> ^
> ```
You mixed up the error messages but I see what is going on.

So you may want to add a comment since it is not apparent that what is going on 
is due the `EXPECT_TRUE` macro eventually expanding to an `if..else` which is 
what is triggering the warning. Since someone may come by in the future and 
just remove the braces since it is not apparent why they are there.

Same below as well.


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

https://reviews.llvm.org/D61046



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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

shafik wrote:
> aganea wrote:
> > Fixes
> > ```
> > [2097/2979] Building CXX object 
> > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> > explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > ^
> > ```
> You mixed up the error messages but I see what is going on.
> 
> So you may want to add a comment since it is not apparent that what is going 
> on is due the `EXPECT_TRUE` macro eventually expanding to an `if..else` which 
> is what is triggering the warning. Since someone may come by in the future 
> and just remove the braces since it is not apparent why they are there.
> 
> Same below as well.
EXPECT_* inside if is quite common, I don't think we should add a comment every 
time it is used.


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

https://reviews.llvm.org/D61046



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


[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

2019-04-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, mgrang, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

We scrape the enclosing scopes from the source file, and use them in the query.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61077

Files:
  clangd/CodeComplete.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -322,6 +322,73 @@
   EXPECT_EQ(IDs["foo"], 2u);
 }
 
+TEST(SourceCodeTests, VisibleNamespaces) {
+  std::vector>> Cases = {
+  {
+  R"cpp(
+// Using directive resolved against enclosing namespaces.
+using namespace foo;
+namespace ns {
+using namespace bar;
+  )cpp",
+  {"ns", "", "bar", "foo", "ns::bar"},
+  },
+  {
+  R"cpp(
+// Don't include namespaces we've closed, ignore namespace aliases.
+using namespace clang;
+using std::swap;
+namespace clang {
+namespace clangd {}
+namespace ll = ::llvm;
+}
+namespace clang {
+  )cpp",
+  {"clang", ""},
+  },
+  {
+  R"cpp(
+// Using directives visible even if a namespace is reopened.
+namespace foo{ using namespace bar; }
+namespace foo{
+  )cpp",
+  {"foo", "", "bar", "foo::bar"},
+  },
+  {
+  R"cpp(
+// Mismatched braces
+namespace foo{}
+}}}
+namespace bar{
+  )cpp",
+  {"bar", ""},
+  },
+  {
+  R"cpp(
+// Namespaces with multiple chunks.
+namespace a::b {
+  using namespace c::d;
+  namespace e::f {
+  )cpp",
+  {
+  "a::b::e::f",
+  "",
+  "a",
+  "a::b",
+  "a::b::c::d",
+  "a::b::e",
+  "a::c::d",
+  "c::d",
+  },
+  },
+  };
+  for (const auto& Case : Cases) {
+EXPECT_EQ(Case.second,
+  visibleNamespaces(Case.first, format::getLLVMStyle()))
+<< Case.first;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -18,6 +18,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "index/Index.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -2452,6 +2453,58 @@
   UnorderedElementsAre(Named("sym1"), Named("sym2")));
 }
 
+TEST(NoCompileCompletionTest, WithIndex) {
+  std::vector Syms = {func("xxx"), func("a::xxx"), func("ns::b::xxx"),
+  func("c::xxx"), func("ns::d::xxx")};
+  auto Results = completionsNoCompile(
+  R"cpp(
+// Current-scopes, unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::";
+  CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  Results = completionsNoCompile(
+  R"cpp(
+// All-scopes unqualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Scope("")),
+   AllOf(Qualifier(""), Scope("a::")),
+   AllOf(Qualifier(""), Scope("ns::b::")),
+   AllOf(Qualifier("c::"), Scope("c::")),
+   AllOf(Qualifier("d::"), Scope("ns::d::";
+  Results = completionsNoCompile(
+  R"cpp(
+// Qualified completion.
+using namespace a;
+namespace ns {
+using namespace b;
+void foo() {
+b::xx^
+}
+  )cpp",
+  Syms, Opts);
+  EXPECT_THAT(Results.Completions,
+  ElementsAre(AllOf(Qualifier(""), Scope("ns::b::";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
=

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

2019-04-24 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
vzakhari added a comment.

In D58537#1476570 , @tstellar wrote:

> Can you test D61054 ?


I will do it ASAP, but I am currently having problems with my build system.  I 
guess it is OK to proceed with this change.  I will send an update, when I try 
it.  Thank you for fixing this!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

Okay, if the TBE isn't suitable for the pre-merged format, then I think that 
this should be okay with the current approach.  This is more about the 
generation and not wanting to have to maintain a separate side table of 
information manually.  It just is a different trade off.  But, sounds like we 
are converging towards a solution that can be used to address the various 
tradeoff points that seem to matter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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


[PATCH] D61079: Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

2019-04-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie created this revision.
dblaikie added a reviewer: aprantl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

(this would regress size without a corresponding LLVM change that avoids
putting other user defined types inside type units when they aren't in
their own type units - instead emitting declarations inside the type
unit and a definition in the primary CU)

Adrian - can you tell me whether the changes to the implicit modular debug info 
tests are OK? What does that feature use the 'identifier' for, if anything?


Repository:
  rC Clang

https://reviews.llvm.org/D61079

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/Modules/ExtDebugInfo.cpp
  test/Modules/ModuleDebugInfo.cpp


Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -119,8 +119,7 @@
 
 // CHECK: ![[A:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, name: "A",
 // CHECK-SAME:   elements:
-// CHECK-SAME:   vtableHolder: ![[A]],
-// CHECK-SAME:   identifier: "_ZTS1A")
+// CHECK-SAME:   vtableHolder: ![[A]])
 
 // CHECK: ![[DERIVED:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, 
name: "Derived",
 // CHECK-SAME: identifier: "_ZTS7Derived")
Index: test/Modules/ExtDebugInfo.cpp
===
--- test/Modules/ExtDebugInfo.cpp
+++ test/Modules/ExtDebugInfo.cpp
@@ -214,7 +214,7 @@
 // CHECK-PCH:dwoId: 18446744073709551614
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A",
-// CHECK-SAME: DIFlagFwdDecl, identifier: "_ZTS1A")
+// CHECK-SAME: DIFlagFwdDecl)
 
 // There is a full definition of the type available in the module.
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual",
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -915,6 +915,11 @@
 
   if (!needsTypeIdentifier(TD, CGM, TheCU))
 return Identifier;
+  if (const auto *RD = dyn_cast(TD))
+if (RD->getDefinition())
+  if (RD->isDynamicClass() &&
+  CGM.getVTableLinkage(RD) == llvm::GlobalValue::ExternalLinkage)
+return Identifier;
 
   // TODO: This is using the RTTI name. Is there a better way to get
   // a unique string for a type?


Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -119,8 +119,7 @@
 
 // CHECK: ![[A:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, name: "A",
 // CHECK-SAME:   elements:
-// CHECK-SAME:   vtableHolder: ![[A]],
-// CHECK-SAME:   identifier: "_ZTS1A")
+// CHECK-SAME:   vtableHolder: ![[A]])
 
 // CHECK: ![[DERIVED:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, name: "Derived",
 // CHECK-SAME: identifier: "_ZTS7Derived")
Index: test/Modules/ExtDebugInfo.cpp
===
--- test/Modules/ExtDebugInfo.cpp
+++ test/Modules/ExtDebugInfo.cpp
@@ -214,7 +214,7 @@
 // CHECK-PCH:dwoId: 18446744073709551614
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A",
-// CHECK-SAME: DIFlagFwdDecl, identifier: "_ZTS1A")
+// CHECK-SAME: DIFlagFwdDecl)
 
 // There is a full definition of the type available in the module.
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual",
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -915,6 +915,11 @@
 
   if (!needsTypeIdentifier(TD, CGM, TheCU))
 return Identifier;
+  if (const auto *RD = dyn_cast(TD))
+if (RD->getDefinition())
+  if (RD->isDynamicClass() &&
+  CGM.getVTableLinkage(RD) == llvm::GlobalValue::ExternalLinkage)
+return Identifier;
 
   // TODO: This is using the RTTI name. Is there a better way to get
   // a unique string for a type?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

Aside from comments that @jakehehrlich already made which I agree with, I'm 
also concerned about the use of `yaml2obj`. AFAIK that tool has always been 
intended as a testing, not a production tool, but with this change this would 
no longer be the case. Specifically, it formalizes the `yaml2obj` input format 
by making it one of the output formats supported by Clang. I believe that this 
is such a fundamental change that it should be discussed separately before 
moving on with this implementation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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


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

2019-04-24 Thread S. B. Tam via Phabricator via cfe-commits
cpplearner added a comment.

(I am ensadc at bugzilla)

Does this cause regression in the following case?

  template
  void f() {
  switch (N) case 0:; // should be diagnosed
  switch (0) case N:; // should be diagnosed
  }

Currently clang diagnoses these `switch` statements even if the template is not 
instantiated, GCC does not.

AFAIK both compilers are correct under [temp.res]/8, but clang's current 
behavior seems more helpful.


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

https://reviews.llvm.org/D61027



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


[PATCH] D59725: Additions to creduce script

2019-04-24 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment.

In D59725#1477042 , @lebedev.ri wrote:

> I've stumbled into an issue with the script:
>  It got a line: `clang: 
> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' 
> failed.`
>  And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, *LI) 
> && "LCSSA required to run indvars!"' t.log || exit 1`
>  But that doesn't work, escapes should be applied. it should be
>  `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
> indvars!\"' t.log || exit 1`


In my script I use `grep -F " + shlex.quote(self.args.crash_message)` which 
should escape both the message and ensure that the grep argument is not 
interpreted as a regex.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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


[PATCH] D59725: Additions to creduce script

2019-04-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+for msg in self.expected_output:
+  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+

>>! In D59725#1477362, @arichardson wrote:
>>>! In D59725#1477042, @lebedev.ri wrote:
>> I've stumbled into an issue with the script:
>> It got a line: `clang: 
>> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
>> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
>> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run indvars!"' 
>> failed.`
>> And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, *LI) 
>> && "LCSSA required to run indvars!"' t.log || exit 1`
>> But that doesn't work, escapes should be applied. it should be
>> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
>> indvars!\"' t.log || exit 1`
> 
> In my script I use `grep -F " + shlex.quote(self.args.crash_message)` which 
> should escape both the message and ensure that the grep argument is not 
> interpreted as a regex.

Great to hear!
It appears that the script is currently more primitive than that currently 
though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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


[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Amy Huang via Phabricator via cfe-commits
akhuang created this revision.
akhuang added a reviewer: rnk.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, aprantl.
Herald added projects: clang, LLVM.

This emits labels around heapallocsite calls and S_HEAPALLOCSITE debug
info in codeview. Currently only changes FastISel, so emitting labels still
needs to be implemented in SelectionDAG.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61083

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/MachineInstr.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,57 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc(void);
+; void foo() {
+;   myalloc()
+; }
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !8 {
+entry:
+  %call = call i8* @myalloc(), !dbg !11, !heapallocsite !2
+  ret void, !dbg !12
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local i8* @myalloc() #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 3, type: !9, scopeLine: 3, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 4, scope: !8)
+!12 = !DILocation(line: 5, scope: !8)
+
Index: llvm/lib/Target/X86/X86FastISel.cpp
===
--- llvm/lib/Target/X86/X86FastISel.cpp
+++ llvm/lib/Target/X86/X86FastISel.cpp
@@ -3985,6 +3985,7 @@
   }
 
   Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI));
+  Result->cloneInstrSymbols(*FuncInfo.MF, *MI);
   MachineBasicBlock::iterator I(MI);
   removeDeadCode(I, std::next(I));
   return true;
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,12 @@
   if (CLI.NumResultRegs && CLI.CS)
 updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite")) {
+MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite");
+MF->addCodeViewHeapAllocSite(CLI.Call, MD);
+  }
+
   return true;
 }
 
Index: llvm/lib/CodeGen/MachineInstr.cpp
===
--- llvm/lib/CodeGen/MachineInstr.cpp
+++ llvm/lib/CodeGen/MachineInstr.cpp
@@ -513,6 +513,19 @@
   MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol));
 }
 
+void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
+ const MachineInstr &MI) {
+  if (this == &MI)
+// Nothing to do for a self-clone!
+return;
+
+  assert(&MF == MI.getMF() &&
+ "Invalid machine functions when cloning inst

[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 196496.
akhuang added a comment.

- remove added whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61083

Files:
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/MachineInstr.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,57 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc(void);
+; void foo() {
+;   myalloc()
+; }
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+; Function Attrs: noinline nounwind optnone
+define dso_local void @f() #0 !dbg !8 {
+entry:
+  %call = call i8* @myalloc(), !dbg !11, !heapallocsite !2
+  ret void, !dbg !12
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+declare dso_local i8* @myalloc() #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "heapallocsite.c", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 3, type: !9, scopeLine: 3, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 4, scope: !8)
+!12 = !DILocation(line: 5, scope: !8)
+
Index: llvm/lib/Target/X86/X86FastISel.cpp
===
--- llvm/lib/Target/X86/X86FastISel.cpp
+++ llvm/lib/Target/X86/X86FastISel.cpp
@@ -3985,6 +3985,7 @@
   }
 
   Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI));
+  Result->cloneInstrSymbols(*FuncInfo.MF, *MI);
   MachineBasicBlock::iterator I(MI);
   removeDeadCode(I, std::next(I));
   return true;
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1234,6 +1234,12 @@
   if (CLI.NumResultRegs && CLI.CS)
 updateValueMap(CLI.CS->getInstruction(), CLI.ResultReg, CLI.NumResultRegs);
 
+  // Set labels for heapallocsite call.
+  if (CLI.CS && CLI.CS->getInstruction()->getMetadata("heapallocsite")) {
+MDNode *MD = CLI.CS->getInstruction()->getMetadata("heapallocsite");
+MF->addCodeViewHeapAllocSite(CLI.Call, MD);
+  }
+
   return true;
 }
 
Index: llvm/lib/CodeGen/MachineInstr.cpp
===
--- llvm/lib/CodeGen/MachineInstr.cpp
+++ llvm/lib/CodeGen/MachineInstr.cpp
@@ -513,6 +513,19 @@
   MF.createMIExtraInfo(memoperands(), getPreInstrSymbol(), Symbol));
 }
 
+void MachineInstr::cloneInstrSymbols(MachineFunction &MF,
+ const MachineInstr &MI) {
+  if (this == &MI)
+// Nothing to do for a self-clone!
+return;
+
+  assert(&MF == MI.getMF() &&
+ "Invalid machine functions when cloning instruction symbols!");
+
+  setPreInstrSymbol(MF, MI.getPreInstrSymbol());
+  setPostInstrSymbol(MF, MI.getPostInstrSymbol());
+}
+
 uint16_t MachineInstr::mergeFlagsWith(const MachineInstr &Other) const {
   // For now, the just retur

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

2019-04-24 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

@rjmccall Would you expect similar conflicts in explicit visibility to result 
in diagnostics? For example, marking a `static` variable with an explicit 
visibility attribute doesn't warn, instead the explicit visibility attribute is 
silently ignored. GCC 7.3 complains with `warning: ‘__visibility__’ attribute 
ignored [-Wattributes]`


Repository:
  rC Clang

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

https://reviews.llvm.org/D60967



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


r359132 - [OPENMP]Initial support for non-rectangular loop nest.

2019-04-24 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Wed Apr 24 12:58:30 2019
New Revision: 359132

URL: http://llvm.org/viewvc/llvm-project?rev=359132&view=rev
Log:
[OPENMP]Initial support for non-rectangular loop nest.

Added basic semantic analysis for the non-rectangular loop nests for
OpenMP 5.0 support.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/for_loop_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359132&r1=359131&r2=359132&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 24 12:58:30 
2019
@@ -9170,6 +9170,8 @@ def warn_omp_allocate_thread_on_task_tar
   InGroup;
 def err_omp_expected_private_copy_for_allocate : Error<
   "the referenced item is not found in any private clause on the same 
directive">;
+def err_omp_stmt_depends_on_loop_counter : Error<
+  "the loop %select{initializer|condition}0 expression depends on the current 
loop control variable">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=359132&r1=359131&r2=359132&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Apr 24 12:58:30 2019
@@ -4487,6 +4487,8 @@ namespace {
 class OpenMPIterationSpaceChecker {
   /// Reference to Sema.
   Sema &SemaRef;
+  /// Data-sharing stack.
+  DSAStackTy &Stack;
   /// A location for diagnostics (when there is no some better location).
   SourceLocation DefaultLoc;
   /// A location for diagnostics (when increment is not compatible).
@@ -4518,10 +4520,14 @@ class OpenMPIterationSpaceChecker {
   bool TestIsStrictOp = false;
   /// This flag is true when step is subtracted on each iteration.
   bool SubtractStep = false;
+  /// Checks if the provide statement depends on the loop counter.
+  bool doesDependOnLoopCounter(const Stmt *S, bool IsInitializer) const;
 
 public:
-  OpenMPIterationSpaceChecker(Sema &SemaRef, SourceLocation DefaultLoc)
-  : SemaRef(SemaRef), DefaultLoc(DefaultLoc), ConditionLoc(DefaultLoc) {}
+  OpenMPIterationSpaceChecker(Sema &SemaRef, DSAStackTy &Stack,
+  SourceLocation DefaultLoc)
+  : SemaRef(SemaRef), Stack(Stack), DefaultLoc(DefaultLoc),
+ConditionLoc(DefaultLoc) {}
   /// Check init-expr for canonical loop form and save loop counter
   /// variable - #Var and its initialization value - #LB.
   bool checkAndSetInit(Stmt *S, bool EmitDiags = true);
@@ -4579,7 +4585,8 @@ private:
   /// expression.
   bool checkAndSetIncRHS(Expr *RHS);
   /// Helper to set loop counter variable and its initializer.
-  bool setLCDeclAndLB(ValueDecl *NewLCDecl, Expr *NewDeclRefExpr, Expr *NewLB);
+  bool setLCDeclAndLB(ValueDecl *NewLCDecl, Expr *NewDeclRefExpr, Expr *NewLB,
+  bool EmitDiags);
   /// Helper to set upper bound.
   bool setUB(Expr *NewUB, llvm::Optional LessOp, bool StrictOp,
  SourceRange SR, SourceLocation SL);
@@ -4599,7 +4606,7 @@ bool OpenMPIterationSpaceChecker::depend
 
 bool OpenMPIterationSpaceChecker::setLCDeclAndLB(ValueDecl *NewLCDecl,
  Expr *NewLCRefExpr,
- Expr *NewLB) {
+ Expr *NewLB, bool EmitDiags) {
   // State consistency checking to ensure correct usage.
   assert(LCDecl == nullptr && LB == nullptr && LCRef == nullptr &&
  UB == nullptr && Step == nullptr && !TestIsLessOp && !TestIsStrictOp);
@@ -4614,6 +4621,8 @@ bool OpenMPIterationSpaceChecker::setLCD
   CE->getNumArgs() > 0 && CE->getArg(0) != nullptr)
 NewLB = CE->getArg(0)->IgnoreParenImpCasts();
   LB = NewLB;
+  if (EmitDiags)
+(void)doesDependOnLoopCounter(LB, /*IsInitializer=*/true);
   return false;
 }
 
@@ -4632,6 +4641,7 @@ bool OpenMPIterationSpaceChecker::setUB(
   TestIsStrictOp = StrictOp;
   ConditionSrcRange = SR;
   ConditionLoc = SL;
+  (void)doesDependOnLoopCounter(UB, /*IsInitializer=*/false);
   return false;
 }
 
@@ -4697,6 +4707,66 @@ bool OpenMPIterationSpaceChecker::setSte
   return false;
 }
 
+namespace {
+/// Checker for the non-rectangular loops. Checks if the initializer or
+/// condition expression references loop counter variable.
+class LoopCounterRefChecker final
+: public ConstStmtVisitor {
+  Sema &SemaRef;
+  DSAStackTy &Stack;
+  const ValueDecl *CurLCDecl = nullptr;
+  bool IsInitializer = true;
+
+public:
+  bool VisitDeclRefExpr(const DeclRefExpr *E) {
+const ValueDecl *V

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

2019-04-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Yeah, that seems like a missing warning.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60967



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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 196505.
aganea marked 7 inline comments as done.
aganea added a comment.

Adressed some of the comments.


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

https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -459,6 +459,8 @@
   }
 }
 
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)HaveInterrupt3;
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3
+//warning: array subscript is above array bounds [-Warray-bounds]
+//if (NewBldVec[i] == NewBldVec[j]) {
+//~~~^
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: clang/trunk/unittests/Tooling/LookupTest.cpp
===
--- clang/trunk/unittests/Tooling/LookupTest.cpp
+++ clang/trunk/unittests/Tooling/LookupTest.cpp
@@ -217,8 +217,9 @@
   // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
   // it's not visible at [0].
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
-if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old") {
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };
   Visitor.runOver(R"(
 // a.h
Index: clang/trunk/unittests/AST/ASTImporterTest.cpp
===
--- clang/trunk/unittests/AST/ASTImporterTest.cpp
+++ clang/trunk/unittests/AST/ASTImporterTest.cpp
@@ -4048,8 +4048,9 @@
 auto *To

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp:1717-1722
+// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below
+// warning: array subscript is above array bounds [-Warray-bounds]
+#if defined(__GNUC__) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3
+if (i >= 4)
+  continue;
+#endif

tstellar wrote:
> We don't want to  have ifdefs for specific compilers, can this be fixed 
> another way or dropped from the patch?
Removed ifdefs, but I kept the test. This looks like an optimizer issue in GCC, 
however the regression tests are unaffected by this change. I will c-reduce and 
report it if it's still there in the latest GCC.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> Same thing here too.
Not sure I understand, there're plenty of compiler-specific examples in the 
.cmake files. Is there an alternate way to fix this?


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

https://reviews.llvm.org/D61046



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


[PATCH] D50294: [Driver] Use -gdwarf-3 by default for FreeBSD

2019-04-24 Thread Ed Maste via Phabricator via cfe-commits
emaste added a subscriber: arichardson.
emaste added a comment.
Herald added a project: clang.

In D50294#1246980 , @MaskRay wrote:

> In D50294#1245454 , @emaste wrote:
>
> > I'm using this change: 
> > https://github.com/emaste/freebsd/commit/1c3deab6d518feb1a7e88de5b342a139e4022a21
> >
> > In FreeBSD 12 and later we use Clang, lld, and ELF Tool Chain. (We still 
> > have gas and objdump from the outdated binutils 2.17.50.)
>
>
> Will you upstream this commit (I can abandon this one)? I created this 
> revision because I was learning clangDriver... I had a vague and probably 
> incorrect impression that some folks said kgdb or dtrace or whatever might 
> not support DWARF 3 or 4.


Hmm, somehow I forgot about this until being reminded by @arichardson just now. 
I will upstream it.

The primary motivation for staying with DWARF2 was indeed kgdb, based on gdb 
6.1.1, and dtrace (specifically dtrace's build tool ctfconvert) also did not 
support DWARF >2. However, these issues have all been addressed:

1. We're deprecating gdb 6.1.1-based kgdb, using a contemporary gdb port.
2. In any case the kernel is still building with explicit -gdwarf2 today so it 
doesn't matter anyhow, there's no need for the toolchain default to be set 
based on the kernel's needs.
3. DTrace tooling limitations arose as a side effect of our (well, ELF Tool 
Chain's) libdwarf but it has been updated to handle DWARF4 some time ago.

Perhaps I should just upload a new diff to this review?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50294



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


[PATCH] D60523: [clang] Don't segfault on incorrect using directive (PR41400)

2019-04-24 Thread Gauthier via Phabricator via cfe-commits
Tyker added a subscriber: jkooker.
Tyker added a comment.

@jkooker
i don't think it is possible for `ASTContext::getDependentNameType` to deal 
with `NSS = nullptr` except by reporting the error. we probably don't want to 
just report the error because the error could have been handled before invoking 
`ASTContext::getDependentNameType`.
and i agree that it should be reflected in the interface of those functions 
because pointer are often used a optional references. but wouldn't an assert + 
doxygen comment suffice ? instead of changing the interface, implementation and 
all callsites. Pointers as argument that are required not to be null are used 
in a lot of places in LLVM. for example `DependentNameType::DependentNameType` 
takes NNS as a pointer even thought it requires it to be non-null(calls a 
method on it). mixing pointers and references adds a lot of `&` and `*` which 
makes the code harder to read.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60523



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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

aganea wrote:
> tstellar wrote:
> > Same thing here too.
> Not sure I understand, there're plenty of compiler-specific examples in the 
> .cmake files. Is there an alternate way to fix this?
I know there are some, but I don't think a warning like this is important 
enough to fix with a compiler specific work-around.


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

https://reviews.llvm.org/D61046



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


[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1078
+  MCSymbol *EndLabel = std::get<1>(HeapAllocSite);
+  if (BeginLabel->isDefined() && EndLabel->isDefined()) {
+DIType *DITy = std::get<2>(HeapAllocSite);

I think it's worth a comment explaining why some of these labels might not be 
defined. Basically, if the instruction gets replaced anywhere in the codegen 
pipeline, they won't be defined.
Also, it would be LLVM-y to invert the condition and use `continue` to reduce 
indentation:
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code



Comment at: llvm/lib/Target/X86/X86FastISel.cpp:3988
   Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI));
+  Result->cloneInstrSymbols(*FuncInfo.MF, *MI);
   MachineBasicBlock::iterator I(MI);

Please add a new test case that fails if this line of code is removed. I think 
you can start from this C++ code:
```
struct Foo {
  __declspec(allocator) virtual void *alloc();
};
void use_alloc(void*);
void do_alloc(Foo *p) {
  use_alloc(p->alloc());
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61083



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


Re: r359048 - C++ DR2387: a variable template declared wtih (or instantiated with) a

2019-04-24 Thread Richard Smith via cfe-commits
Thanks for the revert.

Looks like this is probably an unfortunate interaction with the
internal_linkage attribute.

On Wed, 24 Apr 2019, 01:48 Ilya Biryukov via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:

> Reverted in r359076.
>
> On Wed, Apr 24, 2019 at 10:28 AM Ilya Biryukov 
> wrote:
>
>> Hi Richard,
>>
>> This seems to break libc++, found while doing an integrate. The errors
>> are:
>>
>> In file included from valarray:4:
>> .../include/c++/v1/valarray:1062:60: error: explicit instantiation
>> declaration of 'valarray<_Tp>' with internal linkage
>> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS
>> valarray::valarray(size_t))
>>^
>> .../include/c++/v1/valarray:1063:60: error: explicit instantiation
>> declaration of '~valarray<_Tp>' with internal linkage
>> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::~valarray())
>>^
>>
>> I will likely revert the change to unbreak the integrate.
>>
>>
>
> --
> Regards,
> Ilya Biryukov
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

2019-04-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment.

Thanks for the checker!  We are hitting a crash on `_Atomic` fields, could you 
take a look?  https://bugs.llvm.org/show_bug.cgi?id=41590  Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58573



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


[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

2019-04-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D58573#1477581 , @gribozavr wrote:

> Thanks for the checker!  We are hitting a crash on `_Atomic` fields, could 
> you take a look?  https://bugs.llvm.org/show_bug.cgi?id=41590  Thanks!


Yup, consider it fixed (in a little while)! Thanks for the report!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58573



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


[PATCH] D61079: Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

2019-04-24 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

I hope I'm getting this right, but if I remember correctly, the significant 
part in this test is what is a FwdDecl as opposed to a definition. The 
identifiers no longer make it into the debug info and you should see no 
difference in the produced binaries with this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61079



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 196516.
plotfi added a comment.

Updated to support properly setting OBJECT/FUNC types as well as section flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

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

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -20,11 +20,14 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +161,260 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+  struct MangledSymbol {
+std::string Name = "";
+uint8_t Type = llvm::ELF::STT_NOTYPE;
+uint8_t Binding = llvm::ELF::STB_LOCAL;
+MangledSymbol(const std::string &Name, uint8_t Type, uint8_t Binding)
+: Name(Name), Type(Type), Binding(Binding) {}
+  };
+  using MangledSymbols = std::map;
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!isa(ND))
+  return false;
+if (Symbols.find(ND) != Symbols.end())
+  return true;
+if (isa(ND))
+  return true;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+std::string MangledName = CGName

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

nikic wrote:
> shafik wrote:
> > aganea wrote:
> > > Fixes
> > > ```
> > > [2097/2979] Building CXX object 
> > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> > > explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > ^
> > > ```
> > You mixed up the error messages but I see what is going on.
> > 
> > So you may want to add a comment since it is not apparent that what is 
> > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > `if..else` which is what is triggering the warning. Since someone may come 
> > by in the future and just remove the braces since it is not apparent why 
> > they are there.
> > 
> > Same below as well.
> EXPECT_* inside if is quite common, I don't think we should add a comment 
> every time it is used.
This is a longstanding issue. gtest even includes this beautiful snippet of 
code:

```
// The GNU compiler emits a warning if nested "if" statements are followed by
// an "else" statement and braces are not used to explicitly disambiguate the
// "else" binding.  This leads to problems with code like:
//
//   if (gate)
// ASSERT_*(condition) << "Some message";
//
// The "switch (0) case 0:" idiom is used to suppress this.
#ifdef __INTEL_COMPILER
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_
#else
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // NOLINT
#endif
```
https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834

The expansion looks something like this:
```
if (user_cond)
  switch (0) case 0: default:
if (const TestResult res = ...);
else fail(res, ...) << "User streaming can follow"
```

It seems new versions of GCC have gotten "smarter" and this pattern no longer 
suppresses the warning as desired. It might be worth disabling -Wdangling-else 
for GCC at this point, since this will be an ongoing problem because LLVM 
typically elides braces when possible.

Oh, we even reported it back in 2017:
https://github.com/google/googletest/issues/1119
... and it was closed as inactive. Woohoo. :(


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

https://reviews.llvm.org/D61046



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi added a comment.

In D60974#1477302 , @phosek wrote:

> Aside from comments that @jakehehrlich already made which I agree with, I'm 
> also concerned about the use of `yaml2obj`. AFAIK that tool has always been 
> intended as a testing, not a production tool, but with this change this would 
> no longer be the case. Specifically, it formalizes the `yaml2obj` input 
> format by making it one of the output formats supported by Clang. I believe 
> that this is such a fundamental change that it should be discussed separately 
> before moving on with this implementation.


My main goal here is to emit some kind of a mergable format. If not yaml or 
tbe, then directly emitting the ELF. yaml is a stepping stone for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


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

2019-04-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Nice! I have no objections (other than the nits) to this! Ill take a second 
look later, because I really need to play around with `CFG` a bit to give a 
definitive LGTM.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:723
+  /// should always inline simply because it's small enough.
+  bool isSmall(AnalysisDeclContext *ADC) const;
+

Your explanation of state splitting, and early returns and the like could be 
added here as well :)



Comment at: clang/lib/Analysis/CFG.cpp:4684
+  if (size() <= 3)
+return true;
+

What are the cases for the size being 2 or 1? Empty function? Is a size of 1 
even possible? Can we enforce something here with asserts?



Comment at: clang/lib/Analysis/CFG.cpp:4686
+
+  // Traverse the CFG until we find a branch. TODO: While this should still be
+  // very fast, maybe we should cache the answer.

Break TODO to new line.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:890
   // Do not inline large functions.
-  if (CalleeCFG->getNumBlockIDs() > Opts.MaxInlinableSize)
+  if (CalleeCFG->getNumBlockIDs() >= Opts.MaxInlinableSize)
 return false;

Nice.



Comment at: clang/test/Analysis/inline-if-constexpr.cpp:16
+void bar() {
+  clang_analyzer_eval(f7()); // expected-warning{{TRUE}}
+}

Aha, we wouldve seen UNKNOWN because the analyzer wouldn't inline these many 
functions, right? Neat!



Comment at: clang/unittests/Analysis/CFGTest.cpp:117
+  expectLinear(true,  "void foo() { do {} while (false); }");
+  expectLinear(true,  "void foo() { foo(); }"); // Not our problem.
 }

What do you mean?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61051



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

To be clear I'm only really giving my thumbs up to a specific high level plan 
that I believe handles a case that llvm-elfabi wouldn't otherwise handle on its 
own. I consider the specifics like the output format still in the air right 
now. I strongly believe that a unique yaml or json format should be used for 
this. We should discuss the specifics of what should go in that format and have 
a discussion on that format. The trick is to capture everything in .o files 
that could wind up mattering to a .tbe file and nothing more. I haven't thought 
though what that entails completely yet but I'll think about it and throw up my 
proposal. Once we have at least one proposal we can start iterating on the 
specifics of it.

Here's the plan I think we can converge which at least covers all of my 
concerns:

1. A flag is added to clang that would cause clang to emit a pre-merged format 
and not invoke the backend
2. The pre-merged format can then be merged into a stub or .tbe file as part of 
the same binary (but an effectively different tool since a symlink and options 
parser will be used instead) as llvm-elfabi
3. The yaml code for this format will go in llvm so that both clang and the new 
merging tool could use it
4. We need to discuss the specifics of the pre-merged format to ensure that it 
captures everything that matters (and hopefully nothing more)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 196521.
hintonda added a comment.

- Remove unnecessary comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_')).upper() + '_CHECK',
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
+  if new_module == 'llvm':
+new_namespace = new_module + '_check'
+  else:
+new_namespace = new_module
   if old_module != new_module:
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
===
--- clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
+++ clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
@@ -6,14 +6,14 @@
 //
 //===--===//
 
-#ifndef LLVM_

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

2019-04-24 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment.

I ran the test you provided and it does throw errors without instantiation

  bash-4.2$ clang -cc1 test/SemaTemplate/test2.cpp
  test/SemaTemplate/test2.cpp:3:7: error: statement requires expression of 
integer type ('int *' invalid)
switch (N) case 0:; // should be diagnosed
^   ~
  test/SemaTemplate/test2.cpp:4:23: error: value of type 'int *' is not 
implicitly convertible to 'int'
switch (0) case N:; // should be diagnosed
^
  test/SemaTemplate/test2.cpp:4:25: warning: switch statement has empty body
switch (0) case N:; // should be diagnosed
  ^
  test/SemaTemplate/test2.cpp:4:25: note: put the semicolon on a separate line 
to silence this warning
  1 warning and 2 errors generated.

However, I am surprised it does since I expected it not to :) For example the 
lit test in this patch will not throw an error without the instantiation. I 
need to debug this further to understand what's happening.  Thanks for taking a 
look!


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

https://reviews.llvm.org/D61027



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


[clang-tools-extra] r359142 - [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

2019-04-24 Thread Don Hinton via cfe-commits
Author: dhinton
Date: Wed Apr 24 14:25:57 2019
New Revision: 359142

URL: http://llvm.org/viewvc/llvm-project?rev=359142&view=rev
Log:
[clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

Summary:
Looks at conditionals and finds cases of ``cast<>``, which will
assert rather than return a null pointer, and ``dyn_cast<>`` where
the return value is not captured. Additionally, finds cases that
match the pattern ``var.foo() && isa(var.foo())``, where the
method is called twice and could be expensive.

.. code-block:: c++

  // Finds cases like these:
  if (auto x = cast(y)) <...>
  if (cast(y)) <...>

  // But not cases like these:
  if (auto f = cast(y)->foo()) <...>
  if (cast(y)->foo()) <...>

Reviewers: alexfh, rjmccall, hokein, aaron.ballman, JonasToth

Reviewed By: aaron.ballman

Subscribers: xbolva00, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra, #clang

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

Added:

clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp

clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt?rev=359142&r1=359141&r2=359142&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt Wed Apr 24 14:25:57 
2019
@@ -4,6 +4,7 @@ add_clang_library(clangTidyLLVMModule
   HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
+  PreferIsaOrDynCastInConditionalsCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp?rev=359142&r1=359141&r2=359142&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp Wed Apr 24 
14:25:57 2019
@@ -12,6 +12,7 @@
 #include "../readability/NamespaceCommentCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
+#include "PreferIsaOrDynCastInConditionalsCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
@@ -25,6 +26,8 @@ public:
 CheckFactories.registerCheck("llvm-include-order");
 CheckFactories.registerCheck(
 "llvm-namespace-comment");
+CheckFactories.registerCheck(
+"llvm-prefer-isa-or-dyn-cast-in-conditionals");
 CheckFactories.registerCheck("llvm-twine-local");
   }
 };

Added: 
clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp?rev=359142&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
 Wed Apr 24 14:25:57 2019
@@ -0,0 +1,135 @@
+//===--- PreferIsaOrDynCastInConditionalsCheck.cpp - clang-tidy
+//-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+} // namespace ast_matchers
+
+namespace tidy {
+namespace llvm {
+
+void PreferIsaOrDynCastInConditionalsCheck::registerMatchers(
+MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  auto Condition = hasCondition(implicitCastExpr(has(
+  callExpr(
+  allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
+anyOf(callee(namedDecl(hasName("cast"))),
+  
callee(namedDecl(hasName("dyn_cast")).bind("dyn_cast")
+  .bind("call";
+
+  auto Any = anyO

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 3 inline comments as done.
aganea added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

rnk wrote:
> nikic wrote:
> > shafik wrote:
> > > aganea wrote:
> > > > Fixes
> > > > ```
> > > > [2097/2979] Building CXX object 
> > > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: 
> > > > suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > > ^
> > > > ```
> > > You mixed up the error messages but I see what is going on.
> > > 
> > > So you may want to add a comment since it is not apparent that what is 
> > > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > > `if..else` which is what is triggering the warning. Since someone may 
> > > come by in the future and just remove the braces since it is not apparent 
> > > why they are there.
> > > 
> > > Same below as well.
> > EXPECT_* inside if is quite common, I don't think we should add a comment 
> > every time it is used.
> This is a longstanding issue. gtest even includes this beautiful snippet of 
> code:
> 
> ```
> // The GNU compiler emits a warning if nested "if" statements are followed by
> // an "else" statement and braces are not used to explicitly disambiguate the
> // "else" binding.  This leads to problems with code like:
> //
> //   if (gate)
> // ASSERT_*(condition) << "Some message";
> //
> // The "switch (0) case 0:" idiom is used to suppress this.
> #ifdef __INTEL_COMPILER
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
> #else
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // NOLINT
> #endif
> ```
> https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834
> 
> The expansion looks something like this:
> ```
> if (user_cond)
>   switch (0) case 0: default:
> if (const TestResult res = ...);
> else fail(res, ...) << "User streaming can follow"
> ```
> 
> It seems new versions of GCC have gotten "smarter" and this pattern no longer 
> suppresses the warning as desired. It might be worth disabling 
> -Wdangling-else for GCC at this point, since this will be an ongoing problem 
> because LLVM typically elides braces when possible.
> 
> Oh, we even reported it back in 2017:
> https://github.com/google/googletest/issues/1119
> ... and it was closed as inactive. Woohoo. :(
So revert those braces and disable -Wdangling-else if CMAKE_COMPILER_IS_GNUCXX 
? (somewhere in HandleLLVMOptions.cmake)



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> aganea wrote:
> > tstellar wrote:
> > > Same thing here too.
> > Not sure I understand, there're plenty of compiler-specific examples in the 
> > .cmake files. Is there an alternate way to fix this?
> I know there are some, but I don't think a warning like this is important 
> enough to fix with a compiler specific work-around.
You mean more like
```
# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
if (NOT MSVC)
  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
endif()
```
Or leave the warning? There's already an #ifdef below in 
LoopPassManagerTest.cpp but it's not at the right place, this simply corrects 
it.


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

https://reviews.llvm.org/D61046



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


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

2019-04-24 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359142: [clang-tidy] Add new checker: 
llvm-prefer-isa-or-dyn-cast-in-conditionals (authored by dhinton, committed by 
).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59802?vs=195270&id=196522#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
  
clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst
  
clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-isa-or-dyn-cast-in-conditionals.cpp

Index: clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
@@ -0,0 +1,135 @@
+//===--- PreferIsaOrDynCastInConditionalsCheck.cpp - clang-tidy
+//-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+} // namespace ast_matchers
+
+namespace tidy {
+namespace llvm {
+
+void PreferIsaOrDynCastInConditionalsCheck::registerMatchers(
+MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  auto Condition = hasCondition(implicitCastExpr(has(
+  callExpr(
+  allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
+anyOf(callee(namedDecl(hasName("cast"))),
+  callee(namedDecl(hasName("dyn_cast")).bind("dyn_cast")
+  .bind("call";
+
+  auto Any = anyOf(
+  has(declStmt(containsDeclaration(
+  0,
+  varDecl(hasInitializer(
+  callExpr(allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
+ callee(namedDecl(hasName("cast")
+  .bind("assign")),
+  Condition);
+
+  auto CallExpression =
+  callExpr(
+  allOf(unless(isMacroID()), unless(cxxMemberCallExpr()),
+allOf(callee(namedDecl(anyOf(hasName("isa"), hasName("cast"),
+ hasName("cast_or_null"),
+ hasName("dyn_cast"),
+ hasName("dyn_cast_or_null")))
+ .bind("func")),
+  hasArgument(0, anyOf(declRefExpr().bind("arg"),
+   cxxMemberCallExpr().bind("arg"))
+  .bind("rhs");
+
+  Finder->addMatcher(
+  stmt(anyOf(ifStmt(Any), whileStmt(Any), doStmt(Condition),
+ binaryOperator(
+ allOf(unless(isExpansionInFileMatching(
+   "llvm/include/llvm/Support/Casting.h")),
+   hasOperatorName("&&"),
+   hasLHS(implicitCastExpr().bind("lhs")),
+   hasRHS(anyOf(implicitCastExpr(has(CallExpression)),
+CallExpression
+ .bind("and"))),
+  this);
+}
+
+void PreferIsaOrDynCastInConditionalsCheck::check(
+const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs("assign")) {
+SourceLocation StartLoc = MatchedDecl->getCallee()->getExprLoc();
+SourceLocation EndLoc =
+StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
+
+diag(MatchedDecl->getBeginLoc(),
+ "cast<> in conditional will assert rather than return a null pointer")
+<< FixItHint::CreateReplacement(SourceRange(StartLoc, EndLoc),
+"dyn_cast");
+  } else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs("call")) {
+SourceLocation StartLoc = MatchedDecl->getCallee()->getExprLoc();
+SourceLocation EndLoc =
+StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
+
+

Re: r359048 - C++ DR2387: a variable template declared wtih (or instantiated with) a

2019-04-24 Thread Richard Smith via cfe-commits
On Wed, 24 Apr 2019 at 13:28, Richard Smith  wrote:
>
> Thanks for the revert.
>
> Looks like this is probably an unfortunate interaction with the 
> internal_linkage attribute.

I think this is a libc++ bug. When
_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT is enabled, we have:

#define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_INTERNAL_LINKAGE
#define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI

template class valarray {
public:
  // ...
  inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1
  explicit valarray(size_t __n); // defined later in this header
  // ...
};

_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t))

So: we declare valarray::valarray(size_t) as having internal
linkage, then add an explicit instantiation declaration for it. This
means any use of it can result in a link error (if it happens to not
be inlined): we disallow linking against a version from a different
TU, but also disallow implicit instantiation in the current TU. This
happens to work prior to my patch only by chance: in a conflict
between available_externally and internal linkage, clang happens to
pick internal.

> On Wed, 24 Apr 2019, 01:48 Ilya Biryukov via cfe-commits, 
>  wrote:
>>
>> Reverted in r359076.
>>
>> On Wed, Apr 24, 2019 at 10:28 AM Ilya Biryukov  wrote:
>>>
>>> Hi Richard,
>>>
>>> This seems to break libc++, found while doing an integrate. The errors are:
>>>
>>> In file included from valarray:4:
>>> .../include/c++/v1/valarray:1062:60: error: explicit instantiation 
>>> declaration of 'valarray<_Tp>' with internal linkage
>>> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t))
>>>^
>>> .../include/c++/v1/valarray:1063:60: error: explicit instantiation 
>>> declaration of '~valarray<_Tp>' with internal linkage
>>> _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::~valarray())
>>>^
>>>
>>> I will likely revert the change to unbreak the integrate.
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

@jakehehrlich - when do you expect to have your idea put up?  I don't think 
that it is fair to have this wait until you have time to put something up that 
can be discussed.  I think that getting this working and then iterating on it 
and migrating it over to some shared representation is something which we could 
do - that tends to be a common thing that I have seen happen multiple times 
with the necessary work never materialising.  Re-use of the YAML structure 
means that we can iterate and identify the pieces that are necessary, though, I 
expect that largely, what will be needed is the name, the binding, the 
visibility, possibly the size (for TBEs), the section, and the type, at least 
for anything which adheres to the GABI.  If you have extensions outside of 
GABI, this will need to be adjusted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


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

2019-04-24 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked 4 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:723
+  /// should always inline simply because it's small enough.
+  bool isSmall(AnalysisDeclContext *ADC) const;
+

Szelethus wrote:
> Your explanation of state splitting, and early returns and the like could be 
> added here as well :)
This function can be controlled through -analyzer-config, which breaks the 
explanation. I guess i'll put it inside the body.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:890
   // Do not inline large functions.
-  if (CalleeCFG->getNumBlockIDs() > Opts.MaxInlinableSize)
+  if (CalleeCFG->getNumBlockIDs() >= Opts.MaxInlinableSize)
 return false;

Szelethus wrote:
> Nice.
Whoops, that was actually accidental.



Comment at: clang/test/Analysis/inline-if-constexpr.cpp:16
+void bar() {
+  clang_analyzer_eval(f7()); // expected-warning{{TRUE}}
+}

Szelethus wrote:
> Aha, we wouldve seen UNKNOWN because the analyzer wouldn't inline these many 
> functions, right? Neat!
Aha, yup.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61051



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


[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 196525.
akhuang added a comment.

- Add test case and comment for undefined labels


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61083

Files:
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/MachineInstr.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/test/CodeGen/X86/label-heapallocsite.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,111 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc(void);
+; void f() {
+;   myalloc()
+; }
+;
+; struct Foo {
+;   __declspec(allocator) virtual void *alloc();
+; };
+; void use_alloc(void*);
+; void do_alloc(Foo *p) {
+;   use_alloc(p->alloc());
+; }
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+%struct.Foo = type { i32 (...)** }
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @f() #0 !dbg !8 {
+entry:
+  %call = call i8* @myalloc(), !dbg !11, !heapallocsite !2
+  ret void, !dbg !12
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+declare dso_local i8* @myalloc() #1
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @do_alloc(%struct.Foo* %p) #0 !dbg !13 {
+entry:
+  %p.addr = alloca %struct.Foo*, align 8
+  store %struct.Foo* %p, %struct.Foo** %p.addr, align 8
+  call void @llvm.dbg.declare(metadata %struct.Foo** %p.addr, metadata !18, metadata !DIExpression()), !dbg !19
+  %0 = load %struct.Foo*, %struct.Foo** %p.addr, align 8, !dbg !20
+  %1 = bitcast %struct.Foo* %0 to i8* (%struct.Foo*)***, !dbg !20
+  %vtable = load i8* (%struct.Foo*)**, i8* (%struct.Foo*)*** %1, align 8, !dbg !20
+  %vfn = getelementptr inbounds i8* (%struct.Foo*)*, i8* (%struct.Foo*)** %vtable, i64 0, !dbg !20
+  %2 = load i8* (%struct.Foo*)*, i8* (%struct.Foo*)** %vfn, align 8, !dbg !20
+  %call = call i8* %2(%struct.Foo* %0), !dbg !20, !heapallocsite !2
+  call void @use_alloc(i8* %call), !dbg !20
+  ret void, !dbg !21
+}
+
+; CHECK-LABEL: do_alloc: # @do_alloc
+; CHECK: .Lheapallocsite2:
+; CHECK: callq *(%rax)
+; CHECK: .Lheapallocsite3:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite2
+; CHECK-NEXT:  .secidx .Lheapallocsite2
+; CHECK-NEXT:  .short .Lheapallocsite3-.Lheapallocsite2
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #2
+
+declare dso_local void @use_alloc(i8*) #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "heapallocsite.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 3, type: !9, scopeLine: 3, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 4, scope: !8)
+!12 = !DILocation(line: 5, scope: !8)
+!13 = distinct !DISubprogram(name: "do_allo

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

2019-04-24 Thread Scott Linder via Phabricator via cfe-commits
scott.linder created this revision.
scott.linder added reviewers: rjmccall, espindola.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC warns on these cases, but we currently just silently ignore the attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D61097

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-visibility.c
  test/SemaCXX/ast-print.cpp
  test/SemaCXX/attr-visibility.cpp


Index: test/SemaCXX/attr-visibility.cpp
===
--- test/SemaCXX/attr-visibility.cpp
+++ test/SemaCXX/attr-visibility.cpp
@@ -18,3 +18,9 @@
 struct x3 {
   static int y;
 } __attribute((visibility("default"))); // expected-warning {{attribute 
'visibility' after definition is ignored}}
+
+const int test4 __attribute__((visibility("default"))) = 0; // 
expected-warning {{'visibility' attribute ignored}}
+
+namespace {
+  int test5 __attribute__((visibility("default"))); // expected-warning 
{{'visibility' attribute ignored}}
+};
Index: test/SemaCXX/ast-print.cpp
===
--- test/SemaCXX/ast-print.cpp
+++ test/SemaCXX/ast-print.cpp
@@ -209,10 +209,8 @@
 }
 }
 
-namespace {
 // CHECK: struct {{\[\[gnu::visibility\(\"hidden\"\)\]\]}} S;
 struct [[gnu::visibility("hidden")]] S;
-}
 
 // CHECK:  struct CXXFunctionalCastExprPrint {
 // CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{};
Index: test/Sema/attr-visibility.c
===
--- test/Sema/attr-visibility.c
+++ test/Sema/attr-visibility.c
@@ -26,3 +26,9 @@
 int x __attribute__((type_visibility("default"))); // expected-error 
{{'type_visibility' attribute only applies to types and namespaces}}
 
 int PR17105 __attribute__((visibility(hidden))); // expected-error 
{{'visibility' attribute requires a string}}
+
+static int test8 __attribute__((visibility("default"))); // expected-warning 
{{'visibility' attribute ignored}}
+static int test9 __attribute__((visibility("hidden"))); // expected-warning 
{{'visibility' attribute ignored}}
+static int test10 __attribute__((visibility("internal"))); // expected-warning 
{{'visibility' attribute ignored}}
+
+static int test11() __attribute__((visibility("default"))); // 
expected-warning {{'visibility' attribute ignored}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2615,6 +2615,14 @@
 return;
   }
 
+  // Visibility attributes have no effect on symbols with internal linkage.
+  if (auto ND = dyn_cast(D)) {
+if (!ND->isExternallyVisible()) {
+  S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+  return;
+}
+  }
+
   // Check that the argument is a string literal.
   StringRef TypeStr;
   SourceLocation LiteralLoc;


Index: test/SemaCXX/attr-visibility.cpp
===
--- test/SemaCXX/attr-visibility.cpp
+++ test/SemaCXX/attr-visibility.cpp
@@ -18,3 +18,9 @@
 struct x3 {
   static int y;
 } __attribute((visibility("default"))); // expected-warning {{attribute 'visibility' after definition is ignored}}
+
+const int test4 __attribute__((visibility("default"))) = 0; // expected-warning {{'visibility' attribute ignored}}
+
+namespace {
+  int test5 __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute ignored}}
+};
Index: test/SemaCXX/ast-print.cpp
===
--- test/SemaCXX/ast-print.cpp
+++ test/SemaCXX/ast-print.cpp
@@ -209,10 +209,8 @@
 }
 }
 
-namespace {
 // CHECK: struct {{\[\[gnu::visibility\(\"hidden\"\)\]\]}} S;
 struct [[gnu::visibility("hidden")]] S;
-}
 
 // CHECK:  struct CXXFunctionalCastExprPrint {
 // CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{};
Index: test/Sema/attr-visibility.c
===
--- test/Sema/attr-visibility.c
+++ test/Sema/attr-visibility.c
@@ -26,3 +26,9 @@
 int x __attribute__((type_visibility("default"))); // expected-error {{'type_visibility' attribute only applies to types and namespaces}}
 
 int PR17105 __attribute__((visibility(hidden))); // expected-error {{'visibility' attribute requires a string}}
+
+static int test8 __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute ignored}}
+static int test9 __attribute__((visibility("hidden"))); // expected-warning {{'visibility' attribute ignored}}
+static int test10 __attribute__((visibility("internal"))); // expected-warning {{'visibility' attribute ignored}}
+
+static int test11() __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute ignored}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2615,6 +2615

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 196531.
hintonda added a comment.

- Change check back to previous version which is much clearer.
- Apply namespace change to new check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629

Files:
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.h
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
  clang-tools-extra/clang-tidy/rename_check.py
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -3,7 +3,7 @@
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
-using namespace clang::tidy::llvm;
+using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
Index: clang-tools-extra/clang-tidy/rename_check.py
===
--- clang-tools-extra/clang-tidy/rename_check.py
+++ clang-tools-extra/clang-tidy/rename_check.py
@@ -14,6 +14,18 @@
 import re
 
 
+def replaceInFileRegex(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  txt = re.sub(sFrom, sTo, txt)
+  print("Replacing '%s' -> '%s' in '%s'..." % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
 def replaceInFile(fileName, sFrom, sTo):
   if sFrom == sTo:
 return
@@ -203,6 +215,8 @@
   clang_tidy_path = os.path.dirname(__file__)
 
   header_guard_variants = [
+  (args.old_check_name.replace('-', '_')).upper() + '_CHECK',
+  (old_module + '_' + check_name_camel).upper(),
   (old_module + '_' + new_check_name_camel).upper(),
   args.old_check_name.replace('-', '_').upper()]
   header_guard_new = (new_module + '_' + new_check_name_camel).upper()
@@ -210,18 +224,19 @@
   old_module_path = os.path.join(clang_tidy_path, old_module)
   new_module_path = os.path.join(clang_tidy_path, new_module)
 
-  # Remove the check from the old module.
-  cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
-  check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
-  if not check_found:
-print("Check name '%s' not found in %s. Exiting." %
+  if (args.old_check_name != args.new_check_name):
+# Remove the check from the old module.
+cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt')
+check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel)
+if not check_found:
+  print("Check name '%s' not found in %s. Exiting." %
 (check_name_camel, cmake_lists))
-return 1
+  return 1
 
-  modulecpp = filter(
-  lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
-  os.listdir(old_module_path))[0]
-  deleteMatchingLines(os.path.join(old_module_path, modulecpp),
+modulecpp = filter(
+lambda p: p.lower() == old_module.lower() + 'tidymodule.cpp',
+os.listdir(old_module_path))[0]
+deleteMatchingLines(os.path.join(old_module_path, modulecpp),
   '\\b' + check_name_camel + '|\\b' + args.old_check_name)
 
   for filename in getListOfFiles(clang_tidy_path):
@@ -249,14 +264,21 @@
   new_module + '/' + new_check_name_camel)
 replaceInFile(filename, check_name_camel, new_check_name_camel)
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+if new_module == 'llvm':
+  new_namespace = new_module + '_check'
+else:
+  new_namespace = new_module
 check_implementation_files = glob.glob(
 os.path.join(old_module_path, new_check_name_camel + '*'))
 for filename in check_implementation_files:
   # Move check implementation to the directory of the new module.
   filename = fileRename(filename, old_module_path, new_module_path)
-  replaceInFile(filename, 'namespace ' + old_module,
-'namespace ' + new_module)
+  replaceInFileRegex(filename, 'namespace ' + old_module + '[^ \n]*',
+ 'namespace ' + new_namespace)
+
+  if (args.old_check_name == args.new_check_name):
+return
 
   # Add check to the new module.
   adapt_cmake(new_module_path, new_check_name_camel)
Index: clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h
=

[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

@phosek - I completely agree, I really would prefer that this not promote the 
`yaml2obj` tool to an officially supported tool.  The reason for using the same 
output format is for testing convenience, and this should not really invoke the 
tool (this should work without a `yaml2obj` or `obj2yaml` available on the 
system).  It is similar to the idea of the IR - just a serialization format for 
testing/debugging/inspecting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

In D60974#1477690 , @compnerd wrote:

> @jakehehrlich - when do you expect to have your idea put up?  I don't think 
> that it is fair to have this wait until you have time to put something up 
> that can be discussed.  I think that getting this working and then iterating 
> on it and migrating it over to some shared representation is something which 
> we could do - that tends to be a common thing that I have seen happen 
> multiple times with the necessary work never materialising.  Re-use of the 
> YAML structure means that we can iterate and identify the pieces that are 
> necessary, though, I expect that largely, what will be needed is the name, 
> the binding, the visibility, possibly the size (for TBEs), the section, and 
> the type, at least for anything which adheres to the GABI.  If you have 
> extensions outside of GABI, this will need to be adjusted.


I don't know when but in the next 2 days likely. Regardless of my timeline I 
don't think I'm blocking anyone. I'm just saying that I will put up a proposal. 
You should also put up a proposal as well. The yaml2obj format is just not well 
designed for any purpose but is far from designed for this purpose. yaml2obj 
works ok-ish for testing. I'd rather start with a minimal format and then add 
things to it rather than starting with a bloated and ill designed format and 
then create a new format from that experience. Creating a minimal format 
shouldn't be hard and I suspect our proposal will look extremely similar; I'd 
wager if you put up a proposal I'd probably just review your proposal rather 
than bother writing out my own.

As for what I think it would entail. I think name, weather or not the symbol is 
defined or undefined, visibility, size, alignment (this is a feature of the 
section and the symbol offset) and type will all mater but not all combinations 
make sense. Sections don't matter as it turns out but alignment does for copy 
relocations. When we started llvm-elfabi I thought at least the section 
permissions mattered but they don't really. While .tbe has things like soname 
and dt_needed that isn't needed here.  The top of the file should probably 
contain the architecture. Other details in the ELF header shouldn't be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

aganea wrote:
> rnk wrote:
> > nikic wrote:
> > > shafik wrote:
> > > > aganea wrote:
> > > > > Fixes
> > > > > ```
> > > > > [2097/2979] Building CXX object 
> > > > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: 
> > > > > suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > > > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > > > ^
> > > > > ```
> > > > You mixed up the error messages but I see what is going on.
> > > > 
> > > > So you may want to add a comment since it is not apparent that what is 
> > > > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > > > `if..else` which is what is triggering the warning. Since someone may 
> > > > come by in the future and just remove the braces since it is not 
> > > > apparent why they are there.
> > > > 
> > > > Same below as well.
> > > EXPECT_* inside if is quite common, I don't think we should add a comment 
> > > every time it is used.
> > This is a longstanding issue. gtest even includes this beautiful snippet of 
> > code:
> > 
> > ```
> > // The GNU compiler emits a warning if nested "if" statements are followed 
> > by
> > // an "else" statement and braces are not used to explicitly disambiguate 
> > the
> > // "else" binding.  This leads to problems with code like:
> > //
> > //   if (gate)
> > // ASSERT_*(condition) << "Some message";
> > //
> > // The "switch (0) case 0:" idiom is used to suppress this.
> > #ifdef __INTEL_COMPILER
> > # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
> > #else
> > # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // 
> > NOLINT
> > #endif
> > ```
> > https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834
> > 
> > The expansion looks something like this:
> > ```
> > if (user_cond)
> >   switch (0) case 0: default:
> > if (const TestResult res = ...);
> > else fail(res, ...) << "User streaming can follow"
> > ```
> > 
> > It seems new versions of GCC have gotten "smarter" and this pattern no 
> > longer suppresses the warning as desired. It might be worth disabling 
> > -Wdangling-else for GCC at this point, since this will be an ongoing 
> > problem because LLVM typically elides braces when possible.
> > 
> > Oh, we even reported it back in 2017:
> > https://github.com/google/googletest/issues/1119
> > ... and it was closed as inactive. Woohoo. :(
> So revert those braces and disable -Wdangling-else if 
> CMAKE_COMPILER_IS_GNUCXX ? (somewhere in HandleLLVMOptions.cmake)
On reflection, I'd rather just add the braces and skip the build system 
complexity. It's what we've done in the past anyway, see rL305507 etc. I'm just 
annoyed at the lack of upstream gtest maintenance.


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

https://reviews.llvm.org/D61046



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


[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

2019-04-24 Thread Don Hinton via Phabricator via cfe-commits
hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: clang-tools-extra/clang-tidy/rename_check.py:267
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+if new_module == 'llvm':

alexfh wrote:
> I believe, we should first construct the new namespace name (using exactly 
> the same code as in add_new_check.py) and then check if it differs from the 
> old one.
Changed this back to the original check which was clearer and easier for the 
reader to grok.

We are checking two distinct things here, so be explicit.  First, we check to 
see if the modules changed, e.g., from say from 'google' to 'modernize', then 
we check to see if the new module is 'llvm' and needs special handling.

Then, and only then, do we need to set the new_namespace variable, and we 
condition that on whether or not the new module name is 'llvm'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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


[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61083



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


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

2019-04-24 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D60349#1477183 , @efriedma wrote:

> > For NotPod, it is aggregate which is specific in the document
>
> Yes, it's an aggregate which is returned in registers... but it's returned in 
> integer registers, unlike Pod which is returned in floating-point registers.


`struct Pod` is HFA (Homogenous Floating-Point Aggregates) which is returned in 
floating-point register, `struct NotPod` is not HFA so still returned in 
integer registers. The ARM64 ABI document will be updated to reflect this, 
thanks.


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

https://reviews.llvm.org/D60349



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


[PATCH] D61098: [clang] [RISC-V] Add validation for inline assembly constraints

2019-04-24 Thread Mitchell Horne via Phabricator via cfe-commits
mhorne created this revision.
mhorne added reviewers: asb, apazos, shiva0217.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, kito-cheng, 
niosHD, sabuasal, simoncook, johnrusso, rbar.
Herald added a project: clang.

GCC supports a small number of RISC-V specific inline assembly
constraints, while currently clang only supports the machine independent
ones for RISC-V targets. Add parsing for these constraints, based on
their descriptions in the GCC documentation [1].

[1] 
https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61098

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h


Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -61,9 +61,7 @@
   ArrayRef getGCCRegAliases() const override;
 
   bool validateAsmConstraint(const char *&Name,
- TargetInfo::ConstraintInfo &Info) const override {
-return false;
-  }
+ TargetInfo::ConstraintInfo &Info) const override;
 
   bool hasFeature(StringRef Feature) const override;
 
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -39,6 +39,27 @@
   return llvm::makeArrayRef(GCCRegAliases);
 }
 
+bool RISCVTargetInfo::validateAsmConstraint(
+const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+  switch (*Name) {
+  default:
+return false;
+  case 'f': // floating-point register.
+  case 'A': // address held in a general purpose register.
+Info.setAllowsRegister();
+return true;
+  case 'I': // I-type 12-bit signed immediate.
+Info.setRequiresImmediate(-2048, 2047);
+return true;
+  case 'J': // integer zero.
+Info.setRequiresImmediate(0);
+return true;
+  case 'K': // 5-bit unsigned immediate for accessing CSRs.
+Info.setRequiresImmediate(0, 31);
+return true;
+  }
+}
+
 void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
   Builder.defineMacro("__ELF__");


Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -61,9 +61,7 @@
   ArrayRef getGCCRegAliases() const override;
 
   bool validateAsmConstraint(const char *&Name,
- TargetInfo::ConstraintInfo &Info) const override {
-return false;
-  }
+ TargetInfo::ConstraintInfo &Info) const override;
 
   bool hasFeature(StringRef Feature) const override;
 
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -39,6 +39,27 @@
   return llvm::makeArrayRef(GCCRegAliases);
 }
 
+bool RISCVTargetInfo::validateAsmConstraint(
+const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+  switch (*Name) {
+  default:
+return false;
+  case 'f': // floating-point register.
+  case 'A': // address held in a general purpose register.
+Info.setAllowsRegister();
+return true;
+  case 'I': // I-type 12-bit signed immediate.
+Info.setRequiresImmediate(-2048, 2047);
+return true;
+  case 'J': // integer zero.
+Info.setRequiresImmediate(0);
+return true;
+  case 'K': // 5-bit unsigned immediate for accessing CSRs.
+Info.setRequiresImmediate(0, 31);
+return true;
+  }
+}
+
 void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
   Builder.defineMacro("__ELF__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61098: [clang] [RISC-V] Add validation for inline assembly constraints

2019-04-24 Thread Mitchell Horne via Phabricator via cfe-commits
mhorne added a comment.

Apologies if I didn't tag the right reviewers; I took a look at those who 
seemed to be most involved with the RISC-V target, but if there's anyone more 
appropriate to review clang changes then feel free to add them instead :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61098



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-24 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 196535.
plotfi added a comment.

First stab at trying to properly handle Weak Symbols


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

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

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+
+__attribute__((weak)) int someWeakFunc() { return 42; }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -20,11 +20,14 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +161,256 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+  struct MangledSymbol {
+std::string Name = "";
+uint8_t Type = llvm::ELF::STT_NOTYPE;
+uint8_t Binding = llvm::ELF::STB_LOCAL;
+MangledSymbol(const std::string &Name, uint8_t Type, uint8_t Binding)
+: Name(Name), Type(Type), Binding(Binding) {}
+  };
+  using MangledSymbols = std::map;
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!isa(ND))
+  return false;
+if (Symbols.find(ND) != Symbols.end())
+  return true;
+if (isa(ND))
+  return true;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+  

[PATCH] D60930: [codeview] Fix symbol names for dynamic initializers and atexit stubs

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D60930#1474460 , @jyu2 wrote:

> Looks good to me.  We are basically de-mangled name for those __E  initialize 
> global’s  function and __F destroy global's function.


Thanks!

I'm going to land this, but @rjmccall, let me know if you think this addition 
to GlobalDecl is a bad idea and we can take it back out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60930



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


Re: r359067 - [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

2019-04-24 Thread Richard Smith via cfe-commits
Thanks! Can you update cxx_status.html to mark P0595R2 as done?

On Tue, 23 Apr 2019 at 19:21, Eric Fiselier via cfe-commits
 wrote:
>
> Author: ericwf
> Date: Tue Apr 23 19:23:30 2019
> New Revision: 359067
>
> URL: http://llvm.org/viewvc/llvm-project?rev=359067&view=rev
> Log:
> [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a
>
> Summary:
> This patch implements `__builtin_is_constant_evaluated` as specifier by 
> [P0595R2](https://wg21.link/p0595r2). It is built on the back of Bill 
> Wendling's work for `__builtin_constant_p()`.
>
> More tests to come, but early feedback is appreciated.
>
> I plan to implement warnings for common mis-usages like those belowe in a 
> following patch:
> ```
> void foo(int x) {
>   if constexpr (std::is_constant_evaluated())) { // condition is always 
> `true`. Should use plain `if` instead.
>foo_constexpr(x);
>   } else {
> foo_runtime(x);
>   }
> }
> ```
>
>
>
> Reviewers: rsmith, MaskRay, bruno, void
>
> Reviewed By: rsmith
>
> Subscribers: dexonsmith, zoecarver, fdeazeve, kristina, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D55500
>
> Added:
> cfe/trunk/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
> cfe/trunk/test/SemaCXX/builtin-is-constant-evaluated.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Builtins.def
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/Basic/Builtins.cpp
> cfe/trunk/lib/CodeGen/CGDecl.cpp
> cfe/trunk/test/Sema/builtins.c
>
> Modified: cfe/trunk/include/clang/Basic/Builtins.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=359067&r1=359066&r2=359067&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
> +++ cfe/trunk/include/clang/Basic/Builtins.def Tue Apr 23 19:23:30 2019
> @@ -500,6 +500,7 @@ BUILTIN(__builtin_vsprintf, "ic*cC*a", "
>  BUILTIN(__builtin_vsnprintf, "ic*zcC*a", "nFP:2:")
>  BUILTIN(__builtin_thread_pointer, "v*", "nc")
>  BUILTIN(__builtin_launder, "v*v*", "nt")
> +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "n", CXX_LANG)
>
>  // GCC exception builtins
>  BUILTIN(__builtin_eh_return, "vzv*", "r") // FIXME: Takes intptr_t, not 
> size_t!
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359067&r1=359066&r2=359067&view=diff
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Apr 23 19:23:30 2019
> @@ -8279,6 +8279,9 @@ bool IntExprEvaluator::VisitBuiltinCallE
>  return Success(false, E);
>}
>
> +  case Builtin::BI__builtin_is_constant_evaluated:
> +return Success(Info.InConstantContext, E);
> +
>case Builtin::BI__builtin_ctz:
>case Builtin::BI__builtin_ctzl:
>case Builtin::BI__builtin_ctzll:
> @@ -11139,6 +11142,7 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
>EvalInfo::EvaluationMode EM = EvalInfo::EM_ConstantExpression;
>EvalInfo Info(Ctx, Result, EM);
>Info.InConstantContext = true;
> +
>if (!::Evaluate(Result.Val, Info, this))
>  return false;
>
>
> Modified: cfe/trunk/lib/Basic/Builtins.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Builtins.cpp?rev=359067&r1=359066&r2=359067&view=diff
> ==
> --- cfe/trunk/lib/Basic/Builtins.cpp (original)
> +++ cfe/trunk/lib/Basic/Builtins.cpp Tue Apr 23 19:23:30 2019
> @@ -75,9 +75,12 @@ bool Builtin::Context::builtinIsSupporte
>bool OclCUnsupported = !LangOpts.OpenCL &&
>   (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES);
>bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == OMP_LANG;
> +  bool CPlusPlusUnsupported =
> +  !LangOpts.CPlusPlus && BuiltinInfo.Langs == CXX_LANG;
>return !BuiltinsUnsupported && !MathBuiltinsUnsupported && 
> !OclCUnsupported &&
>   !OclC1Unsupported && !OclC2Unsupported && !OpenMPUnsupported &&
> - !GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported;
> + !GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported &&
> + !CPlusPlusUnsupported;
>  }
>
>  /// initializeBuiltins - Mark the identifiers for all the builtins with their
>
> Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=359067&r1=359066&r2=359067&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Apr 23 19:23:30 2019
> @@ -1783,7 +1783,8 @@ void CodeGenFunction::EmitAutoVarInit(co
>}
>
>llvm::Constant *constant = nullptr;
> -  if (emission.IsConstantAggregate || D.isConstexpr()) {
> +  if (emission.IsConstantAggregate

r359148 - [codeview] Fix symbol names for dynamic initializers and atexit stubs

2019-04-24 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Wed Apr 24 15:45:44 2019
New Revision: 359148

URL: http://llvm.org/viewvc/llvm-project?rev=359148&view=rev
Log:
[codeview] Fix symbol names for dynamic initializers and atexit stubs

Summary:
Add a new variant to GlobalDecl for these so that we can detect them
more easily during debug info emission and handle them appropriately.

Reviewers: rsmith, rjmccall, jyu2

Subscribers: aprantl, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/GlobalDecl.h
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/debug-info-global-ctor-dtor.cpp

Modified: cfe/trunk/include/clang/AST/GlobalDecl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/GlobalDecl.h?rev=359148&r1=359147&r2=359148&view=diff
==
--- cfe/trunk/include/clang/AST/GlobalDecl.h (original)
+++ cfe/trunk/include/clang/AST/GlobalDecl.h Wed Apr 24 15:45:44 2019
@@ -27,6 +27,12 @@
 
 namespace clang {
 
+enum class DynamicInitKind : unsigned {
+  NoStub = 0,
+  Initializer,
+  AtExit,
+};
+
 /// GlobalDecl - represents a global declaration. This can either be a
 /// CXXConstructorDecl and the constructor type (Base, Complete).
 /// a CXXDestructorDecl and the destructor type (Base, Complete) or
@@ -55,6 +61,8 @@ public:
   GlobalDecl(const OMPDeclareReductionDecl *D) { Init(D); }
   GlobalDecl(const CXXConstructorDecl *D, CXXCtorType Type) : Value(D, Type) {}
   GlobalDecl(const CXXDestructorDecl *D, CXXDtorType Type) : Value(D, Type) {}
+  GlobalDecl(const VarDecl *D, DynamicInitKind StubKind)
+  : Value(D, unsigned(StubKind)) {}
 
   GlobalDecl getCanonicalDecl() const {
 GlobalDecl CanonGD;
@@ -77,6 +85,13 @@ public:
 return static_cast(Value.getInt());
   }
 
+  DynamicInitKind getDynamicInitKind() const {
+assert(isa(getDecl()) &&
+   cast(getDecl())->hasGlobalStorage() &&
+   "Decl is not a global variable!");
+return static_cast(Value.getInt());
+  }
+
   unsigned getMultiVersionIndex() const {
 assert(isa(getDecl()) &&
!isa(getDecl()) &&

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=359148&r1=359147&r2=359148&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Apr 24 15:45:44 2019
@@ -1880,6 +1880,58 @@ StringRef CGDebugInfo::getVTableName(con
   return internString("_vptr$", RD->getNameAsString());
 }
 
+StringRef CGDebugInfo::getDynamicInitializerName(const VarDecl *VD,
+ DynamicInitKind StubKind,
+ llvm::Function *InitFn) {
+  // If we're not emitting codeview, use the mangled name. For Itanium, this is
+  // arbitrary.
+  if (!CGM.getCodeGenOpts().EmitCodeView)
+return InitFn->getName();
+
+  // Print the normal qualified name for the variable, then break off the last
+  // NNS, and add the appropriate other text. Clang always prints the global
+  // variable name without template arguments, so we can use rsplit("::") and
+  // then recombine the pieces.
+  SmallString<128> QualifiedGV;
+  StringRef Quals;
+  StringRef GVName;
+  {
+llvm::raw_svector_ostream OS(QualifiedGV);
+VD->printQualifiedName(OS, getPrintingPolicy());
+std::tie(Quals, GVName) = OS.str().rsplit("::");
+if (GVName.empty())
+  std::swap(Quals, GVName);
+  }
+
+  SmallString<128> InitName;
+  llvm::raw_svector_ostream OS(InitName);
+  if (!Quals.empty())
+OS << Quals << "::";
+
+  switch (StubKind) {
+  case DynamicInitKind::NoStub:
+llvm_unreachable("not an initializer");
+  case DynamicInitKind::Initializer:
+OS << "`dynamic initializer for '";
+break;
+  case DynamicInitKind::AtExit:
+OS << "`dynamic atexit destructor for '";
+break;
+  }
+
+  OS << GVName;
+
+  // Add any template specialization args.
+  if (const auto *VTpl = dyn_cast(VD)) {
+printTemplateArgumentList(OS, VTpl->getTemplateArgs().asArray(),
+  getPrintingPolicy());
+  }
+
+  OS << '\'';
+
+  return internString(OS.str());
+}
+
 void CGDebugInfo::CollectVTableInfo(const CXXRecordDecl *RD, llvm::DIFile 
*Unit,
 SmallVectorImpl &EltTys,
 llvm::DICompositeType *RecordTy) {
@@ -3470,6 +3522,11 @@ void CGDebugInfo::EmitFunctionStart(Glob
   } else if (const auto *OMD = dyn_cast(D)) {
 Name = getObjCMethodName(OMD);
 Flags |= llvm::DINode::FlagPrototyped;
+  } else if (isa(D) &&
+ GD.getDynamicInitKind() != DynamicInitKind::NoStub) {
+// This is a global initializer or atexit destructor for a g

[PATCH] D60930: [codeview] Fix symbol names for dynamic initializers and atexit stubs

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359148: [codeview] Fix symbol names for dynamic initializers 
and atexit stubs (authored by rnk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60930?vs=195949&id=196539#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60930

Files:
  include/clang/AST/GlobalDecl.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGDeclCXX.cpp
  test/CodeGenCXX/debug-info-global-ctor-dtor.cpp

Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1880,6 +1880,58 @@
   return internString("_vptr$", RD->getNameAsString());
 }
 
+StringRef CGDebugInfo::getDynamicInitializerName(const VarDecl *VD,
+ DynamicInitKind StubKind,
+ llvm::Function *InitFn) {
+  // If we're not emitting codeview, use the mangled name. For Itanium, this is
+  // arbitrary.
+  if (!CGM.getCodeGenOpts().EmitCodeView)
+return InitFn->getName();
+
+  // Print the normal qualified name for the variable, then break off the last
+  // NNS, and add the appropriate other text. Clang always prints the global
+  // variable name without template arguments, so we can use rsplit("::") and
+  // then recombine the pieces.
+  SmallString<128> QualifiedGV;
+  StringRef Quals;
+  StringRef GVName;
+  {
+llvm::raw_svector_ostream OS(QualifiedGV);
+VD->printQualifiedName(OS, getPrintingPolicy());
+std::tie(Quals, GVName) = OS.str().rsplit("::");
+if (GVName.empty())
+  std::swap(Quals, GVName);
+  }
+
+  SmallString<128> InitName;
+  llvm::raw_svector_ostream OS(InitName);
+  if (!Quals.empty())
+OS << Quals << "::";
+
+  switch (StubKind) {
+  case DynamicInitKind::NoStub:
+llvm_unreachable("not an initializer");
+  case DynamicInitKind::Initializer:
+OS << "`dynamic initializer for '";
+break;
+  case DynamicInitKind::AtExit:
+OS << "`dynamic atexit destructor for '";
+break;
+  }
+
+  OS << GVName;
+
+  // Add any template specialization args.
+  if (const auto *VTpl = dyn_cast(VD)) {
+printTemplateArgumentList(OS, VTpl->getTemplateArgs().asArray(),
+  getPrintingPolicy());
+  }
+
+  OS << '\'';
+
+  return internString(OS.str());
+}
+
 void CGDebugInfo::CollectVTableInfo(const CXXRecordDecl *RD, llvm::DIFile *Unit,
 SmallVectorImpl &EltTys,
 llvm::DICompositeType *RecordTy) {
@@ -3470,6 +3522,11 @@
   } else if (const auto *OMD = dyn_cast(D)) {
 Name = getObjCMethodName(OMD);
 Flags |= llvm::DINode::FlagPrototyped;
+  } else if (isa(D) &&
+ GD.getDynamicInitKind() != DynamicInitKind::NoStub) {
+// This is a global initializer or atexit destructor for a global variable.
+Name = getDynamicInitializerName(cast(D), GD.getDynamicInitKind(),
+ Fn);
   } else {
 // Use llvm function name.
 Name = Fn->getName();
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -226,13 +226,13 @@
   }
 
   const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();
-  llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(ty, FnName.str(),
-  FI,
-  VD.getLocation());
+  llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(
+  ty, FnName.str(), FI, VD.getLocation());
 
   CodeGenFunction CGF(CGM);
 
-  CGF.StartFunction(&VD, CGM.getContext().VoidTy, fn, FI, FunctionArgList());
+  CGF.StartFunction(GlobalDecl(&VD, DynamicInitKind::AtExit),
+CGM.getContext().VoidTy, fn, FI, FunctionArgList());
 
   llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
 
@@ -603,8 +603,8 @@
 
   CurEHLocation = D->getBeginLoc();
 
-  StartFunction(GlobalDecl(D), getContext().VoidTy, Fn,
-getTypes().arrangeNullaryFunction(),
+  StartFunction(GlobalDecl(D, DynamicInitKind::Initializer),
+getContext().VoidTy, Fn, getTypes().arrangeNullaryFunction(),
 FunctionArgList(), D->getLocation(),
 D->getInit()->getExprLoc());
 
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -41,6 +41,7 @@
 class ObjCIvarDecl;
 class UsingDecl;
 class VarDecl;
+enum class DynamicInitKind : unsigned;
 
 namespace CodeGen {
 class CodeGenModule;
@@ -648,6 +649,12 @@
   /

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

2019-04-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 196538.
sammccall added a comment.

Fix comment wrapping behavior:

- when splitting a comment before //, don't add another //
- if the editor inserts // before the cursor to continue a line comment, indent 
it and adjust to three slashes if needed.

This doesn't mean enter at the *end* of a comment will continue the comment,
but if editors do that (vim does) then it will be respected.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Format.cpp
  clangd/Format.h
  test/clangd/formatting.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/FormatTests.cpp

Index: unittests/clangd/FormatTests.cpp
===
--- /dev/null
+++ unittests/clangd/FormatTests.cpp
@@ -0,0 +1,246 @@
+//===-- FormatTests.cpp - Automatic code formatting tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Format.h"
+#include "Annotations.h"
+#include "SourceCode.h"
+#include "TestFS.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string afterTyped(llvm::StringRef CodeWithCursor,
+   llvm::StringRef Typed) {
+  Annotations Code(CodeWithCursor);
+  unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
+  auto Changes =
+  formatIncremental(Code.code(), Cursor, Typed,
+format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+  tooling::Replacements Merged;
+  for (const auto& R : Changes)
+if (llvm::Error E = Merged.add(R))
+  ADD_FAILURE() << llvm::toString(std::move(E));
+  auto NewCode = tooling::applyAllReplacements(Code.code(), Merged);
+  EXPECT_TRUE(bool(NewCode))
+  << "Bad replacements: " << llvm::toString(NewCode.takeError());
+  NewCode->insert(transformCursorPosition(Cursor, Changes), "^");
+  return *NewCode;
+}
+
+// We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
+void expectAfterNewline(const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+}
+void expectAfter(const char *Typed, const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+}
+
+TEST(FormatIncremental, SplitComment) {
+  expectAfterNewline(R"cpp(
+// this comment was
+^split
+)cpp",
+   R"cpp(
+// this comment was
+// ^split
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// trailing whitespace is not a split
+^   
+)cpp",
+   R"cpp(
+// trailing whitespace is not a split
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// extra   
+^ whitespace
+)cpp",
+   R"cpp(
+// extra
+// ^whitespace
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// triple
+^slash
+)cpp",
+   R"cpp(
+/// triple
+/// ^slash
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// editor continuation
+//^
+)cpp",
+   R"cpp(
+/// editor continuation
+/// ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// break before
+^ // slashes
+)cpp",
+   R"cpp(
+// break before
+^// slashes
+)cpp");
+
+
+  expectAfterNewline(R"cpp(
+int x;  // aligned
+^comment
+)cpp",
+   R"cpp(
+int x;  // aligned
+// ^comment
+)cpp");
+}
+
+TEST(FormatIncremental, Indentation) {
+  expectAfterNewline(R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp",
+   R"cpp(
+void foo() {
+  if (bar)
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+  bar(baz(
+^
+)cpp",
+   R"cpp(
+void foo() {
+  bar(baz(
+  ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+void foo() {
+^}
+)cpp",
+   R"cpp(
+void foo() {
+  ^
+}
+)cpp");
+
+  expectAfterNewline(R"cpp(
+class X {
+protected:
+^
+)cpp",
+   R"cpp(
+class X {
+ protected:
+  ^
+)cpp");
+
+// Mismatched brackets (1)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^}
+}
+)cpp");
+// Mismatched brackets (2)
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^text}
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^text}
+}
+)cpp");
+// Matched brackets
+  expectAfterNewline(R"cpp(
+void foo() {
+  foo{bar(
+^)
+}
+)cpp",
+   R"cpp(
+void foo() {
+  foo {
+bar(
+^)
+}
+)cpp");
+}
+
+TEST(FormatIncremental, FormatPreviousLine) {
+  expectAfterNewline(R"cpp(
+void foo() {
+   untouched( );
+int x=2;
+^
+)cpp",
+ R"cpp(
+void foo() {
+   untouched( );
+   int x = 2;
+   ^
+)cpp");
+
+  expec

[PATCH] D61083: Recommitting r358783 and r358786 "[MS] Emit S_HEAPALLOCSITE debug info" with fixes for buildbot error (undefined assembler label).

2019-04-24 Thread Amy Huang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359149: Recommitting r358783 and r358786 "[MS] Emit 
S_HEAPALLOCSITE debug info" with… (authored by akhuang, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61083?vs=196525&id=196540#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61083

Files:
  llvm/trunk/include/llvm/CodeGen/MachineFunction.h
  llvm/trunk/include/llvm/CodeGen/MachineInstr.h
  llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/trunk/lib/CodeGen/MachineFunction.cpp
  llvm/trunk/lib/CodeGen/MachineInstr.cpp
  llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/trunk/lib/Target/X86/X86FastISel.cpp
  llvm/trunk/test/CodeGen/X86/label-heapallocsite.ll

Index: llvm/trunk/test/CodeGen/X86/label-heapallocsite.ll
===
--- llvm/trunk/test/CodeGen/X86/label-heapallocsite.ll
+++ llvm/trunk/test/CodeGen/X86/label-heapallocsite.ll
@@ -0,0 +1,111 @@
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
+
+; Source to regenerate:
+; $ clang --target=x86_64-windows-msvc -S heapallocsite.c  -g -gcodeview -o t.ll \
+;  -emit-llvm -O0 -Xclang -disable-llvm-passes -fms-extensions
+; __declspec(allocator) char *myalloc(void);
+; void f() {
+;   myalloc()
+; }
+;
+; struct Foo {
+;   __declspec(allocator) virtual void *alloc();
+; };
+; void use_alloc(void*);
+; void do_alloc(Foo *p) {
+;   use_alloc(p->alloc());
+; }
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+%struct.Foo = type { i32 (...)** }
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @f() #0 !dbg !8 {
+entry:
+  %call = call i8* @myalloc(), !dbg !11, !heapallocsite !2
+  ret void, !dbg !12
+}
+
+; CHECK-LABEL: f: # @f
+; CHECK: .Lheapallocsite0:
+; CHECK: callq myalloc
+; CHECK: .Lheapallocsite1:
+; CHECK: retq
+
+declare dso_local i8* @myalloc() #1
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @do_alloc(%struct.Foo* %p) #0 !dbg !13 {
+entry:
+  %p.addr = alloca %struct.Foo*, align 8
+  store %struct.Foo* %p, %struct.Foo** %p.addr, align 8
+  call void @llvm.dbg.declare(metadata %struct.Foo** %p.addr, metadata !18, metadata !DIExpression()), !dbg !19
+  %0 = load %struct.Foo*, %struct.Foo** %p.addr, align 8, !dbg !20
+  %1 = bitcast %struct.Foo* %0 to i8* (%struct.Foo*)***, !dbg !20
+  %vtable = load i8* (%struct.Foo*)**, i8* (%struct.Foo*)*** %1, align 8, !dbg !20
+  %vfn = getelementptr inbounds i8* (%struct.Foo*)*, i8* (%struct.Foo*)** %vtable, i64 0, !dbg !20
+  %2 = load i8* (%struct.Foo*)*, i8* (%struct.Foo*)** %vfn, align 8, !dbg !20
+  %call = call i8* %2(%struct.Foo* %0), !dbg !20, !heapallocsite !2
+  call void @use_alloc(i8* %call), !dbg !20
+  ret void, !dbg !21
+}
+
+; CHECK-LABEL: do_alloc: # @do_alloc
+; CHECK: .Lheapallocsite2:
+; CHECK: callq *(%rax)
+; CHECK: .Lheapallocsite3:
+; CHECK: retq
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite0
+; CHECK-NEXT:  .secidx .Lheapallocsite0
+; CHECK-NEXT:  .short .Lheapallocsite1-.Lheapallocsite0
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+; CHECK-LABEL: .short  4423# Record kind: S_GPROC32_ID
+; CHECK:   .short  4446# Record kind: S_HEAPALLOCSITE
+; CHECK-NEXT:  .secrel32 .Lheapallocsite2
+; CHECK-NEXT:  .secidx .Lheapallocsite2
+; CHECK-NEXT:  .short .Lheapallocsite3-.Lheapallocsite2
+; CHECK-NEXT:  .long 3
+; CHECK-NEXT:  .p2align 2
+; CHECK-LABEL: .short  4431# Record kind: S_PROC_ID_END
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #2
+
+declare dso_local void @use_alloc(i8*) #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "heapallocsite.cpp", directory: "C:\5Csrc\5Ctest", checksumkind: CSK_MD5, checksum: "6d758cfa3834154a04ce8a55102772a9")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 4eff3de99423a62fd6e833e29c71c1e62ba6140b)"}
+!8 = distinct !DISubprogram(name: "f", scope: !1, fil

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

aganea wrote:
> tstellar wrote:
> > aganea wrote:
> > > tstellar wrote:
> > > > Same thing here too.
> > > Not sure I understand, there're plenty of compiler-specific examples in 
> > > the .cmake files. Is there an alternate way to fix this?
> > I know there are some, but I don't think a warning like this is important 
> > enough to fix with a compiler specific work-around.
> You mean more like
> ```
> # Workaround for the gcc 6.1 bug 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
> if (NOT MSVC)
>   set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES 
> COMPILE_FLAGS -Wno-unused-function)
> endif()
> ```
> Or leave the warning? There's already an #ifdef below in 
> LoopPassManagerTest.cpp but it's not at the right place, this simply corrects 
> it.
Ok, I see in this case you are just moving the ifdef out of the code and into 
CMake.  I really don't like either approach, but I guess this is fine.  I would 
limit the work-around to only known broken compilers though.  It looks like 
this might be fixed in gcc 9.


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

https://reviews.llvm.org/D61046



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


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

2019-04-24 Thread Jan Korous via Phabricator via cfe-commits
jkorous created this revision.
jkorous added reviewers: gribozavr, arphaman.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Loading external comments and sorting them is expensive - mostly due to 
getDecomposedLoc() begin expensive. For modules with very large number of 
comments (~100k) this is prohibitively expensive.
In this particular case we are actually not at all interested in getting 
comments for declarations - just using a side-effect of the implementation 
which causes documentation comments to be parsed (doxygen) and attached to 
relevant declarations.

The FIXME in tests is fixed now.


Repository:
  rC Clang

https://reviews.llvm.org/D61103

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -760,16 +760,6 @@
 /// \endcode
 int test_verbatim_2();
 
-// FIXME: we give a bad diagnostic here because we throw away non-documentation
-// comments early.
-//
-// expected-warning@+3 {{'\endcode' command does not terminate a verbatim text block}}
-/// \code
-//  foo
-/// \endcode
-int test_verbatim_3();
-
-
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 int test1; ///< \brief\author Aaa
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12380,19 +12380,7 @@
   }
 
   // See if there are any new comments that are not attached to a decl.
-  ArrayRef Comments = Context.getRawCommentList().getComments();
-  if (!Comments.empty() &&
-  !Comments.back()->isAttached()) {
-// There is at least one comment that not attached to a decl.
-// Maybe it should be attached to one of these decls?
-//
-// Note that this way we pick up not only comments that precede the
-// declaration, but also comments that *follow* the declaration -- thanks to
-// the lookahead in the lexer: we've consumed the semicolon and looked
-// ahead through comments.
-for (unsigned i = 0, e = Group.size(); i != e; ++i)
-  Context.getCommentForDecl(Group[i], &PP);
-  }
+  Context.tryToAttachCommentsToDecls(Group, &PP);
 }
 
 /// ActOnParamDeclarator - Called from Parser::ParseFunctionDeclarator()
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -489,6 +489,134 @@
   return RC;
 }
 
+void ASTContext::tryToAttachCommentsToDecls(ArrayRef Decls, const Preprocessor *PP) {
+  // Explicitly not calling ExternalSource->ReadComments() as we're not
+  // interested in those.
+  ArrayRef RawComments = Comments.getComments();
+  if (RawComments.empty())
+return;
+
+  auto CacheCommentForDecl = [this, PP](const Decl *D, const RawComment *C) {
+RawCommentAndCacheFlags CacheEntry;
+CacheEntry.setKind(RawCommentAndCacheFlags::FromDecl);
+CacheEntry.setRaw(C);
+CacheEntry.setOriginalDecl(D);
+RedeclComments[D] = CacheEntry;
+
+// Always try to parse in order to eventually produce diagnostics.
+comments::FullComment *FC = C->parse(*this, PP, D);
+// But cache only if we don't have a comment yet
+const Decl* Canonical = D->getCanonicalDecl();
+auto ParsedComment = ParsedComments.find(Canonical);
+if (ParsedComment != ParsedComments.end())
+  ParsedComment->second = FC;
+  };
+
+  // explicit comment location caching
+  std::unordered_map>
+  DecomposedCommentBegin;
+  std::unordered_map>
+  DecomposedCommentEnd;
+  std::unordered_map> CommentBeginLine;
+
+  // Don't store the result for long - might go dangling.
+  auto GetCachedCommentBegin =
+  [&DecomposedCommentBegin,
+   this](RawComment *RC) -> const std::pair & {
+assert(RC);
+auto BeginIt = DecomposedCommentBegin.find(RC);
+if (BeginIt != DecomposedCommentBegin.end()) {
+  return BeginIt->second;
+}
+DecomposedCommentBegin[RC] =
+SourceMgr.getDecomposedLoc(RC->getSourceRange().getBegin());
+return DecomposedCommentBegin[RC];
+  };
+  // Don't store the result for long - might go dangling.
+  auto GetCachedCommentEnd =
+  [&DecomposedCommentEnd,
+   this](RawComment *RC) -> const std::pair & {
+assert(RC);
+auto EndIt = DecomposedCommentEnd.find(RC);
+if (EndIt != DecomposedCommentEnd.end()) {
+  return EndIt->second;
+}
+DecomposedCommentEnd[RC] =
+SourceMgr.getDecomposedLoc(RC->getSourceRange().getEnd());
+return DecomposedCommentEnd[RC];
+  };
+  auto GetCachedCommentBeginLine =
+  [&CommentBeginLine,
+   this](const std::pair& CommentBeginLoc) -> unsigned {
+auto BeginFileIt = CommentBeginLine.find(CommentBeg

[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

2019-04-24 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1507
+Multilib::flags_list &Flags) {
+  if (Enabled)
+Flags.push_back(std::string("+") + Flag);

I'd have reduced the duplication and just used `Enabled ? "+" : "-"`



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:197
+SmallString<128> P(D.ResourceDir);
+llvm::sys::path::append(P, D.getTargetTriple(), "lib", M.gccSuffix());
+return std::vector({P.str()});

These two lines repeating the path construction logic could be CSEd into a 
lambda.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:201
+
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+for (const auto &Path : PathsCallback(SelectedMultilib))

This merits a comment about the order they're being inserted.



Comment at: clang/test/Driver/fuchsia.cpp:60
+// CHECK-NOEXCEPTIONS-X86: 
"-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia -fno-exceptions \

Might be worth adding a test that noexcept does *not* appear by default (or 
explicit -fexceptions).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61040



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


[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

2019-04-24 Thread Jan Korous via Phabricator via cfe-commits
jkorous created this revision.
jkorous added reviewers: gribozavr, arphaman.
Herald added subscribers: cfe-commits, dexonsmith, mgrang.
Herald added a project: clang.

For large number of deserialized comments (~100k) the way how we try to attach 
them to declaration in completion comments is too expensive.

Currently we're sorting all the comments by source location (expensive) and 
later bisecting them for every interesting declaration to find the closest 
comment before and after the declaration documentation comment.

This patch tries to just iterate over unsorted comments and see if there's any 
of the decls we're interested in nearby.


Repository:
  rC Clang

https://reviews.llvm.org/D61104

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RawCommentList.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp

Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -579,12 +579,46 @@
   StoredResults.reserve(StoredResults.size() + NumResults);
   if (includeFixIts())
 AllocatedResults.FixItsVector.reserve(NumResults);
+
+  auto CanResultHaveComment =
+  [](const CodeCompletionResult &Result) -> bool {
+const auto &Kind = Result.Kind;
+
+if (Kind == CodeCompletionResult::RK_Declaration ||
+Kind == CodeCompletionResult::RK_Pattern) {
+  if (auto D = Result.getDeclaration()) {
+return true;
+  }
+}
+
+return false;
+  };
+
+  std::vector DeclsThatCanHaveCommentAttached;
   for (unsigned I = 0; I != NumResults; ++I) {
-CodeCompletionString *StoredCompletion
-  = Results[I].CreateCodeCompletionString(S, Context, getAllocator(),
-  getCodeCompletionTUInfo(),
-  includeBriefComments());
-
+if (CanResultHaveComment(Results[I])) {
+  if (auto D = Results[I].getDeclaration()) {
+DeclsThatCanHaveCommentAttached.push_back(D);
+  }
+}
+  }
+
+  auto Cache = getCompletionComments(S.getASTContext(),
+ DeclsThatCanHaveCommentAttached);
+
+  for (unsigned I = 0; I != NumResults; ++I) {
+const RawComment *Comment = nullptr;
+if (CanResultHaveComment(Results[I])) {
+  const auto CachedValueIt = Cache.find(Results[I].getDeclaration());
+  if (CachedValueIt != Cache.end()) {
+Comment = CachedValueIt->second;
+  }
+}
+CodeCompletionString *StoredCompletion =
+Results[I].CreateCodeCompletionString(
+S, Context, getAllocator(), getCodeCompletionTUInfo(),
+includeBriefComments(), Comment);
+
 CXCompletionResult R;
 R.CursorKind = Results[I].CursorKind;
 R.CompletionString = StoredCompletion;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3254,7 +3254,11 @@
   auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
   if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
 return;
-  ArrayRef RawComments = Context->Comments.getComments();
+  ArrayRef RawComments =
+  Context->Comments.getComments(/*Sorted=*/false);
+  Context->Comments.sort();
+  Context->Comments.merge(Context->getLangOpts().CommentOpts);
+
   RecordData Record;
   for (const auto *I : RawComments) {
 Record.clear();
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9223,7 +9223,6 @@
   NextCursor:
 // De-serialized SourceLocations get negative FileIDs for other modules,
 // potentially invalidating the original order. Sort it again.
-llvm::sort(Comments, BeforeThanCompare(SourceMgr));
 Context.Comments.addDeserializedComments(Comments);
   }
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3060,9 +3060,9 @@
 CodeCompletionString *CodeCompletionResult::CreateCodeCompletionString(
 Sema &S, const CodeCompletionContext &CCContext,
 CodeCompletionAllocator &Allocator, CodeCompletionTUInfo &CCTUInfo,
-bool IncludeBriefComments) {
+bool IncludeBriefComments

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

2019-04-24 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/include/clang/Driver/Multilib.h:81
 
+  /// Returns the multilib priority.
+  int priority() const { return Priority; }

Say explicitly that the greatest priority is chosen (sometimes 0 is "best").


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

https://reviews.llvm.org/D60990



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


r359155 - PR41427: This has apparently been fixed already, just add a regression

2019-04-24 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Wed Apr 24 16:45:56 2019
New Revision: 359155

URL: http://llvm.org/viewvc/llvm-project?rev=359155&view=rev
Log:
PR41427: This has apparently been fixed already, just add a regression
test.

Added:
cfe/trunk/test/SemaTemplate/ctad.cpp

Added: cfe/trunk/test/SemaTemplate/ctad.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/ctad.cpp?rev=359155&view=auto
==
--- cfe/trunk/test/SemaTemplate/ctad.cpp (added)
+++ cfe/trunk/test/SemaTemplate/ctad.cpp Wed Apr 24 16:45:56 2019
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+namespace pr41427 {
+  template  class A {
+  public:
+A(void (*)(T)) {}
+  };
+  
+  void D(int) {}
+  
+  void f() {
+A a(&D);
+using T = decltype(a);
+using T = A;
+  }
+}


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


[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

2019-04-24 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:179
+  .flag("+fno-exceptions");
+  Multilib ASan = Multilib("/asan", "", "", 2).flag("+fsanitize=address");
+  Multilibs.push_back(Default);

Add a comment about the priorities.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61040



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


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

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk commandeered this revision.
rnk edited reviewers, added: mgrang; removed: rnk.
rnk added a comment.

In D55229#1455472 , @rnk wrote:

> This patch fixes a lot of LNK4286 warnings when running `check-asan` on 
> Windows, so I'd like to get it committed upstream. Are there any remaining 
> objections? Is it OK if I commandeer the revision and make some minor 
> aesthetic adjustments and land it?


I'll go ahead and do this and try to get it rebased.


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

https://reviews.llvm.org/D55229



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


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

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 196553.
rnk added a comment.
Herald added a project: clang.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55229

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/ms-symbol-linkage.cpp
  clang/test/CodeGenCXX/runtime-dllstorage.cpp
  clang/test/CodeGenObjC/gnu-init.m
  clang/test/CodeGenObjCXX/msabi-stret.mm


Index: clang/test/CodeGenObjCXX/msabi-stret.mm
===
--- clang/test/CodeGenObjCXX/msabi-stret.mm
+++ clang/test/CodeGenObjCXX/msabi-stret.mm
@@ -13,6 +13,5 @@
   return [I m:S()];
 }
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)
-
Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -100,6 +100,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we have dllimport on the load function
-// CHECK-WIN: declare dllimport void @__objc_load
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
Index: clang/test/CodeGenCXX/runtime-dllstorage.cpp
===
--- clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -108,7 +108,7 @@
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-STATIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-DAG: declare dso_local noalias i8* @"??2@YAPAXI@Z"
 // CHECK-MS-DAG: declare dso_local void @_Init_thread_header(i32*)
Index: clang/test/CodeGen/ms-symbol-linkage.cpp
===
--- /dev/null
+++ clang/test/CodeGen/ms-symbol-linkage.cpp
@@ -0,0 +1,20 @@
+// RUN: %clangxx -target aarch64-windows \
+// RUN: -fcxx-exceptions -c -o - %s \
+// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s
+
+void foo1() { throw 1; }
+// CHECK-LABEL: foo1
+// CHECK-NOT: __imp__CxxThrowException
+
+void bar();
+void foo2() noexcept(true) { bar(); }
+// CHECK-LABEL: foo2
+// CHECK-NOT: __imp___std_terminate
+
+struct A {};
+struct B { virtual void f(); };
+struct C : A, virtual B {};
+struct T {};
+T *foo3() { return dynamic_cast((C *)0); }
+// CHECK-LABEL: foo3
+// CHECK-NOT: __imp___RTDynamicCast
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3002,7 +3002,8 @@
 
   if (!Local && getTriple().isOSBinFormatCOFF() &&
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
 if (!FD || FD->hasAttr()) {
   F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);


Index: clang/test/CodeGenObjCXX/msabi-stret.mm
===
--- clang/test/CodeGenObjCXX/msabi-stret.mm
+++ clang/test/CodeGenObjCXX/msabi-stret.mm
@@ -13,6 +13,5 @@
   return [I m:S()];
 }
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)
-
Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -100,6 +100,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we have dllimport on the load function
-// CHECK-WIN: declare dllimport void @__objc_load
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
Index: clang/test/CodeGenCXX/runtime-dllstorage.cpp
===
--- clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -108,7 +108,7 @@
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(

  1   2   >