[PATCH] D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible

2019-10-04 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: sylvestre.ledru, tstellar, beanz, phosek.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous.

Use clang_target_link_libraries() in order to support linking against
libclang-cpp instead of static libraries.


https://reviews.llvm.org/D68448

Files:
  clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt
  clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-move/tool/CMakeLists.txt
  clang-tools-extra/clang-query/tool/CMakeLists.txt
  clang-tools-extra/clang-reorder-fields/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clangd/fuzzer/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt
  clang-tools-extra/clangd/indexer/CMakeLists.txt
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/xpc/test-client/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang-tools-extra/pp-trace/CMakeLists.txt
  clang-tools-extra/tool-template/CMakeLists.txt
  clang-tools-extra/unittests/clang-apply-replacements/CMakeLists.txt
  clang-tools-extra/unittests/clang-change-namespace/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/unittests/clang-include-fixer/CMakeLists.txt
  
clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
  clang-tools-extra/unittests/clang-move/CMakeLists.txt
  clang-tools-extra/unittests/clang-query/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -21,7 +21,7 @@
   TransformerClangTidyCheckTest.cpp
   )
 
-target_link_libraries(ClangTidyTests
+clang_target_link_libraries(ClangTidyTests
   PRIVATE
   clangAST
   clangASTMatchers
@@ -29,6 +29,12 @@
   clangFrontend
   clangLex
   clangSerialization
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  )
+target_link_libraries(ClangTidyTests
+  PRIVATE
   clangTidy
   clangTidyAndroidModule
   clangTidyGoogleModule
@@ -36,7 +42,4 @@
   clangTidyObjCModule
   clangTidyReadabilityModule
   clangTidyUtils
-  clangTooling
-  clangToolingCore
-  clangToolingRefactoring
   )
Index: clang-tools-extra/unittests/clang-query/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-query/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-query/CMakeLists.txt
@@ -11,14 +11,17 @@
   QueryParserTest.cpp
   )
 
-target_link_libraries(ClangQueryTests
+clang_target_link_libraries(ClangQueryTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangDynamicASTMatchers
   clangFrontend
-  clangQuery
   clangSerialization
   clangTooling
   )
+target_link_libraries(ClangQueryTests
+  PRIVATE
+  clangQuery
+  )
Index: clang-tools-extra/unittests/clang-move/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-move/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-move/CMakeLists.txt
@@ -15,16 +15,19 @@
   ClangMoveTests.cpp
   )
 
-target_link_libraries(ClangMoveTests
+clang_target_link_libraries(ClangMoveTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangFormat
   clangFrontend
-  clangMove
   clangRewrite
   clangSerialization
   clangTooling
   clangToolingCore
   )
+target_link_libraries(ClangMoveTests
+  PRIVATE
+  clangMove
+  )
Index: clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
@@ -12,7 +12,7 @@
   FindAllSymbolsTests.cpp
   )
 
-target_link_libraries(FindAllSymbolsTests
+clang_target_link_libraries(FindAllSymbolsTests
   PRIVATE
   clangAST
   clangASTMatchers
@@ -21,5 +21,8 @@
   clangLex
   clangSerialization
   clangTooling
+  )
+target_link_libraries(FindAllSymbolsTests
+  PRIVATE
   findAllSymbols
   )
Index: clang-tools-extra/unittests/clang-include-fixer/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-include-fixer/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-include-fixer/CMakeLists.txt
@@ -16,16 +16,19 @@
   FuzzySymbolIndexTests.cpp
   )
 
