date:20200807

[PATCH] D85502: [clangd] Add more error details on the remote index server side

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

kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85502

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -23,6 +23,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
@@ -303,7 +304,8 @@
   if (RelativePath.empty())
 return makeStringError("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath))
-return makeStringError("RelativePath is absolute.");
+return makeStringError(
+llvm::formatv("RelativePath {0} is absolute.", RelativePath).str());
   llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
@@ -316,10 +318,16 @@
   if (!ParsedURI)
 return ParsedURI.takeError();
   if (ParsedURI->scheme() != "file")
-return makeStringError("Can not use URI schemes other than file.");
+return makeStringError(
+llvm::formatv("Can not use URI schemes other than file, given: {0}.",
+  URI)
+.str());
   llvm::SmallString<256> Result = ParsedURI->body();
   if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, ""))
-return makeStringError("File path doesn't start with RemoteIndexRoot.");
+return makeStringError(
+llvm::formatv("File path {0} doesn't start with RemoteIndexRoot.",
+  Result.str())
+.str());
   // Make sure the result has UNIX slashes.
   return llvm::sys::path::convert_to_slash(Result,
llvm::sys::path::Style::posix);


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -23,6 +23,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
@@ -303,7 +304,8 @@
   if (RelativePath.empty())
 return makeStringError("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath))
-return makeStringError("RelativePath is absolute.");
+return makeStringError(
+llvm::formatv("RelativePath {0} is absolute.", RelativePath).str());
   llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
@@ -316,10 +318,16 @@
   if (!ParsedURI)
 return ParsedURI.takeError();
   if (ParsedURI->scheme() != "file")
-return makeStringError("Can not use URI schemes other than file.");
+return makeStringError(
+llvm::formatv("Can not use URI schemes other than file, given: {0}.",
+  URI)
+.str());
   llvm::SmallString<256> Result = ParsedURI->body();
   if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, ""))
-return makeStringError("File path doesn't start with RemoteIndexRoot.");
+return makeStringError(
+llvm::formatv("File path {0} doesn't start with RemoteIndexRoot.",
+  Result.str())
+.str());
   // Make sure the result has UNIX slashes.
   return llvm::sys::path::convert_to_slash(Result,
llvm::sys::path::Style::posix);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added a comment.

In D85417#2199830 , @ArcsinX wrote:

> In D85417#2199694 , @hokein wrote:
>
>> Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may 
>> want to fix that as well.
>
> Yes, you are right. I will fix this in the next patch.

@hokein Sorry for bother you. Seems `bugprone-not-null-terminated-result` has 
the same 3 problems as `bugprone-bad-signal-to-kill-thread`:

- `#undef __STDC_WANT_LIB_EXT1__`
- `#define __STDC_WANT_LIB_EXT1__ ((unsigned)1)`
- `Token::PtrData == nullptr` inside clangd

Do you expect separate 3 patches for this (as we did for 
`bugprone-bad-signal-to-kill-thread`) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

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

hokein added a comment.

In D85417#2201953 , @ArcsinX wrote:

> In D85417#2199830 , @ArcsinX wrote:
>
>> In D85417#2199694 , @hokein wrote:
>>
>>> Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may 
>>> want to fix that as well.
>>
>> Yes, you are right. I will fix this in the next patch.
>
> @hokein Sorry for bother you. Seems `bugprone-not-null-terminated-result` has 
> the same 3 problems as `bugprone-bad-signal-to-kill-thread`:
>
> - `#undef __STDC_WANT_LIB_EXT1__`
> - `#define __STDC_WANT_LIB_EXT1__ ((unsigned)1)`
> - `Token::PtrData == nullptr` inside clangd
>
> Do you expect separate 3 patches for this (as we did for 
> `bugprone-bad-signal-to-kill-thread`) ?

yeah, that would be nice. Thanks for digging into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D85503: [clangd] Have template template arguments target their referenced template decl

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

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

As part of this change, DynTypedNode is enhanced to allow
storing a TemplateArgumentLoc.

Fixes https://github.com/clangd/clangd/issues/473


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85503

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp

Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -23,6 +23,7 @@
 const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
   { NKI_None, "" },
   { NKI_None, "TemplateArgument" },
+  { NKI_None, "TemplateArgumentLoc" },
   { NKI_None, "TemplateName" },
   { NKI_None, "NestedNameSpecifierLoc" },
   { NKI_None, "QualType" },
@@ -129,6 +130,8 @@
  const PrintingPolicy &PP) const {
   if (const TemplateArgument *TA = get())
 TA->print(PP, OS);
+  else if (const TemplateArgumentLoc *TAL = get())
+TAL->getArgument().print(PP, OS);
   else if (const TemplateName *TN = get())
 TN->print(OS, PP);
   else if (const NestedNameSpecifier *NNS = get())
@@ -175,6 +178,8 @@
 return D->getSourceRange();
   if (const Stmt *S = get())
 return S->getSourceRange();
+  if (const TemplateArgumentLoc *TAL = get())
+return TAL->getSourceRange();
   if (const auto *C = get())
 return SourceRange(C->getBeginLoc(), C->getEndLoc());
   return SourceRange();
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -132,6 +132,7 @@
   enum NodeKindId {
 NKI_None,
 NKI_TemplateArgument,
+NKI_TemplateArgumentLoc,
 NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
@@ -191,6 +192,7 @@
   };
 KIND_TO_KIND_ID(CXXCtorInitializer)
 KIND_TO_KIND_ID(TemplateArgument)
+KIND_TO_KIND_ID(TemplateArgumentLoc)
 KIND_TO_KIND_ID(TemplateName)
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
@@ -456,12 +458,13 @@
   /// Note that we can store \c Decls, \c Stmts, \c Types,
   /// \c NestedNameSpecifiers and \c CXXCtorInitializer by pointer as they are
   /// guaranteed to be unique pointers pointing to dedicated storage in the AST.
-  /// \c QualTypes, \c NestedNameSpecifierLocs, \c TypeLocs and
-  /// \c TemplateArguments on the other hand do not have storage or unique
-  /// pointers and thus need to be stored by value.
+  /// \c QualTypes, \c NestedNameSpecifierLocs, \c TypeLocs,
+  /// \c TemplateArguments and \c TemplateArgumentLocs on the other hand do not
+  /// have storage or unique pointers and thus need to be stored by value.
   llvm::AlignedCharArrayUnion Storage;
+  TemplateArgumentLoc, NestedNameSpecifierLoc,
+  QualType, TypeLoc>
+  Storage;
 };
 
 template 
@@ -496,6 +499,10 @@
 struct DynTypedNode::BaseConverter<
 TemplateArgument, void> : public ValueConverter {};
 
+template <>
+struct DynTypedNode::BaseConverter
+: public ValueConverter {};
+
 template <>
 struct DynTypedNode::BaseConverter<
 TemplateName, void> : public ValueConverter {};
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -376,6 +376,14 @@
{"template<> class Foo", Rel::TemplateInstantiation},
{"template  class Foo", Rel::TemplatePattern});
 
+  Code = R"cpp(
+// Default argument of template template parameter.
+template struct Vector {};
+template  class Container = [[Vector]]>
+struct A {};
+  )cpp";
+  EXPECT_DECLS("TemplateArgumentLoc", {"template  struct Vector"});
+
   Flags.push_back("-std=c++17"); // for CTAD tests
 
   Code = R"cpp(
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -479,6 +479,10 @@
   bool TraverseTypeLoc(TypeLoc X) {
 return traverseNode(&X, [&] { return Base::TraverseTypeLoc(X); });
   }
