[PATCH] D80784: [clangd][NFC] Explode ReceivedPreamble into a CV

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:661
+std::unique_lock Lock(Mutex);
+RequestsCV.wait(Lock, [this] {
+  // Block until we reiceve a preamble request, unless a preamble already

kadircet wrote:
> sammccall wrote:
> > Does LatestPreamble signal RequestsCV or just PreambleCV?
> > 
> > Seems like it might be less error-prone to have just one CV, signalled when 
> > preamble requests are scheduled, latest preamble becomes available, and on 
> > shutdown. The spurious wakeups shouldn't be a real problem, right?
> > Does LatestPreamble signal RequestsCV or just PreambleCV?
> 
> it only signals `PreambleCV`. But it makes no sense for this code path to 
> block on it, as it is signalled by the same thread. So in theory we should 
> see `LatestPreamble` and not start blocking in such case.
> 
> > Seems like it might be less error-prone to have just one CV, signalled when 
> > preamble requests are scheduled, latest preamble becomes available, and on 
> > shutdown. The spurious wakeups shouldn't be a real problem, right?
> 
> Yeah spurious wake-ups aren't really a problem. I thought having separate CVs 
> sounded more clear, as predicates would still look the same even if we only 
> had one CV. So it would enforce us to signal/wait on the right condition 
> variable.
> 
> Happy to have only a single one too, do you have any suggestions for its name?
Yeah, I think that having the CV not cover all the conditions, with correctness 
due to what threads the conditions are set on, is a bit too subtle.
I'd prefer a single CV named `PreambleCV` with an appropriate comment seems 
clear, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80784



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

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Let's land this and do further refactoring/renames later.
As you mentioned, it's not possible to inline some of the cases where raw 
pointers are e.g. stored in structs due to lifetime issues. I expect many of 
these to go away once we use FSProvider more pervasively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81173



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

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Yeah, I think this is consistent with the naming/doc of 
IgnoreUnlessSpelledInSource, and the concerns are more about the implications 
of making a less-powerful, more conservative version of matchers the default.
Which isn't really what *this* patch is about, but it's reasonable to be 
concerned at how much functionality is being "buried" beneath a pretty awkward 
API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

I'm not a big objC expert here. The idea looks fine to me and won't affect my 
workflow. So let's take this patch if nobody comments against it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263



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

[PATCH] D81154: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ComputeDependence.cpp:131
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
   // FIXME: why is unexpanded-pack not propagated?
   auto D = toExprDependence(E->getType()->getDependence()) &

I think I know the answer now - you can't expand a param pack over stmtexpr 
boundaries. Could repace this comment...



Comment at: clang/lib/AST/ComputeDependence.cpp:132
   // FIXME: why is unexpanded-pack not propagated?
   auto D = toExprDependence(E->getType()->getDependence()) &
~ExprDependence::UnexpandedPack;

I wonder if this is still needed - how can we get a type for the stmtexpr 
unless we have a result expr providing the type?



Comment at: clang/lib/AST/ComputeDependence.cpp:134
~ExprDependence::UnexpandedPack;
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of

Nit: i'd probably leave this as the last clause as it's the weird edge case.



Comment at: clang/lib/AST/ComputeDependence.cpp:140
+
+  // Populate dependence bits from the expr that consider to be the result
+  // of the compound statements.

Maybe just "propagate dependence of the result"? The compound statement seems 
like a red herring.



Comment at: clang/lib/AST/ComputeDependence.cpp:145
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   return D;

this should avoid propagating the pack bit as above (or factor it out and mask 
it off at the end to avoid writing it twice)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81154



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

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5153
 
-  auto DoCompletion = [&](Expr *Base, bool IsArrow,
+  auto DoCompletion = [&](Expr *Base, bool IsArrow, bool IncludeObjC,
   Optional AccessOpFixIt) -> bool {

IncludeObjC is always `!AccessOpFixIt.hasValue()` - use that directly with a 
suitable comment?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5251
+
+  // Fixits are only included for C++. We avoid this for Objective-C properties
+  // and ivars since most properties are backed by an ivar; otherwise we would

This first sentence isn't accurate - they're included for C, and even in obj-c, 
just not for obj-c objects.
I'd suggest dropping it and also the note about alternative solutions, unless 
there's a reason we're likely to want to do that instead in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263



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

[clang] c6ec352 - Revert "[KernelAddressSanitizer] Make globals constructors compatible with kernel"

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-06-08T10:34:03+02:00
New Revision: c6ec352a6bde1995794c523adc2ebab802ccdf0a

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

LOG: Revert "[KernelAddressSanitizer] Make globals constructors compatible with 
kernel"

This reverts commit 866ee2353f7d0224644799d0d1faed53c7f3a06d.

Building the kernel results in modpost failures due to modpost relying
on debug info and inspecting kernel modules' globals:
https://github.com/ClangBuiltLinux/linux/issues/1045#issuecomment-640381783

Added: 


Modified: 
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 




diff  --git a/clang/test/CodeGen/asan-globals.cpp 
b/clang/test/CodeGen/asan-globals.cpp
index a5374caae5c4..93abb0023cfa 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -1,59 +1,40 @@
 // RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,ASAN
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,KASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
 // The blacklist file uses regexps, so Windows path backslashes.
 // RUN: echo "src:%s" | sed -e 's/\\//g' > %t.blacklist-src
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address 
-fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s 
--check-prefix=BLACKLIST-SRC
 
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
 int blacklisted_global;
-int __attribute__ ((section("__DATA, __common"))) sectioned_global;
 
 void func() {
   static int static_var = 0;
   const char *literal = "Hello, world!";
 }
 
-// CHECK-LABEL: define internal void @asan.module_ctor
-// ASAN-NEXT: call void @__asan_init
-// ASAN-NEXT: call void @__asan_version_mismatch_check
-// KASAN-NOT: call void @__asan_init
-// KASAN-NOT: call void @__asan_version_mismatch_check
-// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 6)
-// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
-// CHECK-NEXT: ret void
-
-// CHECK-LABEL: define internal void @asan.module_dtor
-// CHECK-NEXT: call void @__asan_unregister_globals
-// CHECK-NEXT: ret void
-
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], 
![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], 
![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], 
![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], 
!"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, 
i1 false}
-// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
+// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], 
!"dyn_init_global", i1 true, i1 false}
-// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
+// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 5}
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], 
!"sectioned_global", i1 false, i1 false}
-// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 
51}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 
false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-glo

[PATCH] D81280: (Incomplete)Add support for id-expression

[Eduardo Caldas via Phabricator via cfe-commits]

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

Add support for DeclRefExpr in SyntaxTree, by generating IdExpression


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81280

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
@@ -485,7 +485,7 @@
 | | |-SimpleDeclarator
 | | | `-x
 | | `-:
-| |-UnknownExpression
+| |-IdExpression
 | | `-a
 | |-)
 | `-EmptyStatement
@@ -662,7 +662,7 @@
 |-{
 |-ExpressionStatement
 | |-UnknownExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-test
 | | |-(
 | | `-)
@@ -675,7 +675,7 @@
 | |-)
 | |-ExpressionStatement
 | | |-UnknownExpression
-| | | |-UnknownExpression
+| | | |-IdExpression
 | | | | `-test
 | | | |-(
 | | | `-)
@@ -683,7 +683,7 @@
 | |-else
 | `-ExpressionStatement
 |   |-UnknownExpression
-|   | |-UnknownExpression
+|   | |-IdExpression
 |   | | `-test
 |   | |-(
 |   | `-)
@@ -724,12 +724,10 @@
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-a
+| | | `-a
 | | |-=
 | | `-IdExpression
-| |   `-UnqualifiedId
-| | `-b
+| |   `-b
 | `-;
 `-}
 )txt"));
@@ -800,38 +798,22 @@
   |   `-)
   `-CompoundStatement
 |-{
-|-DeclarationStatement
-| |-SimpleDeclaration
-| | |-a
-| | |-::
-| | |-b
-| | |-::
-| | |-S
-| | `-SimpleDeclarator
-| |   `-UnknownExpression
-| | `-s
-| `-;
 |-ExpressionStatement
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameQualifier
-| | | | | |-GlobalNamespace
-| | | | | `::
-| | | | |-NameQualifier
-| | | | | |-NamespaceName
-| | | | | | `-a
+| | | | |-NameSpecifier
+| | | | | `-::
+| | | | |-NameSpecifier
+| | | | | |-a
 | | | | | `-::
-| | | | |-NameQualifier
-| | | | | |-NamespaceName
-| | | | | | `-b
+| | | | |-NameSpecifier
+| | | | | |-b
 | | | | | `-::
-| | | | `-NameQualifier
-| | | |   |-TypeName
-| | | |   | `-S
+| | | | `-NameSpecifier
+| | | |   |-S
 | | | |   `-::
-| | | `-UnqualifiedId
-| | |   `-f
+| | | `-f
 | | |-(
 | | `-)
 | `-;
@@ -1036,13 +1018,13 @@
 |-{
 |-ExpressionStatement
 | |-PostfixUnaryOperatorExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-a
 | | `-++
 | `-;
 |-ExpressionStatement
 | |-PostfixUnaryOperatorExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-a
 | | `---
 | `-;
@@ -1088,61 +1070,61 @@
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |---
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-++
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-~
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |--
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-+
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-&
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-*
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-ap
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-!
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-__real
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-__imag
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 `-}
@@ -1183,13 +1165,13 @@
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-compl
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpres

[PATCH] D81366: Recognize *.hxx as a C++ header extension, like *.hpp.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81366

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/FrontendOptions.cpp


Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -25,7 +25,7 @@
   .Cases("mm", "M", Language::ObjCXX)
   .Case("mii", InputKind(Language::ObjCXX).getPreprocessed())
   .Cases("C", "cc", "cp", Language::CXX)
-  .Cases("cpp", "CPP", "c++", "cxx", "hpp", Language::CXX)
+  .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
   .Case("iim", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -278,6 +278,7 @@
.Case("gch", TY_PCH)
.Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
+   .Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)


Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -25,7 +25,7 @@
   .Cases("mm", "M", Language::ObjCXX)
   .Case("mii", InputKind(Language::ObjCXX).getPreprocessed())
   .Cases("C", "cc", "cp", Language::CXX)
-  .Cases("cpp", "CPP", "c++", "cxx", "hpp", Language::CXX)
+  .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
   .Case("iim", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -278,6 +278,7 @@
.Case("gch", TY_PCH)
.Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
+   .Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81346: [KernelAddressSanitizer] Ensure global array size remains multiple of type-size

[Marco Elver via Phabricator via cfe-commits]

melver abandoned this revision.
melver added a comment.

With an allmodconfig, mostpost still isn't happy because it expects the 
device_id info to be a certain size. Revert everything while we figure out how 
to make modpost happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81346



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

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D80961#2076242 , @aaron.ballman 
wrote:

> In D80961#2073049 , @klimek wrote:
>
> > Without jumping into the discussion whether it should be the default, I 
> > think we should be able to control template instantiation visitation 
> > separately from other implicit nodes.
> >  Having to put AsIs on a matcher every time you need to match template 
> > instantiations is a rather big change (suddenly you have to change all the 
> > matchers you've written so far).
>
>
> I think that's the intended meaning of `AsIs` though. Template instantiations 
> are not source code the user wrote, they're source code the compiler stamped 
> out from code the user wrote. I hope `IgnoreUnlessSpelledInSource` isn't a 
> misnomer.
>
> > I love the idea of being able to control visitation of template 
> > instantiation.
> >  I am somewhat torn on whether it should be the default, and would like to 
> > see more data.
> >  I feel more strongly about needing AsIs when I want to match template 
> > instantiations.
>
> FWIW, my experience in clang-tidy has been that template instantiations are 
> ignored far more often than they're desired. In fact, instantiations tend to 
> be a source of bugs for us because they're easy to forget about when writing 
> matchers without keeping templates in mind. The times when template 
> instantiations become important to *not* ignore within the checks is when the 
> check is specific to template behavior, but that's a minority of the public 
> checks thus far.


So I assume the idea is that this will work & be what we'll want people to 
write?
traverse(AsIs, decl(traverse(IgnoreImplicit, hasFoo)))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm




Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:3
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
+// RUN: %clang_cl /c /Z7 %s /Fo%t.obj -fdebug-compilation-dir .
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix RELATIVE

This one also needs "-- %s" at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

[Eduardo Caldas via Phabricator via cfe-commits]

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

cleanup for upstreaming


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81168

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
@@ -485,7 +485,7 @@
 | | |-SimpleDeclarator
 | | | `-x
 | | `-:
-| |-UnknownExpression
+| |-IdExpression
 | | `-a
 | |-)
 | `-EmptyStatement
@@ -662,7 +662,7 @@
 |-{
 |-ExpressionStatement
 | |-UnknownExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-test
 | | |-(
 | | `-)
@@ -675,7 +675,7 @@
 | |-)
 | |-ExpressionStatement
 | | |-UnknownExpression
-| | | |-UnknownExpression
+| | | |-IdExpression
 | | | | `-test
 | | | |-(
 | | | `-)
@@ -683,7 +683,7 @@
 | |-else
 | `-ExpressionStatement
 |   |-UnknownExpression
-|   | |-UnknownExpression
+|   | |-IdExpression
 |   | | `-test
 |   | |-(
 |   | `-)
@@ -692,6 +692,135 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, UnqualifiedId) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test(int b) {
+  int a;
+  a = b;
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-b
+  |   `-)
+  `-CompoundStatement
+|-{
+|-DeclarationStatement
+| |-SimpleDeclaration
+| | |-int
+| | `-SimpleDeclarator
+| |   `-a
+| `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-a
+| | |-=
+| | `-IdExpression
+| |   `-b
+| `-;
+`-}
+)txt"));
+}
+
+TEST_P(SyntaxTreeTest, QualifiedId) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+namespace a {
+  namespace b {
+struct S {
+  int i;
+  static void f(){}
+};
+  }
+}
+void test(int b) {
+  ::a::b::S::f();
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-NamespaceDefinition
+| |-namespace
+| |-a
+| |-{
+| |-NamespaceDefinition
+| | |-namespace
+| | |-b
+| | |-{
+| | |-SimpleDeclaration
+| | | |-struct
+| | | |-S
+| | | |-{
+| | | |-SimpleDeclaration
+| | | | |-int
+| | | | |-SimpleDeclarator
+| | | | | `-i
+| | | | `-;
+| | | |-SimpleDeclaration
+| | | | |-static
+| | | | |-void
+| | | | |-SimpleDeclarator
+| | | | | |-f
+| | | | | `-ParametersAndQualifiers
+| | | | |   |-(
+| | | | |   `-)
+| | | | `-CompoundStatement
+| | | |   |-{
+| | | |   `-}
+| | | |-}
+| | | `-;
+| | `-}
+| `-}
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-b
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-NameSpecifier
+| | | | | `-::
+| | | | |-NameSpecifier
+| | | | | |-a
+| | | | | `-::
+| | | | |-NameSpecifier
+| | | | | |-b
+| | | | | `-::
+| | | | `-NameSpecifier
+| | | |   |-S
+| | | |   `-::
+| | | `-f
+| | |-(
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, CxxNullPtrLiteral) {
   if (!GetParam().isCXX11OrLater()) {
 return;
@@ -889,13 +1018,13 @@
 |-{
 |-ExpressionStatement
 | |-PostfixUnaryOperatorExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-a
 | | `-++
 | `-;
 |-ExpressionStatement
 | |-PostfixUnaryOperatorExpression
-| | |-UnknownExpression
+| | |-IdExpression
 | | | `-a
 | | `---
 | `-;
@@ -941,61 +1070,61 @@
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |---
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-++
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-~
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |--
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpression
 | | |-+
-| | `-UnknownExpression
+| | `-IdExpression
 | |   `-a
 | `-;
 |-ExpressionStatement
 | |-PrefixUnaryOperatorExpress

[PATCH] D81304: [llvm][SveEmitter] Emit the bfloat version of `svld1ro`.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks @fpetrogalli, the changes look good to me. Just added some minor 
comments.




Comment at: clang/include/clang/Basic/AArch64SVEACLETypes.def:63
 
-SVE_VECTOR_TYPE("__SVFloat16_t", SveFloat16, SveFloat16Ty, 8, 16, true, true)
-SVE_VECTOR_TYPE("__SVFloat32_t", SveFloat32, SveFloat32Ty, 4, 32, true, true)
-SVE_VECTOR_TYPE("__SVFloat64_t", SveFloat64, SveFloat64Ty, 2, 64, true, true)
+SVE_VECTOR_TYPE("__SVBFloat16_t", SveBFloat16, SveBFloat16Ty, 8, 16, false, 
false, true)
+SVE_VECTOR_TYPE("__SVFloat16_t", SveFloat16, SveFloat16Ty, 8, 16, true, true, 
false)

nit: I'd move this below __SVFloat64_t separated by a newline.



Comment at: clang/utils/TableGen/SveEmitter.cpp:366
 S += "v";