-target_link_libraries(ClangIncludeFixerTests
+clang_target_link_libraries(ClangIn

[PATCH] D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible

2019-10-04 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.

I don't think I can review that but I do appreciate the feature!
Maybe we should make that explicit in the release notes?


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

https://reviews.llvm.org/D68448



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


r373707 - [clang-format] [PR43333] Fix C# breaking before function name when using Attributes

2019-10-04 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Fri Oct  4 00:56:49 2019
New Revision: 373707

URL: http://llvm.org/viewvc/llvm-project?rev=373707&view=rev
Log:
[clang-format] [PR4] Fix C# breaking before function name when using 
Attributes

Summary:
This is  a fix for https://bugs.llvm.org/show_bug.cgi?id=4

This comes with 3 main parts

  - C# attributes cause function names on a new line even when 
AlwaysBreakAfterReturnType is set to None
  - Add AlwaysBreakAfterReturnType  to None by default in the Microsoft style,
  - C# unit tests are not using Microsoft style (which we created to define the 
default C# style to match a vanilla C# project).

Reviewers: owenpan, klimek, russellmcc, mitchell-stellar

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

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

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

Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestCSharp.cpp

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=373707&r1=373706&r2=373707&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Fri Oct  4 00:56:49 2019
@@ -489,38 +489,38 @@ struct FormatStyle {
 
   /// Different ways to break after the template declaration.
   enum BreakTemplateDeclarationsStyle {
-  /// Do not force break before declaration.
-  /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
-  /// \code
-  ///template  T foo() {
-  ///}
-  ///template  T foo(int a,
-  ///int b) {
-  ///}
-  /// \endcode
-  BTDS_No,
-  /// Force break after template declaration only when the following
-  /// declaration spans multiple lines.
-  /// \code
-  ///template  T foo() {
-  ///}
-  ///template 
-  ///T foo(int a,
-  ///  int b) {
-  ///}
-  /// \endcode
-  BTDS_MultiLine,
-  /// Always break after template declaration.
-  /// \code
-  ///template 
-  ///T foo() {
-  ///}
-  ///template 
-  ///T foo(int a,
-  ///  int b) {
-  ///}
-  /// \endcode
-  BTDS_Yes
+/// Do not force break before declaration.
+/// ``PenaltyBreakTemplateDeclaration`` is taken into account.
+/// \code
+///template  T foo() {
+///}
+///template  T foo(int a,
+///int b) {
+///}
+/// \endcode
+BTDS_No,
+/// Force break after template declaration only when the following
+/// declaration spans multiple lines.
+/// \code
+///template  T foo() {
+///}
+///template 
+///T foo(int a,
+///  int b) {
+///}
+/// \endcode
+BTDS_MultiLine,
+/// Always break after template declaration.
+/// \code
+///template 
+///T foo() {
+///}
+///template 
+///T foo(int a,
+///  int b) {
+///}
+/// \endcode
+BTDS_Yes
   };
 
   /// The template declaration breaking style to use.
@@ -2186,6 +2186,10 @@ FormatStyle getWebKitStyle();
 /// http://www.gnu.org/prep/standards/standards.html
 FormatStyle getGNUStyle();
 
+/// Returns a format style complying with Microsoft style guide:
+/// 
https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference?view=vs-2017
+FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language);
+
 /// Returns style indicating formatting should be not applied at all.
 FormatStyle getNoStyle();
 

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=373707&r1=373706&r2=373707&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Oct  4 00:56:49 2019
@@ -473,7 +473,10 @@ bool ContinuationIndenter::mustBreak(con
   }
 
   // If the return type spans multiple lines, wrap before the function name.
-  if ((Current.is(TT_FunctionDeclarationName) ||
+  if (((Current.is(TT_FunctionDeclarationName) &&
+// Don't break before a C# function when no break after return type
+(!Style.isCSharp() ||
+ Style.AlwaysBreakAfterReturnType != FormatStyle::RT

[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

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



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1188
 ExprValueKind ObjectKind) {
+  // This function does not take object type into account.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())

sammccall wrote:
> This comment is a little confusing: the very next line takes object type into 
> account :-)
> 
> Maybe: `// Base/derived shadowing is handled elsewhere.`
I also did not like it, thanks for the great suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68335



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


[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68335

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/member-access-qualifiers.cpp


Index: clang/test/CodeCompletion/member-access-qualifiers.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/member-access-qualifiers.cpp
@@ -0,0 +1,13 @@
+struct deque_base {
+  int &size();
+  const int &size() const;
+};
+
+struct deque : private deque_base {
+int size() const;
+};
+
+auto x = deque().
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:18 %s -o - | 
FileCheck %s
+// CHECK: COMPLETION: size : [#int#]size()[# const#]
+// CHECK: COMPLETION: size (Hidden,InBase,Inaccessible) : [#int 
&#]deque_base::size()
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1185,6 +1185,9 @@
 const CXXMethodDecl &Incumbent,
 const Qualifiers &ObjectQuals,
 ExprValueKind ObjectKind) {
+  // Base/derived shadowing is handled elsewhere.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())
+return OverloadCompare::BothViable;
   if (Candidate.isVariadic() != Incumbent.isVariadic() ||
   Candidate.getNumParams() != Incumbent.getNumParams() ||
   Candidate.getMinRequiredArguments() !=
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2546,6 +2546,25 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
+  // Despite the fact that base method matches the ref-qualifier better,
+  // completion results should only include the derived method.
+  auto Completions = completions(R"cpp(
+struct deque_base {
+  float size();
+  double size() const;
+};
+struct deque : deque_base {
+int size() const;
+};
+
+auto x = deque().^
+  )cpp")
+ .Completions;
+  EXPECT_THAT(Completions,
+  ElementsAre(AllOf(ReturnType("int"), Named("size";
+}
+
 TEST(NoCompileCompletionTest, Basic) {
   auto Results = completionsNoCompile(R"cpp(
 void func() {


Index: clang/test/CodeCompletion/member-access-qualifiers.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/member-access-qualifiers.cpp
@@ -0,0 +1,13 @@
+struct deque_base {
+  int &size();
+  const int &size() const;
+};
+
+struct deque : private deque_base {
+int size() const;
+};
+
+auto x = deque().
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:18 %s -o - | FileCheck %s
+// CHECK: COMPLETION: size : [#int#]size()[# const#]
+// CHECK: COMPLETION: size (Hidden,InBase,Inaccessible) : [#int &#]deque_base::size()
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1185,6 +1185,9 @@
 const CXXMethodDecl &Incumbent,
 const Qualifiers &ObjectQuals,
 ExprValueKind ObjectKind) {
+  // Base/derived shadowing is handled elsewhere.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())
+return OverloadCompare::BothViable;
   if (Candidate.isVariadic() != Incumbent.isVariadic() ||
   Candidate.getNumParams() != Incumbent.getNumParams() ||
   Candidate.getMinRequiredArguments() !=
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2546,6 +2546,25 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
+  // Despite the fact that base method matches the ref-qualifier better,
+  // completion results should only include the derived method.
+  auto Completions = completions(R"cpp(
+struct deque_base {
+  float size();
+  double size() const;
+};
+struct deque : deque_base {
+int size() const;
+};
+
+auto x = deque().^
+  )cpp")
+  

[PATCH] D67629: [clang-format] [PR43333] Fix C# breaking before function name when using Attributes

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373707: [clang-format] [PR4] Fix C# breaking before 
function name when using… (authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67629?vs=220375&id=223162#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67629

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestCSharp.cpp

Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1084,7 +1084,7 @@
 }
 
 FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
-  FormatStyle Style = getLLVMStyle();
+  FormatStyle Style = getLLVMStyle(Language);
   Style.ColumnLimit = 120;
   Style.TabWidth = 4;
   Style.IndentWidth = 4;
@@ -1105,6 +1105,8 @@
   Style.AllowShortCaseLabelsOnASingleLine = false;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   Style.AllowShortLoopsOnASingleLine = false;
+  Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   return Style;
 }
 
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -395,6 +395,12 @@
  Keywords.kw_internal)) {
   return true;
 }
+
+// incase its a [XXX] retval func(
+if (AttrTok->Next &&
+AttrTok->Next->startsSequence(tok::identifier, tok::l_paren))
+  return true;
+
 return false;
   }
 
@@ -489,6 +495,8 @@
   } else if (Style.isCpp() && Contexts.back().ContextKind == tok::l_brace &&
  Parent && Parent->isOneOf(tok::l_brace, tok::comma)) {
 Left->Type = TT_DesignatedInitializerLSquare;
+  } else if (IsCSharp11AttributeSpecifier) {
+Left->Type = TT_AttributeSquare;
   } else if (CurrentToken->is(tok::r_square) && Parent &&
  Parent->is(TT_TemplateCloser)) {
 Left->Type = TT_ArraySubscriptLSquare;
@@ -536,8 +544,6 @@
  // Should only be relevant to JavaScript:
  tok::kw_default)) {
 Left->Type = TT_ArrayInitializerLSquare;
-  } else if (IsCSharp11AttributeSpecifier) {
-Left->Type = TT_AttributeSquare;
   } else {
 BindingIncrease = 10;
 Left->Type = TT_ArraySubscriptLSquare;
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -473,7 +473,10 @@
   }
 
   // If the return type spans multiple lines, wrap before the function name.
-  if ((Current.is(TT_FunctionDeclarationName) ||
+  if (((Current.is(TT_FunctionDeclarationName) &&
+// Don't break before a C# function when no break after return type
+(!Style.isCSharp() ||
+ Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None)) ||
(Current.is(tok::kw_operator) && !Previous.is(tok::coloncolon))) &&
   !Previous.is(tok::kw_template) && State.Stack.back().BreakBeforeParameter)
 return true;
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -32,48 +32,76 @@
 
   static std::string
   format(llvm::StringRef Code,
- const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+ const FormatStyle &Style = getMicrosoftStyle(FormatStyle::LK_CSharp)) {
 return format(Code, 0, Code.size(), Style);
   }
 
   static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
-FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
 Style.ColumnLimit = ColumnLimit;
 return Style;
   }
 
   static void verifyFormat(
   llvm::StringRef Code,
-  const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+  const FormatStyle &Style = getMicrosoftStyle(FormatStyle::LK_CSharp)) {
 EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 };
 
 TEST_F(FormatTestCSharp, CSharpClass) {
-  verifyFormat("public class SomeClass {\n"
-   "  void f() {}\n"
-   "  int g() { return 0; }\n"
-   "  void h() {

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

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



Comment at: clang-tools-extra/clangd/Compiler.cpp:66
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getPreprocessorOpts().DetailedRecord = true;
   return CI;

hokein wrote:
> I'm not sure how does this flag impact the size of Preamble/AST, 
> @ilya-biryukov any thoughts?
Have no idea, but why do we need this in the first place?
`PPCallbacks::SourceRangeSkipped` should allow to record all skipped ranges in 
the main file. Can we use it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


r373710 - [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-04 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Fri Oct  4 01:10:27 2019
New Revision: 373710

URL: http://llvm.org/viewvc/llvm-project?rev=373710&view=rev
Log:
[CodeComplete] Ensure object is the same in compareOverloads()

Summary:
This fixes a regression that led to size() not being available in clangd
when completing 'deque().^' and using libc++.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=373710&r1=373709&r2=373710&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Fri Oct  4 01:10:27 2019
@@ -1185,6 +1185,9 @@ static OverloadCompare compareOverloads(
 const CXXMethodDecl &Incumbent,
 const Qualifiers &ObjectQuals,
 ExprValueKind ObjectKind) {
+  // Base/derived shadowing is handled elsewhere.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())
+return OverloadCompare::BothViable;
   if (Candidate.isVariadic() != Incumbent.isVariadic() ||
   Candidate.getNumParams() != Incumbent.getNumParams() ||
   Candidate.getMinRequiredArguments() !=

Added: cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp?rev=373710&view=auto
==
--- cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp (added)
+++ cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp Fri Oct  4 
01:10:27 2019
@@ -0,0 +1,13 @@
+struct deque_base {
+  int &size();
+  const int &size() const;
+};
+
+struct deque : private deque_base {
+int size() const;
+};
+
+auto x = deque().
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:18 %s -o - | 
FileCheck %s
+// CHECK: COMPLETION: size : [#int#]size()[# const#]
+// CHECK: COMPLETION: size (Hidden,InBase,Inaccessible) : [#int 
&#]deque_base::size()


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


[clang-tools-extra] r373710 - [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-04 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Fri Oct  4 01:10:27 2019
New Revision: 373710

URL: http://llvm.org/viewvc/llvm-project?rev=373710&view=rev
Log:
[CodeComplete] Ensure object is the same in compareOverloads()

Summary:
This fixes a regression that led to size() not being available in clangd
when completing 'deque().^' and using libc++.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=373710&r1=373709&r2=373710&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Fri Oct  4 
01:10:27 2019
@@ -2546,6 +2546,25 @@ TEST(CompletionTest, NamespaceDoubleInse
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
+  // Despite the fact that base method matches the ref-qualifier better,
+  // completion results should only include the derived method.
+  auto Completions = completions(R"cpp(
+struct deque_base {
+  float size();
+  double size() const;
+};
+struct deque : deque_base {
+int size() const;
+};
+
+auto x = deque().^
+  )cpp")
+ .Completions;
+  EXPECT_THAT(Completions,
+  ElementsAre(AllOf(ReturnType("int"), Named("size";
+}
+
 TEST(NoCompileCompletionTest, Basic) {
   auto Results = completionsNoCompile(R"cpp(
 void func() {


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


r373709 - [clang-format] [PR43338] C# clang format has space issues betweern C# only keywords

2019-10-04 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Fri Oct  4 01:10:22 2019
New Revision: 373709

URL: http://llvm.org/viewvc/llvm-project?rev=373709&view=rev
Log:
[clang-format] [PR43338] C# clang format has space issues betweern C# only 
keywords

Summary:
When formatting C# there can be issues with a lack of spaces between `using (` 
, `foreach (` and generic types

The C# code

```
public class Foo
{
Dictionary foo;
}

```
will be formatted as

```
public class Foo
{
Dictionaryfoo;
   ^   missing a space
}
```

This revision also reverts some of {D2} in order to make this cleaner and 
resolve an issues seen by @owenpan that the formatting didn't add a space when 
not in a code block

This also transforms C# foreach commands to be seen as tok::kw_for commands (to 
ensure foreach gets the same Brace Wrapping behavior as for without littering 
the code with `if(Style.isCSharp())`

Reviewers: owenpan, klimek, russellmcc, mitchell-stellar

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Modified:
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/FormatTokenLexer.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestCSharp.cpp

Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=373709&r1=373708&r2=373709&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Fri Oct  4 01:10:22 2019
@@ -80,6 +80,8 @@ void FormatTokenLexer::tryMergePreviousT
   return;
 if (tryMergeCSharpNullConditionals())
   return;
+if (tryTransformCSharpForEach())
+  return;
 static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
 if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
   return;
@@ -254,6 +256,21 @@ bool FormatTokenLexer::tryMergeCSharpNul
   return true;
 }
 
+// In C# transform identifier foreach into kw_foreach
+bool FormatTokenLexer::tryTransformCSharpForEach() {
+  if (Tokens.size() < 1)
+return false;
+  auto &Identifier = *(Tokens.end() - 1);
+  if (!Identifier->is(tok::identifier))
+return false;
+  if (Identifier->TokenText != "foreach")
+return false;
+
+  Identifier->Type = TT_ForEachMacro;
+  Identifier->Tok.setKind(tok::kw_for);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)

Modified: cfe/trunk/lib/Format/FormatTokenLexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.h?rev=373709&r1=373708&r2=373709&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.h (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.h Fri Oct  4 01:10:22 2019
@@ -53,6 +53,7 @@ private:
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
+  bool tryTransformCSharpForEach();
 
   bool tryMergeTokens(ArrayRef Kinds, TokenType NewType);
 

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373709&r1=373708&r2=373709&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  4 01:10:22 2019
@@ -2640,10 +2640,6 @@ bool TokenAnnotator::spaceRequiredBetwee
 return Style.Language == FormatStyle::LK_JavaScript ||
!Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
-// using (FileStream fs...
-if (Style.isCSharp() && Left.is(tok::kw_using) &&
-Style.SpaceBeforeParens != FormatStyle::SBPO_Never)
-  return true;
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
@@ -2742,7 +2738,15 @@ bool TokenAnnotator::spaceRequiredBefore
 // and "%d %d"
 if (Left.is(tok::numeric_constant) && Right.is(tok::percent))
   return Right.WhitespaceRange.getEnd() != 
Right.WhitespaceRange.getBegin();
-  } else if (Style.Language == FormatStyle::LK_JavaScript || Style.isCSharp()) 
{
+  } else if (Style.isCSharp()) {
+// space between type and variable e.g. Dictionary foo;
+if (Left.is(TT_TemplateCloser) && Right.is(TT_StartOfName))
+  return true;
+// space between keywords and paren e.g. "using ("
+if (Right.is(tok::l_paren))
+  if (Left.is(tok::kw_using))
+return spaceRequiredBeforeParens(Left);
+  } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;
 // for await ( ...

[PATCH] D67660: [clang-format] [PR43338] C# clang format has space issues betweern C# only keywords

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373709: [clang-format] [PR43338] C# clang format has space 
issues betweern C# only… (authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67660?vs=220503&id=223164#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67660

Files:
  cfe/trunk/lib/Format/FormatTokenLexer.cpp
  cfe/trunk/lib/Format/FormatTokenLexer.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestCSharp.cpp

Index: cfe/trunk/lib/Format/FormatTokenLexer.h
===
--- cfe/trunk/lib/Format/FormatTokenLexer.h
+++ cfe/trunk/lib/Format/FormatTokenLexer.h
@@ -53,6 +53,7 @@
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpNullConditionals();
   bool tryMergeCSharpDoubleQuestion();
+  bool tryTransformCSharpForEach();
 
   bool tryMergeTokens(ArrayRef Kinds, TokenType NewType);
 
Index: cfe/trunk/lib/Format/FormatTokenLexer.cpp
===
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp
@@ -80,6 +80,8 @@
   return;
 if (tryMergeCSharpNullConditionals())
   return;
+if (tryTransformCSharpForEach())
+  return;
 static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
 if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
   return;
@@ -254,6 +256,21 @@
   return true;
 }
 
+// In C# transform identifier foreach into kw_foreach
+bool FormatTokenLexer::tryTransformCSharpForEach() {
+  if (Tokens.size() < 1)
+return false;
+  auto &Identifier = *(Tokens.end() - 1);
+  if (!Identifier->is(tok::identifier))
+return false;
+  if (Identifier->TokenText != "foreach")
+return false;
+
+  Identifier->Type = TT_ForEachMacro;
+  Identifier->Tok.setKind(tok::kw_for);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2640,10 +2640,6 @@
 return Style.Language == FormatStyle::LK_JavaScript ||
!Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
-// using (FileStream fs...
-if (Style.isCSharp() && Left.is(tok::kw_using) &&
-Style.SpaceBeforeParens != FormatStyle::SBPO_Never)
-  return true;
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
@@ -2742,7 +2738,15 @@
 // and "%d %d"
 if (Left.is(tok::numeric_constant) && Right.is(tok::percent))
   return Right.WhitespaceRange.getEnd() != Right.WhitespaceRange.getBegin();
-  } else if (Style.Language == FormatStyle::LK_JavaScript || Style.isCSharp()) {
+  } else if (Style.isCSharp()) {
+// space between type and variable e.g. Dictionary foo;
+if (Left.is(TT_TemplateCloser) && Right.is(TT_StartOfName))
+  return true;
+// space between keywords and paren e.g. "using ("
+if (Right.is(tok::l_paren))
+  if (Left.is(tok::kw_using))
+return spaceRequiredBeforeParens(Left);
+  } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;
 // for await ( ...
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -147,15 +147,27 @@
 }
 
 TEST_F(FormatTestCSharp, CSharpNullConditional) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+
   verifyFormat(
   "public Person(string firstName, string lastName, int? age=null)");
 
-  verifyFormat("switch(args?.Length)");
+  verifyFormat("foo () {\n"
+   "  switch (args?.Length) {}\n"
+   "}",
+   Style);
+
+  verifyFormat("switch (args?.Length) {}", Style);
 
   verifyFormat("public static void Main(string[] args)\n"
"{\n"
"string dirPath = args?[0];\n"
"}");
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+
+  verifyFormat("switch(args?.Length) {}", Style);
 }
 
 TEST_F(FormatTestCSharp, Attributes) {
@@ -221,16 +233,22 @@
 TEST_F(FormatTestCSharp, CSharpUsing) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
-  verifyFormat("public void foo() {\n"
+  verifyFormat("public void foo () {\n"
  

[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373710: [CodeComplete] Ensure object is the same in 
compareOverloads() (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68335?vs=223160&id=223165#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68335

Files:
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
  clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp


Index: cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
===
--- cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
+++ cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
@@ -0,0 +1,13 @@
+struct deque_base {
+  int &size();
+  const int &size() const;
+};
+
+struct deque : private deque_base {
+int size() const;
+};
+
+auto x = deque().
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:18 %s -o - | 
FileCheck %s
+// CHECK: COMPLETION: size : [#int#]size()[# const#]
+// CHECK: COMPLETION: size (Hidden,InBase,Inaccessible) : [#int 
&#]deque_base::size()
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -1185,6 +1185,9 @@
 const CXXMethodDecl &Incumbent,
 const Qualifiers &ObjectQuals,
 ExprValueKind ObjectKind) {
+  // Base/derived shadowing is handled elsewhere.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())
+return OverloadCompare::BothViable;
   if (Candidate.isVariadic() != Incumbent.isVariadic() ||
   Candidate.getNumParams() != Incumbent.getNumParams() ||
   Candidate.getMinRequiredArguments() !=
Index: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
@@ -2546,6 +2546,25 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
+  // Despite the fact that base method matches the ref-qualifier better,
+  // completion results should only include the derived method.
+  auto Completions = completions(R"cpp(
+struct deque_base {
+  float size();
+  double size() const;
+};
+struct deque : deque_base {
+int size() const;
+};
+
+auto x = deque().^
+  )cpp")
+ .Completions;
+  EXPECT_THAT(Completions,
+  ElementsAre(AllOf(ReturnType("int"), Named("size";
+}
+
 TEST(NoCompileCompletionTest, Basic) {
   auto Results = completionsNoCompile(R"cpp(
 void func() {


Index: cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
===
--- cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
+++ cfe/trunk/test/CodeCompletion/member-access-qualifiers.cpp
@@ -0,0 +1,13 @@
+struct deque_base {
+  int &size();
+  const int &size() const;
+};
+
+struct deque : private deque_base {
+int size() const;
+};
+
+auto x = deque().
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:18 %s -o - | FileCheck %s
+// CHECK: COMPLETION: size : [#int#]size()[# const#]
+// CHECK: COMPLETION: size (Hidden,InBase,Inaccessible) : [#int &#]deque_base::size()
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -1185,6 +1185,9 @@
 const CXXMethodDecl &Incumbent,
 const Qualifiers &ObjectQuals,
 ExprValueKind ObjectKind) {
+  // Base/derived shadowing is handled elsewhere.
+  if (Candidate.getDeclContext() != Incumbent.getDeclContext())
+return OverloadCompare::BothViable;
   if (Candidate.isVariadic() != Incumbent.isVariadic() ||
   Candidate.getNumParams() != Incumbent.getNumParams() ||
   Candidate.getMinRequiredArguments() !=
Index: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
@@ -2546,6 +2546,25 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
+  // Des

[PATCH] D67336: [analyzer][NFC] Introduce SuperChecker<>, a convenient alternative to Checker<> for storing subcheckers

2019-10-04 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

In D67336#1664168 , @NoQ wrote:

> I have mixed feelings. Removing boilerplate is good, but the very fact that 
> we're legalizing this pattern indicates that our checkers will keep bloating 
> up, while i always wanted to actually split them instead (like, make 
> sub-checkers into their own separate //classes//, possibly spread out into 
> different files, kinda micro checkers as opposed to monolithic checkers (?)). 
> But i guess it's about whoever gets things done first :)


I completely agree with you about splitting the checkers. I also plan for the 
iterator checkers to separate them from the modelling and make a few mini 
checker classes beside the still huge modelling class. I am confident that that 
is the right direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67336



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


[PATCH] D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible

2019-10-04 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

What do you mean by 'feature'? clang-cpp library or 
`clang_target_link_libraries` function?


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

https://reviews.llvm.org/D68448



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


[PATCH] D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible

2019-10-04 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.

More dynamic, less static usage :)


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

https://reviews.llvm.org/D68448



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


[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1141
+ReceivedRequestCV.wait(Lock,
+   [this, Num] { return NumRequestsReceived == Num; });
+NumRequestsReceived = 0;

kadircet wrote:
> ilya-biryukov wrote:
> > Could we wait with timeout here (a really high one, e.g. 10 seconds) here 
> > and assert we did not hit the timeout?
> > The `wait` helper from `Threading.h` is a nice API to do this.
> > 
> > This would ensure the test never actually runs into an infinite loop in 
> > practice.
> I don't think it is that important, but sure I suppose a failing test is 
> better than a hanging one :D
Totally agree! Thanks!



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1161
   completions(Code, {}, Opts);
-  return Requests.consumeRequests();
+  const auto Reqs = Requests.consumeRequests(Num);
+  EXPECT_EQ(Reqs.size(), Num);

This code got me thinking whether compiler is allowed to do NRVO if the local 
variable is const.
Given that the object is not const, we definitely cannot call a move 
constructor to go from `Reqs` to the return value.
OTOH, if NRVO applies here, this shouldn't matter.

No need to do anything, just thinking out loud.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68273



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


r373711 - [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

2019-10-04 Thread Raphael Isemann via cfe-commits
Author: teemperor
Date: Fri Oct  4 01:26:17 2019
New Revision: 373711

URL: http://llvm.org/viewvc/llvm-project?rev=373711&view=rev
Log:
[lldb][modern-type-lookup] No longer import temporary declarations into the 
persistent AST

Summary:
As we figured out in D67803, importing declarations from a temporary ASTContext 
that were originally from a persistent ASTContext
causes a bunch of duplicated declarations where we end up having declarations 
in the target AST that have no associated ASTImporter that
can complete them.

I haven't figured out how/if we can solve this in the current way we do things 
in LLDB, but in the modern-type-lookup this is solvable
as we have a saner architecture with the ExternalASTMerger. As we can 
(hopefully) make modern-type-lookup the default mode in the future,
I would say we try fixing this issue here. As we don't use the hack that was 
reinstated in D67803 during modern-type-lookup, the test case for this
is essentially just printing any kind of container in `std::` as we would 
otherwise run into the issue that required a hack like D67803.

What this patch is doing in essence is that instead of importing a declaration 
from a temporary ASTContext, we instead check if the
declaration originally came from a persistent ASTContext (e.g. the debug 
information) and we directly import from there. The ExternalASTMerger
is already connected with ASTImporters to these different sources, so this 
patch is essentially just two parts:
1. Mark our temporary ASTContext/ImporterSource as temporary when we import 
from the expression AST.
2. If the ExternalASTMerger sees we import from the expression AST, instead of 
trying to import these temporary declarations, check if we
can instead import from the persistent ASTContext that is already connected. 
This ensures that all records from the persistent source actually
come from the persistent source and are minimally imported in a way that allows 
them to be completed later on in the target AST.

The next step is to run the ASTImporter for these temporary expressions with 
the MinimalImport mode disabled, but that's a follow up patch.

This patch fixes most test failures with modern-type-lookup enabled by default 
(down to 73 failing tests, which includes the 22 import-std-module tests
which need special treatment).

Reviewers: shafik, martong

Reviewed By: martong

Subscribers: aprantl, rnkovacs, christof, abidh, JDevlieghere, lldb-commits

Tags: #lldb

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

Modified:
cfe/trunk/include/clang/AST/ExternalASTMerger.h
cfe/trunk/lib/AST/ExternalASTMerger.cpp

Modified: cfe/trunk/include/clang/AST/ExternalASTMerger.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTMerger.h?rev=373711&r1=373710&r2=373711&view=diff
==
--- cfe/trunk/include/clang/AST/ExternalASTMerger.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTMerger.h Fri Oct  4 01:26:17 2019
@@ -84,13 +84,22 @@ public:
 ASTContext &AST;
 FileManager &FM;
 const OriginMap &OM;
+/// True iff the source only exists temporary, i.e., it will be removed 
from
+/// the ExternalASTMerger during the life time of the ExternalASTMerger.
+bool Temporary;
+/// If the ASTContext of this source has an ExternalASTMerger that imports
+/// into this source, then this will point to that other ExternalASTMerger.
+ExternalASTMerger *Merger;
 
   public:
-ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM)
-: AST(_AST), FM(_FM), OM(_OM) {}
+ImporterSource(ASTContext &AST, FileManager &FM, const OriginMap &OM,
+   bool Temporary = false, ExternalASTMerger *Merger = nullptr)
+: AST(AST), FM(FM), OM(OM), Temporary(Temporary), Merger(Merger) {}
 ASTContext &getASTContext() const { return AST; }
 FileManager &getFileManager() const { return FM; }
 const OriginMap &getOriginMap() const { return OM; }
+bool isTemporary() const { return Temporary; }
+ExternalASTMerger *getMerger() const { return Merger; }
   };
 
 private:
@@ -106,6 +115,12 @@ public:
   ExternalASTMerger(const ImporterTarget &Target,
 llvm::ArrayRef Sources);
 
+  /// Asks all connected ASTImporters if any of them imported the given
+  /// declaration. If any ASTImporter did import the given declaration,
+  /// then this function returns the declaration that D was imported from.
+  /// Returns nullptr if no ASTImporter did import import D.
+  Decl *FindOriginalDecl(Decl *D);
+
   /// Add a set of ASTContexts as possible origins.
   ///
   /// Usually the set will be initialized in the constructor, but long-lived

Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=373711&r1=373710&r2=373711&view=diff
===

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2019-10-04 Thread Andrew Paverd via Phabricator via cfe-commits
ajpaverd updated this revision to Diff 223167.
ajpaverd marked 22 inline comments as done.
ajpaverd added a comment.

Improved Control Flow Guard implementation.

As suggested by @rnk, the CFGuard dispatch mechanism now uses a new 
`cfguardtarget` operand bundle to pass the indirect call target. Instruction
selection adds this target as a new ArgListEntry and sets a new 
`isCFGuardTarget` bit. CC_X86_Win64_C puts this argument in RAX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-fallback.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86FixupCFGuard.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard-setjmp.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll

Index: llvm/test/CodeGen/X86/cfguard-module-flag.ll
===
--- llvm/test/CodeGen/X86/cfguard-module-flag.ll
+++ llvm/test/CodeGen/X86/cfguard-module-flag.ll
@@ -1,9 +1,7 @@
 
 ; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
 ; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
-
-; This test is disabled on Linux
-; UNSUPPORTED: linux
+; Control Flow Guard is currently only available on Windows
 
 ; Test that Control Flow Guard checks are not added in modules with the 
 ; cfguard=1 flag (emit tables but no checks).
Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===
--- llvm/test/CodeGen/X86/cfguard-checks.ll
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -1,8 +1,6 @@
 ; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
 ; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s -check-prefix=X64
-
-; This test is disabled on Linux
-; UNSUPPORTED: linux
+; Control Flow Guard is currently only available on Windows
 
 ; Test that Control Flow Guard checks are correctly added when required.
 
@@ -32,7 +30,8 @@
 attributes #0 = { nocf_check }
 
 
-; Test that Control Flow Guard checks are correctly added as single instructions even at -O0.
+; Test that Control Flow Guard checks are added even at -O0.
+; FIXME Ideally these checks should be added as a single call instruction, as in the optimized case.
 define i32 @func_optnone_cf() #1 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
@@ -46,13 +45,15 @@
 	; X32: 	 leal  _target_func, %eax
 	; X32: 	 movl  %eax, (%esp)
 	; X32: 	 movl  (%esp), %ecx
-	; X32: 	 calll *___guard_check_icall_fptr
+	; X32: 	 movl ___guard_check_icall_fptr, %eax
+	; X32: 	 calll *%eax
 	; X32-NEXT:  calll *%ecx
 
   ; On x86_64, __guard_dispatch_icall_fptr tail calls the function, so there should be only one call instruction.
   ; X64-LABEL: func_optnone_cf
   ; X64:   leaq	target_func(%rip), %rax
-  ; X64:   callq *__guard_dispatch_icall_fptr(%rip)
+  ; X64:   movq __guard_dispatch_icall_fptr(%rip), %rcx
+  ; X64:   callq *%rcx
   ; X64-NOT:   callq
 }
 attributes #1 = { noinline optnone }
@@ -120,5 +121,

[PATCH] D68380: [Driver] NFC: Remove duplicate call to getLibGccType

2019-10-04 Thread Cullen Rhodes via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373712: [Driver] NFC: Remove duplicate call to getLibGccType 
(authored by c-rhodes, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68380?vs=222977&id=223169#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68380

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1186,7 +1186,6 @@
   case ToolChain::UNW_None:
 return;
   case ToolChain::UNW_Libgcc: {
-LibGccType LGT = getLibGccType(D, Args);
 if (LGT == LibGccType::StaticLibGcc)
   CmdArgs.push_back("-lgcc_eh");
 else


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1186,7 +1186,6 @@
   case ToolChain::UNW_None:
 return;
   case ToolChain::UNW_Libgcc: {
-LibGccType LGT = getLibGccType(D, Args);
 if (LGT == LibGccType::StaticLibGcc)
   CmdArgs.push_back("-lgcc_eh");
 else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2019-10-04 Thread Andrew Paverd via Phabricator via cfe-commits
ajpaverd added a comment.

In D65761#1660121 , @rnk wrote:

> Here is some feedback, I apologize for dragging my feet.
>
> Also, I would really like to get feedback from @pcc. He's OOO currently, 
> though.


Thanks for this really helpful feedback @rnk! I think I've been able to address 
most of your comments in the latest update. Your suggested approach of using 
operand bundles for the dispatch mechanism instead of changing calling 
conventions works well and makes the design a lot cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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


[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

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



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:68
 
+// Lexes from end of \p FD until it finds a semicolon.
+llvm::Optional getSemiColonForDecl(const FunctionDecl *FD) {

"Lexes" is probably a not very relevant implementation detail.
I'd say this function probably doesn't need a comment, it's clear what it does 
from its name.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:169
+  tooling::Replacements Replacements;
+  std::string Failure;
+  findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {

NIT: Maybe store `Error` directly?
It would communicate the intent of the variable more clearly.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:175
+// qualifier.
+if (Ref.Qualifier.hasQualifier())
+  return;

NIT: IIUC, this could be simply `if (Ref.Qualifier)`



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:182
+
+// All Targets should be in the same nested name scope, so we can safely
+// chose any one of them.

As discussed before, `Targets` can actually come from different scopes, e.g. 
from ADL.




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:183
+// All Targets should be in the same nested name scope, so we can safely
+// chose any one of them.
+const NamedDecl *ND = Ref.Targets.front();

It just occurred to me that this is a really bad idea. I **think** the order of 
items in `Targets` is non-deterministic.
Could we instead try to check if all scopes are the same and only qualify in 
that case?



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:196
+
+std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+if (auto Err = Replacements.add(

You would want to use `ND->printNestedNameSpecifier()` instead to avoid 
printing inline namespaces:
```
namespace std { inline namespace v1 { 
 struct vector {};
}}
```

^-- I believe the current code would print `std::v1::` for `vector` and we want 
`std::`.
Could you add this example to tests too?



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:199
+tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) {
+  Failure = llvm::formatv("Failed to add qualifier: {0}",
+  llvm::fmt_consume(std::move(Err)));

Maybe return a generic error in the end instead of the last error?
Something like `createStringError(..., "Failed to compute qualifiers, see 
errors in the logs for more details")`.

Should be a better UX for anyone who tries to explore what went wrong.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:223
+
+/// Generates Replacements for changing parameter names in \p Dest to be the
+/// same as in \p Source.

NIT: mention that both function parameters and template parameters are updated 
here.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:226
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  const SourceManager &SM = Dest->getASTContext().getSourceManager();

This does not rename any references to those parameters, right?
E.g.
```
template  void foo(T x);

template  void foo(U x) {}
```

would turn into
```
template  void foo(T x);
```
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647



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


[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

2019-10-04 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham marked an inline comment as done.
simon_tatham added inline comments.



Comment at: clang/lib/AST/Decl.cpp:3107
+if (!ArmMveAliasValid(BuiltinID, getIdentifier()->getName())) {
+  getASTContext().getDiagnostics().Report(
+getLocation(), diag::err_attribute_arm_mve_alias);

aaron.ballman wrote:
> simon_tatham wrote:
> > aaron.ballman wrote:
> > > I'm not certain how comfortable I am with having this function produce a 
> > > diagnostic. That seems like unexpected behavior for a function attempting 
> > > to get a builtin ID. I think this should be the responsibility of the 
> > > caller.
> > The //caller//? But there are many possible callers of this function. You 
> > surely didn't mean to suggest duplicating the diagnostic at all those call 
> > sites.
> > 
> > Perhaps it would make more sense to have all the calculation in this 
> > `getBuiltinID` method move into a function called once, early in the 
> > `FunctionDecl`'s lifetime, which figures out the builtin ID (if any) and 
> > stashes it in a member variable? Then //that// would issue the diagnostic, 
> > if any (and it would be called from a context where that was a sensible 
> > thing to do), and `getBuiltinID` itself would become a mere accessor 
> > function.
> > The caller? But there are many possible callers of this function. You 
> > surely didn't mean to suggest duplicating the diagnostic at all those call 
> > sites.
> 
> Yes, I did. :-) No caller is going to expect that calling a `const` function 
> that gets a builtin ID is going to issue diagnostics and so this runs the 
> risk of generating diagnostics in surprising situations, such as from AST 
> matchers.
> 
> > Perhaps it would make more sense to have all the calculation in this 
> > getBuiltinID method move into a function called once, early in the 
> > FunctionDecl's lifetime, which figures out the builtin ID (if any) and 
> > stashes it in a member variable? Then that would issue the diagnostic, if 
> > any (and it would be called from a context where that was a sensible thing 
> > to do), and getBuiltinID itself would become a mere accessor function.
> 
> That might make sense, but I don't have a good idea of what performance 
> concerns that might raise. If there are a lot of functions and we never need 
> to check if they have a builtin ID, that could be expensive for little gain.
OK – so actually what you meant to suggest was to put the diagnostic at just 
//some// of the call sites for `getBuiltinId`?

With the intended behavior being that the Sema test in this patch should still 
provoke all the expected diagnostics in an ordinary compilation context, but in 
other situations like AST matchers, it would be better for `getBuiltinId` to 
//silently// returns 0 if there's an illegal ArmMveAlias attribute?

(I'm just checking I've understood you correctly before I do the work...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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


[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2019-10-04 Thread Andrew Paverd via Phabricator via cfe-commits
ajpaverd marked 9 inline comments as done.
ajpaverd added inline comments.



Comment at: clang/include/clang/Driver/Options.td:500
 def b : JoinedOrSeparate<["-"], "b">, Flags<[Unsupported]>;
+def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">, Flags<[CC1Option]>,
+  HelpText<"Emit Windows Control Flow Guard tables only (no checks).">;

hans wrote:
> rnk wrote:
> > @hans, WDYT about the -cc1 interface? I think this is fine.
> The -cc1 interface looks good to me, but since these are cc1 options only, I 
> think they should be in CC1Options.td, not Options.td (I realize this is a 
> pre-existing issue with -cfguard).
Thanks for all the helpful feedback @hans. I think I've addressed all your 
comments in the latest revision.



Comment at: llvm/lib/CodeGen/CFGuardLongjmp.cpp:101
+  // targets.
+  for (MachineInstr *Setjmp : SetjmpCalls) {
+MachineBasicBlock *OldMBB = Setjmp->getParent();

rnk wrote:
> Rather than changing the machine CFG, I would suggest using 
> MachineInstr::setPostInstrSymbol, which I've been planning to use for exactly 
> this purpose. :) You can make labels with MCContext::createSymbol, and you'll 
> want to come up with symbols that won't clash with C or C++ symbols. I see 
> MSVC makes labels like `$LN4`, so maybe `$cfgsjN` or something like that. I 
> think MCContext will add numbers for you.
Thanks for the suggestion! It looks like MCContext::createSymbol is private, so 
I'm generating a new symbol name and number for MCContext::getOrCreateSymbol. 
Does this look OK? 



Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:13
+/// for such cases and replaces the pair of instructions with a single
+/// call/invoke. For example:
+///

rnk wrote:
> hans wrote:
> > Naive question: Why do we generate code as in the examples in the first 
> > place, and can't some general optimization pass do this folding? From the 
> > examples it looks like straight-forward constant propagation.
> Actually, I used this test IR, LLVM seems to always fold the memory operand 
> into the call:
> ```
> @fptr = external dso_local global void()*
> define i32 @foo() {
>   %fp1 = load void()*, void()** @fptr
>   call void %fp1()
>   %fp2 = load void()*, void()** @fptr
>   call void %fp2()
>   ret i32 0
> }
> ```
> 
> Maybe it won't do it if there are more parameters, I'm not sure.
> 
> I ran llc with both isels for x64 and ia32, and it always folded the load 
> into the call. Maybe it's best to make this a verification pass that emits an 
> error via MCContext if there is an unfolded load of the CFG check function 
> pointer?
I'm seeing this when compiling with optimizations disabled. When I run llc with 
`-fast-isel=false`, the following slightly modified IR does not fold the memory 
operand into the call:

```
@fptr = external dso_local global void()*
define i32 @foo() #0 {
  %fp1 = load void()*, void()** @fptr
  call void %fp1()
  %fp2 = load void()*, void()** @fptr
  call void %fp2()
  ret i32 0
}
attributes #0 = { noinline optnone }
```

It looks like this is caused by checks for `OptLevel == CodeGenOpt::None` in:

  - SelectionDAGISel::IsLegalToFold
  - X86DAGToDAGISel::IsProfitableToFold
  - X86DAGToDAGISel::PreprocessISelDAG (in this case OptLevel != 
CodeGenOpt::None)

I guess this is not high priority as it only happens at -O0. Should I look into 
enabling these specific optimizations when CFG is enabled, or just emit a 
warning when this happens?



Comment at: llvm/lib/Transforms/CFGuard/CFGuard.cpp:254
+  // register (i.e. RAX on 64-bit X86 targets)
+  GuardDispatch->setCallingConv(CallingConv::CFGuard_Dispatch);
+

rnk wrote:
> So, this isn't going to work if the original calling convention was something 
> other than the default. For x64, the only commonly used non-standard 
> convention would be vectorcall. Changing the convention here in the IR is 
> going to make it hard to pass that along.
> 
> I think as you can see from how much code you have here already, replacing 
> call instructions in general is really hard. These days there are also 
> bundles, which I guess is something else missing from the above.
> 
> Here's a sketch of an alternative approach:
> - load __guard_dispatch_icall_fptr as normal
> - get the pre-existing function type of the callee
> - cast the loaded pointer to the original function pointer type
> - use `CB->setCalledOperand` to replace the call target
> - add the original call target as an "argument bundle" to the existing 
> instruction
> - change SelectionDAGBuilder.cpp to recognize the new bundle kind and create 
> a new ArgListEntry in the argument list
> - make and set a new bit in ArgListEntry, something like isCFGTarget
> - change CC_X86_Win64_C to put CFGTarget arguments in RAX, similar to how 
> CCIfNest puts things in R10
> 
> This way, the original call instruction remains with exactly the same 
> arrange

r373724 - [Format] Fix docs after r373439

2019-10-04 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Fri Oct  4 02:52:54 2019
New Revision: 373724

URL: http://llvm.org/viewvc/llvm-project?rev=373724&view=rev
Log:
[Format] Fix docs after r373439

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=373724&r1=373723&r2=373724&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Fri Oct  4 02:52:54 2019
@@ -2310,11 +2310,12 @@ the configuration (without a prefix: ``A
  std::unique_ptr foo() {} // Won't be affected
 
 **Standard** (``LanguageStandard``)
+  Parse and format C++ constructs compatible with this standard.
+
   .. code-block:: c++
 
  c++03: latest:
  vector > x;   vs. vector> x;
-  Parse and format C++ constructs compatible with this standard.
 
   Possible values:
 


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


[PATCH] D67541: [ClangFormat] Future-proof Standard option, allow floating or pinning to arbitrary lang version

2019-10-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Thanks, rL373724  should fix.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67541



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


[PATCH] D68388: [PR41008][OpenCL] Support restrict keyword in C++ mode

2019-10-04 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Can you please explain why we are deviating from C++ on this? My concern is 
that this can potentially uncover bugs in C++ parsing due to un-handled 
restrict cases that would have to be fixed in some unclear way... I would 
appreciate more detailed analysis before we go ahead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68388



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


[PATCH] D68388: [PR41008][OpenCL] Support restrict keyword in C++ mode

2019-10-04 Thread Kévin Petit via Phabricator via cfe-commits
kpet added a comment.

Something I should probably have explained in the commit message is that the 
functionality provided by `restrict` is already present in C++, where it is 
already exposed using different keywords. If you look at 
`clang/include/clang/Basic/TokenKinds.def`, there are `__restrict__` and 
`__restrict` aliases that end up producing `tok::kw_restrict`. The only thing 
this patch does is enabling the `restrict` keyword in C++ for OpenCL for 
compatibility with OpenCL C (see more on that on the bug). I don't expect any 
special handling will be required since the functionality is already supported 
both in C++ and OpenCL C.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68388



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


[PATCH] D68458: [clangd] Collect missing macro references.

2019-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Semantic highlghting is missing a few macro references.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68458

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -475,6 +475,20 @@
 $Macro[[assert]]($Variable[[x]] != $Variable[[y]]);
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
+)cpp",
+  // highlighting all macro references
+  R"cpp(
+  #ifndef $Macro[[name]]
+  #define $Macro[[name]]
+  #endif
+
+  #define $Macro[[test]]
+  #undef $Macro[[test]]
+  #ifdef $Macro[[test]]
+  #endif
+
+  #if defined($Macro[[test]])
+  #endif
 )cpp",
   R"cpp(
   struct $Class[[S]] {
Index: clang-tools-extra/clangd/CollectMacros.h
===
--- clang-tools-extra/clangd/CollectMacros.h
+++ clang-tools-extra/clangd/CollectMacros.h
@@ -25,7 +25,8 @@
   std::vector Ranges;
 };
 
-/// Collects macro definitions and expansions in the main file. It is used to:
+/// Collects macro references (e.g. definitions, expansions) in the main file.
+/// It is used to:
 ///  - collect macros in the preamble section of the main file (in 
Preamble.cpp)
 ///  - collect macros after the preamble of the main file (in ParsedAST.cpp)
 class CollectMainFileMacros : public PPCallbacks {
@@ -49,6 +50,27 @@
 add(MacroName, MD.getMacroInfo());
   }
 
+  void MacroUndefined(const clang::Token &MacroName,
+  const clang::MacroDefinition &MD,
+  const clang::MacroDirective *Undef) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
+  void Ifdef(SourceLocation Loc, const Token &MacroName,
+ const MacroDefinition &MD) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
+  void Ifndef(SourceLocation Loc, const Token &MacroName,
+  const MacroDefinition &MD) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
+  void Defined(const Token &MacroName, const MacroDefinition &MD,
+   SourceRange Range) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
 private:
   void add(const Token &MacroNameTok, const MacroInfo *MI) {
 if (!InMainFile)
@@ -57,7 +79,7 @@
 if (Loc.isMacroID())
   return;
 
-if (auto Range = getTokenRange(SM, LangOpts, MacroNameTok.getLocation())) {
+if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
   Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
   Out.Ranges.push_back(*Range);
 }


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -475,6 +475,20 @@
 $Macro[[assert]]($Variable[[x]] != $Variable[[y]]);
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
+)cpp",
+  // highlighting all macro references
+  R"cpp(
+  #ifndef $Macro[[name]]
+  #define $Macro[[name]]
+  #endif
+
+  #define $Macro[[test]]
+  #undef $Macro[[test]]
+  #ifdef $Macro[[test]]
+  #endif
+
+  #if defined($Macro[[test]])
+  #endif
 )cpp",
   R"cpp(
   struct $Class[[S]] {
Index: clang-tools-extra/clangd/CollectMacros.h
===
--- clang-tools-extra/clangd/CollectMacros.h
+++ clang-tools-extra/clangd/CollectMacros.h
@@ -25,7 +25,8 @@
   std::vector Ranges;
 };
 
-/// Collects macro definitions and expansions in the main file. It is used to:
+/// Collects macro references (e.g. definitions, expansions) in the main file.
+/// It is used to:
 ///  - collect macros in the preamble section of the main file (in Preamble.cpp)
 ///  - collect macros after the preamble of the main file (in ParsedAST.cpp)
 class CollectMainFileMacros : public PPCallbacks {
@@ -49,6 +50,27 @@
 add(MacroName, MD.getMacroInfo());
   }
 
+  void MacroUndefined(const clang::Token &MacroName,
+  const clang::MacroDefinition &MD,
+  const clang::MacroDirective *Undef) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
+  void Ifdef(SourceLocation Loc, const Token &MacroName,
+ const MacroDefinition &MD) override {
+add(MacroName, MD.getMacroInfo());
+  }
+
+  void Ifndef(SourceLocation Loc, const Token &MacroName,
+  const M

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

2019-10-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.

In D67847#1691898 , @jyknight wrote:

> The `abort()` function raises SIGABRT, for which the default behavior is to 
> trigger a coredump. Do we actually want that behavior?
>
> Either `_exit()` (long available extension, which lld already uses) or 
> `quick_exit()` (the new C standard way) seem possibly preferable?


It's easy to crash LLVM even without this change, so anyone running LLVM better 
have core dumps configured the way they like. Failed asserts raise SIGABRT 
already, for example, and we have tons of those. The only difference is that 
now end users, who may have never configured this stuff, may see more crashes. 
If it's a problem, I'd consider it QoI: we should fix the report_fatal_error to 
use proper diagnostics anyway so end users don't see them as often, just as we 
would treat any other user-visible crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67847



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


r373712 - [Driver] NFC: Remove duplicate call to getLibGccType

2019-10-04 Thread Cullen Rhodes via cfe-commits
Author: c-rhodes
Date: Fri Oct  4 01:26:37 2019
New Revision: 373712

URL: http://llvm.org/viewvc/llvm-project?rev=373712&view=rev
Log:
[Driver] NFC: Remove duplicate call to getLibGccType

Reviewed By: saugustine

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=373712&r1=373711&r2=373712&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Fri Oct  4 01:26:37 2019
@@ -1186,7 +1186,6 @@ static void AddUnwindLibrary(const ToolC
   case ToolChain::UNW_None:
 return;
   case ToolChain::UNW_Libgcc: {
-LibGccType LGT = getLibGccType(D, Args);
 if (LGT == LibGccType::StaticLibGcc)
   CmdArgs.push_back("-lgcc_eh");
 else


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


[PATCH] D68459: [clang-rename] Fix a crash when renaming a class without definition.

2019-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68459

Files:
  clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  clang/test/clang-rename/ForwardClassDecl.cpp


Index: clang/test/clang-rename/ForwardClassDecl.cpp
===
--- /dev/null
+++ clang/test/clang-rename/ForwardClassDecl.cpp
@@ -0,0 +1,4 @@
+class Foo; // CHECK: class Bar;
+Foo *f();  // CHECK: Bar *f();
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s --  | sed 's,//.*,,' | 
FileCheck %s
\ No newline at end of file
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -102,6 +102,10 @@
 
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+if (!RecordDecl->getDefinition()) {
+  USRSet.insert(getUSRForDecl(RecordDecl));
+  return;
+}
 RecordDecl = RecordDecl->getDefinition();
 if (const auto *ClassTemplateSpecDecl =
 dyn_cast(RecordDecl))


Index: clang/test/clang-rename/ForwardClassDecl.cpp
===
--- /dev/null
+++ clang/test/clang-rename/ForwardClassDecl.cpp
@@ -0,0 +1,4 @@
+class Foo; // CHECK: class Bar;
+Foo *f();  // CHECK: Bar *f();
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s --  | sed 's,//.*,,' | FileCheck %s
\ No newline at end of file
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -102,6 +102,10 @@
 
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+if (!RecordDecl->getDefinition()) {
+  USRSet.insert(getUSRForDecl(RecordDecl));
+  return;
+}
 RecordDecl = RecordDecl->getDefinition();
 if (const auto *ClassTemplateSpecDecl =
 dyn_cast(RecordDecl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r373739 - [clangd] update the package-lock.json.

2019-10-04 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Oct  4 05:35:16 2019
New Revision: 373739

URL: http://llvm.org/viewvc/llvm-project?rev=373739&view=rev
Log:
[clangd] update the package-lock.json.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json?rev=373739&r1=373738&r2=373739&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json Fri 
Oct  4 05:35:16 2019
@@ -1,6 +1,6 @@
 {
 "name": "vscode-clangd",
-"version": "0.0.16",
+"version": "0.0.18",
 "lockfileVersion": 1,
 "requires": true,
 "dependencies": {
@@ -22,10 +22,10 @@
 "integrity": 
"sha512-eqxCp82P+JfqL683wwsL73XmFs1eG6qjw+RD3YHx+Jll1r0jNd4dh8QG9NYAeNGA/hnZjeEDgtTskgJULbxpWQ==",
 "dev": true,
 "requires": {
-"fast-deep-equal": "2.0.1",
-"fast-json-stable-stringify": "2.0.0",
-"json-schema-traverse": "0.4.1",
-"uri-js": "4.2.2"
+"fast-deep-equal": "^2.0.1",
+"fast-json-stable-stringify": "^2.0.0",
+"json-schema-traverse": "^0.4.1",
+"uri-js": "^4.2.2"
 }
 },
 "ansi-cyan": {
@@ -58,7 +58,7 @@
 "integrity": "sha1-2CIM9GYIFSXv6lBhTz3mUU36WPE=",
 "dev": true,
 "requires": {
-"buffer-equal": "1.0.0"
+"buffer-equal": "^1.0.0"
 }
 },
 "arr-diff": {
@@ -67,8 +67,8 @@
 "integrity": "sha1-aHwydYFjWI/vfeezb6vklesaOZo=",
 "dev": true,
 "requires": {
-"arr-flatten": "1.1.0",
-"array-slice": "0.2.3"
+"arr-flatten": "^1.0.1",
+"array-slice": "^0.2.3"
 }
 },
 "arr-flatten": {
@@ -101,7 +101,7 @@
 "integrity": "sha1-mjRBDk9OPaI96jdb5b5w8kd47Dk=",
 "dev": true,
 "requires": {
-"array-uniq": "1.0.3"
+"array-uniq": "^1.0.1"
 }
 },
 "array-uniq": {
@@ -122,7 +122,7 @@
 "integrity": 
"sha512-jxwzQpLQjSmWXgwaCZE9Nz+glAG01yF1QnWgbhGwHI5A6FRIEY6IVqtHhIepHqI7/kyEyQEagBC5mBEFlIYvdg==",
 "dev": true,
 "requires": {
-"safer-buffer": "2.1.2"
+"safer-buffer": "~2.1.0"
 }
 },
 "assert-plus": {
@@ -131,6 +131,12 @@
 "integrity": "sha1-8S4PPF13sLHN2RRpQuTpbB5N1SU=",
 "dev": true
 },
+"async": {
+"version": "1.5.2",
+"resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz";,
+"integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=",
+"dev": true
+},
 "asynckit": {
 "version": "0.4.0",
 "resolved": 
"https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz";,
@@ -161,7 +167,7 @@
 "integrity": "sha1-pDAdOJtqQ/m2f/PKEaP2Y342Dp4=",
 "dev": true,
 "requires": {
-"tweetnacl": "0.14.5"
+"tweetnacl": "^0.14.3"
 }
 },
 "block-stream": {
@@ -170,7 +176,7 @@
 "integrity": "sha1-E+v+d4oDIFz+A3UUgeu0szAMEmo=",
 "dev": true,
 "requires": {
-"inherits": "2.0.3"
+"inherits": "~2.0.0"
 }
 },
 "brace-expansion": {
@@ -179,7 +185,7 @@
 "integrity": 
"sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==",
 "dev": true,
 "requires": {
-"balanced-match": "1.0.0",
+"balanced-match": "^1.0.0",
 "concat-map": "0.0.1"
 }
 },
@@ -213,6 +219,17 @@
 "integrity": "sha1-G2gcIf+EAzyCZUMJBolCDRhxUdw=",
 "dev": true
 },
+"clang-format": {
+"version": "1.2.4",
+"resolved": 
"https://registry.npmjs.org/clang-format/-/clang-format-1.2.4.tgz";,
+"integrity": 
"sha512-sw+nrGUp3hvmANd1qF8vZPuezSYQAiXgGBiEtkXTtJnnu6b00fCqkkDIsnRKrNgg4nv6NYZE92ejvOMIXZoejw==",
+"dev": true,
+"requires": {
+"async": "^1.5.2",
+"glob": "^7.0.0",
+"resolve": "^1.1.6"
+}
+},
 "clone": {
 "version": "0.2.0",
 "resolved": "https://registry.npmjs.org/clone/-/clone-0.2.0.tgz";,
@@ -237,9 +254,9 @@
 "integrity": 
"sha512-Bq6+4t+lbM8vhTs/Bef5c5AdEMtapp/iFb6+s4/Hh9MVTt8OLKH

[PATCH] D68458: [clangd] Collect missing macro references.

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68458



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


r373743 - [NFCI] Improve the -Wbool-operation's warning message

2019-10-04 Thread David Bolvansky via cfe-commits
Author: xbolva00
Date: Fri Oct  4 05:55:13 2019
New Revision: 373743

URL: http://llvm.org/viewvc/llvm-project?rev=373743&view=rev
Log:
[NFCI] Improve the -Wbool-operation's warning message

Based on the request from the post commit review. Also added one new test.


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/Sema/warn-bitwise-negation-bool.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=373743&r1=373742&r2=373743&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct  4 05:55:13 
2019
@@ -6638,7 +6638,7 @@ def note_member_declared_here : Note<
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
 def warn_bitwise_negation_bool : Warning<
-  "bitwise negation of a boolean expression always evaluates to 'true'">,
+  "bitwise negation of a boolean expression; did you mean a logicial 
negation?">,
   InGroup>;
 def err_decrement_bool : Error<"cannot decrement expression of type bool">;
 def warn_increment_bool : Warning<

Modified: cfe/trunk/test/Sema/warn-bitwise-negation-bool.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-bitwise-negation-bool.c?rev=373743&r1=373742&r2=373743&view=diff
==
--- cfe/trunk/test/Sema/warn-bitwise-negation-bool.c (original)
+++ cfe/trunk/test/Sema/warn-bitwise-negation-bool.c Fri Oct  4 05:55:13 2019
@@ -12,9 +12,11 @@ typedef _Bool boolean;
 #endif
 
 void test(boolean b, int i) {
-  b = ~b; // expected-warning {{bitwise negation of a boolean expression 
always evaluates to 'true'}}
+  b = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean a logicial negation?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
-  b = ~(b); // expected-warning {{bitwise negation of a boolean expression 
always evaluates to 'true'}}
+  b = ~(b); // expected-warning {{bitwise negation of a boolean expression; 
did you mean a logicial negation?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
   b = ~i;
+  i = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean a logicial negation?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
 }


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


[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-10-04 Thread Edward Jones via Phabricator via cfe-commits
edward-jones planned changes to this revision.
edward-jones added a comment.

I'm reworking this at the moment and adding some tests. Standby!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68391



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


r373744 - [clang] Prevent false positives in arm-mfpu-none codegen test.

2019-10-04 Thread Simon Tatham via cfe-commits
Author: statham
Date: Fri Oct  4 06:01:41 2019
New Revision: 373744

URL: http://llvm.org/viewvc/llvm-project?rev=373744&view=rev
Log:
[clang] Prevent false positives in arm-mfpu-none codegen test.

A user pointed out to me in private email that this test will fail if
it sees the letter 's' followed by a digit in any part of clang's
assembly output after the function label. That includes the .ident at
the end, which can include a full pathname or hostname or both from
the system clang was built on. So if that path or hostname includes
any text like 's5' then it will cause the test to fail.

Fixed by adding a check for `.fnend`, to limit the scope of the
`CHECK-NOT` to only the actual generated code for the test function.

(Committed without prior review on the basis that it's a simple and
obvious pure test-suite fix and also in a test I contributed myself.)

Modified:
cfe/trunk/test/CodeGen/arm-mfpu-none.c

Modified: cfe/trunk/test/CodeGen/arm-mfpu-none.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm-mfpu-none.c?rev=373744&r1=373743&r2=373744&view=diff
==
--- cfe/trunk/test/CodeGen/arm-mfpu-none.c (original)
+++ cfe/trunk/test/CodeGen/arm-mfpu-none.c Fri Oct  4 06:01:41 2019
@@ -3,6 +3,7 @@
 
 // CHECK-LABEL: compute
 // CHECK-NOT: {{s[0-9]}}
+// CHECK: .fnend
 float compute(float a, float b) {
   return (a+b) * (a-b);
 }


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


[PATCH] D68436: [clang-scan-deps] Improve string/character literal skipping

2019-10-04 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk accepted this revision.
kousikk marked an inline comment as done.
kousikk added inline comments.



Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:206
+if (*First == '\\') {
+  if (++First == End)
+return;

dexonsmith wrote:
> > Should you also check if the character right after a backslash is equal to 
> > Terminator and if it is, continue on without terminating?
> > 
> 
> @kousikk, that was handled before this patch.  This `++First` combined with 
> the one in the loop increment handles skipping over an escaped terminator.
Ah makes sense, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68436



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


r373746 - [clang-format] [PR42417] clang-format inserts a space after '->' for operator->() overloading

2019-10-04 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Fri Oct  4 06:24:15 2019
New Revision: 373746

URL: http://llvm.org/viewvc/llvm-project?rev=373746&view=rev
Log:
[clang-format] [PR42417] clang-format inserts a space after '->' for 
operator->() overloading

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

This revision removes the extra space between the opertor-> and the parens ()

```
class Bug {
auto operator-> () -> int*;
auto operator++(int) -> int;
};
```

Reviewers: klimek, owenpan, byoungyoung, mitchell-stellar

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

Tags: #clang-format, #clang

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373746&r1=373745&r2=373746&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  4 06:24:15 2019
@@ -1393,7 +1393,9 @@ private:
Style.Language == FormatStyle::LK_Java) {
   Current.Type = TT_LambdaArrow;
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
-   Current.NestingLevel == 0) {
+   Current.NestingLevel == 0 &&
+   !Current.Previous->is(tok::kw_operator)) {
+  // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
   TrailingReturnFound = true;
 } else if (Current.is(tok::star) ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=373746&r1=373745&r2=373746&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Oct  4 06:24:15 2019
@@ -4956,6 +4956,10 @@ TEST_F(FormatTest, DontBreakBeforeQualif
 
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
+  // correct trailing return type spacing
+  verifyFormat("auto operator->() -> int;\n");
+  verifyFormat("auto operator++(int) -> int;\n");
+
   verifyFormat("struct S {\n"
"  auto bar() const -> int;\n"
"};");


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


[PATCH] D68242: [clang-format] [PR42417] clang-format inserts a space after '->' for operator->() overloading

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373746: [clang-format] [PR42417] clang-format inserts a 
space after '->' for operator->… (authored by paulhoad, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68242?vs=222487&id=223203#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68242

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1393,7 +1393,9 @@
Style.Language == FormatStyle::LK_Java) {
   Current.Type = TT_LambdaArrow;
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
-   Current.NestingLevel == 0) {
+   Current.NestingLevel == 0 &&
+   !Current.Previous->is(tok::kw_operator)) {
+  // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
   TrailingReturnFound = true;
 } else if (Current.is(tok::star) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -4956,6 +4956,10 @@
 
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
+  // correct trailing return type spacing
+  verifyFormat("auto operator->() -> int;\n");
+  verifyFormat("auto operator++(int) -> int;\n");
+
   verifyFormat("struct S {\n"
"  auto bar() const -> int;\n"
"};");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1393,7 +1393,9 @@
Style.Language == FormatStyle::LK_Java) {
   Current.Type = TT_LambdaArrow;
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
-   Current.NestingLevel == 0) {
+   Current.NestingLevel == 0 &&
+   !Current.Previous->is(tok::kw_operator)) {
+  // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
   TrailingReturnFound = true;
 } else if (Current.is(tok::star) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -4956,6 +4956,10 @@
 
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
+  // correct trailing return type spacing
+  verifyFormat("auto operator->() -> int;\n");
+  verifyFormat("auto operator++(int) -> int;\n");
+
   verifyFormat("struct S {\n"
"  auto bar() const -> int;\n"
"};");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-10-04 Thread Edward Jones via Phabricator via cfe-commits
edward-jones updated this revision to Diff 223205.
edward-jones added a comment.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

Rebased and added tests

I've made this use the Triple from the driver rather than the parsed LLVM 
triple, this means the Triple doesn't get normalized which seems like more 
desirable behavior.

I've added to the riscv{32,64}-toolchain.c test files, however the added tests 
cannot be run without a shell so I've had to disable those tests on Windows. If 
necessary I can split these new tests out into separate files.

I realize that there don't appear to be any tests for the case where no GCC 
install is found and no sysroot is provided to the driver. At the moment this 
will result in a generic linker command using the system linker, such as:

/usr/bin/ld crt0.o crtbegin.o ... -lgcc crtend.o

Is this the desired behaviour? And if so should a test be added for this too?


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

https://reviews.llvm.org/D68391

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
  clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtend.o
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,4 +1,6 @@
 // A basic clang -cc1 command-line, and simple environment check.
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
 
 // RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
@@ -34,6 +36,33 @@
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
+// If there is no GCC install detected then the driver searches for executables
+// and runtime starting from the directory tree above the driver itself.
+// The test below checks that the driver correctly finds the linker and
+// runtime iff they exist.
+// 
+// RUN: mkdir -p %T/testroot-riscv64-baremetal-nogcc/bin
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/bin/clang ] || rm %T/testroot-riscv64-baremetal-nogcc/bin/clang
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld ] || rm %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf ] || rm %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf
+// RUN: ln -s %clang %T/testroot-riscv64-baremetal-nogcc/bin/clang
+// RUN: ln -s %S/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld
+// RUN: ln -s %S/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf
+// RUN: %T/testroot-riscv64-baremetal-nogcc/bin/clang %s -### -no-canonical-prefixes \
+// RUN:-target riscv64-unknown-elf 2>&1 \
+// RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
+
+// C-RV64-BAREMETAL-LP64-NOGCC: InstalledDir: [[DRIVERDIR:.*]]
+// C-RV64-BAREMETAL-LP64-NOGCC: "-fuse-init-array"
+// C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv64-unknown-elf/include"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crt0.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtbegin.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "-L[[DRIVERDIR]]/../riscv64-unknown-elf/lib"
+// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtend.o"
+
+
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv64-unknown-elf -stdlib=libstdc++ \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,4 +1,6 @@
 // A basic clang -cc1 command-line, and simple environment check.
+

[PATCH] D68407: [RISCV] Use compiler-rt if no GCC installation detected

2019-10-04 Thread Edward Jones via Phabricator via cfe-commits
edward-jones updated this revision to Diff 223209.
edward-jones retitled this revision from "[WIP][RISCV] Use compiler-rt if no 
GCC installation detected" to "[RISCV] Use compiler-rt if no GCC installation 
detected".
edward-jones added a comment.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

Rebased, updated tests from D68391  to check 
for existence of compiler-rt crtbegin/crtend and runtime library.


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

https://reviews.llvm.org/D68407

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/clang_rt.crtbegin-riscv32.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/clang_rt.crtend-riscv32.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/clang_rt.crtbegin-riscv64.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/clang_rt.crtend-riscv64.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtend.o
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -57,10 +57,10 @@
 // C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv64-unknown-elf/include"
 // C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crt0.o"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtbegin.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/clang_rt.crtbegin-riscv64.o"
 // C-RV64-BAREMETAL-LP64-NOGCC: "-L[[DRIVERDIR]]/../riscv64-unknown-elf/lib"
-// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtend.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lclang_rt.builtins-riscv64"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/clang_rt.crtend-riscv64.o"
 
 
 // RUN: %clangxx %s -### -no-canonical-prefixes \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -57,10 +57,10 @@
 // C-RV32-BAREMETAL-ILP32-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv32-unknown-elf/include"
 // C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/riscv32-unknown-elf-ld"
 // C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crt0.o"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crtbegin.o"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/clang_rt.crtbegin-riscv32.o"
 // C-RV32-BAREMETAL-ILP32-NOGCC: "-L[[DRIVERDIR]]/../riscv32-unknown-elf/lib"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crtend.o"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lclang_rt.builtins-riscv32"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/clang_rt.crtend-riscv32.o"
 
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -45,13 +45,18 @@
 namespace RISCV {
 class LLVM_LIBRARY_VISIBILITY Linker : public GnuTool {
 public:
-  Linker(const ToolChain &TC) : GnuTool("RISCV::Linker", "ld", TC) {}
+  Linker(const ToolChain &TC, bool ShouldUseLibGCC)
+  : GnuTool("RISCV::Linker", "ld", TC),
+UseLibGCC(ShouldUseLibGCC) {}
   bool hasIntegratedCPP() const override { return false; }
   bool isLinkJob() const override { return true; }
   void ConstructJob(Compilation &C, const JobAction &JA,
 const InputInfo &Output, const InputInfoList &Inputs,
 const llvm::opt::ArgList &TCArgs,
 const char *LinkingOutput) const override;
+
+private:
+  bool UseLibGCC;
 };
 } // end namespace RISCV
 } // end namespace tools
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

@mitchell-stellar I pulled the patch to commit it, but the unit tests fail, 
could you double-check them for me?

  [   OK ] FormatTest.LayoutBraceInitializersInReturnStatement (13 ms)
  [ RUN  ] FormatTest.LayoutCxx11BraceInitializers
  
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(70):
 error:   Expected: Expected.str()
Which is: "vector< int > x{ };"
  To be equal to: format(Expected, Style)
Which is: "vector< int > x{};"
  Expected code is not stable
  
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(72):
 error:   Expected: Expected.str()
Which is: "vector< int > x{ };"
  To be equal to: format(Code, Style)
Which is: "vector< int > x{};"
  
C:/cygwin64/buildareas/clang/llvm-project/clang/unittests/Format/FormatTest.cpp(78):
 error:   Expected: Expected.str()
Which is: "vector< int > x{ };"
  To be equal to: format(test::messUp(Code), ObjCStyle)
Which is: "vector< int > x{};"
  [  FAILED  ] FormatTest.LayoutCxx11BraceInitializers (1024 ms)
  [ RUN  ] FormatTest.FormatsBracedListsInColumnLayout


Repository:
  rC Clang

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

https://reviews.llvm.org/D68415



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


[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment.

You are correct. I'll work on this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68415



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


[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar updated this revision to Diff 223212.
mitchell-stellar added a comment.

Fixed empty parentheses unit test for C++11 braced initializer list.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68415

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2188,7 +2188,8 @@
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2512,7 +2513,9 @@
 return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
 return Right.is(tok::hash);
-  if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
+  if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
+  (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
+   Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
 return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
@@ -2628,7 +2631,7 @@
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+

r373748 - [clang-rename] Fix a crash when renaming a class without definition.

2019-10-04 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Oct  4 07:09:31 2019
New Revision: 373748

URL: http://llvm.org/viewvc/llvm-project?rev=373748&view=rev
Log:
[clang-rename] Fix a crash when renaming a class without definition.

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
Modified:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp?rev=373748&r1=373747&r2=373748&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp Fri Oct  4 
07:09:31 2019
@@ -102,6 +102,10 @@ public:
 
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+if (!RecordDecl->getDefinition()) {
+  USRSet.insert(getUSRForDecl(RecordDecl));
+  return;
+}
 RecordDecl = RecordDecl->getDefinition();
 if (const auto *ClassTemplateSpecDecl =
 dyn_cast(RecordDecl))

Added: cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/clang-rename/ForwardClassDecl.cpp?rev=373748&view=auto
==
--- cfe/trunk/test/clang-rename/ForwardClassDecl.cpp (added)
+++ cfe/trunk/test/clang-rename/ForwardClassDecl.cpp Fri Oct  4 07:09:31 2019
@@ -0,0 +1,4 @@
+class Foo; // CHECK: class Bar;
+Foo *f();  // CHECK: Bar *f();
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s --  | sed 's,//.*,,' | 
FileCheck %s


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


r373749 - Further improve -Wbool-operation bitwise negation message

2019-10-04 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Fri Oct  4 07:11:05 2019
New Revision: 373749

URL: http://llvm.org/viewvc/llvm-project?rev=373749&view=rev
Log:
Further improve -Wbool-operation bitwise negation message

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/Sema/warn-bitwise-negation-bool.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=373749&r1=373748&r2=373749&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct  4 07:11:05 
2019
@@ -6638,7 +6638,7 @@ def note_member_declared_here : Note<
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
 def warn_bitwise_negation_bool : Warning<
-  "bitwise negation of a boolean expression; did you mean a logicial 
negation?">,
+  "bitwise negation of a boolean expression; did you mean logical negation?">,
   InGroup>;
 def err_decrement_bool : Error<"cannot decrement expression of type bool">;
 def warn_increment_bool : Warning<

Modified: cfe/trunk/test/Sema/warn-bitwise-negation-bool.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-bitwise-negation-bool.c?rev=373749&r1=373748&r2=373749&view=diff
==
--- cfe/trunk/test/Sema/warn-bitwise-negation-bool.c (original)
+++ cfe/trunk/test/Sema/warn-bitwise-negation-bool.c Fri Oct  4 07:11:05 2019
@@ -12,11 +12,11 @@ typedef _Bool boolean;
 #endif
 
 void test(boolean b, int i) {
-  b = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean a logicial negation?}}
+  b = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean logical negation?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
-  b = ~(b); // expected-warning {{bitwise negation of a boolean expression; 
did you mean a logicial negation?}}
+  b = ~(b); // expected-warning {{bitwise negation of a boolean expression; 
did you mean logical negation?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
   b = ~i;
-  i = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean a logicial negation?}}
+  i = ~b; // expected-warning {{bitwise negation of a boolean expression; did 
you mean logical negation?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
 }


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


[PATCH] D68459: [clang-rename] Fix a crash when renaming a class without definition.

2019-10-04 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373748: [clang-rename] Fix a crash when renaming a class 
without definition. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68459?vs=223194&id=223216#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68459

Files:
  cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  cfe/trunk/test/clang-rename/ForwardClassDecl.cpp


Index: cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
===
--- cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
+++ cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
@@ -0,0 +1,4 @@
+class Foo; // CHECK: class Bar;
+Foo *f();  // CHECK: Bar *f();
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s --  | sed 's,//.*,,' | 
FileCheck %s
Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -102,6 +102,10 @@
 
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+if (!RecordDecl->getDefinition()) {
+  USRSet.insert(getUSRForDecl(RecordDecl));
+  return;
+}
 RecordDecl = RecordDecl->getDefinition();
 if (const auto *ClassTemplateSpecDecl =
 dyn_cast(RecordDecl))


Index: cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
===
--- cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
+++ cfe/trunk/test/clang-rename/ForwardClassDecl.cpp
@@ -0,0 +1,4 @@
+class Foo; // CHECK: class Bar;
+Foo *f();  // CHECK: Bar *f();
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s --  | sed 's,//.*,,' | FileCheck %s
Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -102,6 +102,10 @@
 
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+if (!RecordDecl->getDefinition()) {
+  USRSet.insert(getUSRForDecl(RecordDecl));
+  return;
+}
 RecordDecl = RecordDecl->getDefinition();
 if (const auto *ClassTemplateSpecDecl =
 dyn_cast(RecordDecl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 223215.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.

- add additional compl(5) and not(5) unit tests
- improve the comments around the condition and explaining the tests
- remove the use of .equals(...)


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

https://reviews.llvm.org/D68332

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14609,6 +14609,48 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for ensuring alternate operators are not
+  // combined with their right most neighbour.
+  verifyFormat("int a and b;");
+  verifyFormat("int a and_eq b;");
+  verifyFormat("int a bitand b;");
+  verifyFormat("int a bitor b;");
+  verifyFormat("int a compl b;");
+  verifyFormat("int a not b;");
+  verifyFormat("int a not_eq b;");
+  verifyFormat("int a or b;");
+  verifyFormat("int a xor b;");
+  verifyFormat("int a xor_eq b;");
+  verifyFormat("return this not_eq bitand other;");
+  verifyFormat("bool operator not_eq(const X bitand other)");
+
+  verifyFormat("int a and 5;");
+  verifyFormat("int a and_eq 5;");
+  verifyFormat("int a bitand 5;");
+  verifyFormat("int a bitor 5;");
+  verifyFormat("int a compl 5;");
+  verifyFormat("int a not 5;");
+  verifyFormat("int a not_eq 5;");
+  verifyFormat("int a or 5;");
+  verifyFormat("int a xor 5;");
+  verifyFormat("int a xor_eq 5;");
+
+  verifyFormat("int a compl(5);");
+  verifyFormat("int a not(5);");
+
+  /* FIXME handle alternate tokens
+   * https://en.cppreference.com/w/cpp/language/operator_alternative
+  // alternative tokens
+  verifyFormat("compl foo();"); //  ~foo();
+  verifyFormat("foo() <%%>;");  // foo();
+  verifyFormat("void foo() <%%>;"); // void foo(){}
+  verifyFormat("int a <:1:>;"); // int a[1];[
+  verifyFormat("%:define ABC abc"); // #define ABC abc
+  verifyFormat("%:%:"); // ##
+  */
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,9 +2900,19 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
+  if (Left.is(TT_UnaryOperator)) {
+// The alternative operators for ~ and ! are "compl" and "not".
+// If they are used instead, we do not want to combine them with
+// the token to the right, unless that is a left paren.
+if (!Right.is(tok::l_paren)) {
+  if (Left.is(tok::exclaim) && Left.TokenText == "not")
+return true;
+  if (Left.is(tok::tilde) && Left.TokenText == "compl")
+return true;
+}
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14609,6 +14609,48 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for ensuring alternate operators are not
+  // combined with their right most neighbour.
+  verifyFormat("int a and b;");
+  verifyFormat("int a and_eq b;");
+  verifyFormat("int a bitand b;");
+  verifyFormat("int a bitor b;");
+  verifyFormat("int a compl b;");
+  verifyFormat("int a not b;");
+  verifyFormat("int a not_eq b;");
+  verifyFormat("int a or b;");
+  verifyFormat("int a xor b;");
+  verifyFormat("int a xor_eq b;");
+  verifyFormat("return this not_eq bitand other;");
+  verifyFormat("bool operator not_eq(const X bitand other)");
+
+  verifyFormat("int a and 5;");
+  verifyFormat("int a and_eq 5;");
+  verifyFormat("int a bitand 5;");
+  verifyFormat("int a bitor 5;");
+  verifyFormat("int a compl 5;");
+  verifyFormat("int a not 5;");
+  verifyFormat("int a not_eq 5;");
+  verifyFormat("int a or 5;");
+  verifyFormat("int a xor 5;");
+  verifyFormat("int a xor_eq 5;");
+
+  verifyFormat("int a compl(5);");
+  verifyFormat("int a not(5);");
+
+  /* FIXME handle alternate tokens
+   * https://en.cppreference.com/w/cpp/language/operator_alternative
+  // alternative tokens
+  verifyFormat("compl foo();"); //  ~foo();
+  verifyFormat("foo() <%%>;");  // foo();
+  verifyFormat("void foo() <%%>;"); // void foo(){}
+  verifyFormat("int a <:1:>;"); // int a[1];[
+  verifyFormat("

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

2019-10-04 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk added a comment.

@dexonsmith when you get a chance, can you take another look at this? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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


[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D68332



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


r373750 - [clang-format] [PR43531] clang-format damages "alternative representations" for operators

2019-10-04 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Fri Oct  4 07:16:59 2019
New Revision: 373750

URL: http://llvm.org/viewvc/llvm-project?rev=373750&view=rev
Log:
[clang-format] [PR43531] clang-format damages "alternative representations" for 
operators

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

Fix for clang-format incorrectly handles "alternative operators" as described 
by https://en.cppreference.com/w/cpp/language/operator_alternative

compl = ~
not = !

these are unary operators, and clang-format will remove the space between them 
and a numeric constant

this incorrectly formats the following code

```
int a compl 5;
int a not 5;
```

into:

```
int a compl5;
int a not5;
```

The code adds FIXME unit tests for "alternative token" representations for {} 
[] and # as defined by the same link, which would require a more detailed 
change to the FormatTokenLexer

Reviewers: klimek, reuk, owenpan, mitchell-stellar, STL_MSFT

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

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

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373750&r1=373749&r2=373750&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  4 07:16:59 2019
@@ -2900,9 +2900,19 @@ bool TokenAnnotator::spaceRequiredBefore
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
+  if (Left.is(TT_UnaryOperator)) {
+// The alternative operators for ~ and ! are "compl" and "not".
+// If they are used instead, we do not want to combine them with
+// the token to the right, unless that is a left paren.
+if (!Right.is(tok::l_paren)) {
+  if (Left.is(tok::exclaim) && Left.TokenText == "not")
+return true;
+  if (Left.is(tok::tilde) && Left.TokenText == "compl")
+return true;
+}
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=373750&r1=373749&r2=373750&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Oct  4 07:16:59 2019
@@ -14609,6 +14609,48 @@ TEST_F(FormatTest, AmbersandInLamda) {
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for ensuring alternate operators are not
+  // combined with their right most neighbour.
+  verifyFormat("int a and b;");
+  verifyFormat("int a and_eq b;");
+  verifyFormat("int a bitand b;");
+  verifyFormat("int a bitor b;");
+  verifyFormat("int a compl b;");
+  verifyFormat("int a not b;");
+  verifyFormat("int a not_eq b;");
+  verifyFormat("int a or b;");
+  verifyFormat("int a xor b;");
+  verifyFormat("int a xor_eq b;");
+  verifyFormat("return this not_eq bitand other;");
+  verifyFormat("bool operator not_eq(const X bitand other)");
+
+  verifyFormat("int a and 5;");
+  verifyFormat("int a and_eq 5;");
+  verifyFormat("int a bitand 5;");
+  verifyFormat("int a bitor 5;");
+  verifyFormat("int a compl 5;");
+  verifyFormat("int a not 5;");
+  verifyFormat("int a not_eq 5;");
+  verifyFormat("int a or 5;");
+  verifyFormat("int a xor 5;");
+  verifyFormat("int a xor_eq 5;");
+
+  verifyFormat("int a compl(5);");
+  verifyFormat("int a not(5);");
+
+  /* FIXME handle alternate tokens
+   * https://en.cppreference.com/w/cpp/language/operator_alternative
+  // alternative tokens
+  verifyFormat("compl foo();"); //  ~foo();
+  verifyFormat("foo() <%%>;");  // foo();
+  verifyFormat("void foo() <%%>;"); // void foo(){}
+  verifyFormat("int a <:1:>;"); // int a[1];[
+  verifyFormat("%:define ABC abc"); // #define ABC abc
+  verifyFormat("%:%:"); // ##
+  */
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


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


[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba12cec21f55: [clang-format] [PR43531] clang-format damages 
"alternative representations" for… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68332

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14609,6 +14609,48 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for ensuring alternate operators are not
+  // combined with their right most neighbour.
+  verifyFormat("int a and b;");
+  verifyFormat("int a and_eq b;");
+  verifyFormat("int a bitand b;");
+  verifyFormat("int a bitor b;");
+  verifyFormat("int a compl b;");
+  verifyFormat("int a not b;");
+  verifyFormat("int a not_eq b;");
+  verifyFormat("int a or b;");
+  verifyFormat("int a xor b;");
+  verifyFormat("int a xor_eq b;");
+  verifyFormat("return this not_eq bitand other;");
+  verifyFormat("bool operator not_eq(const X bitand other)");
+
+  verifyFormat("int a and 5;");
+  verifyFormat("int a and_eq 5;");
+  verifyFormat("int a bitand 5;");
+  verifyFormat("int a bitor 5;");
+  verifyFormat("int a compl 5;");
+  verifyFormat("int a not 5;");
+  verifyFormat("int a not_eq 5;");
+  verifyFormat("int a or 5;");
+  verifyFormat("int a xor 5;");
+  verifyFormat("int a xor_eq 5;");
+
+  verifyFormat("int a compl(5);");
+  verifyFormat("int a not(5);");
+
+  /* FIXME handle alternate tokens
+   * https://en.cppreference.com/w/cpp/language/operator_alternative
+  // alternative tokens
+  verifyFormat("compl foo();"); //  ~foo();
+  verifyFormat("foo() <%%>;");  // foo();
+  verifyFormat("void foo() <%%>;"); // void foo(){}
+  verifyFormat("int a <:1:>;"); // int a[1];[
+  verifyFormat("%:define ABC abc"); // #define ABC abc
+  verifyFormat("%:%:"); // ##
+  */
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,9 +2900,19 @@
   return false;
 return true;
   }
-  if (Left.is(TT_UnaryOperator))
+  if (Left.is(TT_UnaryOperator)) {
+// The alternative operators for ~ and ! are "compl" and "not".
+// If they are used instead, we do not want to combine them with
+// the token to the right, unless that is a left paren.
+if (!Right.is(tok::l_paren)) {
+  if (Left.is(tok::exclaim) && Left.TokenText == "not")
+return true;
+  if (Left.is(tok::tilde) && Left.TokenText == "compl")
+return true;
+}
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
+  }
 
   // If the next token is a binary operator or a selector name, we have
   // incorrectly classified the parenthesis as a cast. FIXME: Detect correctly.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14609,6 +14609,48 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for ensuring alternate operators are not
+  // combined with their right most neighbour.
+  verifyFormat("int a and b;");
+  verifyFormat("int a and_eq b;");
+  verifyFormat("int a bitand b;");
+  verifyFormat("int a bitor b;");
+  verifyFormat("int a compl b;");
+  verifyFormat("int a not b;");
+  verifyFormat("int a not_eq b;");
+  verifyFormat("int a or b;");
+  verifyFormat("int a xor b;");
+  verifyFormat("int a xor_eq b;");
+  verifyFormat("return this not_eq bitand other;");
+  verifyFormat("bool operator not_eq(const X bitand other)");
+
+  verifyFormat("int a and 5;");
+  verifyFormat("int a and_eq 5;");
+  verifyFormat("int a bitand 5;");
+  verifyFormat("int a bitor 5;");
+  verifyFormat("int a compl 5;");
+  verifyFormat("int a not 5;");
+  verifyFormat("int a not_eq 5;");
+  verifyFormat("int a or 5;");
+  verifyFormat("int a xor 5;");
+  verifyFormat("int a xor_eq 5;");
+
+  verifyFormat("int a compl(5);");
+  verifyFormat("int a not(5);");
+
+  /* FIXME handle alternate tokens
+   * https://en.cppreference.com/w/cpp/language/operator_alternative
+  // alternative tokens
+  verifyFormat("compl foo();"); //  ~foo();
+  verifyFormat("foo() <%%>;");  // foo();
+  verifyFormat("void foo() <%%>;"); // void foo(){}
+  verifyFormat("int a <:1:>;"); // int a[1];[
+  verifyFormat("%:define ABC abc"); // #defi

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

LGTM and passes

  [--] Global test environment tear-down
  [==] 700 tests from 21 test cases ran. (67039 ms total)
  [  PASSED  ] 700 tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68415



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


r373751 - [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Fri Oct  4 07:25:20 2019
New Revision: 373751

URL: http://llvm.org/viewvc/llvm-project?rev=373751&view=rev
Log:
[clang-format] C++11 braced lists should respect the SpacesInParentheses setting

Summary:
According to the clang-format documentation, "Fundamentally, C++11 braced lists 
are formatted exactly like function calls would be formatted in their place. If 
the braced list follows a name (e.g. a type or variable name), clang-format 
formats as if the `{}` were the parentheses of a function call with that name."

This patch furthers the treatment of C++11 braced list braces as parentheses by 
respecting the `SpacesInParentheses` setting.

Reviewers: MyDeveloperDay, reuk, owenpan

Reviewed By: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang-format, #clang

Patch By: mitchell-stellar

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373751&r1=373750&r2=373751&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  4 07:25:20 2019
@@ -2196,7 +2196,8 @@ void TokenAnnotator::calculateFormatting
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2520,7 +2521,9 @@ bool TokenAnnotator::spaceRequiredBetwee
 return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
 return Right.is(tok::hash);
-  if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
+  if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
+  (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
+   Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
 return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
@@ -2636,7 +2639,7 @@ bool TokenAnnotator::spaceRequiredBetwee
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=373751&r1=373750&r2=373751&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Oct  4 07:25:20 2019
@@ -8214,6 +8214,34 @@ TEST_F(FormatTest, LayoutCxx11BraceIniti
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {


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

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373751: [clang-format] C++11 braced lists should respect the 
SpacesInParentheses setting (authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68415?vs=223212&id=223219#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68415

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -8214,6 +8214,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2196,7 +2196,8 @@
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2520,7 +2521,9 @@
 return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
 return Right.is(tok::hash);
-  if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
+  if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
+  (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
+   Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
 return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
@@ -2636,7 +2639,7 @@
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||


Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -8214,6 +8214,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3

r373752 - CGBlocks - silence static analyzer getAs<> null dereference warnings. NFCI.

2019-10-04 Thread Simon Pilgrim via cfe-commits
Author: rksimon
Date: Fri Oct  4 08:01:54 2019
New Revision: 373752

URL: http://llvm.org/viewvc/llvm-project?rev=373752&view=rev
Log:
CGBlocks - silence static analyzer getAs<> null dereference warnings. NFCI.

The static analyzer is warning about potential null dereferences, but in these 
cases we should be able to use castAs<> directly and if not assert will fire 
for us.

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=373752&r1=373751&r2=373752&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Fri Oct  4 08:01:54 2019
@@ -1253,8 +1253,7 @@ llvm::Type *CodeGenModule::getGenericBlo
 
 RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
   ReturnValueSlot ReturnValue) {
-  const BlockPointerType *BPT =
-E->getCallee()->getType()->getAs();
+  const auto *BPT = E->getCallee()->getType()->castAs();
   llvm::Value *BlockPtr = EmitScalarExpr(E->getCallee());
   llvm::Type *GenBlockTy = CGM.getGenericBlockLiteralType();
   llvm::Value *Func = nullptr;
@@ -1802,7 +1801,7 @@ struct CallBlockRelease final : EHScopeS
 bool CodeGenFunction::cxxDestructorCanThrow(QualType T) {
   if (const auto *RD = T->getAsCXXRecordDecl())
 if (const CXXDestructorDecl *DD = RD->getDestructor())
-  return DD->getType()->getAs()->canThrow();
+  return DD->getType()->castAs()->canThrow();
   return false;
 }
 


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


r373753 - SemaDeclAttr - silence static analyzer getAs<> null dereference warnings. NFCI.

2019-10-04 Thread Simon Pilgrim via cfe-commits
Author: rksimon
Date: Fri Oct  4 08:02:46 2019
New Revision: 373753

URL: http://llvm.org/viewvc/llvm-project?rev=373753&view=rev
Log:
SemaDeclAttr - silence static analyzer getAs<> null dereference warnings. NFCI.

The static analyzer is warning about potential null dereferences, but in these 
cases we should be able to use castAs<> directly and if not assert will fire 
for us.

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=373753&r1=373752&r2=373753&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Oct  4 08:02:46 2019
@@ -2701,7 +2701,7 @@ static void handleSentinelAttr(Sema &S,
 if (Ty->isBlockPointerType() || Ty->isFunctionPointerType()) {
   const FunctionType *FT = Ty->isFunctionPointerType()
? D->getFunctionType()
-   : 
Ty->getAs()->getPointeeType()->getAs();
+   : 
Ty->castAs()->getPointeeType()->getAs();
   if (!cast(FT)->isVariadic()) {
 int m = Ty->isFunctionPointerType() ? 0 : 1;
 S.Diag(AL.getLoc(), diag::warn_attribute_sentinel_not_variadic) << m;
@@ -3111,7 +3111,7 @@ static void handleFormatArgAttr(Sema &S,
   if (NotNSStringTy &&
   !isCFStringType(Ty, S.Context) &&
   (!Ty->isPointerType() ||
-   !Ty->getAs()->getPointeeType()->isCharType())) {
+   !Ty->castAs()->getPointeeType()->isCharType())) {
 S.Diag(AL.getLoc(), diag::err_format_attribute_not)
 << "a string type" << IdxExpr->getSourceRange()
 << getFunctionOrMethodParamRange(D, 0);
@@ -3121,7 +3121,7 @@ static void handleFormatArgAttr(Sema &S,
   if (!isNSStringType(Ty, S.Context) &&
   !isCFStringType(Ty, S.Context) &&
   (!Ty->isPointerType() ||
-   !Ty->getAs()->getPointeeType()->isCharType())) {
+   !Ty->castAs()->getPointeeType()->isCharType())) {
 S.Diag(AL.getLoc(), diag::err_format_attribute_result_not)
 << (NotNSStringTy ? "string type" : "NSString")
 << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0);
@@ -3297,7 +3297,7 @@ static void handleFormatAttr(Sema &S, De
   return;
 }
   } else if (!Ty->isPointerType() ||
- !Ty->getAs()->getPointeeType()->isCharType()) {
+ !Ty->castAs()->getPointeeType()->isCharType()) {
 S.Diag(AL.getLoc(), diag::err_format_attribute_not)
   << "a string type" << IdxExpr->getSourceRange()
   << getFunctionOrMethodParamRange(D, ArgIdx);


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


[PATCH] D68467: [clangd] If an undocumented definition exists, don't accept documentation from other forward decls.

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

This fixes cases like:

  foo.h
class Undocumented{}
  bar.h
// break an include cycle. we should refactor this!
class Undocumented;

Where the comment doesn't describe the class.

Note that a forward decl that is *visible to the definition* will still have
its doc comment used, by SymbolCollector: Merge isn't involved here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68467

Files:
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp


Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -413,6 +413,16 @@
FileURI("unittest:///test2.cc"));
 }
 
+TEST(MergeIndexTest, NonDocumentation) {
+  Symbol L, R;
+  L.ID = R.ID = SymbolID("x");
+  L.Definition.FileURI = "file:/x.h";
+  R.Documentation = "Forward declarations because x.h is too big to include";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(M.Documentation, "");
+}
+
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == 
References);
 }
Index: clang-tools-extra/clangd/index/Merge.cpp
===
--- clang-tools-extra/clangd/index/Merge.cpp
+++ clang-tools-extra/clangd/index/Merge.cpp
@@ -186,7 +186,10 @@
 S.Signature = O.Signature;
   if (S.CompletionSnippetSuffix == "")
 S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
-  if (S.Documentation == "")
+  // Don't accept documentation from bare forward declarations, if there is a
+  // definition and it didn't provide one. S is often an undocumented class,
+  // and O is a non-canonical forward decl preceded by an irrelevant comment.
+  if (S.Documentation == "" && !S.Definition)
 S.Documentation = O.Documentation;
   if (S.ReturnType == "")
 S.ReturnType = O.ReturnType;


Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -413,6 +413,16 @@
FileURI("unittest:///test2.cc"));
 }
 
+TEST(MergeIndexTest, NonDocumentation) {
+  Symbol L, R;
+  L.ID = R.ID = SymbolID("x");
+  L.Definition.FileURI = "file:/x.h";
+  R.Documentation = "Forward declarations because x.h is too big to include";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(M.Documentation, "");
+}
+
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
Index: clang-tools-extra/clangd/index/Merge.cpp
===
--- clang-tools-extra/clangd/index/Merge.cpp
+++ clang-tools-extra/clangd/index/Merge.cpp
@@ -186,7 +186,10 @@
 S.Signature = O.Signature;
   if (S.CompletionSnippetSuffix == "")
 S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
-  if (S.Documentation == "")
+  // Don't accept documentation from bare forward declarations, if there is a
+  // definition and it didn't provide one. S is often an undocumented class,
+  // and O is a non-canonical forward decl preceded by an irrelevant comment.
+  if (S.Documentation == "" && !S.Definition)
 S.Documentation = O.Documentation;
   if (S.ReturnType == "")
 S.ReturnType = O.ReturnType;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

2019-10-04 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham marked 20 inline comments as done.
simon_tatham added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8529
   "argument should be a multiple of %0">;
+def err_argument_not_power_of_2 : Error<
+  "argument should be a power of 2">;

dmgreen wrote:
> Do we have any/should we have some tests for these errors?
By the time we finish implementing all the intrinsics, there will be tests for 
these errors. The intrinsics that need them aren't in this preliminary commit, 
though.



Comment at: clang/include/clang/Basic/arm_mve_defs.td:266
+// vidupq_wb_u16 -> vidupq_u16.
+def PNT_WB: PolymorphicNameType<0, "wb">;
+

dmgreen wrote:
> These are not used yet, right? They are not meant for the vldr gathers, those 
> don't have polymorphic names (if I'm reading this list of intrinsics right). 
> These are for vidup's as the comment says?
I've defined here the complete list of polymorphic name types that will be 
needed for //all// the MVE intrinsics. I haven't included an example of every 
single one in this preliminary set, though.

Yes, I think `PNT_WB` in particular is only used for the `vidup` family.



Comment at: clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O3 -S -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O3 -S -emit-llvm -o - %s | FileCheck %s

dmgreen wrote:
> These tests all run -O3, the entire pass pipeline. I see quite a few tests in 
> the same folder do the same thing, but in this case we will be adding quite a 
> few tests. Random mid-end optimizations may well keep on altering them.
> 
> Is it possible to use -disable-O0-optnone and pipe them into opt -mem2reg? 
> What would that do to the codegen, would it be a lot more verbose than it is 
> now?
> 
> Also could/should they be using clang_cc1?
The immediate problem is that if you use any form of `clang | opt | FileCheck` 
command line, then `update_cc_test_checks.py` says 'skipping non-FileChecked 
line', because it doesn't support anything more complicated than a two-stage 
pipeline consisting of clang and FileCheck.

I've enhanced `update_cc_test_checks` to handle that case, in D68406, and 
respun these tests accordingly. But if that patch doesn't get approval then 
I'll have to rethink this again.

(For `vld24.c` I ended up running `opt -sroa -early-cse` to avoid the IR 
becoming noticeably more verbose. The rest worked fine with just `mem2reg`, 
though.)


Patching `update_cc_test_checks.py` to support more complex pipelines, it seems 
to work OK: most codegen changes are trivial ones, such as modifiers not 
appearing on IR operations (`call` instead of `tail call`, plain `shl` in place 
of `shl nuw`). Only `vld24` becomes significantly more complicated: for that 
one file I had to run `opt -mem2reg -sroa -early-cse` instead.



Comment at: clang/utils/TableGen/MveEmitter.cpp:82
+#if 0
+} // stop emacs from wanting to auto-indent everything to 2 spaces inside here
+#endif

dmgreen wrote:
> Is this needed still? It seems like something that should be handled in 
> clang-format/emacs directly.
Oh yes, now I look on Stack Overflow there is a way to make emacs stop 
indenting things inside namespaces. Thanks for the prod :-)



Comment at: clang/utils/TableGen/MveEmitter.cpp:1403
+" *\n"
+" * Permission is hereby granted, free of charge, to any person "
+"obtaining a copy\n"

dmgreen wrote:
> Clang format really made a mess of this, didn't it.
> 
> Is this is old license? Should it be updated to the new one. I imagine that 
> these generated headers might well have been missed in the switchover.
Seems likely! I've updated it to the same license that's at the top of this 
source file itself.



Comment at: clang/utils/TableGen/MveEmitter.cpp:132
+// the pointee type.
+Pointer,
+  };

dmgreen wrote:
> The gathers are really a Vector of Pointers, in a way. But in the intrinsics 
> (and IR), they are just treated as a vector of ints, so I presume a new type 
> is not needed? We may (but not yet) want to make use of the llvm masked 
> gathers. We would have to add codegen support for them first though (which I 
> don't think we have plans for in the near term).
Yes, I'm working so far on the assumption that we don't need to represent 
scatter/gather address vectors as a special vector-of-pointers type.

(If nothing else, it would be a bit strange for the 64-bit versions, where the 
vector element isn't even the same //size// as a pointer.)

If auto-generation of gather loads during vectorization turns out to need a 
sp

[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar requested changes to this revision.
mitchell-stellar added a comment.
This revision now requires changes to proceed.

This needs to be more thought out. At the very least it needs to cover more 
keywords like `do`, `switch`, `try`, and `catch`. There may be more. 
Consideration also needs to be made for `class`, functions, `namespace`, etc. 
Otherwise this is an experimental feature at best. There is also confusion with 
how it may interact with `SpaceBeforeCpp11BracedList`. The name 
`SpacesAroundBraces` is just too generic in that regard considering the fact 
that braces play multiple roles in C++.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67773



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


r373756 - [OPENMP50]Suppport for multiple vendors in the same vendor context

2019-10-04 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Fri Oct  4 08:58:45 2019
New Revision: 373756

URL: http://llvm.org/viewvc/llvm-project?rev=373756&view=rev
Log:
[OPENMP50]Suppport for multiple vendors in the same vendor context
selector.

According to OpenMP 5.0, multiple vendors could be specified in the
vendor context selector via ',' as a separator.

Modified:
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/test/OpenMP/declare_variant_ast_print.c
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
cfe/trunk/test/OpenMP/declare_variant_messages.c
cfe/trunk/test/OpenMP/declare_variant_messages.cpp

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=373756&r1=373755&r2=373756&view=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Fri Oct  4 08:58:45 2019
@@ -812,10 +812,13 @@ static ExprResult parseContextScore(Pars
 }
 
 /// Parse context selector for 'implementation' selector set:
-/// 'vendor' '('  ')'
-static void
-parseImplementationSelector(Parser &P,
-Sema::OpenMPDeclareVariantCtsSelectorData &Data) {
+/// 'vendor' '(' [ 'score' '('  ')' ':' ]  { ','  
}
+/// ')'
+static void parseImplementationSelector(
+Parser &P, SourceLocation Loc,
+llvm::function_ref
+Callback) {
   const Token &Tok = P.getCurToken();
   // Parse inner context selector set name, if any.
   if (!Tok.is(tok::identifier)) {
@@ -840,20 +843,33 @@ parseImplementationSelector(Parser &P,
 BalancedDelimiterTracker T(P, tok::l_paren, tok::annot_pragma_openmp_end);
 (void)T.expectAndConsume(diag::err_expected_lparen_after,
  CtxSelectorName.data());
-Data.CtxScore = parseContextScore(P);
-// Parse .
-StringRef VendorName;
-if (Tok.is(tok::identifier)) {
-  VendorName = P.getPreprocessor().getSpelling(P.getCurToken(), Buffer);
-  (void)P.ConsumeToken();
-} else {
-  P.Diag(Tok.getLocation(), diag::err_omp_declare_variant_item_expected)
-  << "vendor identifier" << "vendor" << "implementation";
-}
+const ExprResult Score = parseContextScore(P);
+do {
+  // Parse .
+  StringRef VendorName;
+  if (Tok.is(tok::identifier)) {
+Buffer.clear();
+VendorName = P.getPreprocessor().getSpelling(P.getCurToken(), Buffer);
+(void)P.ConsumeToken();
+  } else {
+P.Diag(Tok.getLocation(), diag::err_omp_declare_variant_item_expected)
+<< "vendor identifier"
+<< "vendor"
+<< "implementation";
+  }
+  if (!VendorName.empty()) {
+Sema::OpenMPDeclareVariantCtsSelectorData Data(
+OMPDeclareVariantAttr::CtxSetImplementation, CSKind, VendorName,
+Score);
+Callback(SourceRange(Loc, Tok.getLocation()), Data);
+  }
+  if (!P.TryConsumeToken(tok::comma) && Tok.isNot(tok::r_paren)) {
+P.Diag(Tok, diag::err_expected_punc)
+<< (VendorName.empty() ? "vendor name" : VendorName);
+  }
+} while (Tok.is(tok::identifier));
 // Parse ')'.
 (void)T.consumeClose();
-if (!VendorName.empty())
-  Data.ImplVendor = VendorName;
 break;
   }
   case OMPDeclareVariantAttr::CtxUnknown:
@@ -865,8 +881,6 @@ parseImplementationSelector(Parser &P,
   ;
 return;
   }
-  Data.CtxSet = OMPDeclareVariantAttr::CtxSetImplementation;
-  Data.Ctx = CSKind;
 }
 
 /// Parses clauses for 'declare variant' directive.
@@ -897,7 +911,6 @@ bool Parser::parseOpenMPContextSelectors
 (void)ConsumeToken();
 // TBD: add parsing of known context selectors.
 // Unknown selector - just ignore it completely.
-Sema::OpenMPDeclareVariantCtsSelectorData Data;
 {
   // Parse '{'.
   BalancedDelimiterTracker TBr(*this, tok::l_brace,
@@ -910,7 +923,7 @@ bool Parser::parseOpenMPContextSelectors
   CtxSelectorSetName, CSSKind);
   switch (CSSKind) {
   case OMPDeclareVariantAttr::CtxSetImplementation:
-parseImplementationSelector(*this, Data);
+parseImplementationSelector(*this, Loc, Callback);
 break;
   case OMPDeclareVariantAttr::CtxSetUnknown:
 // Skip until either '}', ')', or end of directive.
@@ -922,7 +935,6 @@ bool Parser::parseOpenMPContextSelectors
   // Parse '}'.
   (void)TBr.consumeClose();
 }
-Callback(SourceRange(Loc, Tok.getLocation()), Data);
 // Consume ','
 if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::annot_pragma_openmp_end))
   (void)ExpectAndConsume(tok::comma);

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_ast_print.c?rev=373756&r1=373755&r2=373756&view=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_ast_print.

[PATCH] D67980: [WIP][CLANG][BPF] do compile-once run-everywhere relocation for bitfields

2019-10-04 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 223233.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

using 64bit buf always, handling non bitfield members, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980

Files:
  clang/include/clang/Basic/BuiltinsBPF.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/module.modulemap
  clang/lib/Basic/Targets/BPF.cpp
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/lib/Target/BPF/BPF.h
  llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
  llvm/lib/Target/BPF/BPFCORE.h
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BTF.h
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h

Index: llvm/lib/Target/BPF/BTFDebug.h
===
--- llvm/lib/Target/BPF/BTFDebug.h
+++ llvm/lib/Target/BPF/BTFDebug.h
@@ -224,10 +224,11 @@
 };
 
 /// Represent one offset relocation.
-struct BTFOffsetReloc {
+struct BTFFieldReloc {
   const MCSymbol *Label;  ///< MCSymbol identifying insn for the reloc
   uint32_t TypeID;///< Type ID
   uint32_t OffsetNameOff; ///< The string to traverse types
+  uint32_t RelocKind; ///< What to patch the instruction
 };
 
 /// Represent one extern relocation.
@@ -249,12 +250,12 @@
   std::unordered_map DIToIdMap;
   std::map> FuncInfoTable;
   std::map> LineInfoTable;
-  std::map> OffsetRelocTable;
+  std::map> FieldRelocTable;
   std::map> ExternRelocTable;
   StringMap> FileContent;
   std::map> DataSecEntries;
   std::vector StructTypes;
-  std::map AccessOffsets;
+  std::map PatchImms;
   std::map>>
   FixupDerivedTypes;
 
@@ -300,7 +301,7 @@
   void processGlobals(bool ProcessingMapDef);
 
   /// Generate one offset relocation record.
-  void generateOffsetReloc(const MachineInstr *MI, const MCSymbol *ORSym,
+  void generateFieldReloc(const MachineInstr *MI, const MCSymbol *ORSym,
DIType *RootTy, StringRef AccessPattern);
 
   /// Populating unprocessed struct type.
Index: llvm/lib/Target/BPF/BTFDebug.cpp
===
--- llvm/lib/Target/BPF/BTFDebug.cpp
+++ llvm/lib/Target/BPF/BTFDebug.cpp
@@ -754,7 +754,7 @@
 void BTFDebug::emitBTFExtSection() {
   // Do not emit section if empty FuncInfoTable and LineInfoTable.
   if (!FuncInfoTable.size() && !LineInfoTable.size() &&
-  !OffsetRelocTable.size() && !ExternRelocTable.size())
+  !FieldRelocTable.size() && !ExternRelocTable.size())
 return;
 
   MCContext &Ctx = OS.getContext();
@@ -766,8 +766,8 @@
 
   // Account for FuncInfo/LineInfo record size as well.
   uint32_t FuncLen = 4, LineLen = 4;
-  // Do not account for optional OffsetReloc/ExternReloc.
-  uint32_t OffsetRelocLen = 0, ExternRelocLen = 0;
+  // Do not account for optional FieldReloc/ExternReloc.
+  uint32_t FieldRelocLen = 0, ExternRelocLen = 0;
   for (const auto &FuncSec : FuncInfoTable) {
 FuncLen += BTF::SecFuncInfoSize;
 FuncLen += FuncSec.second.size() * BTF::BPFFuncInfoSize;
@@ -776,17 +776,17 @@
 LineLen += BTF::SecLineInfoSize;
 LineLen += LineSec.second.size() * BTF::BPFLineInfoSize;
   }
-  for (const auto &OffsetRelocSec : OffsetRelocTable) {
-OffsetRelocLen += BTF::SecOffsetRelocSize;
-OffsetRelocLen += OffsetRelocSec.second.size() * BTF::BPFOffsetRelocSize;
+  for (const auto &FieldRelocSec : FieldRelocTable) {
+FieldRelocLen += BTF::SecFieldRelocSize;
+FieldRelocLen += FieldRelocSec.second.size() * BTF::BPFFieldRelocSize;
   }
   for (const auto &ExternRelocSec : ExternRelocTable) {
 ExternRelocLen += BTF::SecExternRelocSize;
 ExternRelocLen += ExternRelocSec.second.size() * BTF::BPFExternRelocSize;
   }
 
-  if (OffsetRelocLen)
-OffsetRelocLen += 4;
+  if (FieldRelocLen)
+FieldRelocLen += 4;
   if (ExternRelocLen)
 ExternRelocLen += 4;
 
@@ -795,8 +795,8 @@
   OS.EmitIntValue(FuncLen, 4);
   OS.EmitIntValue(LineLen, 4);
   OS.EmitIntValue(FuncLen + LineLen, 4);
-  OS.EmitIntValue(OffsetRelocLen, 4);
-  OS.EmitIntValue(FuncLen + LineLen + OffsetRelocLen, 4);
+  OS.EmitIntValue(FieldRelocLen, 4);
+  OS.EmitIntValue(FuncLen + LineLen + FieldRelocLen, 4);
   OS.EmitIntValue(ExternRelocLen, 4);
 
   // Emit func_info table.
@@ -832,18 +832,19 @@
   }
 
   // Emit offset reloc table.
-  if (OffsetRelocLen) {
-OS.AddComment("OffsetReloc");
-OS.EmitIntValue(BTF::BPFOffsetRelocSize, 4);
-for (const auto &OffsetRelocSec : OffsetRelocTable) {
+  if (FieldRelocLen) {
+OS.AddComment("FieldReloc");
+OS.EmitIntValue(BTF::BPFFieldRelocSize, 4);
+for (const auto &FieldRelocSec : FieldRe

[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D67773#1694797 , @mitchell-stellar 
wrote:

> This needs to be more thought out. At the very least it needs to cover more 
> keywords like `do`, `switch`, `try`, and `catch`. There may be more. 
> Consideration also needs to be made for `class`, functions, `namespace`, etc. 
> Otherwise this is an experimental feature at best. There is also confusion 
> with how it may interact with `SpaceBeforeCpp11BracedList`. The name 
> `SpacesAroundBraces` is just too generic in that regard considering the fact 
> that braces play multiple roles in C++.


Agreed


Repository:
  rC Clang

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

https://reviews.llvm.org/D67773



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


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-04 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX updated this revision to Diff 223240.

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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp
  clang/test/CodeGenCXX/debug-info-deleted.cpp
  clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 addFlag(SPDie, dwarf::DW_AT_elemental);
   if (SP->isRecursive())
 addFlag(SPDie, dwarf::DW_AT_recursive);
+  if (DD->getDwarfVersion() >= 5) {
+if (SP->isDefaultedInClass())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_in_class);
+if (SP->isDefaultedOutOfClass())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_out_of_class);
+if (SP->isNotDefaulted())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_no);
+if (SP->isDeleted())
+  addFlag(SPDie, dwarf::DW_AT_deleted);
+  }
 }
 
 void DwarfUnit::constructSubrangeDIE(DIE &Buffer, const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   return StringRef();
 }
 
+StringRef llvm::dwarf::DefaultedMemberString(unsigned DefaultedEncodings) {
+  switch (DefaultedEncodings) {
+  // Defaulted Member Encodings codes
+  case DW_DEFAULTED_no:
+return "DW_DEFAULTED_no";
+  case DW_DEFAULTED_in_class:
+return "DW_DEFAULTED_in_class";
+  case DW_DEFAULTED_out_of_class:
+return "DW_DEFAULTED_out_of_class";
+  }
+  return StringRef();
+}
+
 StringRef llvm::dwarf::VisibilityString(unsigned Visibility) {
   switch (Visibility) {
   case DW_VIS_local:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   bool isPure() const { return getSPFlags() & SPFlagPure; }
   bool isElemental() const { return getSPFlags() & SPFlagElemental; }
   bool isRecursive() const { return getSPFlags() & SPFlagRecursive; }
+  bool isDefaultedInClass() const {
+return getSPFlags() & SPFlagDefaultedInClass;
+  }
+  bool isDefaultedOutOfClass() const {
+return getSPFlags() & SPFlagDefaultedOutOfClass;
+  }
+  bool isNotDefaulted() const { return getSPFlags() & SPFlagNotDefaulted; }
+  bool isDeleted() const { return getSPFlags() & SPFlagDeleted; }
 
   /// Check if this is reference-qualified.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 HANDLE_DISP_FLAG((1u << 6), Elemental)
 HANDLE_DISP_FLAG((1u << 7), Recursive)
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), Deleted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
+HANDLE_DISP_FLAG((1u << 11), DefaultedOutOfClass)
+HANDLE_DISP_FLAG((1u << 12), NotDefaulted)
 
 #ifdef DISP_FLAG_LARGEST_NEEDED
 // Intended to be used with ADT/BitmaskEnum.h.
 // NOTE: Always must be equal to largest flag, check this when adding new flags.
-HANDLE_DISP_FLAG((1 << 8), Largest)
+HANDLE_DISP_FLAG((1 << 12), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED
 #endif
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 StringRef DecimalSignString(unsigned Sign);
 StringRef EndianityString(unsigned Endian);
 StringRef AccessibilityString(unsigned Access);
+StringRef DefaultedMemberString(unsigned DefaultedEncodings);
 StringRef VisibilityString(unsigned Visibility);
 StringRef VirtualityString(unsigned Virtuality);
 StringRef LanguageString(unsigned Language);
Index: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
@@ -0,0 +

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-04 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX marked 7 inline comments as done.
SouraVX added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1610
+  if (const auto *CXXC = dyn_cast(Method)) {
+if (CXXC->getCanonicalDecl()->isDeleted())
+  SPFlags |= llvm::DISubprogram::SPFlagDeleted;

aprantl wrote:
> can you factor this block out into a static function or lambda, since it is 
> repeated three times?
Refactored this into lambda, with additional return statement after every 
check, since every attribute is mutually exclusive. No need to check others 
once an attribute is set.



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp:6
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone -dwarf-version=5 \
+// RUN: | FileCheck %s -check-prefix=ATTR

aprantl wrote:
> the -dwarf-version flag should be unnecessary now, right?
Thanks for correcting again! Removed unnecessary flags from test cases.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

aprantl wrote:
> Since these are all mutually exclusive, it might make sense to do the same 
> thing as we did for virtuality above and thus save a bit or two.
Had this in mind when I added 4 flags but couldn't able to solve this back then 
so I put this. 

I tried refactoring this couple times based on your comments with results in 
failure. Virtuality and Defaulted member have same encodings "00-01-02". 
My approach was conflicting with Virtuality attributes, Since Virtuality here 
is already using 2 bits of LSB. 
Could you please suggest or hints to tackle this?


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

https://reviews.llvm.org/D68117



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


[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-10-04 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment.
Herald added a subscriber: mgehre.

Hi @alexfh, @jonathanmeier has reviewed my pull request but lacks commit 
access. It changes ~30 lines of code isolated to modernize-use-using.cpp and 
adds ~60 lines of tests. If you have time I'd greatly appreciate it if you 
could provide any feedback or commit it. Alternatively can you suggest someone 
else who can review it? Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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


[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar created this revision.
mitchell-stellar added reviewers: MyDeveloperDay, reuk, owenpan.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch makes the `SpacesInSquareBrackets` setting also apply to C++ lambdas 
with parameters.

Looking through the revision history, it appears support for only array 
brackets was added, and lambda brackets were ignored. Therefore, I am inclined 
to think it was simply an omission, rather than a deliberate choice.

See https://bugs.llvm.org/show_bug.cgi?id=17887 and 
https://reviews.llvm.org/D4944.


Repository:
  rC Clang

https://reviews.llvm.org/D68473

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10511,10 +10511,6 @@
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInSquareBrackets = true;
-  // Lambdas unchanged.
-  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
-  verifyFormat("return [i, args...] {};", Spaces);
-
   // Not lambdas.
   verifyFormat("int a[ 5 ];", Spaces);
   verifyFormat("a[ 3 ] += 42;", Spaces);
@@ -10525,6 +10521,9 @@
   verifyFormat("std::unique_ptr foo() {}", Spaces);
   verifyFormat("int i = a[ a ][ a ]->f();", Spaces);
   verifyFormat("int i = (*b)[ a ]->f();", Spaces);
+  // Lambdas.
+  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
+  verifyFormat("return [ i, args... ] {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2609,7 +2609,8 @@
 return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) 
&&
 SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
(Left.isOneOf(TT_ArraySubscriptLSquare,
- TT_StructuredBindingLSquare) &&
+ TT_StructuredBindingLSquare,
+ TT_LambdaLSquare) &&
 Style.SpacesInSquareBrackets && Right.isNot(tok::r_square));
   if (Right.is(tok::r_square))
 return Right.MatchingParen &&
@@ -2618,7 +2619,8 @@
  Style)) ||
 (Style.SpacesInSquareBrackets &&
  Right.MatchingParen->isOneOf(TT_ArraySubscriptLSquare,
-  TT_StructuredBindingLSquare)) ||
+  TT_StructuredBindingLSquare,
+  TT_LambdaLSquare)) ||
 Right.MatchingParen->is(TT_AttributeParen));
   if (Right.is(tok::l_square) &&
   !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2301,7 +2301,8 @@
 
 **SpacesInSquareBrackets** (``bool``)
   If ``true``, spaces will be inserted after ``[`` and before ``]``.
-  Lambdas or unspecified size array declarations will not be affected.
+  Lambdas without arguments or unspecified size array declarations will not be
+  affected.
 
   .. code-block:: c++
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10511,10 +10511,6 @@
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInSquareBrackets = true;
-  // Lambdas unchanged.
-  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
-  verifyFormat("return [i, args...] {};", Spaces);
-
   // Not lambdas.
   verifyFormat("int a[ 5 ];", Spaces);
   verifyFormat("a[ 3 ] += 42;", Spaces);
@@ -10525,6 +10521,9 @@
   verifyFormat("std::unique_ptr foo() {}", Spaces);
   verifyFormat("int i = a[ a ][ a ]->f();", Spaces);
   verifyFormat("int i = (*b)[ a ]->f();", Spaces);
+  // Lambdas.
+  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
+  verifyFormat("return [ i, args... ] {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2609,7 +2609,8 @@
 return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) &&
 SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
(Left.isOneOf(TT_ArraySubscriptLSquare,
- TT_StructuredBindingLSquare) &&
+

[PATCH] D68430: Don't use object libraries with Xcode

2019-10-04 Thread Jordan Rose via Phabricator via cfe-commits
jordan_rose added a comment.

In D68430#1693965 , @beanz wrote:

> clang_cpp can't link the libraries "normally" because it has no unresolved 
> symbols to force the contents of the libraries to link. I don't like it, but 
> I think the best option is to disable clang_cpp under Xcode. You can add `AND 
> XCODE` to the `if` on line 2 of clang/tools/clang-shlib/CMakeLists.txt, and 
> that should do the trick.


I do think it's okay to just not support that target for Xcode, but another 
option would be to add `-all_load` to the linker line, the ld64 option that 
just unconditionally includes all archives. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68430



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


[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

Thanks for the patch.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D68473



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


[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

2019-10-04 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment.

Thanks, please commit on my behalf.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68473



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


[PATCH] D68430: Don't use object libraries with Xcode

2019-10-04 Thread Jordan Rose via Phabricator via cfe-commits
jordan_rose updated this revision to Diff 223250.
jordan_rose added a comment.

Okay, having Xcode force-load the static libraries doesn't seem bad at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68430

Files:
  clang/cmake/modules/AddClang.cmake
  clang/tools/clang-shlib/CMakeLists.txt


Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -6,7 +6,13 @@
 get_property(clang_libs GLOBAL PROPERTY CLANG_STATIC_LIBS)
 
 foreach (lib ${clang_libs})
-  list(APPEND _OBJECTS $)
+  if(XCODE)
+# Xcode doesn't support object libraries, so we have to trick it into
+# linking the static libraries instead.
+list(APPEND _DEPS "-force_load" ${lib})
+  else()
+list(APPEND _OBJECTS $)
+  endif()
   list(APPEND _DEPS $)
   list(APPEND _DEPS $)
 endforeach ()
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -86,9 +86,13 @@
 # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
 # so we need to handle it here.
 if(BUILD_SHARED_LIBS)
-  set(LIBTYPE SHARED OBJECT)
+  set(LIBTYPE SHARED)
 else()
-  set(LIBTYPE STATIC OBJECT)
+  set(LIBTYPE STATIC)
+endif()
+if(NOT XCODE)
+  # The Xcode generator doesn't handle object libraries correctly.
+  list(APPEND LIBTYPE OBJECT)
 endif()
 set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
   endif()


Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -6,7 +6,13 @@
 get_property(clang_libs GLOBAL PROPERTY CLANG_STATIC_LIBS)
 
 foreach (lib ${clang_libs})
-  list(APPEND _OBJECTS $)
+  if(XCODE)
+# Xcode doesn't support object libraries, so we have to trick it into
+# linking the static libraries instead.
+list(APPEND _DEPS "-force_load" ${lib})
+  else()
+list(APPEND _OBJECTS $)
+  endif()
   list(APPEND _DEPS $)
   list(APPEND _DEPS $)
 endforeach ()
Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -86,9 +86,13 @@
 # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
 # so we need to handle it here.
 if(BUILD_SHARED_LIBS)
-  set(LIBTYPE SHARED OBJECT)
+  set(LIBTYPE SHARED)
 else()
-  set(LIBTYPE STATIC OBJECT)
+  set(LIBTYPE STATIC)
+endif()
+if(NOT XCODE)
+  # The Xcode generator doesn't handle object libraries correctly.
+  list(APPEND LIBTYPE OBJECT)
 endif()
 set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
   endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

2019-10-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks!


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

https://reviews.llvm.org/D65695



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


[PATCH] D64874: [Sema] Improve handling of function pointer conversions

2019-10-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM, though I'd like to see this generalized to handle noreturn too.




Comment at: clang/lib/Sema/SemaTemplate.cpp:6997
+  QualType ResultTy;
+  if (getLangOpts().CPlusPlus17 &&
+  IsFunctionConversion(((Expr *)RefExpr.get())->getType(),

Can we remove the check for C++17 here? I would expect we want to consider the 
other kind of function conversion (from noreturn function to 
potentially-returning function) here too, and that applies in all language 
modes.

Testcase for that bug:

```
template struct X {};
int f() __attribute__((noreturn));
X<&f> x;
```


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

https://reviews.llvm.org/D64874



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


[PATCH] D64820: [Sema] Avoids an assertion failure when an invalid conversion declaration is used

2019-10-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D64820



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


[PATCH] D68430: Don't use object libraries with Xcode

2019-10-04 Thread Chris Bieneman via Phabricator via cfe-commits
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

That's fine. That said, there are things that just can't be done, or don't work 
well, in the Xcode and Visual Studio generators, so we have precedent for 
disabling functionality based on those generators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68430



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


r373769 - [CMake] Clang: Don't use object libraries with Xcode

2019-10-04 Thread Jordan Rose via cfe-commits
Author: jrose
Date: Fri Oct  4 11:17:58 2019
New Revision: 373769

URL: http://llvm.org/viewvc/llvm-project?rev=373769&view=rev
Log:
[CMake] Clang: Don't use object libraries with Xcode

Undoes some of the effects of r360946 when using the Xcode CMake
generator---it doesn't handle object libraries correctly at all.
Attempts to still honor BUILD_SHARED_LIBS for Xcode, but I didn't
actually test it. Should have no effect on non-Xcode generators.

https://reviews.llvm.org/D68430

Modified:
cfe/trunk/cmake/modules/AddClang.cmake
cfe/trunk/tools/clang-shlib/CMakeLists.txt

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=373769&r1=373768&r2=373769&view=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Fri Oct  4 11:17:58 2019
@@ -86,9 +86,13 @@ macro(add_clang_library name)
 # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
 # so we need to handle it here.
 if(BUILD_SHARED_LIBS)
-  set(LIBTYPE SHARED OBJECT)
+  set(LIBTYPE SHARED)
 else()
-  set(LIBTYPE STATIC OBJECT)
+  set(LIBTYPE STATIC)
+endif()
+if(NOT XCODE)
+  # The Xcode generator doesn't handle object libraries correctly.
+  list(APPEND LIBTYPE OBJECT)
 endif()
 set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
   endif()

Modified: cfe/trunk/tools/clang-shlib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-shlib/CMakeLists.txt?rev=373769&r1=373768&r2=373769&view=diff
==
--- cfe/trunk/tools/clang-shlib/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-shlib/CMakeLists.txt Fri Oct  4 11:17:58 2019
@@ -6,7 +6,13 @@ endif()
 get_property(clang_libs GLOBAL PROPERTY CLANG_STATIC_LIBS)
 
 foreach (lib ${clang_libs})
-  list(APPEND _OBJECTS $)
+  if(XCODE)
+# Xcode doesn't support object libraries, so we have to trick it into
+# linking the static libraries instead.
+list(APPEND _DEPS "-force_load" ${lib})
+  else()
+list(APPEND _OBJECTS $)
+  endif()
   list(APPEND _DEPS $)
   list(APPEND _DEPS $)
 endforeach ()


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


[PATCH] D68430: Don't use object libraries with Xcode

2019-10-04 Thread Jordan Rose via Phabricator via cfe-commits
jordan_rose closed this revision.
jordan_rose added a comment.

Committed as rC373769 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68430



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


[PATCH] D68114: Fix for expanding __pragmas in macro arguments

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

- move TokenCollector out of function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68114

Files:
  clang/lib/Lex/Pragma.cpp
  clang/test/Preprocessor/pragma_microsoft.c

Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -51,6 +51,8 @@
   __pragma(warning(pop)); \
 }
 
+#define PRAGMA_IN_ARGS(p) p
+
 void f()
 {
   __pragma() // expected-warning{{unknown pragma ignored}}
@@ -64,8 +66,16 @@
 // CHECK: #pragma warning(disable: 1)
 // CHECK: ; 1 + (2 > 3) ? 4 : 5;
 // CHECK: #pragma warning(pop)
-}
 
+  // Check that macro arguments can contain __pragma.
+  PRAGMA_IN_ARGS(MACRO_WITH__PRAGMA) // expected-warning {{lower precedence}} \
+ // expected-note 2 {{place parentheses}} \
+ // expected-warning {{expression result unused}}
+// CHECK: #pragma warning(push)
+// CHECK: #pragma warning(disable: 1)
+// CHECK: ; 1 + (2 > 3) ? 4 : 5;
+// CHECK: #pragma warning(pop)
+}
 
 // This should include macro_arg_directive even though the include
 // is looking for test.h  This allows us to assign to "n"
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -121,6 +121,40 @@
 // Preprocessor Pragma Directive Handling.
 //===--===//
 
+namespace {
+// TokenCollector provides the option to collect tokens that were "read"
+// and return them to the stream to be read later.
+// Currently used when reading _Pragma/__pragma directives.
+struct TokenCollector {
+  Preprocessor &Self;
+  bool Collect;
+  SmallVector Tokens;
+  Token &Tok;
+
+  void lex() {
+if (Collect)
+  Tokens.push_back(Tok);
+Self.Lex(Tok);
+  }
+
+  void revert() {
+assert(Collect && "did not collect tokens");
+assert(!Tokens.empty() && "collected unexpected number of tokens");
+
+// Push the ( "string" ) tokens into the token stream.
+auto Toks = std::make_unique(Tokens.size());
+std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get());
+Toks[Tokens.size() - 1] = Tok;
+Self.EnterTokenStream(std::move(Toks), Tokens.size(),
+  /*DisableMacroExpansion*/ true,
+  /*IsReinject*/ true);
+
+// ... and return the pragma token unchanged.
+Tok = *Tokens.begin();
+  }
+};
+} // namespace
+
 /// HandlePragmaDirective - The "\#pragma" directive has been parsed.  Lex the
 /// rest of the pragma, passing it to the registered pragma handlers.
 void Preprocessor::HandlePragmaDirective(PragmaIntroducer Introducer) {
@@ -166,35 +200,6 @@
   // In Case #2, we check the syntax now, but then put the tokens back into the
   // token stream for later consumption.
 
-  struct TokenCollector {
-Preprocessor &Self;
-bool Collect;
-SmallVector Tokens;
-Token &Tok;
-
-void lex() {
-  if (Collect)
-Tokens.push_back(Tok);
-  Self.Lex(Tok);
-}
-
-void revert() {
-  assert(Collect && "did not collect tokens");
-  assert(!Tokens.empty() && "collected unexpected number of tokens");
-
-  // Push the ( "string" ) tokens into the token stream.
-  auto Toks = std::make_unique(Tokens.size());
-  std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get());
-  Toks[Tokens.size() - 1] = Tok;
-  Self.EnterTokenStream(std::move(Toks), Tokens.size(),
-/*DisableMacroExpansion*/ true,
-/*IsReinject*/ true);
-
-  // ... and return the _Pragma token unchanged.
-  Tok = *Tokens.begin();
-}
-  };
-
   TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok};
 
   // Remember the pragma token location.
@@ -328,11 +333,15 @@
 /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text
 /// is not enclosed within a string literal.
 void Preprocessor::HandleMicrosoft__pragma(Token &Tok) {
+  // During macro pre-expansion, check the syntax now but put the tokens back
+  // into the token stream for later consumption. Same as Handle_Pragma.
+  TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok};
+
   // Remember the pragma token location.
   SourceLocation PragmaLoc = Tok.getLocation();
 
   // Read the '('.
-  Lex(Tok);
+  Toks.lex();
   if (Tok.isNot(tok::l_paren)) {
 Diag(PragmaLoc, diag::err__Pragma_malformed);
 return;
@@ -341,14 +350,14 @@
   // Get the tokens enclosed within the __pragma(), as well as the final ')'.
   SmallVector PragmaToks;
   int NumParens = 0;
-  Lex(Tok);
+  Toks.lex();
   while (Tok.isNot(tok::eof)) {
 PragmaToks.push_back(Tok);
 if (Tok.is(tok:

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Decl.cpp:3107
+if (!ArmMveAliasValid(BuiltinID, getIdentifier()->getName())) {
+  getASTContext().getDiagnostics().Report(
+getLocation(), diag::err_attribute_arm_mve_alias);

simon_tatham wrote:
> aaron.ballman wrote:
> > simon_tatham wrote:
> > > aaron.ballman wrote:
> > > > I'm not certain how comfortable I am with having this function produce 
> > > > a diagnostic. That seems like unexpected behavior for a function 
> > > > attempting to get a builtin ID. I think this should be the 
> > > > responsibility of the caller.
> > > The //caller//? But there are many possible callers of this function. You 
> > > surely didn't mean to suggest duplicating the diagnostic at all those 
> > > call sites.
> > > 
> > > Perhaps it would make more sense to have all the calculation in this 
> > > `getBuiltinID` method move into a function called once, early in the 
> > > `FunctionDecl`'s lifetime, which figures out the builtin ID (if any) and 
> > > stashes it in a member variable? Then //that// would issue the 
> > > diagnostic, if any (and it would be called from a context where that was 
> > > a sensible thing to do), and `getBuiltinID` itself would become a mere 
> > > accessor function.
> > > The caller? But there are many possible callers of this function. You 
> > > surely didn't mean to suggest duplicating the diagnostic at all those 
> > > call sites.
> > 
> > Yes, I did. :-) No caller is going to expect that calling a `const` 
> > function that gets a builtin ID is going to issue diagnostics and so this 
> > runs the risk of generating diagnostics in surprising situations, such as 
> > from AST matchers.
> > 
> > > Perhaps it would make more sense to have all the calculation in this 
> > > getBuiltinID method move into a function called once, early in the 
> > > FunctionDecl's lifetime, which figures out the builtin ID (if any) and 
> > > stashes it in a member variable? Then that would issue the diagnostic, if 
> > > any (and it would be called from a context where that was a sensible 
> > > thing to do), and getBuiltinID itself would become a mere accessor 
> > > function.
> > 
> > That might make sense, but I don't have a good idea of what performance 
> > concerns that might raise. If there are a lot of functions and we never 
> > need to check if they have a builtin ID, that could be expensive for little 
> > gain.
> OK – so actually what you meant to suggest was to put the diagnostic at just 
> //some// of the call sites for `getBuiltinId`?
> 
> With the intended behavior being that the Sema test in this patch should 
> still provoke all the expected diagnostics in an ordinary compilation 
> context, but in other situations like AST matchers, it would be better for 
> `getBuiltinId` to //silently// returns 0 if there's an illegal ArmMveAlias 
> attribute?
> 
> (I'm just checking I've understood you correctly before I do the work...)
> OK – so actually what you meant to suggest was to put the diagnostic at just 
> some of the call sites for getBuiltinId?

Yes! Sorry, I can see how I was unclear before. :-)

> With the intended behavior being that the Sema test in this patch should 
> still provoke all the expected diagnostics in an ordinary compilation 
> context, but in other situations like AST matchers, it would be better for 
> getBuiltinId to silently returns 0 if there's an illegal ArmMveAlias 
> attribute?

Yes. `getBuiltinId()` already returns `0` in error cases without diagnosing, 
such as the function being unnamed or not being a builtin. I want to retain 
that property -- this function returns zero if the function is not a builtin. 
It's up to the caller of the function to decide whether a zero return value 
should be diagnosed or not.

To be honest, this diagnostic feels like it belongs in SemaDeclAttr.cpp; it is 
placing a constraint on which declarations can have the attribute, so that 
should be checked *before* applying the attribute to the declaration. This also 
keeps the AST cleaner by not having an attribute on a function which should not 
be attributed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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


[PATCH] D24243: [clang-move] A prototype tool for moving class definition to new file.

2019-10-04 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Maybe it could be mentioned in the release notes?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D24243



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Builtins.def:483
 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF")
 BUILTIN(__builtin_memmove, "v*v*vC*z", "nF")

xbolva00 wrote:
> aaron.ballman wrote:
> > xbolva00 wrote:
> > > aaron.ballman wrote:
> > > > GCC doesn't seem to have `__builtin_memccpy`? 
> > > > https://godbolt.org/z/jbthQ3
> > > Ok, I will drop it.
> > If you drop it, won't that lose the builtin? I was mostly thinking it's in 
> > the wrong part of the list of builtins.
> Rebuilding LLVM + Clang in progress so I just checked it in godbolt with 
> "strtol" - defined only as LIBBUILTIN, no __builtin version.
> 
> nobuiltin attribute is correctly handled, so I think it will work. 
> https://godbolt.org/z/Olfv-w
Ah, I see the issue better now. The description for this review was very terse 
and it wasn't immediately clear what problem you were trying to solve.

Yeah, I think this declaration can go away.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

Isn't `memccpy` supported by Visual Studio? What should happen there?


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

https://reviews.llvm.org/D68377



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


[PATCH] D68114: Fix for expanding __pragmas in macro arguments

2019-10-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: Bigcheese.
rnk added a comment.

+ @Bigcheese, for amusement. We discussed some interesting behavior of _Pragma 
and C++ raw string literals last night.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68114



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


[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for this!




Comment at: clang/test/Sema/format-strings-pedantic.c:1
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-pedantic -isystem 
%S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-type-confusion %s
 

erik.pilkington wrote:
> aaron.ballman wrote:
> > Are we losing test coverage for `-Wformat-pedantic`, or do we have other 
> > tests covering that elsewhere? I would have expected this test file's 
> > contents to exercise pedantic cases.
> The only warning that was in this file is now under -Wformat-type-confusion. 
> New patch adds a test for the `printf("%p", (int*)0);` thing, which was 
> otherwise untested.
Thank you for adding that!



Comment at: clang/test/Sema/format-type-confusion.c:13
+ b, // expected-warning {{format specifies type 'unsigned short' but 
the argument has type '_Bool'}}
+ b, b, b, b, b);
+

erik.pilkington wrote:
> aaron.ballman wrote:
> > Just double-checking, but the reason we don't diagnose the `%c` here is 
> > because of `-Wno-format`?
> Yup, exactly. I just wanted to test -Wformat-type-confusion alone in this 
> file. 
Awesome, thank you for the verification!


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

https://reviews.llvm.org/D67775



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


[PATCH] D24243: [clang-move] A prototype tool for moving class definition to new file.

2019-10-04 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.

In D24243#1695052 , @sylvestre.ledru 
wrote:

> Maybe it could be mentioned in the release notes?


Forget about this comment, I have been distracted by the fact that it is now 
installed
https://reviews.llvm.org/D68423


Repository:
  rL LLVM

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

https://reviews.llvm.org/D24243



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


[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

2019-10-04 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

I confirmed the test case matches the name that MSVC produces. I guess this 
regressed in rC362293  / D62746 
, so the bug is in clang 9.0, but not 8.0. 
Maybe we should merge this to 9.0.1.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68099



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


r373771 - Add missing null pointer check in -ftime-trace code

2019-10-04 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Oct  4 11:57:01 2019
New Revision: 373771

URL: http://llvm.org/viewvc/llvm-project?rev=373771&view=rev
Log:
Add missing null pointer check in -ftime-trace code

createOutputFile diagnoses the error for the caller already, so recover
by not writing the output.

Fixes PR43555

No test, since I couldn't think of a good, portable, simple way to make
the regular -o output file writable, but outputfile.json not writable.

Modified:
cfe/trunk/tools/driver/cc1_main.cpp

Modified: cfe/trunk/tools/driver/cc1_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1_main.cpp?rev=373771&r1=373770&r2=373771&view=diff
==
--- cfe/trunk/tools/driver/cc1_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1_main.cpp Fri Oct  4 11:57:01 2019
@@ -258,19 +258,20 @@ int cc1_main(ArrayRef Argv
   if (llvm::timeTraceProfilerEnabled()) {
 SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
 llvm::sys::path::replace_extension(Path, "json");
-auto profilerOutput =
-Clang->createOutputFile(Path.str(),
-/*Binary=*/false,
-/*RemoveFileOnSignal=*/false, "",
-/*Extension=*/"json",
-/*useTemporary=*/false);
+if (auto profilerOutput =
+Clang->createOutputFile(Path.str(),
+/*Binary=*/false,
+/*RemoveFileOnSignal=*/false, "",
+/*Extension=*/"json",
+/*useTemporary=*/false)) {
 
-llvm::timeTraceProfilerWrite(*profilerOutput);
-// FIXME(ibiryukov): make profilerOutput flush in destructor instead.
-profilerOutput->flush();
-llvm::timeTraceProfilerCleanup();
+  llvm::timeTraceProfilerWrite(*profilerOutput);
+  // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
+  profilerOutput->flush();
+  llvm::timeTraceProfilerCleanup();
 
-llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+  llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+}
   }
 
   // Our error handler depends on the Diagnostics object, which we're


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


r373774 - [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

2019-10-04 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Fri Oct  4 12:20:27 2019
New Revision: 373774

URL: http://llvm.org/viewvc/llvm-project?rev=373774&view=rev
Log:
[Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

The warnings now in -Wformat-type-confusion don't align with how we interpret
'pedantic' in clang, and don't belong in -pedantic.

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

Added:
cfe/trunk/test/Sema/format-type-confusion.c
Modified:
cfe/trunk/include/clang/AST/FormatString.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/FormatString.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-bool.c
cfe/trunk/test/Sema/format-strings-pedantic.c

Modified: cfe/trunk/include/clang/AST/FormatString.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/FormatString.h?rev=373774&r1=373773&r2=373774&view=diff
==
--- cfe/trunk/include/clang/AST/FormatString.h (original)
+++ cfe/trunk/include/clang/AST/FormatString.h Fri Oct  4 12:20:27 2019
@@ -251,7 +251,21 @@ public:
   enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
   AnyCharTy, CStrTy, WCStrTy, WIntTy };
 
-  enum MatchKind { NoMatch = 0, Match = 1, NoMatchPedantic };
+  /// How well a given conversion specifier matches its argument.
+  enum MatchKind {
+/// The conversion specifier and the argument types are incompatible. For
+/// instance, "%d" and float.
+NoMatch = 0,
+/// The conversion specifier and the argument type are compatible. For
+/// instance, "%d" and _Bool.
+Match = 1,
+/// The conversion specifier and the argument type are disallowed by the C
+/// standard, but are in practice harmless. For instance, "%p" and int*.
+NoMatchPedantic,
+/// The conversion specifier and the argument type are compatible, but 
still
+/// seems likely to be an error. For instance, "%hd" and _Bool.
+NoMatchTypeConfusion,
+  };
 
 private:
   const Kind K;

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=373774&r1=373773&r2=373774&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Oct  4 12:20:27 2019
@@ -780,6 +780,7 @@ def FormatSecurity : DiagGroup<"format-s
 def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
+def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
 def Format : DiagGroup<"format",
[FormatExtraArgs, FormatZeroLength, NonNull,
 FormatSecurity, FormatY2K, FormatInvalidSpecifier]>,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=373774&r1=373773&r2=373774&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct  4 12:20:27 
2019
@@ -8092,16 +8092,17 @@ def warn_format_conversion_argument_type
   "%select{type|underlying type}2 %1">,
   InGroup;
 def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
-  "format specifies type %0 but the argument has "
-  "%select{type|underlying type}2 %1">,
+  warn_format_conversion_argument_type_mismatch.Text>,
   InGroup;
+def warn_format_conversion_argument_type_mismatch_confusion : Warning<
+  warn_format_conversion_argument_type_mismatch.Text>,
+  InGroup, DefaultIgnore;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,
   InGroup;
 def warn_format_argument_needs_cast_pedantic : Warning<
-  "%select{values of type|enum values with underlying type}2 '%0' should not "
-  "be used as format arguments; add an explicit cast to %1 instead">,
+  warn_format_argument_needs_cast.Text>,
   InGroup, DefaultIgnore;
 def warn_printf_positional_arg_exceeds_data_args : Warning <
   "data argument position '%0' exceeds the number of data arguments (%1)">,

Modified: cfe/trunk/lib/AST/FormatString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/FormatString.cpp?rev=373774&r1=373773&r2=373774&view=diff
==
--- cfe/trunk/lib/AST/FormatString.cpp (original)
+++ cfe/trunk/lib/AST/FormatString.cpp Fri Oct  4 12:20:27 2019
@@ -389,7 +389,7 @@ ArgType::matchesType(ASTContext &C, Qual
   case Buil

[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

2019-10-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373774: [Sema] Split out -Wformat-type-confusion from 
-Wformat-pedantic (authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67775?vs=223136&id=223262#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67775

Files:
  cfe/trunk/include/clang/AST/FormatString.h
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/FormatString.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/format-bool.c
  cfe/trunk/test/Sema/format-strings-pedantic.c
  cfe/trunk/test/Sema/format-type-confusion.c

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8092,16 +8092,17 @@
   "%select{type|underlying type}2 %1">,
   InGroup;
 def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
-  "format specifies type %0 but the argument has "
-  "%select{type|underlying type}2 %1">,
+  warn_format_conversion_argument_type_mismatch.Text>,
   InGroup;
+def warn_format_conversion_argument_type_mismatch_confusion : Warning<
+  warn_format_conversion_argument_type_mismatch.Text>,
+  InGroup, DefaultIgnore;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,
   InGroup;
 def warn_format_argument_needs_cast_pedantic : Warning<
-  "%select{values of type|enum values with underlying type}2 '%0' should not "
-  "be used as format arguments; add an explicit cast to %1 instead">,
+  warn_format_argument_needs_cast.Text>,
   InGroup, DefaultIgnore;
 def warn_printf_positional_arg_exceeds_data_args : Warning <
   "data argument position '%0' exceeds the number of data arguments (%1)">,
Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -780,6 +780,7 @@
 def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
+def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
 def Format : DiagGroup<"format",
[FormatExtraArgs, FormatZeroLength, NonNull,
 FormatSecurity, FormatY2K, FormatInvalidSpecifier]>,
Index: cfe/trunk/include/clang/AST/FormatString.h
===
--- cfe/trunk/include/clang/AST/FormatString.h
+++ cfe/trunk/include/clang/AST/FormatString.h
@@ -251,7 +251,21 @@
   enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
   AnyCharTy, CStrTy, WCStrTy, WIntTy };
 
-  enum MatchKind { NoMatch = 0, Match = 1, NoMatchPedantic };
+  /// How well a given conversion specifier matches its argument.
+  enum MatchKind {
+/// The conversion specifier and the argument types are incompatible. For
+/// instance, "%d" and float.
+NoMatch = 0,
+/// The conversion specifier and the argument type are compatible. For
+/// instance, "%d" and _Bool.
+Match = 1,
+/// The conversion specifier and the argument type are disallowed by the C
+/// standard, but are in practice harmless. For instance, "%p" and int*.
+NoMatchPedantic,
+/// The conversion specifier and the argument type are compatible, but still
+/// seems likely to be an error. For instance, "%hd" and _Bool.
+NoMatchTypeConfusion,
+  };
 
 private:
   const Kind K;
Index: cfe/trunk/test/Sema/format-type-confusion.c
===
--- cfe/trunk/test/Sema/format-type-confusion.c
+++ cfe/trunk/test/Sema/format-type-confusion.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9.0 -fsyntax-only -verify -Wno-format -Wformat-type-confusion %s
+
+__attribute__((format(__printf__, 1, 2)))
+int printf(const char *msg, ...);
+
+#define FMT "%hd %hu %d %u %hhd %hhu %c"
+
+int main() {
+  _Bool b = 0;
+  printf(FMT,
+ b, // expected-warning {{format specifies type 'short' but the argument has type '_Bool'}}
+ b, // expected-warning {{format specifies type 'unsigned short' but the argument has type '_Bool'}}
+ b, b, b, b, b);
+
+  unsigned char uc = 0;
+  printf(FMT,
+ uc, // expected-warning {{format specifies type 'short' but the argument has type 'unsigned char'}}
+ uc, // expected-warning {{format specifies type 'unsigned short' but the argument has type 'un

[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

aaron.ballman wrote:
> Isn't `memccpy` supported by Visual Studio? What should happen there?
”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy 
instead.“


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

https://reviews.llvm.org/D68377



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

xbolva00 wrote:
> aaron.ballman wrote:
> > Isn't `memccpy` supported by Visual Studio? What should happen there?
> ”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy 
> instead.“
Thanks for verifying it's still supported (deprecated != unsupported). We do 
support other Microsoft builtins, so should this one be added?


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

https://reviews.llvm.org/D68377



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > Isn't `memccpy` supported by Visual Studio? What should happen there?
> > ”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy 
> > instead.“
> Thanks for verifying it's still supported (deprecated != unsupported). We do 
> support other Microsoft builtins, so should this one be added?
Ok, I will add it.

One last question, should I somehow represent that memccpy is in C 20 (if so, 
how) ?


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

https://reviews.llvm.org/D68377



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

xbolva00 wrote:
> aaron.ballman wrote:
> > xbolva00 wrote:
> > > aaron.ballman wrote:
> > > > Isn't `memccpy` supported by Visual Studio? What should happen there?
> > > ”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy 
> > > instead.“
> > Thanks for verifying it's still supported (deprecated != unsupported). We 
> > do support other Microsoft builtins, so should this one be added?
> Ok, I will add it.
> 
> One last question, should I somehow represent that memccpy is in C 20 (if so, 
> how) ?
You should probably start a new section for C2x library functions and add 
`memccpy`, `strdup`, and `strndup` to it.

Also, just to double-check: we don't encode `restrict` into builtin signatures, 
do we? (Should we?)


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

https://reviews.llvm.org/D68377



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/Builtins.def:483
 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF")
 BUILTIN(__builtin_memmove, "v*v*vC*z", "nF")

aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > xbolva00 wrote:
> > > > aaron.ballman wrote:
> > > > > GCC doesn't seem to have `__builtin_memccpy`? 
> > > > > https://godbolt.org/z/jbthQ3
> > > > Ok, I will drop it.
> > > If you drop it, won't that lose the builtin? I was mostly thinking it's 
> > > in the wrong part of the list of builtins.
> > Rebuilding LLVM + Clang in progress so I just checked it in godbolt with 
> > "strtol" - defined only as LIBBUILTIN, no __builtin version.
> > 
> > nobuiltin attribute is correctly handled, so I think it will work. 
> > https://godbolt.org/z/Olfv-w
> Ah, I see the issue better now. The description for this review was very 
> terse and it wasn't immediately clear what problem you were trying to solve.
> 
> Yeah, I think this declaration can go away.
Yeah, I am sorry, more context why this is needed:

https://reviews.llvm.org/D67986


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

https://reviews.llvm.org/D68377



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


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

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



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1618
+  return;
+} else if (const auto Def = Method->getDefinition()) {
+  if (Def->isDefaulted()) {

LLVM style is not to use 'else' after 'return'.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

SouraVX wrote:
> aprantl wrote:
> > Since these are all mutually exclusive, it might make sense to do the same 
> > thing as we did for virtuality above and thus save a bit or two.
> Had this in mind when I added 4 flags but couldn't able to solve this back 
> then so I put this. 
> 
> I tried refactoring this couple times based on your comments with results in 
> failure. Virtuality and Defaulted member have same encodings "00-01-02". 
> My approach was conflicting with Virtuality attributes, Since Virtuality here 
> is already using 2 bits of LSB. 
> Could you please suggest or hints to tackle this?
Wouldn't the values for those constants be (1u << 9), (2u << 9), (3u << 9), (4u 
<< 9) ?  Look at how the Single/Multiple/VirtualInheritance flags work, earlier 
in this file.

And if we said a zero value meant NotDefaulted (which is what zero has meant up 
to now, so might as well keep that), then you need only 3 new flag values which 
will fit in 2 bits.



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

https://reviews.llvm.org/D68117



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > xbolva00 wrote:
> > > > aaron.ballman wrote:
> > > > > Isn't `memccpy` supported by Visual Studio? What should happen there?
> > > > ”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy 
> > > > instead.“
> > > Thanks for verifying it's still supported (deprecated != unsupported). We 
> > > do support other Microsoft builtins, so should this one be added?
> > Ok, I will add it.
> > 
> > One last question, should I somehow represent that memccpy is in C 20 (if 
> > so, how) ?
> You should probably start a new section for C2x library functions and add 
> `memccpy`, `strdup`, and `strndup` to it.
> 
> Also, just to double-check: we don't encode `restrict` into builtin 
> signatures, do we? (Should we?)
>> You should probably start a new section for C2x library functions and add 
>> memccpy, strdup, and strndup to it.
Okay, thanks - probably not needed for now (should be separate patch to handle 
new functions in C 2X anyway; maybe more functions to be added?)

>> Also, just to double-check: we don't encode restrict into builtin 
>> signatures, do we? (Should we?)
Maybe catch very simple overlap issues? I dont think this is worth it (we have 
sanitizers). LLVM annotates libc functions with noalias anyway.


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

https://reviews.llvm.org/D68377



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


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

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



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

probinson wrote:
> SouraVX wrote:
> > aprantl wrote:
> > > Since these are all mutually exclusive, it might make sense to do the 
> > > same thing as we did for virtuality above and thus save a bit or two.
> > Had this in mind when I added 4 flags but couldn't able to solve this back 
> > then so I put this. 
> > 
> > I tried refactoring this couple times based on your comments with results 
> > in failure. Virtuality and Defaulted member have same encodings "00-01-02". 
> > My approach was conflicting with Virtuality attributes, Since Virtuality 
> > here is already using 2 bits of LSB. 
> > Could you please suggest or hints to tackle this?
> Wouldn't the values for those constants be (1u << 9), (2u << 9), (3u << 9), 
> (4u << 9) ?  Look at how the Single/Multiple/VirtualInheritance flags work, 
> earlier in this file.
> 
> And if we said a zero value meant NotDefaulted (which is what zero has meant 
> up to now, so might as well keep that), then you need only 3 new flag values 
> which will fit in 2 bits.
> 
And if you need to create an additional zero enum or mask enum, you can do that 
in DebugInfoMetadata.h; both DIFlags and DISPFlags do this kind of thing.


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

https://reviews.llvm.org/D68117



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Builtins.def:483
 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF")
 BUILTIN(__builtin_memmove, "v*v*vC*z", "nF")

xbolva00 wrote:
> aaron.ballman wrote:
> > xbolva00 wrote:
> > > aaron.ballman wrote:
> > > > xbolva00 wrote:
> > > > > aaron.ballman wrote:
> > > > > > GCC doesn't seem to have `__builtin_memccpy`? 
> > > > > > https://godbolt.org/z/jbthQ3
> > > > > Ok, I will drop it.
> > > > If you drop it, won't that lose the builtin? I was mostly thinking it's 
> > > > in the wrong part of the list of builtins.
> > > Rebuilding LLVM + Clang in progress so I just checked it in godbolt with 
> > > "strtol" - defined only as LIBBUILTIN, no __builtin version.
> > > 
> > > nobuiltin attribute is correctly handled, so I think it will work. 
> > > https://godbolt.org/z/Olfv-w
> > Ah, I see the issue better now. The description for this review was very 
> > terse and it wasn't immediately clear what problem you were trying to solve.
> > 
> > Yeah, I think this declaration can go away.
> Yeah, I am sorry, more context why this is needed:
> 
> https://reviews.llvm.org/D67986
Thank you for the extra context!



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

xbolva00 wrote:
> aaron.ballman wrote:
> > xbolva00 wrote:
> > > aaron.ballman wrote:
> > > > xbolva00 wrote:
> > > > > aaron.ballman wrote:
> > > > > > Isn't `memccpy` supported by Visual Studio? What should happen 
> > > > > > there?
> > > > > ”This POSIX function is deprecated. Use the ISO C++ conformant 
> > > > > _memccpy instead.“
> > > > Thanks for verifying it's still supported (deprecated != unsupported). 
> > > > We do support other Microsoft builtins, so should this one be added?
> > > Ok, I will add it.
> > > 
> > > One last question, should I somehow represent that memccpy is in C 20 (if 
> > > so, how) ?
> > You should probably start a new section for C2x library functions and add 
> > `memccpy`, `strdup`, and `strndup` to it.
> > 
> > Also, just to double-check: we don't encode `restrict` into builtin 
> > signatures, do we? (Should we?)
> >> You should probably start a new section for C2x library functions and add 
> >> memccpy, strdup, and strndup to it.
> Okay, thanks - probably not needed for now (should be separate patch to 
> handle new functions in C 2X anyway; maybe more functions to be added?)
> 
> >> Also, just to double-check: we don't encode restrict into builtin 
> >> signatures, do we? (Should we?)
> Maybe catch very simple overlap issues? I dont think this is worth it (we 
> have sanitizers). LLVM annotates libc functions with noalias anyway.
> Okay, thanks - probably not needed for now (should be separate patch to 
> handle new functions in C 2X anyway; maybe more functions to be added?)

Yeah, can totally be done in a separate patch.

> Maybe catch very simple overlap issues? I dont think this is worth it (we 
> have sanitizers). LLVM annotates libc functions with noalias anyway.

Sanitizers don't run everywhere, but more importantly, I was wondering about 
the possible optimization opportunities by noting which arguments cannot 
overlap with one another (perhaps noalias covers that; I'm less familiar with 
the llvm side of things).

However, I looked at the file and see we don't have any encodings for it, so 
nothing to be done here.


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

https://reviews.llvm.org/D68377



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


[PATCH] D68377: [Builtins] Teach Clang about memccpy

2019-10-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/Builtins.def:983
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)

aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > xbolva00 wrote:
> > > > aaron.ballman wrote:
> > > > > xbolva00 wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Isn't `memccpy` supported by Visual Studio? What should happen 
> > > > > > > there?
> > > > > > ”This POSIX function is deprecated. Use the ISO C++ conformant 
> > > > > > _memccpy instead.“
> > > > > Thanks for verifying it's still supported (deprecated != 
> > > > > unsupported). We do support other Microsoft builtins, so should this 
> > > > > one be added?
> > > > Ok, I will add it.
> > > > 
> > > > One last question, should I somehow represent that memccpy is in C 20 
> > > > (if so, how) ?
> > > You should probably start a new section for C2x library functions and add 
> > > `memccpy`, `strdup`, and `strndup` to it.
> > > 
> > > Also, just to double-check: we don't encode `restrict` into builtin 
> > > signatures, do we? (Should we?)
> > >> You should probably start a new section for C2x library functions and 
> > >> add memccpy, strdup, and strndup to it.
> > Okay, thanks - probably not needed for now (should be separate patch to 
> > handle new functions in C 2X anyway; maybe more functions to be added?)
> > 
> > >> Also, just to double-check: we don't encode restrict into builtin 
> > >> signatures, do we? (Should we?)
> > Maybe catch very simple overlap issues? I dont think this is worth it (we 
> > have sanitizers). LLVM annotates libc functions with noalias anyway.
> > Okay, thanks - probably not needed for now (should be separate patch to 
> > handle new functions in C 2X anyway; maybe more functions to be added?)
> 
> Yeah, can totally be done in a separate patch.
> 
> > Maybe catch very simple overlap issues? I dont think this is worth it (we 
> > have sanitizers). LLVM annotates libc functions with noalias anyway.
> 
> Sanitizers don't run everywhere, but more importantly, I was wondering about 
> the possible optimization opportunities by noting which arguments cannot 
> overlap with one another (perhaps noalias covers that; I'm less familiar with 
> the llvm side of things).
> 
> However, I looked at the file and see we don't have any encodings for it, so 
> nothing to be done here.
>> I was wondering about the possible optimization opportunities by noting 
>> which arguments cannot overlap with one another

This is implemented in LLVM. 

For example: https://godbolt.org/z/E8x2Ev (see line 10).

>> Sanitizers don't run everywhere

GCC emits a lot of warnings from middle-end. While I dont like some 
implementations which are part of regular pass, but extra warning pass just to 
check specific thing is nice idea I think (that pass is enabled only with 
-W flag).


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

https://reviews.llvm.org/D68377



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


[PATCH] D68429: [clang] [cmake] Use add_clang_tool() to install all tools

2019-10-04 Thread Chris Bieneman via Phabricator via cfe-commits
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

fair enough


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

https://reviews.llvm.org/D68429



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


[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

2019-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, owenpan, mitchell-stellar.
MyDeveloperDay added a project: clang-format.
Herald added a project: clang.

clang-format is incorrectly thinking the parameter parens are part of a cast 
operation, this is resulting in there sometimes being not space between the 
paren and the noexcept (and other keywords like volatile etc..)

  void operator++(int) noexcept;
  void operator++(int &) noexcept;
  void operator delete(void *, std::size_t, const std::nothrow_t &)noexcept;


Repository:
  rC Clang

https://reviews.llvm.org/D68481

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14679,6 +14679,31 @@
   */
 }
 
+TEST_F(FormatTest, NotCastRPaen) {
+
+  verifyFormat("void operator++(int) noexcept;");
+  verifyFormat("void operator++(int &) noexcept;");
+  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t 
"
+   "&) noexcept;");
+  verifyFormat(
+  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(foo &) noexcept;");
+  verifyFormat("void operator delete(foo) noexcept;");
+  verifyFormat("void operator delete(int) noexcept;");
+  verifyFormat("void operator delete(int &) noexcept;");
+  verifyFormat("void operator delete(int &) volatile noexcept;");
+  verifyFormat("void operator delete(int &) const");
+  verifyFormat("void operator delete(int &) = default");
+  verifyFormat("void operator delete(int &) = delete");
+  verifyFormat("void operator delete(int &) [[noreturn]]");
+  verifyFormat("void operator delete(int &) throw();");
+  verifyFormat("void operator delete(int &) throw(int);");
+  verifyFormat("auto operator delete(int &) -> int;");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1611,6 +1611,12 @@
 if (Tok.Next->is(tok::question))
   return false;
 
+// Functions which end with decorations like volatile, noexcept are 
unlikely
+// to be casts.
+if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const,
+  tok::kw_throw, tok::l_square, tok::arrow))
+  return false;
+
 // As Java has no function types, a "(" after the ")" likely means that 
this
 // is a cast.
 if (Style.Language == FormatStyle::LK_Java && Tok.Next->is(tok::l_paren))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14679,6 +14679,31 @@
   */
 }
 
+TEST_F(FormatTest, NotCastRPaen) {
+
+  verifyFormat("void operator++(int) noexcept;");
+  verifyFormat("void operator++(int &) noexcept;");
+  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t "
+   "&) noexcept;");
+  verifyFormat(
+  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(foo &) noexcept;");
+  verifyFormat("void operator delete(foo) noexcept;");
+  verifyFormat("void operator delete(int) noexcept;");
+  verifyFormat("void operator delete(int &) noexcept;");
+  verifyFormat("void operator delete(int &) volatile noexcept;");
+  verifyFormat("void operator delete(int &) const");
+  verifyFormat("void operator delete(int &) = default");
+  verifyFormat("void operator delete(int &) = delete");
+  verifyFormat("void operator delete(int &) [[noreturn]]");
+  verifyFormat("void operator delete(int &) throw();");
+  verifyFormat("void operator delete(int &) throw(int);");
+  verifyFormat("auto operator delete(int &) -> int;");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1611,6 +1611,12 @@
 if (Tok.Next->is(tok::question))
   return false;
 
+// Functions which end with decorations like volatile, noexcept are unlikely
+// to be casts.
+if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_

  1   2   >