+  bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &X) {
+return traverseNode(&X,
+[&] { return Base::TraverseTemplateArgumentLoc(X); });
+  }
   bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc X) {
 return traverseNode(
 &X, [&] { return Base::TraverseNestedNameSpecifierLoc(X)

[PATCH] D85500: [clangd] Highlight structured bindings at local scope as LocalVariable

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

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

Fix clang-tidy warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85500

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = 
$LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = 
$Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = 
$Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (const auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = $LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = $Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (const auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 57f9518 - [clangd] Highlight structured bindings at local scope as LocalVariable

2020-08-07 Thread Nathan Ridge via cfe-commits


Author: Nathan Ridge
Date: 2020-08-07T03:23:10-04:00
New Revision: 57f9518bf032d773c35c86ec99983a47849fd322

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

LOG: [clangd] Highlight structured bindings at local scope as LocalVariable

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 4bedea457e5c..fb1ef1e326b4 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@ llvm::Optional kindForDecl(const NamedDecl 
*D) {
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (const auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 1b90b21b2c58..f9c2e7433cc4 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -515,13 +515,14 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = 
$LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = 
$Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = 
$Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(



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

[PATCH] D85500: [clangd] Highlight structured bindings at local scope as LocalVariable

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57f9518bf032: [clangd] Highlight structured bindings at 
local scope as LocalVariable (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85500

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = 
$LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = 
$Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = 
$Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (const auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = $LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = $Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (const auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

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

hokein added a comment.

Thinking more about this,

> Inside clangd, clang-tidy checks don't see preprocessor events in the 
> preamble.
> This leads to Token::PtrData == nullptr for tokens that the macro is defined 
> to.
> E.g. #define SIGTERM 15:
>
> Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
> Token::UintData == 2
> Token::PtrData == nullptr

The token is in a pretty-broken state. Do you know why the UintData is set to 
2, I suppose this is the length of the macro definition (`15`). If the PtrData 
is nullptr, I'd expect the UintData is 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[clang] f3cc4df - Revert "[CMake] Simplify CMake handling for zlib"

2020-08-07 Thread Christian Kühnel via cfe-commits


Author: Christian Kühnel
Date: 2020-08-07T09:36:49+02:00
New Revision: f3cc4df51da4331b170065e01a6a80ee05ed0c23

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

LOG: Revert "[CMake] Simplify CMake handling for zlib"

This reverts commit 1adc494bce44f6004994deed61b30d4b71fe1d05.
This patch broke the Windows compilation on buildbot and pre-merge testing:
http://lab.llvm.org:8011/builders/mlir-windows/builds/5945
https://buildkite.com/llvm-project/llvm-master-build/builds/780

Added: 


Modified: 
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
compiler-rt/test/lit.common.configured.in
lld/test/CMakeLists.txt
lld/test/lit.site.cfg.py.in
lldb/cmake/modules/LLDBStandalone.cmake
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/cmake/config-ix.cmake
llvm/cmake/modules/LLVMConfig.cmake.in
llvm/include/llvm/Config/config.h.cmake
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/CRC.cpp
llvm/lib/Support/Compression.cpp
llvm/test/CMakeLists.txt
llvm/test/lit.site.cfg.py.in
llvm/unittests/Support/CompressionTest.cpp
llvm/utils/gn/secondary/clang/test/BUILD.gn
llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
llvm/utils/gn/secondary/lld/test/BUILD.gn
llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
llvm/utils/gn/secondary/llvm/test/BUILD.gn

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 334a90498d0d..38bbc5be90d5 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,6 +9,15 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+if(CLANG_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
@@ -16,7 +25,7 @@ llvm_canonicalize_cmake_booleans(
   CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 286ea06d798c..d9b5b2f2592e 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 000bf9b98470..1f746c067b84 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -57,7 +57,7 @@ elif config.android:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
+set_default("have_zlib", "@HAVE_LIBZ@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index 52e6118ba876..74b29f5d65b8 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,8 +4,17 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
+if(LLD_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_LIBXML2_ENABLED
   )
 

diff  --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index 3d4c51f4ab64..4aa2fcda73bb 100644
--- a/lld/test/lit.site.cfg.py.in
+++ b/lld/test/lit.site.cfg.py.in
@@ -14,7 +14,7 @@ config.lld_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
 config.lld_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ

[PATCH] D85503: [clangd] Have template template arguments target their referenced template decl

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

hokein added a comment.

The code looks good.

I think we'd better to have some sort of tests in clang (rather than relying on 
clangd test). It seems that the only way to test it in clang is through 
ASTMatcher. Do you mind adding a `templateArgumentLoc` matcher as well?  and 
add a test in `unittests/ASTMatchers/ASTMatchersTraversalTest.cpp`.
With that, I think it would be nice to have two patches, one for clang, the 
other is for clangd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85503

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

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added a comment.

In D85417#2201973 , @hokein wrote:

> Thinking more about this,
>
>> Inside clangd, clang-tidy checks don't see preprocessor events in the 
>> preamble.
>> This leads to Token::PtrData == nullptr for tokens that the macro is defined 
>> to.
>> E.g. #define SIGTERM 15:
>>
>> Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
>> Token::UintData == 2
>> Token::PtrData == nullptr
>
> The token is in a pretty-broken state. Do you know why the UintData is set to 
> 2, I suppose this is the length of the macro definition (`15`). If the 
> PtrData is nullptr, I'd expect the UintData is 0.

I think it's here:

  Token ASTReader::ReadToken(ModuleFile &F, const RecordDataImpl &Record,
 unsigned &Idx) {
Token Tok;
Tok.startToken();
Tok.setLocation(ReadSourceLocation(F, Record, Idx));
Tok.setLength(Record[Idx++]);
if (IdentifierInfo *II = getLocalIdentifier(F, Record[Idx++]))
  Tok.setIdentifierInfo(II);
Tok.setKind((tok::TokenKind)Record[Idx++]);
Tok.setFlag((Token::TokenFlags)Record[Idx++]);
return Tok;
  }

So, we set `Token::UintData` via `Token::setLength()` at preamble read, but do 
not set `Token::PtrData` without preprocessor events.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D85507: [VE] Remove obsoleted getVEAsmModeForCPU function

2020-08-07 Thread Kazushi Marukawa via Phabricator via cfe-commits

kaz7 created this revision.
kaz7 added reviewers: simoll, k-ishizaka.
kaz7 added projects: clang, VE.
Herald added a subscriber: cfe-commits.
kaz7 requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85507

Files:
  clang/lib/Driver/ToolChains/Arch/VE.cpp
  clang/lib/Driver/ToolChains/Arch/VE.h


Index: clang/lib/Driver/ToolChains/Arch/VE.h
===
--- clang/lib/Driver/ToolChains/Arch/VE.h
+++ clang/lib/Driver/ToolChains/Arch/VE.h
@@ -22,8 +22,6 @@
 
 void getVETargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
  std::vector &Features);
-const char *getVEAsmModeForCPU(llvm::StringRef Name,
-   const llvm::Triple &Triple);
 
 } // end namespace ve
 } // namespace tools
Index: clang/lib/Driver/ToolChains/Arch/VE.cpp
===
--- clang/lib/Driver/ToolChains/Arch/VE.cpp
+++ clang/lib/Driver/ToolChains/Arch/VE.cpp
@@ -18,9 +18,5 @@
 using namespace clang;
 using namespace llvm::opt;
 
-const char *ve::getVEAsmModeForCPU(StringRef Name, const llvm::Triple &Triple) 
{
-  return "";
-}
-
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
  std::vector &Features) {}


Index: clang/lib/Driver/ToolChains/Arch/VE.h
===
--- clang/lib/Driver/ToolChains/Arch/VE.h
+++ clang/lib/Driver/ToolChains/Arch/VE.h
@@ -22,8 +22,6 @@
 
 void getVETargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
  std::vector &Features);
-const char *getVEAsmModeForCPU(llvm::StringRef Name,
-   const llvm::Triple &Triple);
 
 } // end namespace ve
 } // namespace tools
Index: clang/lib/Driver/ToolChains/Arch/VE.cpp
===
--- clang/lib/Driver/ToolChains/Arch/VE.cpp
+++ clang/lib/Driver/ToolChains/Arch/VE.cpp
@@ -18,9 +18,5 @@
 using namespace clang;
 using namespace llvm::opt;
 
-const char *ve::getVEAsmModeForCPU(StringRef Name, const llvm::Triple &Triple) {
-  return "";
-}
-
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
  std::vector &Features) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85507: [VE] Remove obsoleted getVEAsmModeForCPU function

2020-08-07 Thread Simon Moll via Phabricator via cfe-commits

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

Feel free to commit something like this without review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85507

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

[PATCH] D85367: [clang, test, Darwin] Fix tests expecting Darwin target

2020-08-07 Thread Thomas Preud'homme via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6dc863cd858f: [clang, test, Darwin] Fix tests expecting 
Darwin target (authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85367

Files:
  clang/test/Driver/apple-arm64-arch.c
  clang/test/Driver/darwin-warning-options.c


Index: clang/test/Driver/darwin-warning-options.c
===
--- clang/test/Driver/darwin-warning-options.c
+++ clang/test/Driver/darwin-warning-options.c
@@ -1,4 +1,4 @@
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 
 // Always error about undefined 'TARGET_OS_*' macros on Darwin.
 // RUN: %clang -### %s 2>&1 | FileCheck %s
Index: clang/test/Driver/apple-arm64-arch.c
===
--- clang/test/Driver/apple-arm64-arch.c
+++ clang/test/Driver/apple-arm64-arch.c
@@ -1,7 +1,7 @@
 // RUN: env SDKROOT="/" %clang -arch arm64 -c -### %s 2>&1 | \
 // RUN:   FileCheck %s
 //
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 // XFAIL: apple-silicon-mac
 //
 // CHECK: "-triple" "arm64-apple-ios{{[0-9.]+}}"


Index: clang/test/Driver/darwin-warning-options.c
===
--- clang/test/Driver/darwin-warning-options.c
+++ clang/test/Driver/darwin-warning-options.c
@@ -1,4 +1,4 @@
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 
 // Always error about undefined 'TARGET_OS_*' macros on Darwin.
 // RUN: %clang -### %s 2>&1 | FileCheck %s
Index: clang/test/Driver/apple-arm64-arch.c
===
--- clang/test/Driver/apple-arm64-arch.c
+++ clang/test/Driver/apple-arm64-arch.c
@@ -1,7 +1,7 @@
 // RUN: env SDKROOT="/" %clang -arch arm64 -c -### %s 2>&1 | \
 // RUN:   FileCheck %s
 //
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 // XFAIL: apple-silicon-mac
 //
 // CHECK: "-triple" "arm64-apple-ios{{[0-9.]+}}"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6dc863c - [clang, test, Darwin] Fix tests expecting Darwin target

2020-08-07 Thread Thomas Preud'homme via cfe-commits


Author: Thomas Preud'homme
Date: 2020-08-07T09:21:31+01:00
New Revision: 6dc863cd858f32ef0060c46edda2ab56a3cf76b4

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

LOG: [clang, test, Darwin] Fix tests expecting Darwin target

Clang tests Driver/apple-arm64-arch.c and
Driver/darwin-warning-options.c test Darwin driver functionality but
only require the host system to be Darwin. This leads the tests to fail
when building a cross-compiler on Darwin and to be marked unsupported
when cross-compiling to Darwin from another system. This commit changes
the requirements for those tests to require the target to be Darwin.

Reviewed By: steven_wu

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

Added: 


Modified: 
clang/test/Driver/apple-arm64-arch.c
clang/test/Driver/darwin-warning-options.c

Removed: 




diff  --git a/clang/test/Driver/apple-arm64-arch.c 
b/clang/test/Driver/apple-arm64-arch.c
index a37346b1a9bb..2c771ee40368 100644
--- a/clang/test/Driver/apple-arm64-arch.c
+++ b/clang/test/Driver/apple-arm64-arch.c
@@ -1,7 +1,7 @@
 // RUN: env SDKROOT="/" %clang -arch arm64 -c -### %s 2>&1 | \
 // RUN:   FileCheck %s
 //
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 // XFAIL: apple-silicon-mac
 //
 // CHECK: "-triple" "arm64-apple-ios{{[0-9.]+}}"

diff  --git a/clang/test/Driver/darwin-warning-options.c 
b/clang/test/Driver/darwin-warning-options.c
index b0a591eac820..a3301b3fae9a 100644
--- a/clang/test/Driver/darwin-warning-options.c
+++ b/clang/test/Driver/darwin-warning-options.c
@@ -1,4 +1,4 @@
-// REQUIRES: system-darwin
+// REQUIRES: darwin
 
 // Always error about undefined 'TARGET_OS_*' macros on Darwin.
 // RUN: %clang -### %s 2>&1 | FileCheck %s



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

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

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

fhahn updated this revision to Diff 283837.
fhahn added a comment.

Thanks for taking a look!

I adjusted the LangOpts description and added the flag to f_Group, as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

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


Index: clang/test/CodeGen/pass-by-value-noalias.c
===
--- /dev/null
+++ clang/test/CodeGen/pass-by-value-noalias.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos 
-emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck 
--check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm 
-disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3419,6 +3419,7 @@
   Opts.PCHInstantiateTemplates = Args.hasArg(OPT_fpch_instantiate_templates);
 
   Opts.MatrixTypes = Args.hasArg(OPT_fenable_matrix);
+  Opts.PassByValueIsNoAlias = Args.hasArg(OPT_fpass_by_value_is_noalias);
 
   Opts.MaxTokens = getLastArgIntValue(Args, OPT_fmax_tokens_EQ, 0, Diags);
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2192,6 +2192,11 @@
   if (AI.getIndirectByVal())
 Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
 
+  if (getLangOpts().PassByValueIsNoAlias)
+// When calling the function, the pointer passed in will be the only
+// reference to the underlying object. Mark it accordingly.
+Attrs.addAttribute(llvm::Attribute::NoAlias);
+
   // TODO: We could add the byref attribute if not byval, but it would
   // require updating many testcases.
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4289,6 +4289,9 @@
 def fcompatibility_qualified_id_block_param_type_checking : Flag<["-"], 
"fcompatibility-qualified-id-block-type-checking">,
   HelpText<"Allow using blocks with parameters of more specific type than "
"the type system guarantees when a parameter is qualified id">;
+def fpass_by_value_is_noalias: Flag<["-"], "fpass-by-value-is-noalias">,
+  HelpText<"Allows assuming by-value parameters do not alias any other value."
+   "Note that this may not hold for C++.">, Group;
 
 // FIXME: Remove these entirely once functionality/tests have been excised.
 def fobjc_gc_only : Flag<["-"], "fobjc-gc-only">, Group,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -368,6 +368,8 @@
 LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors")
 
 LANGOPT(MatrixTypes, 1, 0, "Enable or disable the builtin matrix type")
+LANGOPT(PassByValueIsNoAlias, 1, 0, "assumption that by-value parameters do "
+"not alias any other values")
 
 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0")
 


Index: clang/test/CodeGen/pass-by-value-noalias.c
===
--- /dev/null
+++ clang/test/CodeGen/pass-by-value-noalias.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fpass-by-value-is-noalias -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+// A struct large enough so it is not passed in registers on ARM64.
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3419,6 +3419,7

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

2020-08-07 Thread Gábor Horváth via Phabricator via cfe-commits

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

LGTM, thanks!

I think all the problems that left should be solved in a separate patch or even 
out of the scope for the GSoC.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:33
 
+// Static global pointer to NullDereferenceBugType.
+static const BugType *NullDereferenceBugTypePtr;

I find this comment redundant as well. It just repeats what is already evident 
from the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

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

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:30
+  // markers which are used by some LLVM analysis (e.g. AliasAnalysis).
+  CGO.OptimizationLevel = 2; // -O2
+

TODO overwrite ALL optimization related config.

Artem:
> we should not be taking -O flags into account at all, but pick some default 
> -O2 regardless of flags; and ideally all flags should be ignored by default, 
> to ensure experience as consistent as possible.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

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

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-07 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:30
+  // markers which are used by some LLVM analysis (e.g. AliasAnalysis).
+  CGO.OptimizationLevel = 2; // -O2
+

martong wrote:
> TODO overwrite ALL optimization related config.
> 
> Artem:
> > we should not be taking -O flags into account at all, but pick some default 
> > -O2 regardless of flags; and ideally all flags should be ignored by 
> > default, to ensure experience as consistent as possible.
> 
> 
Additional bit of info from the mailing list: relying on the set of 
optimizations from O2 might not suffice as it might contain passes with bad 
side effects. One example is removing a static function that was inlined to all 
of the call sites which would make us unable to query the analysis results for 
that function. Overall, we might not want to make the analysis dependent on 
inlining heuristics. (Or we might not care. This is up for discussion.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

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

[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

2020-08-07 Thread Sam McCall via Phabricator via cfe-commits

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

Thanks!




Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:997
+)cpp",
+R"cpp(
+  class Foo {};

this is basically the same as the above case (which is fine)
but what about `class [[Foo]]; using Bar = ^Foo;`?

(maybe this is tested elsewhere already, but it should be moved here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74054

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

[PATCH] D85427: [SyntaxTree][NFC] remove redundant namespace-specifiers

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba41a0f7339c: [SyntaxTree][NFC] remove redundant 
namespace-specifiers (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85427

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -40,7 +40,7 @@
 using namespace clang;
 
 namespace {
-static llvm::ArrayRef tokens(syntax::Node *N) {
+static ArrayRef tokens(syntax::Node *N) {
   assert(N->isOriginal() && "tokens of modified nodes are not well-defined");
   if (auto *L = dyn_cast(N))
 return llvm::makeArrayRef(L->token(), 1);
@@ -53,7 +53,7 @@
public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *buildTree(llvm::StringRef Code,
+  syntax::TranslationUnit *buildTree(StringRef Code,
  const TestClangConfig &ClangConfig) {
 // FIXME: this code is almost the identical to the one in TokensTest. Share
 //it.
@@ -169,7 +169,7 @@
   }
 
   // Adds a file to the test VFS.
-  void addFile(llvm::StringRef Path, llvm::StringRef Contents) {
+  void addFile(StringRef Path, StringRef Contents) {
 if (!FS->addFile(Path, time_t(),
  llvm::MemoryBuffer::getMemBufferCopy(Contents))) {
   ADD_FAILURE() << "could not add a file to VFS: " << Path;
@@ -179,7 +179,7 @@
   /// Finds the deepest node in the tree that covers exactly \p R.
   /// FIXME: implement this efficiently and move to public syntax tree API.
   syntax::Node *nodeByRange(llvm::Annotations::Range R, syntax::Node *Root) {
-llvm::ArrayRef Toks = tokens(Root);
+ArrayRef Toks = tokens(Root);
 
 if (Toks.front().location().isFileID() &&
 Toks.back().location().isFileID() &&
@@ -198,15 +198,14 @@
   }
 
   // Data fields.
-  llvm::IntrusiveRefCntPtr DiagOpts =
-  new DiagnosticOptions();
-  llvm::IntrusiveRefCntPtr Diags =
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr Diags =
   new DiagnosticsEngine(new DiagnosticIDs, DiagOpts.get());
   IntrusiveRefCntPtr FS =
   new llvm::vfs::InMemoryFileSystem;
-  llvm::IntrusiveRefCntPtr FileMgr =
+  IntrusiveRefCntPtr FileMgr =
   new FileManager(FileSystemOptions(), FS);
-  llvm::IntrusiveRefCntPtr SourceMgr =
+  IntrusiveRefCntPtr SourceMgr =
   new SourceManager(*Diags, *FileMgr);
   std::shared_ptr Invocation;
   // Set after calling buildTree().
Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -36,11 +36,9 @@
  const TokenBuffer &Tokens)
 : SourceMgr(SourceMgr), LangOpts(LangOpts), Tokens(Tokens) {}
 
-const clang::syntax::TokenBuffer &syntax::Arena::tokenBuffer() const {
-  return Tokens;
-}
+const syntax::TokenBuffer &syntax::Arena::tokenBuffer() const { return Tokens; }
 
-std::pair>
+std::pair>
 syntax::Arena::lexBuffer(std::unique_ptr Input) {
   auto FID = SourceMgr.createFileID(std::move(Input));
   auto It = ExtraTokens.try_emplace(FID, tokenize(FID, SourceMgr, LangOpts));
@@ -135,7 +133,7 @@
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef Tokens,
+static void dumpTokens(raw_ostream &OS, ArrayRef Tokens,
const SourceManager &SM) {
   assert(!Tokens.empty());
   bool First = true;
@@ -153,7 +151,7 @@
   }
 }
 
-static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
+static void dumpTree(raw_ostream &OS, const syntax::Node *N,
  const syntax::Arena &A, std::vector IndentMask) {
   std::string Marks;
   if (!N->isOriginal())
@@ -165,13 +163,13 @@
   if (!Marks.empty())
 OS << Marks << ": ";
 
-  if (auto *L = llvm::dyn_cast(N)) {
+  if (auto *L = dyn_cast(N)) {
 dumpTokens(OS, *L->token(), A.sourceManager());
 OS << "\n";
 return;
   }
 
-  auto *T = llvm::cast(N);
+  auto *T = cast(N);
   OS << T->kind() << "\n";
 
   for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
@@ -205,7 +203,7 @@
   std::string Storage;
   llvm::raw_string_ostream OS(Storage);
   traverse(this, [&](const syntax::Node *N) {
-auto *L = llvm::dyn_cast(N);
+auto *L = dyn_cast(N);
 if (!L)
   return;
 ::dumpTokens(OS, *L->token(), A.sourceManager());
Index: clang/lib/Tooling/Syntax/Nodes.cpp
==

[clang] ba41a0f - [SyntaxTree][NFC] remove redundant namespace-specifiers

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


Author: Eduardo Caldas
Date: 2020-08-07T08:45:29Z
New Revision: ba41a0f7339c8cbd065f032cc5f8c1d87a74e124

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

LOG: [SyntaxTree][NFC] remove redundant namespace-specifiers

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Nodes.h
clang/include/clang/Tooling/Syntax/Tree.h
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/lib/Tooling/Syntax/Nodes.cpp
clang/lib/Tooling/Syntax/Tree.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h 
b/clang/include/clang/Tooling/Syntax/Nodes.h
index 8a873f9d5273..a5972a394583 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -100,7 +100,7 @@ enum class NodeKind : uint16_t {
   UnqualifiedId
 };
 /// For debugging purposes.
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeKind K);
+raw_ostream &operator<<(raw_ostream &OS, NodeKind K);
 
 /// A relation between a parent and child node, e.g. 'left-hand-side of
 /// a binary expression'. Used for implementing accessors.
@@ -170,7 +170,7 @@ enum class NodeRole : uint8_t {
   ParenExpression_subExpression
 };
 /// For debugging purposes.
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeRole R);
+raw_ostream &operator<<(raw_ostream &OS, NodeRole R);
 
 class SimpleDeclarator;
 
@@ -212,7 +212,7 @@ class NestedNameSpecifier final : public Tree {
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
-  std::vector specifiers();
+  std::vector specifiers();
 };
 
 /// Models an `unqualified-id`. C++ [expr.prim.id.unqual]
@@ -238,10 +238,10 @@ class IdExpression final : public Expression {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::IdExpression;
   }
-  syntax::NestedNameSpecifier *qualifier();
+  NestedNameSpecifier *qualifier();
   // TODO after expose `id-expression` from `DependentScopeDeclRefExpr`:
   // Add accessor for `template_opt`.
-  syntax::UnqualifiedId *unqualifiedId();
+  UnqualifiedId *unqualifiedId();
 };
 
 /// An expression of an unknown kind, i.e. one not currently handled by the
@@ -262,9 +262,9 @@ class ParenExpression final : public Expression {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::ParenExpression;
   }
-  syntax::Leaf *openParen();
-  syntax::Expression *subExpression();
-  syntax::Leaf *closeParen();
+  Leaf *openParen();
+  Expression *subExpression();
+  Leaf *closeParen();
 };
 
 /// Expression for literals. C++ [lex.literal]
@@ -283,7 +283,7 @@ class LiteralExpression : public Expression {
N->kind() == NodeKind::CharUserDefinedLiteralExpression ||
N->kind() == NodeKind::StringUserDefinedLiteralExpression;
   }
-  syntax::Leaf *literalToken();
+  Leaf *literalToken();
 };
 
 /// Expression for integer literals. C++ [lex.icon]
@@ -418,8 +418,8 @@ class UnaryOperatorExpression : public Expression {
 return N->kind() == NodeKind::PrefixUnaryOperatorExpression ||
N->kind() == NodeKind::PostfixUnaryOperatorExpression;
   }
-  syntax::Leaf *operatorToken();
-  syntax::Expression *operand();
+  Leaf *operatorToken();
+  Expression *operand();
 };
 
 ///  
@@ -467,9 +467,9 @@ class BinaryOperatorExpression final : public Expression {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::BinaryOperatorExpression;
   }
-  syntax::Expression *lhs();
-  syntax::Leaf *operatorToken();
-  syntax::Expression *rhs();
+  Expression *lhs();
+  Leaf *operatorToken();
+  Expression *rhs();
 };
 
 /// An abstract node for C++ statements, e.g. 'while', 'if', etc.
@@ -518,8 +518,8 @@ class SwitchStatement final : public Statement {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::SwitchStatement;
   }
-  syntax::Leaf *switchKeyword();
-  syntax::Statement *body();
+  Leaf *switchKeyword();
+  Statement *body();
 };
 
 /// case : 
@@ -529,9 +529,9 @@ class CaseStatement final : public Statement {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::CaseStatement;
   }
-  syntax::Leaf *caseKeyword();
-  syntax::Expression *value();
-  syntax::Statement *body();
+  Leaf *caseKeyword();
+  Expression *value();
+  Statement *body();
 };
 
 /// default: 
@@ -541,8 +541,8 @@ class DefaultStatement final : public Statement {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::DefaultStatement;
   }
-  syntax::Leaf *defaultKeyword();
-  syntax::Statement *body();
+  Leaf *defaultKeyword();
+  Statement *body();
 };
 
 /// if (cond)  else 
@@ -553,10 +553,10 @@ class IfStatement final : public Statement {

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

2020-08-07 Thread David Chisnall via Phabricator via cfe-commits

theraven added a comment.

This feature looks generally useful.  A few small suggestions:

- This is really a way of transforming a formal protocol into an informal 
protocol.  Objective-C has had a convention of informal protocols since the 
'80s, but they're implemented as categories on the root class with no 
`@implementation`.  I'd suggest that `__attribute__((objc_informal_protocol))` 
or similar might be a better spelling for this, explicitly bringing the 
informal notion into the language.  A lot of the informal protocols in Cocoa 
could be better expressed using this and `@optional` methods than as categories 
on `NSObject`.
- Given that this doesn't depend on any features in the runtime (from the 
runtime's perspective, the protocol doesn't exist), I don't think it makes 
sense to have an `ObjCRuntime` method to query whether this is supported by the 
runtime.  We should enable it everywhere if it's going in anywhere.
- The changes required in CGObjcCGNU.cpp are fairly small and I agree that 
@rjmccall's proposal  for a callback-driven visitor would simplify the changes 
in both runtimes.
- The semantics are slightly confusing with the deep approach though.  
Normally, if you iterate over the protocols that a class conforms to, you only 
see the ones that it directly conforms to.  With this model, you'd see indirect 
ones.  We might want to set some metadata to allow programmers to differentiate 
the two, or we might want to have a warning (off by default?) if an informal 
protocol conforms to a formal one, or simply disallow it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D85282: [Concepts] Dump template arguments for immediately declared constraint.

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

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

address review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85282

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-concepts.cpp


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -15,8 +15,12 @@
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}}  'bool' 
Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}}  'bool' 
Concept {{.*}} 'binary_concept'
+  // CHECK-NEXT:   |-TemplateArgument {{.*}} type 'R'
+  // CHECK-NEXT:   | `-TemplateTypeParmType {{.*}} 'R'
+  // CHECK-NEXT:   |   `-TemplateTypeParm {{.*}} 'R'
+  // CHECK-NEXT:   `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-BuiltinType {{.*}} 'int'
   template  R>
   Foo(R);
 
@@ -25,11 +29,11 @@
   template 
   Foo(R);
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} 
Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, int) requires unary_concept;
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} 
Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, char) requires unary_concept {
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -2000,7 +2000,6 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -543,9 +543,7 @@
 
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
 if (const auto *TC = D->getTypeConstraint())
-  if (TC->hasExplicitTemplateArgs())
-for (const auto &ArgLoc : TC->getTemplateArgsAsWritten()->arguments())
-  dumpTemplateArgumentLoc(ArgLoc);
+  Visit(TC->getImmediatelyDeclaredConstraint());
 if (D->hasDefaultArgument())
   Visit(D->getDefaultArgument(), SourceRange(),
 D->getDefaultArgStorage().getInheritedFrom(),
@@ -574,6 +572,12 @@
 Visit(D->getConstraintExpr());
   }
 
+  void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *CSE) {
+if (CSE->hasExplicitTemplateArgs())
+  for (const auto &ArgLoc : CSE->getTemplateArgsAsWritten()->arguments())
+dumpTemplateArgumentLoc(ArgLoc);
+  }
+
   void VisitUsingShadowDecl(const UsingShadowDecl *D) {
 if (auto *TD = dyn_cast(D->getUnderlyingDecl()))
   Visit(TD->getTypeForDecl());


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -15,8 +15,12 @@
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}}  'bool' Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}}  'bool' Concept {{.*}} 'binary_concept'
+  // CHECK-NEXT:   |-TemplateArgument {{.*}} type 'R'
+  // CHECK-NEXT:   | `-TemplateTypeParmType {{.*}} 'R'
+  // CHECK-NEXT:   |   `-TemplateTypeParm {{.*}} 'R'
+  // CHECK-NEXT:   `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-BuiltinType {{.*}} 'int'
   template  R>
   Foo(R);
 