-  else if (!Float)
+  else if (!isFloat() && !isBFloat())
 switch (ElementBitwidth) {

Maybe it's worth adding a
  bool isFloatingPoint() const { return Float || BFloat; }
to avoid having to write both `isFloat` and `isBFloat` everywhere.



Comment at: clang/utils/TableGen/SveEmitter.cpp:385
+switch (ElementBitwidth) {
+case 16: S += "y"; break;
+default: llvm_unreachable("Unhandled case!");

It's easier to just assert that ElementBitwidth == 16.



Comment at: clang/utils/TableGen/SveEmitter.cpp:942
+  if (T.isBFloat()) {
+switch (T.getElementSizeInBits()) {
+case 16:

It's easier to just assert that ElementBitwidth == 16


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81304



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

[clang] 9c39095 - Recognize *.hxx as a C++ header extension, like *.hpp.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-06-08T11:50:34+02:00
New Revision: 9c3909556b31e1ca5c63ba0a54db69e298b08b1a

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

LOG: Recognize *.hxx as a C++ header extension, like *.hpp.

Reviewers: kadircet

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Driver/Types.cpp
clang/lib/Frontend/FrontendOptions.cpp

Removed: 




diff  --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index 3ab48a0180c1..399e26d8d64a 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -278,6 +278,7 @@ types::ID types::lookupTypeForExtension(llvm::StringRef 
Ext) {
.Case("gch", TY_PCH)
.Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
+   .Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)

diff  --git a/clang/lib/Frontend/FrontendOptions.cpp 
b/clang/lib/Frontend/FrontendOptions.cpp
index 63088b95c310..9f080db733f1 100644
--- a/clang/lib/Frontend/FrontendOptions.cpp
+++ b/clang/lib/Frontend/FrontendOptions.cpp
@@ -25,7 +25,7 @@ InputKind FrontendOptions::getInputKindForExtension(StringRef 
Extension) {
   .Cases("mm", "M", Language::ObjCXX)
   .Case("mii", InputKind(Language::ObjCXX).getPreprocessed())
   .Cases("C", "cc", "cp", Language::CXX)
-  .Cases("cpp", "CPP", "c++", "cxx", "hpp", Language::CXX)
+  .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
   .Case("iim", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)



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

[PATCH] D81366: Recognize *.hxx as a C++ header extension, like *.hpp.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c3909556b31: Recognize *.hxx as a C++ header extension, 
like *.hpp. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81366

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/FrontendOptions.cpp


Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -25,7 +25,7 @@
   .Cases("mm", "M", Language::ObjCXX)
   .Case("mii", InputKind(Language::ObjCXX).getPreprocessed())
   .Cases("C", "cc", "cp", Language::CXX)
-  .Cases("cpp", "CPP", "c++", "cxx", "hpp", Language::CXX)
+  .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
   .Case("iim", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -278,6 +278,7 @@
.Case("gch", TY_PCH)
.Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
+   .Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)


Index: clang/lib/Frontend/FrontendOptions.cpp
===
--- clang/lib/Frontend/FrontendOptions.cpp
+++ clang/lib/Frontend/FrontendOptions.cpp
@@ -25,7 +25,7 @@
   .Cases("mm", "M", Language::ObjCXX)
   .Case("mii", InputKind(Language::ObjCXX).getPreprocessed())
   .Cases("C", "cc", "cp", Language::CXX)
-  .Cases("cpp", "CPP", "c++", "cxx", "hpp", Language::CXX)
+  .Cases("cpp", "CPP", "c++", "cxx", "hpp", "hxx", Language::CXX)
   .Case("cppm", Language::CXX)
   .Case("iim", InputKind(Language::CXX).getPreprocessed())
   .Case("cl", Language::OpenCL)
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -278,6 +278,7 @@
.Case("gch", TY_PCH)
.Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
+   .Case("hxx", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
.Case("mii", TY_PP_ObjCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] abca3b7 - Revert "[clang] Implement VectorType logic not operator."

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-06-08T06:45:21-04:00
New Revision: abca3b7b2ce539ce22c9d3ebdd3cb6c02bb4c009

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

LOG: Revert "[clang] Implement VectorType logic not operator."

This reverts commit a0de3335edcf19305dad592d21ebe402825184f6.
Breaks check-clang on Windows, see e.g.
https://reviews.llvm.org/D80979#2078750 (but fails on all
other Windows bots too).

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/vector-gcc-compat.cpp

Removed: 
clang/test/CodeGen/vector-logic-not.cpp



diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 06ecc186c7dc..ba0a7d9cf95c 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -475,7 +475,7 @@ unary operators +, --yes yes   yes  
   --
 +,--,*,/,%   yes yes   yes --
 bitwise operators &,|,^,~yes yes   yes --
 >>,<, <, >=, <= yes yes   yes --
 =yes yes   yes yes
 ?: [#]_  yes --yes --
@@ -488,6 +488,7 @@ const_cast   no  nono   
   no
 
 See also :ref:`langext-__builtin_shufflevector`, 
:ref:`langext-__builtin_convertvector`.
 
+.. [#] unary operator ! is not implemented, however && and || are.
 .. [#] ternary operator(?:) has 
diff erent behaviors depending on condition
   operand's vector type. If the condition is a GNU vector (i.e. 
__vector_size__),
   it's only available in C++ and uses normal bool conversions (that is, != 0).

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index b2bc38b329ef..4e61349cf4d5 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2762,9 +2762,7 @@ Value *ScalarExprEmitter::VisitUnaryNot(const 
UnaryOperator *E) {
 
 Value *ScalarExprEmitter::VisitUnaryLNot(const UnaryOperator *E) {
   // Perform vector logical not on comparison with zero vector.
-  if (E->getType()->isVectorType() &&
-  E->getType()->castAs()->getVectorKind() ==
-  VectorType::GenericVector) {
+  if (E->getType()->isExtVectorType()) {
 Value *Oper = Visit(E->getSubExpr());
 Value *Zero = llvm::Constant::getNullValue(Oper->getType());
 Value *Result;

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4bec413f3042..45c1acecbe94 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14481,19 +14481,12 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation 
OpLoc,
   return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
<< resultType << Input.get()->getSourceRange());
   }
-  // Vector logical not returns the signed variant of the operand type.
-  resultType = GetSignedVectorType(resultType);
-  break;
-} else if (Context.getLangOpts().CPlusPlus && resultType->isVectorType()) {
-  const VectorType *VTy = resultType->castAs();
-  if (VTy->getVectorKind() != VectorType::GenericVector)
-return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
-
   // Vector logical not returns the signed variant of the operand type.
   resultType = GetSignedVectorType(resultType);
   break;
 } else {
+  // FIXME: GCC's vector extension permits the usage of '!' with a vector
+  //type in C++. We should allow that here too.
   return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
 << resultType << Input.get()->getSourceRange());
 }

diff  --git a/clang/test/CodeGen/vector-logic-not.cpp 
b/clang/test/CodeGen/vector-logic-not.cpp
deleted file mode 100644
index 2ac026711e82..
--- a/clang/test/CodeGen/vector-logic-not.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-
-typedef __attribute__((__vector_size__(16))) float float4;
-typedef __attribute__((__vector_size__(16))) int int4;
-typedef __attribute__((__vector_size__(16))) unsigned int uint4;
-
-// CHECK: @_Z5test1Dv4_j
-int4 test1(uint4 V0) {
-  // CHECK: [[CMP0:%.*]] = icmp eq <4 x i32> [[V0:%.*]], zeroinitializer
-  // CHECK-NEXT: [[V1:%.*]] = sext <4 x i1> [[CMP0]] to <4 x i32>
-  int4 V = !V0;
-  return V;
-}
-
-// CHECK: @_Z5test2Dv4_fS_
-int4 test2(

[PATCH] D81315: [Draft] [Prototype] warning for default constructed unique pointer dereferences

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:72-76
+  // STL smart pointer methods which resets to null
+  const llvm::StringSet<> ResetMethods = {"reset", "release", "swap"};
+
+  // STL smart pointer methods which resets to null via null argument
+  const llvm::StringSet<> NullResetMethods = {"reset", "swap"};

Please consider `CallDescription` and `CallDescriptionMap`!



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:81
+// to track the mem region and curresponding states
+REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, 
RegionState)
+// to track the Symbols which will get inner raw pointer via unique_ptr.get()

I ultimately believe this map should go away. The only thing we really need is 
the map from smart pointer region to the symbol for its current raw pointer. As 
long as we have such data we can discover the nullness of that symbol (which 
*is* the nullness of the smart pointer as well) from Range Constraints.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:213-214
+
+  if (!BT)
+BT.reset(new BugType(this, "unique_ptr", "Dont call unique_ptr"));
+  auto R = std::make_unique(

These days we don't bother with lazy initialization, the usual syntax is:
```lang=c++
class YourChecker {
  // ...
  BugType BT { this, "unique_ptr", "Dont call unique_ptr" };
  // ...
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81315



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 269155.
dang marked an inline comment as done.
dang added a comment.

Address some code review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80383

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h

Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,24 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
+// RUN: cp %t/MyHeader2.pcm %t1.pcm
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
+// RUN: cp %t/MyHeader2.pcm %t2.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm > %t1.dump
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t2.pcm > %t2.dump
+// RUN: cat %t1.dump %t2.dump | FileCheck %s
+
+#include "my_header_2.h"
+
+my_int var = 42;
+
+// CHECK: [[AST_BLOCK_HASH:]]
+// CHECK: [[SIGNATURE:]]
+// CHECK: [[AST_BLOCK_HASH]]
+// CHECK-NOT: [[SIGNATURE]]
+// The modules built by this test are designed to yield the same AST. If this
+// test fails, it means that the AST block is has become non-relocatable.
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2434,12 +2434,12 @@
   SourceLocation Loc = D->getLocation();
   unsigned Index = ID - FirstDeclID;
   if (DeclOffsets.size() == Index)
-DeclOffsets.emplace_back(Loc, Offset);
+DeclOffsets.emplace_back(Loc, Offset, ASTBlockRange.first);
   else if (DeclOffsets.size() < Index) {
 // FIXME: Can/should this happen?
 DeclOffsets.resize(Index+1);
 DeclOffsets[Index].setLocation(Loc);
-DeclOffsets[Index].setBitOffset(Offset);
+DeclOffsets[Index].setBitOffset(Offset, ASTBlockRange.first);
   } else {
 llvm_unreachable("declarations should be emitted in ID order");
   }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -10,14 +10,12 @@
 //
 //===--===//
 
-#include "clang/AST/OpenMPClause.h"
-#include "clang/Serialization/ASTRecordWriter.h"
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "MultiOnDiskHashTable.h"
-#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTUnresolvedSet.h"
+#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -31,6 +29,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -65,7 +64,9 @@
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "clang/Serialization/ModuleFile.h"
 #include 

[PATCH] D80979: [clang] Implement VectorType logic not operator.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted in abca3b7b2ce539ce22c9d3ebdd3cb6c02bb4c009 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80979



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

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

See D81347  for the parent revision that makes 
ASTFileSignature a `std::array`. Adding the abbreviation doesn't 
make that much sense at the moment because of how the `IMPORTS` record is 
structured as discussed offline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80383



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

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase
- Move GetFSProvider to a lambda inside scanPreamble with a FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81173

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/support/FSProvider.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/CompilerTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "Compiler.h"
 #include "ParsedAST.h"
+#include "TestFS.h"
 #include "index/Index.h"
 #include "support/Path.h"
 #include "llvm/ADT/StringMap.h"
@@ -69,7 +70,7 @@
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
   std::shared_ptr preamble() const;
-  ParseInputs inputs() const;
+  ParseInputs inputs(MockFSProvider &FSProvider) const;
   SymbolSlab headerSymbols() const;
   RefSlab headerRefs() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -20,7 +20,7 @@
 namespace clang {
 namespace clangd {
 
-ParseInputs TestTU::inputs() const {
+ParseInputs TestTU::inputs(MockFSProvider &FSProvider) const {
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
@@ -29,10 +29,10 @@
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
 
-  llvm::StringMap Files(AdditionalFiles);
-  Files[FullFilename] = Code;
-  Files[FullHeaderName] = HeaderCode;
-  Files[ImportThunk] = ThunkContents;
+  FSProvider.Files = AdditionalFiles;
+  FSProvider.Files[FullFilename] = Code;
+  FSProvider.Files[FullHeaderName] = HeaderCode;
+  FSProvider.Files[ImportThunk] = ThunkContents;
 
   ParseInputs Inputs;
   auto &Argv = Inputs.CompileCommand.CommandLine;
@@ -54,7 +54,7 @@
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS(Files);
+  Inputs.FSProvider = &FSProvider;
   Inputs.Opts = ParseOptions();
   Inputs.Opts.BuildRecoveryAST = true;
   Inputs.Opts.PreserveRecoveryASTType = true;
@@ -67,7 +67,8 @@
 }
 
 std::shared_ptr TestTU::preamble() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   IgnoreDiagnostics Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
@@ -77,7 +78,8 @@
 }
 
 ParsedAST TestTU::build() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -31,11 +31,12 @@
 class MockFSProvider : public FileSystemProvider {
 public:
   IntrusiveRefCntPtr getFileSystem() const override {
-return buildTestFS(Files);
+return buildTestFS(Files, Timestamps);
   }
 
   // If relative paths are used, they are resolved with testPath().
   llvm::StringMap Files;
+  llvm::StringMap Timestamps;
 };
 
 // A Compilation database that returns a fixed set of compile flags.
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -16,6 +16,7 @@
 #include "TestFS.h"
 #include "support/Cancellation.h"
 #include "support/Context.h"
+#include "support/FSProvider.h"
 #include "s

[PATCH] D80210: [analyzer] Turn off reports in system headers

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

Szelethus added a comment.

Sounds nice. @bruntib, would this be sufficient for the user's request? Are we 
sure that the actual request makes sense in light of the analyzer's viewpoint?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80210



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

[clang-tools-extra] f693ce4 - [clangd] Change ParseInputs to store FSProvider rather than VFS

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-06-08T13:23:55+02:00
New Revision: f693ce4aa97e7afe34a3f41609ffbbaf32b571de

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

LOG: [clangd] Change ParseInputs to store FSProvider rather than VFS

Summary: This ensures ParseInputs provides a read-only access to FS.

Reviewers: sammccall

Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/support/FSProvider.h
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/CompilerTests.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 0721c8ddaf73..3539de0e561e 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -179,18 +179,16 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase &CDB,
 void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
llvm::StringRef Version,
WantDiagnostics WantDiags, bool ForceRebuild) {
-  auto FS = FSProvider.getFileSystem();
-
   ParseOptions Opts;
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   // FIXME: call tidy options builder on the worker thread, it can do IO.
   if (GetClangTidyOptions)
-Opts.ClangTidyOpts = GetClangTidyOptions(*FS, File);
+Opts.ClangTidyOpts = GetClangTidyOptions(*FSProvider.getFileSystem(), 
File);
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.FS = FS;
+  Inputs.FSProvider = &FSProvider;
   Inputs.Contents = std::string(Contents);
   Inputs.Version = Version.str();
   Inputs.ForceRebuild = ForceRebuild;
@@ -214,8 +212,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
   if (!CodeCompleteOpts.Index) // Respect overridden index.
 CodeCompleteOpts.Index = Index;
 
-  auto Task = [Pos, FS = FSProvider.getFileSystem(), CodeCompleteOpts,
-   File = File.str(), CB = std::move(CB),
+  auto Task = [Pos, CodeCompleteOpts, File = File.str(), CB = std::move(CB),
this](llvm::Expected IP) mutable {
 if (!IP)
   return CB(IP.takeError());
@@ -238,7 +235,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
 }
   }
 }
-ParseInputs ParseInput{IP->Command, FS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &FSProvider, IP->Contents.str()};
 ParseInput.Index = Index;
 ParseInput.Opts.BuildRecoveryAST = BuildRecoveryAST;
 ParseInput.Opts.PreserveRecoveryASTType = PreserveRecoveryASTType;
@@ -275,8 +272,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
 void ClangdServer::signatureHelp(PathRef File, Position Pos,
  Callback CB) {
 
-  auto Action = [Pos, FS = FSProvider.getFileSystem(), File = File.str(),
- CB = std::move(CB),
+  auto Action = [Pos, File = File.str(), CB = std::move(CB),
  this](llvm::Expected IP) mutable {
 if (!IP)
   return CB(IP.takeError());
@@ -286,7 +282,7 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
   return CB(llvm::createStringError(llvm::inconvertibleErrorCode(),
 "Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, FS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &FSProvider, IP->Contents.str()};
 ParseInput.Index = Index;
 ParseInput.Opts.BuildRecoveryAST = BuildRecoveryAST;
 ParseInput.Opts.PreserveRecoveryASTType = PreserveRecoveryASTType;
@@ -399,8 +395,9 @@ void ClangdServer::rename(PathRef File, Position Pos, 
llvm::StringRef NewName,
   return CB(Edits.takeError());
 
 if (Opts.WantFormat) {
-  auto Style = getFormatStyleForF

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 269168.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Inline VFS uses in ClangdServer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81173

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/support/FSProvider.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/CompilerTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "Compiler.h"
 #include "ParsedAST.h"
+#include "TestFS.h"
 #include "index/Index.h"
 #include "support/Path.h"
 #include "llvm/ADT/StringMap.h"
@@ -69,7 +70,7 @@
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
   std::shared_ptr preamble() const;
-  ParseInputs inputs() const;
+  ParseInputs inputs(MockFSProvider &FSProvider) const;
   SymbolSlab headerSymbols() const;
   RefSlab headerRefs() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -20,7 +20,7 @@
 namespace clang {
 namespace clangd {
 
-ParseInputs TestTU::inputs() const {
+ParseInputs TestTU::inputs(MockFSProvider &FSProvider) const {
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
@@ -29,10 +29,10 @@
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
 
-  llvm::StringMap Files(AdditionalFiles);
-  Files[FullFilename] = Code;
-  Files[FullHeaderName] = HeaderCode;
-  Files[ImportThunk] = ThunkContents;
+  FSProvider.Files = AdditionalFiles;
+  FSProvider.Files[FullFilename] = Code;
+  FSProvider.Files[FullHeaderName] = HeaderCode;
+  FSProvider.Files[ImportThunk] = ThunkContents;
 
   ParseInputs Inputs;
   auto &Argv = Inputs.CompileCommand.CommandLine;
@@ -54,7 +54,7 @@
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS(Files);
+  Inputs.FSProvider = &FSProvider;
   Inputs.Opts = ParseOptions();
   Inputs.Opts.BuildRecoveryAST = true;
   Inputs.Opts.PreserveRecoveryASTType = true;
@@ -67,7 +67,8 @@
 }
 
 std::shared_ptr TestTU::preamble() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   IgnoreDiagnostics Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
@@ -77,7 +78,8 @@
 }
 
 ParsedAST TestTU::build() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -31,11 +31,12 @@
 class MockFSProvider : public FileSystemProvider {
 public:
   IntrusiveRefCntPtr getFileSystem() const override {
-return buildTestFS(Files);
+return buildTestFS(Files, Timestamps);
   }
 
   // If relative paths are used, they are resolved with testPath().
   llvm::StringMap Files;
+  llvm::StringMap Timestamps;
 };
 
 // A Compilation database that returns a fixed set of compile flags.
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -16,6 +16,7 @@
 #include "TestFS.h"
 #include "support/Cancellation.h"
 #include "support/Context.h"
+#include "support/FSProvider.h"
 #include "

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

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

Szelethus marked 2 inline comments as done.
Szelethus added a comment.

Gentle ping




Comment at: clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td:141-142
+/// Describes preferred registration and evaluation order in between checkers.
+/// Unlike strong dependencies, this expresses dependencies in between
+/// diagnostics, and *not* modeling. In the case of an unsatisfied (disabled)
+/// weak dependency, the dependent checker might still be registered. If the

NoQ wrote:
> I wouldn't mind having predictable callback evaluation order for modeling as 
> well. What's causing you to drop this scenario?
D80905#2066219


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D80928: [BFloat] Add convert/copy instrinsic support

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:789
 
+def int_arm_neon_vcvtfp2bf
+: Intrinsic<[llvm_anyvector_ty], [llvm_anyvector_ty], [IntrNoMem]>;

I only see this being used for f32 -> bf16 conversion, so could this have 
concrete types, instead of llvm_anyvector_ty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80928



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

[PATCH] D81380: [clangd] Don't produce snippets when completion location is followed by parenthesis

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Prevent a second pair of parenthesis from being added when there already is one
right after cursor.

Related issue and more context: https://github.com/clangd/clangd/issues/387


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81380

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2878,6 +2878,27 @@
   }
 }
 
+TEST(CompletionTest, RemoveSnippetOnContext) {
+  clangd::CodeCompleteOptions Opts;
+  Opts.EnableSnippets = true;
+  std::string Context = R"cpp(
+int foo(int A);
+int bar();
+int Variable = 42;
+  )cpp";
+  EXPECT_THAT(completions(Context + "int y = fo^", {}, Opts).Completions,
+  UnorderedElementsAre(
+  AllOf(Labeled("foo(int A)"), SnippetSuffix("(${1:int 
A})";
+  EXPECT_THAT(
+  completions(Context + "int y = fo^(42)", {}, Opts).Completions,
+  UnorderedElementsAre(AllOf(Labeled("foo(int A)"), SnippetSuffix("";
+  EXPECT_THAT(
+  completions(Context + "int y = ba^", {}, Opts).Completions,
+  UnorderedElementsAre(AllOf(Labeled("bar()"), SnippetSuffix("()";
+  EXPECT_THAT(completions(Context + "int y = ba^()", {}, Opts).Completions,
+  UnorderedElementsAre(AllOf(Labeled("bar()"), 
SnippetSuffix("";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -48,6 +48,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
@@ -1221,6 +1222,10 @@
   CompletionRecorder *Recorder = nullptr;
   CodeCompletionContext::Kind CCContextKind = CodeCompletionContext::CCC_Other;
   bool IsUsingDeclaration = false;
+  // The snippets will not be generated if the token following completion
+  // location is an opening parenthesis (tok::l_paren) because this would add
+  // extra parenthesis.
+  bool HasParenthesisAfter = false;
   // Counters for logging.
   int NSema = 0, NIndex = 0, NSemaAndIndex = 0, NIdent = 0;
   bool Incomplete = false; // Would more be available with a higher limit?
@@ -1272,6 +1277,10 @@
   assert(Recorder && "Recorder is not set");
   CCContextKind = Recorder->CCContext.getKind();
   IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
+  const auto NextToken = Lexer::findNextToken(
+  Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
+  Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts);
+  HasParenthesisAfter = NextToken->getKind() == tok::l_paren;
   auto Style = getFormatStyleForFile(SemaCCInput.FileName,
  SemaCCInput.ParseInput.Contents,
  SemaCCInput.ParseInput.FS.get());
@@ -1689,11 +1698,18 @@
   CodeCompletionString *SemaCCS =
   Item.SemaResult ? Recorder->codeCompletionString(*Item.SemaResult)
   : nullptr;
+  // FIXME(kirillbobyrev): Instead of not generating any snippets when
+  // tok::l_paren is the next token after completion location, use more
+  // advanced techniques. For example, if the snippet is a pair of
+  // parenthesis with some arguments `(${1: int I}, ${2: char C})` and the
+  // completion location is only followed by a pair of parenthesis without
+  // any arguments inside (or without the sufficient number of arguments),
+  // replace these parenthesis with the contents of the snippet.
   if (!Builder)
-Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : 
nullptr,
-Item, SemaCCS, QueryScopes, *Inserter, FileName,
-CCContextKind, Opts,
-/*GenerateSnippets=*/!IsUsingDeclaration);
+Builder.emplace(
+Recorder ? &Recorder->CCSema->getASTContext() : nullptr, Item,
+SemaCCS, QueryScopes, *Inserter, FileName, CCContextKind, Opts,
+/*GenerateSnippets=*/!IsUsingDeclaration && !HasParenthesisAfter);
   else
 Builder->add(Item, SemaCCS);
 }


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf693ce4aa97e: [clangd] Change ParseInputs to store 
FSProvider rather than VFS (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81173

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/support/FSProvider.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/CompilerTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,6 +19,7 @@
 
 #include "Compiler.h"
 #include "ParsedAST.h"
+#include "TestFS.h"
 #include "index/Index.h"
 #include "support/Path.h"
 #include "llvm/ADT/StringMap.h"
@@ -69,7 +70,7 @@
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
   std::shared_ptr preamble() const;
-  ParseInputs inputs() const;
+  ParseInputs inputs(MockFSProvider &FSProvider) const;
   SymbolSlab headerSymbols() const;
   RefSlab headerRefs() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -20,7 +20,7 @@
 namespace clang {
 namespace clangd {
 
-ParseInputs TestTU::inputs() const {
+ParseInputs TestTU::inputs(MockFSProvider &FSProvider) const {
   std::string FullFilename = testPath(Filename),
   FullHeaderName = testPath(HeaderFilename),
   ImportThunk = testPath("import_thunk.h");
@@ -29,10 +29,10 @@
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
 
-  llvm::StringMap Files(AdditionalFiles);
-  Files[FullFilename] = Code;
-  Files[FullHeaderName] = HeaderCode;
-  Files[ImportThunk] = ThunkContents;
+  FSProvider.Files = AdditionalFiles;
+  FSProvider.Files[FullFilename] = Code;
+  FSProvider.Files[FullHeaderName] = HeaderCode;
+  FSProvider.Files[ImportThunk] = ThunkContents;
 
   ParseInputs Inputs;
   auto &Argv = Inputs.CompileCommand.CommandLine;
@@ -54,7 +54,7 @@
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
-  Inputs.FS = buildTestFS(Files);
+  Inputs.FSProvider = &FSProvider;
   Inputs.Opts = ParseOptions();
   Inputs.Opts.BuildRecoveryAST = true;
   Inputs.Opts.PreserveRecoveryASTType = true;
@@ -67,7 +67,8 @@
 }
 
 std::shared_ptr TestTU::preamble() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   IgnoreDiagnostics Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
@@ -77,7 +78,8 @@
 }
 
 ParsedAST TestTU::build() const {
-  auto Inputs = inputs();
+  MockFSProvider FSProvider;
+  auto Inputs = inputs(FSProvider);
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -31,11 +31,12 @@
 class MockFSProvider : public FileSystemProvider {
 public:
   IntrusiveRefCntPtr getFileSystem() const override {
-return buildTestFS(Files);
+return buildTestFS(Files, Timestamps);
   }
 
   // If relative paths are used, they are resolved with testPath().
   llvm::StringMap Files;
+  llvm::StringMap Timestamps;
 };
 
 // A Compilation database that returns a fixed set of compile flags.
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -16,6 +16,7 @@
 #include "TestFS.h"
 #include "support/Cancellation.h"
 #include "support/Context.h"

[clang] 615673f - [Preamble] Invalidate preamble when missing headers become present.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-06-08T14:03:08+02:00
New Revision: 615673f3a10e98f33e2db64512be0452145236fe

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

LOG: [Preamble] Invalidate preamble when missing headers become present.

Summary:
To avoid excessive extra stat()s, only check the possible locations of
headers that weren't found at all (leading to a compile error).
For headers that *were* found, we don't check for files earlier on the
search path that could override them.

Reviewers: kadircet

Subscribers: javed.absar, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang/include/clang/Frontend/PrecompiledPreamble.h
clang/lib/Frontend/PrecompiledPreamble.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp 
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index fe752821859d..431c36c72b91 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -131,11 +131,19 @@ TEST_F(LSPTest, Diagnostics) {
 
 TEST_F(LSPTest, DiagnosticsHeaderSaved) {
   auto &Client = start();
-  FS.Files["foo.h"] = "#define VAR original";
   Client.didOpen("foo.cpp", R"cpp(
 #include "foo.h"
 int x = VAR;
   )cpp");
+  EXPECT_THAT(Client.diagnostics("foo.cpp"),
+  llvm::ValueIs(testing::ElementsAre(
+  DiagMessage("'foo.h' file not found"),
+  DiagMessage("Use of undeclared identifier 'VAR'";
+  // Now create the header.
+  FS.Files["foo.h"] = "#define VAR original";
+  Client.notify(
+  "textDocument/didSave",
+  llvm::json::Object{{"textDocument", Client.documentID("foo.h")}});
   EXPECT_THAT(Client.diagnostics("foo.cpp"),
   llvm::ValueIs(testing::ElementsAre(
   DiagMessage("Use of undeclared identifier 'original'";

diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index ee54a2f71e10..c04e186ca6b4 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -735,15 +735,24 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   ASSERT_EQ(S.fileStats().lookup(Source).PreambleBuilds, 3u);
 }
 
-TEST_F(TUSchedulerTests, ForceRebuild) {
+// We rebuild if a completely missing header exists, but not if one is added
+// on a higher-priority include path entry (for performance).
+// (Previously we wouldn't automatically rebuild when files were added).
+TEST_F(TUSchedulerTests, MissingHeader) {
+  CDB.ExtraClangFlags.push_back("-I" + testPath("a"));
+  CDB.ExtraClangFlags.push_back("-I" + testPath("b"));
+  // Force both directories to exist so they don't get pruned.
+  FSProvider.Files.try_emplace("a/__unused__");
+  FSProvider.Files.try_emplace("b/__unused__");
   TUScheduler S(CDB, optsForTest(), captureDiags());
 
   auto Source = testPath("foo.cpp");
-  auto Header = testPath("foo.h");
+  auto HeaderA = testPath("a/foo.h");
+  auto HeaderB = testPath("b/foo.h");
 
   auto SourceContents = R"cpp(
   #include "foo.h"
-  int b = a;
+  int c = b;
 )cpp";
 
   ParseInputs Inputs = getInputs(Source, SourceContents);
@@ -758,37 +767,48 @@ TEST_F(TUSchedulerTests, ForceRebuild) {
 EXPECT_THAT(Diags,
 ElementsAre(Field(&Diag::Message, "'foo.h' file not 
found"),
 Field(&Diag::Message,
-  "use of undeclared identifier 'a'")));
+  "use of undeclared identifier 'b'")));
   });
   S.blockUntilIdle(timeoutSeconds(10));
 
-  // Add the header file. We need to recreate the inputs since we changed a
-  // file from underneath the test FS.
-  FSProvider.Files[Header] = "int a;";
-  FSProvider.Timestamps[Header] = time_t(1);
-  Inputs = getInputs(Source, SourceContents);
+  FSProvider.Files[HeaderB] = "int b;";
+  FSProvider.Timestamps[HeaderB] = time_t(1);
 
-  // The addition of the missing header file shouldn't trigger a rebuild since
-  // we don't track missing files.
+  // The addition of the missing header file triggers a rebuild, no errors.
   updateWithDiags(
   S, Source, Inputs, WantDiagnostics::Yes,
   [&DiagCount](std::vector Diags) {
 ++DiagCount;
-ADD_FAILURE() << "Did not expect diagnostics for missing header 
update";
+EXPECT_THAT(Diags, IsEmpty());
   });
 
-  // Forcing the reload should should cause a rebuild which no longer has 

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D80961#2079044 , @klimek wrote:

> In D80961#2076242 , @aaron.ballman 
> wrote:
>
> > In D80961#2073049 , @klimek wrote:
> >
> > > Without jumping into the discussion whether it should be the default, I 
> > > think we should be able to control template instantiation visitation 
> > > separately from other implicit nodes.
> > >  Having to put AsIs on a matcher every time you need to match template 
> > > instantiations is a rather big change (suddenly you have to change all 
> > > the matchers you've written so far).
> >
> >
> > I think that's the intended meaning of `AsIs` though. Template 
> > instantiations are not source code the user wrote, they're source code the 
> > compiler stamped out from code the user wrote. I hope 
> > `IgnoreUnlessSpelledInSource` isn't a misnomer.
> >
> > > I love the idea of being able to control visitation of template 
> > > instantiation.
> > >  I am somewhat torn on whether it should be the default, and would like 
> > > to see more data.
> > >  I feel more strongly about needing AsIs when I want to match template 
> > > instantiations.
> >
> > FWIW, my experience in clang-tidy has been that template instantiations are 
> > ignored far more often than they're desired. In fact, instantiations tend 
> > to be a source of bugs for us because they're easy to forget about when 
> > writing matchers without keeping templates in mind. The times when template 
> > instantiations become important to *not* ignore within the checks is when 
> > the check is specific to template behavior, but that's a minority of the 
> > public checks thus far.
>
>
> So I assume the idea is that this will work & be what we'll want people to 
> write?
>  traverse(AsIs, decl(traverse(IgnoreImplicit, hasFoo)))


I believe so, yes. It's explicit about which traversal mode you want any given 
set of matchers to match with, so it is more flexible at the expense of being 
more verbose. Alternatively, you could be in `AsIs` mode for all of it and 
explicitly ignore the implicit nodes you wish to ignore (which may be even more 
verbose with the same results, depending on the matchers involved).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

[PATCH] D80210: [analyzer] Turn off reports in system headers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

(if we agree to do this, the next obvious experiment would be to cut out the 
beginning of the path from the report we *do* emit - i.e., everything until our 
first interesting stack frame)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80210



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

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thank you!




Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:543
+
+  const StackFrameContext *StackFrame = Call.getCalleeStackFrame(BlockCount);
+  if (!StackFrame)

baloghadamsoftware wrote:
> NoQ wrote:
> > It is clear that the final return value of `getConstructionContext()` does 
> > not depend on the current block count; in fact the information you're 
> > trying to obtain is purely syntactic. You probably might as well pass 0 
> > here. As i mentioned before, the ideal solution would be to make 
> > `CallEvent` carry a `CFGElementRef` to begin with (as a replacement for the 
> > origin expr), so that it didn't need to be recovered like this.
> > 
> > We should absolutely eliminate the `BlockCount` parameter of 
> > `getReturnValueUnderConstruction()` as well. Checker developers shouldn't 
> > understand what it is, especially given that it doesn't matter at all which 
> > value do you pass.
> OK, I eliminated it. Do we need `BlockCount` for `getParameterLocation()` 
> (the other patch)?
We absolutely need `BlockCount` in the other patch because it's part of the 
identity of the parameter region.

That said, the API that requires the user to provide `BlockCount` manually is 
still terrible and extremely error-prone. Namely, the contract of 
`getParameterLocation()` would be "The behavior is undefined unless you provide 
the same block count that is there when call happens". Which is a good 
indication that we should have stored `BlockCount` in `CallEvent` from the 
start.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:539
+static const ConstructionContext
+*getConstructionContext(const CallEvent &Call) {
+  const StackFrameContext *StackFrame = Call.getCalleeStackFrame(0);

This looks useful. Let's turn it into a `CallEvent` method as well.


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

https://reviews.llvm.org/D80366



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

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td:141-142
+/// Describes preferred registration and evaluation order in between checkers.
+/// Unlike strong dependencies, this expresses dependencies in between
+/// diagnostics, and *not* modeling. In the case of an unsatisfied (disabled)
+/// weak dependency, the dependent checker might still be registered. If the

Szelethus wrote:
> NoQ wrote:
> > I wouldn't mind having predictable callback evaluation order for modeling 
> > as well. What's causing you to drop this scenario?
> D80905#2066219
Ok, so do i understand it correctly that the only reason you're omitting the 
"and modeling" clause here is because in your ultimate vision (which i 
completely agree with) checkers either do modeling or checking, and 
modeling-checkers never need to be disabled to begin with, so weak dependencies 
are useless for them, which is why this whole patch is entirely about 
diagnostics?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:404-407
   /// Returns memory location for a parameter variable within the callee stack
   /// frame. May fail; returns null on failure.
-  const VarRegion *getParameterLocation(unsigned Index,
-unsigned BlockCount) const;
+  const ParamVarRegion *getParameterLocation(unsigned Index,
+ unsigned BlockCount) const;

I guess we should document the landmine discussed in D80366#inline-747804.


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

https://reviews.llvm.org/D80522



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

[PATCH] D81380: [clangd] Don't produce snippets when completion location is followed by parenthesis

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this is nice!




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1228
+  // extra parenthesis.
+  bool HasParenthesisAfter = false;
   // Counters for logging.

I'd suggest rather storing the token kind as `NextToken`, and then deferring 
the actual "is it l_paren" check until toCodeCompletion.
That way the should-we-generate-snippets logic is more localized and IMO easier 
to read.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1283
+  Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts);
+  HasParenthesisAfter = NextToken->getKind() == tok::l_paren;
   auto Style = getFormatStyleForFile(SemaCCInput.FileName,

need to decide what to do when it returns none, or add an explicit assert with 
a message ("code completing in macro?")



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1701
   : nullptr;
+  // FIXME(kirillbobyrev): Instead of not generating any snippets when
+  // tok::l_paren is the next token after completion location, use more

This is a bit lengthy for describing an implementation strategy we're *not* 
using. Do you think we're very likely to do this, and this comment would save a 
lot of the work of finding out how?

I'd rather add a comment explaining what we *are* doing
e.g. `Suppress function argument snippets if args are already present, or not 
needed (using decl).`
And at most `// Should we consider sometimes replacing parens with the snippet 
instead?`



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1712
+SemaCCS, QueryScopes, *Inserter, FileName, CCContextKind, Opts,
+/*GenerateSnippets=*/!IsUsingDeclaration && !HasParenthesisAfter);
   else

how sure are we that paren-after is the right condition in all cases? Does 
"snippet" cover template snippets (std::vector<{$0}>)?

Don't need to handle this but it'd be nice to cover in tests.



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2881
 
+TEST(CompletionTest, RemoveSnippetOnContext) {
+  clangd::CodeCompleteOptions Opts;

nit: I don't really follow this test name, but it seems to be explaining part 
of the implementation.

What about `FunctionArgsExist` which rather explains the scenario?



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2893
+  EXPECT_THAT(
+  completions(Context + "int y = fo^(42)", {}, Opts).Completions,
+  UnorderedElementsAre(AllOf(Labeled("foo(int A)"), SnippetSuffix("";

can we have fo^o(42) as a case as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81380



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

[PATCH] D77942: [Preamble] Invalidate preamble when missing headers become present.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG615673f3a10e: [Preamble] Invalidate preamble when missing 
headers become present. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D77942?vs=266868&id=269185#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -19,15 +19,19 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -74,6 +78,68 @@
   bool needSystemDependencies() override { return true; }
 };
 
+// Collects files whose existence would invalidate the preamble.
+// Collecting *all* of these would make validating it too slow though, so we
+// just find all the candidates for 'file not found' diagnostics.
+//
+// A caveat that may be significant for generated files: we'll omit files under
+// search path entries whose roots don't exist when the preamble is built.
+// These are pruned by InitHeaderSearch and so we don't see the search path.
+// It would be nice to include them but we don't want to duplicate all the rest
+// of the InitHeaderSearch logic to reconstruct them.
+class MissingFileCollector : public PPCallbacks {
+  llvm::StringSet<> &Out;
+  const HeaderSearch &Search;
+  const SourceManager &SM;
+
+public:
+  MissingFileCollector(llvm::StringSet<> &Out, const HeaderSearch &Search,
+   const SourceManager &SM)
+  : Out(Out), Search(Search), SM(SM) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  StringRef FileName, bool IsAngled,
+  CharSourceRange FilenameRange, const FileEntry *File,
+  StringRef SearchPath, StringRef RelativePath,
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override {
+// File is null if it wasn't found.
+// (We have some false negatives if PP recovered e.g.  -> "foo")
+if (File != nullptr)
+  return;
+
+// If it's a rare absolute include, we know the full path already.
+if (llvm::sys::path::is_absolute(FileName)) {
+  Out.insert(FileName);
+  return;
+}
+
+// Reconstruct the filenames that would satisfy this directive...
+llvm::SmallString<256> Buf;
+auto NotFoundRelativeTo = [&](const DirectoryEntry *DE) {
+  Buf = DE->getName();
+  llvm::sys::path::append(Buf, FileName);
+  llvm::sys::path::remove_dots(Buf, /*remove_dot_dot=*/true);
+  Out.insert(Buf);
+};
+// ...relative to the including file.
+if (!IsAngled) {
+  if (const FileEntry *IncludingFile =
+  SM.getFileEntryForID(SM.getFileID(IncludeTok.getLocation(
+if (IncludingFile->getDir())
+  NotFoundRelativeTo(IncludingFile->getDir());
+}
+// ...relative to the search paths.
+for (const auto &Dir : llvm::make_range(
+ IsAngled ? Search.angled_dir_begin() : Search.search_dir_begin(),
+ Search.search_dir_end())) {
+  // No support for frameworks or header maps yet.
+  if (Dir.isNormalDir())
+NotFoundRelativeTo(Dir.getDir());
+}
+  }
+};
+
 /// Keeps a track of files to be deleted in destructor.
 class TemporaryFiles {
 public:
@@ -353,6 +419,11 @@
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
   if (auto CommentHandler = Callbacks.getCommentHandler())
 Clang->getPreprocessor().addCommentHandler(CommentHandler);
+  llvm::StringSet<> MissingFiles;
+  Clang->getPreprocessor().addPPCallbacks(
+  std::make_unique(
+  MissingFiles, Clang->getPreprocessor().getHeaderSearchInfo(),
+  Clang->getSourceManager()));
 
   if (llvm::Error Err = Act->Execute())
 return errorToErrorCode(std::move(Err));
@@ -387,9 +458,9 @@
 }
   }
 
-  return Precompiled

[clang] a679499 - [clang-format] treat 'lock' as a keyword for C# code

[Jonathan Coe via cfe-commits]

Author: Jonathan Coe
Date: 2020-06-08T13:31:22+01:00
New Revision: a67949913a6b19860fdc616da2fad00bf7beb3f8

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

LOG: [clang-format] treat 'lock' as a keyword for C# code

Summary: This will put a space in `lock (process)` when spaces are required 
after keywords.

Reviewers: krasimir

Reviewed By: krasimir

Subscribers: cfe-commits

Tags: #clang-format, #clang

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 9f2580367d0f..6e8dc690eeb9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3080,7 +3080,8 @@ bool TokenAnnotator::spaceRequiredBefore(const 
AnnotatedLine &Line,
 
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
-  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when))
+  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when,
+   Keywords.kw_lock))
 return Style.SpaceBeforeParens == FormatStyle::SBPO_ControlStatements 
||
spaceRequiredBeforeParens(Right);
   } else if (Style.Language == FormatStyle::LK_JavaScript) {

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp 
b/clang/unittests/Format/FormatTestCSharp.cpp
index dd2ed292ccd8..584b8aea383f 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -741,6 +741,9 @@ foreach ((A a, B b) in someList) {
 })",
Style);
 
+  // space after lock in `lock (processes)`.
+  verifyFormat("lock (process)", Style);
+
   Style.SpacesInSquareBrackets = true;
   verifyFormat(R"(private float[ , ] Values;)", Style);
   verifyFormat(R"(string dirPath = args?[ 0 ];)", Style);



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

[PATCH] D81315: [analyzer][Draft] [Prototype] warning for default constructed unique pointer dereferences

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

Szelethus added a comment.

Best of luck on your GSoC! I don't have much else to add to your patch, but you 
seem to have made good progress already!

In D81315#2078043 , @xazax.hun wrote:

> some folks have automated scripts based on that tag to add themselves as 
> subscriber/reviewer.


Hope you don't mind my intrusion :)

> I am not sure about whether I should use eval::Call or both check::PreCall 
> and check::PostCall. In the eval::Call documentation I found this "Note, that 
> only one checker can evaluate a call.". So I am little bit confused about 
> using it.

Inlining (when we model a function call, https://youtu.be/yh2qdnJjizE?t=238) is 
rather expensive. Creating a new stack frame, parameters, new `ExplodedNode`s, 
running checkers, etc., eats memory for breakfast, is slow and limits how deep 
the analysis can go. Worse still, the analysis could lose precision if the 
called function's definition isn't available. `eval::Call` serves to circumvent 
this by allowing the analyzer to give a precise summary of the function. 
`StreamChecker`, for instance, uses this for functions such as `clearerr()` -- 
the C standard defines how this function should behave, so upon encountering a 
call to it, we don't need all the extra work regular inlining demands, just ask 
`StreamChecker` to model it for us.

Use `eval::Call` if you can provide a precise model for a function. Only a 
single checker is allowed to do that -- you can see that it returns with a 
boolean value to sign whether the checker could provide an evaluation, and as 
far as I know, the first checker that returns true will be doing it.

I think it would be appropriate in this instance, because we're modeling a well 
established API. In general, I think we should use it when appropriate more 
often, like in MallocChecker.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:46
+};
+} // end of anonymous namespace
+

xazax.hun wrote:
> You can merge the two anonymous namespaces, this and the one below.
[[https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | I prefer 
them like this. ]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81315



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

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

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

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



Comment at: clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td:141-142
+/// Describes preferred registration and evaluation order in between checkers.
+/// Unlike strong dependencies, this expresses dependencies in between
+/// diagnostics, and *not* modeling. In the case of an unsatisfied (disabled)
+/// weak dependency, the dependent checker might still be registered. If the

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > I wouldn't mind having predictable callback evaluation order for modeling 
> > > as well. What's causing you to drop this scenario?
> > D80905#2066219
> Ok, so do i understand it correctly that the only reason you're omitting the 
> "and modeling" clause here is because in your ultimate vision (which i 
> completely agree with) checkers either do modeling or checking, and 
> modeling-checkers never need to be disabled to begin with, so weak 
> dependencies are useless for them, which is why this whole patch is entirely 
> about diagnostics?
Pretty much, yes. Its not just useless -- I don't think establishing a 
//preferred// evaluation order over //guaranteed// for modeling checkers could 
ever be correct. Not only that, I think adding new kinds of dependencies must 
be very well justified to counterweight the extra complexity, they should be 
intuitive and "just work". There would be a lot of corner cases if these 
dependencies could be too intertwined, and not only would they be a mess to 
implement, it could be confusing on the checker developer's end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D81061: [Analyzer][VLASizeChecker] Fix problem with zero index assumption.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:130-131
+// At least check for zero again.
+// Assume that this is a more exact fact than the previous assumptions
+// (in checkVLAIndexSize), so report error too.
+reportBug(VLA_Zero, SizeE, State, C);

Why not do the same in `checkVLAIndexSize` then?



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:182-183
 
-  SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LT, SizeD, Zero, SizeTy);
-  if (Optional LessThanZeroDVal =
-  LessThanZeroVal.getAs()) {
-ConstraintManager &CM = C.getConstraintManager();
-ProgramStateRef StatePos, StateNeg;
+  // Check if the size is zero or negative.
+  SVal PositiveVal = SVB.evalBinOp(State, BO_GT, SizeD, Zero, SizeTy);
+  if (Optional PositiveDVal = PositiveVal.getAs()) {

The type of binary operator `>=` is `bool`, not `size_t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81061



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

[PATCH] D81332: Thread safety analysis: Support deferring locks

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! Should we update the public docs for this change as well? Specifically, I 
am wondering if we should update 
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81332



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

[PATCH] D81384: [AST] Fix a clang crash on an invalid for-range statement.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

crash stack:

  llvm-project/clang/lib/AST/ASTContext.cpp:2248: clang::TypeInfo 
clang::ASTContext::getTypeInfoImpl(const clang::Type *) const: Assertion 
`!A->getDeducedType().isNull() && "cannot request the size of an undeduced or 
dependent auto type"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
   #0 0x025bb0bf llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
llvm-project/llvm/lib/Support/Unix/Signals.inc:564:13
   #1 0x025b92b0 llvm::sys::RunSignalHandlers() 
llvm-project/llvm/lib/Support/Signals.cpp:69:18
   #2 0x025bb535 SignalHandler(int) 
llvm-project/llvm/lib/Support/Unix/Signals.inc:396:3
   #3 0x7f9ef9298110 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14110)
   #4 0x7f9ef8d72761 raise 
/build/glibc-M65Gwz/glibc-2.30/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #5 0x7f9ef8d5c55b abort 
/build/glibc-M65Gwz/glibc-2.30/stdlib/abort.c:81:7
   #6 0x7f9ef8d5c42f get_sysdep_segment_value 
/build/glibc-M65Gwz/glibc-2.30/intl/loadmsgcat.c:509:8
   #7 0x7f9ef8d5c42f _nl_load_domain 
/build/glibc-M65Gwz/glibc-2.30/intl/loadmsgcat.c:970:34
   #8 0x7f9ef8d6b092 (/lib/x86_64-linux-gnu/libc.so.6+0x34092)
   #9 0x0458abe0 clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const llvm-project/clang/lib/AST/ASTContext.cpp:0:5


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81384

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/for-range-crash.cpp


Index: clang/test/SemaCXX/for-range-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/for-range-crash.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template 
+class Bar {
+  Bar *variables_to_modify;
+   foo() {  // expected-error {{C++ requires a type specifier for all 
declarations}}
+for (auto *c : *variables_to_modify)
+   delete c;
+  }
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2434,8 +2434,11 @@
 QualType RangeType = Range->getType();
 
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())
+LoopVar->setInvalidDecl();
   return StmtError();
+}
 
 // Build auto __begin = begin-expr, __end = end-expr.
 // Divide by 2, since the variables are in the inner scope (loop body).


Index: clang/test/SemaCXX/for-range-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/for-range-crash.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template 
+class Bar {
+  Bar *variables_to_modify;
+   foo() {  // expected-error {{C++ requires a type specifier for all declarations}}
+for (auto *c : *variables_to_modify)
+   delete c;
+  }
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2434,8 +2434,11 @@
 QualType RangeType = Range->getType();
 
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())
+LoopVar->setInvalidDecl();
   return StmtError();
+}
 
 // Build auto __begin = begin-expr, __end = end-expr.
 // Divide by 2, since the variables are in the inner scope (loop body).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 269189.
dgoldman added a comment.

Check AccessOpFixIt.hasValue()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/objc-member-access.m


Index: clang/test/CodeCompletion/objc-member-access.m
===
--- /dev/null
+++ clang/test/CodeCompletion/objc-member-access.m
@@ -0,0 +1,22 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+@interface TypeWithPropertiesBackedByIvars {
+  int _bar;
+  int _foo;
+}
+@property(nonatomic) int foo;
+@property(nonatomic) int bar;
+@end
+
+int getFoo(id object) {
+  TypeWithPropertiesBackedByIvars *model = (TypeWithPropertiesBackedByIvars 
*)object;
+  int foo = model.;
+  return foo;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits 
-code-completion-at=%s:14:19 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1-NOT: [#int#]_bar
+// CHECK-CC1-NOT: [#int#]_foo
+// CHECK-CC1: [#int#]bar
+// CHECK-CC1: [#int#]foo
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5195,7 +5195,13 @@
 Results.AddResult(std::move(Result));
   }
 } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
-  // Objective-C property reference.
+  // Objective-C property reference. Bail if we're performing fix-it code
+  // completion since we don't handle forwarding the fix-it for Objective-C
+  // objects. Since Objective-C properties are normally backed by ivars,
+  // most Objecive-C fix-its here would have little value anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }
   AddedPropertiesSet AddedProperties;
 
   if (const ObjCObjectPointerType *ObjCPtr =
@@ -5215,7 +5221,13 @@
   /*InOriginalClass*/ false);
 } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
(!IsArrow && BaseType->isObjCObjectType())) {
-  // Objective-C instance variable access.
+  // Objective-C instance variable access. Bail if we're performing fix-it
+  // code completion since we don't handle forwarding the fix-it for
+  // Objective-C objects. Since Objective-C properties are normally backed
+  // by ivars, most Objecive-C fix-its here would have little value 
anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }
   ObjCInterfaceDecl *Class = nullptr;
   if (const ObjCObjectPointerType *ObjCPtr =
   BaseType->getAs())
@@ -5240,6 +5252,11 @@
   Results.EnterNewScope();
 
   bool CompletionSucceded = DoCompletion(Base, IsArrow, None);
+
+  // Fixits are only included for C++. We avoid this for Objective-C properties
+  // and ivars since most properties are backed by an ivar; otherwise we would
+  // recommend an ivar fixit when code-completing a property. Another possible
+  // solution would be to de-duplicate the ivar/property mixing.
   if (CodeCompleter->includeFixIts()) {
 const CharSourceRange OpRange =
 CharSourceRange::getTokenRange(OpLoc, OpLoc);


Index: clang/test/CodeCompletion/objc-member-access.m
===
--- /dev/null
+++ clang/test/CodeCompletion/objc-member-access.m
@@ -0,0 +1,22 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+@interface TypeWithPropertiesBackedByIvars {
+  int _bar;
+  int _foo;
+}
+@property(nonatomic) int foo;
+@property(nonatomic) int bar;
+@end
+
+int getFoo(id object) {
+  TypeWithPropertiesBackedByIvars *model = (TypeWithPropertiesBackedByIvars *)object;
+  int foo = model.;
+  return foo;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:14:19 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1-NOT: [#int#]_bar
+// CHECK-CC1-NOT: [#int#]_foo
+// CHECK-CC1: [#int#]bar
+// CHECK-CC1: [#int#]foo
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5195,7 +5195,13 @@
 Results.AddResult(std::move(Result));
   }
 } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
-  // Objective-C property reference.
+  // Objective-C property reference. Bail if we're performing fix-it code
+  // completion since we don't handle forwarding the fix-it for Objective-C
+  // objects. Since Objective-C properties are normally backed by ivars,
+  // most Objecive-C fix-its here would have little value anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }

[PATCH] D81255: [clang-format] treat 'lock' as a keyword for C# code

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa67949913a6b: [clang-format] treat 'lock' as a 
keyword for C# code (authored by Jonathan Coe ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81255

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


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -741,6 +741,9 @@
 })",
Style);
 
+  // space after lock in `lock (processes)`.
+  verifyFormat("lock (process)", Style);
+
   Style.SpacesInSquareBrackets = true;
   verifyFormat(R"(private float[ , ] Values;)", Style);
   verifyFormat(R"(string dirPath = args?[ 0 ];)", Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3080,7 +3080,8 @@
 
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
-  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when))
+  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when,
+   Keywords.kw_lock))
 return Style.SpaceBeforeParens == FormatStyle::SBPO_ControlStatements 
||
spaceRequiredBeforeParens(Right);
   } else if (Style.Language == FormatStyle::LK_JavaScript) {


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -741,6 +741,9 @@
 })",
Style);
 
+  // space after lock in `lock (processes)`.
+  verifyFormat("lock (process)", Style);
+
   Style.SpacesInSquareBrackets = true;
   verifyFormat(R"(private float[ , ] Values;)", Style);
   verifyFormat(R"(string dirPath = args?[ 0 ];)", Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3080,7 +3080,8 @@
 
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
-  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when))
+  if (Left.isOneOf(tok::kw_using, Keywords.kw_async, Keywords.kw_when,
+   Keywords.kw_lock))
 return Style.SpaceBeforeParens == FormatStyle::SBPO_ControlStatements ||
spaceRequiredBeforeParens(Right);
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 269192.
dgoldman marked 4 inline comments as done.
dgoldman added a comment.

Remove stale comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/objc-member-access.m


Index: clang/test/CodeCompletion/objc-member-access.m
===
--- /dev/null
+++ clang/test/CodeCompletion/objc-member-access.m
@@ -0,0 +1,22 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+@interface TypeWithPropertiesBackedByIvars {
+  int _bar;
+  int _foo;
+}
+@property(nonatomic) int foo;
+@property(nonatomic) int bar;
+@end
+
+int getFoo(id object) {
+  TypeWithPropertiesBackedByIvars *model = (TypeWithPropertiesBackedByIvars 
*)object;
+  int foo = model.;
+  return foo;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits 
-code-completion-at=%s:14:19 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1-NOT: [#int#]_bar
+// CHECK-CC1-NOT: [#int#]_foo
+// CHECK-CC1: [#int#]bar
+// CHECK-CC1: [#int#]foo
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5195,7 +5195,13 @@
 Results.AddResult(std::move(Result));
   }
 } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
-  // Objective-C property reference.
+  // Objective-C property reference. Bail if we're performing fix-it code
+  // completion since we don't handle forwarding the fix-it for Objective-C
+  // objects. Since Objective-C properties are normally backed by ivars,
+  // most Objecive-C fix-its here would have little value anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }
   AddedPropertiesSet AddedProperties;
 
   if (const ObjCObjectPointerType *ObjCPtr =
@@ -5215,7 +5221,13 @@
   /*InOriginalClass*/ false);
 } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
(!IsArrow && BaseType->isObjCObjectType())) {
-  // Objective-C instance variable access.
+  // Objective-C instance variable access. Bail if we're performing fix-it
+  // code completion since we don't handle forwarding the fix-it for
+  // Objective-C objects. Since Objective-C properties are normally backed
+  // by ivars, most Objecive-C fix-its here would have little value 
anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }
   ObjCInterfaceDecl *Class = nullptr;
   if (const ObjCObjectPointerType *ObjCPtr =
   BaseType->getAs())


Index: clang/test/CodeCompletion/objc-member-access.m
===
--- /dev/null
+++ clang/test/CodeCompletion/objc-member-access.m
@@ -0,0 +1,22 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+@interface TypeWithPropertiesBackedByIvars {
+  int _bar;
+  int _foo;
+}
+@property(nonatomic) int foo;
+@property(nonatomic) int bar;
+@end
+
+int getFoo(id object) {
+  TypeWithPropertiesBackedByIvars *model = (TypeWithPropertiesBackedByIvars *)object;
+  int foo = model.;
+  return foo;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:14:19 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1-NOT: [#int#]_bar
+// CHECK-CC1-NOT: [#int#]_foo
+// CHECK-CC1: [#int#]bar
+// CHECK-CC1: [#int#]foo
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5195,7 +5195,13 @@
 Results.AddResult(std::move(Result));
   }
 } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
-  // Objective-C property reference.
+  // Objective-C property reference. Bail if we're performing fix-it code
+  // completion since we don't handle forwarding the fix-it for Objective-C
+  // objects. Since Objective-C properties are normally backed by ivars,
+  // most Objecive-C fix-its here would have little value anyways.
+  if (AccessOpFixIt.hasValue()) {
+return false;
+  }
   AddedPropertiesSet AddedProperties;
 
   if (const ObjCObjectPointerType *ObjCPtr =
@@ -5215,7 +5221,13 @@
   /*InOriginalClass*/ false);
 } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
(!IsArrow && BaseType->isObjCObjectType())) {
-  // Objective-C instance variable access.
+  // Objective-C instance variable access. Bail if we're performing fix-it
+  // code completion since we don't handle forwarding the fix-it for
+  // Objective-

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5153
 
-  auto DoCompletion = [&](Expr *Base, bool IsArrow,
+  auto DoCompletion = [&](Expr *Base, bool IsArrow, bool IncludeObjC,
   Optional AccessOpFixIt) -> bool {

sammccall wrote:
> IncludeObjC is always `!AccessOpFixIt.hasValue()` - use that directly with a 
> suitable comment?
Ah good call, done



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5251
+
+  // Fixits are only included for C++. We avoid this for Objective-C properties
+  // and ivars since most properties are backed by an ivar; otherwise we would

sammccall wrote:
> This first sentence isn't accurate - they're included for C, and even in 
> obj-c, just not for obj-c objects.
> I'd suggest dropping it and also the note about alternative solutions, unless 
> there's a reason we're likely to want to do that instead in the future.
Removed this in favor of a comment in the checks above. Think they're useful 
(albeit duplicated) since they give some explanation as to why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263



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

[PATCH] D81384: [AST] Fix a clang crash on an invalid for-range statement.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaStmt.cpp:2437
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())

some details:
for the crash case, the RangeType is a `Bar` class specialization, 
`RequireCompleteType` generates a definition decl of `Bar`, which is 
invalid, and `RequireCompleteType` returns true, and doesn't emit a diagnostic 
(seems to suppress the "incomplete-type" diagnostic for invalid decls), so the 
trick `InvalidateOnErrorScope` (on Line 2396) doesn't invalidate the LoopVar 
Decl, we have to manually invalidate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81384



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

[PATCH] D81352: Thread safety analysis: Add note for double unlock

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a nit.




Comment at: clang/include/clang/Analysis/Analyses/ThreadSafety.h:111
   /// \param Loc -- The SourceLocation of the Unlock
+  /// \param LocPreviousUnlock -- Optionally the location of a previous Unlock.
   virtual void handleUnmatchedUnlock(StringRef Kind, Name LockName,

The parameter itself isn't optional, so the "optionally" seems a bit strange to 
me. I think it should say, `If the source location is valid, it represents the 
location of a previous Unlock`, or rework the interface to use an 
`llvm::Optional`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81352



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

The patch as it stands now LGTM and I think it can be committed. Is there any 
objection remaining?

Any further comments @simoncook @asb?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

[Gabor Marton via Phabricator via cfe-commits]

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

Thank you Kristof for working on this. Looks good to me as it is!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Tobias Hieta via Phabricator via cfe-commits]

thieta created this revision.
thieta added reviewers: mstorsjo, beanz, lebedev.ri.
thieta added a project: clang.
Herald added subscribers: cfe-commits, mgorny.

As noted in https://reviews.llvm.org/D80492 that change broke standalone builds 
of clang. This reverts the old behavior of libdl linking when in standalone 
mode.

As a note I think it would be safe to just do:

`list(APPEND LIBS ${CMAKE_DL_LIBS})`

But I opted for the pragmatic fix this time. Let me know if you think we should 
try that instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81385

Files:
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81154: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 269197.
hokein marked 5 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81154

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/test/SemaCXX/invalid-constructor-init.cpp


Index: clang/test/SemaCXX/invalid-constructor-init.cpp
===
--- clang/test/SemaCXX/invalid-constructor-init.cpp
+++ clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -14,6 +14,12 @@
   constexpr X2() {} // expected-error {{constexpr constructor never produces a 
constant expression}}
 };
 
+struct X3 {
+  int Y;
+  constexpr X3() // expected-error {{constexpr constructor never produces}}
+  : Y(({foo();})) {} // expected-error {{use of undeclared identifier 
'foo'}}
+};
+
 struct CycleDelegate {
   int Y;
   CycleDelegate(int)
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -128,15 +128,19 @@
 }
 
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
-  // FIXME: why is unexpanded-pack not propagated?
-  auto D = toExprDependence(E->getType()->getDependence()) &
-   ~ExprDependence::UnexpandedPack;
+  auto D = toExprDependence(E->getType()->getDependence());
+  // Propagate dependence of the result.
+  if (const auto *CompoundExprResult =
+  dyn_cast_or_null(E->getSubStmt()->getStmtExprResult()))
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
   // lambda-expressions and GCC.
   if (TemplateDepth)
 D |= ExprDependence::ValueInstantiation;
-  return D;
+  // A param pack cannot be expanded over stmtexpr boundaries.
+  return D & ~ExprDependence::UnexpandedPack;
 }
 
 ExprDependence clang::computeDependence(ConvertVectorExpr *E) {


Index: clang/test/SemaCXX/invalid-constructor-init.cpp
===
--- clang/test/SemaCXX/invalid-constructor-init.cpp
+++ clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -14,6 +14,12 @@
   constexpr X2() {} // expected-error {{constexpr constructor never produces a constant expression}}
 };
 
+struct X3 {
+  int Y;
+  constexpr X3() // expected-error {{constexpr constructor never produces}}
+  : Y(({foo();})) {} // expected-error {{use of undeclared identifier 'foo'}}
+};
+
 struct CycleDelegate {
   int Y;
   CycleDelegate(int)
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -128,15 +128,19 @@
 }
 
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
-  // FIXME: why is unexpanded-pack not propagated?
-  auto D = toExprDependence(E->getType()->getDependence()) &
-   ~ExprDependence::UnexpandedPack;
+  auto D = toExprDependence(E->getType()->getDependence());
+  // Propagate dependence of the result.
+  if (const auto *CompoundExprResult =
+  dyn_cast_or_null(E->getSubStmt()->getStmtExprResult()))
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
   // lambda-expressions and GCC.
   if (TemplateDepth)
 D |= ExprDependence::ValueInstantiation;
-  return D;
+  // A param pack cannot be expanded over stmtexpr boundaries.
+  return D & ~ExprDependence::UnexpandedPack;
 }
 
 ExprDependence clang::computeDependence(ConvertVectorExpr *E) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@lebedev.ri fix for your issue posted here: https://reviews.llvm.org/D81385


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80492



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

[PATCH] D81154: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/lib/AST/ComputeDependence.cpp:132
   // FIXME: why is unexpanded-pack not propagated?
   auto D = toExprDependence(E->getType()->getDependence()) &
~ExprDependence::UnexpandedPack;

sammccall wrote:
> I wonder if this is still needed - how can we get a type for the stmtexpr 
> unless we have a result expr providing the type?
it is possible that the stmtexpr has a type and doesn't have a result expr, e.g 
`({ if (1) {}; })`, the default type is `void`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81154



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

[clang] 28ccd09 - [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-06-08T15:31:08+02:00
New Revision: 28ccd09d700311f0d1ce3828a614aad231bdbb22

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

LOG: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result 
expr to StmtExpr.

Summary:
We lost errorBit for StmtExpr if a recoveryExpr is the result
expr of a CompoundStmt, which will lead to crashes.

```
// `-StmtExpr
//   `-CompoundStmt
// `-RecoveryExp
({ invalid(); });
```

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/AST/ComputeDependence.cpp
clang/test/SemaCXX/invalid-constructor-init.cpp

Removed: 




diff  --git a/clang/lib/AST/ComputeDependence.cpp 
b/clang/lib/AST/ComputeDependence.cpp
index 3dcb9ba52356..53c43b194b38 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -128,15 +128,19 @@ ExprDependence 
clang::computeDependence(BinaryConditionalOperator *E) {
 }
 
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
-  // FIXME: why is unexpanded-pack not propagated?
-  auto D = toExprDependence(E->getType()->getDependence()) &
-   ~ExprDependence::UnexpandedPack;
+  auto D = toExprDependence(E->getType()->getDependence());
+  // Propagate dependence of the result.
+  if (const auto *CompoundExprResult =
+  dyn_cast_or_null(E->getSubStmt()->getStmtExprResult()))
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
   // lambda-expressions and GCC.
   if (TemplateDepth)
 D |= ExprDependence::ValueInstantiation;
-  return D;
+  // A param pack cannot be expanded over stmtexpr boundaries.
+  return D & ~ExprDependence::UnexpandedPack;
 }
 
 ExprDependence clang::computeDependence(ConvertVectorExpr *E) {

diff  --git a/clang/test/SemaCXX/invalid-constructor-init.cpp 
b/clang/test/SemaCXX/invalid-constructor-init.cpp
index 8fda9cc525ba..df10afb1d726 100644
--- a/clang/test/SemaCXX/invalid-constructor-init.cpp
+++ b/clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -14,6 +14,12 @@ struct X2 {
   constexpr X2() {} // expected-error {{constexpr constructor never produces a 
constant expression}}
 };
 
+struct X3 {
+  int Y;
+  constexpr X3() // expected-error {{constexpr constructor never produces}}
+  : Y(({foo();})) {} // expected-error {{use of undeclared identifier 
'foo'}}
+};
+
 struct CycleDelegate {
   int Y;
   CycleDelegate(int)



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

[PATCH] D81154: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG28ccd09d7003: [AST][RecoveryExpr] Populate the dependence 
bits from CompoundStmt result expr… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81154

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/test/SemaCXX/invalid-constructor-init.cpp


Index: clang/test/SemaCXX/invalid-constructor-init.cpp
===
--- clang/test/SemaCXX/invalid-constructor-init.cpp
+++ clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -14,6 +14,12 @@
   constexpr X2() {} // expected-error {{constexpr constructor never produces a 
constant expression}}
 };
 
+struct X3 {
+  int Y;
+  constexpr X3() // expected-error {{constexpr constructor never produces}}
+  : Y(({foo();})) {} // expected-error {{use of undeclared identifier 
'foo'}}
+};
+
 struct CycleDelegate {
   int Y;
   CycleDelegate(int)
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -128,15 +128,19 @@
 }
 
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
-  // FIXME: why is unexpanded-pack not propagated?
-  auto D = toExprDependence(E->getType()->getDependence()) &
-   ~ExprDependence::UnexpandedPack;
+  auto D = toExprDependence(E->getType()->getDependence());
+  // Propagate dependence of the result.
+  if (const auto *CompoundExprResult =
+  dyn_cast_or_null(E->getSubStmt()->getStmtExprResult()))
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
   // lambda-expressions and GCC.
   if (TemplateDepth)
 D |= ExprDependence::ValueInstantiation;
-  return D;
+  // A param pack cannot be expanded over stmtexpr boundaries.
+  return D & ~ExprDependence::UnexpandedPack;
 }
 
 ExprDependence clang::computeDependence(ConvertVectorExpr *E) {


Index: clang/test/SemaCXX/invalid-constructor-init.cpp
===
--- clang/test/SemaCXX/invalid-constructor-init.cpp
+++ clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -14,6 +14,12 @@
   constexpr X2() {} // expected-error {{constexpr constructor never produces a constant expression}}
 };
 
+struct X3 {
+  int Y;
+  constexpr X3() // expected-error {{constexpr constructor never produces}}
+  : Y(({foo();})) {} // expected-error {{use of undeclared identifier 'foo'}}
+};
+
 struct CycleDelegate {
   int Y;
   CycleDelegate(int)
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -128,15 +128,19 @@
 }
 
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
-  // FIXME: why is unexpanded-pack not propagated?
-  auto D = toExprDependence(E->getType()->getDependence()) &
-   ~ExprDependence::UnexpandedPack;
+  auto D = toExprDependence(E->getType()->getDependence());
+  // Propagate dependence of the result.
+  if (const auto *CompoundExprResult =
+  dyn_cast_or_null(E->getSubStmt()->getStmtExprResult()))
+if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+  D |= ResultExpr->getDependence();
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
   // lambda-expressions and GCC.
   if (TemplateDepth)
 D |= ExprDependence::ValueInstantiation;
-  return D;
+  // A param pack cannot be expanded over stmtexpr boundaries.
+  return D & ~ExprDependence::UnexpandedPack;
 }
 
 ExprDependence clang::computeDependence(ConvertVectorExpr *E) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81388: [TEST] TreeTest.cpp - Add a comma to avoid build error with -werror

[Kan Shengchen via Phabricator via cfe-commits]

skan created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
skan added reviewers: gribozavr, hlopko, eduucaldas.

The macro `INSTANTIATE_TEST_CASE_P` is defined as

  \# define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator, ...) \
  ...

If we build the test case with -werror, we will get an error like

  error: ISO C++11 requires at least one argument for the "..." in a
  variadic macro
  
  testing::ValuesIn(TestClangConfig::allConfigs()));
  ^

This patch fixes that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81388

Files:
  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
@@ -2995,6 +2995,6 @@
 }
 
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(TestClangConfig::allConfigs()));
+testing::ValuesIn(TestClangConfig::allConfigs()), );
 
 } // namespace


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2995,6 +2995,6 @@
 }
 
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(TestClangConfig::allConfigs()));
+testing::ValuesIn(TestClangConfig::allConfigs()), );
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:190
+return N->kind() == NodeKind::NameSpecifier;
+  }
+};

Should there be getters for various parts of the specifier?



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:203
+
+/// An identifier expression, e.g. `n::S::a`. Modeled according to the grammar
+class IdExpression final : public Expression {

Could you add more details? Feel free to even quote the relevant grammar bits.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:211
+
+  syntax::Leaf *unqualifiedId();
+  syntax::NestedNameSpecifier *qualifier();

unqualified-id in the grammar can be more than one token.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:212
+  syntax::Leaf *unqualifiedId();
+  syntax::NestedNameSpecifier *qualifier();
+};

Could you swap the order of getters to match the source order?



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:618
+  Builder.foldNode(Builder.getRange(it.getLocalSourceRange()), NS, 
nullptr);
+  Builder.markChild(NS, syntax::NodeRole::Unknown);
+}

Do we need to mark the role if it is unknown?




Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:751
+void test(int b) {
+  ::a::b::S::f();
+}

Could you add more tests even if they fail today due to missing implementation? 

`a::b::operator-(a::b::S())`
`a::b::S2::f()`

etc. Basically cover the whole grammar in tests, if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81168



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added a comment.

I'm not a reviewer but the patch LGTM, thanks for all the changes @HsiangKai.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D81390: [KernelAddressSanitizer] Make globals constructors compatible with kernel

[Marco Elver via Phabricator via cfe-commits]

melver created this revision.
melver added a reviewer: glider.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, aprantl.
Herald added a reviewer: aaron.ballman.
Herald added projects: clang, LLVM.

[ The first version of this feature was reverted due to modpost causing

  failures. This version fixes this. More info:
  https://github.com/ClangBuiltLinux/linux/issues/1045#issuecomment-640381783 ]

This makes -fsanitize=kernel-address emit the correct globals
constructors for the kernel. We had to do the following:

- Disable generation of constructors that rely on linker features such as 
dead-global elimination.

- Only instrument globals *not* in explicit sections. The kernel uses sections 
for special globals, which we should not touch.

- Do not instrument globals that are aliased by a symbol that is prefixed with 
"__". For example, modpost relies on specially named aliases to find globals 
and checks their contents. Unfortunately mod post relies on size stored as ELF 
debug info and any padding of globals currently causes the debug info to cause 
size reported to be *with* redzone which throws modpost off.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203493

Tested:

1. With 'clang/test/CodeGen/asan-globals.cpp'.

2. With test_kasan.ko, we can see:

BUG: KASAN: global-out-of-bounds in kasan_global_oob+0xb3/0xba 
[test_kasan]

3. allyesconfig, allmodconfig


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81390

Files:
  clang/test/CodeGen/asan-globals.cpp
  llvm/include/llvm/Transforms/Utils/ModuleUtils.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/ModuleUtils.cpp

Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -119,6 +119,15 @@
   AttributeList());
 }
 
+Function *llvm::createSanitizerCtor(Module &M, StringRef CtorName) {
+  Function *Ctor = Function::Create(
+  FunctionType::get(Type::getVoidTy(M.getContext()), false),
+  GlobalValue::InternalLinkage, CtorName, &M);
+  BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
+  ReturnInst::Create(M.getContext(), CtorBB);
+  return Ctor;
+}
+
 std::pair llvm::createSanitizerCtorAndInitFunctions(
 Module &M, StringRef CtorName, StringRef InitName,
 ArrayRef InitArgTypes, ArrayRef InitArgs,
@@ -128,11 +137,8 @@
  "Sanitizer's init function expects different number of arguments");
   FunctionCallee InitFunction =
   declareSanitizerInitFunction(M, InitName, InitArgTypes);
-  Function *Ctor = Function::Create(
-  FunctionType::get(Type::getVoidTy(M.getContext()), false),
-  GlobalValue::InternalLinkage, CtorName, &M);
-  BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
-  IRBuilder<> IRB(ReturnInst::Create(M.getContext(), CtorBB));
+  Function *Ctor = createSanitizerCtor(M, CtorName);
+  IRBuilder<> IRB(Ctor->getEntryBlock().getTerminator());
   IRB.CreateCall(InitFunction, InitArgs);
   if (!VersionCheckName.empty()) {
 FunctionCallee VersionCheckFunction = M.getOrInsertFunction(
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -589,11 +589,10 @@
   AddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseAfterScope = false)
-  : UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
-this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-this->CompileKernel =
-ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+  : CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+: CompileKernel),
+Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
 C = &(M.getContext());
 LongSize = M.getDataLayout().getPointerSizeInBits();
 IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -742,7 +741,11 @@
   ModuleAddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
  bool CompileKernel = false, bool Recover = false,
  bool UseGlobalsGC = true, bool UseOdrIndicator = false)
-  : GlobalsMD(*GlobalsMD), UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC),
+  : GlobalsMD(*GlobalsMD),
+CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+: CompileKernel),
+Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+UseGlobals

[PATCH] D81392: [clang] Rename Decl::isHidden() to isUnconditionallyVisible()

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
mboehme added a reviewer: rsmith.
mboehme added a project: clang.
Herald added a subscriber: cfe-commits.

Also invert the sense of the return value.

As pointed out by the FIXME that this change resolves, isHidden() wasn't a very 
accurate name for this function.

I haven't yet changed any of the strings that are output in ASTDumper.cpp / 
JSONNodeDumper.cpp / TextNodeDumper.cpp in response to whether isHidden() is 
set because

a) I'm not sure whether it's actually desired to change these strings (would 
appreciate feedback on this), and

b) In any case, I'd like to get this pure rename out of the way first, without 
any changes to tests. Changing the strings that are output in the various 
...Dumper.cpp files will require changes to quite a few tests, and I'd like to 
make those in a separate change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81392

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6041,7 +6041,7 @@
 void ASTWriter::RedefinedHiddenDefinition(const NamedDecl *D, Module *M) {
   if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
-  assert(D->isHidden() && "expected a hidden declaration");
+  assert(!D->isUnconditionallyVisible() && "expected a hidden declaration");
   DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_EXPORTED, M));
 }
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4030,7 +4030,7 @@
 void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) {
   assert(Owner->NameVisibility != Module::Hidden && "nothing to make visible?");
   for (Decl *D : Names) {
-bool wasHidden = D->isHidden();
+bool wasHidden = !D->isUnconditionallyVisible();
 D->setVisibleDespiteOwningModule();
 
 if (wasHidden && SemaObj) {
@@ -4092,9 +4092,9 @@
 /// visible.
 void ASTReader::mergeDefinitionVisibility(NamedDecl *Def,
   NamedDecl *MergedDef) {
-  if (Def->isHidden()) {
+  if (!Def->isUnconditionallyVisible()) {
 // If MergedDef is visible or becomes visible, make the definition visible.
-if (!MergedDef->isHidden())
+if (MergedDef->isUnconditionallyVisible())
   Def->setVisibleDespiteOwningModule();
 else {
   getContext().mergeDefinitionIntoModule(
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1710,7 +1710,8 @@
 /// path (by instantiating a template, you allow it to see the declarations that
 /// your module can see, including those later on in your module).
 bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
-  assert(D->isHidden() && "should not call this: not in slow case");
+  assert(!D->isUnconditionallyVisible() &&
+ "should not call this: not in slow case");
 
   Module *DeclModule = SemaRef.getOwningModule(D);
   assert(DeclModule && "hidden decl has no owning module");
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -1272,7 +1272,8 @@
 
 static bool NestedProtocolHasNoDefinition(ObjCProtocolDecl *PDecl,
   ObjCProtocolDecl *&UndefinedProtocol) {
-  if (!PDecl->hasDefinition() || PDecl->getDefinition()->isHidden()) {
+  if (!PDecl->hasDefinition() ||
+  !PDecl->getDefinition()->isUnconditionallyVisible()) {
 UndefinedProtocol = PDecl;
 return true;
   }
@@ -3235,7 +3236,7 @@
 return false;
 
   // If either is hidden, it is not considered to match.
-  if (left->isHidden() || right->isHidden())
+  if (!left->isUnconditionallyVisible() || !right->isUnconditionallyVisible())
 return false;
 
   if (left->isDirectMethod() != right->isDirectMethod())
@@ -3494,7 +3495,7 @@
   ObjCMethodList &MethList = InstanceFirst ? Pos->second.first :
  Pos->second.second;
   for (ObjCMethodList *M = &MethList; M; M = M->getNext())
-if (M->getMethod() && !M->getMethod()->isHidden()) {
+if (M->getMethod() && M->getMethod()->isUnconditionallyVisible()) {
   if (FilterMethodsByTypeBound(M->getMethod(), TypeBound))

[PATCH] D81395: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Sema/Sema.h:3900
+  /// Rebuild the given Expr with the TypoExpr degraded to RecoveryExpr.
+  ExprResult rebuildTypoExprs(Expr *TypoExpr);
   /// Attempts to produce a RecoveryExpr after some AST node cannot be created.

not quite happy with the naming, open for suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81395



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

[PATCH] D81395: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein marked an inline comment as done.
hokein added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:3900
+  /// Rebuild the given Expr with the TypoExpr degraded to RecoveryExpr.
+  ExprResult rebuildTypoExprs(Expr *TypoExpr);
   /// Attempts to produce a RecoveryExpr after some AST node cannot be created.

not quite happy with the naming, open for suggestions.


And don't invalidate the VarDecl if the type is known.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81395

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-recovery.cpp

Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -25,8 +25,11 @@
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -176,6 +179,12 @@
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() {
   // CHECK: `-VarDecl {{.*}} invalid a 'auto'
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8305,17 +8305,7 @@
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
   if (FullExpr.isInvalid()) {
-// Typo-correction fails, we rebuild the broken AST with the typos degraded
-// to RecoveryExpr.
-struct TyposReplace : TreeTransform {
-  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
-  ExprResult TransformTypoExpr(TypoExpr *E) {
-return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
-E->getEndLoc(), {});
-  }
-} TT(*this);
-
-return TT.TransformExpr(FE);
+return rebuildTypoExprs(FE);
   }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19175,6 +19175,17 @@
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
 
+ExprResult Sema::rebuildTypoExprs(Expr *TypoExpr) {
+  struct TyposReplace : TreeTransform {
+TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+ExprResult TransformTypoExpr(clang::TypoExpr *E) {
+  return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
+  {});
+}
+  } TT(*this);
+  return TT.TransformExpr(TypoExpr);
+}
+
 ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
 ArrayRef SubExprs, QualType T) {
   // FIXME: enable it for C++, RecoveryExpr is type-dependent to suppress
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12015,6 +12015,9 @@
 InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
 return Init.Failed() ? ExprError() : E;
   });
+  if (Res.isInvalid())
+Res = rebuildTypoExprs(Args[Idx]);
+
   if (Res.isInvalid()) {
 VDecl->setInvalidDecl();
   } else if (Res.get() != Args[Idx]) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3896,6 +3896,8 @@
   void DiagnoseAmbiguousLookup(LookupResult &Result);
   //@}
 
+  /// Rebuild the given Expr with the TypoExpr degraded to RecoveryExpr.
+  ExprResult rebuildTypoExprs(Expr *TypoExpr);
   /// Attempts to produce a RecoveryExpr after some AST node cannot be created.
   ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
 ArrayRef SubExprs,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

[Caroline via Phabricator via cfe-commits]

CarolineConcatto abandoned this revision.
CarolineConcatto added a comment.

I'll close this PR as we believe we will not need this flag for Flang driver.
There are some development happening in:
https://github.com/banach-space/llvm-project
for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73951



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

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

[Caroline via Phabricator via cfe-commits]

CarolineConcatto added a comment.

I'll close this PR as we believe we will not need this flag for Flang driver.
There are some development happening in:
https://github.com/banach-space/llvm-project
for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73951



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

[PATCH] D81388: [TEST] TreeTest.cpp - Add a comma to avoid build error with -werror

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

Related revision: D80822 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81388



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

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137-142
+.Case("static", llvm::Reloc::Static)
+.Case("pic", llvm::Reloc::PIC_)
+.Case("ropi", llvm::Reloc::ROPI)
+.Case("rwpi", llvm::Reloc::RWPI)
+.Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
+.Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)

dexonsmith wrote:
> I wonder if it's worth creating a `.def` file for the driver enums, something 
> like:
> ```
> #ifdef HANDLE_RELOCATION_MODEL
> HANDLE_RELOCATION_MODEL("static", llvm::Reloc::Static)
> HANDLE_RELOCATION_MODEL("pic", llvm::Reloc::PIC_)
> HANDLE_RELOCATION_MODEL("ropi", llvm::Reloc::ROPI)
> HANDLE_RELOCATION_MODEL("rwpi", llvm::Reloc::RWPI)
> HANDLE_RELOCATION_MODEL("ropi-rwpio", llvm::Reloc::ROPI_RWPI)
> HANDLE_RELOCATION_MODEL("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
> #undef HANDLE_RELOCATION_MODEL
> #endif // HANDLE_RELOCATION_MODEL
> 
> #ifdef HANDLE_DEBUG_INFO_KIND
> HANDLE_DEBUG_INFO_KIND("line-tables-only", 
> codegenoptions::DebugLineTablesOnly)
> HANDLE_DEBUG_INFO_KIND("line-directives-only", 
> codegenoptions::DebugDirectivesOnly)
> HANDLE_DEBUG_INFO_KIND("limited", codegenoptions::LimitedDebugInfo)
> HANDLE_DEBUG_INFO_KIND("standalone", codegenoptions::FullDebugInfo)
> #undef HANDLE_DEBUG_INFO_KIND
> #endif // HANDLE_DEBUG_INFO_KIND
> 
> // ...
> ```
> Then you can use `HANDLE_RELOCATION_MODEL` in both `normalize` and 
> `denormalize`, rather than duplicating the table.
> 
> Maybe we can go even further. Can you expand the `Values` array from the 
> tablegen to include this info? Or rejigger the help text to leverage 
> `HANDLE_RELOCATION_MODEL` (maybe pass in 
> `ValuesDefine`)? The current patch adds a value 
> table; my first suggestion leaves us even; but maybe we can get rid of one.
I think this suggestion, I can definitely do at least this to generate the 
necessary switch statements. I don't see why this should be a separate .def 
file, I can generate this from the tablegen with an extra field named 
`ValuesAssociatedDefinitions` or something to that effect. I'll do that now.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3647
+ DiagnosticsEngine &Diags) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,   
\

dexonsmith wrote:
> Seems like `Options.inc` could provide this as a default definition, not sure 
> if that seems error-prone?
Not sure how much of this do you mean? `OPTION_WITH_MARSHALLING` is a 
convenience thing that forces users to opt into getting the marshalling 
definitions. I think it might be better to provide default empty definitions 
for `OPTION_WITH_MARSHALLING_FLAG` and friends to achieve a similar effect 
without needing `OPTION_WITH_MARSHALLING`. I you mean the actual definitions 
here they rely on the `ArgList &` to be named Args which might make it error 
prone to include these as defaults.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3665-3667
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING

dexonsmith wrote:
> I prefer the style where the `.inc` file is responsible for the `#undef` 
> calls. It avoids having to duplicate it everywhere (and the risk of 
> forgetting it). WDYT?
I prefer it too, but the current tablegen backend doesn't generate them for 
other macros it defines, so I wanted to make it consistent... I could change 
the existing backend to work that way but I would then need to track all the 
usages of this across all the llvm-projects which I don't fancy doing. Let me 
know if you thing it is fine to break with the existing style for this one.


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

https://reviews.llvm.org/D79796



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

[PATCH] D81352: Thread safety analysis: Add note for double unlock

[Aaron Puchert via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/Analyses/ThreadSafety.h:111
   /// \param Loc -- The SourceLocation of the Unlock
+  /// \param LocPreviousUnlock -- Optionally the location of a previous Unlock.
   virtual void handleUnmatchedUnlock(StringRef Kind, Name LockName,

aaron.ballman wrote:
> The parameter itself isn't optional, so the "optionally" seems a bit strange 
> to me. I think it should say, `If the source location is valid, it represents 
> the location of a previous Unlock`, or rework the interface to use an 
> `llvm::Optional`.
The former (not available = invalid) is what I intended, so I'll rephrase this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81352



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

[PATCH] D81332: Thread safety analysis: Support deferring locks

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D81332#2079489 , @aaron.ballman 
wrote:

> LGTM! Should we update the public docs for this change as well? Specifically, 
> I am wondering if we should update 
> https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader


Yes, some existing functionality seems undocumented as well (like adopting 
locks), which is why I was planning a bigger change.

My plan is basically to go through the test cases (which seem to be quite 
complete in terms of what we intend to support) and see what deserves mention 
in the official docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81332



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

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: aaron.ballman, gribozavr2, JonasToth, 
alexfh, hokein.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: martong, steakhal, gamesh411, Szelethus, dkrupp, 
rnkovacs, xazax.hun, whisperity, mgorny.
Herald added a project: clang.
baloghadamsoftware added a child revision: D81272: [clang-tidy] New check 
`misc-redundant-condition`.

Function `hasPtrOrReferenceInfFunc()` of `bugprone-infinite-loop` is a generic 
function which could be reused in another checks. This patch moves this 
function into a newly created utility module.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81396

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/clang-tidy/utils/Aliasing.cpp
  clang-tools-extra/clang-tidy/utils/Aliasing.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt

Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/utils/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_library(clangTidyUtils
+  Aliasing.cpp
   ASTUtils.cpp
   DeclRefExprUtils.cpp
   ExceptionAnalyzer.cpp
Index: clang-tools-extra/clang-tidy/utils/Aliasing.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/Aliasing.h
@@ -0,0 +1,25 @@
+//===- Aliasing.h - clang-tidy ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
+
+#include "clang/AST/Decl.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+
+/// Returns whether a ``Var`` has a pointer or reference in ``Func``
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var);
+
+} // namespace utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H
Index: clang-tools-extra/clang-tidy/utils/Aliasing.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -0,0 +1,66 @@
+//===- Aliasing.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Aliasing.h"
+
+#include "clang/AST/Expr.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+
+/// Return whether `S` is a reference to the declaration of `Var`.
+static bool isAccessForVar(const Stmt *S, const VarDecl *Var) {
+  if (const auto *DRE = dyn_cast(S))
+return DRE->getDecl() == Var;
+
+  return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `S`.
+static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
+  if (const auto *DS = dyn_cast(S)) {
+for (const Decl *D : DS->getDeclGroup()) {
+  if (const auto *LeftVar = dyn_cast(D)) {
+if (LeftVar->hasInit() && LeftVar->getType()->isReferenceType()) {
+  return isAccessForVar(LeftVar->getInit(), Var);
+}
+  }
+}
+  } else if (const auto *UnOp = dyn_cast(S)) {
+if (UnOp->getOpcode() == UO_AddrOf)
+  return isAccessForVar(UnOp->getSubExpr(), Var);
+  }
+
+  return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `S`.
+static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
+  if (isPtrOrReferenceForVar(S, Var))
+return true;
+
+  for (const Stmt *Child : S->children()) {
+if (!Child)
+  continue;
+
+if (hasPtrOrReferenceInStmt(Child, Var))
+  return true;
+  }
+
+  return false;
+}
+
+/// Return whether `Var` has a pointer or reference in `Func`.
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) {
+  return hasPtrOrReferenceInStmt(Func->getBody(), Var);
+}
+
+} // namespace utils
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -10,8 +10,10 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "../utils/Aliasing.h"

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

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

baloghadamsoftware marked 11 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:39
+ declRefExpr(hasDeclaration(varDecl().bind("cond_var"))),
+ binaryOperator(hasOperatorName("&&"),
+hasEitherOperand(declRefExpr(hasDeclaration(

njames93 wrote:
> Recursive matchers are a pain, but this will miss:
>  ```
> if (A && B && ... Y && Z)
> ```
> 
I suppose that for this purpose we have to make our own matcher? Right? This 
also makes the fixits more complex. I think this could be done in a later 
phase. I added a `FIXME` now.


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

https://reviews.llvm.org/D81272



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

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

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

baloghadamsoftware updated this revision to Diff 269222.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D81272

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp
  clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-condition.cpp
@@ -0,0 +1,1058 @@
+// RUN: %check_clang_tidy %s misc-redundant-condition %t
+
+extern unsigned peopleInTheBuilding;
+extern unsigned fireFighters;
+
+bool isBurning();
+bool isReallyBurning();
+bool isCollapsing();
+void tryToExtinguish(bool&);
+void scream();
+
+bool someOtherCondition();
+
+//===--- Basic Positives --===//
+
+void positive_direct() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+}
+  }
+}
+
+void positive_direct_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire && peopleInTheBuilding > 0) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+  // CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire && peopleInTheBuilding > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+// CHECK-FIXES: if ( peopleInTheBuilding > 0) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (peopleInTheBuilding > 0 && onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+  // CHECK-FIXES: if (peopleInTheBuilding > 0) {
+  scream();
+}
+  }
+}
+
+void positive_indirect_inner_and_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (peopleInTheBuilding > 0 && onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+// CHECK-FIXES: if (peopleInTheBuilding > 0) {
+scream();
+  }
+}
+  }
+}
+
+void positive_direct_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (onFire || isCollapsing()) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_lhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (onFire || isCollapsing()) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+  }
+  // CHECK-FIXES: {{^\ *$}}
+}
+  }
+}
+
+void positive_direct_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (isCollapsing() || onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition]
+  // CHECK-FIXES: {{^\ *$}}
+  scream();
+}
+// CHECK-FIXES: {{^\ *$}}
+  }
+}
+
+void positive_indirect_inner_or_rhs() {
+  bool onFire = isBurning();
+  if (onFire) {
+if (someOtherCondition()) {
+  if (isCollapsing() || onFire) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [misc-redundant-condition]
+// CHECK-FIXES: {{^\ *$}}
+scream();
+  }
+  // CHECK-FIXES: {{^\ *$}}
+}
+  }
+}
+
+void positive_direct_outer_and_lhs() {
+  bool onFire = isBurning();
+  if (onFire && fireFighters < 10) {
+if (onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [misc-redundant-condition

[clang] 1850f56 - Thread safety analysis: Support deferring locks

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-06-08T17:00:29+02:00
New Revision: 1850f56c8abae637c2cc1b8d27b8577c5700101a

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

LOG: Thread safety analysis: Support deferring locks

Summary:
The standard std::unique_lock can be constructed to manage a lock without
initially acquiring it by passing std::defer_lock as second parameter.
It can be acquired later by calling lock().

To support this, we use the locks_excluded attribute. This might seem
like an odd choice at first, but its consistent with the other
annotations we support on scoped capability constructors. By excluding
the lock we state that it is currently not in use and the function
doesn't change that, which is exactly what the constructor does.

Along the way we slightly simplify handling of scoped capabilities.

Reviewers: aaron.ballman, delesley

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index e0ff23df5ab4..6e518e7d38ef 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -905,11 +905,7 @@ class ScopedLockableFactEntry : public FactEntry {
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
   : FactEntry(CE, LK_Exclusive, Loc, false) {}
 
-  void addExclusiveLock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-  }
-
-  void addSharedLock(const CapabilityExpr &M) {
+  void addLock(const CapabilityExpr &M) {
 UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
   }
 
@@ -1801,7 +1797,7 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
   SourceLocation Loc = Exp->getExprLoc();
   CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd;
   CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
-  CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
+  CapExprSet ScopedReqsAndExcludes;
   StringRef CapDiagKind = "mutex";
 
   // Figure out if we're constructing an object of scoped lockable class
@@ -1892,19 +1888,20 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
  POK_FunctionCall, ClassifyDiagnostic(A),
  Exp->getExprLoc());
   // use for adopting a lock
-  if (isScopedVar) {
-Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
-: ScopedExclusiveReqs,
-  A, Exp, D, VD);
-  }
+  if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
 }
 break;
   }
 
   case attr::LocksExcluded: {
 const auto *A = cast(At);
-for (auto *Arg : A->args())
+for (auto *Arg : A->args()) {
   warnIfMutexHeld(D, Exp, Arg, ClassifyDiagnostic(A));
+  // use for deferring a lock
+  if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
+}
 break;
   }
 
@@ -1944,13 +1941,11 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
 
 auto ScopedEntry = std::make_unique(Scp, MLoc);
 for (const auto &M : ExclusiveLocksToAdd)
-  ScopedEntry->addExclusiveLock(M);
-for (const auto &M : ScopedExclusiveReqs)
-  ScopedEntry->addExclusiveLock(M);
+  ScopedEntry->addLock(M);
 for (const auto &M : SharedLocksToAdd)
-  ScopedEntry->addSharedLock(M);
-for (const auto &M : ScopedSharedReqs)
-  ScopedEntry->addSharedLock(M);
+  ScopedEntry->addLock(M);
+for (const auto &M : ScopedReqsAndExcludes)
+  ScopedEntry->addLock(M);
 for (const auto &M : ExclusiveLocksToRemove)
   ScopedEntry->addExclusiveUnlock(M);
 for (const auto &M : SharedLocksToRemove)

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index cdb22cd22a99..02700147a032 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2625,9 +2625,12 @@ void Foo::test5() {
 
 namespace RelockableScopedLock {
 
+class DeferTraits {};
+
 class SCOPED_LOCKABLE RelockableExclusiveMutexLock {
 public:
   RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+  RelockableExclusiveMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu);
   ~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION();
 
   void Lock() EXCLUSIVE_LOCK_FUNCTION();
@@ -2639,6 +2642,7 @@ struct ExclusiveTraits {};
 
 c

[clang] f70912f - Thread safety analysis: Add note for double unlock

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-06-08T17:00:29+02:00
New Revision: f70912f885f991d5af11d8ecb10b703f3cbed982

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

LOG: Thread safety analysis: Add note for double unlock

Summary:
When getting a warning that we release a capability that isn't held it's
sometimes not clear why. So just like we do for double locking, we add a
note on the previous release operation, which marks the point since when
the capability isn't held any longer.

We can find this previous release operation by looking up the
corresponding negative capability.

Reviewers: aaron.ballman, delesley

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/ThreadSafety.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Sema/warn-thread-safety-analysis.c
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 18659aa4e5bb..0d3dda1256fb 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -108,8 +108,10 @@ class ThreadSafetyHandler {
   /// \param LockName -- A StringRef name for the lock expression, to be 
printed
   /// in the error message.
   /// \param Loc -- The SourceLocation of the Unlock
+  /// \param LocPreviousUnlock -- If valid, the location of a previous Unlock.
   virtual void handleUnmatchedUnlock(StringRef Kind, Name LockName,
- SourceLocation Loc) {}
+ SourceLocation Loc,
+ SourceLocation LocPreviousUnlock) {}
 
   /// Warn about an unlock function call that attempts to unlock a lock with
   /// the incorrect lock kind. For instance, a shared lock being unlocked

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 45a7f1c700b4..cce6dcc54c2f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3401,6 +3401,7 @@ def warn_expecting_lock_held_on_loop : Warning<
   "expecting %0 '%1' to be held at start of each loop">,
   InGroup, DefaultIgnore;
 def note_locked_here : Note<"%0 acquired here">;
+def note_unlocked_here : Note<"%0 released here">;
 def warn_lock_exclusive_and_shared : Warning<
   "%0 '%1' is acquired exclusively and shared in the same scope">,
   InGroup, DefaultIgnore;

diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 6e518e7d38ef..1208eaf93e25 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -995,7 +995,10 @@ class ScopedLockableFactEntry : public FactEntry {
   FSet.addLock(FactMan, std::make_unique(
 !Cp, LK_Exclusive, loc));
 } else if (Handler) {
-  Handler->handleUnmatchedUnlock(DiagKind, Cp.toString(), loc);
+  SourceLocation PrevLoc;
+  if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
+PrevLoc = Neg->loc();
+  Handler->handleUnmatchedUnlock(DiagKind, Cp.toString(), loc, PrevLoc);
 }
   }
 };
@@ -1322,7 +1325,10 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, 
const CapabilityExpr &Cp,
 
   const FactEntry *LDat = FSet.findLock(FactMan, Cp);
   if (!LDat) {
-Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc);
+SourceLocation PrevLoc;
+if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
+  PrevLoc = Neg->loc();
+Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc, PrevLoc);
 return;
   }
 

diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp 
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 995d776d6565..3b7356893833 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1700,6 +1700,14 @@ class ThreadSafetyReporter : public 
clang::threadSafety::ThreadSafetyHandler {
: getNotes();
   }
 
+  OptionalNotes makeUnlockedHereNote(SourceLocation LocUnlocked,
+ StringRef Kind) {
+return LocUnlocked.isValid()
+   ? getNotes(PartialDiagnosticAt(
+ LocUnlocked, S.PDiag(diag::note_unlocked_here) << Kind))
+   : getNotes();
+  }
+
  public:
   ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
 : S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1726,13 +1734,14 @@ class ThreadSafetyReporter : public 
clang::thre

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst:6
+
+Finds condition variables in nested `if` statements that were also checked in
+the outer `if` statement and were not changed.

Please use double back-ticks for language constructs. Same below.


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

https://reviews.llvm.org/D81272



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

[PATCH] D81390: [KernelAddressSanitizer] Make globals constructors compatible with kernel

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 269229.
melver added a comment.

Also ignore non-alias globals prefixed by __.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81390

Files:
  clang/test/CodeGen/asan-globals.cpp
  llvm/include/llvm/Transforms/Utils/ModuleUtils.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/ModuleUtils.cpp

Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -119,6 +119,15 @@
   AttributeList());
 }
 