@@ -25,11 +29,11 @@
   template 
   Foo(R);
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, int) requires unary_concept;
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, char) requires unary_concept {
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -2000,7 +2000,6 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else
Index: clang/include/clang/AST/ASTNodeTraverser.h
==

[PATCH] D85485: Fix quiet mode in git-clang-format

2020-08-07 Thread serge via Phabricator via cfe-commits

serge-sans-paille accepted this revision.
serge-sans-paille added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/tools/clang-format/git-clang-format:151
   if not changed_lines:
-print('no modified files to format')
+if opts.verbose >=0:
+  print('no modified files to format')

nit: PEP8 recommends `opts.verbose >= 0`. LGTM otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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

[PATCH] D85282: [Concepts] Dump template arguments for immediately declared constraint.

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

hokein added a comment.

In D85282#2198737 , @nridge wrote:

> This patch has helped me understand the difference between the two 
> `ConceptReference`s in the AST (the `TypeConstraint` and the 
> `ConceptSpecializationExpr`).
>
> Given that
>
> `template  R>`
>
> is a shorthand for
>
> `template  requires binary_concept`
>
> the `TypeConstraint` is the explicitly written `binary_concept` whose 
> `getTemplateArgsAsWritten()` has one element: ``.
>
> Meanwhile the `ConceptSpecializationExpr` is the implicit `binary_concept int>` whose `getTemplateArgsAsWritten()` has two elements: ``.

Yes, true.

> Is there a reason to prefer dumping one over the other (or even both)?

This is a good question. Currently we don't dump `TypeConstrain` instead we 
dump the responding immediately declared constraint (which is a 
`ConceptSpecializationExpr`).

I think it is ok to just dump the immediately declared constraint as 
TypeConstrain is just a wrapper of it.

  template  R>
  
  before this patch:
  TemplateTypeParmDecl R
|- ConceptSpecializationExpr
`- TemplateArgument {{.*}} type 'int'
  
  after this patch:
  TemplateTypeParmDecl R
|- ConceptSpecializationExpr
   |- TemplateArgument {{.*}} type 'R'
   `- TemplateArgument {{.*}} type 'int'
  
  maybe the following is better:
  TemplateTypeParmDecl R
|- TypeContraint
   `- ConceptSpecializationExpr
   |- TemplateArgument {{.*}} type 'R'
   `- TemplateArgument {{.*}} type 'int'

The behavior (before this patch) is to dump the written template arguments in 
`TypeConstraint` as part of the `TemplateTypeParmDecl`, which doesn't seem to 
reasonable.
It also confuses the default template argument case.

  template
  
  // is dump as 
  TemplateTypeParmDecl  T
 `-TemplateArgument type 'int'






Comment at: clang/test/AST/ast-dump-concepts.cpp:18
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}}  'bool' 
Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'

nridge wrote:
> The ending column number here was important (it tested the fix to 
> `ConceptSpecializationExpr::getEndLoc()` in D86413).
oh, I lost it during the rebase, added it back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85282

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

[clang] 6717476 - [AST][RecoveryExpr] Fix the missing type when rebuilding RecoveryExpr in TreeTransform.

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


Author: Haojian Wu
Date: 2020-08-07T11:15:19+02:00
New Revision: 67174765db527ed1911f33109a34026f3913601d

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

LOG: [AST][RecoveryExpr] Fix the missing type when rebuilding RecoveryExpr in 
TreeTransform.

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

Added: 
clang/test/SemaTemplate/recovery-tree-transform-crash.cpp

Modified: 
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 29d263de6e88..d23b441cad1b 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3618,8 +3618,8 @@ class TreeTransform {
   }
 
   ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation 
EndLoc,
- ArrayRef SubExprs) {
-return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+ ArrayRef SubExprs, QualType Type) {
+return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs, Type);
   }
 
 private:
@@ -10209,7 +10209,7 @@ ExprResult 
TreeTransform::TransformRecoveryExpr(RecoveryExpr *E) {
   if (!getDerived().AlwaysRebuild() && !Changed)
 return E;
   return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
-  Children);
+  Children, E->getType());
 }
 
 template

diff  --git a/clang/test/SemaTemplate/recovery-tree-transform-crash.cpp 
b/clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
new file mode 100644
index ..9776aeb9b331
--- /dev/null
+++ b/clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -frecovery-ast -frecovery-ast-type %s
+
+template  struct Ptr { T *operator->() const; };
+
+struct ABC {
+  void run();
+};
+
+Ptr call(int); // expected-note {{candidate function not viable}}
+
+void test() {
+  call()->run(undef); // expected-error {{no matching function for call to 
'call'}} \
+ expected-error {{use of undeclared identifier}}
+}



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

[PATCH] D85423: [AST][RecoveryExpr] Fix the missing type when rebuilding RecoveryExpr in TreeTransform.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67174765db52: [AST][RecoveryExpr] Fix the missing type when 
rebuilding RecoveryExpr in… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85423

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/recovery-tree-transform-crash.cpp


Index: clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -frecovery-ast -frecovery-ast-type %s
+
+template  struct Ptr { T *operator->() const; };
+
+struct ABC {
+  void run();
+};
+
+Ptr call(int); // expected-note {{candidate function not viable}}
+
+void test() {
+  call()->run(undef); // expected-error {{no matching function for call to 
'call'}} \
+ expected-error {{use of undeclared identifier}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3618,8 +3618,8 @@
   }
 
   ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation 
EndLoc,
- ArrayRef SubExprs) {
-return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+ ArrayRef SubExprs, QualType Type) {
+return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs, Type);
   }
 
 private:
@@ -10209,7 +10209,7 @@
   if (!getDerived().AlwaysRebuild() && !Changed)
 return E;
   return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
-  Children);
+  Children, E->getType());
 }
 
 template


Index: clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -frecovery-ast -frecovery-ast-type %s
+
+template  struct Ptr { T *operator->() const; };
+
+struct ABC {
+  void run();
+};
+
+Ptr call(int); // expected-note {{candidate function not viable}}
+
+void test() {
+  call()->run(undef); // expected-error {{no matching function for call to 'call'}} \
+ expected-error {{use of undeclared identifier}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3618,8 +3618,8 @@
   }
 
   ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation EndLoc,
- ArrayRef SubExprs) {
-return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+ ArrayRef SubExprs, QualType Type) {
+return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs, Type);
   }
 
 private:
@@ -10209,7 +10209,7 @@
   if (!getDerived().AlwaysRebuild() && !Changed)
 return E;
   return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
-  Children);
+  Children, E->getType());
 }
 
 template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85507: [VE] Remove obsoleted getVEAsmModeForCPU function

2020-08-07 Thread Kazushi Marukawa via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ac1eb6358b1: [VE] Remove obsoleted getVEAsmModeForCPU 
function (authored by kaz7).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85507

Files:
  clang/lib/Driver/ToolChains/Arch/VE.cpp
  clang/lib/Driver/ToolChains/Arch/VE.h


Index: clang/lib/Driver/ToolChains/Arch/VE.h
===
--- clang/lib/Driver/ToolChains/Arch/VE.h
+++ clang/lib/Driver/ToolChains/Arch/VE.h
@@ -22,8 +22,6 @@
 
 void getVETargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
  std::vector &Features);
-const char *getVEAsmModeForCPU(llvm::StringRef Name,
-   const llvm::Triple &Triple);
 
 } // end namespace ve
 } // namespace tools
Index: clang/lib/Driver/ToolChains/Arch/VE.cpp
===
--- clang/lib/Driver/ToolChains/Arch/VE.cpp
+++ clang/lib/Driver/ToolChains/Arch/VE.cpp
@@ -18,9 +18,5 @@
 using namespace clang;
 using namespace llvm::opt;
 
-const char *ve::getVEAsmModeForCPU(StringRef Name, const llvm::Triple &Triple) 
{
-  return "";
-}
-
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
  std::vector &Features) {}


Index: clang/lib/Driver/ToolChains/Arch/VE.h
===
--- clang/lib/Driver/ToolChains/Arch/VE.h
+++ clang/lib/Driver/ToolChains/Arch/VE.h
@@ -22,8 +22,6 @@
 
 void getVETargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
  std::vector &Features);
-const char *getVEAsmModeForCPU(llvm::StringRef Name,
-   const llvm::Triple &Triple);
 
 } // end namespace ve
 } // namespace tools
Index: clang/lib/Driver/ToolChains/Arch/VE.cpp
===
--- clang/lib/Driver/ToolChains/Arch/VE.cpp
+++ clang/lib/Driver/ToolChains/Arch/VE.cpp
@@ -18,9 +18,5 @@
 using namespace clang;
 using namespace llvm::opt;
 
-const char *ve::getVEAsmModeForCPU(StringRef Name, const llvm::Triple &Triple) {
-  return "";
-}
-
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
  std::vector &Features) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3ac1eb6 - [VE] Remove obsoleted getVEAsmModeForCPU function

2020-08-07 Thread Kazushi Marukawa via cfe-commits


Author: Kazushi (Jam) Marukawa
Date: 2020-08-07T18:20:34+09:00
New Revision: 3ac1eb6358b172f54788def4125f80ddd9170d8e

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

LOG: [VE] Remove obsoleted getVEAsmModeForCPU function

Reviewed By: simoll

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/VE.cpp
clang/lib/Driver/ToolChains/Arch/VE.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/VE.cpp 
b/clang/lib/Driver/ToolChains/Arch/VE.cpp
index fa10e4810f1c..9dfd37c2106d 100644
--- a/clang/lib/Driver/ToolChains/Arch/VE.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/VE.cpp
@@ -18,9 +18,5 @@ using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
-const char *ve::getVEAsmModeForCPU(StringRef Name, const llvm::Triple &Triple) 
{
-  return "";
-}
-
 void ve::getVETargetFeatures(const Driver &D, const ArgList &Args,
  std::vector &Features) {}

diff  --git a/clang/lib/Driver/ToolChains/Arch/VE.h 
b/clang/lib/Driver/ToolChains/Arch/VE.h
index 713e3e7d042f..531433534914 100644
--- a/clang/lib/Driver/ToolChains/Arch/VE.h
+++ b/clang/lib/Driver/ToolChains/Arch/VE.h
@@ -22,8 +22,6 @@ namespace ve {
 
 void getVETargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
  std::vector &Features);
-const char *getVEAsmModeForCPU(llvm::StringRef Name,
-   const llvm::Triple &Triple);
 
 } // end namespace ve
 } // namespace tools



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

[clang-tools-extra] 01bc708 - [NFC] Replace hasName in loop for hasAnyName

2020-08-07 Thread Nathan James via cfe-commits


Author: Nathan James
Date: 2020-08-07T10:22:45+01:00
New Revision: 01bc708126c55c94495d0b55cb2003f67b9b24bf

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

LOG: [NFC] Replace hasName in loop for hasAnyName

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
index ecdef6a1bc68..04dc61f02df1 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
@@ -119,16 +119,11 @@ AST_MATCHER_P(QualType, isSugarFor, Matcher, 
SugarMatcher) {
 /// \endcode
 ///
 /// namedDecl(hasStdIteratorName()) matches \c I and \c CI.
-AST_MATCHER(NamedDecl, hasStdIteratorName) {
-  static const char *const IteratorNames[] = {"iterator", "reverse_iterator",
-  "const_iterator",
-  "const_reverse_iterator"};
-
-  for (const char *Name : IteratorNames) {
-if (hasName(Name).matches(Node, Finder, Builder))
-  return true;
-  }
-  return false;
+Matcher hasStdIteratorName() {
+  static const StringRef IteratorNames[] = {"iterator", "reverse_iterator",
+"const_iterator",
+"const_reverse_iterator"};
+  return hasAnyName(IteratorNames);
 }
 
 /// Matches named declarations that have one of the standard container
@@ -143,26 +138,21 @@ AST_MATCHER(NamedDecl, hasStdIteratorName) {
 ///
 /// recordDecl(hasStdContainerName()) matches \c vector and \c forward_list
 /// but not \c my_vec.
-AST_MATCHER(NamedDecl, hasStdContainerName) {
-  static const char *const ContainerNames[] = {
-  "array", "deque",
-  "forward_list",  "list",
-  "vector",
+Matcher hasStdContainerName() {
+  static StringRef ContainerNames[] = {"array", "deque",
+   "forward_list",  "list",
+   "vector",
 
-  "map",   "multimap",
-  "set",   "multiset",
+   "map",   "multimap",
+   "set",   "multiset",
 
-  "unordered_map", "unordered_multimap",
-  "unordered_set", "unordered_multiset",
+   "unordered_map", "unordered_multimap",
+   "unordered_set", "unordered_multiset",
 
-  "queue", "priority_queue",
-  "stack"};
+   "queue", "priority_queue",
+   "stack"};
 
-  for (const char *Name : ContainerNames) {
-if (hasName(Name).matches(Node, Finder, Builder))
-  return true;
-  }
-  return false;
+  return hasAnyName(ContainerNames);
 }
 
 /// Matches declarations whose declaration context is the C++ standard library



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

[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

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

hokein updated this revision to Diff 283851.
hokein marked 2 inline comments as done.
hokein added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74054

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -47,6 +47,11 @@
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
@@ -945,15 +950,77 @@
   Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None)));
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-template  struct function {};
-template  using callback = function;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+R"cpp(
+  template  struct function {};
+  template  using [[callback]] = function;
 
-c^allback foo;
-  )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+  c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Bar]];
+
+  B^ar b;
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[Bar]] = Foo; // definition
+  Ba^r b;
+)cpp",
+
+// triggered on the underlying decl of a renaming alias.
+R"cpp(
+  class [[Foo]];
+  using Bar = Fo^o;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::F^oo;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int [[x]](double); }
+  using ns::^x;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int x(double); }
+  using ns::x;
+  int y = ^x('a');
+)cpp",
+
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::Foo;
+  F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[B^ar]] = Foo;
+)cpp",
+  };
+
+  for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -290,6 +290,18 @@
   }
 }
 
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+  // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
+  // all overload candidates, we only want the targeted one if the cursor is
+  // on an using-alias usage, workround it with getDeclAtPosition.
+  llvm::for_each(
+  getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+  [&](const NamedDecl *UD) { AddResultDecl(UD); });
+  continue;
+}
+
 // Otherwise the target declaration is the right one.
 AddResultDecl(D);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa70161808bcd: [clangd] Include the underlying decls in 
go-to-definition. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74054

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -47,6 +47,11 @@
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
@@ -945,15 +950,77 @@
   Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None)));
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-template  struct function {};
-template  using callback = function;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+R"cpp(
+  template  struct function {};
+  template  using [[callback]] = function;
 
-c^allback foo;
-  )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+  c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Bar]];
+
+  B^ar b;
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[Bar]] = Foo; // definition
+  Ba^r b;
+)cpp",
+
+// triggered on the underlying decl of a renaming alias.
+R"cpp(
+  class [[Foo]];
+  using Bar = Fo^o;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::F^oo;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int [[x]](double); }
+  using ns::^x;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int x(double); }
+  using ns::x;
+  int y = ^x('a');
+)cpp",
+
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::Foo;
+  F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[B^ar]] = Foo;
+)cpp",
+  };
+
+  for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -290,6 +290,18 @@
   }
 }
 
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+  // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
+  // all overload candidates, we only want the targeted one if the cursor is
+  // on an using-alias usage, workround it with getDeclAtPosition.
+  llvm::for_each(
+  getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+  [&](const NamedDecl *UD) { AddResultDecl(UD); });
+  continue;
+}
+
 // Otherwise the target declaration is the right one.
 AddResultDecl(D);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] a701618 - [clangd] Include the underlying decls in go-to-definition.

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


Author: Haojian Wu
Date: 2020-08-07T11:39:49+02:00
New Revision: a70161808bcd6560e268aecf90138ff68f41fe54

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

LOG: [clangd] Include the underlying decls in go-to-definition.

Fixes https://github.com/clangd/clangd/issues/277

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 26653aa409d7..b61ff4979320 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -290,6 +290,18 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
   }
 }
 
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+  // FIXME: address more complicated cases. TargetDecl(... Underlying) 
gives
+  // all overload candidates, we only want the targeted one if the cursor 
is
+  // on an using-alias usage, workround it with getDeclAtPosition.
+  llvm::for_each(
+  getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+  [&](const NamedDecl *UD) { AddResultDecl(UD); });
+  continue;
+}
+
 // Otherwise the target declaration is the right one.
 AddResultDecl(D);
   }

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index c9c115fd19d8..686ba023a282 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -47,6 +47,11 @@ using ::testing::UnorderedElementsAreArray;
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
@@ -945,15 +950,77 @@ TEST(LocateSymbol, TextualDependent) {
   Sym("uniqueMethodName", Header.range("BarLoc"), 
llvm::None)));
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-template  struct function {};
-template  using callback = function;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+R"cpp(
+  template  struct function {};
+  template  using [[callback]] = function;
 
-c^allback foo;
-  )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+  c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Bar]];
+
+  B^ar b;
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[Bar]] = Foo; // definition
+  Ba^r b;
+)cpp",
+
+// triggered on the underlying decl of a renaming alias.
+R"cpp(
+  class [[Foo]];
+  using Bar = Fo^o;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::F^oo;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int [[x]](double); }
+  using ns::^x;
+)cpp",
+
+R"cpp(
+  namespace ns { int [[x]](char); int x(double); }
+  using ns::x;
+  int y = ^x('a');
+)cpp",
+
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::Foo;
+  F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+  class Foo {};
+  typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+  class Foo {};
+  using [[B^ar]] = Foo;
+)cpp",
+  };
+
+  for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {



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

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

2020-08-07 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D85426#2199501 , @kbobyrev wrote:

> @sammccall This is the change I was talking about during the standup: the 
> code looks legit, but `BackgroundIndexTests.IndexTwoFiles` fails because with 
> the following code path in `Background.cpp`'s `FileFilter`:

What location is invalid, and why?




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:643
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;

kbobyrev wrote:
> hokein wrote:
> > A drive-by comment from D84811: the file granularity vs symbol granularity 
> > is tricky here.
> > 
> > Note that a *full* symbol (with declaration, definition, etc) may be formed 
> > from different files (.h, .cc), thinking of a following case:
> > 
> > ```
> > // foo.h
> > void func();
> > 
> > // user.cc
> > #include "foo.h"
> > 
> > // foo.cc
> > #include "foo.h"
> > void func() {}
> > ```
> > 
> > if our indexer indexes `user.cc` first, then `foo.h` is considered indexed, 
> > later when indexing `foo.cc`, we will skip the `func` symbol. so the symbol 
> > `foo` will not have definition.
> >  
> > 
> Oh, you're right, good catch! That's why the post-filtering would probably 
> work but maybe won't be as fancy :(
> 
> Thank you for mentioning it!
This is indeed happening in the wrong place, I think, but postfiltering isn't 
the only solution.

The FileFilter refers to what files you want to index, not which symbols. So I 
think you want to make the decision in handle*Occurrence, based on the location 
of the occurrence, not the location of the declaration it refers to.

By the time you get to addDefinition/addDeclaration it really is too late, the 
caller has already decided that we want to index this symbol and which 
declaration should be used. (BTW, weakening addDeclaration to return nullptr 
sometimes seems wrong for the same reason).

This is all very confusing, and we should rewrite the indexer to make it more 
explicit how data is combined, and less stateful. (More like a mapreduce)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

2020-08-07 Thread Balázs Kéri via Phabricator via cfe-commits

balazske updated this revision to Diff 283856.
balazske added a comment.
Herald added a subscriber: whisperity.

NFC changes, added some tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84520

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/Inputs/expected-plists/null-deref-path-notes.m.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Analysis/invalid-deref.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/null-deref-offsets.c
  clang/test/Analysis/null-deref-path-notes.c
  clang/test/Analysis/null-deref-path-notes.cpp
  clang/test/Analysis/null-deref-path-notes.m
  clang/test/Analysis/null-deref-ps.c
  clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp

Index: clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
===
--- clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
+++ clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
@@ -14,5 +14,5 @@
 return;
 
   int x = p[0];
-  // expected-warning@-1 {{Array access (from variable 'p') results in a null pointer dereference}}
+  // expected-warning@-1 {{Array access (from variable 'p') results in a dereference of a null pointer}}
 }
Index: clang/test/Analysis/null-deref-ps.c
===
--- clang/test/Analysis/null-deref-ps.c
+++ clang/test/Analysis/null-deref-ps.c
@@ -34,7 +34,7 @@
   if (x)
 return x[i - 1];
   
-  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
+  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a dereference of a null pointer}}
 }
 
 int f3_b(char* x) {
@@ -44,7 +44,7 @@
   if (x)
 return x[i - 1];
   
-  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
+  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a dereference of a null pointer}}
 }
 
 int f4(int *p) {
Index: clang/test/Analysis/null-deref-path-notes.m
===
--- clang/test/Analysis/null-deref-path-notes.m
+++ clang/test/Analysis/null-deref-path-notes.m
@@ -58,8 +58,8 @@
 @public int *p;
 }
 - (void)useArray {
-  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a null pointer dereference}}
-// expected-note@-1{{Array access (via ivar 'p') results in a null pointer dereference}}
+  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a dereference of a null pointer}}
+// expected-note@-1{{Array access (via ivar 'p') results in a dereference of a null pointer}}
 }
 @end
 
Index: clang/test/Analysis/null-deref-path-notes.cpp
===
--- clang/test/Analysis/null-deref-path-notes.cpp
+++ clang/test/Analysis/null-deref-path-notes.cpp
@@ -15,8 +15,8 @@
 // Properly track the null pointer in the array field back to the default
 // constructor of 'h'.
 void c::f(B &g, int &i) {
-  e(g.d[9], i); // expected-warning{{Array access (via field 'd') results in a null pointer dereference}}
-// expected-note@-1{{Array access (via field 'd') results in a null pointer dereference}}
+  e(g.d[9], i); // expected-warning{{Array access (via field 'd') results in a dereference of a null pointer}}
+// expected-note@-1{{Array access (via field 'd') results in a dereference of a null pointer}}
   B h, a; // expected-note{{Value assigned to 'h.d'}}
   a.d == __null; // expected-note{{Assuming the condition is true}}
   a.d != h.d; // expected-note{{Assuming 'a.d' is equal to 'h.d'}}
Index: clang/test/Analysis/null-deref-path-notes.c
===
--- clang/test/Analysis/null-deref-path-notes.c
+++ clang/test/Analysis/null-deref-path-notes.c
@@ -4,8 +4,8 @@
 // of the null pointer for path notes.
 void pr34373() {
   int *a = 0; // expected-note{{'a' initialized to a null pointer value}}
-  (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
-  // expected-note@-1{{Array access results in a null pointer dereference}}
+  (a + 0)[0]; // expected-warning{{Array access results in a dereference of a null pointer}}
+  // expected-note@-1{{Array access results in a dereference of a null pointer}}
 }
 
 typedef __typeof(sizeof(int)) size_t;
Index: clang/test/Analysis/null-deref-offsets.c
===
--- clang/test/Analysis/null-deref-offsets.c
+++ clang/test/Analysis/null-deref-offsets.c
@@ -3

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits

vabridgers updated this revision to Diff 283858.
vabridgers added a comment.

address Bjorn's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/warn-bad-function-cast.c


Index: clang/test/Sema/warn-bad-function-cast.c
===
--- clang/test/Sema/warn-bad-function-cast.c
+++ clang/test/Sema/warn-bad-function-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-triple x86_64-unknown-unknown -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-ffixed-point -triple x86_64-unknown-unknown -verify
 // rdar://9103192
 
 void vf(void);
@@ -12,15 +12,20 @@
 _Bool bf(void);
 char *pf1(void);
 int *pf2(void);
+_Fract ff1(void);
 
 void
 foo(void)
 {
+
+  /* default, no cast, should always be ok */ 
+  ff1();
   /* Casts to void types are always OK.  */
   (void)vf();
   (void)if1();
   (void)cf();
   (const void)bf();
+  (void)ff1();
   /* Casts to the same type or similar types are OK.  */
   (int)if1();
   (long)if2();
@@ -32,6 +37,7 @@
   (_Bool)bf();
   (void *)pf1();
   (char *)pf2();
+  (_Fract) ff1();
   /* All following casts issue warning */
   (float)if1(); /* expected-warning {{cast from function call of type 'int' to 
non-matching type 'float'}} */
   (double)if2(); /* expected-warning {{cast from function call of type 'char' 
to non-matching type 'double'}} */
@@ -43,5 +49,7 @@
   (int)bf(); /* expected-warning {{cast from function call of type '_Bool' to 
non-matching type 'int'}} */
   (__SIZE_TYPE__)pf1(); /* expected-warning {{cast from function call of type 
'char *' to non-matching type 'unsigned long'}} */
   (__PTRDIFF_TYPE__)pf2(); /* expected-warning {{cast from function call of 
type 'int *' to non-matching type 'long'}} */
+  (_Fract) if1(); /* expected-warning{{cast from function call of type 'int' 
to non-matching type '_Fract'}} */
+  (int)ff1(); /* expected-warning{{cast from function call of type 
'_Fract' to non-matching type 'int'}} */
 }
 
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2657,6 +2657,8 @@
 return;
   if (SrcType->isComplexIntegerType() && DestType->isComplexIntegerType())
 return;
+  if (SrcType->isFixedPointType() && DestType->isFixedPointType())
+return;
 
   Self.Diag(SrcExpr.get()->getExprLoc(),
 diag::warn_bad_function_cast)


Index: clang/test/Sema/warn-bad-function-cast.c
===
--- clang/test/Sema/warn-bad-function-cast.c
+++ clang/test/Sema/warn-bad-function-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast -triple x86_64-unknown-unknown -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast -ffixed-point -triple x86_64-unknown-unknown -verify
 // rdar://9103192
 
 void vf(void);
@@ -12,15 +12,20 @@
 _Bool bf(void);
 char *pf1(void);
 int *pf2(void);
+_Fract ff1(void);
 
 void
 foo(void)
 {
+
+  /* default, no cast, should always be ok */ 
+  ff1();
   /* Casts to void types are always OK.  */
   (void)vf();
   (void)if1();
   (void)cf();
   (const void)bf();
+  (void)ff1();
   /* Casts to the same type or similar types are OK.  */
   (int)if1();
   (long)if2();
@@ -32,6 +37,7 @@
   (_Bool)bf();
   (void *)pf1();
   (char *)pf2();
+  (_Fract) ff1();
   /* All following casts issue warning */
   (float)if1(); /* expected-warning {{cast from function call of type 'int' to non-matching type 'float'}} */
   (double)if2(); /* expected-warning {{cast from function call of type 'char' to non-matching type 'double'}} */
@@ -43,5 +49,7 @@
   (int)bf(); /* expected-warning {{cast from function call of type '_Bool' to non-matching type 'int'}} */
   (__SIZE_TYPE__)pf1(); /* expected-warning {{cast from function call of type 'char *' to non-matching type 'unsigned long'}} */
   (__PTRDIFF_TYPE__)pf2(); /* expected-warning {{cast from function call of type 'int *' to non-matching type 'long'}} */
+  (_Fract) if1(); /* expected-warning{{cast from function call of type 'int' to non-matching type '_Fract'}} */
+  (int)ff1(); /* expected-warning{{cast from function call of type '_Fract' to non-matching type 'int'}} */
 }
 
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2657,6 +2657,8 @@
 return;
   if (SrcType->isComplexIntegerType() && DestType->isComplexIntegerType())
 return;
+  if (SrcType->isFixedPointType() && DestType->isFixedPointType())
+return;
 
   Self.Diag(SrcExpr.get()->getExprLoc(),
 diag::warn_bad_function_cast)
__

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits

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



Comment at: clang/test/Sema/warn-bad-function-cast.c:49
+#ifdef FIXED_POINT
+  (void)(_Fract) if1(); // no warning
+#endif

bjope wrote:
> bjope wrote:
> > vabridgers wrote:
> > > bjope wrote:
> > > > bjope wrote:
> > > > > bjope wrote:
> > > > > > This should be added before the line saying `/* All following casts 
> > > > > > issue warning */`.
> > > > > Is the `(void)` needed/relevant here?
> > > > As questioned earlier, shouldn't we expect a warning for this scenario?
> > > > 
> > > > There is however a problem that we get the warning for _Fract to _Fract 
> > > > conversion. And it would be nice with a more complete set of tests 
> > > > involving both FixedPoint->FixedPoint, FixedPoint->Integer and 
> > > > Integer->FixedPoint casts.
> > > If you have any *specific* suggestions for test cases, I'm open to that.
> > Add:
> > 
> > ```
> > _Fract ff1(void);
> > ```
> > 
> > And inside foo add these three tests (you'll need to add appropriate 
> > expects):
> > ```
> > (_Fract)ff1();  // No warning expected.
> > (_Fract)if1();  // Warning expected.
> > (int)ff1();  // Warning expected.
> > ```
> I still think it would be nice not to break the structure of this test. Tests 
> seem to be divided into three categories:
> 
> /* Casts to void types are always OK.  */
> 
>   /* Casts to the same type or similar types are OK.  */
> 
> /* All following casts issue warning */
> 
> And you have currently inserted all new tests in the last section.
> 
When I've seen bew



Comment at: clang/test/Sema/warn-bad-function-cast.c:49
+#ifdef FIXED_POINT
+  (void)(_Fract) if1(); // no warning
+#endif

vabridgers wrote:
> bjope wrote:
> > bjope wrote:
> > > vabridgers wrote:
> > > > bjope wrote:
> > > > > bjope wrote:
> > > > > > bjope wrote:
> > > > > > > This should be added before the line saying `/* All following 
> > > > > > > casts issue warning */`.
> > > > > > Is the `(void)` needed/relevant here?
> > > > > As questioned earlier, shouldn't we expect a warning for this 
> > > > > scenario?
> > > > > 
> > > > > There is however a problem that we get the warning for _Fract to 
> > > > > _Fract conversion. And it would be nice with a more complete set of 
> > > > > tests involving both FixedPoint->FixedPoint, FixedPoint->Integer and 
> > > > > Integer->FixedPoint casts.
> > > > If you have any *specific* suggestions for test cases, I'm open to that.
> > > Add:
> > > 
> > > ```
> > > _Fract ff1(void);
> > > ```
> > > 
> > > And inside foo add these three tests (you'll need to add appropriate 
> > > expects):
> > > ```
> > > (_Fract)ff1();  // No warning expected.
> > > (_Fract)if1();  // Warning expected.
> > > (int)ff1();  // Warning expected.
> > > ```
> > I still think it would be nice not to break the structure of this test. 
> > Tests seem to be divided into three categories:
> > 
> > /* Casts to void types are always OK.  */
> > 
> >   /* Casts to the same type or similar types are OK.  */
> > 
> > /* All following casts issue warning */
> > 
> > And you have currently inserted all new tests in the last section.
> > 
> When I've seen bew
Done. ok to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

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

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Vince Bridgers via Phabricator via cfe-commits

vabridgers marked 4 inline comments as done.
vabridgers added a comment.

All comments marked as done. ok to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

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

[clang] 24cca30 - Remove unreachable return (PR47026)

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


Author: Simon Pilgrim
Date: 2020-08-07T11:23:43+01:00
New Revision: 24cca30f7f0de49e77af7e8c7719680daec2418e

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

LOG: Remove unreachable return (PR47026)

Added: 


Modified: 
clang/lib/Basic/Targets/Mips.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/Mips.cpp 
b/clang/lib/Basic/Targets/Mips.cpp
index ead5e91f7c8f..3a32fd492c6b 100644
--- a/clang/lib/Basic/Targets/Mips.cpp
+++ b/clang/lib/Basic/Targets/Mips.cpp
@@ -41,7 +41,6 @@ bool MipsTargetInfo::processorSupportsGPR64() const {
   .Case("octeon", true)
   .Case("octeon+", true)
   .Default(false);
-  return false;
 }
 
 static constexpr llvm::StringLiteral ValidCPUNames[] = {



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

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

2020-08-07 Thread Momchil Velikov via Phabricator via cfe-commits

chill requested changes to this revision.
chill added inline comments.
This revision now requires changes to proceed.
Herald added a subscriber: dang.



Comment at: llvm/lib/Target/AArch64/AArch64.td:352
 
+def FeatureEmitNoteBTIProperty : SubtargetFeature<"markbtiproperty", 
"MarkBTIProperty",
+"true", "Emit .note.gnu.property for Branch Target Identification" >;

No, this is an abuse of subtarget features. Subtarget features represent 
characteristics of the chip, they shouldn't be used to pass arbitrary bits of 
information.
Possible alternatives - `TargetOptions` (cf. 
`BackendUtil.cpp:initTargetOptions()`) or
LLVM command-line arguments (cf. `BackendUtil.cpp:setCommandLineOpts()`.


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

https://reviews.llvm.org/D81930

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

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

2020-08-07 Thread Simon Tatham via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d782942500b: [Sema][BFloat] Forbid arithmetic on vectors of 
bfloat. (authored by simon_tatham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85009

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/arm-bfloat.cpp


Index: clang/test/Sema/arm-bfloat.cpp
===
--- clang/test/Sema/arm-bfloat.cpp
+++ clang/test/Sema/arm-bfloat.cpp
@@ -27,3 +27,21 @@
   fp16 = bf16; // expected-error {{assigning to '__fp16' from incompatible 
type '__bf16'}}
   bf16 + (b ? fp16 : bf16); // expected-error {{incompatible operand types 
('__fp16' and '__bf16')}}
 }
+
+#include 
+
+void test_vector(bfloat16x4_t a, bfloat16x4_t b, float16x4_t c) {
+  a + b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a - b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a * b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a / b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+
+  a + c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a - c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a * c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a / c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  c + b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c - b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c * b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c / b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9789,6 +9789,10 @@
   const VectorType *RHSVecType = RHSType->getAs();
   assert(LHSVecType || RHSVecType);
 
+  if ((LHSVecType && LHSVecType->getElementType()->isBFloat16Type()) ||
+  (RHSVecType && RHSVecType->getElementType()->isBFloat16Type()))
+return InvalidOperands(Loc, LHS, RHS);
+
   // AltiVec-style "vector bool op vector bool" combinations are allowed
   // for some operators but not others.
   if (!AllowBothBool &&


Index: clang/test/Sema/arm-bfloat.cpp
===
--- clang/test/Sema/arm-bfloat.cpp
+++ clang/test/Sema/arm-bfloat.cpp
@@ -27,3 +27,21 @@
   fp16 = bf16; // expected-error {{assigning to '__fp16' from incompatible type '__bf16'}}
   bf16 + (b ? fp16 : bf16); // expected-error {{incompatible operand types ('__fp16' and '__bf16')}}
 }
+
+#include 
+
+void test_vector(bfloat16x4_t a, bfloat16x4_t b, float16x4_t c) {
+  a + b; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a - b; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a * b; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a / b; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+
+  a + c; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 4 'float16_t' values))}}
+  a - c; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 4 'float16_t' values))}}
+  a * c; // expected-error {{invalid operands to binary expression ('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 4 'float16_t' values

[clang] 1d78294 - [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

2020-08-07 Thread Simon Tatham via cfe-commits


Author: Simon Tatham
Date: 2020-08-07T11:25:19+01:00
New Revision: 1d782942500b2cbc9765ccf16264bb498850cefb

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

LOG: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

Vectors of bfloat are a storage format only; you're supposed to
explicitly convert them to a wider type to do arithmetic on them.
But currently, if you write something like

  bfloat16x4_t test(bfloat16x4_t a, bfloat16x4_t b) { return a + b; }

then the clang frontend accepts it without error, and (ARM or AArch64)
isel fails to generate code for it.

Added a rule in Sema that forbids the attempt from even being made,
and tests that check it. In particular, we also outlaw arithmetic
between vectors of bfloat and any other vector type.

Patch by Luke Cheeseman.

Reviewed By: LukeGeeson

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/arm-bfloat.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4931cf46cffd..b681c930b2a7 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9789,6 +9789,10 @@ QualType Sema::CheckVectorOperands(ExprResult &LHS, 
ExprResult &RHS,
   const VectorType *RHSVecType = RHSType->getAs();
   assert(LHSVecType || RHSVecType);
 
+  if ((LHSVecType && LHSVecType->getElementType()->isBFloat16Type()) ||
+  (RHSVecType && RHSVecType->getElementType()->isBFloat16Type()))
+return InvalidOperands(Loc, LHS, RHS);
+
   // AltiVec-style "vector bool op vector bool" combinations are allowed
   // for some operators but not others.
   if (!AllowBothBool &&

diff  --git a/clang/test/Sema/arm-bfloat.cpp b/clang/test/Sema/arm-bfloat.cpp
index f7ee3c596eb8..ce3fc44baa39 100644
--- a/clang/test/Sema/arm-bfloat.cpp
+++ b/clang/test/Sema/arm-bfloat.cpp
@@ -27,3 +27,21 @@ void test(bool b) {
   fp16 = bf16; // expected-error {{assigning to '__fp16' from incompatible 
type '__bf16'}}
   bf16 + (b ? fp16 : bf16); // expected-error {{incompatible operand types 
('__fp16' and '__bf16')}}
 }
+
+#include 
+
+void test_vector(bfloat16x4_t a, bfloat16x4_t b, float16x4_t c) {
+  a + b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a - b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a * b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+  a / b; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'bfloat16x4_t')}}
+
+  a + c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a - c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a * c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  a / c; // expected-error {{invalid operands to binary expression 
('bfloat16x4_t' (vector of 4 'bfloat16_t' values) and 'float16x4_t' (vector of 
4 'float16_t' values))}}
+  c + b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c - b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c * b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+  c / b; // expected-error {{invalid operands to binary expression 
('float16x4_t' (vector of 4 'float16_t' values) and 'bfloat16x4_t' (vector of 4 
'bfloat16_t' values))}}
+}



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

[clang] ffd258f - Remove unreachable return (PR47028)

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


Author: Simon Pilgrim
Date: 2020-08-07T11:33:23+01:00
New Revision: ffd258f7bbba2a65800970fcea6648fb285d95f4

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

LOG: Remove unreachable return (PR47028)

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/Mips.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/Mips.cpp 
b/clang/lib/Driver/ToolChains/Arch/Mips.cpp
index 7b4dd703c0c7..5a509dbb2bd3 100644
--- a/clang/lib/Driver/ToolChains/Arch/Mips.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/Mips.cpp
@@ -452,8 +452,6 @@ bool mips::isNaN2008(const ArgList &Args, const 
llvm::Triple &Triple) {
   return llvm::StringSwitch(getCPUName(Args, Triple))
   .Cases("mips32r6", "mips64r6", true)
   .Default(false);
-
-  return false;
 }
 
 bool mips::isFP64ADefault(const llvm::Triple &Triple, StringRef CPUName) {



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

[clang] 18212cb - Remove duplicate/unreachable break (PR47029)

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


Author: Simon Pilgrim
Date: 2020-08-07T11:48:48+01:00
New Revision: 18212cbcdadd130e5b2d1bc2bb38e4396ef8a46f

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

LOG: Remove duplicate/unreachable break (PR47029)

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index ff5223c0795e..b2be31ac0990 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -1543,7 +1543,8 @@ static QualType ConvertDeclSpecToType(TypeProcessingState 
&state) {
 << "__float128";
 Result = Context.Float128Ty;
 break;
-  case DeclSpec::TST_bool: Result = Context.BoolTy; break; // _Bool or bool
+  case DeclSpec::TST_bool:
+Result = Context.BoolTy; // _Bool or bool
 break;
   case DeclSpec::TST_decimal32:// _Decimal32
   case DeclSpec::TST_decimal64:// _Decimal64



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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Xun Li via Phabricator via cfe-commits

lxfind added a comment.

@phosek, Under this change, now when I build LLVM (with a basic config `cmake 
-G Ninja --LLVM_ENABLE_PROJECTS="clang" ../llvm`), in file 
`build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see this:

  set_target_properties(LLVMSupport PROPERTIES
INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"

This seems broken to me. Can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

In D82317#2200789 , @rjmccall wrote:

> Are you seriously adding an attribute to literally every argument and return 
> value?  Why is this the right representation?

As far as I understand, this should not be literally every argument. If it is, 
I did get something wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread Gui Andrade via Phabricator via cfe-commits

guiand added a comment.

In D82317#2200789 , @rjmccall wrote:

> Are you seriously adding an attribute to literally every argument and return 
> value?  Why is this the right representation?

This adds an attribute to every argument and return value where the language 
rules denote it cannot be undef (which is ideally in most places, but 
definitely not everywhere). In a previous incarnation of this idea, the 
attribute was called `partialinit`, and had the inverse meaning -- the idea 
being that it would be a less invasive change because it is more common for 
these values to be fully defined (@rsmith at some point noted that in C, return 
values in general may be undef, which would complicate that a bit, but anyway 
was the idea). `partialinit` was scrapped as pretty much everybody agreed it 
wasn't ideal to have a negative attribute: the presence of `partialinit` 
meaning "proceed as normal", and its absence denoting a different behavior.

As for being an attribute, that was the level of granularity we had decided on. 
The alternative I suppose would be to some form of metadata.

As for being enabled by default, the idea was that transformation maintainers 
could get to using the attribute right away. This might be a good place to 
compromise.

And as for applying them to all the tests (rather than suppressing the 
attribute by default or something along those lines), we figured it would be 
best to keep the tests reflecting the actual common-case codegen. If `noundef` 
was on and showing up everywhere, the idea was to have the tests reflect that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread Evgenii Stepanov via Phabricator via cfe-commits

eugenis added a comment.

In D82317#2200789 , @rjmccall wrote:

> Are you seriously adding an attribute to literally every argument and return 
> value?  Why is this the right representation?

We've discussed quite a few options in D81678 
, which is a better place for this 
conversation.

The original patch used an inverted attribute, partialinit, which would be a 
lot less common. The problem with that is that it redefines the meaning of when 
an argument does not have the attribute, which breaks compatibility with 
existing bitcode and requires updating all frontends and any code that emits 
new functions or edits function signatures in IR.

There was an option of a function-level attribute in addition to this one, 
which would mean "all arguments and the return value are noundef". That would 
be a bit uncomfortable to work with in IR transforms, and, arguably, would not 
reduce the total number of attributes that much.

There are also a bunch of options that introduce divergence between

- tests and real use - a test-only cc1 flag to suppress attribute generation
- msan and non-msan compilation - but the reviewers in D81678 
 seem pretty excited to use this info for 
undef/poison optimizations in all configurations

In fact, we already have the test-only cc1 flag that disables the new 
attribute, and use it in some of the tests. This could be extended to all 
tests, that should make the tests diff smaller and more mechanical,  and 
hopefully help with the downstream concerns.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread Gui Andrade via Phabricator via cfe-commits

guiand added a comment.

In D82317#2200809 , @jrtc27 wrote:

> At the very least, make whatever script you used to update these public, as I 
> don't want to have to recreate it from scratch when merging this in. I had 
> enough "fun" with the LLD mass-renaming (UpperCamel -> lowerCamel) and that 
> was _with_ a supposedly-working script (it didn't quite do the right thing 
> and I seem to recall there being two refactoring commits, only one of which 
> had a script); I do not want a repeat of that experience.

That's totally fair, and I can try and prepare a more polished version of the 
script I was using to get started with these changes. But the script isn't 
perfect, unfortunately, and follows roughly the 80-20 rule. The tests aren't 
written in a regular language so there are lots of places where manual 
intervention was necessary.

Sadly I think you're right in that this will cause headaches for downstream 
forks. Maybe this is another good reason to have `noundef` turned off by 
default, at least for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D82317#2200964 , @guiand wrote:

> In D82317#2200789 , @rjmccall wrote:
>
>> Are you seriously adding an attribute to literally every argument and return 
>> value?  Why is this the right representation?
>
> This adds an attribute to every argument and return value where the language 
> rules denote it cannot be undef (which is ideally in most places, but 
> definitely not everywhere).

Okay, so it's just 90% of arguments and return values.  Can you link the 
discussion?  I seem to have completely missed this.

I agree that a positive attribute seems like the right polarity in the 
abstract, and since undef-ness is a core static property in LLVM, it probably 
does need to be fairly invasive.  It's just unfortunate for a couple of 
reasons: first, this is going to be quite disruptive for downstream users and 
forks; second, it's yet another contribution towards the giant pile of 
attributes that seem to have become necessary to do any work in LLVM; and 
third, it's progress towards working around the problems with `undef` rather 
than towards replacing it entirely with a proper indeterminate-but-consistent 
abstraction, which is what's been badly needed for years.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

How can I test this change on pre-merge bots? I haven't seen any builds posted 
on this change before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits

kuhnel added a comment.

In D79219#2202048 , @phosek wrote:

> How can I test this change on pre-merge bots? I haven't seen any builds 
> posted on this change before.

You can just look at the mlir-windows buildbot, this also showed the failing 
build: http://lab.llvm.org:8011/builders/mlir-windows?numbuilds=200
However it's lagging behind quite a bit.

Pre-merge testing is using a different infrastructure and is not sending 
notifications on failing master build. We're still trying to figure out how to 
deal with a broken master branch properly without building every revision...
Right now it's building master every 4 hours using the same setup used for 
pre-merge testing: https://buildkite.com/llvm-project/llvm-master-build/builds

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits

kuhnel added a comment.

This patch broke the Windows compilation on buildbot 
 and pre-merge 
testing . So 
I'll revert it to get pre-merge testing back online. Otherwise this will cause 
false-positives for pre-merge testing.

The error message:

  FAILED: bin/llvm-profdata.exe 
  cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
--intdir=tools\llvm-profdata\CMakeFiles\llvm-profdata.dir --rc="C:\Program 
Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\rc.exe" --mt="C:\Program Files 
(x86)\Windows Kits\10\bin\10.0.18362.0\x64\mt.exe" --manifests  -- 
C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
  /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
/pdb:bin\llvm-profdata.pdb /version:0.0  /machine:x64 /STACK:1000 
/INCREMENTAL:NO /subsystem:console  lib\LLVMCore.lib lib\LLVMProfileData.lib 
lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib delayimp.lib 
-delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib 
user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib 
comdlg32.lib advapi32.lib && cd ."
  LINK: command 
"C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
 /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
/pdb:bin\llvm-profdata.pdb /version:0.0 /machine:x64 /STACK:1000 
/INCREMENTAL:NO /subsystem:console lib\LLVMCore.lib lib\LLVMProfileData.lib 
lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib delayimp.lib 
-delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib 
user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib 
comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTFILE:bin\llvm-profdata.exe.manifest" failed (exit code 1120) with the 
following output:
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol compress2 referenced in function "class llvm::Error __cdecl 
llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,int)" 
(?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol compressBound referenced in function "class llvm::Error __cdecl 
llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,int)" 
(?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol uncompress referenced in function "class llvm::Error __cdecl 
llvm::zlib::uncompress(class llvm::StringRef,class llvm::SmallVectorImpl 
&,unsigned __int64)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
Hint on symbols that are defined and could potentially match:
  "class llvm::Error __cdecl llvm::zlib::uncompress(class 
llvm::StringRef,class llvm::SmallVectorImpl &,unsigned __int64)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
  "class llvm::Error __cdecl llvm::zlib::uncompress(class 
llvm::StringRef,char *,unsigned __int64 &)" 
(?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@PEADAEA_K@Z)
  LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
symbol crc32 referenced in function "unsigned int __cdecl 
llvm::zlib::crc32(class llvm::StringRef)" (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
Hint on symbols that are defined and could potentially match:
  "unsigned int __cdecl llvm::zlib::crc32(class llvm::StringRef)" 
(?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
  C:\GnuWin\lib\zlib.lib : warning LNK4272: library machine type 'x86' 
conflicts with target machine type 'x64'
  bin\llvm-profdata.exe : fatal error LNK1120: 4 unresolved externals


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

In D79219#2201109 , @lxfind wrote:

> @phosek, Under this change, now when I build LLVM (with a basic config `cmake 
> -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm`), in file 
> `build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see this:
>
>   set_target_properties(LLVMSupport PROPERTIES
> INTERFACE_LINK_LIBRARIES "curses;m;ZLIB::ZLIB;LLVMDemangle"
>
> This seems broken to me. Can you take a look?

This is correct. That target is provided by `find_package(ZLIB)`. In 
LLVMConfig.cmake, we invoke `find_package(ZLIB)` when zlib support is enabled. 
If you're using `LLVMExports.cmake`, you'll need to invoke `find_package(ZLIB)` 
yourself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-07 Thread Gui Andrade via Phabricator via cfe-commits

guiand added a comment.

Most of the discussion has been in https://reviews.llvm.org/D81678 
(implementing the attribute in clang) and https://reviews.llvm.org/D82316 
(adding the attribute to langref).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

In D79219#2201994 , @kuhnel wrote:

> This patch broke the Windows compilation on buildbot 
>  and pre-merge 
> testing . So 
> I'll revert it to get pre-merge testing back online. Otherwise this will 
> cause false-positives for pre-merge testing.
>
> The error message:
>
>   FAILED: bin/llvm-profdata.exe 
>   cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
> --intdir=tools\llvm-profdata\CMakeFiles\llvm-profdata.dir --rc="C:\Program 
> Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\rc.exe" --mt="C:\Program 
> Files (x86)\Windows Kits\10\bin\10.0.18362.0\x64\mt.exe" --manifests  -- 
> C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
>   /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
> /pdb:bin\llvm-profdata.pdb /version:0.0  /machine:x64 /STACK:1000 
> /INCREMENTAL:NO /subsystem:console  lib\LLVMCore.lib lib\LLVMProfileData.lib 
> lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
> lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
> shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib 
> delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib 
> kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
> oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
>   LINK: command 
> "C:\BuildTools\VC\Tools\MSVC\14.26.28801\bin\Hostx64\x64\link.exe /nologo 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\llvm-profdata.cpp.obj 
> tools\llvm-profdata\CMakeFiles\llvm-profdata.dir\__\__\resources\windows_version_resource.rc.res
>  /out:bin\llvm-profdata.exe /implib:lib\llvm-profdata.lib 
> /pdb:bin\llvm-profdata.pdb /version:0.0 /machine:x64 /STACK:1000 
> /INCREMENTAL:NO /subsystem:console lib\LLVMCore.lib lib\LLVMProfileData.lib 
> lib\LLVMSupport.lib lib\LLVMCore.lib lib\LLVMBinaryFormat.lib 
> lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMSupport.lib psapi.lib 
> shell32.lib ole32.lib uuid.lib advapi32.lib C:\GnuWin\lib\zlib.lib 
> delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib 
> kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
> oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
> /MANIFESTFILE:bin\llvm-profdata.exe.manifest" failed (exit code 1120) with 
> the following output:
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol compress2 referenced in function "class llvm::Error __cdecl 
> llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
> &,int)" 
> (?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol compressBound referenced in function "class llvm::Error __cdecl 
> llvm::zlib::compress(class llvm::StringRef,class llvm::SmallVectorImpl 
> &,int)" 
> (?compress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@H@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol uncompress referenced in function "class llvm::Error __cdecl 
> llvm::zlib::uncompress(class llvm::StringRef,class 
> llvm::SmallVectorImpl &,unsigned __int64)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
> Hint on symbols that are defined and could potentially match:
>   "class llvm::Error __cdecl llvm::zlib::uncompress(class 
> llvm::StringRef,class llvm::SmallVectorImpl &,unsigned __int64)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@AEAV?$SmallVectorImpl@D@2@_K@Z)
>   "class llvm::Error __cdecl llvm::zlib::uncompress(class 
> llvm::StringRef,char *,unsigned __int64 &)" 
> (?uncompress@zlib@llvm@@YA?AVError@2@VStringRef@2@PEADAEA_K@Z)
>   LLVMSupport.lib(Compression.cpp.obj) : error LNK2019: unresolved external 
> symbol crc32 referenced in function "unsigned int __cdecl 
> llvm::zlib::crc32(class llvm::StringRef)" 
> (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
> Hint on symbols that are defined and could potentially match:
>   "unsigned int __cdecl llvm::zlib::crc32(class llvm::StringRef)" 
> (?crc32@zlib@llvm@@YAIVStringRef@2@@Z)
>   C:\GnuWin\lib\zlib.lib : warning LNK4272: library machine type 'x86' 
> conflicts with target machine type 'x64'
>   bin\llvm-profdata.exe : fatal error LNK1120: 4 unresolved externals

Looking at the error, it seems like you have a 32-bit version of zlib installed 
and in your search path on a 64-bit version of Windows. I'm not quite sure how 
to handle that in CMake.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  h

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-07 Thread Christian Kühnel via Phabricator via cfe-commits

kuhnel added a comment.

> Looking at the error, it seems like you have a 32-bit version of zlib 
> installed and in your search path on a 64-bit version of Windows. I'm not 
> quite sure how to handle that in CMake.

For mlir-windows buildbot:
I have no clue on what is installed there.

For pre-merge testing:
I guess zlib is part of GNUWin32 package being installed. This is the 
recommended setup for Visual Studio 
. The pre-merge builds are running 
in a docker container on Windows 
,
 so you can check what software is being installed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

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

RKSimon added inline comments.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:81
+  "optional",
+  "packaged_task"
+  "promise",

@NoQ https://bugs.llvm.org/show_bug.cgi?id=47030 is reporting that this 
contains a missing comma - please can you take a look?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55307

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

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Bjorn Pettersson via Phabricator via cfe-commits

bjope accepted this revision.
bjope 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/D85157/new/

https://reviews.llvm.org/D85157

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

[PATCH] D85312: [ADT] Move FixedPoint.h from Clang to LLVM.

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

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

Fix some formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85312

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/OptionalDiagnostic.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/FixedPointTest.cpp
  llvm/include/llvm/ADT/APFixedPoint.h
  llvm/lib/Support/APFixedPoint.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/unittests/ADT/APFixedPointTest.cpp
  llvm/unittests/ADT/CMakeLists.txt

Index: llvm/unittests/ADT/CMakeLists.txt
===
--- llvm/unittests/ADT/CMakeLists.txt
+++ llvm/unittests/ADT/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_llvm_unittest(ADTTests
   AnyTest.cpp
+  APFixedPointTest.cpp
   APFloatTest.cpp
   APIntTest.cpp
   APSIntTest.cpp
Index: llvm/unittests/ADT/APFixedPointTest.cpp
===
--- llvm/unittests/ADT/APFixedPointTest.cpp
+++ llvm/unittests/ADT/APFixedPointTest.cpp
@@ -1,4 +1,4 @@
-//===- unittests/Basic/FixedPointTest.cpp -- fixed point number tests -===//
+//===- unittests/ADT/FixedPointTest.cpp -- fixed point number tests -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,12 +6,12 @@
 //
 //===--===//
 
-#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APFixedPoint.h"
 #include "llvm/ADT/APSInt.h"
 #include "gtest/gtest.h"
 
-using clang::APFixedPoint;
-using clang::FixedPointSemantics;
+using llvm::APFixedPoint;
+using llvm::FixedPointSemantics;
 using llvm::APInt;
 using llvm::APSInt;
 
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -56,6 +56,7 @@
   ABIBreak.cpp
   ARMTargetParser.cpp
   AMDGPUMetadata.cpp
+  APFixedPoint.cpp
   APFloat.cpp
   APInt.cpp
   APSInt.cpp
Index: llvm/lib/Support/APFixedPoint.cpp
===
--- llvm/lib/Support/APFixedPoint.cpp
+++ llvm/lib/Support/APFixedPoint.cpp
@@ -1,4 +1,4 @@
-//===- FixedPoint.cpp - Fixed point constant handling ---*- C++ -*-===//
+//===- APFixedPoint.cpp - Fixed point constant handling -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -11,13 +11,13 @@
 //
 //===--===//
 
-#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APFixedPoint.h"
 
-namespace clang {
+namespace llvm {
 
 APFixedPoint APFixedPoint::convert(const FixedPointSemantics &DstSema,
bool *Overflow) const {
-  llvm::APSInt NewVal = Val;
+  APSInt NewVal = Val;
   unsigned DstWidth = DstSema.getWidth();
   unsigned DstScale = DstSema.getScale();
   bool Upscaling = DstScale > getScale();
@@ -31,10 +31,10 @@
 NewVal >>= (getScale() - DstScale);
   }
 
-  auto Mask = llvm::APInt::getBitsSetFrom(
+  auto Mask = APInt::getBitsSetFrom(
   NewVal.getBitWidth(),
   std::min(DstScale + DstSema.getIntegralBits(), NewVal.getBitWidth()));
-  llvm::APInt Masked(NewVal & Mask);
+  APInt Masked(NewVal & Mask);
 
   // Change in the bits above the sign
   if (!(Masked == Mask || Masked == 0)) {
@@ -61,8 +61,8 @@
 }
 
 int APFixedPoint::compare(const APFixedPoint &Other) const {
-  llvm::APSInt ThisVal = getValue();
-  llvm::APSInt OtherVal = Other.getValue();
+  APSInt ThisVal = getValue();
+  APSInt OtherVal = Other.getValue();
   bool ThisSigned = Val.isSigned();
   bool OtherSigned = OtherVal.isSigned();
   unsigned OtherScale = Other.getScale();
@@ -113,14 +113,14 @@
 
 APFixedPoint APFixedPoint::getMax(const FixedPointSemantics &Sema) {
   bool IsUnsigned = !Sema.isSigned();
-  auto Val = llvm::APSInt::getMaxValue(Sema.getWidth(), IsUnsigned);
+  auto Val = APSInt::getMaxValue(Sema.getWidth(), IsUnsigned);
   if (IsUnsigned && Sema.hasUnsignedPadding())
 Val = Val.lshr(1);
   return APFixedPoint(Val, Sema);
 }
 
 APFixedPoint APFixedPoint::getMin(const FixedPointSemantics &Sema) {
-  auto Val = llvm::APSInt::getMinValue(Sema.getWidth(), !Sema.isSigned());
+  auto Val = APSInt::getMinVal

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

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

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

rebase to add this commit from a further patch

- [SyntaxTree] Fix crash on name specifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -873,24 +873,47 @@
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
-namespace a {
+namespace n {
   struct S {
 template
-static T f(){}
+struct ST {
+  static void f();
+};
   };
 }
+template
+struct ST {
+  struct S {
+template
+static U f();
+  };
+};
 void test() {
-  ::  // global-namespace-specifier
-  a:: // namespace-specifier
-  S:: // type-name-specifier
+  :: // global-namespace-specifier
+  n::// namespace-specifier
+  S::// type-name-specifier
+  template ST:: // type-template-instantiation-specifier
+  f();
+
+  n::// namespace-specifier
+  S::// type-name-specifier
+  ST::  // type-template-instantiation-specifier
+  f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
   f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
+  template f();
 }
 )cpp",
   R"txt(
 *: TranslationUnit
 |-NamespaceDefinition
 | |-namespace
-| |-a
+| |-n
 | |-{
 | |-SimpleDeclaration
 | | |-struct
@@ -904,19 +927,58 @@
 | | | | `-T
 | | | |->
 | | | `-SimpleDeclaration
-| | |   |-static
-| | |   |-T
-| | |   |-SimpleDeclarator
-| | |   | |-f
-| | |   | `-ParametersAndQualifiers
-| | |   |   |-(
-| | |   |   `-)
-| | |   `-CompoundStatement
-| | | |-{
-| | | `-}
+| | |   |-struct
+| | |   |-ST
+| | |   |-{
+| | |   |-SimpleDeclaration
+| | |   | |-static
+| | |   | |-void
+| | |   | |-SimpleDeclarator
+| | |   | | |-f
+| | |   | | `-ParametersAndQualifiers
+| | |   | |   |-(
+| | |   | |   `-)
+| | |   | `-;
+| | |   |-}
+| | |   `-;
 | | |-}
 | | `-;
 | `-}
+|-TemplateDeclaration
+| |-template
+| |-<
+| |-UnknownDeclaration
+| | |-typename
+| | `-T
+| |->
+| `-SimpleDeclaration
+|   |-struct
+|   |-ST
+|   |-{
+|   |-SimpleDeclaration
+|   | |-struct
+|   | |-S
+|   | |-{
+|   | |-TemplateDeclaration
+|   | | |-template
+|   | | |-<
+|   | | |-UnknownDeclaration
+|   | | | |-typename
+|   | | | `-U
+|   | | |->
+|   | | `-SimpleDeclaration
+|   | |   |-static
+|   | |   |-U
+|   | |   |-SimpleDeclarator
+|   | |   | |-f
+|   | |   | `-ParametersAndQualifiers
+|   | |   |   |-(
+|   | |   |   `-)
+|   | |   `-;
+|   | |-}
+|   | `-;
+|   |-}
+|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -930,14 +992,81 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameSpecifier
-| | | | | `-::
-| | | | |-NameSpecifier
-| | | | | |-a
-| | | | | `-::
-| | | | `-NameSpecifier
-| | | |   |-S
-| | | |   `-::
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-template
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   |-f
+| | |   |-<
+| | |   |-int
+| | |   `->
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-Ident

[PATCH] D84781: [SyntaxTree] Use PointerUnion instead of inheritance for alternative clauses in NNS

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

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

Remove Fix crash on name specifier commit from patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84781

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -993,18 +993,19 @@
 | | |-IdExpression
 | | | |-NestedNameSpecifier
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-template
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-template
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1015,17 +1016,18 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1036,13 +1038,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | `-UnqualifiedId
@@ -1057,13 +1060,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | |-template
@@ -1121,15 +1125,16 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-SimpleTemplateNameSpecifier
-  | | | | | |-template
-  | | | | | |-U
-  | | | | | |-<
-  | | | | | |-int
-  | | | | | `->
+  | | | | |-NameSpecifier
+  | | | | | `-SimpleTemplateSpecifier
+  | | | | |   |-template
+  | | | | |   |-U
+  | | | | |   |-<
+  | | | | |   |-int
+  | | | | |   `->
   | | | | `-::
   | | | `-UnqualifiedId
   | | |   `-f
@@ -1140,10 +1145,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-U
   | | | | `-::
   | | | `-UnqualifiedId
@@ -1155,7 +1160,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | `-::
   | | | |-template
@@ -1222,13 +1227,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-DecltypeNameSpecifier
-| | | | | |-decltype
-| | | | | |-(
-| | | | | |-IdExpression
-| | | | | | `-UnqualifiedId
-| | | | | |   `-s
-| | | | | `-)
+| | | | |-NameSpecifier
+| | | | | `-DecltypeSpecifier
+| | | | |   |-decltype
+| | | | |   |-(
+| | | | |   |-IdExpression
+| | | | |   | `-UnqualifiedId
+| | | | |   |   `-s
+| | | | |   `-)
 | | | | `-::
 | | | `-Unqu

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

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

hokein added a comment.

In D85417#2202046 , @ArcsinX wrote:

> In D85417#2201973 , @hokein wrote:
>
>> Thinking more about this,
>>
>>> Inside clangd, clang-tidy checks don't see preprocessor events in the 
>>> preamble.
>>> This leads to Token::PtrData == nullptr for tokens that the macro is 
>>> defined to.
>>> E.g. #define SIGTERM 15:
>>>
>>> Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
>>> Token::UintData == 2
>>> Token::PtrData == nullptr
>>
>> The token is in a pretty-broken state. Do you know why the UintData is set 
>> to 2, I suppose this is the length of the macro definition (`15`). If the 
>> PtrData is nullptr, I'd expect the UintData is 0.
>
> I think it's here:
>
>   Token ASTReader::ReadToken(ModuleFile &F, const RecordDataImpl &Record,
>  unsigned &Idx) {
> Token Tok;
> Tok.startToken();
> Tok.setLocation(ReadSourceLocation(F, Record, Idx));
> Tok.setLength(Record[Idx++]);
> if (IdentifierInfo *II = getLocalIdentifier(F, Record[Idx++]))
>   Tok.setIdentifierInfo(II);
> Tok.setKind((tok::TokenKind)Record[Idx++]);
> Tok.setFlag((Token::TokenFlags)Record[Idx++]);
> return Tok;
>   }
>
> So, we set `Token::UintData` via `Token::setLength()` at preamble read, but 
> do not set `Token::PtrData` without preprocessor events.

I see, thanks! I think this is a missing feature in AST serialization, see 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Serialization/ASTWriter.cpp#L4260-L4261.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[clang] 351aac0 - [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread via cfe-commits


Author: Vince Bridgers
Date: 2020-08-07T07:58:53-05:00
New Revision: 351aac09854399dfb3abb581ef1762a9cd2e4991

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

LOG: [Sema] Add casting check for fixed to fixed point conversions

This change squelches the warning for a cast from fixed to fixed point
conversions when -Wbad-function-cast is enabled.

Fixes:

cast from function call of type '_Fract' to non-matching type '_Fract'
[-Wbad-function-cast]

Reviewed By: bjope

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

Added: 


Modified: 
clang/lib/Sema/SemaCast.cpp
clang/test/Sema/warn-bad-function-cast.c

Removed: 




diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 58cf3a1be730..93752766f4a2 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2657,6 +2657,8 @@ static void DiagnoseBadFunctionCast(Sema &Self, const 
ExprResult &SrcExpr,
 return;
   if (SrcType->isComplexIntegerType() && DestType->isComplexIntegerType())
 return;
+  if (SrcType->isFixedPointType() && DestType->isFixedPointType())
+return;
 
   Self.Diag(SrcExpr.get()->getExprLoc(),
 diag::warn_bad_function_cast)

diff  --git a/clang/test/Sema/warn-bad-function-cast.c 
b/clang/test/Sema/warn-bad-function-cast.c
index 41a3f7824e91..1b720747b5cd 100644
--- a/clang/test/Sema/warn-bad-function-cast.c
+++ b/clang/test/Sema/warn-bad-function-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-triple x86_64-unknown-unknown -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-ffixed-point -triple x86_64-unknown-unknown -verify
 // rdar://9103192
 
 void vf(void);
@@ -12,15 +12,20 @@ enum e { E1 } ef(void);
 _Bool bf(void);
 char *pf1(void);
 int *pf2(void);
+_Fract ff1(void);
 
 void
 foo(void)
 {
+
+  /* default, no cast, should always be ok */
+  ff1();
   /* Casts to void types are always OK.  */
   (void)vf();
   (void)if1();
   (void)cf();
   (const void)bf();
+  (void)ff1();
   /* Casts to the same type or similar types are OK.  */
   (int)if1();
   (long)if2();
@@ -32,6 +37,7 @@ foo(void)
   (_Bool)bf();
   (void *)pf1();
   (char *)pf2();
+  (_Fract) ff1();
   /* All following casts issue warning */
   (float)if1(); /* expected-warning {{cast from function call of type 'int' to 
non-matching type 'float'}} */
   (double)if2(); /* expected-warning {{cast from function call of type 'char' 
to non-matching type 'double'}} */
@@ -43,5 +49,7 @@ foo(void)
   (int)bf(); /* expected-warning {{cast from function call of type '_Bool' to 
non-matching type 'int'}} */
   (__SIZE_TYPE__)pf1(); /* expected-warning {{cast from function call of type 
'char *' to non-matching type 'unsigned long'}} */
   (__PTRDIFF_TYPE__)pf2(); /* expected-warning {{cast from function call of 
type 'int *' to non-matching type 'long'}} */
+  (_Fract) if1();  /* expected-warning{{cast from function call of 
type 'int' to non-matching type '_Fract'}} */
+  (int)ff1();  /* expected-warning{{cast from function call of 
type '_Fract' to non-matching type 'int'}} */
 }
 



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

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-07 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG351aac098543: [Sema] Add casting check for fixed to fixed 
point conversions (authored by vabridgers, committed by einvbri 
).

Changed prior to commit:
  https://reviews.llvm.org/D85157?vs=283858&id=283887#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/warn-bad-function-cast.c


Index: clang/test/Sema/warn-bad-function-cast.c
===
--- clang/test/Sema/warn-bad-function-cast.c
+++ clang/test/Sema/warn-bad-function-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-triple x86_64-unknown-unknown -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast 
-ffixed-point -triple x86_64-unknown-unknown -verify
 // rdar://9103192
 
 void vf(void);
@@ -12,15 +12,20 @@
 _Bool bf(void);
 char *pf1(void);
 int *pf2(void);
+_Fract ff1(void);
 
 void
 foo(void)
 {
+
+  /* default, no cast, should always be ok */
+  ff1();
   /* Casts to void types are always OK.  */
   (void)vf();
   (void)if1();
   (void)cf();
   (const void)bf();
+  (void)ff1();
   /* Casts to the same type or similar types are OK.  */
   (int)if1();
   (long)if2();
@@ -32,6 +37,7 @@
   (_Bool)bf();
   (void *)pf1();
   (char *)pf2();
+  (_Fract) ff1();
   /* All following casts issue warning */
   (float)if1(); /* expected-warning {{cast from function call of type 'int' to 
non-matching type 'float'}} */
   (double)if2(); /* expected-warning {{cast from function call of type 'char' 
to non-matching type 'double'}} */
@@ -43,5 +49,7 @@
   (int)bf(); /* expected-warning {{cast from function call of type '_Bool' to 
non-matching type 'int'}} */
   (__SIZE_TYPE__)pf1(); /* expected-warning {{cast from function call of type 
'char *' to non-matching type 'unsigned long'}} */
   (__PTRDIFF_TYPE__)pf2(); /* expected-warning {{cast from function call of 
type 'int *' to non-matching type 'long'}} */
+  (_Fract) if1();  /* expected-warning{{cast from function call of 
type 'int' to non-matching type '_Fract'}} */
+  (int)ff1();  /* expected-warning{{cast from function call of 
type '_Fract' to non-matching type 'int'}} */
 }
 
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2657,6 +2657,8 @@
 return;
   if (SrcType->isComplexIntegerType() && DestType->isComplexIntegerType())
 return;
+  if (SrcType->isFixedPointType() && DestType->isFixedPointType())
+return;
 
   Self.Diag(SrcExpr.get()->getExprLoc(),
 diag::warn_bad_function_cast)


Index: clang/test/Sema/warn-bad-function-cast.c
===
--- clang/test/Sema/warn-bad-function-cast.c
+++ clang/test/Sema/warn-bad-function-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast -triple x86_64-unknown-unknown -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wbad-function-cast -ffixed-point -triple x86_64-unknown-unknown -verify
 // rdar://9103192
 
 void vf(void);
@@ -12,15 +12,20 @@
 _Bool bf(void);
 char *pf1(void);
 int *pf2(void);
+_Fract ff1(void);
 
 void
 foo(void)
 {
+
+  /* default, no cast, should always be ok */
+  ff1();
   /* Casts to void types are always OK.  */
   (void)vf();
   (void)if1();
   (void)cf();
   (const void)bf();
+  (void)ff1();
   /* Casts to the same type or similar types are OK.  */
   (int)if1();
   (long)if2();
@@ -32,6 +37,7 @@
   (_Bool)bf();
   (void *)pf1();
   (char *)pf2();
+  (_Fract) ff1();
   /* All following casts issue warning */
   (float)if1(); /* expected-warning {{cast from function call of type 'int' to non-matching type 'float'}} */
   (double)if2(); /* expected-warning {{cast from function call of type 'char' to non-matching type 'double'}} */
@@ -43,5 +49,7 @@
   (int)bf(); /* expected-warning {{cast from function call of type '_Bool' to non-matching type 'int'}} */
   (__SIZE_TYPE__)pf1(); /* expected-warning {{cast from function call of type 'char *' to non-matching type 'unsigned long'}} */
   (__PTRDIFF_TYPE__)pf2(); /* expected-warning {{cast from function call of type 'int *' to non-matching type 'long'}} */
+  (_Fract) if1();  /* expected-warning{{cast from function call of type 'int' to non-matching type '_Fract'}} */
+  (int)ff1();  /* expected-warning{{cast from function call of type '_Fract' to non-matching type 'int'}} */
 }
 
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2657,6 +2657,8 @@
 return;
   if (SrcType->isCom

[clang-tools-extra] 90d1c66 - [clangd] Fix a typo, NFC.

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


Author: Haojian Wu
Date: 2020-08-07T15:02:06+02:00
New Revision: 90d1c66c7ce48fdaabd842786034e301fdd40fff

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

LOG: [clangd] Fix a typo, NFC.

Added: 


Modified: 
clang-tools-extra/clangd/ParsedAST.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.h 
b/clang-tools-extra/clangd/ParsedAST.h
index c01f1fa0e6d8..361b20aeff4c 100644
--- a/clang-tools-extra/clangd/ParsedAST.h
+++ b/clang-tools-extra/clangd/ParsedAST.h
@@ -131,7 +131,7 @@ class ParsedAST {
   std::unique_ptr Action;
   /// Tokens recorded after the preamble finished.
   ///   - Includes all spelled tokens for the main file.
-  ///   - Includes expanded tokens produced **after** preabmle.
+  ///   - Includes expanded tokens produced **after** preamble.
   ///   - Does not have spelled or expanded tokens for files from preamble.
   syntax::TokenBuffer Tokens;
 



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

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

2020-08-07 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:227-254
+namespace llvm {
+template <> struct DenseMapInfo {
+  using FirstInfo = DenseMapInfo;
+  using SecondInfo = DenseMapInfo;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),

eduucaldas wrote:
> Inpired on the definition of: 
> * `template struct DenseMapInfo`
> * `template struct DenseMapInfo>`
> 
> Please tell me if this is all silly.
This is the right way to use DenseMapInfo, just move the specialization into 
the file that defines NNSLoc.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:892
 
-  syntax::NestedNameSpecifier *
-  BuildNestedNameSpecifier(const NestedNameSpecifierLoc &QualifierLoc) {
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc QualifierLoc) {
 if (!QualifierLoc)

Please add a comment that explains why we override Traverse for NNSLoc instead 
of WalkUpFrom.

Something like:

To build syntax tree nodes for NestedNameSpecifierLoc we override Traverse 
instead of WalkUpFrom because we want to traverse the children ourselves and 
build a list instead of a nested tree of name specifier prefixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85439

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

[clang] aa0d19a - [Fixed Point] Add fixed-point shift operations and consteval.

2020-08-07 Thread Bevin Hansson via cfe-commits


Author: Bevin Hansson
Date: 2020-08-07T15:09:24+02:00
New Revision: aa0d19a0c8f5ebccb5768411dfc4feddff7bed08

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

LOG: [Fixed Point] Add fixed-point shift operations and consteval.

Reviewers: rjmccall, leonardchan, bjope

Subscribers: cfe-commits

Tags: #clang

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

Added: 
clang/test/Frontend/fixed_point_shift.c

Modified: 
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Basic/FixedPoint.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Frontend/fixed_point_errors.c

Removed: 




diff  --git a/clang/include/clang/Basic/FixedPoint.h 
b/clang/include/clang/Basic/FixedPoint.h
index 0d181f30907f..ee52a2b0a615 100644
--- a/clang/include/clang/Basic/FixedPoint.h
+++ b/clang/include/clang/Basic/FixedPoint.h
@@ -132,17 +132,20 @@ class APFixedPoint {
   APFixedPoint mul(const APFixedPoint &Other, bool *Overflow = nullptr) const;
   APFixedPoint div(const APFixedPoint &Other, bool *Overflow = nullptr) const;
 
-  /// Perform a unary negation (-X) on this fixed point type, taking into
-  /// account saturation if applicable.
-  APFixedPoint negate(bool *Overflow = nullptr) const;
-
-  APFixedPoint shr(unsigned Amt) const {
+  // Perform shift operations on a fixed point type. Unlike the other binary
+  // operations, the resulting fixed point value will be in the original
+  // semantic.
+  APFixedPoint shl(unsigned Amt, bool *Overflow = nullptr) const;
+  APFixedPoint shr(unsigned Amt, bool *Overflow = nullptr) const {
+// Right shift cannot overflow.
+if (Overflow)
+  *Overflow = false;
 return APFixedPoint(Val >> Amt, Sema);
   }
 
-  APFixedPoint shl(unsigned Amt) const {
-return APFixedPoint(Val << Amt, Sema);
-  }
+  /// Perform a unary negation (-X) on this fixed point type, taking into
+  /// account saturation if applicable.
+  APFixedPoint negate(bool *Overflow = nullptr) const;
 
   /// Return the integral part of this fixed point number, rounded towards
   /// zero. (-2.5k -> -2)

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index d0587cb723bc..11ba8db24355 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13003,6 +13003,29 @@ bool 
FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
   .convert(ResultFXSema, &ConversionOverflow);
 break;
   }
+  case BO_Shl:
+  case BO_Shr: {
+FixedPointSemantics LHSSema = LHSFX.getSemantics();
+llvm::APSInt RHSVal = RHSFX.getValue();
+
+unsigned ShiftBW =
+LHSSema.getWidth() - (unsigned)LHSSema.hasUnsignedPadding();
+unsigned Amt = RHSVal.getLimitedValue(ShiftBW - 1);
+// Embedded-C 4.1.6.2.2:
+//   The right operand must be nonnegative and less than the total number
+//   of (nonpadding) bits of the fixed-point operand ...
+if (RHSVal.isNegative())
+  Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHSVal;
+else if (Amt != RHSVal)
+  Info.CCEDiag(E, diag::note_constexpr_large_shift)
+  << RHSVal << E->getType() << ShiftBW;
+
+if (E->getOpcode() == BO_Shl)
+  Result = LHSFX.shl(Amt, &OpOverflow);
+else
+  Result = LHSFX.shr(Amt, &OpOverflow);
+break;
+  }
   default:
 return false;
   }

diff  --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp
index ed8b92c98fdb..892b8c19fd79 100644
--- a/clang/lib/Basic/FixedPoint.cpp
+++ b/clang/lib/Basic/FixedPoint.cpp
@@ -309,6 +309,40 @@ APFixedPoint APFixedPoint::div(const APFixedPoint &Other,
   CommonFXSema);
 }
 
+APFixedPoint APFixedPoint::shl(unsigned Amt, bool *Overflow) const {
+  llvm::APSInt ThisVal = Val;
+  bool Overflowed = false;
+
+  // Widen the LHS.
+  unsigned Wide = Sema.getWidth() * 2;
+  if (Sema.isSigned())
+ThisVal = ThisVal.sextOrSelf(Wide);
+  else
+ThisVal = ThisVal.zextOrSelf(Wide);
+
+  // Clamp the shift amount at the original width, and perform the shift.
+  Amt = std::min(Amt, ThisVal.getBitWidth());
+  llvm::APSInt Result = ThisVal << Amt;
+  Result.setIsSigned(Sema.isSigned());
+
+  // If our result lies outside of the representative range of the
+  // semantic, we either have overflow or saturation.
+  llvm::APSInt Max = APFixedPoint::getMax(Sema).getValue().extOrTrunc(Wide);
+  llvm::APSInt Min = APFixedPoint::getMin(Sema).getValue().extOrTrunc(Wide);
+  if (Sema.isSaturated()) {
+if (Result < Min)
+  Result = Min;
+else if (Result > Max)
+  Result = Max;
+  } else
+Overflowed = Result < Min || Result > Max;
+
+  if (Overflow)
+*Overflow = Overflowed;
+
+  return APFixedPoint(Result.sextOrTrunc(Sema.getWidth()), Sema);
+}
+
 void APFixedPoint:

[PATCH] D83212: [Fixed Point] Add fixed-point shift operations and consteval.

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaa0d19a0c8f5: [Fixed Point] Add fixed-point shift operations 
and consteval. (authored by ebevhan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83212

Files:
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Frontend/fixed_point_errors.c
  clang/test/Frontend/fixed_point_shift.c

Index: clang/test/Frontend/fixed_point_shift.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_shift.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
+short _Accum sa_const1 = 1.0hk << 2;   // CHECK-DAG: @sa_const1 = {{.*}}global i16 512
+short _Accum sa_const2 = 0.5hk << 2;   // CHECK-DAG: @sa_const2 = {{.*}}global i16 256
+short _Accum sa_const3 = 10.0hk >> 3;  // CHECK-DAG: @sa_const3 = {{.*}}global i16 160
+short _Accum sa_const4 = 0.0546875hk << 8; // CHECK-DAG: @sa_const4 = {{.*}}global i16 1792
+short _Accum sa_const5 = -1.0hk << 2;  // CHECK-DAG: @sa_const5 = {{.*}}global i16 -512
+short _Accum sa_const6 = -255.0hk >> 8;// CHECK-DAG: @sa_const6 = {{.*}}global i16 -128
+
+_Fract f_const1 = -1.0r >> 5;  // CHECK-DAG: @f_const1 = {{.*}}global i16 -1024
+_Fract f_const2 = 0.0052490234375r >> 3;   // CHECK-DAG: @f_const2 = {{.*}}global i16 21
+_Fract f_const3 = -0.0001r << 5;   // CHECK-DAG: @f_const3 = {{.*}}global i16 -96
+_Fract f_const4 = -0.75r >> 15;// CHECK-DAG: @f_const4 = {{.*}}global i16 -1
+_Fract f_const5 = 0.078216552734375r << 3; // CHECK-DAG: @f_const5 = {{.*}}global i16 20504
+
+unsigned _Fract uf_const1 = 0.375ur >> 13;
+// SIGNED-DAG:   @uf_const1 = {{.*}}global i16 3
+// UNSIGNED-DAG: @uf_const1 = {{.*}}global i16 1
+unsigned _Fract uf_const2 = 0.0546875ur << 3;
+// SIGNED-DAG:   @uf_const2 = {{.*}}global i16 28672
+// UNSIGNED-DAG: @uf_const2 = {{.*}}global i16 14336
+
+_Sat short _Accum ssa_const1 = (_Sat short _Accum)31.875hk << 4; // CHECK-DAG: @ssa_const1 = {{.*}}global i16 32767
+_Sat short _Accum ssa_const2 = (_Sat short _Accum) - 1.0hk << 8; // CHECK-DAG: @ssa_const2 = {{.*}}global i16 -32768
+_Sat short _Accum ssa_const3 = (_Sat short _Accum)128.0hk << 8;  // CHECK-DAG: @ssa_const3 = {{.*}}global i16 32767
+_Sat short _Fract ssf_const1 = (_Sat short _Fract) - 0.5hr << 3; // CHECK-DAG: @ssf_const1 = {{.*}}global i8 -128
+
+_Sat unsigned _Fract suf_const1 = (_Sat unsigned _Fract)0.5r << 1;
+// SIGNED-DAG:   @suf_const1 = {{.*}}global i16 -1
+// UNSIGNED-DAG: @suf_const1 = {{.*}}global i16 32767
+_Sat unsigned _Fract suf_const2 = (_Sat unsigned _Fract)0.25r << 1;
+// SIGNED-DAG:   @suf_const2 = {{.*}}global i16 -32768
+// UNSIGNED-DAG: @suf_const2 = {{.*}}global i16 16384
+_Sat unsigned _Accum sua_const2 = (_Sat unsigned _Accum)128.0uk << 10;
+// SIGNED-DAG:   @sua_const2 = {{.*}}global i32 -1
+// UNSIGNED-DAG: @sua_const2 = {{.*}}global i32 2147483647
Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -259,11 +259,30 @@
 short _Accum mul_ovf2 = (-0.5hr - 0.5hr) * (-0.5hr - 0.5hr);  // expected-warning {{overflow in expression; result is -1.0 with type 'short _Fract'}}
 short _Accum div_ovf1 = 255.0hk / 0.5hk;  // expected-warning {{overflow in expression; result is -2.0 with type 'short _Accum'}}
 
+short _Accum shl_ovf1 = 255.0hk << 8;   // expected-warning {{overflow in expression; result is -256.0 with type 'short _Accum'}}
+short _Fract shl_ovf2 = -0.25hr << 3;   // expected-warning {{overflow in expression; result is 0.0 with type 'short _Fract'}}
+unsigned short _Accum shl_ovf3 = 100.5uhk << 3; // expected-warning {{overflow in expression; result is 36.0 with type 'unsigned short _Accum'}}
+short _Fract shl_ovf4 = 0.25hr << 2;// expected-warning {{overflow in expression; result is -1.0 with type 'short _Fract'}}
+
+_Accum shl_bw1 = 0.91552734375k << 32;   // expected-warning {{shift count >= width of type}} \
+ expected-warning {{overflow in expression; result is -65536.0 with type '_Accum'}}
+unsigned _Fract shl_bw2 = 0.65ur << 16;  // expected-warning {{shift count >= width of type}} \
+ expected-warning {{overflow in expression; result is 0.0 with type 'unsigned _Fract'}

[PATCH] D34654: Allow passing a regex for headers to exclude from clang-tidy

2020-08-07 Thread Saimus Dev via Phabricator via cfe-commits

saimusdev added a comment.

Hello,

Any news on accepting this feature?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D34654

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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-07 Thread Jason Liu via Phabricator via cfe-commits

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

Thanks. LGTM.


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

https://reviews.llvm.org/D84534

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

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ArcsinX requested review of this revision.

PP->getMacroInfo() returns nullptr for undefined macro, so we need to check 
this return value before dereference.
Stack dump:

  #0 0x02185e6a llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/llvm-project/build/bin/clang-tidy+0x2185e6a)
  #1 0x02183e8c llvm::sys::RunSignalHandlers() 
(/llvm-project/build/bin/clang-tidy+0x2183e8c)
  #2 0x02183ff3 SignalHandler(int) 
(/llvm-project/build/bin/clang-tidy+0x2183ff3)
  #3 0x7f37df9b1390 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
  #4 0x0052054e 
clang::tidy::bugprone::NotNullTerminatedResultCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (/llvm-project/build/bin/clang-tidy+0x52054e)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85523

Files:
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+#undef __STDC_WANT_LIB_EXT1__
+
+void f(const char *src) {
+  char dest[13];
+  memcpy_s(dest, 13, src, strlen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 
'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: char dest[14];
+  // CHECK-FIXES-NEXT: strncpy_s(dest, 14, src, strlen(src) - 1);
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -802,11 +802,14 @@
 while (It != PP->macro_end() && !AreSafeFunctionsWanted.hasValue()) {
   if (It->first->getName() == "__STDC_WANT_LIB_EXT1__") {
 const auto *MI = PP->getMacroInfo(It->first);
-const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+// PP->getMacroInfo() returns nullptr if macro has no definition.
+if (MI) {
+  const auto &T = MI->tokens().back();
+  StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+  llvm::APInt IntValue;
+  ValueStr.getAsInteger(10, IntValue);
+  AreSafeFunctionsWanted = IntValue.getZExtValue();
+}
   }
 
   ++It;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+#undef __STDC_WANT_LIB_EXT1__
+
+void f(const char *src) {
+  char dest[13];
+  memcpy_s(dest, 13, src, strlen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: char dest[14];
+  // CHECK-FIXES-NEXT: strncpy_s(dest, 14, src, strlen(src) - 1);
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -802,11 +802,14 @@
 while (It != PP->macro_end() && !AreSafeFunctionsWanted.hasValue()) {
   if (It->first->getName() == "__STDC_WANT_LIB_EXT1__") {
 const auto *MI = PP->getMacroInfo(It->first);
-const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = Int

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ArcsinX requested review of this revision.

If `__STDC_WANT_LIB_EXT1__` is not a literal (e.g. `#define 
__STDC_WANT_LIB_EXT1__ ((unsigned)1)`) bugprone-not-null-terminated-result 
check crashes.
Stack dump:

  #0 0x02185e6a llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/llvm-project/build/bin/clang-tidy+0x2185e6a)
  #1 0x02183e8c llvm::sys::RunSignalHandlers() 
(/llvm-project/build/bin/clang-tidy+0x2183e8c)
  #2 0x02183ff3 SignalHandler(int) 
(/llvm-project/build/bin/clang-tidy+0x2183ff3)
  #3 0x7f08d91b1390 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
  #4 0x021338bb llvm::StringRef::getAsInteger(unsigned int, 
llvm::APInt&) const (/llvm-project/build/bin/clang-tidy+0x21338bb)
  #5 0x0052051c 
clang::tidy::bugprone::NotNullTerminatedResultCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (/llvm-project/build/bin/clang-tidy+0x52051c)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85525

Files:
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ ((unsigned)1)
+
+void f(const char *src) {
+  char dest[13];
+  memcpy_s(dest, 13, src, strlen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 
'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: char dest[14];
+  // CHECK-FIXES-NEXT: strncpy_s(dest, 14, src, strlen(src) - 1);
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -803,10 +803,12 @@
   if (It->first->getName() == "__STDC_WANT_LIB_EXT1__") {
 const auto *MI = PP->getMacroInfo(It->first);
 const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+if (T.isLiteral()) {
+  StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+  llvm::APInt IntValue;
+  ValueStr.getAsInteger(10, IntValue);
+  AreSafeFunctionsWanted = IntValue.getZExtValue();
+}
   }
 
   ++It;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ ((unsigned)1)
+
+void f(const char *src) {
+  char dest[13];
+  memcpy_s(dest, 13, src, strlen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: char dest[14];
+  // CHECK-FIXES-NEXT: strncpy_s(dest, 14, src, strlen(src) - 1);
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -803,10 +803,12 @@
   if (It->first->getName() == "__STDC_WANT_LIB_EXT1__") {
 const auto *MI = PP->getMacroInfo(It->first);
 const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+if (T.isLiteral()) {
+  StringRef ValueStr = Str

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

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

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:806
 const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+if (T.isLiteral()) {
+  StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());

let's add the `getLiteralData` check here --  `if (T.isLiteral() && 
T.getLiteralData())`



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c:3
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"

I think you probably need a `// UNSUPPORTED: system-windows`, as the 
`bugprone-not-null-terminated-result-strlen.c` does.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c:16
+}
\ No newline at end of file


nit: add EOF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85525

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

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

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

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c:3
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"

The same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85523

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

[clang] 4a7aedb - [OPENMP]Simplify representation for atomic, critical, master and section

2020-08-07 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2020-08-07T09:58:23-04:00
New Revision: 4a7aedb843a591900b419e86dcf46d9bec4c5526

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

LOG: [OPENMP]Simplify representation for atomic, critical, master and section
constrcut.

Several constructs may be represented wityout relying on CapturedStmt.
It saves memory and improves compilation speed.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/StmtPrinter.cpp
clang/lib/Analysis/CFG.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Sema/JumpDiagnostics.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/test/AST/ast-dump-openmp-atomic.c
clang/test/AST/ast-dump-openmp-critical.c
clang/test/AST/ast-dump-openmp-master.c
clang/test/AST/ast-dump-openmp-section.c
clang/test/OpenMP/atomic_messages.c
clang/test/OpenMP/atomic_messages.cpp
clang/test/OpenMP/critical_codegen.cpp
clang/test/OpenMP/critical_messages.cpp
clang/test/OpenMP/master_messages.cpp
clang/test/OpenMP/parallel_sections_misc_messages.c
clang/test/OpenMP/sections_misc_messages.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1aeaf590cbb4..2f4eb428dfad 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10353,6 +10353,10 @@ def err_omp_allocator_used_in_clauses : Error<
   "data-sharing or data-mapping attribute clauses">;
 def err_omp_allocator_not_in_uses_allocators : Error<
   "allocator must be specified in the 'uses_allocators' clause">;
+def note_omp_protected_structured_block
+: Note<"jump bypasses OpenMP structured block">;
+def note_omp_exits_structured_block
+: Note<"jump exits scope of OpenMP structured block">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index ea160025ae3d..1390876dc0df 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -647,7 +647,7 @@ void 
StmtPrinter::PrintOMPExecutableDirective(OMPExecutableDirective *S,
 }
   OS << NL;
   if (!ForceNoStmt && S->hasAssociatedStmt())
-PrintStmt(S->getInnermostCapturedStmt()->getCapturedStmt());
+PrintStmt(S->getRawStmt());
 }
 
 void StmtPrinter::VisitOMPParallelDirective(OMPParallelDirective *Node) {

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cde0bf448ecb..edc86c41c3b9 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4913,14 +4913,13 @@ CFGBlock 
*CFGBuilder::VisitOMPExecutableDirective(OMPExecutableDirective *D,
   B = R;
   }
   // Visit associated structured block if any.
-  if (!D->isStandaloneDirective())
-if (CapturedStmt *CS = D->getInnermostCapturedStmt()) {
-  Stmt *S = CS->getCapturedStmt();
-  if (!isa(S))
-addLocalScopeAndDtors(S);
-  if (CFGBlock *R = addStmt(S))
-B = R;
-}
+  if (!D->isStandaloneDirective()) {
+Stmt *S = D->getRawStmt();
+if (!isa(S))
+  addLocalScopeAndDtors(S);
+if (CFGBlock *R = addStmt(S))
+  B = R;
+  }
 
   return B;
 }

diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 4d88b39175ac..7a49fe02dada 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3554,12 +3554,9 @@ void CodeGenFunction::EmitOMPSectionsDirective(const 
OMPSectionsDirective &S) {
 }
 
 void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
-  auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &) {
-CGF.EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
-  };
-  OMPLexicalScope Scope(*this, S, OMPD_unknown);
-  CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_section, CodeGen,
-  S.hasCancel());
+  LexicalScope Scope(*this, S.getSourceRange());
+  EmitStopPoint(&S);
+  EmitStmt(S.getAssociatedStmt());
 }
 
 void CodeGenFunction::EmitOMPSingleDirective(const OMPSingleDirective &S) {
@@ -3610,7 +3607,7 @@ void CodeGenFunction::EmitOMPSingleDirective(const 
OMPSingleDirective &S) {
 static void emitMaster(CodeGenFunction &CGF, const OMPExecutableDirective &S) {
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
 Action.Enter(CGF);
-CGF.EmitStmt(S.getInnermostCapturedStmt()->getCapturedStmt());
+CGF.EmitStmt(S.getRawStmt());
   };
   CGF.CGM.getOpenMPRuntime().emitMasterRegion(CGF, CodeGen, S.getBeginLoc());
 }
@@ -3620,8 +3617,7 @@ void CodeGenFunction::EmitOMPMasterDirective(const 
OMPMasterDirective &S) {
 llvm:

[PATCH] D85144: [clang] Improve Dumping of APValues

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

riccibruno added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:222
+  /// Whether null pointers should be printed as nullptr or as NULL.
+  unsigned UseNullptr : 1;
+

riccibruno wrote:
> This seems to be unrelated. And anyway shouldn't this be inferred from the 
> language mode?
My bad. It is inferred from the language mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85144

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

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:806
 const auto &T = MI->tokens().back();
-StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-llvm::APInt IntValue;
-ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+if (T.isLiteral()) {
+  StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());

hokein wrote:
> let's add the `getLiteralData` check here --  `if (T.isLiteral() && 
> T.getLiteralData())`
Could we also add clangd test here for that case (when `T.isLiteral() == true` 
and `T.getLiteralData() == nullptr`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85525

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

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

2020-08-07 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c:3
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"

hokein wrote:
> I think you probably need a `// UNSUPPORTED: system-windows`, as the 
> `bugprone-not-null-terminated-result-strlen.c` does.
Seems we don't need this.  
Problem with `bugprone-not-null-terminated-result-strlen.c` test on Windows 
related with `strncmp`, according with comment.
```
// FIXME: Something wrong with the APInt un/signed conversion on Windows:
// in 'strncmp(str6, "string", 7);' it tries to inject '4294967302' as length.

// UNSUPPORTED: system-windows
```

E.g. `bugprone-not-null-terminated-result-memcpy-safe` also uses `strlen`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85525

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

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

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

steakhal created this revision.
steakhal added reviewers: NoQ, Szelethus, baloghadamsoftware, vsavchenko, 
xazax.hun, martong, ASDenysPetrov.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a project: clang.
steakhal requested review of this revision.

We ignored the cast if the enum was **scoped**.
This is bad since there is **no implicit conversion** from the scoped enum 
**to** the corresponding **underlying type**.

This materialized in crashes on analyzing the LLVM itself using the Z3 
refutation.
Refutation synthesized the given Z3 expression with the wrong bitwidth in the 
end.

Now, we evaluate the cast according to the standard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85528

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/test/Analysis/z3-refute-enum-crash.cpp


Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+
+void clang_analyzer_warnIfReached();
+
+using sugar_t = unsigned char;
+enum class ScopedSugared : sugar_t {};
+enum class ScopedPrimitive : unsigned char {};
+enum UnscopedSugared : sugar_t {};
+enum UnscopedPrimitive : unsigned char {};
+
+ScopedSugared conjure_scoped_enum_with_sugar_type();
+ScopedPrimitive conjure_scoped_enum_with_primitive_type();
+UnscopedSugared conjure_unscoped_enum_with_sugar_type();
+UnscopedPrimitive conjure_unscoped_enum_with_primitive_type();
+
+void test() {
+  auto var1 = conjure_scoped_enum_with_sugar_type();
+  auto var2 = conjure_scoped_enum_with_primitive_type();
+  auto var3 = conjure_unscoped_enum_with_sugar_type();
+  auto var4 = conjure_unscoped_enum_with_primitive_type();
+
+  int sym1 = static_cast(var1) & 0x0F;
+  if (sym1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym2 = static_cast(var2) & 0x0F;
+  if (sym2)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym3 = static_cast(var3) & 0x0F;
+  if (sym3)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym4 = static_cast(var4) & 0x0F;
+  if (sym4)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -95,11 +95,22 @@
   }
 
   bool haveSameType(QualType Ty1, QualType Ty2) {
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+  return Ty->isIntegralOrEnumerationType();
+};
+
 // FIXME: Remove the second disjunct when we support symbolic
 // truncation/extension.
-return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) ||
-(Ty1->isIntegralOrEnumerationType() &&
- Ty2->isIntegralOrEnumerationType()));
+const bool BothHaveSameCanonicalTypes =
+Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2);
+const bool BothHaveIntegralLikeTypes =
+IsIntegralOrUnscopedCompleteEnumerationType(Ty1) &&
+IsIntegralOrUnscopedCompleteEnumerationType(Ty2);
+return BothHaveSameCanonicalTypes || BothHaveIntegralLikeTypes;
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);


Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+
+void clang_analyzer_warnIfReached();
+
+using sugar_t = unsigned char;
+enum class ScopedSugared : sugar_t {};
+enum class ScopedPrimitive : unsigned char {};
+enum UnscopedSugared : sugar_t {};
+enum UnscopedPrimitive : unsigned char {};
+
+ScopedSugared conjure_scoped_enum_with_sugar_type();
+ScopedPrimitive conjure_scoped_enum_with_primitive_type();
+UnscopedSugared conjure_unscoped_enum_with_sugar_type();
+UnscopedPrimitive conjure_unscoped_enum_with_primitive_type();
+
+void test() {
+  auto var1 = conjure_scoped_enum_with_sugar_type();
+  auto var2 = conjure_scoped_enum_with_primitive_type();
+  auto var3 = conjure_unscoped_enum_with_sugar_type();
+  auto var4 = conjure_unscoped_enum_with_pr

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-08-07 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

In D72705#2199333 , @balazske wrote:

> Test results for tmux are available here 
> .
>  (Could not manage to get other results uploaded.)

I guess this is a somewhat smarter version, correct? You seem to have 
`NoteTag`s and handling for `localtime` etc. Anyways, I trust that the logic of 
mostly the same.

I don't know how likely it is for time management functions to fail, I guess 
its a rarity, but I guess this is exactly what static analysis is for. I think 
these results are nice, but as cheap of a response this is, I crave some more. 
You specifically mentioned a few false positives, I wonder what is up with them.

In D72705#2161576 , @balazske wrote:

> This checker can have two purposes, one is to verify that all paths have the 
> error check, other is to find at least one path without error check. The 
> first is the one that can be done with dataflow based analysis but looks like 
> a difficult problem. For example is it possible to handle this case (`X` does 
> not change)?
>
>   int Ret;
>   if (X)
> Ret = fgetc(fd);
>   else
> Ret = strtol(SomeString);
>   ...
>   bool IsError;
>   if (X)
> IsError = (Ret == EOF);
>   else
> IsError = (Ret < -100 || Ret > 100);
>
> The other "purpose" is to find one path with missing error check, the current 
> code should do something like that. The path sensitivity is used here to 
> store for a symbol from which function call it comes from, and probably to 
> determine value of other symbols in the program if they appear in a 
> comparison.

You are right, dataflow definitely has shortcomings that pathsensitivity 
doesn't, but it is true the other way around as well, which is why I believe 
(D72705#2141439 ) the combination of 
the two would be the correct solution, even if the question is whether the 
return value is checked on a given path of execution (that is precisely what 
the example in the comment demonstrates, not to mention that it could be far 
more obfuscated).

---

I think we have dedicated a lot of time to discuss our long term plans. I feel 
confident in my stance regarding the future of this checker, you seem to have a 
good grasp on it, we've gotten feedback a wide range of reviewers, and we also 
have this as a feature request outside Ericsson. Since this patch intends to 
lend an alpha checker, I think its about time we start talking about moving 
forward.

Here is what I like about this patch and want to see it in clang sooner rather 
than later:

- Documentation is great. Its hard to overstate the value of that, when I 
literally have to sink in weeks into a codebase to understand whats going on 
due to the lack of it.
- @NoQ mentioned the callback choice may not be ideal/wrong, but I personally 
disagree. Its far easier, and I think cheaper to climb up on the AST. The high 
level idea behind the visitor analyzing whether a statement constitutes as a 
check is great, I don't we have anything similar.
- I think you tested the added functionality for an alpha checker quite well.
- If my worries (did we think about all the  cases that could constitute as a 
check?) won't be an issue, the infrastructure proposed by this patch seems 
easily expandable.

Here is what I want to see moving forward:

- This comment D72705#2088319  worries 
me. @NoQ, could you please expand on this? I feel like you have a legitimate 
worry I'm not seeing, and I perceive you to be a bit skeptical about this whole 
shebang. While I'd hate to stall the progress of this patch, it'd be nice to be 
settled on this before moving too fast forward.
- More evaluations. I realize that reviewers don't demand too much on this 
front for an initial patch for an alpha checker, but both the problem and our 
different visions on the solution could use some backing up. It would be a 
shame if an issue we could've easily foreseen came about that would require 
serious architectural changes. What was the problem in uploading the results 
you already had on hand?

I left some inlines, but I didn't go too nitty just yet. For the time being, 
lets have these two goals met, and after that I'll definitely do my best to 
help you get this landed ASAP.

Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:55-60
+  /// Test if an encountered binary operator where the return value is involved
+  /// is a valid check statement. The return value appears in one side of the
+  /// operator (`ChildIsLHS` indicates if it is on the LHS). If the other side
+  /// contains a known (mostly constant) value, it is already calculated in
+  /// `Know

[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

2020-08-07 Thread Ole Strohm via Phabricator via cfe-commits

olestrohm updated this revision to Diff 283909.
olestrohm added a comment.

I have amended the relevant test to validate this behaviour in C++ for OpenCL.


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

https://reviews.llvm.org/D85429

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro_variadic.cl


Index: clang/test/Preprocessor/macro_variadic.cl
===
--- clang/test/Preprocessor/macro_variadic.cl
+++ clang/test/Preprocessor/macro_variadic.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -verify %s -cl-std=CL1.2
 // RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -cl-std=CLC++
+// RUN: %clang_cc1 -verify %s -pedantic -cl-std=CLC++
 
 
 #define NO_VAR_FUNC(...)  5
@@ -15,6 +17,11 @@
 
 void foo() {
   NO_VAR_FUNC(1, 2, 3);
-  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' 
is invalid in OpenCL}}
+  VAR_FUNC(1, 2, 3);
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2{{implicit declaration of function 'func' is invalid in 
OpenCL}}
+#else
+// expected-error@-4{{use of undeclared identifier 'func'}}
+#endif
   VAR_PRINTF("%i", 1);
 }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2394,7 +2394,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 


Index: clang/test/Preprocessor/macro_variadic.cl
===
--- clang/test/Preprocessor/macro_variadic.cl
+++ clang/test/Preprocessor/macro_variadic.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -verify %s -cl-std=CL1.2
 // RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -cl-std=CLC++
+// RUN: %clang_cc1 -verify %s -pedantic -cl-std=CLC++
 
 
 #define NO_VAR_FUNC(...)  5
@@ -15,6 +17,11 @@
 
 void foo() {
   NO_VAR_FUNC(1, 2, 3);
-  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' is invalid in OpenCL}}
+  VAR_FUNC(1, 2, 3);
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2{{implicit declaration of function 'func' is invalid in OpenCL}}
+#else
+// expected-error@-4{{use of undeclared identifier 'func'}}
+#endif
   VAR_PRINTF("%i", 1);
 }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2394,7 +2394,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

2020-08-07 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko added a comment.

Good catch!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+  return Ty->isIntegralOrEnumerationType();
+};