+Function *llvm::createSanitizerCtor(Module &M, StringRef CtorName) {
+  Function *Ctor = Function::Create(
+  FunctionType::get(Type::getVoidTy(M.getContext()), false),
+  GlobalValue::InternalLinkage, CtorName, &M);
+  BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
+  ReturnInst::Create(M.getContext(), CtorBB);
+  return Ctor;
+}
+
 std::pair llvm::createSanitizerCtorAndInitFunctions(
 Module &M, StringRef CtorName, StringRef InitName,
 ArrayRef InitArgTypes, ArrayRef InitArgs,
@@ -128,11 +137,8 @@
  "Sanitizer's init function expects different number of arguments");
   FunctionCallee InitFunction =
   declareSanitizerInitFunction(M, InitName, InitArgTypes);
-  Function *Ctor = Function::Create(
-  FunctionType::get(Type::getVoidTy(M.getContext()), false),
-  GlobalValue::InternalLinkage, CtorName, &M);
-  BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
-  IRBuilder<> IRB(ReturnInst::Create(M.getContext(), CtorBB));
+  Function *Ctor = createSanitizerCtor(M, CtorName);
+  IRBuilder<> IRB(Ctor->getEntryBlock().getTerminator());
   IRB.CreateCall(InitFunction, InitArgs);
   if (!VersionCheckName.empty()) {
 FunctionCallee VersionCheckFunction = M.getOrInsertFunction(
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -589,11 +589,10 @@
   AddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseAfterScope = false)
-  : UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
-this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-this->CompileKernel =
-ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+  : CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+: CompileKernel),
+Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
 C = &(M.getContext());
 LongSize = M.getDataLayout().getPointerSizeInBits();
 IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -742,7 +741,11 @@
   ModuleAddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
  bool CompileKernel = false, bool Recover = false,
  bool UseGlobalsGC = true, bool UseOdrIndicator = false)
-  : GlobalsMD(*GlobalsMD), UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC),
+  : GlobalsMD(*GlobalsMD),
+CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+: CompileKernel),
+Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC && !this->CompileKernel),
 // Enable aliases as they should have no downside with ODR indicators.
 UsePrivateAlias(UseOdrIndicator || ClUsePrivateAlias),
 UseOdrIndicator(UseOdrIndicator || ClUseOdrIndicator),
@@ -753,11 +756,7 @@
 // argument is designed as workaround. Therefore, disable both
 // ClWithComdat and ClUseGlobalsGC unless the frontend says it's ok to
 // do globals-gc.
-UseCtorComdat(UseGlobalsGC && ClWithComdat) {
-this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-this->CompileKernel =
-ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
 C = &(M.getContext());
 int LongSize = M.getDataLayout().getPointerSizeInBits();
 IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -792,8 +791,9 @@
   StringRef InternalSuffix);
   Instruction *CreateAsanModuleDtor(Module &M);
 
-  bool ShouldInstrumentGlobal(GlobalVariable *G);
-  bool ShouldUseMachOGlobalsSection() const;
+  bool canInstrumentAliasedGlobal(const GlobalAlias &GA) const;
+  bool shoul

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

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

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

LGTM except for the nit I put in a comment.

However, I have little experience with these particular pragmas.  Before 
pushing, please give others a few more days in case they want to review.