I don't really see any reasons why is this a lambda and not a free function



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+

I think it's better to add tests for this condition.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:106-107
+
 // FIXME: Remove the second disjunct when we support symbolic
 // truncation/extension.
+const bool BothHaveSameCanonicalTypes =

Maybe this comment should be moved closer to the `return` statement?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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

[PATCH] D85282: [Concepts] Dump template arguments for immediately declared constraint.

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

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

Ok, sounds reasonable to me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85282

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

[PATCH] D85431: [analyzer] Implement a new checker ThreadPrimitivesChecker

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

ASDenysPetrov updated this revision to Diff 283916.
ASDenysPetrov added a comment.

@vsavchenko 
Made changes due to your remarks. Thanks you.


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

https://reviews.llvm.org/D85431

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
  clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp

Index: clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.ThreadPrimitives -verify %s
+
+namespace std {
+struct mutex {
+  void lock();
+  void unlock();
+};
+template 
+struct guard_lock {
+  T &t;
+  guard_lock(T &m) : t(m) {
+t.lock();
+  }
+  ~guard_lock() {
+t.unlock();
+  }
+};
+} // namespace std
+
+void correct_lock_unlock(std::mutex m) {
+  m.lock();
+  m.unlock();
+}
+
+void incorrect_lock_unlock(std::mutex m1, std::mutex m2) {
+  m1.lock();
+  m2.unlock(); // expected-warning{{unlock without lock}}
+}
+
+void incorrect_lock_unlock2(std::mutex m, bool b) {
+  m.lock();
+  if (b)
+m.unlock();
+}
+
+void unlock_without_lock(std::mutex m) {
+  m.unlock(); // expected-warning{{unlock without lock}}
+}
+
+void twice_lock(std::mutex m) {
+  m.lock();
+  m.lock(); // expected-warning{{lock more than once}}
+}
+
+void twice_lock2(std::mutex m) {
+  while (true)
+m.lock(); // expected-warning{{lock more than once}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,111 @@
+//=== ConversionChecker.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker finds STL thread primitives misuse.
+// - std::mutex::unlock without std::mutex::lock
+// - std::mutex::lock twice
+//
+//===--===//
+#include "clang/AST/ParentMap.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/APFloat.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+enum MutexFunc {
+
+};
+
+class ThreadPrimitivesChecker : public Checker {
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+  BugType BT{this, "STL thread primitives misuse", "std::mutex misuse"};
+  CallDescription LockFunc{{"std", "mutex", "lock"}, 0, 0};
+  CallDescription UnlockFunc{{"std", "mutex", "unlock"}, 0, 0};
+
+  std::pair FindMutexLockOrUnlock(const CallEvent &Call,
+  CheckerContext &C) const;
+  void reportBug(CheckerContext &C, const char Msg[]) const;
+};
+
+} // namespace
+
+REGISTER_SET_WITH_PROGRAMSTATE(LockedMutexes, SVal)
+
+std::pair
+ThreadPrimitivesChecker::FindMutexLockOrUnlock(const CallEvent &Call,
+   CheckerContext &C) const {
+
+  if (const auto *MCall = dyn_cast(&Call)) {
+const bool IsLockFunc = MCall->isCalled(LockFunc);
+const bool IsUnlockFunc = IsLockFunc ? false : MCall->isCalled(UnlockFunc);
+return {IsLockFunc, IsUnlockFunc};
+  }
+  return {false, false};
+}
+
+void ThreadPrimitivesChecker::checkPostCall(const CallEvent &Call,
+CheckerContext &C) const {
+  // Find mutex::lock or mutex::unlock functions.
+  bool IsLockFunc;
+  bool IsUnlockFunc;
+  std::tie(IsLockFunc, IsUnlockFunc) = FindMutexLockOrUnlock(Call, C);
+
+  if (!IsLockFunc && !IsUnlockFunc)
+return;
+
+  // We are sure about cast here, because mutex::lock/unlock met before.
+  const auto *MCall = cast(&Call);
+  SVal SymVal = MCall->getCXXThisVal();
+  const bool LockFound = C.getState()->contains(SymVal);
+
+  if (LockFound) {
+if (IsLockFunc) {
+  reportBug(C, "Call mutex::lock more than once.");
+} else if (IsUnlockFunc) {
+  ProgramStateRef State = C.getState()->remove(SymVal);
+  C.a

[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-07 Thread Abhina Sreeskantharajan via Phabricator via cfe-commits

abhina.sreeskantharajan marked 2 inline comments as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:732
+Builder.defineMacro("_OPEN_DEFAULT");
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");

hubert.reinterpretcast wrote:
> This is not defined by z/OS XL C/C++. It seems that this is more of a macro 
> to be defined by a user application (perhaps as part of its 
> configuration/port for z/OS) and less a macro that should be predefined by 
> the compiler.
This is required for building libcxx, I'll add a comment here.



Comment at: clang/lib/Basic/Targets/OSTargets.h:750
+
+if (Opts.C11 || Opts.GNUMode)
+  Builder.defineMacro("__IBM_UTF_LITERAL");

hubert.reinterpretcast wrote:
> Shouldn't UTF literals be reported as being enabled under strict C++11?
I will get back to you on this. I will need to investigate whether we are 
keeping this macro.



Comment at: clang/lib/Basic/Targets/OSTargets.h:753
+
+if (Opts.C11 || (Opts.GNUMode && !Opts.CPlusPlus))
+  Builder.defineMacro("__IBMC_GENERIC");

hubert.reinterpretcast wrote:
> Is this consistent with `__has_extension` with respect to `-pedantic-errors`?
> That is, `-pedantic-errors` causes `__has_extension` to report the value that 
> `__has_feature` would report. Compiler Explorer link: 
> https://godbolt.org/z/EEn8rr
> 
> Same question for all of the other IBM-style feature test macros.
This macro is unused, so we are able to remove this.



Comment at: clang/lib/Basic/Targets/OSTargets.h:758
+  Builder.defineMacro("__DLL__");
+  // Macro __wchar_t exposes the definition of wchar_t data type
+  // in system headers.

hubert.reinterpretcast wrote:
> Should the comment instead say that `__wchar_t` should be defined so that the 
> system headers do not try to declare `wchar_t` as a typedef?
You're right. I'll update the comment to be more accurate.



Comment at: clang/lib/Basic/Targets/OSTargets.h:761
+  Builder.defineMacro("__wchar_t");
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}

hubert.reinterpretcast wrote:
> Same comment as before re: macros that should be declared by the application.
Same as above: This is required for building libcxx.



Comment at: clang/lib/Basic/Targets/OSTargets.h:764
+
+if (Opts.C11 || Opts.CPlusPlus11 || Opts.GNUMode) 
+  Builder.defineMacro("__IBMC_NORETURN");

hubert.reinterpretcast wrote:
> I don't see the relation between C++11 and `_Noreturn`. It's an extension in 
> C++ that's available under, e.g., `-std=c++03`.
Thanks, I will remove CPlusPlus11.



Comment at: clang/lib/Basic/Targets/OSTargets.h:767
+
+if (Opts.C11 || (Opts.GNUMode && Opts.CPlusPlus)) {
+  Builder.defineMacro("__IBM_CHAR16_T__");

hubert.reinterpretcast wrote:
> `char16_t` is not a keyword in C11, so `__IBM_CHAR16_T__` should not be 
> defined for C. Same re: `char32_t`.
> 
> Also, `char16_t` and `char32_t` are indeed keywords in C++11.
Right, these seem to only require C++, so I will update the check. But I will 
also follow up on whether we plan to keep these macros here.



Comment at: clang/lib/Basic/Targets/OSTargets.h:770
+  Builder.defineMacro("__IBM_CHAR32_T__");
+  Builder.defineMacro("__IBMCPP_UTF_LITERAL__");
+}

hubert.reinterpretcast wrote:
> The "IBMCPP" macro should not be defined in C modes.
See above: I will update this, but continue to investigate whether we need this 
macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-08-07 Thread Balázs Kéri via Phabricator via cfe-commits

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:55-60
+  /// Test if an encountered binary operator where the return value is involved
+  /// is a valid check statement. The return value appears in one side of the
+  /// operator (`ChildIsLHS` indicates if it is on the LHS). If the other side
+  /// contains a known (mostly constant) value, it is already calculated in
+  /// `KnownValue`. `RetTy` is the type of the return value (return type of the
+  /// function call in the code to check).

Szelethus wrote:
> So, for the time being we're saying that a checking must involve a binary 
> operator. That sounds about right, but I wonder if there is a case we're not 
> thinking about.
For the "fputs" case yes, comparison to EOF (or negative) can be detected by 
binary operator. The extended version has support for unary operator too (and 
some other tricks) (detect check of nullness of a value).



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:79-80
+  bool ChildIsLHS) const override {
+if (!KnownValue)
+  return false;
+

Szelethus wrote:
> So if we failed to get retrieve the value we're checking against, we 
> automatically assume that its not a proper check? Shouldn't we be 
> conservative here? What if that value is something like `getEOFValue()`?
This case needs attention, now it is detected as failure (maybe the function 
can return `Optional` to indicate a non-determinable case).



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:329-330
+  // Value appears in other statement.
+  // FIXME: Statements that affects control-flow should be checked separately.
+  // For example `Child` may appear as a condition of `if`.
+  VisitResult VisitStmt(const Stmt *S) { return StopExamineNoError; }

Szelethus wrote:
> Why?
This comment looks invalid now, can be removed. (`VisitIfStmt` already handles 
the case.)



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:428-432
+  // Check for explicit cast to void.
+  if (auto *Cast = dyn_cast(S)) {
+if (Cast->getTypeAsWritten().getTypePtr()->isVoidType())
+  return nullptr;
+  }

Szelethus wrote:
> Ah, okay, so you mean to check whether someone did something like this:
> ```lang=c++
> // Silence compiler warning.
> (void)localtime(...);
> ```
> 
> I don't think that the parent though will get you the correct answer, how 
> about this: 
> 
> ```lang=c++
> // Silence compiler warning.
> (void) (coin() ? NULL : localtime(...));
> ```
> 
> Please put a TODO here.
Probably this case can be handled by the `ErrorCheckTestStmtVisitor` in better 
way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

2020-08-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
mgorny.
Herald added a project: clang.
adamcz requested review of this revision.
Herald added a subscriber: ilya-biryukov.

This fixes a crash bug in clangd when used with modules. ASTWriter would
end up writing references to submodules into the PCH file, but upon
reading the submodules would not exists and
HeaderFileInfoTrait::ReadData would crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85532

Files:
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ModulesTests.cpp


Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,45 @@
+//===-- ModulesTests.cpp  ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+
+void foo() {}
+)cpp");
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fmodule-name=M");
+  TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+  TU.AdditionalFiles["Textual.h"] = "void foo();";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+  module Textual {
+textual header "Textual.h"
+  }
+}
+)modulemap";
+  // Test that we do not crash.
+  TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@
   IndexTests.cpp
   JSONTransportTests.cpp
   LSPClient.cpp