Comment at: clang/test/AST/sourceranges.cpp:116
+  void f() {
+// CHECK: AttributedStmt {{.*}} 
+DO_PRAGMA (unroll(2))

Is there any reason not to check for `LoopHintAttr` here and on the `_Pragma` 
case below?  If so, I suggest a comment to explain why these cases are 
different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80944



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

[PATCH] D81384: [AST] Fix a clang crash on an invalid for-range statement.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/for-range-crash.cpp:6
+  Bar *variables_to_modify;
+   foo() {  // expected-error {{C++ requires a type specifier for all 
declarations}}
+for (auto *c : *variables_to_modify)

What's the significance of the missing return type here? Can we add void)

Can we move this function out of the class?

(In both cases just to remove any confusion about what the problematic pattern 
actually is)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81384



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

@efriedma Any more comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D81395: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Very nice! Just want to give some thought to what these callsites look like.




Comment at: clang/include/clang/Sema/Sema.h:3899
 
+  /// Rebuild the given Expr with the TypoExpr degraded to RecoveryExpr.
+  ExprResult rebuildTypoExprs(Expr *TypoExpr);

This definitely seems like something that makes sense to call in more places. 
Wonder if the API can be smoother.

Both of the examples are of the form: CorrectDelayedTypos, if invalid, 
rebuildTypos.
Are *all* likely cases of this form? (Could make it an optional part of 
CorrectDelayedTypos)
Are there any places where you want CorrectDelayedTypos and don't want rebuild? 
(Could make it an eventually-mandatory part of CorrectDelayedTypos)



Comment at: clang/include/clang/Sema/Sema.h:3900
+  /// Rebuild the given Expr with the TypoExpr degraded to RecoveryExpr.
+  ExprResult rebuildTypoExprs(Expr *TypoExpr);
   /// Attempts to produce a RecoveryExpr after some AST node cannot be created.

hokein wrote:
> not quite happy with the naming, open for suggestions.
given current API, I'd suggest RecoverFromUncorrectedTypos or so


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81395



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

[PATCH] D81098: [OpenMP] Upgrade default version of OpenMP to 5.0

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 269239.
saiislam added a comment.

Fixed 25 out of 75 failing test cases by marking all old default cases as omp4 
tests, and new tests as omp5.
All these tests are now specific to the version of openmp, so that we don't run 
into similar problem during next version upgrade.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81098

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/distribute_simd_loop_messages.cpp
  clang/test/OpenMP/driver.c
  clang/test/OpenMP/for_collapse_messages.cpp
  clang/test/OpenMP/for_loop_messages.cpp
  clang/test/OpenMP/for_simd_loop_messages.cpp
  clang/test/OpenMP/master_taskloop_loop_messages.cpp
  clang/test/OpenMP/master_taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/nesting_of_regions.cpp
  clang/test/OpenMP/parallel_for_loop_messages.cpp
  clang/test/OpenMP/parallel_for_simd_codegen.cpp
  clang/test/OpenMP/parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_loop_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/simd_loop_messages.cpp
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/taskloop_loop_messages.cpp
  clang/test/OpenMP/taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp

Index: clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
+// RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
 // RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp5 %s -Wuninitialized
 
-// RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
+// RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
 // RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp5 %s -Wuninitialized
 
 class S {
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
+// RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
 // RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp5 %s -Wuninitialized
 
-// RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
+// RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
 // RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp5 %s -Wuninitialized
 
 class S {
Index: clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
+// RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp4 %s -Wuninitialized
 // RUN: %clang_cc1 -fsyntax-only -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -fexceptions -fcxx-exceptions -verify=expected,omp5 %s -Wunini

[PATCH] D81388: [TEST] TreeTest.cpp - Add a comma to avoid build error with -werror

[Kan Shengchen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c63ea6eded3: [TEST] TreeTest.cpp - Add a comma to avoid 
build error with -werror (authored by skan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81388

Files:
  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
@@ -2995,6 +2995,6 @@
 }
 
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(TestClangConfig::allConfigs()));
+testing::ValuesIn(TestClangConfig::allConfigs()), );
 
 } // namespace


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2995,6 +2995,6 @@
 }
 
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(TestClangConfig::allConfigs()));
+testing::ValuesIn(TestClangConfig::allConfigs()), );
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2c63ea6 - [TEST] TreeTest.cpp - Add a comma to avoid build error with -werror

[Shengchen Kan via cfe-commits]

Author: Shengchen Kan
Date: 2020-06-08T23:14:16+08:00
New Revision: 2c63ea6eded3eed4018306363c28b7f63d7b5898

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

LOG: [TEST] TreeTest.cpp - Add a comma to avoid build error with -werror

Summary:
The macro `INSTANTIATE_TEST_CASE_P` is defined as
```
\# define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator, ...) \
...
```

If we build the test case with -werror, we will get an error like
```
error: ISO C++11 requires at least one argument for the "..." in a
variadic macro

testing::ValuesIn(TestClangConfig::allConfigs()));
^
```
This patch fixes that.

Reviewers: gribozavr, hlopko, eduucaldas, gribozavr2

Reviewed By: gribozavr2

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp 
b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index 49fdcb7a9c54..64acfe30a05b 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2995,6 +2995,6 @@ TEST_P(SyntaxTreeTest, SynthesizedNodes) {
 }
 
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, SyntaxTreeTest,
-testing::ValuesIn(TestClangConfig::allConfigs()));
+testing::ValuesIn(TestClangConfig::allConfigs()), );
 
 } // namespace



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

[PATCH] D81352: Thread safety analysis: Add note for double unlock

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
aaronpuchert marked an inline comment as done.
Closed by commit rGf70912f885f9: Thread safety analysis: Add note for double 
unlock (authored by aaronpuchert).

Changed prior to commit:
  https://reviews.llvm.org/D81352?vs=269080&id=269242#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81352

Files:
  clang/include/clang/Analysis/Analyses/ThreadSafety.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Analysis/ThreadSafety.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/Sema/warn-thread-safety-analysis.c
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2606,7 +2606,7 @@
 
 void Foo::test4() {
   ReleasableMutexLock rlock(&mu_);
-  rlock.Release();
+  rlock.Release();  // expected-note{{mutex released here}}
   rlock.Release();  // expected-warning {{releasing mutex 'mu_' that was not held}}
 }
 
@@ -2724,7 +2724,7 @@
 
 void doubleUnlock() {
   RelockableExclusiveMutexLock scope(&mu);
-  scope.Unlock();
+  scope.Unlock(); // expected-note{{mutex released here}}
   scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
 }
 
@@ -2885,7 +2885,7 @@
 }
 
 void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
-  MutexUnlock scope(&mu);
+  MutexUnlock scope(&mu); // expected-note{{mutex released here}}
   scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
 }
 
@@ -3200,7 +3200,7 @@
   mu_->Lock();  // expected-note 2 {{mutex acquired here}}
   mu_.get()->Lock();// expected-warning {{acquiring mutex 'mu_' that is already held}}
   (*mu_).Lock();// expected-warning {{acquiring mutex 'mu_' that is already held}}
-  mu_.get()->Unlock();
+  mu_.get()->Unlock();  // expected-note {{mutex released here}}
   Unlock(); // expected-warning {{releasing mutex 'mu_' that was not held}}
 }
 
@@ -,7 +,7 @@
 
   foo.lock(); // expected-note{{mutex acquired here}}
   foo.lock(); // expected-warning {{acquiring mutex 'foo' that is already held}}
-  foo.unlock();
+  foo.unlock();   // expected-note{{mutex released here}}
   foo.unlock();   // expected-warning {{releasing mutex 'foo' that was not held}}
 }
 
@@ -3347,7 +3347,7 @@
   foo.lock1();// expected-note{{mutex acquired here}}
   foo.lock1();// expected-warning {{acquiring mutex 'foo.mu1_' that is already held}}
   foo.a = 0;
-  foo.unlock1();
+  foo.unlock1();  // expected-note{{mutex released here}}
   foo.unlock1();  // expected-warning {{releasing mutex 'foo.mu1_' that was not held}}
 }
 
@@ -3361,7 +3361,7 @@
   foo.slock1();// expected-note{{mutex acquired here}}
   foo.slock1();// expected-warning {{acquiring mutex 'foo.mu1_' that is already held}}
   int d2 = foo.a;
-  foo.unlock1();
+  foo.unlock1();   // expected-note{{mutex released here}}
   foo.unlock1();   // expected-warning {{releasing mutex 'foo.mu1_' that was not held}}
   return d1 + d2;
 }
@@ -3383,7 +3383,7 @@
   foo.a = 0;
   foo.b = 0;
   foo.c = 0;
-  foo.unlock3();
+  foo.unlock3(); // expected-note 3 {{mutex released here}}
   foo.unlock3(); // \
 // expected-warning {{releasing mutex 'foo.mu1_' that was not held}} \
 // expected-warning {{releasing mutex 'foo.mu2_' that was not held}} \
@@ -3407,7 +3407,7 @@
   foo.a = 0;
   foo.b = 0;
   foo.c = 0;
-  foo.unlocklots();
+  foo.unlocklots(); // expected-note 3 {{mutex released here}}
   foo.unlocklots(); // \
 // expected-warning {{releasing mutex 'foo.mu1_' that was not held}} \
 // expected-warning {{releasing mutex 'foo.mu2_' that was not held}} \
Index: clang/test/Sema/warn-thread-safety-analysis.c
===
--- clang/test/Sema/warn-thread-safety-analysis.c
+++ clang/test/Sema/warn-thread-safety-analysis.c
@@ -119,10 +119,12 @@
 
   mutex_exclusive_lock(&mu1);// expected-note {{mutex acquired here}}
   mutex_shared_unlock(&mu1); // expected-warning {{releasing mutex 'mu1' using shared access, expected exclusive access}}
+ // expected-note@-1{{mutex released here}}
   mutex_exclusive_unlock(&mu1);  // expected-warning {{releasing mutex 'mu1' that was not held}}
 
   mutex_shared_lock(&mu1);  // expected-note {{mutex acquired here}}
   mutex_exclusive_unlock(&mu1); // expected-warning {{releasing mutex 'mu1' using exclusive access, expected shared access}}
+// expected-note@-1{{mutex released here}}
   mutex_shared_unlock(&mu1);// expected-warning {{releasing mutex 'mu1' that was not held}}
 
   return 0;
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp

[PATCH] D81332: Thread safety analysis: Support deferring locks

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1850f56c8aba: Thread safety analysis: Support deferring 
locks (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81332

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2625,9 +2625,12 @@
 
 namespace RelockableScopedLock {
 
+class DeferTraits {};
+
 class SCOPED_LOCKABLE RelockableExclusiveMutexLock {
 public:
   RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+  RelockableExclusiveMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu);
   ~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION();
 
   void Lock() EXCLUSIVE_LOCK_FUNCTION();
@@ -2639,6 +2642,7 @@
 
 class SCOPED_LOCKABLE RelockableMutexLock {
 public:
+  RelockableMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu);
   RelockableMutexLock(Mutex *mu, SharedTraits) SHARED_LOCK_FUNCTION(mu);
   RelockableMutexLock(Mutex *mu, ExclusiveTraits) EXCLUSIVE_LOCK_FUNCTION(mu);
   ~RelockableMutexLock() UNLOCK_FUNCTION();
@@ -2669,6 +2673,13 @@
   x = 4;
 }
 
+void deferLock() {
+  RelockableExclusiveMutexLock scope(&mu, DeferTraits{});
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+  scope.Lock();
+  x = 3;
+}
+
 void relockExclusive() {
   RelockableMutexLock scope(&mu, SharedTraits{});
   print(x);
@@ -2703,6 +2714,14 @@
   x = 5;
 }
 
+void deferLockShared() {
+  RelockableMutexLock scope(&mu, DeferTraits{});
+  print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
+  scope.ReaderLock();
+  print(x);
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
 void doubleUnlock() {
   RelockableExclusiveMutexLock scope(&mu);
   scope.Unlock();
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -905,11 +905,7 @@
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
   : FactEntry(CE, LK_Exclusive, Loc, false) {}
 
-  void addExclusiveLock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-  }
-
-  void addSharedLock(const CapabilityExpr &M) {
+  void addLock(const CapabilityExpr &M) {
 UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
   }
 
@@ -1801,7 +1797,7 @@
   SourceLocation Loc = Exp->getExprLoc();
   CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd;
   CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
-  CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
+  CapExprSet ScopedReqsAndExcludes;
   StringRef CapDiagKind = "mutex";
 
   // Figure out if we're constructing an object of scoped lockable class
@@ -1892,19 +1888,20 @@
  POK_FunctionCall, ClassifyDiagnostic(A),
  Exp->getExprLoc());
   // use for adopting a lock
-  if (isScopedVar) {
-Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
-: ScopedExclusiveReqs,
-  A, Exp, D, VD);
-  }
+  if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
 }
 break;
   }
 
   case attr::LocksExcluded: {
 const auto *A = cast(At);
-for (auto *Arg : A->args())
+for (auto *Arg : A->args()) {
   warnIfMutexHeld(D, Exp, Arg, ClassifyDiagnostic(A));
+  // use for deferring a lock
+  if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
+}
 break;
   }
 
@@ -1944,13 +1941,11 @@
 
 auto ScopedEntry = std::make_unique(Scp, MLoc);
 for (const auto &M : ExclusiveLocksToAdd)
-  ScopedEntry->addExclusiveLock(M);
-for (const auto &M : ScopedExclusiveReqs)
-  ScopedEntry->addExclusiveLock(M);
+  ScopedEntry->addLock(M);
 for (const auto &M : SharedLocksToAdd)
-  ScopedEntry->addSharedLock(M);
-for (const auto &M : ScopedSharedReqs)
-  ScopedEntry->addSharedLock(M);
+  ScopedEntry->addLock(M);
+for (const auto &M : ScopedReqsAndExcludes)
+  ScopedEntry->addLock(M);
 for (const auto &M : ExclusiveLocksToRemove)
   ScopedEntry->addExclusiveUnlock(M);
 for (const auto &M : SharedLocksToRemove)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls created this revision.
kristof.beyls added a reviewer: ostannard.
Herald added subscribers: cfe-commits, danielkiss.
Herald added a project: clang.

The accepted options to -mharden-sls= are:

- all: enable all mitigations against Straight Line Speculation that are 
implemented.
- none: disable all mitigations against Straight Line Speculation.
- retbr: enable the mitigation against Straight Line Speculation for RET and BR 
instructions.
- blr: enable the mitigation against Straight Line Speculation for BLR 
instructions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81404

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-sls-hardening-options.c

Index: clang/test/Driver/aarch64-sls-hardening-options.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-sls-hardening-options.c
@@ -0,0 +1,45 @@
+// Check the -mharden-sls= option, which has a required argument to select
+// scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=all 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr,retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr,r 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=none,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=all,-blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RETBR-OFF-NOT: "harden-sls-retbr"
+// RETBR-ON:  "+harden-sls-retbr"
+
+// BLR-OFF-NOT: "harden-sls-blr"
+// BLR-ON:  "+harden-sls-blr"
+
+// BAD-SLS-SPEC: invalid sls hardening option '{{[^']+}}' in '-mharden-sls=
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -218,6 +218,39 @@
   D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args);
   }
 
+  // Enable/disable straight line speculation hardening.
+  if (Arg *A = Args.getLastArg(options::OPT_mharden_sls_EQ)) {
+StringRef Scope = A->getValue();
+bool EnableRetBr = false;
+bool EnableBlr = false;
+if (Scope != "none" && Scope != "all") {
+  SmallVector Opts;
+  Scope.split(Opts, ",");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+StringRef Opt = Opts[I].trim();
+if (Opt == "retbr") {
+  EnableRetBr = true;
+  continue;
+}
+if (Opt == "blr") {
+  EnableBlr = true;
+  continue;
+}
+D.Diag(diag::err_invalid_sls_hardening)
+<< Scope << A->getAsString(Args);
+break;
+  }
+} else if (Scope == "all") {
+  EnableRetBr = true;
+  EnableBlr = true;
+}
+
+if (EnableRetBr)
+  Features.push_back("+harden-sls-retbr");
+if (EnableBlr)
+  Features.push_back("+harden-sls-blr");
+  }
+
   // En/disable crc
   if (Arg *A = Args.getLastArg(options::OPT_mcrc, options::OPT_mnocrc)) {
 if (A->getOption().matches(options::OPT_mcrc))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2302,6 +2302,9 @@
 def mbranch_protection_EQ : Joined<["-"], "mbranch-protection=">,
   HelpText<"Enforce targets of indirect branches and function returns">;
 
+def mharden_sls_EQ : Joined<["-"], "mharden-sls=">,
+  HelpText<"Sele

[clang-tools-extra] 806342b - [clangd] Resolve driver symlinks, and look up unknown relative drivers in PATH.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-06-08T17:24:52+02:00
New Revision: 806342b8ef54ec07511d0ce5d3d1335451e952da

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

LOG: [clangd] Resolve driver symlinks, and look up unknown relative drivers in 
PATH.

Summary:
This fixes a reported bug: if clang and libc++ are installed under
/usr/lib/llvm-11/...  but there'- a symlink /usr/bin/clang++-11, then a
compile_commands.json with "/usr/bin/clang++-11 -stdlib=libc++" would previously
look for libc++ under /usr/include instead of /usr/lib/llvm-11/include.
The PATH change makes this work if the compiler is just "clang++-11" too.

As this is now doing IO potentially on every getCompileCommand(), we cache
the results for each distinct driver.

While here:
- Added a Memoize helper for this as multithreaded caching is a bit noisy.
- Used this helper to simplify QueryDriverDatabase and reduce blocking there.
  (This makes use of the fact that llvm::Regex is now threadsafe)

Reviewers: kadircet

Subscribers: jyknight, ormris, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, 
usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/support/Threading.h
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index b3b2bdd976bf..db9932a43e5a 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -514,7 +514,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   if (ClangdServerOpts.ResourceDir)
 Mangler.ResourceDir = *ClangdServerOpts.ResourceDir;
   CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
-  tooling::ArgumentsAdjuster(Mangler));
+  tooling::ArgumentsAdjuster(std::move(Mangler)));
   {
 // Switch caller's context with LSPServer's background context. Since we
 // rather want to propagate information from LSPServer's context into the

diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 84f72f5f58c7..a73312a521e8 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -119,6 +119,48 @@ std::string detectStandardResourceDir() {
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
 }
 
+// The path passed to argv[0] is important:
+//  - its parent directory is Driver::Dir, used for library discovery
+//  - its basename affects CLI parsing (clang-cl) and other settings
+// Where possible it should be an absolute path with sensible directory, but
+// with the original basename.
+static std::string resolveDriver(llvm::StringRef Driver, bool FollowSymlink,
+ llvm::Optional ClangPath) {
+  auto SiblingOf = [&](llvm::StringRef AbsPath) {
+llvm::SmallString<128> Result = llvm::sys::path::parent_path(AbsPath);
+llvm::sys::path::append(Result, llvm::sys::path::filename(Driver));
+return Result.str().str();
+  };
+
+  // First, eliminate relative paths.
+  std::string Storage;
+  if (!llvm::sys::path::is_absolute(Driver)) {
+// If the driver is a generic like "g++" with no path, add clang dir.
+if (ClangPath &&
+(Driver == "clang" || Driver == "clang++" || Driver == "gcc" ||
+ Driver == "g++" || Driver == "cc" || Driver == "c++")) {
+  return SiblingOf(*ClangPath);
+}
+// Otherwise try to look it up on PATH. This won't change basename.
+auto Absolute = llvm::sys::findProgramByName(Driver);
+if (Absolute && llvm::sys::path::is_absolute(*Absolute))
+  Driver = Storage = std::move(*Absolute);
+else if (ClangPath) // If we don't find it, use clang dir again.
+  return SiblingOf(*ClangPath);
+else // Nothing to do: can't find the command and no detected dir.
+  return Driver.str();
+  }
+
+  // Now we have an absolute path, but it may be a symlink.
+  assert(llvm::sys::path::is_absolute(Driver));
+  if (FollowSymlink) {
+llvm::SmallString<256> Resolved;
+if (!llvm::sys::fs::real_path(Driver, Resolved))
+  return SiblingOf(Resolved);
+  }
+  return Driver.str();
+}
+
 } // namespace
 
 CommandMangler CommandMangler::detect() {
@@ -162,25 +204,22 @@ void CommandMangler::adjust(std::vector 
&Cmd) const {
 Cmd.push_back(*Sysroot);
   }
 
-  // If the driver is a g

[clang] 936ec89 - [AST] Fix a clang crash on an invalid for-range statement.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-06-08T17:32:10+02:00
New Revision: 936ec89e91e2dda8b6110b1fd1f9920509d7a17b

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

LOG: [AST] Fix a clang crash on an invalid for-range statement.

Summary:
crash stack:

```
llvm-project/clang/lib/AST/ASTContext.cpp:2248: clang::TypeInfo 
clang::ASTContext::getTypeInfoImpl(const clang::Type *) const: Assertion 
`!A->getDeducedType().isNull() && "cannot request the size of an undeduced or 
dependent auto type"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
Stack dump:
 #0 0x025bb0bf llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
llvm-project/llvm/lib/Support/Unix/Signals.inc:564:13
 #1 0x025b92b0 llvm::sys::RunSignalHandlers() 
llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x025bb535 SignalHandler(int) 
llvm-project/llvm/lib/Support/Unix/Signals.inc:396:3
 #3 0x7f9ef9298110 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14110)
 #4 0x7f9ef8d72761 raise 
/build/glibc-M65Gwz/glibc-2.30/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x7f9ef8d5c55b abort /build/glibc-M65Gwz/glibc-2.30/stdlib/abort.c:81:7
 #6 0x7f9ef8d5c42f get_sysdep_segment_value 
/build/glibc-M65Gwz/glibc-2.30/intl/loadmsgcat.c:509:8
 #7 0x7f9ef8d5c42f _nl_load_domain 
/build/glibc-M65Gwz/glibc-2.30/intl/loadmsgcat.c:970:34
 #8 0x7f9ef8d6b092 (/lib/x86_64-linux-gnu/libc.so.6+0x34092)
 #9 0x0458abe0 clang::ASTContext::getTypeInfoImpl(clang::Type const*) 
const llvm-project/clang/lib/AST/ASTContext.cpp:0:5
```

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 
clang/test/SemaCXX/for-range-crash.cpp

Modified: 
clang/lib/Sema/SemaStmt.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index dda0d3486e0e..8d32fc094494 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2434,8 +2434,11 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation 
ForLoc,
 QualType RangeType = Range->getType();
 
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())
+LoopVar->setInvalidDecl();
   return StmtError();
+}
 
 // Build auto __begin = begin-expr, __end = end-expr.
 // Divide by 2, since the variables are in the inner scope (loop body).

diff  --git a/clang/test/SemaCXX/for-range-crash.cpp 
b/clang/test/SemaCXX/for-range-crash.cpp
new file mode 100644
index ..21637264a2bf
--- /dev/null
+++ b/clang/test/SemaCXX/for-range-crash.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template 
+class Bar {
+  Bar *variables_to_modify;
+  foo() { // expected-error {{C++ requires a type specifier for all 
declarations}}
+for (auto *c : *variables_to_modify)
+  delete c;
+  }
+};



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

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

LG, thanks!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5199
+  // Objective-C property reference. Bail if we're performing fix-it code
+  // completion since we don't handle forwarding the fix-it for Objective-C
+  // objects. Since Objective-C properties are normally backed by ivars,

I don't really follow what the comment means by "forwarding the fixit". As I 
understood it, the issue is -> and . are likely both valid, so changing one to 
the other is confusing. Might be able to reword to clarify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81263



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

[PATCH] D72781: [Matrix] Add __builtin_matrix_column_load to Clang.

[Florian Hahn via Phabricator via cfe-commits]

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

Ping.

Applied feedback from D72778  to this patch, 
improved tests, support conversions/placeholders.

One thing I am not sure is how to properly handle template substitutions for 
the pointer expression for code like the one below, where we need to apply 
substitutions to get the actual pointer type. Currently the patch looks through 
SubstTemplateTypeParmType types in Sema to construct the result type. Should we 
look through SubstTemplateTypeParmType in IRGen too to decide whether to call 
EmitPointerWithAlignment or EmitArrayToPointerDecay? Or is there a place in 
sema that should get rid of the substitution (perhaps in SemaChecking.cpp)?

  template  struct remove_pointer {
  typedef T type;
  };
  
  template  struct remove_pointer{
  typedef typename remove_pointer::type type;
  };
  
  // Same as column_major_load_with_stride, but with the PtrT argument itself 
begin a pointer type.
  template 
  matrix_t::type, R, C> 
column_major_load_with_stride2(PtrT Ptr) {
  return __builtin_matrix_column_major_load(Ptr, R, C, S);
  }
  
  void call_column_major_load_with_stride2(float *Ptr) {
  matrix_t m = column_major_load_with_stride2(Ptr);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72781

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/matrix-type-builtins.c
  clang/test/CodeGenCXX/matrix-type-builtins.cpp
  clang/test/CodeGenObjC/matrix-type-builtins.m
  clang/test/Sema/matrix-type-builtins.c
  clang/test/SemaCXX/matrix-type-builtins.cpp

Index: clang/test/SemaCXX/matrix-type-builtins.cpp
===
--- clang/test/SemaCXX/matrix-type-builtins.cpp
+++ clang/test/SemaCXX/matrix-type-builtins.cpp
@@ -39,3 +39,65 @@
   Mat3.value = transpose(Mat2);
   // expected-note@-1 {{in instantiation of function template specialization 'transpose' requested here}}
 }
+
+template 
+typename MyMatrix::matrix_t column_major_load(MyMatrix &A, EltTy0 *Ptr) {
+  char *v1 = __builtin_matrix_column_major_load(Ptr, 9, 4, 10);
+  // expected-error@-1 {{cannot initialize a variable of type 'char *' with an rvalue of type 'unsigned int __attribute__((matrix_type(9, 4)))'}}
+  // expected-error@-2 {{cannot initialize a variable of type 'char *' with an rvalue of type 'unsigned int __attribute__((matrix_type(9, 4)))'}}
+  // expected-error@-3 {{cannot initialize a variable of type 'char *' with an rvalue of type 'float __attribute__((matrix_type(9, 4)))'}}
+
+  return __builtin_matrix_column_major_load(Ptr, R0, C0, R0);
+  // expected-error@-1 {{cannot initialize return object of type 'typename MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') with an rvalue of type 'unsigned int __attribute__((matrix_type(2, 3)))'}}
+  // expected-error@-2 {{cannot initialize return object of type 'typename MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 3)))') with an rvalue of type 'float __attribute__((matrix_type(2, 3)))'}}
+}
+
+void test_column_major_loads_template(unsigned *Ptr1, float *Ptr2) {
+  MyMatrix Mat1;
+  Mat1.value = column_major_load(Mat1, Ptr1);
+  // expected-note@-1 {{in instantiation of function template specialization 'column_major_load' requested here}}
+  column_major_load(Mat1, Ptr1);
+  // expected-note@-1 {{in instantiation of function template specialization 'column_major_load' requested here}}
+
+  MyMatrix Mat2;
+  Mat1.value = column_major_load(Mat2, Ptr2);
+  // expected-note@-1 {{in instantiation of function template specialization 'column_major_load' requested here}}
+}
+
+constexpr int constexpr1() { return 1; }
+constexpr int constexpr_neg1() { return -1; }
+
+void test_column_major_load_constexpr(unsigned *Ptr) {
+  (void)__builtin_matrix_column_major_load(Ptr, 2, 2, constexpr1());
+  // expected-error@-1 {{stride must be greater or equal to the number of rows}}
+  (void)__builtin_matrix_column_major_load(Ptr, constexpr_neg1(), 2, 4);
+  // expected-error@-1 {{row dimension is outside the allowed range [1, 1048575]}}
+  (void)__builtin_matrix_column_major_load(Ptr, 2, constexpr_neg1(), 4);
+  // expected-error@-1 {{column dimension is outside the allowed range [1, 1048575]}}
+}
+
+struct IntWrapper {
+  operator int() {
+return 1;
+  }
+};
+
+void test_column_major_load_wrapper(unsigned *Ptr, IntWrapper &W) {
+  (void)__builtin_matrix_column_major_load(Ptr, W, 2, 2);
+  // expected-error@-1 {{row argument must be a constant unsigned integer expression}}
+  (void)__builtin_matrix_column_major_load(Ptr, 2, W, 2);
+  // expected-error@-1 {{column argument must be a constant unsigned

[PATCH] D81384: [AST] Fix a clang crash on an invalid for-range statement.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/for-range-crash.cpp:6
+  Bar *variables_to_modify;
+   foo() {  // expected-error {{C++ requires a type specifier for all 
declarations}}
+for (auto *c : *variables_to_modify)

sammccall wrote:
> What's the significance of the missing return type here? Can we add void)
> 
> Can we move this function out of the class?
> 
> (In both cases just to remove any confusion about what the problematic 
> pattern actually is)
I tried both at the very beginning and failed, the crash needs both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81384



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

[PATCH] D81098: [OpenMP] Upgrade default version of OpenMP to 5.0

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Need to update few other places.
CompilerInvocation.cpp:3082 and CompilerInvocation.cpp:3085, which sets the 
default version to 45 in the presense of the target option or simd option. Most 
probably, need to remove this check. It was used to use 45 as the default 
version for target(simd)-based switches when the default version was 40.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81098



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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

patch for gcc: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547404.html


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

https://reviews.llvm.org/D75044



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

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.cpp:59
+
+/// Return whether `Var` has a pointer or reference in `Func`.
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) {

Please don't duplicate the comment from the header in the cpp file.



Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.h:18
+
+/// Returns whether a ``Var`` has a pointer or reference in ``Func``
+bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var);

Could you expand the comment? Best if you could add an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81396



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

[PATCH] D75414: [clangd] Resolve driver symlinks, and look up unknown relative drivers in PATH.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG806342b8ef54: [clangd] Resolve driver symlinks, and look up 
unknown relative drivers in PATH. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D75414?vs=266861&id=269258#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75414

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/support/Threading.h
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp

Index: clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp
===
--- clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp
+++ clang-tools-extra/clangd/unittests/support/ThreadingTests.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "support/Threading.h"
+#include "llvm/ADT/DenseMap.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -60,5 +62,64 @@
   std::lock_guard Lock(Mutex);
   ASSERT_EQ(Counter, TasksCnt * IncrementsPerTask);
 }
+
+TEST_F(ThreadingTest, Memoize) {
+  const unsigned NumThreads = 5;
+  const unsigned NumKeys = 100;
+  const unsigned NumIterations = 100;
+
+  Memoize> Cache;
+  std::atomic ComputeCount(0);
+  std::atomic ComputeResult[NumKeys];
+  std::fill(std::begin(ComputeResult), std::end(ComputeResult), -1);
+
+  AsyncTaskRunner Tasks;
+  for (unsigned I = 0; I < NumThreads; ++I)
+Tasks.runAsync("worker" + std::to_string(I), [&] {
+  for (unsigned J = 0; J < NumIterations; J++)
+for (unsigned K = 0; K < NumKeys; K++) {
+  int Result = Cache.get(K, [&] { return ++ComputeCount; });
+  EXPECT_THAT(ComputeResult[K].exchange(Result),
+  testing::AnyOf(-1, Result))
+  << "Got inconsistent results from memoize";
+}
+});
+  Tasks.wait();
+  EXPECT_GE(ComputeCount, NumKeys) << "Computed each key once";
+  EXPECT_LE(ComputeCount, NumThreads * NumKeys)
+  << "Worst case, computed each key in every thread";
+  for (int Result : ComputeResult)
+EXPECT_GT(Result, 0) << "All results in expected domain";
+}
+
+TEST_F(ThreadingTest, MemoizeDeterministic) {
+  Memoize> Cache;
+
+  // Spawn two parallel computations, A and B.
+  // Force concurrency: neither can finish until both have started.
+  // Verify that cache returns consistent results.
+  AsyncTaskRunner Tasks;
+  std::atomic ValueA(0), ValueB(0);
+  Notification ReleaseA, ReleaseB;
+  Tasks.runAsync("A", [&] {
+ValueA = Cache.get(0, [&] {
+  ReleaseB.notify();
+  ReleaseA.wait();
+  return 'A';
+});
+  });
+  Tasks.runAsync("A", [&] {
+ValueB = Cache.get(0, [&] {
+  ReleaseA.notify();
+  ReleaseB.wait();
+  return 'B';
+});
+  });
+  Tasks.wait();
+
+  ASSERT_EQ(ValueA, ValueB);
+  ASSERT_THAT(ValueA.load(), testing::AnyOf('A', 'B'));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -9,7 +9,11 @@
 #include "CompileCommands.h"
 #include "TestFS.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -103,12 +107,81 @@
 
   Cmd = {"unknown-binary", "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_EQ("unknown-binary", Cmd.front());
+  EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());
 
   Cmd = {testPath("path/clang++"), "foo.cc"};
   Mangler.adjust(Cmd);
   EXPECT_EQ(testPath("path/clang++"), Cmd.front());
+
+  Cmd = {"foo/unknown-binary", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());
+}
+
+// Only run the PATH/symlink resolving test on unix, we need to fiddle
+// with permissions and environment variables...
+#ifdef LLVM_ON_UNIX
+MATCHER(Ok, "") {
+  if (arg) {
+*result_listener << arg.message();
+return false;
+  }
+  return true;
+}
+
+TEST(CommandMangler, ClangPathResolve) {
+  // Set up filesystem:
+  //   /temp/
+  // bin/
+  //   foo -> temp/lib/bar
+  // lib/
+  //   bar
+  llvm::SmallString<256> TempDir;
+  ASSERT_THAT(llvm::sys::fs::createUniqueDirectory("ClangPathResolve", TempDir),
+  Ok());
+  auto CleanDir = llvm::make_scope_exit(
+  [&] { llvm::sys::fs::remove_directories(TempDir); });
+  ASSERT_THAT(llvm::sys::fs::create_d

[PATCH] D81384: [AST] Fix a clang crash on an invalid for-range statement.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rG936ec89e91e2: [AST] Fix a clang crash on an invalid 
for-range statement. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D81384?vs=269188&id=269259#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81384

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/for-range-crash.cpp


Index: clang/test/SemaCXX/for-range-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/for-range-crash.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template 
+class Bar {
+  Bar *variables_to_modify;
+  foo() { // expected-error {{C++ requires a type specifier for all 
declarations}}
+for (auto *c : *variables_to_modify)
+  delete c;
+  }
+};
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2434,8 +2434,11 @@
 QualType RangeType = Range->getType();
 
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())
+LoopVar->setInvalidDecl();
   return StmtError();
+}
 
 // Build auto __begin = begin-expr, __end = end-expr.
 // Divide by 2, since the variables are in the inner scope (loop body).


Index: clang/test/SemaCXX/for-range-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/for-range-crash.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template 
+class Bar {
+  Bar *variables_to_modify;
+  foo() { // expected-error {{C++ requires a type specifier for all declarations}}
+for (auto *c : *variables_to_modify)
+  delete c;
+  }
+};
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2434,8 +2434,11 @@
 QualType RangeType = Range->getType();
 
 if (RequireCompleteType(RangeLoc, RangeType,
-diag::err_for_range_incomplete_type))
+diag::err_for_range_incomplete_type)) {
+  if (LoopVar->getType()->isUndeducedType())
+LoopVar->setInvalidDecl();
   return StmtError();
+}
 
 // Build auto __begin = begin-expr, __end = end-expr.
 // Divide by 2, since the variables are in the inner scope (loop body).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Bug reports of resource leak are now improved.
If there are multiple resource leak paths for the same stream,
only one wil be reported.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81407

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-note.c
  clang/test/Analysis/stream.c

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -139,7 +139,7 @@
   if (!p)
 return;
   if(c)
-return; // expected-warning {{Opened File never closed. Potential Resource leak}}
+return; // expected-warning {{Opened stream never closed. Potential resource leak}}
   fclose(p);
 }
 
Index: clang/test/Analysis/stream-note.c
===
--- /dev/null
+++ clang/test/Analysis/stream-note.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void check_note_at_correct_open() {
+  FILE *F1 = tmpfile(); // expected-note {{Stream opened here}}
+  if (!F1)
+// expected-note@-1 {{'F1' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  FILE *F2 = tmpfile();
+  if (!F2) {
+// expected-note@-1 {{'F2' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+fclose(F1);
+return;
+  }
+  rewind(F2);
+  fclose(F2);
+  rewind(F1);
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_fopen() {
+  FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  if (!F)
+// expected-note@-1 {{'F' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
+
+void check_note_freopen() {
+  FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  if (!F)
+// expected-note@-1 {{'F' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  F = freopen(0, "w", F); // expected-note {{Stream reopened here}}
+  if (!F)
+// expected-note@-1 {{'F' is non-null}}
+// expected-note@-2 {{Taking false branch}}
+return;
+}
+// expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
+// expected-note@-2 {{Opened stream never closed. Potential resource leak}}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -216,8 +216,8 @@
   "Read function called when stream is in EOF state. "
   "Function has no effect."};
   BuiltinBug BT_ResourceLeak{
-  this, "Resource Leak",
-  "Opened File never closed. Potential Resource leak."};
+  this, "Resource leak",
+  "Opened stream never closed. Potential resource leak."};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -365,6 +365,21 @@
 
 return FnDescriptions.lookup(Call);
   }
+
+  static const ExplodedNode *getAcquireSite(const ExplodedNode *N,
+SymbolRef Sym, CheckerContext &Ctx);
+};
+
+struct NoteFn {
+  SymbolRef StreamSym;
+  std::string Message;
+
+  std::string operator()(PathSensitiveBugReport &BR) const {
+if (BR.isInteresting(StreamSym))
+  return Message;
+
+return "";
+  }
 };
 
 } // end anonymous namespace
@@ -376,6 +391,26 @@
  "Previous create of error node for non-opened stream failed?");
 }
 
+const ExplodedNode *StreamChecker::getAcquireSite(const ExplodedNode *N,
+  SymbolRef Sym,
+  CheckerContext &Ctx) {
+  ProgramStateRef State = N->getState();
+  // When bug type is resource leak, exploded node N may not have state info
+  // for leaked file descriptor, but predecessor should have it.
+  if (!State->get(Sym))
+N = N->getFirstPred();
+
+  const ExplodedNode *Pred = N;
+  while (N) {
+State = N->getState();
+if (!State->get(Sym))
+  return Pred;
+Pred = N;
+N = N->getFirstPred();
+  }
+  return nullptr;
+}
+
 void StreamChecker::checkPreCall(const CallEvent &Call,
  CheckerContext &C) const {
   const FnDescription *Desc =

[PATCH] D81315: [analyzer][Draft] [Prototype] warning for default constructed unique pointer dereferences

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Great start!
I think you are on the right track, so maybe this code won't be thrown away at 
all :-)
Try to work on tests and get familiar with `lit`.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:31
 namespace {
-class SmartPtrModeling : public Checker {
+struct RegionState {
+private:

xazax.hun wrote:
> I think `RegionState` is not very descriptive. I'd call it something like 
> `RegionNullness`.
linter: LLVM coding standards require to use `class` keyword in situations like 
this 
(https://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords).  
I would even say that `struct` is good for POD types.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:33
+private:
+  enum Kind { Null, NonNull, Unknown } K;
+  RegionState(Kind InK) : K(InK) {}

I think that it would be better to put declarations for `enum` and for a field 
separately.
Additionally, I don't think that `K` is a very good name for a data member.  It 
should be evident from the name of the member what is it.  Shot names like that 
can be fine only for iterators or for certain `clang`-specific structures 
because of existing traditions (like `SM` for `SourceManager` and `LO` for 
`LanguageOptions`).



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:119
+  ProgramStateRef State = C.getState();
+  const auto OC = dyn_cast(&Call);
+  if (!OC)

xazax.hun wrote:
> Here the const applies for the pointer, not the pointee. We usually do `const 
> auto *OC` instead.
As I said above, I think we should be really careful about abbreviated and 
extremely short variable names.  Longer names make it much easier to read the 
code without paying a lot of attention to declarations.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:160-187
+  if (const auto IC = dyn_cast(&Call)) {
+const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
+if (!MethodDecl)
+  return;
+auto ArgsNum = IC->getNumArgs();
+
+if (ArgsNum == 0 && isResetMethod(MethodDecl)) {

Considering the fact that more cases and more functions will get supported in 
the future, I vote for merging common parts of these two blocks.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:230
+
+bool SmartPtrModeling::isResetMethod(const CXXMethodDecl *MethodDec) const {
+  if (!MethodDec)

I believe that methods (and data members related to them) can be `static`.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:233
+return false;
+  if (MethodDec->getDeclName().isIdentifier()) {
+return ResetMethods.count(MethodDec->getName().lower());

I'm not sure about it myself, but can `DeclName` be `isEmpty()`?  If yes, it is 
a potential null-pointer dereference.  Again, I don't know it for a fact, but I 
think it should be checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81315



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

[clang] 8b529e3 - [ObjC] Fix AST serialization for pseudo-strong parameters

[Erik Pilkington via cfe-commits]

Author: David Goldman
Date: 2020-06-08T11:54:01-04:00
New Revision: 8b529e311a9052ee7a0676a1b517728efa44a3ba

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

LOG: [ObjC] Fix AST serialization for pseudo-strong parameters

This bit was assumed to be always false for ParmVarDecls, but attribute
objc_externally_retained now can produce it.

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

Added: 
clang/test/PCH/externally-retained.m

Modified: 
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 8c5be6cacac0..a28a21b29339 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1100,8 +1100,6 @@ void ASTDeclWriter::VisitParmVarDecl(ParmVarDecl *D) {
 Record.AddStmt(D->getUninstantiatedDefaultArg());
   Code = serialization::DECL_PARM_VAR;
 
-  assert(!D->isARCPseudoStrong()); // can be true of ImplicitParamDecl
-
   // If the assumptions about the DECL_PARM_VAR abbrev are true, use it.  Here
   // we dynamically check for the properties that we optimize for, but don't
   // know are true of all PARM_VAR_DECLs.
@@ -2121,7 +2119,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));   // SClass
   Abv->Add(BitCodeAbbrevOp(0));   // TSCSpec
   Abv->Add(BitCodeAbbrevOp(0));   // InitStyle
-  Abv->Add(BitCodeAbbrevOp(0));   // ARCPseudoStrong
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(0));   // Linkage
   Abv->Add(BitCodeAbbrevOp(0));   // HasInit
   Abv->Add(BitCodeAbbrevOp(0));   // 
HasMemberSpecializationInfo

diff  --git a/clang/test/PCH/externally-retained.m 
b/clang/test/PCH/externally-retained.m
new file mode 100644
index ..6a1debf8f467
--- /dev/null
+++ b/clang/test/PCH/externally-retained.m
@@ -0,0 +1,30 @@
+// Test for assertion failure due to objc_externally_retained on a function.
+
+// Without PCH
+// RUN: %clang_cc1 -fsyntax-only -verify -fobjc-arc -include %s %s
+
+// With PCH
+// RUN: %clang_cc1 %s -emit-pch -fobjc-arc -o %t
+// RUN: %clang_cc1 -emit-llvm-only -verify %s -fobjc-arc -include-pch %t 
-debug-info-kind=limited
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+//===--===//
+// Header
+
+__attribute__((objc_externally_retained)) void doSomething(id someObject);
+
+id sharedObject = 0;
+
+//===--===//
+#else
+//===--===//
+
+void callDoSomething() {
+  doSomething(sharedObject);
+}
+
+//===--===//
+#endif



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