+  ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,45 @@
+//===-- ModulesTests.cpp  ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+
+void foo() {}
+)cpp");
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fmodule-name=M");
+  TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+  TU.AdditionalFiles["Textual.h"] = "void foo();";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+  module Textual {
+textual header "Textual.h"
+  }
+}
+)modulemap";
+  // Test that we do not crash.
+  TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@
   IndexTests.cpp
   JSONTransportTests.cpp
   LSPClient.cpp
+  ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

2020-08-07 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz updated this revision to Diff 283922.
adamcz added a comment.

This time the fix is included ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85532

Files:
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ModulesTests.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -208,6 +208,11 @@
 Callbacks.AfterPCHEmitted(Writer);
   }
 
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+CI.getLangOpts().CompilingPCH = true;
+return ASTFrontendAction::BeginSourceFileAction(CI);
+  }
+
   bool shouldEraseOutputFiles() override { return !hasEmittedPreamblePCH(); }
   bool hasCodeCompletionSupport() const override { return false; }
   bool hasASTFileSupport() const override { return false; }
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,45 @@
+//===-- ModulesTests.cpp  ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+
+void foo() {}
+)cpp");
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fmodule-name=M");
+  TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+  TU.AdditionalFiles["Textual.h"] = "void foo();";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+  module Textual {
+textual header "Textual.h"
+  }
+}
+)modulemap";
+  // Test that we do not crash.
+  TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@
   IndexTests.cpp
   JSONTransportTests.cpp
   LSPClient.cpp
+  ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -208,6 +208,11 @@
 Callbacks.AfterPCHEmitted(Writer);
   }
 
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+CI.getLangOpts().CompilingPCH = true;
+return ASTFrontendAction::BeginSourceFileAction(CI);
+  }
+
   bool shouldEraseOutputFiles() override { return !hasEmittedPreamblePCH(); }
   bool hasCodeCompletionSupport() const override { return false; }
   bool hasASTFileSupport() const override { return false; }
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,45 @@
+//===-- ModulesTests.cpp  ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+
+void foo() {}
+)cpp");
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fmodule-name=M");
+  TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+  TU.AdditionalFiles["Textual.h"] = "void foo();";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+  module Textual {
+textual header "Textual.h"
+  }
+}
+)modulemap";
+  // Test that we do not crash.
+  TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- c

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

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

MaskRay added a comment.

Added the tie breaker when progress cannot be made:)


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

https://reviews.llvm.org/D82081

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

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

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

steakhal planned changes to this revision.
steakhal added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+  return Ty->isIntegralOrEnumerationType();
+};

vsavchenko wrote:
> I don't really see any reasons why is this a lambda and not a free function
It's somewhat domain-specific that we require:
 - `bool-uint128` builtin types
 - complete, scoped `enum` types ((I think we need completeness for the 
analysis))

---
The one that comes quite close to these requirements was the 
`isIntegralOrEnumerationType`, but that does not check if the enum is 
//unscoped//.

We can extend the `Type` header with this.
Should we go on that route?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+

vsavchenko wrote:
> I think it's better to add tests for this condition.
You are right, I will add such cases as well.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:106-107
+
 // FIXME: Remove the second disjunct when we support symbolic
 // truncation/extension.
+const bool BothHaveSameCanonicalTypes =

vsavchenko wrote:
> Maybe this comment should be moved closer to the `return` statement?
Yes, I will move it closer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

2020-08-07 Thread Whisperity via Phabricator via cfe-commits

whisperity added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+  return Ty->isIntegralOrEnumerationType();
+};

steakhal wrote:
> vsavchenko wrote:
> > I don't really see any reasons why is this a lambda and not a free function
> It's somewhat domain-specific that we require:
>  - `bool-uint128` builtin types
>  - complete, scoped `enum` types ((I think we need completeness for the 
> analysis))
> 
> ---
> The one that comes quite close to these requirements was the 
> `isIntegralOrEnumerationType`, but that does not check if the enum is 
> //unscoped//.
> 
> We can extend the `Type` header with this.
> Should we go on that route?
I think they meant that this function should be a lambda, but a function inside 
the current translation unit (either in namespace anonymous, or a member 
function of SValBuilder).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

2020-08-07 Thread Whisperity via Phabricator via cfe-commits

whisperity added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+  return Ty->isIntegralOrEnumerationType();
+};

whisperity wrote:
> steakhal wrote:
> > vsavchenko wrote:
> > > I don't really see any reasons why is this a lambda and not a free 
> > > function
> > It's somewhat domain-specific that we require:
> >  - `bool-uint128` builtin types
> >  - complete, scoped `enum` types ((I think we need completeness for the 
> > analysis))
> > 
> > ---
> > The one that comes quite close to these requirements was the 
> > `isIntegralOrEnumerationType`, but that does not check if the enum is 
> > //unscoped//.
> > 
> > We can extend the `Type` header with this.
> > Should we go on that route?
> I think they meant that this function should be a lambda, but a function 
> inside the current translation unit (either in namespace anonymous, or a 
> member function of SValBuilder).
Edit: should //not// be
It does not have to go into `Type.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

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

kadircet added a comment.

Regarding tests, it feels like we can also test this in ASTUnitTests which is 
directly in clang, as it is also using PrecompiledPreamble::Build. What about 
moving the test there instead?




Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp:24
+  TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+

nit: indent the string literals by at least 4 spaces. (that's what other tests 
look like, same below)



Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp:28
+)cpp");
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fmodule-name=M");

this one is not needed right?



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:421
   StoreInMemory ? &Storage.asMemory().Data : nullptr, Callbacks));
   Callbacks.BeforeExecute(*Clang);
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))

anything operating on CompilerInstance before and including the callbacks 
invoked here will see a broken LangOtps. Why not set it here (and maybe assert 
in the override, or just not do anything at all)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85532

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

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

2020-08-07 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

I confirm that I have reviewed the Clang part of this change, which is entirely 
consistent with the status quo for XCOFF. I'm not seeing any outstanding 
comments, and I do not believe that the changes are controversial. I think this 
patch meets the "likely community consensus" requirements for being committed.


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

https://reviews.llvm.org/D82081

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

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

2020-08-07 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann accepted this revision.
tahonermann added a comment.

I'm not a regular Clang reviewer.  But for what it is worth, the changes look 
correct, clear, and appropriate from my lens (though I disagree slightly with 
some of the lint recommendations in the cases where the recommendation deviates 
from the surrounding code style).  It looks like the concerns Hubert raised 
have been addressed.


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

https://reviews.llvm.org/D82081

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

[PATCH] D85536: [clang] Add a matcher for template template parameters.

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

riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, klimek, njames93.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno requested review of this revision.

There are already matchers for type template parameters and non-type template 
parameters, but somehow no matcher exists for template template parameters and 
I need it to write unit tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85536

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -408,6 +408,15 @@
   EXPECT_TRUE(notMatches("template  void f();", 
templateTypeParmDecl()));
 }
 
+TEST_P(ASTMatchersTest, TemplateTemplateParmDecl) {
+  if (!GetParam().isCXX())
+return;
+  EXPECT_TRUE(matches("template  class Z> void f();",
+  templateTemplateParmDecl(hasName("Z";
+  EXPECT_TRUE(notMatches("template  void f();",
+ templateTemplateParmDecl()));
+}
+
 TEST_P(ASTMatchersTest, UserDefinedLiteral) {
   if (!GetParam().isCXX11OrLater()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -514,6 +514,7 @@
   REGISTER_MATCHER(templateArgumentCountIs);
   REGISTER_MATCHER(templateName);
   REGISTER_MATCHER(templateSpecializationType);
+  REGISTER_MATCHER(templateTemplateParmDecl);
   REGISTER_MATCHER(templateTypeParmDecl);
   REGISTER_MATCHER(templateTypeParmType);
   REGISTER_MATCHER(throughUsingDecl);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -739,6 +739,9 @@
 nonTypeTemplateParmDecl;
 const internal::VariadicDynCastAllOfMatcher
 templateTypeParmDecl;
+const internal::VariadicDynCastAllOfMatcher
+templateTemplateParmDecl;
+
 const internal::VariadicAllOfMatcher qualType;
 const internal::VariadicAllOfMatcher type;
 const internal::VariadicAllOfMatcher typeLoc;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -549,6 +549,18 @@
 extern const internal::VariadicDynCastAllOfMatcher
 templateTypeParmDecl;
 
+/// Matches template template parameter declarations.
+///
+/// Given
+/// \code
+///   template  class Z, int N> struct C {};
+/// \endcode
+/// templateTypeParmDecl()
+///   matches 'Z', but not 'N'.
+extern const internal::VariadicDynCastAllOfMatcher
+templateTemplateParmDecl;
+
 /// Matches public C++ declarations and C++ base specifers that specify public
 /// inheritance.
 ///


Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -408,6 +408,15 @@
   EXPECT_TRUE(notMatches("template  void f();", templateTypeParmDecl()));
 }
 
+TEST_P(ASTMatchersTest, TemplateTemplateParmDecl) {
+  if (!GetParam().isCXX())
+return;
+  EXPECT_TRUE(matches("template  class Z> void f();",
+  templateTemplateParmDecl(hasName("Z";
+  EXPECT_TRUE(notMatches("template  void f();",
+ templateTemplateParmDecl()));
+}
+
 TEST_P(ASTMatchersTest, UserDefinedLiteral) {
   if (!GetParam().isCXX11OrLater()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -514,6 +514,7 @@
   REGISTER_MATCHER(templateArgumentCountIs);
   REGISTER_MATCHER(templateName);
   REGISTER_MATCHER(templateSpecializationType);
+  REGISTER_MATCHER(templateTemplateParmDecl);
   REGISTER_MATCHER(templateTypeParmDecl);
   REGISTER_MATCHER(templateTypeParmType);
   REGISTER_MATCHER(throughUsingDecl);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -739,6 +739,9 @@
 nonTypeTemplateParmDecl;
 const internal::VariadicDynCastAllOfMatcher
 templateTypeParmDecl;
+const internal::VariadicDynCastAllOfMatcher
+templateTemplateParmDecl;
+
 const internal::Varia

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

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

steakhal updated this revision to Diff 283936.
steakhal added a comment.

- Moved the FIXME closer to the subject.
- Added tests for covering incomplete enums as well.
- Added `REQUIRES: z3`. This will mark the test case `unsupported` on every 
buildbots. See my notes about this behavior at D83677 
.
- Refined test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/test/Analysis/z3-refute-enum-crash.cpp

Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+//
+// Requires z3 only for refutation. Works with both constraint managers.
+
+void clang_analyzer_warnIfReached();
+
+using sugar_t = unsigned char;
+
+// Complete enum types
+enum class ScopedSugaredComplete : sugar_t {};
+enum class ScopedPrimitiveComplete : unsigned char {};
+enum UnscopedSugaredComplete : sugar_t {};
+enum UnscopedPrimitiveComplete : unsigned char {};
+
+// Incomplete enum types
+enum class ScopedSugaredIncomplete : sugar_t;
+enum class ScopedPrimitiveIncomplete : unsigned char;
+enum UnscopedSugaredIncomplete : sugar_t;
+enum UnscopedPrimitiveIncomplete : unsigned char;
+
+template 
+T conjure();
+
+void test_complete_enum_types() {
+  auto var1 = conjure();
+  auto var2 = conjure();
+  auto var3 = conjure();
+  auto var4 = conjure();
+
+  int sym1 = static_cast(var1) & 0x0F;
+  if (sym1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym2 = static_cast(var2) & 0x0F;
+  if (sym2)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym3 = static_cast(var3) & 0x0F;
+  if (sym3)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym4 = static_cast(var4) & 0x0F;
+  if (sym4)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+}
+
+void test_incomplete_enum_types() {
+  auto var1 = conjure();
+  auto var2 = conjure();
+  auto var3 = conjure();
+  auto var4 = conjure();
+
+  int sym1 = static_cast(var1) & 0x0F;
+  if (sym1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym2 = static_cast(var2) & 0x0F;
+  if (sym2)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym3 = static_cast(var3) & 0x0F;
+  if (sym3)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym4 = static_cast(var4) & 0x0F;
+  if (sym4)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -95,11 +95,22 @@
   }
 
   bool haveSameType(QualType Ty1, QualType Ty2) {
+const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+  const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+  if (const auto *ET = dyn_cast(CanonicalType))
+return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+  return Ty->isIntegralOrEnumerationType();
+};
+
+const bool BothHaveSameCanonicalTypes =
+Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2);
+const bool BothHaveIntegralLikeTypes =
+IsIntegralOrUnscopedCompleteEnumerationType(Ty1) &&
+IsIntegralOrUnscopedCompleteEnumerationType(Ty2);
+
 // FIXME: Remove the second disjunct when we support symbolic
 // truncation/extension.
-return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) ||
-(Ty1->isIntegralOrEnumerationType() &&
- Ty2->isIntegralOrEnumerationType()));
+return BothHaveSameCanonicalTypes || BothHaveIntegralLikeTypes;
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

2020-08-07 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D75574#2202136 , @theraven wrote:

> This feature looks generally useful.  A few small suggestions:
>
> - This is really a way of transforming a formal protocol into an informal 
> protocol.  Objective-C has had a convention of informal protocols since the 
> '80s, but they're implemented as categories on the root class with no 
> `@implementation`.  I'd suggest that 
> `__attribute__((objc_informal_protocol))` or similar might be a better 
> spelling for this, explicitly bringing the informal notion into the language. 
>  A lot of the informal protocols in Cocoa could be better expressed using 
> this and `@optional` methods than as categories on `NSObject`.

It's still a formal protocol, it just doesn't have runtime representation.  I 
think the name is appropriate.  It's an interesting point that some of the 
informal protocols could be formalized without penalty using this, though.

> - Given that this doesn't depend on any features in the runtime (from the 
> runtime's perspective, the protocol doesn't exist), I don't think it makes 
> sense to have an `ObjCRuntime` method to query whether this is supported by 
> the runtime.  We should enable it everywhere if it's going in anywhere.

Agreed.

> - The changes required in CGObjcCGNU.cpp are fairly small and I agree that 
> @rjmccall's proposal  for a callback-driven visitor would simplify the 
> changes in both runtimes.
> - The semantics are slightly confusing with the deep approach though.  
> Normally, if you iterate over the protocols that a class conforms to, you 
> only see the ones that it directly conforms to.  With this model, you'd see 
> indirect ones.  We might want to set some metadata to allow programmers to 
> differentiate the two, or we might want to have a warning (off by default?) 
> if an informal protocol conforms to a formal one, or simply disallow it.

I don't see why this information would be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

1 2 >

1 - 100 of 191 matches

Mail list logo